fix(admin-ui): service status shows accurate values on dashboard instead of marking Not present as Down #2505
Conversation
…ead of marking Not present as Down (#2500)
faisalsiddique4400
requested review from
duttarnab and
syntrydy
as code owners
📝 WalkthroughWalkthroughAdds and tightens TypeScript types across Redux slices and UI components, introduces explicit payload interfaces, updates reducer action typings, and makes health-status reducer assignments conditional to avoid misreporting service statuses. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 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 |
mo-auto
added
comp-admin-ui
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
admin-ui/app/redux/features/healthSlice.ts(3 hunks)admin-ui/app/redux/features/initSlice.ts(2 hunks)admin-ui/app/routes/Dashboards/DashboardPage.tsx(8 hunks)admin-ui/app/utils/PermChecker.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-06T08:23:20.948Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2418
File: admin-ui/plugins/user-management/components/UserForm.tsx:290-297
Timestamp: 2025-11-06T08:23:20.948Z
Learning: In the UserForm component (admin-ui/plugins/user-management/components/UserForm.tsx), the Back button should fall back to '/user/usersmanagement' when browser history is unavailable, not '/home/dashboard', as this keeps users in the user management context. This aligns with the new requirement that users should be redirected to the respective listing screen of the component rather than the dashboard.
Applied to files:
admin-ui/app/routes/Dashboards/DashboardPage.tsx
📚 Learning: 2025-12-05T18:24:41.681Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2498
File: admin-ui/plugins/saml/components/SamlConfigurationForm.tsx:30-30
Timestamp: 2025-12-05T18:24:41.681Z
Learning: In the GluuFederation/flex admin-ui codebase, `SetTitle` (imported from 'Utils/SetTitle') is a custom React hook that should be called at the component's top level, not inside `useEffect` or other hooks, because it internally manages side effects for setting page titles.
Applied to files:
admin-ui/app/routes/Dashboards/DashboardPage.tsx
🧬 Code graph analysis (2)
admin-ui/app/redux/features/initSlice.ts (1)
admin-ui/plugins/admin/redux/sagas/types/userAction.ts (1)
ActionPayload(7-9)
admin-ui/app/routes/Dashboards/DashboardPage.tsx (7)
admin-ui/app/cedarling/types/index.ts (2)
RootState(128-135)CedarPermissionsState(72-80)admin-ui/app/redux/features/initSlice.ts (1)
InitState(8-17)admin-ui/app/redux/features/types/authTypes.ts (1)
AuthState(25-45)admin-ui/app/redux/features/licenseDetailsSlice.ts (1)
LicenseDetailsState(21-24)admin-ui/app/redux/features/healthSlice.ts (1)
HealthState(22-27)admin-ui/app/utils/PermChecker.ts (3)
UserAction(138-142)buildPayload(154-161)ActionData(145-152)admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(75-104)
🔇 Additional comments (6)
admin-ui/app/redux/features/healthSlice.ts (1)
36-76: LGTM! Payload interfaces and typed reducers are well-structured.The conditional updates in
getHealthStatusResponse(lines 60-65) correctly prevent overwriting existing state when fields are missing from the response. The explicitPayloadActiontyping improves type safety.admin-ui/app/redux/features/initSlice.ts (1)
68-91: LGTM!The response handlers use appropriate defensive patterns with optional chaining and fallback values. The typed
PayloadActionparameters improve type safety.admin-ui/app/routes/Dashboards/DashboardPage.tsx (3)
317-346: LGTM! Status display logic correctly handles all backend states.The implementation now accurately displays the exact service status returned by the backend:
'Running'and'Online'show as healthy (check icon, green styling)'Not present','Down', and other states show their actual values with cross styling'Unknown'is only shown when the value is null/undefinedThis correctly addresses the PR objective of displaying "Not present" as-is instead of mapping it to "Down".
234-248: LGTM!The defensive
Array.isArray(lock)check before accessinglock[0]correctly handles the union typeLockDetailItem[] | LockDetail. The type assertion is safe given this guard.
60-74: Remove the suggestion to centralizeRootState— this is a component-specific definition, not a duplicate.The
RootStateinadmin-ui/app/cedarling/types/index.tscontains onlyauthReducer,cedarPermissions, andapiPermissionReducer. TheRootStatein DashboardPage.tsx includes those plus five additional reducers (mauReducer,initReducer,lockReducer,licenseDetailsReducer,healthReducer) required specifically for this component. This is an appropriate local type definition matching the component's actual Redux dependencies, not a duplicate causing maintenance drift.Likely an incorrect or invalid review comment.
admin-ui/app/utils/PermChecker.ts (1)
138-152: LGTM!The exported
UserActioninterface andActionDatatype provide clear contracts for action payloads. The union type forActionDataappropriately covers the various payload shapes used across the codebase.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/app/routes/Dashboards/DashboardPage.tsx (1)
180-204: PassuserActiontodispatch(getClients())on line 204.The
buildPayloadcall on line 203 populatesuserActionwith tracking data, butdispatch(getClients())doesn't pass it. ThegetClientssaga expects the action via payload to track user activity. Usedispatch(getClients({ action: userAction }))to match the pattern in Reports.tsx (line 68).Note:
dispatch(getLicenseDetails())on line 181 is correct since the corresponding saga doesn't use the action payload.
♻️ Duplicate comments (2)
admin-ui/app/redux/features/initSlice.ts (2)
4-21: Consolidate duplicate interface definitions.
GenericItem(lines 4-6) andActionDataPayload(lines 19-21) have identical structures. Consider using a single type to avoid duplication.Additionally, as noted in a previous review,
ActionPayloadis already defined inadmin-ui/plugins/admin/redux/sagas/types/userAction.ts. Consider importing from there instead.+import type { ActionPayload } from 'Plugins/admin/redux/sagas/types/userAction' + +type GenericItem = ActionPayload + -interface GenericItem { - [key: string]: string | number | boolean | string[] | number[] | boolean[] | null -} - export interface InitState { scripts: GenericItem[] // ... } -interface ActionDataPayload { - [key: string]: string | number | boolean | string[] | number[] | boolean[] | null -} +type ActionDataPayload = ActionPayload
93-96:isLoadingis never set totrue.As noted in a previous review,
isLoadingis only set tofalsehere, but no reducer sets it totrue. This makes the loading state ineffective. Consider addingstate.isLoading = truein an appropriate action likegetScripts,getClients,getScopes, orgetAttributes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
admin-ui/app/redux/features/initSlice.ts(2 hunks)admin-ui/app/routes/Dashboards/DashboardPage.tsx(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-05T18:24:41.681Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2498
File: admin-ui/plugins/saml/components/SamlConfigurationForm.tsx:30-30
Timestamp: 2025-12-05T18:24:41.681Z
Learning: In the GluuFederation/flex admin-ui codebase, `SetTitle` (imported from 'Utils/SetTitle') is a custom React hook that should be called at the component's top level, not inside `useEffect` or other hooks, because it internally manages side effects for setting page titles.
Applied to files:
admin-ui/app/routes/Dashboards/DashboardPage.tsx
🧬 Code graph analysis (1)
admin-ui/app/routes/Dashboards/DashboardPage.tsx (6)
admin-ui/app/cedarling/types/index.ts (2)
RootState(128-135)CedarPermissionsState(72-80)admin-ui/app/redux/features/initSlice.ts (1)
InitState(8-17)admin-ui/app/redux/features/types/authTypes.ts (1)
AuthState(25-45)admin-ui/app/redux/features/licenseDetailsSlice.ts (1)
LicenseDetailsState(21-24)admin-ui/app/utils/PermChecker.ts (3)
UserAction(138-142)buildPayload(154-161)ActionData(145-152)admin-ui/app/cedarling/hooks/useCedarling.ts (1)
useCedarling(16-268)
🔇 Additional comments (4)
admin-ui/app/redux/features/initSlice.ts (1)
61-98: LGTM on reducer implementations.The typed
PayloadActionusages are well-structured. Optional chaining and fallback values (|| [],|| 0) correctly guard against undefined data, ensuring state consistency.admin-ui/app/routes/Dashboards/DashboardPage.tsx (3)
317-346: Status handling correctly displays actual API values.The updated logic properly displays the exact service state from the backend:
getStatusTextreturns the raw value (e.g., "Running", "Down", "Not present")getClassNameandgetStatusIconuse visual indicators for "Running"/"Online" vs other statesThis directly addresses the PR objective of showing accurate service statuses instead of force-mapping "Not present" to "Down".
234-248: Good defensive guards on lock data access.The runtime check
Array.isArray(lock) && lock.length > 0before accessinglock[0]prevents potential runtime errors. The optional chaining (lockItem?.monthly_active_users) provides additional safety.
82-116: LGTM on typed selectors and state.The typed
useSelectorcalls withRootStateand explicit state types (number | null) improve type safety and IDE support. The typing is consistent with the imported state interfaces.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/app/routes/Dashboards/DashboardPage.tsx (1)
178-188: Module-level constant in dependency array.
FETCHING_LICENSE_DETAILSis a constant defined outside the component (line 72) and will never change. Including it in the dependency array is harmless but unnecessary.}, [ access_token, hasViewPermissions, license, loading, requestStates.licenseRequested, dispatch, userAction, options, - FETCHING_LICENSE_DETAILS, ])
♻️ Duplicate comments (2)
admin-ui/app/routes/Dashboards/DashboardPage.tsx (1)
50-69: Duplicate interfaces flagged in past reviews still present.
LockDetailItem(lines 50-53) is structurally identical to what was previously calledLockDetail. Additionally, the localRootStateinterface (lines 55-69) duplicates the one inadmin-ui/app/cedarling/types/index.ts. Consider consolidating these to a single source of truth.admin-ui/app/redux/features/initSlice.ts (1)
18-20:ActionDataPayloadduplicatesGenericItemstructure.Both
ActionDataPayloadandGenericItemhave identical shapes. Additionally, per the past review,ActionPayloadmay already exist inadmin-ui/plugins/admin/redux/sagas/types/userAction.ts. Consider consolidating to a single type.-interface ActionDataPayload { - [key: string]: string | number | boolean | string[] | number[] | boolean[] | null -} +type ActionDataPayload = GenericItem
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
admin-ui/app/redux/features/initSlice.ts(2 hunks)admin-ui/app/routes/Dashboards/DashboardPage.tsx(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-05T18:24:41.681Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2498
File: admin-ui/plugins/saml/components/SamlConfigurationForm.tsx:30-30
Timestamp: 2025-12-05T18:24:41.681Z
Learning: In the GluuFederation/flex admin-ui codebase, `SetTitle` (imported from 'Utils/SetTitle') is a custom React hook that should be called at the component's top level, not inside `useEffect` or other hooks, because it internally manages side effects for setting page titles.
Applied to files:
admin-ui/app/routes/Dashboards/DashboardPage.tsx
🧬 Code graph analysis (1)
admin-ui/app/routes/Dashboards/DashboardPage.tsx (6)
admin-ui/app/cedarling/types/index.ts (2)
RootState(128-135)CedarPermissionsState(72-80)admin-ui/plugins/auth-server/components/Sessions/types/index.ts (1)
RootState(10-10)admin-ui/app/redux/features/initSlice.ts (1)
InitState(8-16)admin-ui/app/redux/features/healthSlice.ts (1)
HealthState(22-27)admin-ui/app/utils/PermChecker.ts (2)
UserAction(138-142)ActionData(145-152)admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(75-104)
🔇 Additional comments (7)
admin-ui/app/routes/Dashboards/DashboardPage.tsx (4)
77-78: Good initialization pattern for typed user action.The explicit shape
{ action_message: '', action_data: null }ensuresuserActionconforms to theUserActioninterface from the start.
312-341: Status handling correctly addresses the PR objective.The refactored
getClassName,getStatusText, andgetStatusIconfunctions now:
- Display the exact backend status value (e.g., "Running", "Down", "Not present") via
getStatusText- Apply appropriate styling only for known "up" states (
RUNNING/ONLINE)- Return "Unknown" as a sensible fallback for missing values
This aligns with the fix to prevent "Not present" from being incorrectly mapped to "Down".
229-243: Defensive array handling forlockdata is appropriate.The guard
Array.isArray(lock) && lock.length > 0correctly handles the union typeLockDetailItem[] | LockDetailItemdefined inRootState. The optional chaining onlockItem(lines 234, 239) is slightly redundant after the length check but adds resilience.
297-297: Hardcodedstatus: falsefor SCIM is unused.The
statusfield instatusDetailsis only used fordbStatusandserverStatuslookups. For SCIM (key'jans-scim'),getStatusValuereturnsserverHealth['jans-scim'], ignoring the hardcodedfalse. Consider removing the unusedstatusfield from this entry or aligning the data structure.admin-ui/app/redux/features/initSlice.ts (3)
8-16:InitStateexport provides proper typing for consumers.The interface correctly types the slice's state shape and is properly exported for use in
DashboardPage.tsxand other consumers. The removal of the unusedisLoadingfield (noted in past reviews) has been addressed.
63-93: Reducer typing and null-safety improvements look correct.The reducers now use explicit payload types with proper optional chaining guards. The
handleApiTimeoutreducer directly assigns from the payload (line 92), addressing the prior concern aboutisTimeoutnever being set totrue.
80-83: Scopes response structure differs from other endpoints.
getScopesResponseassignsaction.payload.datadirectly (an array), while other handlers extract fromdata.entries. This is correctly reflected in the distinctScopesResponsePayloadinterface (lines 35-37).
|
The service status shows accurate values on dashboard instead of marking Not present as Down. |
|
4 participants
fix(admin-ui): service status shows accurate values on dashboard instead of marking Not present as Down (#2500)
Summary
The dashboard was incorrectly marking services with
"Not present"status as"Down", which created confusion and did not reflect the actual response returned by/jans-config-api/api/v1/health/service-status?service=all.Fix Summary
"Not present"state."Down".Result
Dashboard now displays real service state including
"Running","Down"and"Not present"without misinterpretation.🔗 Ticket
Closes: #2500
Summary by CodeRabbit
New Features
Bug Fixes
Other
✏️ Tip: You can customize this high-level summary in your review settings.