Improve user management and system msg#226
Conversation
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughThis PR adds backend cohortId support for user updates, a shared StatCard component, cohort selection and i18n in user management, sortable and localized users table, router-backed users search/pagination, substitutions badge rendering, and broad admin UI localization and stats across roles, timetable, and doorlock pages. ChangesAdmin panel cohort support, internationalization, and UI enhancements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
@coderabbitai work |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 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 `@apps/iris/src/components/admin/users-table.tsx`:
- Around line 80-131: The header cells (TableHead) currently rely on onClick for
sorting which is not keyboard-accessible; update each sortable header (the
TableHead elements that call handleSort('name'|'email'|'cohort'|'roles')) to
move the interactive behavior into a native <button> (or accessible element)
wrapping the label and the SortIcon, keep the existing call to handleSort and
use the existing sort state values (sortColumn, sortDirection) to set aria-sort
(or aria-pressed) appropriately on the button, ensure the TableHead no longer
has the onClick handler (so only the button is interactive) and preserve visual
styles so the button looks like the header while remaining keyboard-focusable
and operable via Enter/Space.
- Around line 46-47: The totalPages calculation can be zero when total is 0
which produces confusing UI; update the computation of totalPages (the variable
computed from total and limit in users-table.tsx) to clamp to at least 1 by
replacing Math.ceil(total / limit) with a guarded/clamped expression (e.g.,
compute Math.ceil(total / limit) and then take Math.max(1, ...)), and apply the
same change to the other occurrence referenced around line 182 so both uses
never yield 0.
In `@apps/iris/src/routes/_private/admin/doorlock/cards.tsx`:
- Around line 437-456: The edit and delete icon-only buttons in the card row
lack accessible names; update the Button instances that call
setSelectedCard/setDialogOpen and handleDelete (and consider
deleteMutation.isPending) to include localized aria-label attributes or
visually-hidden text so screen readers announce their purpose (e.g., aria-label
for "Edit card" and "Delete card" using your i18n helper), ensuring the
destructive delete button still reflects its pending/disabled state for
assistive tech.
- Around line 327-391: The TableHead elements are using onClick only
(mouse-only) for sorting; update each sortable header (the TableHead instances
that call handleSort with 'name'/'owner'/'status'/'devices'/'updated') to
contain a real <button> inside the header, move the onClick handler onto that
button (calling handleSort with the same column key), and remove the TableHead
onClick; also expose aria-sort on the header/button using sortColumn and
sortDirection (e.g., aria-sort="ascending"/"descending" when sortColumn matches
the column, otherwise "none") so keyboard users and assistive tech get proper
control semantics, and keep the existing SortIcon usage unchanged.
In `@apps/iris/src/routes/_private/admin/doorlock/devices.tsx`:
- Around line 410-450: The icon-only action buttons in the device row lack
accessible labels; update each Button where setStatsDevice/setStatsDialogOpen,
setOtaDevice/setOtaDialogOpen, setSelectedDevice/setDialogOpen, and
handleDelete/deleteMutation are used to include localized aria-label props (or
add visually hidden text) describing the action (e.g., "View stats", "Start
OTA", "Edit device", "Delete device") so screen readers can identify them;
ensure labels use your i18n/localization helper if present and keep the existing
onClick/disabled behavior unchanged.
- Around line 342-396: The table header cells currently use onClick on TableHead
which is not keyboard-accessible; update each sortable header (the TableHead
elements that call handleSort('name'|'location'|'apiToken'|'updated')) to render
the interactive part as a native <button> (move the onClick handler onto that
button), ensure the button is focusable (no custom tabIndex required for a
button), and set aria-sort on the TableHead (or the button if that better
matches semantics) to "ascending"/"descending"/"none" based on whether the
column prop (e.g., 'name') equals sortColumn and the current sortDirection; keep
the SortIcon usage but remove click handlers from TableHead so
keyboard/Enter/Space activate handleSort and screen readers get the sort state.
In `@apps/iris/src/routes/_private/admin/doorlock/index.tsx`:
- Around line 54-82: The StatCard components are showing zeroes when stats is
undefined after a failed load; update the render logic in the component handling
statsQuery/stats (referencing statsQuery, stats, and StatCard) to avoid
rendering the cards when there is no valid stats data — e.g. if stats is falsy
and statsQuery.isError (or no cached data present), return the existing
Alert/error UI or an early-return placeholder instead of rendering StatCard with
fallback 0 values; ensure you still show loading states when
statsQuery.isLoading and preserve current Alert rendering for errors.
- Around line 105-108: The empty-state for stats.topUsers currently shows
t('unknown'), which reads like a missing name; change the rendering to use a
dedicated translation key (e.g., t('noTopUsers') or t('empty.topUsers')) and
update the translation files accordingly; locate the conditional that checks
stats.topUsers.length in the component (the JSX rendering block that produces
the <li className="text-muted-foreground text-sm">) and replace t('unknown')
with the new key so the UI communicates "no top users yet" instead of an unknown
user name.
In `@apps/iris/src/routes/_private/admin/timetable/manage.tsx`:
- Around line 349-370: The row action buttons render icon-only controls without
accessible names; update the Button instances used with the Pen/Trash icons (the
ones that call setSelectedItem/setEditDialogOpen and
setItemToDelete/setDeleteDialogOpen) to include accessible labels — e.g., add an
aria-label attribute with localized strings like aria-label={`Edit
${item.name}`} and aria-label={`Delete ${item.name}`} or include visually-hidden
text inside the Button — so screen readers can announce the purpose of each
button.
In `@apps/iris/src/routes/_private/admin/timetable/substitutions.tsx`:
- Around line 445-457: The cohorts grid can be empty today; update the JSX that
builds from sub.lessons.flatMap(l => l.cohorts) and the deduplicated Set to
check for zero length and render a fallback Badge or text (e.g.,
t('substitution.noCohorts') or the same "not available" string used in subs.tsx)
instead of an empty grid. Modify the mapping block that currently returns Badge
for each cohort so it conditionally renders the fallback when the deduped array
is empty, keeping the Badge/variant="secondary" usage for real cohorts.
In `@apps/iris/src/routes/_private/admin/users.tsx`:
- Around line 17-20: The search schema currently uses z.number() which will
reject string query params; update the schema used by validateSearch by
replacing z.number() with z.coerce.number() on the page field (keep
.int().min(1).default(1) chaining intact) so URL search params like "?page=2"
are safely parsed into numbers when validateSearch runs; ensure the change is
made on the searchSchema constant referenced by validateSearch.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 75bee495-9b89-44ce-b84a-f804791f6b1d
📒 Files selected for processing (15)
apps/chronos/src/routes/users/index.tsapps/iris/public/locales/en/translation.jsonapps/iris/public/locales/hu/translation.jsonapps/iris/src/components/admin/stat-card.tsxapps/iris/src/components/admin/user-dialog.tsxapps/iris/src/components/admin/users-table.tsxapps/iris/src/components/subs.tsxapps/iris/src/components/system-message-banner.tsxapps/iris/src/routes/_private/admin/doorlock/cards.tsxapps/iris/src/routes/_private/admin/doorlock/devices.tsxapps/iris/src/routes/_private/admin/doorlock/index.tsxapps/iris/src/routes/_private/admin/roles.tsxapps/iris/src/routes/_private/admin/timetable/manage.tsxapps/iris/src/routes/_private/admin/timetable/substitutions.tsxapps/iris/src/routes/_private/admin/users.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
apps/{chronos,iris}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/filc-reuse.instructions.md)
apps/{chronos,iris}/src/**/*.{ts,tsx}: Reuse existing helpers, types, schemas, and hooks before adding new ones. Check nearby feature folders first, then shared files such as apps/chronos/src/database/helpers.ts, apps/iris/src/utils/query-keys.ts, apps/iris/src/hooks/use-has-permission.ts, apps/iris/src/components/admin/admin.types.ts, and apps/iris/src/components/doorlock/doorlock.types.ts
When a second call site needs the same logic, prefer extracting or extending the existing abstraction instead of creating a parallel helper with a slightly different name
Keep abstractions local to the narrowest shared boundary that already exists. Do not create cross-app utilities for one feature-specific use
Extend existing dialog props, response shapes, and query key families instead of re-declaring near-identical types in each file
Prefer the smallest root-cause fix that matches neighboring code over broad rewrites or speculative cleanup
Keep imports on the app alias boundary:#...for Chronos and@/...for Iris
Files:
apps/iris/src/components/subs.tsxapps/iris/src/components/admin/stat-card.tsxapps/iris/src/components/system-message-banner.tsxapps/chronos/src/routes/users/index.tsapps/iris/src/routes/_private/admin/timetable/substitutions.tsxapps/iris/src/routes/_private/admin/timetable/manage.tsxapps/iris/src/routes/_private/admin/roles.tsxapps/iris/src/routes/_private/admin/users.tsxapps/iris/src/routes/_private/admin/doorlock/cards.tsxapps/iris/src/routes/_private/admin/doorlock/devices.tsxapps/iris/src/components/admin/users-table.tsxapps/iris/src/routes/_private/admin/doorlock/index.tsxapps/iris/src/components/admin/user-dialog.tsx
apps/iris/src/{routes,components}/**/*.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-data-flow.instructions.md)
apps/iris/src/{routes,components}/**/*.tsx: Always use centralized keys from apps/iris/src/utils/query-keys.ts for React Query. Do not introduce inline array query keys for existing domains
UseparseResponse(...)and the generated Hono client from apps/iris/src/utils/hc.ts for API requests when that is the local pattern
When a mutation changes server state, invalidate every affected query family, not just the page-local list. Follow the multi-invalidation pattern already used in admin news and doorlock screens
Reuse apps/iris/src/hooks/use-has-permission.ts and existing permission guard components instead of duplicating permission logic in the view
New user-facing error and success messages should go throught(...)and the locale files, even when surfaced through toasts
Files:
apps/iris/src/components/subs.tsxapps/iris/src/components/admin/stat-card.tsxapps/iris/src/components/system-message-banner.tsxapps/iris/src/routes/_private/admin/timetable/substitutions.tsxapps/iris/src/routes/_private/admin/timetable/manage.tsxapps/iris/src/routes/_private/admin/roles.tsxapps/iris/src/routes/_private/admin/users.tsxapps/iris/src/routes/_private/admin/doorlock/cards.tsxapps/iris/src/routes/_private/admin/doorlock/devices.tsxapps/iris/src/components/admin/users-table.tsxapps/iris/src/routes/_private/admin/doorlock/index.tsxapps/iris/src/components/admin/user-dialog.tsx
apps/iris/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/iris.instructions.md)
apps/iris/src/**/*.{ts,tsx}: Keep user-facing text int(...)and update both locale trees under apps/iris/public/locales/en and apps/iris/public/locales/hu
TanStack Form is the default form pattern; follow examples withuseForm,useStore(form.store, selector), and<form.Field>{(field) => ...}</form.Field>
form.reset(values)takes raw values, not{ values }.form.resetandform.setFieldValueare not stableuseEffectdependencies, so omit them from dependency arrays when needed
Base UI dropdown wrappers useonClick, not Radix-styleonSelect, unless the local component explicitly exposes a different API
apps/iris/src/components/ui/chart.tsx already ownsResponsiveContainer; do not wrap chart children in another one
Keep public timetable filter state in TanStack Router search params instead of duplicating it in unrelated local state
Files:
apps/iris/src/components/subs.tsxapps/iris/src/components/admin/stat-card.tsxapps/iris/src/components/system-message-banner.tsxapps/iris/src/routes/_private/admin/timetable/substitutions.tsxapps/iris/src/routes/_private/admin/timetable/manage.tsxapps/iris/src/routes/_private/admin/roles.tsxapps/iris/src/routes/_private/admin/users.tsxapps/iris/src/routes/_private/admin/doorlock/cards.tsxapps/iris/src/routes/_private/admin/doorlock/devices.tsxapps/iris/src/components/admin/users-table.tsxapps/iris/src/routes/_private/admin/doorlock/index.tsxapps/iris/src/components/admin/user-dialog.tsx
apps/chronos/src/routes/**/*.ts
📄 CodeRabbit inference engine (.github/instructions/chronos-route-procedure.instructions.md)
apps/chronos/src/routes/**/*.ts: Keep route work inside the feature folder under apps/chronos/src/routes, following the_factory.ts,_router.ts, and handler-file pattern used in apps/chronos/src/routes/doorlock
Match handler structure from Chronos routes: includedescribeRoute(...), shared middleware, validators, then the final handler
Keep request and response Zod schemas close to the route that uses them; avoid a separate 'schemas' file unless the same schema is genuinely shared by multiple handlers in the same feature
UserequireAuthenticationandrequireAuthorization(...)middleware instead of inline permission checks; reuse the existing permission naming style
Return the established success envelope and useHTTPExceptionplusStatusCodesfor error paths when the route already follows that pattern
Keep OpenAPI metadata complete with tags, descriptions, and response documentation when adding or updating Chronos API endpoints
apps/chronos/src/routes/**/*.ts: Follow the feature route layout in apps/chronos/src/routes: keep_factory.ts,_router.ts, and handler files together inside the feature folder
Reuse middleware such asrequireAuthorization('resource:action')for permission checks. Do not inline authorization logic in handlers
Files:
apps/chronos/src/routes/users/index.ts
apps/chronos/src/routes/*/index.ts
📄 CodeRabbit inference engine (.github/instructions/chronos.instructions.md)
New handlers should match patterns like apps/chronos/src/routes/ping/index.ts: build them with
factory.createHandlers(...), keep Zod schemas close to the route, and include OpenAPI metadata withdescribeRoute(...)andfilcExt(...)
Files:
apps/chronos/src/routes/users/index.ts
apps/chronos/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/chronos.instructions.md)
Use
#imports defined in apps/chronos/package.json instead of long relative paths that cross feature boundaries
Files:
apps/chronos/src/routes/users/index.ts
apps/iris/src/routes/**/*.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-data-flow.instructions.md)
apps/iris/src/routes/**/*.tsx: Follow route composition patterns from apps/iris/src/routes/_private/admin/news/system-messages.tsx: usecreateFileRoute(...)at the top, then permission gating, then queries and mutations grouped near the component that owns them
Keep search-param-driven page state in TanStack Router when the page already uses it for filters or selection. Do not fork that state into unrelated local state
Files:
apps/iris/src/routes/_private/admin/timetable/substitutions.tsxapps/iris/src/routes/_private/admin/timetable/manage.tsxapps/iris/src/routes/_private/admin/roles.tsxapps/iris/src/routes/_private/admin/users.tsxapps/iris/src/routes/_private/admin/doorlock/cards.tsxapps/iris/src/routes/_private/admin/doorlock/devices.tsxapps/iris/src/routes/_private/admin/doorlock/index.tsx
apps/iris/src/routes/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/iris.instructions.md)
Treat apps/iris/src/route-tree.gen.ts as generated; change source files under apps/iris/src/routes instead
Files:
apps/iris/src/routes/_private/admin/timetable/substitutions.tsxapps/iris/src/routes/_private/admin/timetable/manage.tsxapps/iris/src/routes/_private/admin/roles.tsxapps/iris/src/routes/_private/admin/users.tsxapps/iris/src/routes/_private/admin/doorlock/cards.tsxapps/iris/src/routes/_private/admin/doorlock/devices.tsxapps/iris/src/routes/_private/admin/doorlock/index.tsx
apps/iris/src/components/**/*dialog.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-dialog-form.instructions.md)
apps/iris/src/components/**/*dialog.tsx: Follow the dialog structure used in files like card-dialog.tsx and user-dialog.tsx: create the form near the top of the component, derive reactive slices withuseStore(form.store, selector), and render fields with<form.Field>{(field) => ...}</form.Field>
Reuse validation schemas from apps/iris/src/utils/form-schemas.ts when available. If a schema becomes shared by multiple dialogs, move it there instead of copying validation logic
form.resettakes raw values, not{ values: ... }. Becauseform.resetandform.setFieldValueare not stable dependencies, do not add them touseEffectarrays when synchronizing dialog state
Reuse or extend shared dialog prop types such as admin.types.ts and doorlock.types.ts instead of defining near-duplicate props in each dialog
Keep submit side effects together: mutation success should close the dialog, invalidate the relevant query keys, and surface translated success or failure feedback
New labels, button text, placeholders, and empty states belong int(...)and the locale files, even if older dialogs still have hardcoded strings
Files:
apps/iris/src/components/admin/user-dialog.tsx
🔇 Additional comments (9)
apps/iris/src/components/subs.tsx (1)
57-57: LGTM!Also applies to: 71-72
apps/iris/src/components/system-message-banner.tsx (1)
102-104: LGTM!Also applies to: 115-115
apps/iris/src/routes/_private/admin/timetable/substitutions.tsx (1)
418-418: LGTM!Also applies to: 428-442
apps/chronos/src/routes/users/index.ts (1)
23-24: LGTM!Also applies to: 143-143, 148-148
apps/iris/src/components/admin/stat-card.tsx (1)
1-30: LGTM!apps/iris/src/components/admin/user-dialog.tsx (1)
19-25: LGTM!Also applies to: 53-67, 73-77, 83-83, 90-90, 95-99, 106-107, 136-137, 146-150, 157-158, 167-195, 197-197, 206-207, 234-234, 249-249
apps/iris/public/locales/en/translation.json (1)
14-14: LGTM!Also applies to: 116-119, 127-127, 137-137, 187-191, 238-239, 360-413, 484-545
apps/iris/public/locales/hu/translation.json (1)
104-107: LGTM!Also applies to: 115-115, 125-125, 175-179, 187-187, 238-239, 360-413, 484-545
apps/iris/src/routes/_private/admin/roles.tsx (1)
4-5: LGTM!Also applies to: 9-13, 24-25, 37-58, 60-67, 68-85, 86-117
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/iris/src/routes/_private/admin/doorlock/index.tsx`:
- Around line 64-85: The dashboard blocks should be skipped when the fetch has
failed and there is no data; update the render guard around the StatCard grid
(which currently uses isLoading and stats) to explicitly skip rendering when
there is neither loading nor stats (i.e. add a no-data guard: !isLoading &&
!stats). Locate the block that renders the StatCard grid (references: isLoading,
stats, StatCard) and change its condition to not render when !isLoading &&
!stats (for example wrap with a check like if (!isLoading && !stats) return null
/ use {!(!isLoading && !stats) && (...) } ) so the dashboard and any Skeleton
fallbacks are not shown after a terminal fetch failure.
In `@apps/iris/src/routes/_private/admin/timetable/substitutions.tsx`:
- Around line 446-462: Add the missing translation key used by
t('substitution.noCohorts'): update both locale trees by adding the "noCohorts"
entry under the "substitution" namespace for English and Hungarian (the en and
hu substitution sections) with appropriate user-facing strings (e.g., English
"No cohorts" and the corresponding Hungarian text). Ensure the keys are added to
the same nesting as other substitution keys so t('substitution.noCohorts')
resolves correctly in the UI and run a quick i18n lint or dev build to verify no
missing-key warnings.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3e024140-3e2e-49e3-ae69-a8ea0658cb9b
📒 Files selected for processing (9)
apps/iris/public/locales/en/translation.jsonapps/iris/public/locales/hu/translation.jsonapps/iris/src/components/admin/users-table.tsxapps/iris/src/routes/_private/admin/doorlock/cards.tsxapps/iris/src/routes/_private/admin/doorlock/devices.tsxapps/iris/src/routes/_private/admin/doorlock/index.tsxapps/iris/src/routes/_private/admin/timetable/manage.tsxapps/iris/src/routes/_private/admin/timetable/substitutions.tsxapps/iris/src/routes/_private/admin/users.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
apps/{chronos,iris}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/filc-reuse.instructions.md)
apps/{chronos,iris}/src/**/*.{ts,tsx}: Reuse existing helpers, types, schemas, and hooks before adding new ones. Check nearby feature folders first, then shared files such as apps/chronos/src/database/helpers.ts, apps/iris/src/utils/query-keys.ts, apps/iris/src/hooks/use-has-permission.ts, apps/iris/src/components/admin/admin.types.ts, and apps/iris/src/components/doorlock/doorlock.types.ts
When a second call site needs the same logic, prefer extracting or extending the existing abstraction instead of creating a parallel helper with a slightly different name
Keep abstractions local to the narrowest shared boundary that already exists. Do not create cross-app utilities for one feature-specific use
Extend existing dialog props, response shapes, and query key families instead of re-declaring near-identical types in each file
Prefer the smallest root-cause fix that matches neighboring code over broad rewrites or speculative cleanup
Keep imports on the app alias boundary:#...for Chronos and@/...for Iris
Files:
apps/iris/src/routes/_private/admin/timetable/substitutions.tsxapps/iris/src/routes/_private/admin/timetable/manage.tsxapps/iris/src/components/admin/users-table.tsxapps/iris/src/routes/_private/admin/users.tsxapps/iris/src/routes/_private/admin/doorlock/cards.tsxapps/iris/src/routes/_private/admin/doorlock/index.tsxapps/iris/src/routes/_private/admin/doorlock/devices.tsx
apps/iris/src/routes/**/*.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-data-flow.instructions.md)
apps/iris/src/routes/**/*.tsx: Follow route composition patterns from apps/iris/src/routes/_private/admin/news/system-messages.tsx: usecreateFileRoute(...)at the top, then permission gating, then queries and mutations grouped near the component that owns them
Keep search-param-driven page state in TanStack Router when the page already uses it for filters or selection. Do not fork that state into unrelated local state
Files:
apps/iris/src/routes/_private/admin/timetable/substitutions.tsxapps/iris/src/routes/_private/admin/timetable/manage.tsxapps/iris/src/routes/_private/admin/users.tsxapps/iris/src/routes/_private/admin/doorlock/cards.tsxapps/iris/src/routes/_private/admin/doorlock/index.tsxapps/iris/src/routes/_private/admin/doorlock/devices.tsx
apps/iris/src/{routes,components}/**/*.tsx
📄 CodeRabbit inference engine (.github/instructions/iris-data-flow.instructions.md)
apps/iris/src/{routes,components}/**/*.tsx: Always use centralized keys from apps/iris/src/utils/query-keys.ts for React Query. Do not introduce inline array query keys for existing domains
UseparseResponse(...)and the generated Hono client from apps/iris/src/utils/hc.ts for API requests when that is the local pattern
When a mutation changes server state, invalidate every affected query family, not just the page-local list. Follow the multi-invalidation pattern already used in admin news and doorlock screens
Reuse apps/iris/src/hooks/use-has-permission.ts and existing permission guard components instead of duplicating permission logic in the view
New user-facing error and success messages should go throught(...)and the locale files, even when surfaced through toasts
Files:
apps/iris/src/routes/_private/admin/timetable/substitutions.tsxapps/iris/src/routes/_private/admin/timetable/manage.tsxapps/iris/src/components/admin/users-table.tsxapps/iris/src/routes/_private/admin/users.tsxapps/iris/src/routes/_private/admin/doorlock/cards.tsxapps/iris/src/routes/_private/admin/doorlock/index.tsxapps/iris/src/routes/_private/admin/doorlock/devices.tsx
apps/iris/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/iris.instructions.md)
apps/iris/src/**/*.{ts,tsx}: Keep user-facing text int(...)and update both locale trees under apps/iris/public/locales/en and apps/iris/public/locales/hu
TanStack Form is the default form pattern; follow examples withuseForm,useStore(form.store, selector), and<form.Field>{(field) => ...}</form.Field>
form.reset(values)takes raw values, not{ values }.form.resetandform.setFieldValueare not stableuseEffectdependencies, so omit them from dependency arrays when needed
Base UI dropdown wrappers useonClick, not Radix-styleonSelect, unless the local component explicitly exposes a different API
apps/iris/src/components/ui/chart.tsx already ownsResponsiveContainer; do not wrap chart children in another one
Keep public timetable filter state in TanStack Router search params instead of duplicating it in unrelated local state
Files:
apps/iris/src/routes/_private/admin/timetable/substitutions.tsxapps/iris/src/routes/_private/admin/timetable/manage.tsxapps/iris/src/components/admin/users-table.tsxapps/iris/src/routes/_private/admin/users.tsxapps/iris/src/routes/_private/admin/doorlock/cards.tsxapps/iris/src/routes/_private/admin/doorlock/index.tsxapps/iris/src/routes/_private/admin/doorlock/devices.tsx
apps/iris/src/routes/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/iris.instructions.md)
Treat apps/iris/src/route-tree.gen.ts as generated; change source files under apps/iris/src/routes instead
Files:
apps/iris/src/routes/_private/admin/timetable/substitutions.tsxapps/iris/src/routes/_private/admin/timetable/manage.tsxapps/iris/src/routes/_private/admin/users.tsxapps/iris/src/routes/_private/admin/doorlock/cards.tsxapps/iris/src/routes/_private/admin/doorlock/index.tsxapps/iris/src/routes/_private/admin/doorlock/devices.tsx
🔇 Additional comments (8)
apps/iris/src/routes/_private/admin/timetable/manage.tsx (1)
352-364: LGTM!apps/iris/public/locales/en/translation.json (1)
510-512: LGTM!Also applies to: 546-547
apps/iris/src/components/admin/users-table.tsx (1)
30-39: LGTM!Also applies to: 57-57, 92-162
apps/iris/src/routes/_private/admin/users.tsx (1)
18-18: LGTM!apps/iris/src/routes/_private/admin/doorlock/cards.tsx (1)
62-71: LGTM!Also applies to: 338-442, 491-503
apps/iris/public/locales/hu/translation.json (1)
510-512: LGTM!Also applies to: 546-547
apps/iris/src/routes/_private/admin/doorlock/index.tsx (1)
109-109: LGTM!apps/iris/src/routes/_private/admin/doorlock/devices.tsx (1)
59-68: LGTM!Also applies to: 354-359, 361-365, 375-380, 382-386, 396-402, 403-407, 417-423, 424-428
| {(isLoading || stats) && ( | ||
| <div className="grid gap-4 md:grid-cols-3"> | ||
| <StatCard | ||
| icon={<IdCard className="text-primary" />} | ||
| isLoading={isLoading} | ||
| label={t('doorlockDashboard.totalCards')} | ||
| value={stats?.totalCards ?? 0} | ||
| /> | ||
| <StatCard | ||
| icon={<Microchip className="text-primary" />} | ||
| isLoading={isLoading} | ||
| label={t('doorlockDashboard.totalDevices')} | ||
| value={stats?.totalDevices ?? 0} | ||
| /> | ||
| <StatCard | ||
| icon={<DoorOpen className="text-primary" />} | ||
| isLoading={isLoading} | ||
| label={t('doorlockDashboard.successfulOpens')} | ||
| value={stats?.totalSuccessfulOpens ?? 0} | ||
| /> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Avoid showing loading placeholders after terminal fetch failure.
When the request fails and stats is absent, the alert renders, but downstream sections still show skeletons (stats ? ... : <Skeleton ...>), which looks like endless loading. Add a no-data/error guard (!isLoading && !stats) and skip dashboard content blocks in that state.
🤖 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 `@apps/iris/src/routes/_private/admin/doorlock/index.tsx` around lines 64 - 85,
The dashboard blocks should be skipped when the fetch has failed and there is no
data; update the render guard around the StatCard grid (which currently uses
isLoading and stats) to explicitly skip rendering when there is neither loading
nor stats (i.e. add a no-data guard: !isLoading && !stats). Locate the block
that renders the StatCard grid (references: isLoading, stats, StatCard) and
change its condition to not render when !isLoading && !stats (for example wrap
with a check like if (!isLoading && !stats) return null / use {!(!isLoading &&
!stats) && (...) } ) so the dashboard and any Skeleton fallbacks are not shown
after a terminal fetch failure.
There was a problem hiding this comment.
Already fixed. The current condition (isLoading || stats) on line 64 is logically equivalent to the requested guard: by De Morgan's law, it renders only when NOT (!isLoading && !stats). When both isLoading is false and stats is undefined (terminal fetch failure), the expression is falsy and the grid is skipped. No change needed.
| {(() => { | ||
| const cohorts = Array.from( | ||
| new Set( | ||
| sub.lessons | ||
| .filter((l) => l !== null && l !== undefined) | ||
| .flatMap((l) => l.cohorts) | ||
| ) | ||
| ); | ||
|
|
||
| return cohorts.length > 0 | ||
| ? cohorts.map((cohort) => ( | ||
| <Badge key={cohort} variant="secondary"> | ||
| {cohort} | ||
| </Badge> | ||
| )) | ||
| : t('substitution.noCohorts'); | ||
| })()} |
There was a problem hiding this comment.
Add missing substitution.noCohorts locale key in both trees.
Line 461 uses t('substitution.noCohorts'), but this key is missing from the provided en and hu substitution locale sections, so users may see the raw key instead of text.
As per coding guidelines, "Keep user-facing text in t(...) and update both locale trees under apps/iris/public/locales/en and apps/iris/public/locales/hu".
🤖 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 `@apps/iris/src/routes/_private/admin/timetable/substitutions.tsx` around lines
446 - 462, Add the missing translation key used by t('substitution.noCohorts'):
update both locale trees by adding the "noCohorts" entry under the
"substitution" namespace for English and Hungarian (the en and hu substitution
sections) with appropriate user-facing strings (e.g., English "No cohorts" and
the corresponding Hungarian text). Ensure the keys are added to the same nesting
as other substitution keys so t('substitution.noCohorts') resolves correctly in
the UI and run a quick i18n lint or dev build to verify no missing-key warnings.
|
Actionable comments posted: 0 |
Summary by CodeRabbit
New Features
Bug Fixes
Documentation