Client #128: UX hardening — errors, toasts, validation, and loading states#133
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR hardens the web client with field-level validation, standardized error notices and toast feedback, coordinated query-cache settlement, improved dialogs and navigation, expanded E2E consistency coverage, updated documentation, and revised Keycloak login theme styling. ChangesClient UX hardening
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web-client/src/features/organization/components/TeamEditorDialog.tsx (1)
295-346: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winShow hidden-step field errors on the active step. A server-side validation error for a
detailsfield can land while the wizard is onmembers, but those inputs aren’t rendered there, so the user gets no visible feedback. Snap back todetailswhen the error map includes one of its fields.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-client/src/features/organization/components/TeamEditorDialog.tsx` around lines 295 - 346, Update the error handling in handleSubmit so that when formMutationErrorFields returns errors for a details field (name, sportId/sport, description, or address), set the wizard step to the details step before displaying the errors. Preserve the existing error state behavior for member-field errors and use the existing steps/stepIndex flow to locate the details step.web-client/src/features/feedback/model/useFeedbackViewModel.ts (1)
86-96: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winComment describes
rating, but now sits aboveerror.The inline comment about ratings always being present on loaded feedback is positioned above
error: Error | null, making it read as documentation forerror's semantics instead ofrating's.📝 Proposed fix
export interface FeedbackDetailView { detail: Feedback | undefined eventName: string | undefined memberName: string | undefined creatorName: string | undefined - rating: number | undefined - isLoading: boolean - // Loaded feedback always has a rating; undefined means detail is still unavailable. + // Loaded feedback always has a rating; undefined means detail is still unavailable. + rating: number | undefined + isLoading: boolean error: Error | null refetch: () => void }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-client/src/features/feedback/model/useFeedbackViewModel.ts` around lines 86 - 96, Move the inline comment in the FeedbackDetailView interface so it directly precedes the rating property, documenting rating’s undefined semantics rather than error. Keep the property types and interface structure unchanged.
🧹 Nitpick comments (7)
web-client/e2e/payments.spec.ts (1)
61-62: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using
toastRegionfor consistency.Other end-to-end tests use the newly introduced
toastRegion(page)helper to verify toast notifications. Using it here keeps the test assertions consistent across the suite.
web-client/e2e/payments.spec.ts#L61-L62: Useawait expect(toastRegion(page)).toContainText(...)instead of checking the text globally on the page.web-client/e2e/payments.spec.ts#L99-L99: Useawait expect(toastRegion(page)).toContainText(...)instead of checking the text globally on the page.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-client/e2e/payments.spec.ts` around lines 61 - 62, Update both toast assertions in web-client/e2e/payments.spec.ts at lines 61-62 and 99 to use toastRegion(page) with toContainText(paymentMember.name’s transaction message), replacing the global getByText visibility checks while preserving the surrounding test flow.web-client/src/components/ui/input.tsx (1)
5-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRefactor to use
refas a standard prop (React 19).In React 19,
refis passed as a regular prop to functional components, makingReact.forwardRefunnecessary. You can simplify the component definition by accessingrefdirectly from the props object.[source_library_documentation]
♻️ Proposed refactor
-const Input = React.forwardRef<HTMLInputElement, React.ComponentProps<"input">>( - function Input({ className, type, ...props }, ref) { +const Input = function Input({ className, type, ref, ...props }: React.ComponentProps<"input">) { return ( <input ref={ref} type={type} data-slot="input" className={cn( "h-10 w-full min-w-0 border border-input bg-transparent px-3 py-1 text-base transition-[color,border-color] outline-none file:inline-flex file:h-7 file:border-0 file:bg-transparent file:text-sm file:font-medium file:text-foreground placeholder:text-text-tertiary focus-visible:border-ring disabled:pointer-events-none disabled:cursor-not-allowed disabled:opacity-50 aria-invalid:border-destructive md:text-sm dark:aria-invalid:border-destructive/50", className )} {...props} /> ) } -)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-client/src/components/ui/input.tsx` around lines 5 - 20, Refactor the Input component to remove React.forwardRef and accept ref directly as a standard prop in its function parameters, while preserving the existing input attributes, className handling, and ref forwarding to the underlying <input>.web-client/src/lib/server-error.ts (1)
93-101: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winExact-string match on
'Validation failed'is a fragile way to decide message trust for 400s.For non-401/non-5xx statuses (including 400), any backend
messageis shown verbatim unless it's the exact literal'Validation failed'. If Spring's default validation-exception wording ever changes, this filter silently stops working and a generic/internal message could leak straight to users instead of falling back to the safe copy. Since field-level detail is already surfaced viafieldErrorsfor 400s, consider preferring the safe copy wheneverfieldErrorswas extracted, rather than string-matching the boilerplate.♻️ Possible alternative
- const trustServerMessage = - status !== 401 && status < 500 && serverMessage !== 'Validation failed' + const trustServerMessage = + status !== 401 && status < 500 && !(status === 400 && fieldErrors)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-client/src/lib/server-error.ts` around lines 93 - 101, Update the trustServerMessage decision in the server-error message construction to reject serverMessage whenever fieldErrors were extracted, instead of relying on the exact 'Validation failed' string. Preserve the existing 401 and 5xx protections and allow other 4xx messages only when no field-level errors are present.web-client/e2e/cache-consistency.spec.ts (1)
55-63: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove unnecessary and partial string replacement in
RegExp.The string replacement
.replace('/', '\\/')is unnecessary because forward slashes do not need to be escaped when using thenew RegExp(...)constructor with a string. Furthermore, because it uses a string search instead of a global regex (/\//g), it would only replace the first slash if multiple were present.♻️ Proposed refactor
async function navigateTo(page: Page, path: string) { const navItem = fixtureOrThrow( NAV_ITEMS.find((item) => item.to === path), `navigation item for ${path}`, ) await page.getByRole('link', { name: navItem.label, exact: true }).click() - await expect(page).toHaveURL(new RegExp(`${path.replace('/', '\\/')}$`)) + await expect(page).toHaveURL(new RegExp(`${path}$`)) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-client/e2e/cache-consistency.spec.ts` around lines 55 - 63, Update navigateTo so the URL regular expression uses path directly without the unnecessary .replace('/', '\\/') call. Preserve the existing end-of-string anchor and URL assertion behavior.web-client/src/features/payments/model/transactionEditor.ts (1)
12-19: 📐 Maintainability & Code Quality | 🔵 TrivialZod 4:
messageparam is deprecated in favor oferror.Verified against the Zod v4 migration guide:
messageis still supported but deprecated;erroris the recommended replacement.♻️ Optional migration to Zod 4 `error` param
const transactionCreateSchema = z.object({ - memberId: z.string().trim().min(1, { message: 'Select a member.' }), + memberId: z.string().trim().min(1, { error: 'Select a member.' }), amount: z.string().refine((value) => parseEuroAmountCents(value) !== null, { - message: 'Enter an amount in euros with up to two decimals.', + error: 'Enter an amount in euros with up to two decimals.', }), - title: z.string().trim().min(1, { message: 'Title is required.' }), + title: z.string().trim().min(1, { error: 'Title is required.' }), description: z.string(), })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-client/src/features/payments/model/transactionEditor.ts` around lines 12 - 19, Update the validation options in transactionCreateSchema to use Zod 4’s error parameter instead of the deprecated message parameter for memberId, amount, and title. Preserve the existing validation rules and user-facing error text.web-client/src/app/pages/RouteErrorPage.tsx (1)
12-16: 🩺 Stability & Availability | 🔵 TrivialNo production-time error reporting for route crashes.
Logging is gated to
import.meta.env.DEV, so in production a routed crash is only ever shown to the user — nothing is sent to logs or an error-tracking service, making these failures invisible to the team.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-client/src/app/pages/RouteErrorPage.tsx` around lines 12 - 16, Update the error handling in RouteErrorPage’s useEffect so route/render errors are reported in production as well as development, using the application’s existing logging or error-tracking mechanism rather than gating all reporting behind import.meta.env.DEV. Preserve the current error context and avoid exposing development-only console behavior as the sole production path.web-client/src/lib/query-cache.test.ts (1)
1-78: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLGTM! Good coverage of the list-patch-before-refetch ordering, the single-refetch-for-overlapping-keys behavior, and the mutation-only-cache exclusion from refetch. Consider adding a case where
refetchQueriesrejects, to pin down the behavior flagged inquery-cache.ts(whether the rejection currently propagates out ofsettleMutation).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-client/src/lib/query-cache.test.ts` around lines 1 - 78, Add a test in the settleMutation suite that makes refetchQueries reject and asserts whether settleMutation propagates that rejection, matching the current intended behavior in query-cache.ts. Use a controlled query/refetch setup and verify the rejected error rather than allowing the test to pass without observing the promise outcome.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@infra/keycloak/themes/roost/login/resources/css/roost.css`:
- Around line 419-537: Fix the stylelint violations in the alert CSS block: add
a blank line between the custom-property declarations and the regular
declarations before border-radius, and remove quotes from every Poppins
font-family name in the affected alert rules, including the declarations near
the alert title and feedback/helper text selectors.
In `@web-client/src/features/organization/pages/OrganizationPage.tsx`:
- Around line 247-258: Update the AlertDialogAction handlers at
web-client/src/features/organization/pages/OrganizationPage.tsx:247-258 and
:273-283, and web-client/src/features/helper/pages/HelperPage.tsx:198-208, to
call event.preventDefault() before invoking the async delete handlers. Preserve
the existing deleteTeam and helper deletion behavior while keeping each dialog
mounted until its mutation settles so pending state remains visible.
In `@web-client/src/features/sport-events/components/SportEventEditorDialog.tsx`:
- Around line 253-305: Extract the duplicated stepper submission flow from
handleSubmit and the other listed editor components into a shared hook, such as
useStepperFormSubmit, that owns step-field selection, scoped validation, step
advancement, final mutation execution, and formMutationErrorFields mapping.
Refactor SportEventEditorDialog and the other affected components to use the
hook while preserving each component’s existing payload construction, mutation
feedback, success actions, and step behavior.
In `@web-client/src/lib/query-cache.ts`:
- Around line 31-62: Handle errors from the awaited qc.refetchQueries call in
settleMutation so cache-refresh failures do not reject the successful mutation
or propagate through mutateAsync. Swallow the refresh error, or log it if
consistent with existing conventions, while preserving the current invalidation
and refetch behavior.
---
Outside diff comments:
In `@web-client/src/features/feedback/model/useFeedbackViewModel.ts`:
- Around line 86-96: Move the inline comment in the FeedbackDetailView interface
so it directly precedes the rating property, documenting rating’s undefined
semantics rather than error. Keep the property types and interface structure
unchanged.
In `@web-client/src/features/organization/components/TeamEditorDialog.tsx`:
- Around line 295-346: Update the error handling in handleSubmit so that when
formMutationErrorFields returns errors for a details field (name, sportId/sport,
description, or address), set the wizard step to the details step before
displaying the errors. Preserve the existing error state behavior for
member-field errors and use the existing steps/stepIndex flow to locate the
details step.
---
Nitpick comments:
In `@web-client/e2e/cache-consistency.spec.ts`:
- Around line 55-63: Update navigateTo so the URL regular expression uses path
directly without the unnecessary .replace('/', '\\/') call. Preserve the
existing end-of-string anchor and URL assertion behavior.
In `@web-client/e2e/payments.spec.ts`:
- Around line 61-62: Update both toast assertions in
web-client/e2e/payments.spec.ts at lines 61-62 and 99 to use toastRegion(page)
with toContainText(paymentMember.name’s transaction message), replacing the
global getByText visibility checks while preserving the surrounding test flow.
In `@web-client/src/app/pages/RouteErrorPage.tsx`:
- Around line 12-16: Update the error handling in RouteErrorPage’s useEffect so
route/render errors are reported in production as well as development, using the
application’s existing logging or error-tracking mechanism rather than gating
all reporting behind import.meta.env.DEV. Preserve the current error context and
avoid exposing development-only console behavior as the sole production path.
In `@web-client/src/components/ui/input.tsx`:
- Around line 5-20: Refactor the Input component to remove React.forwardRef and
accept ref directly as a standard prop in its function parameters, while
preserving the existing input attributes, className handling, and ref forwarding
to the underlying <input>.
In `@web-client/src/features/payments/model/transactionEditor.ts`:
- Around line 12-19: Update the validation options in transactionCreateSchema to
use Zod 4’s error parameter instead of the deprecated message parameter for
memberId, amount, and title. Preserve the existing validation rules and
user-facing error text.
In `@web-client/src/lib/query-cache.test.ts`:
- Around line 1-78: Add a test in the settleMutation suite that makes
refetchQueries reject and asserts whether settleMutation propagates that
rejection, matching the current intended behavior in query-cache.ts. Use a
controlled query/refetch setup and verify the rejected error rather than
allowing the test to pass without observing the promise outcome.
In `@web-client/src/lib/server-error.ts`:
- Around line 93-101: Update the trustServerMessage decision in the server-error
message construction to reject serverMessage whenever fieldErrors were
extracted, instead of relying on the exact 'Validation failed' string. Preserve
the existing 401 and 5xx protections and allow other 4xx messages only when no
field-level errors are present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3be45e7e-5e9e-4572-9e0b-3636388ded68
⛔ Files ignored due to path filters (1)
web-client/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (144)
infra/keycloak/themes/roost/login/resources/css/roost.cssweb-client/README.mdweb-client/e2e/app-shell.spec.tsweb-client/e2e/boot.spec.tsweb-client/e2e/cache-consistency.spec.tsweb-client/e2e/events.spec.tsweb-client/e2e/feedback.spec.tsweb-client/e2e/helper.spec.tsweb-client/e2e/letters.spec.tsweb-client/e2e/members.spec.tsweb-client/e2e/organization.spec.tsweb-client/e2e/pages.spec.tsweb-client/e2e/payments.spec.tsweb-client/e2e/profile.spec.tsweb-client/e2e/support/api.tsweb-client/e2e/support/auth.tsweb-client/e2e/support/data.tsweb-client/e2e/support/fixtures.tsweb-client/e2e/support/server/dashboard.tsweb-client/e2e/support/server/events.tsweb-client/e2e/support/server/feedback.tsweb-client/e2e/support/server/helper.tsweb-client/e2e/support/server/letters.tsweb-client/e2e/support/server/members.tsweb-client/e2e/support/server/organization.tsweb-client/e2e/support/server/payments.tsweb-client/package.jsonweb-client/playwright.config.tsweb-client/src/__tests__/AuthenticatedApp.test.tsxweb-client/src/__tests__/NotFoundPage.test.tsxweb-client/src/__tests__/keycloak.test.tsweb-client/src/__tests__/useAuth.test.tsweb-client/src/app/ErrorBoundary.tsxweb-client/src/app/layout/AppShell.tsxweb-client/src/app/pages/DashboardPage.tsxweb-client/src/app/pages/NotFoundPage.tsxweb-client/src/app/pages/RouteErrorPage.test.tsxweb-client/src/app/pages/RouteErrorPage.tsxweb-client/src/app/pages/model/useDashboardViewModel.tsweb-client/src/app/router/routes.tsxweb-client/src/app/theme/ThemeProvider.tsxweb-client/src/components/ui/ErrorMessage.tsxweb-client/src/components/ui/ErrorNotice.tsxweb-client/src/components/ui/calendar.tsxweb-client/src/components/ui/data-table.tsxweb-client/src/components/ui/date-picker.tsxweb-client/src/components/ui/dialog-form-skeleton.tsxweb-client/src/components/ui/dialog-stepper.tsxweb-client/src/components/ui/dialog.tsxweb-client/src/components/ui/dropdown-menu.tsxweb-client/src/components/ui/input.tsxweb-client/src/components/ui/password-input.tsxweb-client/src/components/ui/pending-button.test.tsxweb-client/src/components/ui/pending-button.tsxweb-client/src/components/ui/phone-input.tsxweb-client/src/components/ui/popover.tsxweb-client/src/components/ui/sonner.tsxweb-client/src/components/ui/spinner.tsxweb-client/src/components/ui/tabs.tsxweb-client/src/features/auth/currentUser.tsweb-client/src/features/auth/useAuth.tsweb-client/src/features/feedback/api/queries.tsweb-client/src/features/feedback/components/FeedbackComposeDialog.test.tsxweb-client/src/features/feedback/components/FeedbackComposeDialog.tsxweb-client/src/features/feedback/components/FeedbackEditDialog.test.tsxweb-client/src/features/feedback/components/FeedbackEditDialog.tsxweb-client/src/features/feedback/index.tsweb-client/src/features/feedback/model/feedbackEditor.test.tsweb-client/src/features/feedback/model/feedbackEditor.tsweb-client/src/features/feedback/model/feedbackUiStore.tsweb-client/src/features/feedback/model/useFeedbackViewModel.tsweb-client/src/features/feedback/pages/FeedbackPage.test.tsxweb-client/src/features/feedback/pages/FeedbackPage.tsxweb-client/src/features/helper/api/queries.tsweb-client/src/features/helper/model/helperUiStore.tsweb-client/src/features/helper/model/useReportViewModel.tsweb-client/src/features/helper/pages/HelperPage.test.tsxweb-client/src/features/helper/pages/HelperPage.tsxweb-client/src/features/helper/pages/ReportMarkdown.test.tsxweb-client/src/features/helper/pages/ReportMarkdown.tsxweb-client/src/features/letters/model/letterComposer.test.tsweb-client/src/features/letters/model/letterComposer.tsweb-client/src/features/letters/pages/LettersPage.test.tsxweb-client/src/features/letters/pages/LettersPage.tsxweb-client/src/features/members/api/queries.tsweb-client/src/features/members/components/MemberEditorDialog.test.tsxweb-client/src/features/members/components/MemberEditorDialog.tsxweb-client/src/features/members/model/memberEditor.test.tsweb-client/src/features/members/model/memberEditor.tsweb-client/src/features/members/model/membersUiStore.tsweb-client/src/features/members/model/useMembersViewModel.test.tsweb-client/src/features/members/model/useMembersViewModel.tsweb-client/src/features/members/pages/MembersPage.tsxweb-client/src/features/organization/api/queries.tsweb-client/src/features/organization/components/SportEditorDialog.tsxweb-client/src/features/organization/components/TeamEditorDialog.test.tsxweb-client/src/features/organization/components/TeamEditorDialog.tsxweb-client/src/features/organization/model/organizationUiStore.tsweb-client/src/features/organization/model/rolePickerOptions.tsweb-client/src/features/organization/model/sportEditor.test.tsweb-client/src/features/organization/model/sportEditor.tsweb-client/src/features/organization/model/teamEditor.test.tsweb-client/src/features/organization/model/teamEditor.tsweb-client/src/features/organization/model/useTeamsViewModel.tsweb-client/src/features/organization/pages/OrganizationPage.tsxweb-client/src/features/payments/api/queries.tsweb-client/src/features/payments/model/paymentsUiStore.tsweb-client/src/features/payments/model/transactionEditor.test.tsweb-client/src/features/payments/model/transactionEditor.tsweb-client/src/features/payments/model/usePaymentsViewModel.tsweb-client/src/features/payments/pages/PaymentsPage.test.tsxweb-client/src/features/payments/pages/PaymentsPage.tsxweb-client/src/features/profile/pages/ProfilePage.test.tsxweb-client/src/features/profile/pages/ProfilePage.tsxweb-client/src/features/sport-events/api/queries.tsweb-client/src/features/sport-events/components/SportEventEditorDialog.test.tsxweb-client/src/features/sport-events/components/SportEventEditorDialog.tsxweb-client/src/features/sport-events/model/eventEditor.test.tsweb-client/src/features/sport-events/model/eventEditor.tsweb-client/src/features/sport-events/model/eventsUiStore.tsweb-client/src/features/sport-events/model/useEventsViewModel.tsweb-client/src/features/sport-events/pages/SportEventsPage.test.tsxweb-client/src/features/sport-events/pages/SportEventsPage.tsxweb-client/src/index.cssweb-client/src/lib/format.tsweb-client/src/lib/forms.tsweb-client/src/lib/keycloak.tsweb-client/src/lib/mutation-feedback-copy.tsweb-client/src/lib/mutation-feedback.test.tsweb-client/src/lib/mutation-feedback.tsweb-client/src/lib/query-cache.test.tsweb-client/src/lib/query-cache.tsweb-client/src/lib/query-keys.tsweb-client/src/lib/server-error.test.tsweb-client/src/lib/server-error.tsweb-client/src/lib/validation.tsweb-client/src/testing/fixtures/dashboard.tsweb-client/src/testing/fixtures/feedback.tsweb-client/src/testing/fixtures/members.tsweb-client/src/testing/fixtures/organization.tsweb-client/src/testing/fixtures/report.tsweb-client/src/testing/scope.tsweb-client/src/types.test.tsweb-client/src/types.ts
💤 Files with no reviewable changes (19)
- web-client/src/features/helper/model/helperUiStore.ts
- web-client/src/components/ui/ErrorMessage.tsx
- web-client/e2e/support/server/dashboard.ts
- web-client/e2e/boot.spec.ts
- web-client/e2e/app-shell.spec.ts
- web-client/src/features/auth/currentUser.ts
- web-client/src/testing/fixtures/members.ts
- web-client/src/features/helper/pages/ReportMarkdown.test.tsx
- web-client/src/features/helper/pages/ReportMarkdown.tsx
- web-client/e2e/support/server/letters.ts
- web-client/src/features/sport-events/pages/SportEventsPage.test.tsx
- web-client/src/lib/format.ts
- web-client/src/types.test.ts
- web-client/src/features/members/model/useMembersViewModel.test.ts
- web-client/src/testing/fixtures/organization.ts
- web-client/src/app/theme/ThemeProvider.tsx
- web-client/src/features/feedback/pages/FeedbackPage.test.tsx
- web-client/src/testing/fixtures/report.ts
- web-client/src/tests/AuthenticatedApp.test.tsx
Comment on lines
+419
to
537
| .pf-v5-c-alert, | ||
| .alert { | ||
| --roost-alert-accent: var(--roost-muted-foreground); | ||
| --roost-alert-background: color-mix(in oklch, var(--roost-muted-foreground) 8%, transparent); | ||
| --roost-alert-border: var(--roost-border); | ||
| --pf-c-alert--BoxShadow: none; | ||
| --pf-c-alert--BackgroundColor: var(--roost-alert-background); | ||
| --pf-c-alert--BorderTopWidth: 0; | ||
| --pf-c-alert--BorderTopColor: transparent; | ||
| --pf-c-alert__icon--Color: var(--roost-alert-accent); | ||
| --pf-c-alert__title--Color: var(--roost-alert-accent); | ||
| --pf-c-alert--m-inline--BackgroundColor: var(--roost-alert-background); | ||
| --pf-c-alert--m-inline--m-success--BackgroundColor: var(--roost-alert-background); | ||
| --pf-c-alert--m-inline--m-danger--BackgroundColor: var(--roost-alert-background); | ||
| --pf-c-alert--m-inline--m-warning--BackgroundColor: var(--roost-alert-background); | ||
| --pf-c-alert--m-inline--m-info--BackgroundColor: var(--roost-alert-background); | ||
| --pf-v5-c-alert--BoxShadow: none; | ||
| --pf-v5-c-alert--BackgroundColor: var(--roost-alert-background); | ||
| --pf-v5-c-alert--BorderTopWidth: 0; | ||
| --pf-v5-c-alert--BorderTopColor: transparent; | ||
| --pf-v5-c-alert__icon--Color: var(--roost-alert-accent); | ||
| --pf-v5-c-alert__title--Color: var(--roost-alert-accent); | ||
| --pf-v5-c-alert--m-inline--m-custom--BackgroundColor: var(--roost-alert-background); | ||
| --pf-v5-c-alert--m-inline--m-success--BackgroundColor: var(--roost-alert-background); | ||
| --pf-v5-c-alert--m-inline--m-danger--BackgroundColor: var(--roost-alert-background); | ||
| --pf-v5-c-alert--m-inline--m-warning--BackgroundColor: var(--roost-alert-background); | ||
| --pf-v5-c-alert--m-inline--m-info--BackgroundColor: var(--roost-alert-background); | ||
| border-radius: var(--roost-radius); | ||
| border: 1px solid var(--roost-alert-border); | ||
| border-left: 3px solid var(--roost-alert-accent); | ||
| border-top: 0; | ||
| border-block-start: 0; | ||
| box-shadow: none; | ||
| background: var(--roost-alert-background); | ||
| font-family: 'Poppins', sans-serif; | ||
| font-size: 0.875rem; | ||
| color: var(--roost-foreground); | ||
| margin-bottom: 1.25rem; | ||
| } | ||
|
|
||
| .pf-c-alert.pf-m-danger, | ||
| .pf-v5-c-alert.pf-m-danger, | ||
| .alert-error { | ||
| border-radius: var(--roost-radius); | ||
| border-left: 3px solid var(--roost-destructive); | ||
| background: color-mix(in oklch, var(--roost-destructive) 10%, transparent); | ||
| --roost-alert-accent: var(--roost-destructive); | ||
| --roost-alert-background: color-mix(in oklch, var(--roost-destructive) 10%, transparent); | ||
| --roost-alert-border: color-mix(in oklch, var(--roost-destructive) 30%, transparent); | ||
| --pf-c-alert--m-danger--BorderTopColor: var(--roost-alert-accent); | ||
| --pf-c-alert--m-danger__icon--Color: var(--roost-alert-accent); | ||
| --pf-c-alert--m-danger__title--Color: var(--roost-alert-accent); | ||
| --pf-v5-c-alert--m-danger--BorderTopColor: var(--roost-alert-accent); | ||
| --pf-v5-c-alert--m-danger__icon--Color: var(--roost-alert-accent); | ||
| --pf-v5-c-alert--m-danger__title--Color: var(--roost-alert-accent); | ||
| } | ||
|
|
||
| .pf-c-alert.pf-m-warning, | ||
| .pf-v5-c-alert.pf-m-warning, | ||
| .alert-warning { | ||
| --roost-alert-accent: var(--roost-warning); | ||
| --roost-alert-background: color-mix(in oklch, var(--roost-warning) 10%, transparent); | ||
| --roost-alert-border: color-mix(in oklch, var(--roost-warning) 32%, transparent); | ||
| --pf-c-alert--m-warning--BorderTopColor: var(--roost-alert-accent); | ||
| --pf-c-alert--m-warning__icon--Color: var(--roost-alert-accent); | ||
| --pf-c-alert--m-warning__title--Color: var(--roost-alert-accent); | ||
| --pf-v5-c-alert--m-warning--BorderTopColor: var(--roost-alert-accent); | ||
| --pf-v5-c-alert--m-warning__icon--Color: var(--roost-alert-accent); | ||
| --pf-v5-c-alert--m-warning__title--Color: var(--roost-alert-accent); | ||
| } | ||
|
|
||
| .pf-c-alert.pf-m-success, | ||
| .pf-v5-c-alert.pf-m-success, | ||
| .alert-success { | ||
| --roost-alert-accent: var(--roost-success); | ||
| --roost-alert-background: color-mix(in oklch, var(--roost-success) 10%, transparent); | ||
| --roost-alert-border: color-mix(in oklch, var(--roost-success) 30%, transparent); | ||
| --pf-c-alert--m-success--BorderTopColor: var(--roost-alert-accent); | ||
| --pf-c-alert--m-success__icon--Color: var(--roost-alert-accent); | ||
| --pf-c-alert--m-success__title--Color: var(--roost-alert-accent); | ||
| --pf-v5-c-alert--m-success--BorderTopColor: var(--roost-alert-accent); | ||
| --pf-v5-c-alert--m-success__icon--Color: var(--roost-alert-accent); | ||
| --pf-v5-c-alert--m-success__title--Color: var(--roost-alert-accent); | ||
| } | ||
|
|
||
| .pf-c-alert.pf-m-info, | ||
| .pf-v5-c-alert.pf-m-info, | ||
| .alert-info { | ||
| --roost-alert-accent: var(--roost-muted-foreground); | ||
| --roost-alert-background: color-mix(in oklch, var(--roost-muted-foreground) 8%, transparent); | ||
| --roost-alert-border: var(--roost-border); | ||
| --pf-c-alert--m-info--BorderTopColor: var(--roost-alert-border); | ||
| --pf-c-alert--m-info__icon--Color: var(--roost-alert-accent); | ||
| --pf-c-alert--m-info__title--Color: var(--roost-alert-accent); | ||
| --pf-v5-c-alert--m-info--BorderTopColor: var(--roost-alert-border); | ||
| --pf-v5-c-alert--m-info__icon--Color: var(--roost-alert-accent); | ||
| --pf-v5-c-alert--m-info__title--Color: var(--roost-alert-accent); | ||
| } | ||
|
|
||
| .pf-c-alert__icon, | ||
| .pf-v5-c-alert__icon, | ||
| .pf-c-alert__title, | ||
| .pf-v5-c-alert__title, | ||
| .alert .kc-feedback-text { | ||
| color: var(--roost-alert-accent); | ||
| } | ||
|
|
||
| .pf-c-alert__title, | ||
| .pf-v5-c-alert__title { | ||
| font-family: 'Poppins', sans-serif; | ||
| } | ||
|
|
||
| .kc-feedback-text, | ||
| .pf-c-form__helper-text.pf-m-error { | ||
| .pf-c-form__helper-text.pf-m-error, | ||
| .pf-v5-c-form__helper-text.pf-m-error { | ||
| font-family: 'Poppins', sans-serif; | ||
| } | ||
|
|
||
| .kc-feedback-text:not(.pf-c-alert__title):not(.pf-v5-c-alert__title), | ||
| .pf-c-form__helper-text.pf-m-error, | ||
| .pf-v5-c-form__helper-text.pf-m-error { | ||
| color: var(--roost-destructive); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix stylelint violations flagged in the new alert block.
Static analysis flags two real issues in this new block: missing blank line before the border-radius declaration (line 446, separating the custom-property group from regular declarations), and quoted 'Poppins' font-family names at lines 453, 525, and 531 that the configured font-family-name-quotes rule disallows.
🎨 Proposed fix
--pf-v5-c-alert--m-inline--m-info--BackgroundColor: var(--roost-alert-background);
+
border-radius: var(--roost-radius);
border: 1px solid var(--roost-alert-border);
border-left: 3px solid var(--roost-alert-accent);
border-top: 0;
border-block-start: 0;
box-shadow: none;
background: var(--roost-alert-background);
- font-family: 'Poppins', sans-serif;
+ font-family: Poppins, sans-serif;Apply the same unquoted Poppins fix at lines 525 and 531.
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 446-446: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 453-453: Expected no quotes around "Poppins" (font-family-name-quotes)
(font-family-name-quotes)
[error] 525-525: Expected no quotes around "Poppins" (font-family-name-quotes)
(font-family-name-quotes)
[error] 531-531: Expected no quotes around "Poppins" (font-family-name-quotes)
(font-family-name-quotes)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infra/keycloak/themes/roost/login/resources/css/roost.css` around lines 419 -
537, Fix the stylelint violations in the alert CSS block: add a blank line
between the custom-property declarations and the regular declarations before
border-radius, and remove quotes from every Poppins font-family name in the
affected alert rules, including the declarations near the alert title and
feedback/helper text selectors.
Source: Linters/SAST tools
Comment on lines
247
to
258
| <AlertDialogFooter> | ||
| <AlertDialogCancel disabled={deleteTeam.isPending}>Cancel</AlertDialogCancel> | ||
| <AlertDialogAction disabled={deleteTeam.isPending} onClick={confirmDeleteTeam}> | ||
| {deleteTeam.isPending ? 'Deleting' : 'Delete'} | ||
| {deleteTeam.isPending ? ( | ||
| <PendingButtonContent pendingLabel="Deleting…" /> | ||
| ) : ( | ||
| 'Delete' | ||
| )} | ||
| </AlertDialogAction> | ||
| </AlertDialogFooter> | ||
| </AlertDialogContent> | ||
| </AlertDialog> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -e
printf '--- OrganizationPage.tsx (around delete dialogs) ---\n'
sed -n '200,340p' web-client/src/features/organization/pages/OrganizationPage.tsx
printf '\n--- HelperPage.tsx (around delete dialog) ---\n'
sed -n '150,240p' web-client/src/features/helper/pages/HelperPage.tsx
printf '\n--- Search for AlertDialogAction usage and close handlers ---\n'
rg -n "AlertDialogAction|closeDelete|confirmDeleteTeam|confirmDeleteSport|confirmDelete" web-client/src/features -g '!**/node_modules/**'Repository: AET-DevOps26/team-devoops
Length of output: 254
🏁 Script executed:
set -e
printf '--- File list for dialog components ---\n'
git ls-files web-client/src | rg 'AlertDialog|dialog|Dialog'
printf '\n--- Search for AlertDialog component wrappers ---\n'
rg -n "export .*AlertDialogAction|function AlertDialogAction|const AlertDialogAction" web-client/src -g '!**/node_modules/**'Repository: AET-DevOps26/team-devoops
Length of output: 254
🏁 Script executed:
set -e
python3 - <<'PY'
from pathlib import Path
for p in [
Path('web-client/src/features/organization/pages/OrganizationPage.tsx'),
Path('web-client/src/features/helper/pages/HelperPage.tsx'),
]:
print(f'--- {p} ---')
text = p.read_text()
for needle in ['confirmDeleteTeam', 'confirmDeleteSport', 'confirmDelete', 'closeDelete', 'deleteTeam', 'deleteSport', 'deleteReport']:
if needle in text:
print(f'found {needle}')
PYRepository: AET-DevOps26/team-devoops
Length of output: 456
🏁 Script executed:
set -e
printf '--- OrganizationPage.tsx handler definitions ---\n'
sed -n '1,220p' web-client/src/features/organization/pages/OrganizationPage.tsx | nl -ba | sed -n '1,220p'
printf '\n--- HelperPage.tsx handler definitions ---\n'
sed -n '1,220p' web-client/src/features/helper/pages/HelperPage.tsx | nl -ba | sed -n '1,220p'Repository: AET-DevOps26/team-devoops
Length of output: 254
🏁 Script executed:
set -e
echo '--- OrganizationPage.tsx (delete handlers) ---'
sed -n '1,360p' web-client/src/features/organization/pages/OrganizationPage.tsx | nl -ba | sed -n '1,360p'
echo
echo '--- HelperPage.tsx (delete handlers) ---'
sed -n '1,280p' web-client/src/features/helper/pages/HelperPage.tsx | nl -ba | sed -n '1,280p'Repository: AET-DevOps26/team-devoops
Length of output: 324
🏁 Script executed:
set -e
echo '--- Search for AlertDialogAction implementations/imports ---'
rg -n "AlertDialogAction|AlertDialogCancel|closeDelete|confirmDeleteTeam|confirmDeleteSport|confirmDelete|deleteTeam|deleteSport|deleteReport" web-client/src/features/organization/pages/OrganizationPage.tsx web-client/src/features/helper/pages/HelperPage.tsx web-client/src -g '!**/node_modules/**'Repository: AET-DevOps26/team-devoops
Length of output: 13142
🏁 Script executed:
set -e
python3 - <<'PY'
from pathlib import Path
files = [
Path('web-client/src/features/organization/pages/OrganizationPage.tsx'),
Path('web-client/src/features/helper/pages/HelperPage.tsx'),
]
for p in files:
print(f'=== {p} ===')
text = p.read_text().splitlines()
needles = ['confirmDeleteTeam', 'confirmDeleteSport', 'confirmDelete(', 'confirmDelete ', 'closeDelete', 'deleteTeam', 'deleteSport', 'deleteReport']
for i, line in enumerate(text, 1):
if any(n in line for n in needles):
start = max(1, i - 4)
end = min(len(text), i + 8)
print(f'-- context around line {i} --')
for j in range(start, end + 1):
print(f'{j:4}: {text[j-1]}')
print()
PYRepository: AET-DevOps26/team-devoops
Length of output: 16801
🏁 Script executed:
set -e
echo '--- web-client/src/components/ui/alert-dialog.tsx ---'
sed -n '1,260p' web-client/src/components/ui/alert-dialog.tsx
echo
echo '--- any explicit preventDefault on AlertDialogAction clicks ---'
rg -n "preventDefault\\(|AlertDialogAction" web-client/src -g '!**/node_modules/**' | sed -n '1,120p'Repository: AET-DevOps26/team-devoops
Length of output: 8330
Prevent these alert dialogs from dismissing on click. AlertDialogAction still uses Radix’s default close behavior here, so these async delete handlers need event.preventDefault(); otherwise the dialog unmounts before the mutation settles and the pending state disappears.
web-client/src/features/organization/pages/OrganizationPage.tsx#L247-L258web-client/src/features/organization/pages/OrganizationPage.tsx#L273-L283web-client/src/features/helper/pages/HelperPage.tsx#L198-L208
📍 Affects 2 files
web-client/src/features/organization/pages/OrganizationPage.tsx#L247-L258(this comment)web-client/src/features/organization/pages/OrganizationPage.tsx#L273-L283web-client/src/features/helper/pages/HelperPage.tsx#L198-L208
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-client/src/features/organization/pages/OrganizationPage.tsx` around lines
247 - 258, Update the AlertDialogAction handlers at
web-client/src/features/organization/pages/OrganizationPage.tsx:247-258 and
:273-283, and web-client/src/features/helper/pages/HelperPage.tsx:198-208, to
call event.preventDefault() before invoking the async delete handlers. Preserve
the existing deleteTeam and helper deletion behavior while keeping each dialog
mounted until its mutation settles so pending state remains visible.
Comment on lines
253
to
305
| const handleSubmit = async (submitEvent: FormEvent<HTMLFormElement>) => { | ||
| submitEvent.preventDefault() | ||
|
|
||
| const stepError = validateStep(eventSteps[stepIndex].id, form) | ||
| if (stepError) { | ||
| setFormError(stepError) | ||
| setFieldErrors(null) | ||
|
|
||
| const stepFields = | ||
| currentStepId === 'details' | ||
| ? (['name'] as const) | ||
| : currentStepId === 'schedule' | ||
| ? (['startLocal', 'endLocal'] as const) | ||
| : null | ||
| const validationErrors = isLastStep | ||
| ? validateSportEventEditorForm(form) | ||
| : stepFields | ||
| ? validateSportEventEditorForm(form, stepFields) | ||
| : null | ||
|
|
||
| if (validationErrors) { | ||
| setFieldErrors(validationErrors) | ||
| return | ||
| } | ||
|
|
||
| setFormError(null) | ||
|
|
||
| if (!isLastStep) { | ||
| setStepIndex((current) => current + 1) | ||
| return | ||
| } | ||
|
|
||
| try { | ||
| if (target.mode === 'create') { | ||
| const created = await createEvent.mutateAsync({ | ||
| name: form.name.trim(), | ||
| description: cleanedDescription(form.description), | ||
| start_time: localDateTimeToIso(form.startLocal), | ||
| end_time: localDateTimeToIso(form.endLocal), | ||
| sports_linked: resolvedSportIds(form, teams), | ||
| teams_linked: form.teamIds, | ||
| attendees: form.attendeeIds, | ||
| }) | ||
|
|
||
| setMutationNotice('Event created.') | ||
| const created = await createEvent.mutateAsync(buildSportEventCreatePayload(form, teams)) | ||
|
|
||
| toast.success('Event created.') | ||
| openEvent(created.id) | ||
| closeEditor() | ||
| return | ||
| } | ||
|
|
||
| if (!event) return | ||
|
|
||
| const payload = buildUpdatePayload(event, form, teams) | ||
| const payload = buildSportEventUpdatePayload(event, form, teams) | ||
| if (Object.keys(payload).length > 0) { | ||
| await updateEvent.mutateAsync({ id: event.id, ...payload }) | ||
| setMutationNotice('Event updated.') | ||
| toast.success('Event updated.') | ||
| } | ||
| closeEditor() | ||
| } catch (error) { | ||
| setFormError(serverErrorMessage(error)) | ||
| setFieldErrors( | ||
| formMutationErrorFields( | ||
| error, | ||
| target.mode === 'create' ? mutationFeedbackCopy.event.create : mutationFeedbackCopy.event.update, | ||
| ), | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Duplicated stepper-submit logic across ~9 editor components.
The handleSubmit pattern here (step-field selection → step-scoped validation → advance-or-submit → try/catch mapping mutation errors to field errors via formMutationErrorFields) is copy-pasted almost verbatim across FeedbackComposeDialog.tsx, FeedbackEditDialog.tsx, LettersPage.tsx, MemberEditorDialog.tsx, SportEditorDialog.tsx, TeamEditorDialog.tsx, PaymentsPage.tsx, and ProfilePage.tsx. A shared hook (e.g. useStepperFormSubmit({ steps, stepFieldsFor, validate, mutate, onSuccess })) would centralize this control flow so future fixes/behavior changes don't need to be replicated in every dialog.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-client/src/features/sport-events/components/SportEventEditorDialog.tsx`
around lines 253 - 305, Extract the duplicated stepper submission flow from
handleSubmit and the other listed editor components into a shared hook, such as
useStepperFormSubmit, that owns step-field selection, scoped validation, step
advancement, final mutation execution, and formMutationErrorFields mapping.
Refactor SportEventEditorDialog and the other affected components to use the
hook while preserving each component’s existing payload construction, mutation
feedback, success actions, and step behavior.
Comment on lines
+31
to
+62
| export async function settleMutation<T extends { id: string } = { id: string }>( | ||
| qc: QueryClient, | ||
| { replace = [], remove = [], evict = [], invalidate }: SettleMutation<T>, | ||
| ): Promise<void> { | ||
| for (const { key, id, next } of replace) { | ||
| replaceById(qc, key, id, next) | ||
| } | ||
|
|
||
| for (const { key, id } of remove) { | ||
| removeById(qc, key, id) | ||
| } | ||
|
|
||
| for (const key of evict) { | ||
| qc.removeQueries({ queryKey: key }) | ||
| } | ||
|
|
||
| // Invalidate every matching entry without starting one refetch per key. Keys deliberately | ||
| // overlap (for example, a resource root and one detail key); launching those in parallel can | ||
| // cancel and restart the same request. A single predicate refetches their union exactly once. | ||
| for (const queryKey of invalidate) { | ||
| void qc.invalidateQueries({ queryKey, refetchType: 'none' }) | ||
| } | ||
|
|
||
| await qc.refetchQueries({ | ||
| type: 'all', | ||
| predicate: (query) => | ||
| // A detail cache populated only from a mutation response has no query function until its | ||
| // hook is mounted. It is already authoritative and cannot be refetched yet. | ||
| typeof query.options.queryFn === 'function' && | ||
| invalidate.some((queryKey) => partialMatchKey(query.queryKey, queryKey)), | ||
| }) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
TanStack Query v5 useMutation onSuccess callback throws error propagates to mutateAsync catch
💡 Result:
In TanStack Query v5, if an error is thrown within an onSuccess callback defined in useMutation, it does propagate to the promise returned by mutateAsync [1][2]. When you use mutateAsync, it returns a promise that resolves on success and throws on error [2]. If you define an onSuccess callback (either in the useMutation configuration or as an option when calling mutateAsync), TanStack Query executes that callback as part of the mutation lifecycle [2]. If the code inside that onSuccess callback throws an error, that error will bubble up, causing the promise returned by mutateAsync to reject [2]. To manage this behavior effectively: 1. Understand the lifecycle: The mutateAsync promise will reject if either the mutation function itself fails or if any of the associated lifecycle callbacks (like onSuccess) throw an error [2]. 2. Use try/catch with mutateAsync: Since mutateAsync returns a promise, you should wrap your call in a try/catch block to handle errors originating from the mutation or the onSuccess/onError callbacks [2][3]. Example of error propagation: try { await mutateAsync(variables); } catch (error) { // This block will catch errors from the mutationFn // AND errors thrown inside your onSuccess callback console.error('Mutation failed or onSuccess threw an error:', error); } If you want to perform side effects in onSuccess that might fail without causing the entire mutation to be considered failed, ensure you catch errors internally within that callback [2].
Citations:
- 1: https://tanstack.com/query/v5/docs/framework/react/reference/useMutation
- 2: https://tanstack.com/query/latest/docs/framework/react/guides/mutations?from=reactQueryV3
- 3: https://www.answeroverflow.com/m/1321184371658723409
🏁 Script executed:
rg -n "settleMutation\(" web-client/src organization -SRepository: AET-DevOps26/team-devoops
Length of output: 2344
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## query-cache.ts\n'
cat -n web-client/src/lib/query-cache.ts | sed -n '1,220p'
printf '\n## query-cache.test.ts\n'
cat -n web-client/src/lib/query-cache.test.ts | sed -n '1,220p'
printf '\n## organization api queries\n'
cat -n web-client/src/features/organization/api/queries.ts | sed -n '1,240p'
printf '\n## helper api queries\n'
cat -n web-client/src/features/helper/api/queries.ts | sed -n '1,180p'Repository: AET-DevOps26/team-devoops
Length of output: 15373
Catch refetch failures inside settleMutation settleMutation is returned from onSuccess at every call site here, so a rejected qc.refetchQueries(...) will reject mutateAsync and mark an already-successful write as failed. Swallow or log the cache-refresh error so post-mutation refetches do not surface as mutation failures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-client/src/lib/query-cache.ts` around lines 31 - 62, Handle errors from
the awaited qc.refetchQueries call in settleMutation so cache-refresh failures
do not reject the successful mutation or propagate through mutateAsync. Swallow
the refresh error, or log it if consistent with existing conventions, while
preserving the current invalidation and refetch behavior.
# Conflicts: # web-client/src/features/helper/api/queries.ts # web-client/src/features/helper/model/useReportViewModel.ts # web-client/src/features/helper/pages/HelperPage.tsx # web-client/src/features/profile/pages/ProfilePage.test.tsx # web-client/src/features/profile/pages/ProfilePage.tsx
Collaborator
|
lgtm! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A bug-review pass of the running app found a cluster of UX defects that made the admin flows feel broken: mutation failures surfaced raw
error.message(or nothing at all), success banners never went away, loading looked different on every surface, forms fell back to browser-native validation bubbles, a route-level crash took down the whole app, admins could not create feedback at all, and the Keycloak login screens rendered off-theme error colors. This PR is the UX hardening track from #128, plus the two bugs it subsumes.What changed
Errors and mutation feedback
error.messagereaches the UI.Forms and validation
noValidate, themed field errors) with the server still the source of truth.Data layer
Bug fixes
Tests and docs
web-client/README.mdrewritten against the actual code — scripts, env vars, project structure, and the Playwright/mock-persona workflow.Notes
VITE_USE_MOCKSinstructions). The rest of Chore: Repository hardening — configuration, CI, automation, and governance #132 — CI wiring, dependency automation, governance — is out of scope here and stays open.Testing
pnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (enforced by the pre-commit hook on every commit in this branch).Closes #128
Closes #130
Closes #131
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests