feat(admin): adding uniformity in cancel and back buttons present in Users and Whole Home Module #2418
Conversation
…i-issue-2361-scim
…Jans Lock (#2405) * feat(admin): adding uniformity in cancel and back buttons present in Jans Lock * feat(admin): adding uniformity in cancel and back buttons present in Jans Lock * Code Rabbit fixes
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/plugins/services/Components/Configuration/LdapAddPage.js (1)
15-23: Add missing dependencies to useEffect.The useEffect hook uses
navigateBackanddispatchbut doesn't include them in the dependency array. This violates the React hooks exhaustive-deps rule and could lead to stale closures.Apply this diff to fix the dependency array:
return () => { dispatch(toggleSavedFormFlag(false)) } - }, [savedForm]) + }, [savedForm, navigateBack, dispatch])
♻️ Duplicate comments (4)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx (1)
76-78: UsenavigateBackwith dashboard fallback for Back buttonThe Back button currently always routes directly to
HOME_DASHBOARD, ignoring browser history. UsingnavigateBack(ROUTES.HOME_DASHBOARD)would honor actual navigation history when available while maintaining the dashboard fallback.Apply this diff:
- const { navigateToRoute } = useAppNavigation() + const { navigateBack } = useAppNavigation() const handleBack = () => { - navigateToRoute(ROUTES.HOME_DASHBOARD) + navigateBack(ROUTES.HOME_DASHBOARD) }Also update line 34 to destructure
navigateBackinstead ofnavigateToRoute.admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
43-43: Type casting via unknown is a workaround.Similar to
UserAddPage.tsx, the double castas unknown as PersonAttribute[]works around a type mismatch. Consider the same refactoring approach suggested for UserAddPage to improve type safety.admin-ui/plugins/user-management/components/UserList.tsx (1)
120-123: Use stable domain identifier (inum) instead oftableData.uuidfor edit navigationThe edit action currently derives the route id as:
const userId = row.tableData?.uuid || row.inum
tableData.uuidis a MaterialTable‑internal identifier, not a stable domain key, while the table explicitly configuresidSynonym: 'inum'. Routing via an internal uuid risks breakage if MaterialTable changes its internals or if uuid differs from the backend user identifier.Prefer the domain identifier (e.g.,
inum, oruserIdif that is the real key) and drop reliance ontableData.uuid:- const userData = row as unknown as CustomUser - const userId = row.tableData?.uuid || row.inum + const userData = row as unknown as CustomUser + const userId = row.inum || row.userId if (!userId) return navigateToRoute(ROUTES.USER_EDIT(userId), { state: { selectedUser: userData } })Please align
userIdwith whatROUTES.USER_EDITand the backend expect.Also applies to: 146-154, 219-233
admin-ui/plugins/user-management/components/UserForm.tsx (1)
34-79: Cancel still doesn’t fully revert dynamically added custom claims
setupCustomAttributesappends attributes fromuserDetails.customAttributesonto the existingselectedClaims:setSelectedClaims((prevSelectedClaims) => { const existingClaimNames = new Set(prevSelectedClaims.map((claim) => claim.name)) const tempList = [...prevSelectedClaims] ... tempList.push(data) ... return tempList })
handleCancelcallssetupCustomAttributeswithout clearingselectedClaimsfirst:const handleCancel = useCallback(() => { const resetValues = initializeCustomAttributes(...) formik.resetForm({ values: resetValues }) setModifiedFields({}) if (userDetails) { setupCustomAttributes(userDetails, memoizedPersonAttributes, setSelectedClaims) initializedRef.current = userDetails.inum || 'new' } else { setSelectedClaims([]) initializedRef.current = 'new' } }, ...)If the user added new custom claims during this session that are not present in
userDetails.customAttributes, those extra claims remain inselectedClaimsafter Cancel, so their rows stay visible even though the form values were reset. This means Cancel does not fully restore the original dynamic-claims UI state.Reset
selectedClaimsbefore repopulating fromuserDetailsso Cancel truly reverts to the initial set of claims:const handleCancel = useCallback(() => { const resetValues = initializeCustomAttributes(userDetails || null, memoizedPersonAttributes) formik.resetForm({ values: resetValues }) setModifiedFields({}) if (userDetails) { - setupCustomAttributes(userDetails, memoizedPersonAttributes, setSelectedClaims) - initializedRef.current = userDetails.inum || 'new' + // Drop any newly added claims and restore from original userDetails + setSelectedClaims([]) + setupCustomAttributes(userDetails, memoizedPersonAttributes, setSelectedClaims) + initializedRef.current = userDetails.inum || 'new' } else { setSelectedClaims([]) initializedRef.current = 'new' } }, [formik, userDetails, memoizedPersonAttributes, setSelectedClaims])This keeps the initialization guard while ensuring Cancel resets both field values and dynamic claim rows.
Also applies to: 155-167, 182-185, 186-203, 383-393
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (28)
admin-ui/app/helpers/navigation.ts(1 hunks)admin-ui/app/routes/Apps/Gluu/GluuAppSidebar.tsx(3 hunks)admin-ui/app/routes/Apps/Gluu/GluuFormFooter.tsx(5 hunks)admin-ui/app/routes/Apps/Gluu/GluuSessionTimeout.tsx(3 hunks)admin-ui/app/routes/Apps/Gluu/GluuTabs.tsx(4 hunks)admin-ui/app/routes/Apps/Profile/ProfilePage.tsx(2 hunks)admin-ui/app/routes/Dashboards/DashboardPage.tsx(3 hunks)admin-ui/app/routes/License/LicenseDetailsPage.tsx(2 hunks)admin-ui/app/routes/components/Dropdowns/DropdownProfile.tsx(1 hunks)admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx(3 hunks)admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js(2 hunks)admin-ui/plugins/saml/components/SamlIdentityList.js(3 hunks)admin-ui/plugins/saml/components/SamlIdpForm.js(3 hunks)admin-ui/plugins/saml/components/TrustRelationForm.js(3 hunks)admin-ui/plugins/saml/components/TrustRelationshipList.js(3 hunks)admin-ui/plugins/schema/components/Person/AttributeAddPage.tsx(2 hunks)admin-ui/plugins/schema/components/Person/AttributeEditPage.tsx(3 hunks)admin-ui/plugins/schema/components/Person/AttributeListPage.tsx(4 hunks)admin-ui/plugins/services/Components/Configuration/LdapAddPage.js(1 hunks)admin-ui/plugins/services/Components/Configuration/LdapEditPage.js(1 hunks)admin-ui/plugins/services/Components/Configuration/LdapListPage.js(5 hunks)admin-ui/plugins/services/Components/Configuration/SqlAddPage.js(1 hunks)admin-ui/plugins/services/Components/Configuration/SqlEditPage.js(1 hunks)admin-ui/plugins/services/Components/Configuration/SqlListPage.js(4 hunks)admin-ui/plugins/user-management/components/UserAddPage.tsx(3 hunks)admin-ui/plugins/user-management/components/UserEditPage.tsx(4 hunks)admin-ui/plugins/user-management/components/UserForm.tsx(5 hunks)admin-ui/plugins/user-management/components/UserList.tsx(6 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
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.
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.
Applied to files:
admin-ui/plugins/services/Components/Configuration/SqlAddPage.jsadmin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsxadmin-ui/plugins/user-management/components/UserAddPage.tsxadmin-ui/app/routes/Apps/Profile/ProfilePage.tsxadmin-ui/plugins/schema/components/Person/AttributeEditPage.tsxadmin-ui/app/routes/Apps/Gluu/GluuAppSidebar.tsxadmin-ui/plugins/user-management/components/UserForm.tsxadmin-ui/plugins/saml/components/TrustRelationForm.jsadmin-ui/plugins/auth-server/components/Configuration/ConfigPage.jsadmin-ui/plugins/services/Components/Configuration/LdapListPage.jsadmin-ui/plugins/services/Components/Configuration/LdapEditPage.jsadmin-ui/plugins/saml/components/SamlIdpForm.jsadmin-ui/app/routes/Apps/Gluu/GluuFormFooter.tsxadmin-ui/app/routes/Apps/Gluu/GluuSessionTimeout.tsxadmin-ui/plugins/saml/components/SamlIdentityList.jsadmin-ui/app/helpers/navigation.tsadmin-ui/app/routes/Dashboards/DashboardPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapAddPage.jsadmin-ui/plugins/schema/components/Person/AttributeAddPage.tsxadmin-ui/plugins/services/Components/Configuration/SqlEditPage.js
📚 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/plugins/services/Components/Configuration/SqlAddPage.jsadmin-ui/plugins/user-management/components/UserEditPage.tsxadmin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsxadmin-ui/plugins/schema/components/Person/AttributeListPage.tsxadmin-ui/app/routes/License/LicenseDetailsPage.tsxadmin-ui/plugins/user-management/components/UserAddPage.tsxadmin-ui/app/routes/Apps/Profile/ProfilePage.tsxadmin-ui/plugins/schema/components/Person/AttributeEditPage.tsxadmin-ui/app/routes/Apps/Gluu/GluuAppSidebar.tsxadmin-ui/plugins/user-management/components/UserForm.tsxadmin-ui/plugins/saml/components/TrustRelationForm.jsadmin-ui/plugins/auth-server/components/Configuration/ConfigPage.jsadmin-ui/plugins/services/Components/Configuration/LdapListPage.jsadmin-ui/plugins/user-management/components/UserList.tsxadmin-ui/plugins/services/Components/Configuration/LdapEditPage.jsadmin-ui/plugins/saml/components/SamlIdpForm.jsadmin-ui/app/routes/Apps/Gluu/GluuFormFooter.tsxadmin-ui/app/routes/Apps/Gluu/GluuSessionTimeout.tsxadmin-ui/plugins/saml/components/SamlIdentityList.jsadmin-ui/app/routes/components/Dropdowns/DropdownProfile.tsxadmin-ui/app/helpers/navigation.tsadmin-ui/app/routes/Dashboards/DashboardPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapAddPage.jsadmin-ui/plugins/schema/components/Person/AttributeAddPage.tsxadmin-ui/plugins/services/Components/Configuration/SqlEditPage.js
📚 Learning: 2025-11-03T08:47:33.943Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/services/Components/Configuration/SqlAddPage.jsadmin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsxadmin-ui/app/routes/Apps/Gluu/GluuAppSidebar.tsxadmin-ui/plugins/user-management/components/UserForm.tsxadmin-ui/plugins/saml/components/TrustRelationForm.jsadmin-ui/plugins/auth-server/components/Configuration/ConfigPage.jsadmin-ui/plugins/services/Components/Configuration/LdapEditPage.jsadmin-ui/plugins/saml/components/SamlIdpForm.jsadmin-ui/app/routes/Apps/Gluu/GluuFormFooter.tsxadmin-ui/app/routes/Apps/Gluu/GluuSessionTimeout.tsxadmin-ui/app/helpers/navigation.tsadmin-ui/plugins/services/Components/Configuration/LdapAddPage.jsadmin-ui/plugins/services/Components/Configuration/SqlEditPage.js
🧬 Code graph analysis (21)
admin-ui/plugins/services/Components/Configuration/SqlAddPage.js (2)
admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(66-95)admin-ui/app/utils/PermChecker.ts (1)
buildPayload(153-160)
admin-ui/plugins/schema/components/Person/AttributeListPage.tsx (12)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (1)
useAppNavigation(29-29)admin-ui/plugins/saml/components/SamlIdentityList.js (1)
useAppNavigation(59-59)admin-ui/plugins/saml/components/SamlIdpForm.js (1)
useAppNavigation(37-37)admin-ui/plugins/saml/components/TrustRelationForm.js (1)
useAppNavigation(47-47)admin-ui/plugins/saml/components/TrustRelationshipList.js (1)
useAppNavigation(36-36)admin-ui/plugins/services/Components/Configuration/LdapAddPage.js (1)
useAppNavigation(12-12)admin-ui/plugins/services/Components/Configuration/LdapEditPage.js (1)
useAppNavigation(14-14)admin-ui/plugins/services/Components/Configuration/LdapListPage.js (1)
useAppNavigation(46-46)admin-ui/plugins/services/Components/Configuration/SqlAddPage.js (1)
useAppNavigation(13-13)admin-ui/plugins/services/Components/Configuration/SqlEditPage.js (1)
useAppNavigation(15-15)admin-ui/plugins/services/Components/Configuration/SqlListPage.js (1)
useAppNavigation(76-76)
admin-ui/app/routes/License/LicenseDetailsPage.tsx (2)
admin-ui/app/redux/features/licenseDetailsSlice.ts (1)
LicenseDetailsState(21-24)admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(66-95)
admin-ui/plugins/user-management/components/UserAddPage.tsx (4)
admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(66-95)admin-ui/plugins/user-management/types/UserApiTypes.ts (1)
PersonAttribute(6-6)admin-ui/plugins/user-management/helper/userAuditHelpers.ts (1)
logUserCreation(100-128)admin-ui/plugins/user-management/helper/userWebhookHelpers.ts (1)
triggerUserWebhook(14-26)
admin-ui/app/routes/Apps/Profile/ProfilePage.tsx (2)
admin-ui/app/routes/Apps/Profile/types.ts (2)
ProfileDetails(8-15)AppDispatch(44-44)admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)
admin-ui/plugins/schema/components/Person/AttributeEditPage.tsx (2)
admin-ui/plugins/services/Components/Configuration/LdapAddPage.js (2)
dispatch(10-10)useAppNavigation(12-12)admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)
admin-ui/app/routes/Apps/Gluu/GluuAppSidebar.tsx (1)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)
admin-ui/plugins/saml/components/TrustRelationForm.js (1)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)
admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (1)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)
admin-ui/plugins/saml/components/TrustRelationshipList.js (2)
admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(66-95)admin-ui/plugins/saml/components/SamlIdentityList.js (1)
useAppNavigation(59-59)
admin-ui/app/routes/Apps/Gluu/GluuTabs.tsx (1)
admin-ui/app/customColors.ts (1)
customColors(4-25)
admin-ui/plugins/user-management/components/UserList.tsx (4)
admin-ui/plugins/user-management/types/ComponentTypes.ts (1)
UserTableRowData(54-77)admin-ui/plugins/user-management/types/CommonTypes.ts (1)
ThemeContext(13-17)admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)admin-ui/plugins/user-management/types/UserApiTypes.ts (1)
CustomUser(5-5)
admin-ui/plugins/services/Components/Configuration/LdapEditPage.js (1)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)
admin-ui/plugins/saml/components/SamlIdpForm.js (2)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)admin-ui/plugins/services/Components/Configuration/LdapListPage.js (1)
useAppNavigation(46-46)
admin-ui/app/routes/Apps/Gluu/GluuSessionTimeout.tsx (12)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (1)
useAppNavigation(29-29)admin-ui/plugins/saml/components/SamlIdentityList.js (1)
useAppNavigation(59-59)admin-ui/plugins/saml/components/SamlIdpForm.js (1)
useAppNavigation(37-37)admin-ui/plugins/saml/components/TrustRelationForm.js (1)
useAppNavigation(47-47)admin-ui/plugins/saml/components/TrustRelationshipList.js (1)
useAppNavigation(36-36)admin-ui/plugins/services/Components/Configuration/LdapAddPage.js (1)
useAppNavigation(12-12)admin-ui/plugins/services/Components/Configuration/LdapEditPage.js (1)
useAppNavigation(14-14)admin-ui/plugins/services/Components/Configuration/LdapListPage.js (1)
useAppNavigation(46-46)admin-ui/plugins/services/Components/Configuration/SqlAddPage.js (1)
useAppNavigation(13-13)admin-ui/plugins/services/Components/Configuration/SqlEditPage.js (1)
useAppNavigation(15-15)admin-ui/plugins/services/Components/Configuration/SqlListPage.js (1)
useAppNavigation(76-76)
admin-ui/plugins/saml/components/SamlIdentityList.js (13)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (1)
useAppNavigation(29-29)admin-ui/plugins/saml/components/SamlIdpForm.js (1)
useAppNavigation(37-37)admin-ui/plugins/saml/components/TrustRelationForm.js (1)
useAppNavigation(47-47)admin-ui/plugins/saml/components/TrustRelationshipList.js (1)
useAppNavigation(36-36)admin-ui/plugins/services/Components/Configuration/LdapAddPage.js (1)
useAppNavigation(12-12)admin-ui/plugins/services/Components/Configuration/LdapEditPage.js (1)
useAppNavigation(14-14)admin-ui/plugins/services/Components/Configuration/LdapListPage.js (1)
useAppNavigation(46-46)admin-ui/plugins/services/Components/Configuration/SqlAddPage.js (1)
useAppNavigation(13-13)admin-ui/plugins/services/Components/Configuration/SqlEditPage.js (1)
useAppNavigation(15-15)admin-ui/plugins/services/Components/Configuration/SqlListPage.js (1)
useAppNavigation(76-76)admin-ui/plugins/admin/components/Webhook/WebhookListPage.js (1)
useAppNavigation(31-31)admin-ui/plugins/auth-server/components/Clients/ClientEditPage.js (1)
viewOnly(16-16)
admin-ui/app/routes/components/Dropdowns/DropdownProfile.tsx (1)
admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(66-95)
admin-ui/plugins/services/Components/Configuration/SqlListPage.js (4)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)admin-ui/plugins/services/Components/Configuration/SqlAddPage.js (3)
useAppNavigation(13-13)dispatch(10-10)userAction(12-12)admin-ui/plugins/services/Components/Configuration/SqlEditPage.js (3)
useAppNavigation(15-15)dispatch(12-12)userAction(14-14)admin-ui/app/utils/PermChecker.ts (1)
buildPayload(153-160)
admin-ui/app/routes/Dashboards/DashboardPage.tsx (1)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)
admin-ui/plugins/services/Components/Configuration/LdapAddPage.js (1)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)
admin-ui/plugins/services/Components/Configuration/SqlEditPage.js (2)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)admin-ui/app/utils/PermChecker.ts (1)
buildPayload(153-160)
🔇 Additional comments (30)
admin-ui/app/routes/components/Dropdowns/DropdownProfile.tsx (1)
3-3: LGTM! Clean migration to centralized navigation.The refactoring successfully migrates from direct
useNavigateusage to the centralizeduseAppNavigationhook and replaces hardcoded paths withROUTESconstants. The changes improve maintainability while preserving all original functionality:
- Logout flow remains intact (audit log → navigate on success)
- Profile link correctly uses
ROUTES.PROFILE- Effect dependencies properly updated to include
navigateToRouteThis aligns well with the PR's objective to standardize navigation patterns across the admin UI.
Also applies to: 7-7, 13-13, 21-21, 23-23, 31-31
admin-ui/plugins/schema/components/Person/AttributeAddPage.tsx (1)
16-32: Attribute add success navigation correctly uses centralized helperUsing
navigateBack(ROUTES.ATTRIBUTES_LIST)on successful create gives the expected “back to where I came from, else to attributes list” behavior and aligns with the pattern of falling back to the relevant listing screen instead of the dashboard.
Based on learnings, this matches the intended listing‑screen fallback behavior for schema-related forms.admin-ui/app/routes/License/LicenseDetailsPage.tsx (1)
22-55: License expiry logout flow is correctly centralizedRedirecting on
item.licenseExpiredvianavigateToRoute(ROUTES.LOGOUT)cleanly reuses the shared navigation helper and centralized route constants, with a safe dependency list on the effect. Behavior and wiring look good.admin-ui/app/routes/Apps/Gluu/GluuSessionTimeout.tsx (1)
6-7: Session timeout logout navigation is consistent with shared helperHooking
logoutAuditSucceededtonavigateToRoute(ROUTES.LOGOUT)makes the timeout flow use the same centralized logout route as the rest of the app, with correct effect dependencies.Also applies to: 22-24, 79-84
admin-ui/app/helpers/navigation.ts (1)
1-97: Navigation helper and ROUTES centralization look solid and fix prior footgunsThe new
ROUTESconstant plususeAppNavigation(navigateToRoute,navigateBack,navigate) provide a clear, centralized navigation API. Dynamic routes correctly useencodeURIComponent, andnavigateBack(fallbackRoute)cleanly supports “back with listing-screen fallback” behavior. Importantly,navigateToRoutenow only accepts a string path and no longer auto‑invokes function‑valued routes with empty parameters, addressing the earlierUSER_EDITfootgun from previous reviews.
Based on learnings, this aligns with the agreed back‑navigation semantics for dashboard and listing screens.admin-ui/app/routes/Apps/Gluu/GluuAppSidebar.tsx (1)
32-33: Sidebar logout redirect correctly uses shared navigation helperRedirecting from the sidebar on
logoutAuditSucceeded === truevianavigateToRoute(ROUTES.LOGOUT)is consistent with other parts of the app and centralizes logout routing through the navigation helper. Dependencies are correct.Also applies to: 92-93, 179-183
admin-ui/app/routes/Dashboards/DashboardPage.tsx (1)
30-31: Dashboard permission-based logout wiring is correctSwitching the “no access token” branch in
handleLogouttonavigateToRoute(ROUTES.LOGOUT)keeps the existing behavior while centralizing the logout path through the shared navigation helper. Dependencies onaccess_token,dispatch, andnavigateToRouteare appropriate.Also applies to: 64-65, 342-353
admin-ui/plugins/services/Components/Configuration/LdapEditPage.js (1)
8-25: LGTM! Navigation implementation follows best practices.The implementation correctly uses
navigateBack(ROUTES.LDAP_LIST)to navigate after save, keeping users in the LDAP management context. The dependency array is properly updated to includenavigateBackanddispatch.Based on learnings, this aligns with the requirement that users should be redirected to the respective listing screen rather than the dashboard.
admin-ui/plugins/schema/components/Person/AttributeEditPage.tsx (1)
28-101: LGTM! Navigation implementation follows best practices.The implementation correctly uses
navigateBack(ROUTES.ATTRIBUTES_LIST)after successful attribute update, keeping users in the attribute management context. The dependency array is properly updated to includenavigateBack.Based on learnings, this aligns with the requirement that users should be redirected to the respective listing screen rather than the dashboard.
admin-ui/plugins/saml/components/SamlIdentityList.js (1)
86-95: LGTM! Forward navigation correctly usesnavigateToRoute.The handlers correctly use
navigateToRoutefor forward navigation to edit and add pages. This is the appropriate choice for list-to-detail navigation (as opposed tonavigateBackwhich is for returning from detail to list).admin-ui/plugins/services/Components/Configuration/SqlEditPage.js (1)
7-23: LGTM! Navigation implementation follows best practices.The implementation correctly uses
navigateBack(ROUTES.SQL_LIST)after form submission, keeping users in the SQL database configuration context.Based on learnings, this aligns with the requirement that users should be redirected to the respective listing screen rather than the dashboard.
admin-ui/plugins/saml/components/TrustRelationForm.js (1)
214-223: LGTM! Navigation implementation follows best practices.The implementation correctly uses
navigateBack(ROUTES.SAML_SP_LIST)after saving the trust relationship, keeping users in the SAML service provider context. The dependency array is properly updated to includenavigateBackanddispatch.Based on learnings, this aligns with the requirement that users should be redirected to the respective listing screen rather than the dashboard.
admin-ui/plugins/saml/components/SamlIdpForm.js (1)
162-170: LGTM! Navigation implementation follows best practices.The implementation correctly uses
navigateBack(ROUTES.SAML_IDP_LIST)after saving the identity provider, keeping users in the SAML identity provider context. The dependency array is properly updated to includenavigateBackanddispatch.Based on learnings, this aligns with the requirement that users should be redirected to the respective listing screen rather than the dashboard.
admin-ui/plugins/services/Components/Configuration/SqlAddPage.js (2)
7-7: LGTM: Navigation imports are correct.The centralized navigation approach using
useAppNavigationandROUTESaligns with the PR objectives.
13-20: LGTM: Back navigation with context-appropriate fallback.Using
navigateBack(ROUTES.SQL_LIST)after successful submission keeps users in the SQL configuration context, which aligns with the established pattern. Based on learnings, fallback routes should direct users to the respective listing screen rather than the dashboard.admin-ui/app/routes/Apps/Gluu/GluuFormFooter.tsx (1)
7-7: LGTM: Footer navigation refactor with extensible API.The addition of
backIconClassprop and the switch touseAppNavigationprovides a clean, extensible API. The default fallback toROUTES.HOME_DASHBOARDis sensible for the global component while allowing context-specific overrides via theonBackprop.Also applies to: 21-21, 58-58
admin-ui/plugins/saml/components/TrustRelationshipList.js (2)
21-21: LGTM: Navigation imports are correct.The centralized navigation approach is consistent with the PR objectives.
Also applies to: 36-36
62-71: LGTM: Navigation handlers updated correctly.The callbacks use
navigateToRoutewith ROUTES constants and have proper dependency arrays includingnavigateToRoute. The state passing is preserved correctly.admin-ui/app/routes/Apps/Profile/ProfilePage.tsx (1)
17-17: LGTM: Navigation imports are correct.The centralized navigation approach is consistent across the codebase.
Also applies to: 27-27
admin-ui/plugins/schema/components/Person/AttributeListPage.tsx (2)
4-4: LGTM: Navigation imports are correct.The centralized navigation approach aligns with the PR objectives.
Also applies to: 139-139
195-221: LGTM: Navigation handlers with proper guards.The early return checks (
if (!row?.inum) return) prevent navigation with invalid data, and dependency arrays are correctly updated to includenavigateToRoute. This is a defensive coding practice that prevents potential runtime errors.admin-ui/plugins/services/Components/Configuration/LdapAddPage.js (1)
7-7: LGTM: Navigation imports are correct.The centralized navigation approach is consistent with other add pages.
Also applies to: 12-12
admin-ui/plugins/user-management/components/UserAddPage.tsx (2)
2-2: LGTM: Navigation refactor is correct.Using
useAppNavigationand navigating toROUTES.USER_MANAGEMENTkeeps users in the user-management context, consistent with the established pattern.Also applies to: 25-25
124-126: LGTM: Loader wraps form content.The
GluuLoadernow wraps theUserFormwithblocking={loadingAttributes}, which provides a consistent loading UX while attributes are fetched.admin-ui/plugins/user-management/components/UserEditPage.tsx (2)
3-3: LGTM: Navigation refactor is correct.Replacing
useNavigatewithuseAppNavigationand using ROUTES constants aligns with the PR objectives.Also applies to: 27-27
60-60: LGTM: Consistent navigation and loader pattern.Navigation to
ROUTES.USER_MANAGEMENTafter successful update keeps users in the user-management context. The loader wrapper provides consistent UX during attribute loading.Also applies to: 185-187
admin-ui/plugins/services/Components/Configuration/SqlListPage.js (1)
4-4: Navigation refactor and guard look correctUsing
useAppNavigationwithnavigateToRoute/navigateBackand theROUTES.SQL_*constants is consistent, and the!row?.configIdguard inhandleGoToSqlEditPageprevents invalid navigation. The deletion confirm handler correctly routes back to the SQL list vianavigateBack(ROUTES.SQL_LIST)with appropriate dependencies.Also applies to: 76-76, 154-161, 168-171, 172-180
admin-ui/plugins/user-management/components/UserList.tsx (1)
47-48: 2FA devices modal integration and loading/deletion flow look consistentThe new
User2FADevicesModalwiring withselectedUserFor2FA/isViewDetailModalOpen, and theuseDeleteUsermutation with toast, audit logging, webhook triggering, and query invalidation, are coherently hooked into the list and appear consistent with the rest of the users module.Also applies to: 97-111, 330-337
admin-ui/plugins/user-management/components/UserForm.tsx (2)
81-92: Back/Apply/Cancel footer wiring aligns with the new UX contractThe combination of:
navigateBack(ROUTES.USER_MANAGEMENT)inhandleNavigateBack,handleCancelresetting Formik values andmodifiedFields,handleApplybuildingoperationsand openingGluuCommitDialog, anddisableCancel={!formik.dirty},disableApply={!formik.dirty || !formik.isValid}gives the expected behavior: Back returns to the user listing (via history, with a
/user/usersmanagementfallback viaROUTES.USER_MANAGEMENT), Cancel/Apply are enabled only when the form is dirty, and Apply saves without leaving the page. Based on learnings, this matches the updated requirement to fall back to the component’s listing screen rather than the dashboard.Also applies to: 108-129, 151-154, 168-178, 417-428
93-107: Custom attributes initialization and AvailableClaimsPanel wiring look solidThe use of
personAttributesfrom Redux, the derivedpersonAttributesKey+memoizedPersonAttributes, and theinitializedRefguard in theuseEffectthat callssetupCustomAttributesall work together to avoid redundant recomputation while still reacting to user/attribute changes. The right‑sideAvailableClaimsPanelandsetSelectedClaimsToState/removeSelectedClaimsFromStatehooks are wired coherently to manage dynamic claims.Also applies to: 110-121, 186-203, 395-415
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/plugins/services/Components/Configuration/LdapListPage.js (1)
197-205: Past dependency issue resolved, but consider removing unnecessary navigation.The dependency array now correctly references
navigateBackinstead of the previously undefinednavigate, resolving the past critical issue.However, calling
navigateBack(ROUTES.LDAP_LIST)after deletion appears unnecessary. Since the user is already on the LDAP list page when confirming deletion, the Redux state update fromdispatch(deleteLdap(...))should automatically refresh the table, andtoggle()closes the modal. The navigation call could cause unexpected behavior:
- If browser history exists, it might navigate away from the list page.
- If no history, it reloads the current page unnecessarily.
Consider removing the navigation call:
const onDeletionConfirmed = useCallback( (message) => { buildPayload(userAction, message, item.configId) dispatch(deleteLdap({ configId: item.configId })) - navigateBack(ROUTES.LDAP_LIST) toggle() }, - [item.configId, navigateBack, toggle, dispatch], + [item.configId, toggle, dispatch], )
♻️ Duplicate comments (8)
admin-ui/app/routes/Apps/Gluu/GluuTabs.tsx (1)
44-58: Previous review concern has been addressed.The index misalignment issue flagged in the prior review is now resolved. The implementation correctly:
- Operates on
tabNamesdirectly instead of a filterednavigationTabssubset- Uses
isNavigationTabtype guard consistently to handle mixed tab types- Keeps
valueindices aligned with the actualtabNamesarrayAlso applies to: 79-90, 117-144
admin-ui/plugins/user-management/components/PasswordChangeModal.tsx (4)
122-131: Audit log fires before mutation success — may log failed attempts.
logPasswordChangeis called immediately afterchangePasswordMutation.mutate()at lines 127-130, regardless of whether the mutation succeeds. This means failed password change attempts will still be recorded in the audit log as if they succeeded.Move the audit logging into the mutation's
onSuccesshandler to ensure only successful changes are logged.const changePasswordMutation = usePatchUserByInum({ mutation: { - onSuccess: async (data: CustomUser) => { + onSuccess: async (data: CustomUser, variables) => { dispatch(updateToast(true, 'success', t('messages.password_changed_successfully'))) - await triggerUserWebhook(data as Record<string, unknown>) + triggerUserWebhook(data) + + // Log audit after successful password change + const auditPayload: Record<string, unknown> = { + ...variables.data, + inum: userDetails?.inum, + performedOn: { + user_inum: userDetails?.inum, + userId: userDetails?.userId || userDetails?.displayName, + }, + message: 'Password changed', // Consider passing userMessage through context + } + logPasswordChange(userDetails?.inum || '', auditPayload).catch((error) => { + console.error('Failed to log password change:', error) + }) + queryClient.invalidateQueries({ queryKey: getGetUserQueryKey() })Note: You'll need to pass the
userMessagethrough mutation context or reconstruct it inonSuccess.
261-264:disableCancel={false}inconsistent with PR objectives.The PR aims to standardize Cancel button behavior: "Cancel: resets all fields to their previous values without navigating away; disabled until a change is made."
Currently,
disableCancelis hardcoded tofalse, making Cancel always enabled. For consistency with other forms (UserForm, SmtpForm, ScopeForm, etc.), derive this frompasswordFormik.dirty:<GluuFormFooter showBack={false} showCancel={true} showApply={true} onCancel={handleCancel} onApply={handleApply} - disableCancel={false} + disableCancel={!passwordFormik.dirty} disableApply={isApplyDisabled}
31-37:selectedThemeprop is unused.The prop is destructured as
_selectedThemebut never used. If theming for this modal is handled elsewhere, consider removing this prop from both the interface and callers to avoid confusion.
54-70: Type mismatch intriggerUserWebhookcall.Line 58 casts
datatoRecord<string, unknown>, buttriggerUserWebhooknow expectsCustomUseras its parameter (seeuserWebhookHelpers.tsline 15). Since the mutation'sonSuccessalready receivesdata: CustomUser, the cast is unnecessary and introduces a type inconsistency.onSuccess: async (data: CustomUser) => { dispatch(updateToast(true, 'success', t('messages.password_changed_successfully'))) - await triggerUserWebhook(data as Record<string, unknown>) + triggerUserWebhook(data)Also,
triggerUserWebhookreturnsvoid, so theawaitis unnecessary.admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
172-176: Typo:useIdshould beuserId.The property name
useIdappears to be a typo. Based on the pattern and context, this should beuserId.performedOn: { user_inum: userDetails?.inum, - useId: userDetails?.displayName, + userId: userDetails?.displayName, },admin-ui/plugins/user-management/helper/validations.ts (1)
113-167: Birthdate and custom-attribute initialization are now robustThe new helpers (
processBirthdateAttribute,processMultiValuedAttribute,processSingleValuedAttribute) andinitializeCustomAttributes:
- Stop JSON‑stringifying birthdate before parsing, instead parsing the raw value and only assigning when
moment(...).isValid().- Respect multi‑valued vs single‑valued attributes when seeding
initialValues.- Implement the documented fallbacks (
familyName→sn,jansStatus→status) viagetUserProperty, with clear JSDoc oninitializeCustomAttributes.This resolves the earlier “Invalid date” regression and keeps initialization behavior consistent with the previous utilities.
Based on prior review feedback, this directly addresses the birthdate and initialization issues that were previously raised.
Also applies to: 176-208
admin-ui/plugins/user-management/components/UserForm.tsx (1)
34-79: Cancel still doesn’t fully reset dynamically added claims
handleCancelnow recomputesselectedClaimsviasetupCustomAttributes, butsetupCustomAttributesbuilds on the previousselectedClaimsstate (tempList = [...prevSelectedClaims]) without clearing it first. Any custom claims added during the current session but absent fromuserDetails.customAttributeswill remain after Cancel, so the UI doesn’t fully revert.Make Cancel restore exactly the original claims set for existing users by clearing before re‑setup:
const handleCancel = useCallback(() => { const resetValues = initializeCustomAttributes(userDetails || null, memoizedPersonAttributes) formik.resetForm({ values: resetValues }) setModifiedFields({}) if (userDetails) { - setupCustomAttributes(userDetails, memoizedPersonAttributes, setSelectedClaims) + // Ensure we drop any dynamically added claims and rebuild from userDetails + setSelectedClaims([]) + setupCustomAttributes(userDetails, memoizedPersonAttributes, setSelectedClaims) initializedRef.current = userDetails.inum || 'new' } else { setSelectedClaims([]) initializedRef.current = 'new' } }, [formik, userDetails, memoizedPersonAttributes, setSelectedClaims])Alternatively, have
setupCustomAttributesitself start by clearingselectedClaimsbefore populating fromuserDetails.This is the same behavioral issue previously flagged for Cancel not fully reverting dynamic claims.
Also applies to: 155-166, 186-202
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
admin-ui/app/routes/Apps/Gluu/GluuTabs.tsx(4 hunks)admin-ui/app/routes/Apps/Profile/ProfilePage.tsx(2 hunks)admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx(1 hunks)admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js(2 hunks)admin-ui/plugins/services/Components/Configuration/LdapListPage.js(5 hunks)admin-ui/plugins/user-management/components/PasswordChangeModal.tsx(1 hunks)admin-ui/plugins/user-management/components/UserAddPage.tsx(5 hunks)admin-ui/plugins/user-management/components/UserEditPage.tsx(6 hunks)admin-ui/plugins/user-management/components/UserForm.tsx(5 hunks)admin-ui/plugins/user-management/helper/userAuditHelpers.ts(3 hunks)admin-ui/plugins/user-management/helper/userWebhookHelpers.ts(1 hunks)admin-ui/plugins/user-management/helper/validations.ts(1 hunks)admin-ui/plugins/user-management/types/ComponentTypes.ts(1 hunks)admin-ui/plugins/user-management/utils/userFormUtils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
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.
📚 Learning: 2025-11-03T08:47:33.943Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapListPage.jsadmin-ui/plugins/auth-server/components/Configuration/ConfigPage.jsadmin-ui/plugins/user-management/components/UserForm.tsx
📚 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/plugins/admin/components/Cedarling/CedarlingConfigPage.tsxadmin-ui/app/routes/Apps/Profile/ProfilePage.tsxadmin-ui/plugins/services/Components/Configuration/LdapListPage.jsadmin-ui/plugins/user-management/components/UserEditPage.tsxadmin-ui/plugins/auth-server/components/Configuration/ConfigPage.jsadmin-ui/plugins/user-management/components/PasswordChangeModal.tsxadmin-ui/plugins/user-management/components/UserForm.tsxadmin-ui/plugins/user-management/components/UserAddPage.tsx
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.
Applied to files:
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapListPage.jsadmin-ui/plugins/auth-server/components/Configuration/ConfigPage.jsadmin-ui/plugins/user-management/components/UserForm.tsxadmin-ui/plugins/user-management/components/UserAddPage.tsx
📚 Learning: 2025-11-10T14:18:58.310Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
Applied to files:
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
📚 Learning: 2025-11-07T12:17:39.857Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
Applied to files:
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Applied to files:
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsxadmin-ui/plugins/user-management/components/PasswordChangeModal.tsx
🧬 Code graph analysis (9)
admin-ui/app/routes/Apps/Profile/ProfilePage.tsx (1)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)
admin-ui/plugins/user-management/helper/validations.ts (3)
admin-ui/plugins/user-management/types/UserApiTypes.ts (2)
CustomUser(5-5)PersonAttribute(6-6)admin-ui/plugins/user-management/utils/userFormUtils.ts (2)
validatePassword(9-16)initializeCustomAttributes(66-100)admin-ui/plugins/user-management/types/ComponentTypes.ts (1)
UserEditFormValues(32-40)
admin-ui/plugins/services/Components/Configuration/LdapListPage.js (3)
admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(66-95)admin-ui/plugins/services/Components/Configuration/LdapAddPage.js (2)
useAppNavigation(12-12)dispatch(10-10)admin-ui/plugins/services/Components/Configuration/SqlListPage.js (2)
useAppNavigation(76-76)dispatch(43-43)
admin-ui/plugins/user-management/components/UserEditPage.tsx (5)
admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(66-95)admin-ui/plugins/user-management/types/UserApiTypes.ts (2)
CustomUser(5-5)PersonAttribute(6-6)admin-ui/plugins/user-management/utils/userFormUtils.ts (1)
mapToPersonAttributes(144-149)admin-ui/plugins/user-management/helper/userWebhookHelpers.ts (1)
triggerUserWebhook(15-28)admin-ui/plugins/user-management/types/ComponentTypes.ts (1)
FormValueEntry(117-122)
admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (1)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)
admin-ui/plugins/user-management/helper/userWebhookHelpers.ts (1)
admin-ui/plugins/user-management/types/UserApiTypes.ts (1)
CustomUser(5-5)
admin-ui/plugins/user-management/components/PasswordChangeModal.tsx (5)
admin-ui/plugins/user-management/types/UserFormTypes.ts (1)
PasswordChangeModalProps(23-31)admin-ui/plugins/user-management/types/UserApiTypes.ts (1)
CustomUser(5-5)admin-ui/plugins/user-management/helper/userWebhookHelpers.ts (1)
triggerUserWebhook(15-28)admin-ui/plugins/user-management/helper/userAuditHelpers.ts (2)
getErrorMessage(253-270)logPasswordChange(205-236)admin-ui/plugins/user-management/helper/validations.ts (1)
passwordChangeValidationSchema(68-82)
admin-ui/plugins/user-management/components/UserAddPage.tsx (6)
admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(66-95)admin-ui/plugins/user-management/types/UserApiTypes.ts (2)
PersonAttribute(6-6)CustomUser(5-5)admin-ui/plugins/user-management/utils/userFormUtils.ts (1)
mapToPersonAttributes(144-149)admin-ui/plugins/user-management/helper/userWebhookHelpers.ts (1)
triggerUserWebhook(15-28)admin-ui/plugins/user-management/types/ComponentTypes.ts (1)
FormValueEntry(117-122)admin-ui/plugins/user-management/common/Constants.ts (1)
BIRTHDATE_ATTR(6-6)
admin-ui/app/routes/Apps/Gluu/GluuTabs.tsx (1)
admin-ui/app/customColors.ts (1)
customColors(4-25)
🪛 Biome (2.1.2)
admin-ui/plugins/user-management/helper/validations.ts
[error] 46-46: Do not add then to an object.
(lint/suspicious/noThenProperty)
🔇 Additional comments (23)
admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (1)
25-25: LGTM! Browser-history-aware navigation correctly implemented.The navigation changes properly address the previous review feedback. The
navigateBackimplementation will use browser history when available (navigate(-1)) and only fall back to the dashboard when history is unavailable, which is appropriate behavior for a general configuration page.Also applies to: 29-29, 138-140
admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx (1)
332-337: Back button enablement viashowBacklooks correctExposing the Back button through
showBack={true}cleanly aligns this page with the shared footer behavior and does not affect submit, loading, or permission handling. Please just double‑check that the GluuFormFooter’s configured back navigation/fallback for this page matches the current UX requirement (dashboard vs any Cedarling-specific listing context), since this component itself doesn’t specify a custom route.admin-ui/plugins/services/Components/Configuration/LdapListPage.js (3)
4-4: LGTM! Navigation refactor aligns with PR objectives.The migration to centralized
useAppNavigationandROUTESconstants standardizes navigation across the admin UI, consistent with the PR's goal of uniformity.Also applies to: 46-46
101-108: LGTM! Guard added and past dependency issue resolved.The configId guard on line 103 prevents navigation errors, and the dependency array now correctly references
navigateToRouteinstead of the previously undefinednavigate.
115-117: LGTM!The navigation logic and dependency array are correct.
admin-ui/app/routes/Apps/Profile/ProfilePage.tsx (1)
17-17: LGTM! Centralized navigation pattern adopted correctly.The refactoring to use
useAppNavigation()andROUTES.USER_EDIT()aligns with the PR's goal of standardizing navigation across the admin UI. The dependency optimization from the previous review has been successfully implemented—removing the redundantprofileDetails?.inumfrom the dependency array while keepingprofileDetailsto ensure the latest profile data is passed in the navigation state.Also applies to: 27-27, 70-75
admin-ui/app/routes/Apps/Gluu/GluuTabs.tsx (5)
16-35: Good use ofReact.memofor TabPanel.The memoization is appropriate here since TabPanel is a pure presentational component. Setting
displayNameis a good practice for React DevTools debugging.
44-58: Type system and initialization logic look correct.The previous review concern about index misalignment has been addressed.
initTabValuenow correctly operates on the fulltabNamesarray using theisNavigationTabguard, ensuring indices align with the actual tabs array.
70-76: Lazy initialization and memoized getter are well implemented.The lazy initializer in
useStateavoids unnecessary computation on re-renders. ThegetTabLabelcallback correctly has an empty dependency array sinceisNavigationTabis a stable module-level function.
78-90:handleChangecorrectly handles mixed tab types.The implementation properly uses
tabNames[newValue]directly and checksisNavigationTabbefore navigating. This ensures indices remain aligned regardless of whether tabs are plain strings or navigation tabs.
92-112: Memoized styling is correctly implemented.The empty dependency array is appropriate since
customColorsis a stable module-level import. The styling will be computed once and reused across renders.admin-ui/plugins/user-management/helper/userWebhookHelpers.ts (1)
1-28: LGTM! Type improvements enhance type safety.The changes properly introduce the
CustomUsertype for stronger typing of webhook payloads. The intersection cast on lines 21-22 correctly satisfies both the specificCustomUsertype and the broader record union in the payload interface.admin-ui/plugins/user-management/helper/userAuditHelpers.ts (3)
61-63: LGTM! Compact JSON format is cleaner.The inline JSON string is valid and more readable than escaped multi-line format.
73-75: LGTM! Type assertion ensures correct array typing.The
as typeof payload.customAttributesassertion correctly preserves the array type after themapoperation.
238-251: LGTM! ExportingErrorResponseenables reuse.Making this interface public allows other modules to properly type error handling.
admin-ui/plugins/user-management/types/ComponentTypes.ts (1)
116-122: LGTM! Flexible union type for form values.
FormValueEntryproperly models the various shapes form values can take (strings, arrays, null/undefined, and select-like objects withvalue/labelproperties). This enables type-safe handling of diverse form inputs.admin-ui/plugins/user-management/components/UserEditPage.tsx (3)
35-39: LGTM! Dependency array correctly updated.The
useEffectnow properly depends onuserDetails(not justuserDetails?.inum), ensuring the redirect triggers wheneveruserDetailschanges.
117-129: LGTM! ProperFormValueEntryhandling for multi-valued attributes.The code correctly handles the various shapes of
FormValueEntry:
- Strings are trimmed directly
- Objects (like select options) extract
valueor attribute-named property- Empty values are filtered out
45-48: LGTM! Proper memoization of person attributes.Using
useMemoto derivepersonAttributesfromattributesData?.entriesprevents unnecessary recomputation and aligns with the centralizedmapToPersonAttributesutility.admin-ui/plugins/user-management/components/PasswordChangeModal.tsx (1)
149-159: LGTM! Proper form reset handling on modal close.Using a ref for
resetFormavoids stale closure issues, and the effect correctly resets form state when the modal closes.admin-ui/plugins/user-management/utils/userFormUtils.ts (1)
136-142: The type-unsafe cast creates a contract violation—objects validated only fornameare cast as completePersonAttributeobjects.The
isPersonAttributepredicate only validates thatnameis a non-empty string, butmapToPersonAttributesat line 148 casts the filtered results asPersonAttribute[]without verifying other required fields (displayName,description, etc.). The double cast(entries as unknown[]).filter(isPersonAttribute) as PersonAttribute[]bypasses TypeScript's type checker.While downstream code uses optional chaining (e.g.,
data.displayName?.toLowerCase()) and React safely renders undefined values, this masks a fundamental type safety violation: incomplete objects are being represented as completePersonAttributeinstances.Recommendation: Either restore validation of all required fields in
isPersonAttribute, or adjust the return type toPartial<PersonAttribute>[]to accurately reflect what the predicate guarantees. The current implementation violates the type contract and is fragile against future changes that remove the defensive?.operators downstream.admin-ui/plugins/user-management/components/UserAddPage.tsx (1)
28-28: User creation flow, webhook triggering, and redirect look correctUsing
useAppNavigationwithnavigateToRoute(ROUTES.USER_MANAGEMENT)after a successful create, plustriggerUserWebhook(data)on success, keeps users in the user‑management context and emits the webhook with the persisted server payload. This aligns with the newer navigation requirement and webhook helper typing.Based on learnings, this matches the “fallback to the listing page instead of dashboard” rule for user management.
Also applies to: 41-47
admin-ui/plugins/user-management/components/UserForm.tsx (1)
81-92: Back button behavior matches the new centralized navigation requirementUsing
useAppNavigationand wiringhandleNavigateBackasnavigateBack(ROUTES.USER_MANAGEMENT)ensures:
- History‑back when available.
- Fallback to the user‑management listing when there’s no history.
Plumbing this through
GluuFormFooter(showBack,onBack) matches the newer “return to listing screen, not dashboard” rule for this module and keeps behavior consistent across create/edit flows.Based on learnings, this is the desired pattern for user‑management forms.
Also applies to: 151-154, 423-434
duttarnab
left a comment
duttarnab
left a comment
There was a problem hiding this comment.
-
The cancel button not working for added custom fields
https://www.loom.com/share/708f40c93f20463bb727fe247600f4cf -
All the added custom fields are multiple item fields
There was a problem hiding this comment.
Actionable comments posted: 10
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/Apps/Gluu/GluuRemovableInputRow.tsx (1)
107-122: PropTypes are redundant with TypeScript interface.Since the component now has a strongly-typed
GluuRemovableInputRowPropsinterface, the PropTypes block is redundant and can be removed to reduce maintenance burden.-GluuRemovableInputRow.propTypes = { - label: PropTypes.string, - name: PropTypes.string, - type: PropTypes.string, - value: PropTypes.oneOfType([PropTypes.string, PropTypes.bool]), - formik: PropTypes.object, - required: PropTypes.bool, - lsize: PropTypes.number, - rsize: PropTypes.number, - handler: PropTypes.func, - doc_category: PropTypes.string, - isDirect: PropTypes.bool, - isBoolean: PropTypes.bool, - modifiedFields: PropTypes.object, - setModifiedFields: PropTypes.func, -}Also remove the unused import:
-import PropTypes from 'prop-types'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
admin-ui/app/routes/Apps/Gluu/GluuRemovableInputRow.tsx(6 hunks)admin-ui/app/routes/Apps/Gluu/GluuRemovableSelectRow.tsx(4 hunks)admin-ui/app/routes/Apps/Gluu/GluuRemovableTypeAhead.tsx(3 hunks)admin-ui/plugins/user-management/components/AvailableClaimsPanel.tsx(4 hunks)admin-ui/plugins/user-management/components/UserForm.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
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.
📚 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/plugins/user-management/components/UserForm.tsx
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.
Applied to files:
admin-ui/plugins/user-management/components/UserForm.tsx
📚 Learning: 2025-11-03T08:47:33.943Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/user-management/components/UserForm.tsx
🧬 Code graph analysis (4)
admin-ui/plugins/user-management/components/AvailableClaimsPanel.tsx (3)
admin-ui/plugins/user-management/types/UserFormTypes.ts (1)
AvailableClaimsPanelProps(33-40)admin-ui/plugins/user-management/types/UserApiTypes.ts (1)
PersonAttribute(6-6)admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (1)
dispatch(35-35)
admin-ui/app/routes/Apps/Gluu/GluuRemovableTypeAhead.tsx (1)
admin-ui/plugins/admin/__tests__/components/CustomScripts/CustomScriptDetailPage.test.js (1)
name(18-18)
admin-ui/app/routes/Apps/Gluu/GluuRemovableInputRow.tsx (1)
admin-ui/app/layout/components/types.ts (1)
InputProps(221-226)
admin-ui/app/routes/Apps/Gluu/GluuRemovableSelectRow.tsx (1)
admin-ui/plugins/admin/__tests__/components/CustomScripts/CustomScriptDetailPage.test.js (1)
name(18-18)
🪛 Biome (2.1.2)
admin-ui/app/routes/Apps/Gluu/GluuRemovableTypeAhead.tsx
[error] 98-101: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 98-101: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
admin-ui/app/routes/Apps/Gluu/GluuRemovableSelectRow.tsx
[error] 79-82: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 79-82: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
🔇 Additional comments (9)
admin-ui/plugins/user-management/components/AvailableClaimsPanel.tsx (1)
118-118: Good addition of explicitreturn null.Explicitly returning
nullfor filtered-out items improves code clarity and prevents React warnings about missing return values in map callbacks.admin-ui/plugins/user-management/components/UserForm.tsx (4)
226-257: Effect may miss element if ref is null on first render.The effect runs once (
[]deps) and readsformContentRef.current. If the ref element is conditionally rendered or not yet mounted when the effect runs,elementwill be null and the observer won't attach. Subsequent renders won't re-run the effect.Since the
<div ref={formContentRef}>is always rendered within the form, this should work in practice. If you encounter height tracking issues, consider adding a callback ref or running the effect when the ref changes.
147-149: Navigation fallback correctly targets user management listing.The
navigateBack(ROUTES.USER_MANAGEMENT)implementation aligns with the updated requirement to keep users in context rather than redirecting to the dashboard. Based on learnings.
465-476: Form footer correctly implements the specified button behaviors.The implementation properly enforces:
- Cancel disabled until a change is made (
!formik.dirty)- Apply disabled when form is clean or invalid (
!formik.dirty || !formik.isValid)- Back always enabled with proper navigation
This aligns well with the PR objectives.
88-91: Loading state now properly wired from Redux.The previous concern about hardcoded
loading = falsehas been addressed. The loading state is now correctly sourced fromattributesReducerRoot.loadingand passed to both<GluuLoader blocking={loading}>and<GluuFormFooter isLoading={loading}>.Also applies to: 260-260
admin-ui/app/routes/Apps/Gluu/GluuRemovableSelectRow.tsx (1)
71-72: Options rendering is clear and correctly keyed.Using
item.cca2as both thevalueandkeyis appropriate here and should produce stable option identities.admin-ui/app/routes/Apps/Gluu/GluuRemovableTypeAhead.tsx (3)
7-12: LGTM! Type definitions improve type safety.The introduction of internal type aliases and explicit FormikProps import strengthens type checking and eliminates the previous reliance on
anytypes.
14-30: LGTM! Props interface properly typed.The refined prop types replace loose
anyannotations with specific types, improving type safety and IDE support.
49-50: LGTM! Proper controlled component pattern.The selectedValue derivation with fallback chain correctly implements controlled component behavior, preventing undefined values while maintaining type safety.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
admin-ui/app/routes/Apps/Gluu/GluuRemovableTypeAhead.tsx(3 hunks)admin-ui/plugins/user-management/components/UserForm.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
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.
📚 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/plugins/user-management/components/UserForm.tsx
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.
Applied to files:
admin-ui/plugins/user-management/components/UserForm.tsx
📚 Learning: 2025-11-03T08:47:33.943Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/user-management/components/UserForm.tsx
🧬 Code graph analysis (1)
admin-ui/app/routes/Apps/Gluu/GluuRemovableTypeAhead.tsx (1)
admin-ui/plugins/admin/__tests__/components/CustomScripts/CustomScriptDetailPage.test.js (1)
name(18-18)
🪛 Biome (2.1.2)
admin-ui/app/routes/Apps/Gluu/GluuRemovableTypeAhead.tsx
[error] 119-122: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 119-122: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
🔇 Additional comments (8)
admin-ui/app/routes/Apps/Gluu/GluuRemovableTypeAhead.tsx (3)
7-39: LGTM: Excellent type safety improvements.The migration from
anytypes to strict TypeScript types (FormikProps, specific option types, properly typed state setters) significantly improves type safety and maintainability.
83-109: LGTM: Well-implemented controlled component pattern.The
selectedValueinitialization correctly prioritizes Formik state over thevalueprop. TheonChangehandler properly maintains both Formik state (full objects) andmodifiedFields(extracted strings) for change tracking.
41-64: Helper function correctly extracts mapping logic with intentional field precedence.The
extractValueFromOptionfunction successfully addresses the previous review suggestion to extract mapping logic. The implementation is sound and the field precedence (role → label → fieldName) is intentional:
- When handling role objects,
roleis prioritized for semantic correctness- When handling other data, the function falls back to the field matching the component's
nameparameter- In actual usage (UserClaimEntry.tsx), role objects and user data are passed in separate scenarios, so the precedence works correctly
The precedence logic is deliberate but relies on consistent usage patterns. If the component's usage evolves to mix different object types, this precedence could produce unexpected results. Document the precedence assumption or add logic to handle mixed option types if component usage expands in the future.
admin-ui/plugins/user-management/components/UserForm.tsx (5)
88-91: Loading state now properly wired from Redux.The component correctly consumes
loadingfromattributesReducerRoot, enabling theGluuLoaderandGluuFormFooterto reflect actual loading state. This addresses the previous concern about hardcodedloading = false.
152-154: Navigation fallback correctly targets user management context.The
handleNavigateBackimplementation usingnavigateBack(ROUTES.USER_MANAGEMENT)correctly keeps users within the user management workflow when browser history is unavailable, aligning with the updated requirement. Based on learnings.
186-202: Initialization guard prevents infinite loops.The use of
initializedRefwith a compound key (${userKey}-${attrsKey}) is a good pattern to prevent the effect from re-running unnecessarily when dependencies change but the logical state hasn't.
231-262: Well-structured ResizeObserver with graceful fallback.The effect correctly implements feature detection for
ResizeObserver, falls back towindow.resizelistener when unavailable, and properly cleans up both in the return function. The empty dependency array is appropriate since the ref is accessed synchronously after render.
470-481: Button states correctly implement PR requirements.The
GluuFormFooterconfiguration properly implements the standardized button behavior:
- Back: Always enabled via
showBack={true}with no disable prop- Cancel: Disabled until form changes (
disableCancel={!formik.dirty})- Apply: Enabled only when form is dirty AND valid (
disableApply={!formik.dirty || !formik.isValid})This aligns with the PR objectives for preventing accidental state loss and providing consistent UX.
| <div | ||
| style={applicationstyle.removableInputRow as React.CSSProperties} | ||
| onClick={() => handler()} | ||
| > | ||
| <i className={'fa fa-fw fa-close'} style={{ color: customColors.accentRed }}></i> | ||
| </div> |
There was a problem hiding this comment.
Critical accessibility issue: replace div with semantic button.
This accessibility violation was flagged in the previous review but remains unresolved. The clickable div prevents keyboard-only users from removing items and is not announced by screen readers as an interactive element.
Apply this diff to fix the accessibility issue:
- <div
+ <button
+ type="button"
style={applicationstyle.removableInputRow as React.CSSProperties}
onClick={() => handler()}
+ aria-label={`Remove ${label}`}
>
<i className={'fa fa-fw fa-close'} style={{ color: customColors.accentRed }}></i>
- </div>
+ </button>Ensure applicationstyle.removableInputRow includes button reset styles (remove default border/background/padding) to maintain the current visual appearance.
📝 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.
| <div | |
| style={applicationstyle.removableInputRow as React.CSSProperties} | |
| onClick={() => handler()} | |
| > | |
| <i className={'fa fa-fw fa-close'} style={{ color: customColors.accentRed }}></i> | |
| </div> | |
| <button | |
| type="button" | |
| style={applicationstyle.removableInputRow as React.CSSProperties} | |
| onClick={() => handler()} | |
| aria-label={`Remove ${label}`} | |
| > | |
| <i className={'fa fa-fw fa-close'} style={{ color: customColors.accentRed }}></i> | |
| </button> |
🧰 Tools
🪛 Biome (2.1.2)
[error] 119-122: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 119-122: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
🤖 Prompt for AI Agents
In admin-ui/app/routes/Apps/Gluu/GluuRemovableTypeAhead.tsx around lines 119 to
124, replace the clickable <div> with a semantic <button type="button">, keep
the existing onClick handler, and add an accessible label (aria-label or title)
for screen readers; ensure the button uses applicationstyle.removableInputRow
cast to React.CSSProperties and that applicationstyle.removableInputRow includes
button reset styles (no border, background, padding, outline handled) so the
visual appearance remains unchanged while making it keyboard-focusable and
announced as interactive.
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 (5)
admin-ui/plugins/user-management/components/UserAddPage.tsx(5 hunks)admin-ui/plugins/user-management/components/UserEditPage.tsx(6 hunks)admin-ui/plugins/user-management/components/UserForm.tsx(2 hunks)admin-ui/plugins/user-management/helper/userAuditHelpers.ts(3 hunks)admin-ui/plugins/user-management/types/ComponentTypes.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
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.
📚 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/plugins/user-management/components/UserEditPage.tsxadmin-ui/plugins/user-management/components/UserAddPage.tsxadmin-ui/plugins/user-management/components/UserForm.tsx
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.
Applied to files:
admin-ui/plugins/user-management/components/UserAddPage.tsxadmin-ui/plugins/user-management/components/UserForm.tsx
📚 Learning: 2025-11-03T08:47:33.943Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/user-management/components/UserForm.tsx
🧬 Code graph analysis (1)
admin-ui/plugins/user-management/components/UserEditPage.tsx (5)
admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(66-95)admin-ui/plugins/user-management/utils/userFormUtils.ts (1)
mapToPersonAttributes(144-149)admin-ui/plugins/user-management/helper/userWebhookHelpers.ts (1)
triggerUserWebhook(15-28)admin-ui/plugins/user-management/helper/userAuditHelpers.ts (1)
getErrorMessage(265-291)admin-ui/plugins/user-management/types/ComponentTypes.ts (1)
FormValueEntry(118-123)
🔇 Additional comments (21)
admin-ui/plugins/user-management/helper/userAuditHelpers.ts (3)
74-74: LGTM!The type assertion correctly preserves the original type after mapping, addressing TypeScript's type widening behavior.
238-263: LGTM!The exported
ErrorResponseinterface with additional fields and thepickFirstStringutility improve error handling flexibility. The utility correctly handles edge cases including undefined, empty, and whitespace-only strings.
265-290: LGTM!The refactored
getErrorMessagewith status-based prioritization is a good improvement. Prioritizingdescriptionfor 4xx client errors surfaces actionable messages to users, while the comprehensive fallback chain handles various API response structures gracefully.admin-ui/plugins/user-management/types/ComponentTypes.ts (2)
22-22: LGTM!The
isSubmittingprop addition is correctly typed as optional and aligns with the form submission state management inUserForm,UserAddPage, andUserEditPage.
117-123: LGTM!The
FormValueEntrytype union comprehensively covers the various form value shapes (primitives, arrays, nullish values, and select-style objects withvalue/label). This provides proper type safety for the multi-valued attribute handling in the user forms.admin-ui/plugins/user-management/components/UserEditPage.tsx (4)
1-3: LGTM!The import changes properly bring in
useMemofor memoization and the centralized navigation utilities (useAppNavigation,ROUTES).
35-39: LGTM!The dependency array now correctly includes
userDetailsinstead of justuserDetails?.inum, ensuring the effect re-runs whenever the entire object reference changes. This addresses the previous review feedback.
45-48: LGTM!Using
mapToPersonAttributeswithuseMemofor attribute transformation is cleaner than the previous inline casting and ensures the derived data is properly memoized.
187-192: LGTM!The loader correctly blocks on both
loadingAttributesandisSubmitting, preventing user interaction during data fetching or form submission. PassingisSubmittingtoUserFormenables proper button state management.admin-ui/plugins/user-management/components/UserAddPage.tsx (4)
35-38: LGTM!The use of
mapToPersonAttributeswithuseMemoproperly addresses the previous type casting concerns. This provides cleaner type transformation compared to the earlieras unknown as PersonAttribute[]approach.
64-78: LGTM!The
toStringValuefunction correctly handles allFormValueEntryvariants including the object-like{ value, label }pattern. The logic properly extracts string values from various input shapes.
80-102: LGTM!The
normalizeValuesfunction now properly acceptsFormValueEntry | FormValueEntry[] | null | undefined, addressing the previous review feedback about type alignment. The null/undefined guards at lines 89-91 and 95 ensure robust handling.
142-144: LGTM!The loader correctly combines
loadingAttributesandisSubmittingstates, andUserFormreceives theisSubmittingprop for proper button state management.admin-ui/plugins/user-management/components/UserForm.tsx (8)
75-77: LGTM!The component signature correctly incorporates
isSubmittingwith a default value offalse, and the navigation hook integration is properly structured.
88-91: Loading state is now properly wired to Redux.The
loadingstate is correctly derived fromstate.attributesReducerRoot.loading, addressing the previous concern about the hardcodedfalsevalue. This ensures the loader reflects the actual attributes fetching state.
155-157: Back button fallback correctly targets user management.The
navigateBack(ROUTES.USER_MANAGEMENT)fallback keeps users in the user management context when browser history is unavailable, which aligns with the updated requirement. Based on learnings.
159-171: LGTM!The
handleCancelimplementation properly resets form values, clears modified fields tracking, and re-initializes the claims state. This ensures the form returns to its original state without navigation, matching the PR objectives.
177-186: LGTM!The
submitFormfunction correctly guards against double submission withisSubmittingcheck, closes the modal, and delegates toonSubmitDatawith the current form values and modified fields.
270-273: LGTM!The
blockingLoadercorrectly combines Reduxloadingstate with theisSubmittingprop, ensuring the UI is blocked during both attribute fetching and form submission.
478-490: LGTM!The
GluuFormFooterintegration correctly implements the PR objectives:
- Back: always enabled unless submitting
- Cancel: disabled when submitting or form is clean
- Apply: disabled when submitting, form is clean, or form is invalid
This aligns with the documented Cancel/Back/Apply button behavior.
504-504: LGTM!Wrapping with
React.memois appropriate for this component to prevent unnecessary re-renders when parent components update but props remain unchanged.
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 (2)
admin-ui/plugins/user-management/components/UserEditPage.tsx (2)
80-142: Align single‑valued attributes to handle object‑shapedFormValueEntryvalues consistently with multi‑valued logicThe code inconsistency is confirmed.
FormValueEntrytype explicitly allows{ value?: string; label?: string; [key: string]: string | undefined }(verified inadmin-ui/plugins/user-management/types/ComponentTypes.ts), yet the single‑valued branch increateCustomAttributes(lines 80–142) ignores object entries while the multi‑valued branch handles them.Current UI components (
GluuSelectRow,GluuRemovableSelectRow) only emit string values, so this is not a present functional bug. However, the type definition permits objects, making this a defensible hardening: extend the single‑valued path to extractvalueor[attributeName]from objects (lines 95–99), matching the multi‑valued logic (lines 117–124), so both branches handle the same input shapes.
148-181: Remove post-save navigation to align with "save in place" UX objectiveThe
onSuccesshandler redirects toROUTES.USER_MANAGEMENTafter mutation completes, which contradicts the PR objective of saving changes without leaving the page. Apply should update the user in place and remain on the edit screen; use the separate Back button for navigation to listing.Additionally, verify that the extra fields in
submittableValues(modifiedFields, performedOn, action_message, and conditional customObjectClasses) match your backend API contract—these are cast asCustomUserwithout explicit type safety, which could mask payload mismatches.
♻️ Duplicate comments (1)
admin-ui/plugins/user-management/components/UserForm.tsx (1)
34-73: DuplicatesetupCustomAttributeslogic; consider reusing shared helperThis file defines a local
setupCustomAttributesthat hydratesselectedClaimsfromuserDetails.customAttributes. Per the provided snippets, a very similarsetupCustomAttributesalready exists in../utils/userFormUtilswith overlapping responsibilities.Maintaining two versions of this logic risks subtle divergence (e.g., usedClaims set, how options are attached, merge vs replace semantics). Since this function is already pure and used from multiple places (
useEffectinitialization and Cancel handler), consider:
- removing the local implementation, and
- importing and using the shared helper from
../utils/userFormUtils(or consolidating into a single exported helper).This keeps the attribute‑to‑claims mapping consistent across Add/Edit flows and simplifies future changes.
Also applies to: 159-171, 192-209
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
admin-ui/plugins/user-management/components/UserEditPage.tsx(6 hunks)admin-ui/plugins/user-management/components/UserForm.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
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.
📚 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/plugins/user-management/components/UserEditPage.tsxadmin-ui/plugins/user-management/components/UserForm.tsx
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Applied to files:
admin-ui/plugins/user-management/components/UserEditPage.tsx
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.
Applied to files:
admin-ui/plugins/user-management/components/UserForm.tsx
📚 Learning: 2025-11-03T08:47:33.943Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/user-management/components/UserForm.tsx
🧬 Code graph analysis (2)
admin-ui/plugins/user-management/components/UserEditPage.tsx (5)
admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(66-95)admin-ui/plugins/user-management/types/UserApiTypes.ts (2)
CustomUser(5-5)PersonAttribute(6-6)admin-ui/plugins/user-management/utils/userFormUtils.ts (1)
mapToPersonAttributes(144-149)admin-ui/plugins/user-management/helper/userWebhookHelpers.ts (1)
triggerUserWebhook(15-28)admin-ui/plugins/user-management/types/ComponentTypes.ts (1)
FormValueEntry(118-123)
admin-ui/plugins/user-management/components/UserForm.tsx (6)
admin-ui/plugins/user-management/utils/userFormUtils.ts (2)
setupCustomAttributes(102-134)initializeCustomAttributes(66-100)admin-ui/plugins/user-management/types/UserApiTypes.ts (2)
CustomUser(5-5)PersonAttribute(6-6)admin-ui/plugins/user-management/types/ComponentTypes.ts (2)
UserFormProps(15-23)FormOperation(43-47)admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)admin-ui/plugins/user-management/types/CommonTypes.ts (1)
ThemeContext(13-17)admin-ui/plugins/user-management/helper/validations.ts (2)
initializeCustomAttributes(176-208)getUserValidationSchema(8-66)
🔇 Additional comments (3)
admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
187-193: Loader integration correctly blocks form during attribute load and submitWrapping
UserFormin<GluuLoader blocking={loadingAttributes || isSubmitting}>ensures the edit form doesn’t render or remain interactive while attributes are loading or while an update is in progress. This addresses prior concerns about partially initialized forms and aligns well with the innerblockingLoaderinUserForm.admin-ui/plugins/user-management/components/UserForm.tsx (2)
270-276: Inner loader now correctly reflects attribute loading and submit state
const blockingLoader = loading || isSubmittingwired into<GluuLoader blocking={blockingLoader}>ensures the form is blocked both while attributes are loading from the reducer and while a submit is in progress. This resolves the earlier hard‑codedconst loading = falsebehavior and makes the inner loader behavior consistent with parent‑level loaders.
223-235: No action required — Gluu rows update Formik before invoking handleChange.GluuInputRow and GluuSelectRow call formik.handleChange(event) before calling the optional handleChange; UserClaimEntry delegates to GluuRemovableInputRow/GluuRemovableSelectRow/GluuRemovableTypeAhead which use formik.setFieldValue/formik.handleChange internally. The UserForm.handleChange that only records modifiedFields is safe as-is.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
admin-ui/plugins/user-management/components/UserForm.tsx (2)
34-73: Implementation is correct.The helper properly hydrates custom claims while excluding standard fields via the
usedClaimsSet. Placing this pure function outside the component prevents unnecessary recreations.A previous review suggested extracting this to
../helper/validations.tsfor consistency with other validation helpers.
398-435: Password fields correctly conditional on new user creation.The password and confirm password fields are appropriately rendered only when
!userDetails(new user flow), while existing users access password changes via the modal. This separation is clean.A previous review noted the
showErrorcondition includes!!formik.values.userPasswordfor earlier feedback display.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/user-management/components/UserForm.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
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.
📚 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/plugins/user-management/components/UserForm.tsx
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.
Applied to files:
admin-ui/plugins/user-management/components/UserForm.tsx
📚 Learning: 2025-11-03T08:47:33.943Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/user-management/components/UserForm.tsx
🔇 Additional comments (9)
admin-ui/plugins/user-management/components/UserForm.tsx (9)
1-33: Imports are well-organized and appropriate for the refactored component.The separation of React hooks, third-party libraries, UI components, context/Redux, API/services, and types follows good organization practices. The navigation abstraction via
useAppNavigationandROUTESimproves maintainability.
134-158: Guards properly address the Enter-key bypass vulnerability.The
handleApplycallback now correctly guards against invalid, pristine, or empty submissions with comprehensive checks forisSubmitting,formik.dirty,formik.isValid, andmodifiedFieldslength. This aligns with theGluuFormFooterdisable conditions and prevents bypassing via Enter key.
160-162: Navigation fallback correctly targets user management listing.Based on learnings, the fallback to
ROUTES.USER_MANAGEMENTis the intended behavior, keeping users in context rather than redirecting to the dashboard.
197-213: Initialization guard pattern is appropriate.Using a ref to track the initialization key (
${userKey}-${attrsKey}) effectively prevents infinite re-runs whenenableReinitializeis true. This is a valid pattern for coordinating external state hydration with Formik's reinitialization.
242-273: ResizeObserver implementation is well-structured.The effect correctly:
- Feature-detects
ResizeObserveravailability- Falls back to window resize listener
- Properly cleans up observers/listeners on unmount
- Uses functional state update to avoid unnecessary re-renders
275-278: Loading state properly wired to Redux.The
blockingLoadernow correctly combines Reduxloadingstate with theisSubmittingprop, addressing the earlier concern about the hardcodedfalsevalue. This ensures the loader displays during both attribute fetching and form submission.
483-494: GluuFormFooter correctly implements the PR requirements.The footer configuration aligns with the stated behavior:
- Back: Always enabled (no
disableBackprop) — navigates appropriately- Cancel: Disabled until form is dirty (
disableCancel={isSubmitting || !formik.dirty}) — resets without navigating- Apply: Disabled until form has valid unsaved changes (
disableApply={isSubmitting || !formik.dirty || !formik.isValid}) — saves without leaving pageThe
applyButtonType="button"prevents the Apply button from acting as a submit button, avoiding double-triggering with theForm.onSubmithandler.
508-508: Good use of React.memo for performance optimization.Wrapping the component with
React.memohelps prevent unnecessary re-renders when parent components update but props haven't changed.
228-240: Review comment is incorrect—GluuInputRow properly calls formik.handleChange in all cases.The verification shows that
GluuInputRowcallsformik.handleChange(event)unconditionally in both branches of its onChange handler (lines 54 and 57 in admin-ui/app/routes/Apps/Gluu/GluuInputRow.tsx). The customhandleChangeprop is only invoked as a supplementary callback when provided. Therefore,formik.dirtywill be correctly updated, and the form state management works as expected. No action is required.Likely an incorrect or invalid review comment.
4 participants
feat(admin): adding uniformity in cancel and back buttons present in Users and App-wide navigation via named ROUTES with navigate helpers.
Summary
This update introduces consistent behavior for Cancel, Back, and Apply buttons across Admin UI forms, including correct enabling and disabling based on form state.
Improvements
Additional Enhancements
Affected Modules
This change is applied across all forms where configuration changes are possible, including but not limited to:
Parent issue: #2268
Testing Steps
Summary by CodeRabbit
New Features
Improvements
Localization
✏️ Tip: You can customize this high-level summary in your review settings.