feat: implement maintenance mode frontend#950
Merged
Conversation
📝 WalkthroughWalkthroughAdds client- and realm-scoped maintenance mode UI and API hooks: toggle maintenance, manage client/realm whitelists, new WhitelistPicker component, maintenance pages and tabs, login error render for maintenance messages, and webhook trigger identifiers for maintenance enable/disable. Changes
Sequence Diagram(s)sequenceDiagram
participant User as rgba(66,133,244,0.5) User
participant UI as rgba(0,150,136,0.5) Frontend UI
participant TanStack as rgba(255,193,7,0.5) window.tanstackApi
participant Server as rgba(156,39,176,0.5) Backend API
participant Cache as rgba(3,169,244,0.5) QueryCache
participant Toast as rgba(233,30,99,0.5) Toast
User->>UI: Click toggle / add/remove whitelist
UI->>TanStack: call mutation/query options
TanStack->>Server: HTTP PUT/POST/DELETE/GET
Server-->>TanStack: response
TanStack-->>Cache: invalidate/update relevant queryKey(s)
TanStack-->>UI: resolve promise (data/error)
UI->>Toast: show success/error toast
UI-->>User: update UI state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
🧹 Nitpick comments (6)
front/src/pages/client/feature/page-client-maintenance-feature.tsx (3)
132-144: Local overrides not cleared after saving.After
handleSaveSettingssucceeds,reasonOverrideandstrategyOverrideremain set. While the query invalidation will update the server values, thehasSettingsChangescheck (line 130) will still betrue, keeping the FloatingActionBar visible until the component re-renders with updated data from the server.Consider clearing the overrides optimistically or in the mutation's
onSuccesscallback.♻️ Suggested fix to clear overrides after save
const handleSaveSettings = () => { toggleMaintenance({ body: { enabled: client.maintenance_enabled ?? false, reason: reason || undefined, session_strategy: strategy, }, path: { client_id: client.id, realm_name: realmName, }, - }) + }, { + onSuccess: () => { + setReasonOverride(undefined) + setStrategyOverride(undefined) + }, + }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/src/pages/client/feature/page-client-maintenance-feature.tsx` around lines 132 - 144, The local overrides (reasonOverride and strategyOverride) are not cleared after calling handleSaveSettings, so hasSettingsChanges stays true and the FloatingActionBar remains visible; update handleSaveSettings to clear those overrides optimistically or, better, clear them in the toggleMaintenance mutation's onSuccess callback: after toggleMaintenance completes, reset reasonOverride and strategyOverride (and any local state tied to hasSettingsChanges) so the UI reflects the saved server state; reference handleSaveSettings, toggleMaintenance, reasonOverride, strategyOverride and hasSettingsChanges when making the change.
65-65: Consider showing a loading skeleton instead of returning null.Returning
nullwhileclientis loading results in a blank page flash. A loading skeleton or spinner would improve the user experience.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/src/pages/client/feature/page-client-maintenance-feature.tsx` at line 65, The early-return "if (!client) return null" causes a blank flash; replace it with a loading UI so users see progress. Update the render logic in the Page/Component where the "client" variable is checked (the block with "if (!client) return null") to render a skeleton or spinner component (e.g., a <LoadingSkeleton> or <Spinner>) while client is undefined, and keep the existing UI render path unchanged once client is available; ensure the placeholder matches the layout so there’s no layout shift.
54-83: Heavy type casting suggests API typing gaps.The extensive
as unknown as ...andas anycasts forwhitelistResponse,realmWhitelistResponse,usersResponse, androlesResponseindicate the generated API client may be missing types for these endpoints. This is acceptable for now but should be addressed when regenerating the API client.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/src/pages/client/feature/page-client-maintenance-feature.tsx` around lines 54 - 83, The code uses heavy unsafe casts for whitelistResponse, realmWhitelistResponse, usersResponse, and rolesResponse which hides API typing gaps; replace the ad-hoc casts in the variables whitelist, realmWhitelist, users, and roles by using proper typed response shapes (create or import interfaces for the endpoint payloads or use the API client's response generics) and apply those concrete types instead of `any`/`unknown`/double-casts; if the API client is missing types, add explicit DTOs (e.g., WhitelistItem, RealmWhitelistItem, UserItem, RoleItem) and type the responses or update the generated client so you can rely on typed responses in the expressions that assign whitelist, realmWhitelist, users, and roles.front/src/api/maintenance.api.ts (1)
90-98: Consider adding explicitenabledoption for consistency.Unlike
useGetClientWhitelist, this hook doesn't explicitly control when the query is enabled. While the defaultrealm = 'master'ensures a truthy value, adding an explicitenabledoption would improve consistency and make the behavior clearer.♻️ Suggested change for consistency
export const useGetRealmWhitelist = ({ realm = 'master' }: BaseQuery) => { return useQuery( - // eslint-disable-next-line `@typescript-eslint/no-explicit-any` - (window.tanstackApi.get as any)( - '/realms/{realm_name}/settings/maintenance/whitelist', - { path: { realm_name: realm } } - ).queryOptions + { + // eslint-disable-next-line `@typescript-eslint/no-explicit-any` + ...(window.tanstackApi.get as any)( + '/realms/{realm_name}/settings/maintenance/whitelist', + { path: { realm_name: realm } } + ).queryOptions, + enabled: !!realm, + } ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/src/api/maintenance.api.ts` around lines 90 - 98, The hook useGetRealmWhitelist should explicitly pass an enabled flag to the query options like useGetClientWhitelist does; update the return value where you call (window.tanstackApi.get as any)('/realms/{realm_name}/settings/maintenance/whitelist', { path: { realm_name: realm } }).queryOptions to merge or set an enabled property (e.g., enabled: Boolean(realm) or enabled: realm !== undefined) so the query only runs when a valid realm is present and to keep behavior consistent with useGetClientWhitelist.front/src/pages/client/components/whitelist-picker.tsx (1)
148-157: Potential undefined access inentryIdMap.When calling
onRemove(entryIdMap[entry.id]), ifentryIdMapdoesn't containentry.id, this will passundefinedtoonRemove. While this should only happen forclient-sourced entries (which should always be in the map), adding a guard would make the code more defensive.🛡️ Suggested defensive check
{entry.source === 'client' && ( <Button type='button' variant='ghost' size='sm' - onClick={() => onRemove(entryIdMap[entry.id])} + onClick={() => { + const entryId = entryIdMap[entry.id] + if (entryId) onRemove(entryId) + }} > <Trash2 className='h-4 w-4 text-destructive' /> </Button> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/src/pages/client/components/whitelist-picker.tsx` around lines 148 - 157, The click handler may pass undefined to onRemove when entryIdMap lacks entry.id; update the onClick in the WhitelistPicker component to guard against missing mapping by resolving const removeId = entryIdMap[entry.id] and only calling onRemove(removeId) when removeId is defined (or alternatively disable/hide the Button when mapping is missing) so onRemove never receives undefined; reference entryIdMap, entry.id and onRemove to locate and change the handler.front/src/pages/client/components/client-maintenance-section.tsx (1)
166-194: Raw ID inputs for whitelist entries is poor UX.This component uses raw text inputs for user/role IDs instead of the searchable picker used in
PageClientMaintenanceFeature. Users would need to know and manually enter UUIDs, which is error-prone and unfriendly.If this component is still needed, consider using
WhitelistPickerfor consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/src/pages/client/components/client-maintenance-section.tsx` around lines 166 - 194, The UI currently asks for raw UUIDs via InputText (newUserId, newRoleId) and handlers handleAddUser/handleAddRole, which is poor UX; replace those raw text fields with the searchable WhitelistPicker used by PageClientMaintenanceFeature so users can lookup/select users and roles instead of typing IDs. Swap the InputText components for WhitelistPicker instances bound to the same state (or adapt state to accept the picker output), update the onChange to setNewUserId/setNewRoleId from the picker's selection, and ensure handleAddUser/handleAddRole consume the selected object/ID format the picker returns. Ensure labels/name props remain appropriate and remove/disable free-text entry so UUIDs are not required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@front/src/api/maintenance.api.ts`:
- Around line 12-19: Remove the unnecessary "as any" casts in
maintenance.api.ts: stop casting window.tanstackApi.get/put to any and use the
generated client types (e.g., get_Get_client_whitelist, put_Toggle_maintenance)
directly so queryOptions keeps full type-safety; replace lines like
"(window.tanstackApi.get as any)(...)." with "window.tanstackApi.get(...)" (and
the same for put) and remove the eslint-disable comment, and scan the file for
other "as any" occurrences and delete them so the generated types are used
end-to-end.
In `@front/src/pages/authentication/ui/page-login.tsx`:
- Around line 97-106: Normalize maintenance detection and parsing by using a
single case-insensitive regex against errorMessage (e.g., /under
maintenance(?:[:\s-]*)?(.*)$/i) to both detect the maintenance condition and
capture the optional reason; replace the current includes('under maintenance')
check with this regex test, use the captured group as the banner text but fall
back to a default like "Service under maintenance" if the capture is empty or
whitespace, and keep the existing banner structure (the Wrench icon and styling)
while removing the separate case-sensitive includes and the replace(/^.*under
maintenance:\s*/i, '') call.
In `@front/src/pages/client/components/client-maintenance-section.tsx`:
- Around line 33-63: Delete the unused ClientMaintenanceSection component and
all related dead code: remove the entire client-maintenance-section.tsx file
(including the ClientMaintenanceSection React component, its state hooks
reason/strategy/newUserId/newRoleId, and usages of useToggleMaintenance,
useGetClientWhitelist, useAddClientWhitelistEntry, useRemoveClientWhitelistEntry
and the local whitelist variable/handleToggle function) since
PageClientMaintenanceFeature replaces its behavior; ensure no imports reference
ClientMaintenanceSection remain elsewhere.
In `@front/src/pages/realm/feature/page-realm-settings-maintenance-feature.tsx`:
- Line 14: The current default assignment const realmName = realm_name ??
'master' can silently route writes to the wrong realm; change this to fail fast
when realm_name is missing by validating realm_name (the param) before use and
returning an error/early render instead of defaulting to 'master'. Locate the
realmName assignment in page-realm-settings-maintenance-feature (look for
realm_name and realmName) and replace the fallback with a guard that throws or
renders an error/placeholder UI when realm_name is undefined or empty, ensuring
any subsequent mutations always use an explicitly provided realmName.
---
Nitpick comments:
In `@front/src/api/maintenance.api.ts`:
- Around line 90-98: The hook useGetRealmWhitelist should explicitly pass an
enabled flag to the query options like useGetClientWhitelist does; update the
return value where you call (window.tanstackApi.get as
any)('/realms/{realm_name}/settings/maintenance/whitelist', { path: {
realm_name: realm } }).queryOptions to merge or set an enabled property (e.g.,
enabled: Boolean(realm) or enabled: realm !== undefined) so the query only runs
when a valid realm is present and to keep behavior consistent with
useGetClientWhitelist.
In `@front/src/pages/client/components/client-maintenance-section.tsx`:
- Around line 166-194: The UI currently asks for raw UUIDs via InputText
(newUserId, newRoleId) and handlers handleAddUser/handleAddRole, which is poor
UX; replace those raw text fields with the searchable WhitelistPicker used by
PageClientMaintenanceFeature so users can lookup/select users and roles instead
of typing IDs. Swap the InputText components for WhitelistPicker instances bound
to the same state (or adapt state to accept the picker output), update the
onChange to setNewUserId/setNewRoleId from the picker's selection, and ensure
handleAddUser/handleAddRole consume the selected object/ID format the picker
returns. Ensure labels/name props remain appropriate and remove/disable
free-text entry so UUIDs are not required.
In `@front/src/pages/client/components/whitelist-picker.tsx`:
- Around line 148-157: The click handler may pass undefined to onRemove when
entryIdMap lacks entry.id; update the onClick in the WhitelistPicker component
to guard against missing mapping by resolving const removeId =
entryIdMap[entry.id] and only calling onRemove(removeId) when removeId is
defined (or alternatively disable/hide the Button when mapping is missing) so
onRemove never receives undefined; reference entryIdMap, entry.id and onRemove
to locate and change the handler.
In `@front/src/pages/client/feature/page-client-maintenance-feature.tsx`:
- Around line 132-144: The local overrides (reasonOverride and strategyOverride)
are not cleared after calling handleSaveSettings, so hasSettingsChanges stays
true and the FloatingActionBar remains visible; update handleSaveSettings to
clear those overrides optimistically or, better, clear them in the
toggleMaintenance mutation's onSuccess callback: after toggleMaintenance
completes, reset reasonOverride and strategyOverride (and any local state tied
to hasSettingsChanges) so the UI reflects the saved server state; reference
handleSaveSettings, toggleMaintenance, reasonOverride, strategyOverride and
hasSettingsChanges when making the change.
- Line 65: The early-return "if (!client) return null" causes a blank flash;
replace it with a loading UI so users see progress. Update the render logic in
the Page/Component where the "client" variable is checked (the block with "if
(!client) return null") to render a skeleton or spinner component (e.g., a
<LoadingSkeleton> or <Spinner>) while client is undefined, and keep the existing
UI render path unchanged once client is available; ensure the placeholder
matches the layout so there’s no layout shift.
- Around line 54-83: The code uses heavy unsafe casts for whitelistResponse,
realmWhitelistResponse, usersResponse, and rolesResponse which hides API typing
gaps; replace the ad-hoc casts in the variables whitelist, realmWhitelist,
users, and roles by using proper typed response shapes (create or import
interfaces for the endpoint payloads or use the API client's response generics)
and apply those concrete types instead of `any`/`unknown`/double-casts; if the
API client is missing types, add explicit DTOs (e.g., WhitelistItem,
RealmWhitelistItem, UserItem, RoleItem) and type the responses or update the
generated client so you can rely on typed responses in the expressions that
assign whitelist, realmWhitelist, users, and roles.
🪄 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
Run ID: d4557d33-88fb-413f-b672-acdd64e1a87a
📒 Files selected for processing (13)
front/src/api/api.client.tsfront/src/api/maintenance.api.tsfront/src/pages/authentication/ui/page-login.tsxfront/src/pages/client/components/client-maintenance-section.tsxfront/src/pages/client/components/whitelist-picker.tsxfront/src/pages/client/feature/page-client-maintenance-feature.tsxfront/src/pages/client/layout/client-layout.tsxfront/src/pages/client/page-client.tsxfront/src/pages/realm/feature/page-realm-settings-maintenance-feature.tsxfront/src/pages/realm/layouts/realm-settings-layout.tsxfront/src/pages/realm/page-realm.tsxfront/src/pages/realm/ui/page-realm-settings.tsxfront/src/utils/webhook-utils.ts
Comment on lines
+97
to
+106
| errorMessage.includes('under maintenance') ? ( | ||
| <div className='rounded-md border border-amber-500/30 bg-amber-500/10 px-4 py-3 text-sm text-amber-700 dark:text-amber-400 flex items-start gap-2'> | ||
| <Wrench className='h-4 w-4 mt-0.5 shrink-0' /> | ||
| <span>{errorMessage.replace(/^.*under maintenance:\s*/i, '')}</span> | ||
| </div> | ||
| ) : ( | ||
| <div className='rounded-md border border-destructive/30 bg-destructive/10 px-4 py-3 text-sm text-destructive'> | ||
| {errorMessage} | ||
| </div> | ||
| ) |
There was a problem hiding this comment.
Harden maintenance-message parsing to avoid false negatives and empty banners.
On Line 97, includes('under maintenance') is case-sensitive, while Line 100 strips the prefix case-insensitively. Also, if no reason follows the prefix, the <span> can render empty text. Please normalize detection/parsing once and provide a fallback message.
💡 Proposed fix
const providers = loginSettings.identity_providers ?? []
+ const isMaintenanceError = /under maintenance/i.test(errorMessage ?? '')
+ const maintenanceReason =
+ errorMessage?.match(/under maintenance:\s*(.*)$/i)?.[1]?.trim() ||
+ 'This service is currently under maintenance.'
...
- {errorMessage && (
- errorMessage.includes('under maintenance') ? (
- <div className='rounded-md border border-amber-500/30 bg-amber-500/10 px-4 py-3 text-sm text-amber-700 dark:text-amber-400 flex items-start gap-2'>
+ {errorMessage && (
+ isMaintenanceError ? (
+ <div
+ role='alert'
+ className='rounded-md border border-amber-500/30 bg-amber-500/10 px-4 py-3 text-sm text-amber-700 dark:text-amber-400 flex items-start gap-2'
+ >
<Wrench className='h-4 w-4 mt-0.5 shrink-0' />
- <span>{errorMessage.replace(/^.*under maintenance:\s*/i, '')}</span>
+ <span>{maintenanceReason}</span>
</div>
) : (📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| errorMessage.includes('under maintenance') ? ( | |
| <div className='rounded-md border border-amber-500/30 bg-amber-500/10 px-4 py-3 text-sm text-amber-700 dark:text-amber-400 flex items-start gap-2'> | |
| <Wrench className='h-4 w-4 mt-0.5 shrink-0' /> | |
| <span>{errorMessage.replace(/^.*under maintenance:\s*/i, '')}</span> | |
| </div> | |
| ) : ( | |
| <div className='rounded-md border border-destructive/30 bg-destructive/10 px-4 py-3 text-sm text-destructive'> | |
| {errorMessage} | |
| </div> | |
| ) | |
| const providers = loginSettings.identity_providers ?? [] | |
| const isMaintenanceError = /under maintenance/i.test(errorMessage ?? '') | |
| const maintenanceReason = | |
| errorMessage?.match(/under maintenance:\s*(.*)$/i)?.[1]?.trim() || | |
| 'This service is currently under maintenance.' | |
| // ... other code ... | |
| {errorMessage && ( | |
| isMaintenanceError ? ( | |
| <div | |
| role='alert' | |
| className='rounded-md border border-amber-500/30 bg-amber-500/10 px-4 py-3 text-sm text-amber-700 dark:text-amber-400 flex items-start gap-2' | |
| > | |
| <Wrench className='h-4 w-4 mt-0.5 shrink-0' /> | |
| <span>{maintenanceReason}</span> | |
| </div> | |
| ) : ( | |
| <div className='rounded-md border border-destructive/30 bg-destructive/10 px-4 py-3 text-sm text-destructive'> | |
| {errorMessage} | |
| </div> | |
| ) | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@front/src/pages/authentication/ui/page-login.tsx` around lines 97 - 106,
Normalize maintenance detection and parsing by using a single case-insensitive
regex against errorMessage (e.g., /under maintenance(?:[:\s-]*)?(.*)$/i) to both
detect the maintenance condition and capture the optional reason; replace the
current includes('under maintenance') check with this regex test, use the
captured group as the banner text but fall back to a default like "Service under
maintenance" if the capture is empty or whitespace, and keep the existing banner
structure (the Wrench icon and styling) while removing the separate
case-sensitive includes and the replace(/^.*under maintenance:\s*/i, '') call.
|
|
||
| export default function PageRealmSettingsMaintenanceFeature() { | ||
| const { realm_name } = useParams<RouterParams>() | ||
| const realmName = realm_name ?? 'master' |
There was a problem hiding this comment.
Avoid defaulting to master when realm_name is missing.
Line 14 can silently redirect mutations to the wrong realm. If params are missing, fail fast (or render an error) instead of writing to master.
💡 Safer guard
export default function PageRealmSettingsMaintenanceFeature() {
const { realm_name } = useParams<RouterParams>()
- const realmName = realm_name ?? 'master'
+ if (!realm_name) {
+ return <div className='text-sm text-destructive'>Missing realm context.</div>
+ }
+ const realmName = realm_name🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@front/src/pages/realm/feature/page-realm-settings-maintenance-feature.tsx` at
line 14, The current default assignment const realmName = realm_name ?? 'master'
can silently route writes to the wrong realm; change this to fail fast when
realm_name is missing by validating realm_name (the param) before use and
returning an error/early render instead of defaulting to 'master'. Locate the
realmName assignment in page-realm-settings-maintenance-feature (look for
realm_name and realmName) and replace the fallback with a guard that throws or
renders an error/placeholder UI when realm_name is undefined or empty, ensuring
any subsequent mutations always use an explicitly provided realmName.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
front/src/pages/client/feature/page-client-maintenance-feature.tsx (1)
57-69: Usereact-hook-form+ Zod for the maintenance settings form.This page has field state, dirty tracking, save, and reset behavior, but it is implemented with ad-hoc
useState. Moving reason/strategy into a schema-backed form would centralize defaults, validation, and reset semantics and reduce the state drift already showing up here. As per coding guidelines, "Frontend: Use react-hook-form with Zod validation for form handling" and "Validate frontend forms with react-hook-form + Zod validation schemas."Also applies to: 198-228, 279-290
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/src/pages/client/feature/page-client-maintenance-feature.tsx` around lines 57 - 69, Replace the ad-hoc useState tracking (reasonOverride, setReasonOverride, strategyOverride, setStrategyOverride) and computed values (serverReason, serverStrategy, reason, strategy) with a react-hook-form form using a Zod schema: create a Zod schema for maintenance_reason and maintenance_session_strategy, initialize useForm with zodResolver and defaultValues set from client (serverReason/serverStrategy), use formState.isDirty to track edits, use handleSubmit for save, and call reset(formDefaults) whenever the client prop changes to restore server defaults; update UI bindings to use register/watch instead of the old state setters and remove the manual reason/strategy variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@front/src/pages/client/feature/page-client-maintenance-feature.tsx`:
- Around line 105-132: handleToggle is persisting unsaved draft fields
(reason/strategy) and handleSaveSettings reads the stale
client.maintenance_enabled; change handleToggle to only send the enabled flag
(no reason/session_strategy) so flipping the switch doesn't save drafts, and
change handleSaveSettings to send the current draft payload (reason or
reasonOverride, strategy or strategyOverride) plus the current UI enabled state
(track a local enabledDraft or use the enabled passed into handleToggle/save)
instead of reading client.maintenance_enabled from query state; update
occurrences in handleToggle and handleSaveSettings (and the similar block at
lines ~178-183) to reference these local draft values and avoid relying on
client.maintenance_enabled.
- Around line 57-69: The dirty-tracking currently uses whether an override was
set (reasonOverride/strategyOverride) rather than whether the effective values
differ from the server; change the change-detection to compare the effective
values (reason = reasonOverride ?? serverReason, strategy = strategyOverride ??
serverStrategy) against serverReason and serverStrategy (e.g. set
hasSettingsChanges = reason !== serverReason || strategy !== serverStrategy),
and after a successful save clear the overrides (setReasonOverride(undefined),
setStrategyOverride(undefined)) so the action bar closes; apply the same
comparison fix to the other occurrences that compute effective values (the other
blocks referencing reasonOverride/strategyOverride at the noted sections).
---
Nitpick comments:
In `@front/src/pages/client/feature/page-client-maintenance-feature.tsx`:
- Around line 57-69: Replace the ad-hoc useState tracking (reasonOverride,
setReasonOverride, strategyOverride, setStrategyOverride) and computed values
(serverReason, serverStrategy, reason, strategy) with a react-hook-form form
using a Zod schema: create a Zod schema for maintenance_reason and
maintenance_session_strategy, initialize useForm with zodResolver and
defaultValues set from client (serverReason/serverStrategy), use
formState.isDirty to track edits, use handleSubmit for save, and call
reset(formDefaults) whenever the client prop changes to restore server defaults;
update UI bindings to use register/watch instead of the old state setters and
remove the manual reason/strategy variables.
🪄 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
Run ID: d1a46e0a-a879-4f96-9dd1-70bffc7c6d7b
📒 Files selected for processing (4)
front/src/api/maintenance.api.tsfront/src/pages/client/components/whitelist-picker.tsxfront/src/pages/client/feature/page-client-maintenance-feature.tsxfront/src/pages/realm/feature/page-realm-settings-maintenance-feature.tsx
✅ Files skipped from review due to trivial changes (1)
- front/src/pages/client/components/whitelist-picker.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- front/src/pages/realm/feature/page-realm-settings-maintenance-feature.tsx
- front/src/api/maintenance.api.ts
Comment on lines
+57
to
+69
| // Track user edits — undefined means "not edited, use server value" | ||
| const [reasonOverride, setReasonOverride] = useState<string | undefined>(undefined) | ||
| const [strategyOverride, setStrategyOverride] = useState<'terminate' | 'expire' | undefined>( | ||
| undefined | ||
| ) | ||
|
|
||
| if (!client) return null | ||
|
|
||
| const serverReason = client.maintenance_reason ?? '' | ||
| const serverStrategy = | ||
| (client.maintenance_session_strategy as 'terminate' | 'expire') ?? 'expire' | ||
| const reason = reasonOverride ?? serverReason | ||
| const strategy = strategyOverride ?? serverStrategy |
There was a problem hiding this comment.
Dirty tracking is based on “was edited” instead of “actually changed”.
hasSettingsChanges stays true once an override is set, even if the user restores the original value. It also keeps the action bar open after a successful save until they manually cancel. Compare the effective values against the server values instead.
♻️ Minimal fix
- const hasSettingsChanges = reasonOverride !== undefined || strategyOverride !== undefined
+ const hasSettingsChanges = reason !== serverReason || strategy !== serverStrategyAlso applies to: 119-138, 279-290
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@front/src/pages/client/feature/page-client-maintenance-feature.tsx` around
lines 57 - 69, The dirty-tracking currently uses whether an override was set
(reasonOverride/strategyOverride) rather than whether the effective values
differ from the server; change the change-detection to compare the effective
values (reason = reasonOverride ?? serverReason, strategy = strategyOverride ??
serverStrategy) against serverReason and serverStrategy (e.g. set
hasSettingsChanges = reason !== serverReason || strategy !== serverStrategy),
and after a successful save clear the overrides (setReasonOverride(undefined),
setStrategyOverride(undefined)) so the action bar closes; apply the same
comparison fix to the other occurrences that compute effective values (the other
blocks referencing reasonOverride/strategyOverride at the noted sections).
Comment on lines
+105
to
+132
| const handleToggle = (enabled: boolean) => { | ||
| toggleMaintenance({ | ||
| body: { | ||
| enabled, | ||
| reason: reason || undefined, | ||
| session_strategy: strategy, | ||
| }, | ||
| path: { | ||
| client_id: client.id, | ||
| realm_name: realmName, | ||
| }, | ||
| }) | ||
| } | ||
|
|
||
| const hasSettingsChanges = reasonOverride !== undefined || strategyOverride !== undefined | ||
|
|
||
| const handleSaveSettings = () => { | ||
| toggleMaintenance({ | ||
| body: { | ||
| enabled: client.maintenance_enabled ?? false, | ||
| reason: reason || undefined, | ||
| session_strategy: strategy, | ||
| }, | ||
| path: { | ||
| client_id: client.id, | ||
| realm_name: realmName, | ||
| }, | ||
| }) |
There was a problem hiding this comment.
Separate the immediate toggle from the draft settings payload.
handleToggle currently persists reason and strategy, so flipping the switch also saves unsaved edits even though the page still shows “Save Changes”. Then handleSaveSettings reads client.maintenance_enabled back from query state, so a toggle+save flow can send the old flag back before the refetch lands and leave maintenance in the wrong state.
Also applies to: 178-183
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@front/src/pages/client/feature/page-client-maintenance-feature.tsx` around
lines 105 - 132, handleToggle is persisting unsaved draft fields
(reason/strategy) and handleSaveSettings reads the stale
client.maintenance_enabled; change handleToggle to only send the enabled flag
(no reason/session_strategy) so flipping the switch doesn't save drafts, and
change handleSaveSettings to send the current draft payload (reason or
reasonOverride, strategy or strategyOverride) plus the current UI enabled state
(track a local enabledDraft or use the enabled passed into handleToggle/save)
instead of reading client.maintenance_enabled from query state; update
occurrences in handleToggle and handleSaveSettings (and the similar block at
lines ~178-183) to reference these local draft values and avoid relying on
client.maintenance_enabled.
NathaelB
approved these changes
Apr 14, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@front/src/api/maintenance.api.ts`:
- Around line 5-14: The hook useGetClientWhitelist incorrectly defaults
realm='master' causing requests to silently target master; remove the default so
realm is optional (do not set realm = 'master'), require callers to pass an
explicit realm, and change the query enabled gating to check for an explicit
realm value (e.g., enabled: !!clientId && realm != null && realm !== '') so it
does not run when realm is unresolved; apply the same change to the other
maintenance whitelist hook referenced around lines 76-81 so both align with
useGetClient's explicit-realm behavior.
🪄 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
Run ID: 94282172-3188-4253-980b-1da0aa58990e
📒 Files selected for processing (1)
front/src/api/maintenance.api.ts
Comment on lines
+5
to
+14
| export function useGetClientWhitelist({ | ||
| realm = 'master', | ||
| clientId, | ||
| }: BaseQuery & { clientId?: string }) { | ||
| return useQuery({ | ||
| ...window.tanstackApi.get('/realms/{realm_name}/clients/{client_id}/maintenance/whitelist', { | ||
| path: { realm_name: realm, client_id: clientId! }, | ||
| }).queryOptions, | ||
| enabled: !!clientId && !!realm, | ||
| }) |
There was a problem hiding this comment.
Don't silently fall back to the master realm.
Both query hooks will hit /realms/master/... whenever the caller hasn't resolved realm context yet. In useGetClientWhitelist, Line 13 no longer protects that case because !!realm is always true after the fallback. That can cache and render another realm's whitelist in multi-realm flows. Please align these with useGetClient and require an explicit realm plus enabled gating.
🛠️ Proposed fix
export function useGetClientWhitelist({
- realm = 'master',
+ realm,
clientId,
}: BaseQuery & { clientId?: string }) {
return useQuery({
...window.tanstackApi.get('/realms/{realm_name}/clients/{client_id}/maintenance/whitelist', {
- path: { realm_name: realm, client_id: clientId! },
+ path: { realm_name: realm!, client_id: clientId! },
}).queryOptions,
enabled: !!clientId && !!realm,
})
}
@@
-export function useGetRealmWhitelist({ realm = 'master' }: BaseQuery) {
- return useQuery(
- window.tanstackApi.get('/realms/{realm_name}/clients/settings/maintenance/whitelist', {
- path: { realm_name: realm },
- }).queryOptions
- )
+export function useGetRealmWhitelist({ realm }: BaseQuery) {
+ return useQuery({
+ ...window.tanstackApi.get('/realms/{realm_name}/clients/settings/maintenance/whitelist', {
+ path: { realm_name: realm! },
+ }).queryOptions,
+ enabled: !!realm,
+ })
}Also applies to: 76-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@front/src/api/maintenance.api.ts` around lines 5 - 14, The hook
useGetClientWhitelist incorrectly defaults realm='master' causing requests to
silently target master; remove the default so realm is optional (do not set
realm = 'master'), require callers to pass an explicit realm, and change the
query enabled gating to check for an explicit realm value (e.g., enabled:
!!clientId && realm != null && realm !== '') so it does not run when realm is
unresolved; apply the same change to the other maintenance whitelist hook
referenced around lines 76-81 so both align with useGetClient's explicit-realm
behavior.
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.
Summary by CodeRabbit
New Features
Bug Fixes