-
Notifications
You must be signed in to change notification settings - Fork 22
feat(admin): adding uniformity in cancel and back buttons present in Auth Server(SSA, Keys) rest menus are under TS migration #2492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughCentralized navigation updates (new auth-server SSA/Scopes routes, HOME_DASHBOARD path change), widespread migration to useAppNavigation/ROUTES, TypeScript typing strengthens, SSA form/validation/util refactor (expiration handling), UI tweaks, and JSON viewer typing and dialog loading changes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (9)
🧰 Additional context used🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-11-05T19:57:35.143ZApplied to files:
📚 Learning: 2025-11-03T08:47:33.943ZApplied to files:
📚 Learning: 2025-11-06T08:23:20.948ZApplied to files:
🧬 Code graph analysis (3)admin-ui/plugins/auth-server/components/Ssa/validations.ts (1)
admin-ui/plugins/auth-server/components/Ssa/utils.ts (1)
admin-ui/app/routes/Apps/Gluu/GluuToogleRow.tsx (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (16)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
admin-ui/app/routes/Apps/Gluu/GluuRemovableInputRow.tsx (1)
107-122: Consider removing redundant PropTypes.PropTypes are redundant now that TypeScript types are defined. While they can coexist for runtime validation, modern TypeScript projects typically rely solely on compile-time type checking.
Apply this diff to remove PropTypes:
-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, -} export default GluuRemovableInputRowAnd remove the unused import:
import GluuToogle from 'Routes/Apps/Gluu/GluuToogle' -import PropTypes from 'prop-types' import customColors from '@/customColors'admin-ui/plugins/auth-server/components/Sessions/SessionListPage.tsx (1)
514-527: AddtableActionsto the dependency array.The
renderContentcallback usestableActions(line 509) but doesn't include it in its dependency array. This violates the exhaustive-deps rule and could cause the table to display stale actions if permissions change.Apply this diff:
}, [ loading, authenticatedSessions, columnState.updatedColumns, tableOptions, handleDetailPanel, TableContainer, tableKey, searchParams, isFilterApplied, filterState.pattern, filterState.searchFilter, t, + tableActions, ])admin-ui/plugins/auth-server/components/Ssa/SsaDetailPage.tsx (1)
88-93: Defensively handle possible missinggrant_typesbefore callingjoin
row.ssa.grant_types.join(', ')will throw ifgrant_typesis everundefined/null. Unless the DTO guarantees this array is always present, this is a potential runtime error.Consider a safer variant:
- value={row.ssa.grant_types.join(', ')} + value={(row.ssa.grant_types && row.ssa.grant_types.length > 0) + ? row.ssa.grant_types.join(', ') + : ''}(or use optional chaining with a fallback), so the detail page remains robust if the field is absent.
admin-ui/app/helpers/navigation.ts (1)
85-93:navigateBacksilently does nothing when history is empty and no fallback is provided.When
window.history.length <= 1andfallbackRouteis undefined, the function returns without navigating anywhere. This could leave users stuck. Consider makingfallbackRouterequired or defaulting toROUTES.HOME_DASHBOARD.const navigateBack = useCallback( - (fallbackRoute?: string): void => { + (fallbackRoute: string = ROUTES.HOME_DASHBOARD): void => { if (window.history.length > 1) { navigate(-1) - } else if (fallbackRoute) { + } else { navigateToRoute(fallbackRoute) } }, [navigate, navigateToRoute], )Based on learnings, callers should provide context-specific fallbacks (e.g., SSA pages fall back to
/auth-server/config/ssa), but having a default ensures navigation always occurs.admin-ui/plugins/auth-server/components/Ssa/SsaListPage.tsx (1)
168-173: Potential state inconsistency intoggleSsaDialog.The toggle function clears
jwtDataonly when the dialog is currently open (ssaDialogOpenis true before toggling). However, the condition checks the value before the toggle happens:if (ssaDialogOpen) { // This is the OLD value setJwtData(null) }This means
jwtDatais cleared when closing the dialog, which is correct. But if this function is ever called to open the dialog (whenssaDialogOpenis false), it won't clear stale data. Currently,handleViewSsahandles this by clearing data before opening, so it's safe, but the logic intoggleSsaDialogcould be clearer:const toggleSsaDialog = (): void => { - setSsaDialogOpen(!ssaDialogOpen) - if (ssaDialogOpen) { + const wasOpen = ssaDialogOpen + setSsaDialogOpen(!wasOpen) + if (wasOpen) { setJwtData(null) } }This is minor since current usage is correct, but improves readability.
admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx (1)
69-86: Formik + centralized validation look good; consider i18n for expiration toastsThe switch to:
useFormik<SsaFormValues>withinitialValues={getSsaInitialValues()}andvalidationSchema={ssaValidationSchema}- centralized epoch conversion via
toEpochSecondsFromDayjsinopenCommitDialogsubmitFormdelegating tocreateSsaMutation.mutateAsyncand audit loggingis clean and aligns the form with the shared utilities/types for SSA. That part looks solid.
Two minor follow-ups:
Manual expiration validation vs schema
Expiration rules (required + future date whenisExpirable) are enforced imperatively inonSubmit. That’s fine, but if you plan to reuse this pattern elsewhere, consider pushing those rules into the validation schema so everything is expressed in one place and remains type-aligned withSsaFormValues.Hard-coded error messages should use i18n
These toasts:dispatch(updateToast(true, 'error', 'Please select an expiration date')) dispatch(updateToast(true, 'error', 'Expiration date must be in the future'))bypass
t(...)and will not be localized. Prefer introducing translation keys and usingt('...')here (and, if necessary, wiring those keys into your toast slice) so error messaging is consistent with the rest of the UI.For example (keys are illustrative):
if (isExpirable && !expirationDate) {dispatch(updateToast(true, 'error', 'Please select an expiration date'))
if (isExpirable && !expirationDate) {dispatch(updateToast(true, 'error', t('messages.expiration_date_required'))) return }
if (isExpirable && expirationDate && expirationDate.isBefore(dayjs())) {dispatch(updateToast(true, 'error', 'Expiration date must be in the future'))
if (isExpirable && expirationDate && expirationDate.isBefore(dayjs())) {dispatch(updateToast(true, 'error', t('messages.expiration_date_in_future'))) return }(You can adjust the actual keys/texts to match your translation files.)Also applies to: 101-105, 155-177
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (21)
admin-ui/app/helpers/navigation.ts(2 hunks)admin-ui/app/routes/Apps/Gluu/GluuLabel.tsx(1 hunks)admin-ui/app/routes/Apps/Gluu/GluuRemovableInputRow.tsx(3 hunks)admin-ui/docs/CODE_SPLITTING.md(7 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkListPage.tsx(2 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/KeysPage.tsx(2 hunks)admin-ui/plugins/auth-server/components/JsonViewer/JsonViewer.tsx(2 hunks)admin-ui/plugins/auth-server/components/JsonViewer/JsonViewerDialog.tsx(3 hunks)admin-ui/plugins/auth-server/components/Scopes/ScopeForm.tsx(3 hunks)admin-ui/plugins/auth-server/components/Scopes/ScopeListPage.tsx(3 hunks)admin-ui/plugins/auth-server/components/Scopes/hooks/useScopeActions.ts(3 hunks)admin-ui/plugins/auth-server/components/Sessions/SessionDetailPage.tsx(1 hunks)admin-ui/plugins/auth-server/components/Sessions/SessionListPage.tsx(8 hunks)admin-ui/plugins/auth-server/components/Sessions/types/sessionTypes.ts(0 hunks)admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx(6 hunks)admin-ui/plugins/auth-server/components/Ssa/SsaDetailPage.tsx(1 hunks)admin-ui/plugins/auth-server/components/Ssa/SsaListPage.tsx(7 hunks)admin-ui/plugins/auth-server/components/Ssa/types/ssaTypes.ts(1 hunks)admin-ui/plugins/auth-server/components/Ssa/utils.ts(1 hunks)admin-ui/plugins/auth-server/components/Ssa/validations.js(0 hunks)admin-ui/plugins/auth-server/components/Ssa/validations.ts(1 hunks)
💤 Files with no reviewable changes (2)
- admin-ui/plugins/auth-server/components/Sessions/types/sessionTypes.ts
- admin-ui/plugins/auth-server/components/Ssa/validations.js
🧰 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: 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.
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/docs/CODE_SPLITTING.mdadmin-ui/plugins/auth-server/components/Scopes/ScopeForm.tsxadmin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsxadmin-ui/plugins/auth-server/components/Scopes/ScopeListPage.tsxadmin-ui/app/helpers/navigation.ts
📚 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/auth-server/components/Scopes/ScopeForm.tsxadmin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsxadmin-ui/plugins/auth-server/components/Scopes/ScopeListPage.tsxadmin-ui/plugins/auth-server/components/Ssa/SsaListPage.tsxadmin-ui/app/helpers/navigation.ts
📚 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/auth-server/components/Scopes/ScopeForm.tsxadmin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsxadmin-ui/plugins/auth-server/components/Configuration/Keys/KeysPage.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/auth-server/components/Scopes/ScopeListPage.tsx
🧬 Code graph analysis (11)
admin-ui/plugins/auth-server/components/Scopes/hooks/useScopeActions.ts (1)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(75-104)ROUTES(106-106)
admin-ui/plugins/auth-server/components/Scopes/ScopeForm.tsx (1)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(75-104)ROUTES(106-106)
admin-ui/plugins/auth-server/components/Scopes/ScopeListPage.tsx (3)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(75-104)ROUTES(106-106)admin-ui/plugins/auth-server/components/Scopes/types/index.ts (1)
ScopeTableRow(33-33)admin-ui/plugins/auth-server/components/Scopes/types/componentTypes.ts (1)
ScopeTableRow(24-28)
admin-ui/plugins/auth-server/components/Ssa/utils.ts (1)
admin-ui/plugins/auth-server/components/Ssa/types/ssaTypes.ts (1)
SsaFormValues(24-32)
admin-ui/plugins/auth-server/components/JsonViewer/JsonViewer.tsx (1)
admin-ui/plugins/auth-server/redux/sagas/SsaSaga.js (1)
jsonData(135-135)
admin-ui/plugins/auth-server/components/Ssa/SsaListPage.tsx (3)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(75-104)ROUTES(106-106)admin-ui/plugins/auth-server/components/Ssa/types/ssaTypes.ts (1)
SsaData(14-18)admin-ui/plugins/auth-server/components/Ssa/types/index.ts (1)
SsaData(3-3)
admin-ui/plugins/auth-server/components/Sessions/SessionListPage.tsx (3)
admin-ui/plugins/auth-server/components/Sessions/types/sessionTypes.ts (1)
Session(14-23)admin-ui/plugins/auth-server/components/Sessions/types/index.ts (1)
Session(3-3)admin-ui/app/customColors.ts (1)
customColors(4-25)
admin-ui/plugins/auth-server/components/JsonViewer/JsonViewerDialog.tsx (1)
admin-ui/app/customColors.ts (1)
customColors(4-25)
admin-ui/plugins/auth-server/components/Ssa/types/ssaTypes.ts (2)
admin-ui/plugins/schema/components/types/AttributeListPage.types.ts (1)
ModifiedFieldValue(9-16)admin-ui/plugins/auth-server/components/Ssa/types/index.ts (1)
ModifiedFields(10-10)
admin-ui/plugins/auth-server/components/Ssa/validations.ts (1)
admin-ui/plugins/auth-server/components/Ssa/types/ssaTypes.ts (1)
SsaFormValues(24-32)
admin-ui/plugins/auth-server/components/Sessions/SessionDetailPage.tsx (2)
admin-ui/plugins/auth-server/components/Sessions/types/sessionTypes.ts (1)
SessionDetailPageProps(35-37)admin-ui/plugins/auth-server/components/Sessions/types/index.ts (1)
SessionDetailPageProps(5-5)
🔇 Additional comments (26)
admin-ui/plugins/auth-server/components/Sessions/SessionDetailPage.tsx (1)
43-48: LGTM! Type safety improvement.The refinement from
anyto a specific union type improves type safety and aligns with TypeScript best practices. The new signature correctly reflects the actual usage at lines 108 and 119.admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkListPage.tsx (1)
9-11: Stable key helper improves type-safety and readability
generateStableKeytaking a non-optionalkidand handling the empty-string case keeps the implementation simple while making the type contract explicit. This looks good.admin-ui/app/routes/Apps/Gluu/GluuRemovableInputRow.tsx (1)
15-47: LGTM! Good TypeScript generic implementation.The addition of the generic type parameter
TValuesappropriately improves type safety for Formik integration while maintaining backward compatibility through the default type parameter.admin-ui/plugins/auth-server/components/Sessions/SessionListPage.tsx (6)
1-8: LGTM! Type safety improvements.The addition of explicit type imports (
SyntheticEvent,Action) and the migration fromanytoAuditRootStateimprove type safety throughout the component.Also applies to: 11-11, 46-47, 57-57
73-73: LGTM! Improved state typing.Replacing
anywithAuditRootStateprovides proper type checking for the auth state selector.
389-394: LGTM! Correct event typing.The signature now properly matches Material-UI Autocomplete's
onChangeprop expectations withSyntheticEvent.
451-454: LGTM! Better component typing.Using
React.ComponentProps<typeof Paper>provides accurate type checking for the Paper container props.
478-478: LGTM! Consistent i18n usage.Using the translation key ensures the loading message is properly internationalized.
509-509: LGTM! Consistent with tableActions refactor.The MaterialTable now correctly uses the new memoized
tableActionsarray.admin-ui/plugins/auth-server/components/Configuration/Keys/KeysPage.tsx (1)
12-44: GluuFormFooter integration on Keys page looks consistentAdding
GluuFormFooterwith onlyshowBackenabled matches this read‑only configuration page and aligns with the shared footer pattern; no issues spotted here.admin-ui/docs/CODE_SPLITTING.md (1)
11-133: Updated code‑splitting docs are clear and aligned with current routingThe examples (lazy routes, plugin/component loading,
/adm/dashboardroute, analyze/build commands) are coherent and match the current navigation patterns; nothing blocking here.admin-ui/plugins/auth-server/components/Scopes/ScopeForm.tsx (1)
39-40: Back/Cancel/Apply wiring in ScopeForm aligns with centralized navigation and dirty‑state rulesUsing
useAppNavigationwithnavigateBack(ROUTES.AUTH_SERVER_SCOPES_LIST)ensures Back goes to the previous page when possible and otherwise falls back to the scopes list, matching the “redirect to listing screen” behavior adopted in other forms. Footer props (disableCancel/disableApplybased onhasActualChangesand Formik state) correctly implement the desired Back/Cancel/Apply semantics without navigation side effects.Based on learnings, this matches the pattern used for SSA and UserForm back‑navigation.
Also applies to: 61-62, 159-162, 672-687
admin-ui/plugins/auth-server/components/Scopes/hooks/useScopeActions.ts (1)
7-8: Scope navigation hook now consistently uses centralized ROUTESSwitching
navigateToScopeList/Add/EdittonavigateToRoute(ROUTES.AUTH_SERVER_*)viauseAppNavigationis clean and keeps navigation logic centralized; dependency arrays are correct and no regressions are apparent.Also applies to: 27-28, 83-96
admin-ui/plugins/auth-server/components/Scopes/ScopeListPage.tsx (1)
47-48: Centralized navigation in ScopeListPage is correct and guardedUsing
useAppNavigationwithROUTES.AUTH_SERVER_SCOPE_ADD/ROUTES.AUTH_SERVER_SCOPE_EDIT(row.inum)brings this list page in line with the new navigation helper, and therow.inumguard on edit avoids navigating to an invalid route when the key is missing. Looks good.Also applies to: 81-84, 314-325
admin-ui/plugins/auth-server/components/Ssa/utils.ts (2)
1-12: LGTM!Good TypeScript migration. The explicit return type
SsaFormValuesensures type safety, and the initial values correctly match all fields defined in theSsaFormValuesinterface.
15-27: LGTM!The function properly handles edge cases: null input, invalid dates, and exceptions. The explicit return type
number | nullis clear.admin-ui/plugins/auth-server/components/JsonViewer/JsonViewer.tsx (1)
7-21: LGTM!Clean TypeScript migration with well-defined props interface and sensible defaults. The removal of PropTypes in favor of TypeScript interfaces improves type safety.
admin-ui/app/helpers/navigation.ts (2)
34-41: LGTM!The new Auth Server routes are well-structured with proper URL encoding for dynamic parameters. The naming convention is consistent with existing routes.
5-6: Dashboard path migration is complete.All references to the old
/home/dashboardpath have been successfully updated to/adm/dashboard. No remaining references to the old path were found in the codebase.admin-ui/plugins/auth-server/components/JsonViewer/JsonViewerDialog.tsx (2)
8-26: LGTM!Clean TypeScript interface with appropriate optional props and sensible defaults. The component signature is clear and well-typed.
39-48: LGTM!The conditional rendering correctly handles all states: loading spinner when fetching, JsonViewer when data is available, and nothing when neither condition is met. This aligns well with the loading state integration mentioned in the PR objectives.
admin-ui/plugins/auth-server/components/Ssa/SsaListPage.tsx (4)
39-39: LGTM!Good adoption of the centralized navigation hook. This aligns with the PR objective of consistent navigation behavior.
145-159: LGTM!Disabling actions while
getSsaJwtMutation.isPendingprevents users from triggering multiple concurrent JWT fetches, which is good UX.
206-218: LGTM!The refactored
handleViewSsaprovides good UX:
- Clears previous data immediately
- Opens dialog with loading state
- Populates data on success or closes dialog on error
The error handling properly closes the dialog and shows a toast, preventing users from seeing a stuck loading state.
296-306: LGTM!The
JsonViewerDialogcorrectly receives theisLoadingprop tied to the mutation state, providing immediate feedback to users while the JWT is being fetched.admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx (1)
36-41: Navigation refactor touseAppNavigationand SSA list route looks correctUsing
useAppNavigationwith:
handleNavigateBack→navigateBack(ROUTES.AUTH_SERVER_SSA_LIST)- post-create redirect →
navigateToRoute(ROUTES.AUTH_SERVER_SSA_LIST)in thecreateSsaMutation.isSuccesseffectkeeps the user within the SSA management context and provides a clear fallback to the SSA list rather than an unrelated screen. This matches the prior requirement that the SSA Add Back behavior should fall back to
/auth-server/config/ssawhen history is unavailable. Based on learnings, this is the expected behavior for this page.No changes needed here.
Also applies to: 65-67, 90-95
admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkListPage.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Apply button not working in SSA
- Cancel button not working for
Is Expirable?field in SSA page
…i-issue-2361-auth-server
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx(1 hunks)admin-ui/app/routes/Apps/Gluu/styles/applicationstyle.tsx(1 hunks)admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx(5 hunks)admin-ui/plugins/auth-server/components/Ssa/types/ssaTypes.ts(3 hunks)admin-ui/plugins/auth-server/components/Ssa/utils.ts(1 hunks)admin-ui/plugins/auth-server/components/Ssa/validations.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 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: 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.
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/auth-server/components/Ssa/SsaAddPage.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/auth-server/components/Ssa/SsaAddPage.tsx
🧬 Code graph analysis (5)
admin-ui/app/routes/Apps/Gluu/styles/applicationstyle.tsx (1)
admin-ui/app/customColors.ts (1)
customColors(4-25)
admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx (7)
admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(75-104)admin-ui/plugins/auth-server/components/Ssa/types/index.ts (2)
SsaFormValues(5-5)SsaCreatePayload(6-6)admin-ui/plugins/auth-server/components/Ssa/utils.ts (2)
getSsaInitialValues(4-14)toEpochSecondsFromDayjs(17-30)admin-ui/plugins/auth-server/components/Ssa/validations.ts (1)
ssaValidationSchema(5-31)admin-ui/plugins/admin/components/Webhook/WebhookForm.js (1)
openCommitDialog(84-84)admin-ui/plugins/auth-server/components/Ssa/constants.ts (2)
FORM_LAYOUT(14-20)GRANT_TYPES(1-8)admin-ui/plugins/auth-server/redux/audit/Resources.js (2)
SSA(6-6)SSA(6-6)
admin-ui/plugins/auth-server/components/Ssa/utils.ts (1)
admin-ui/plugins/auth-server/components/Ssa/types/ssaTypes.ts (2)
SsaFormValues(26-36)ExpirationDate(24-24)
admin-ui/plugins/auth-server/components/Ssa/validations.ts (1)
admin-ui/plugins/auth-server/components/Ssa/types/ssaTypes.ts (1)
SsaFormValues(26-36)
admin-ui/plugins/auth-server/components/Ssa/types/ssaTypes.ts (2)
admin-ui/plugins/auth-server/components/Ssa/types/index.ts (2)
ExpirationDate(11-11)ModifiedFields(10-10)admin-ui/plugins/schema/components/types/AttributeListPage.types.ts (1)
ModifiedFieldValue(9-16)
🔇 Additional comments (8)
admin-ui/app/routes/Apps/Gluu/styles/applicationstyle.tsx (1)
44-47: LGTM! Clean style addition for vertical separation.The
verticalDividerstyle is well-formed and uses the existingcustomColors.lightGrayconstant. This aligns with the PR's goal of improving button layout consistency.admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (1)
87-91: LGTM! Good refactoring—consolidates cleanup in one place.Moving
setUserMessage('')to occur afteronCloseModal()ensures the message remains available throughout the close flow and centralizes cleanup logic. The existinguseEffect(lines 75-77) already clears the message when the modal reopens, preventing unintended persistence.admin-ui/plugins/auth-server/components/Ssa/utils.ts (1)
1-30: LGTM!The utility functions are well-typed with explicit return types, proper null handling in
toEpochSecondsFromDayjs, and the initial values align with the updatedSsaFormValuesinterface including the newis_expirableandexpirationDatefields.admin-ui/plugins/auth-server/components/Ssa/types/ssaTypes.ts (2)
24-36: LGTM!The new
ExpirationDatetype alias and extendedSsaFormValuesinterface properly support the expiration date functionality. The types align well with the validation schema and utility functions.
62-64: Type scope is appropriate.
ModifiedFieldValueis kept as a private type (not exported), limiting its scope to SSA-specific usage. This addresses the previous concern about divergent definitions across modules.admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx (3)
61-63: Correct fallback route for SSA context.The Back button correctly falls back to
ROUTES.AUTH_SERVER_SSA_LIST, keeping users in the SSA management context. This aligns with the learned pattern from previous PRs.
142-145: Expiration fields now properly tracked via Formik.Since
is_expirableandexpirationDateare now Formik-managed fields,formik.dirtycorrectly reflects changes to these values, andformik.resetForm()in the Cancel handler properly resets them. This addresses the previous review concern about expiration state not being part of dirty tracking.
381-396: Form footer logic correctly implements the PR objectives.The Cancel/Apply button behavior aligns with the stated requirements:
- Cancel resets form state and is disabled when no changes exist
- Apply triggers form submission and is disabled when form is invalid, unchanged, or submitting
- Back navigates to the appropriate context-specific route
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
admin-ui/plugins/auth-server/components/Ssa/validations.ts (1)
5-13: Confirm mandatory SSA fields and avoid hiding schema/type mismatchesRight now only
software_idanddescriptionare required;org_id,software_roles, andgrant_typesare effectively optional/empty, even thoughSsaFormValuestypes them as always present. Please double‑check the SSA business rules: if those fields must be provided and non‑empty, add.required()fororg_idand.min(1, ...)on the arrays, or relax the TS type instead so UI validation and types stay in sync. Also, theas Yup.ObjectSchema<SsaFormValues>cast can mask future drift (e.g., adding a field toSsaFormValuesbut forgetting to update the schema); consider typing the constant instead of casting, or usingsatisfiesto keep compiler checks.- org_id: Yup.string(), + org_id: Yup.string().required('Organization ID is required'), @@ - software_roles: Yup.array().of(Yup.string()), + software_roles: Yup.array().of(Yup.string()).min(1, 'At least one role is required'), @@ - grant_types: Yup.array().of(Yup.string()), + grant_types: Yup.array().of(Yup.string()).min(1, 'At least one grant type is required'),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/auth-server/components/Ssa/validations.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 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: 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.
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.
🧬 Code graph analysis (1)
admin-ui/plugins/auth-server/components/Ssa/validations.ts (1)
admin-ui/plugins/auth-server/components/Ssa/types/ssaTypes.ts (1)
SsaFormValues(26-36)
|



feat(admin): adding uniformity in cancel and back buttons present in Auth Server(SSA, Keys) rest menus are under TS migration
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
UI/UX
Other
✏️ Tip: You can customize this high-level summary in your review settings.