fix(admin-ui): Use typescript generated SCRIPTS pages#2478
Conversation
syntrydy
added
comp-admin-ui
|
Warning Rate limit exceeded@syntrydy has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 35 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces legacy sidebar animation modules with typed TypeScript classes; migrates Custom Scripts UI from Redux/sagas to React Query hooks/mutations with webhooks and audit logging; adds new Custom Scripts components, hooks, types, constants, translations, and several small tightening/refactor edits. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant UI as Custom Scripts UI
participant Hooks as useCustomScriptApi
participant Cache as React Query Cache
participant API as Backend API
participant Webhook as WebhookService
participant Audit as AuditService
U->>UI: Open list / interact (search/filter)
UI->>Hooks: useCustomScriptsByType(type)
Hooks->>Cache: check cache key
alt cache miss
Hooks->>API: GET /scripts?type=...
API-->>Hooks: 200 scripts
Hooks->>Cache: store response
else cache hit
Cache-->>Hooks: cached data
end
Hooks-->>UI: data + loading
U->>UI: create/update/delete script
UI->>Hooks: mutateAsync(payload)
Hooks->>API: POST/PUT/DELETE /scripts
API-->>Hooks: success/error
Hooks->>Cache: invalidate related queries
Hooks->>Webhook: trigger webhook (if configured)
Hooks->>Audit: post audit log
Hooks-->>UI: mutation result
UI->>U: toast / optional navigate (useMutationEffects)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
admin-ui/plugins/admin/components/Webhook/WebhookURLChecker.js (1)
24-33: Preferfor...ofor array methods overfor...infor arrays.The change from
lettoconstis fine. However,for...initerates over array indices (keys), not values, which is error-prone and non-idiomatic for arrays. Consider refactoring to useArray.prototype.some()for clarity and conciseness.const isAllowed = (url) => { - let result = true - for (const extention in NOT_ALLOWED) { - if (url.startsWith(NOT_ALLOWED[extention])) { - result = false - break - } - } - return result + return !NOT_ALLOWED.some((prefix) => url.startsWith(prefix)) }admin-ui/plugins/admin/redux/features/mappingSlice.js (1)
44-54: Add boundary check and simplify Redux Toolkit mutation pattern.Two issues:
Missing boundary check:
findIndexreturns-1if the role is not found. Lines 48-49 will attempt to access and mutatestate.items[-1].permissions, causing undefined behavior or runtime errors.Redundant alias-mutate-spread pattern: With Redux Toolkit's Immer, you can directly remove the permission using
spliceonstate.items[index].permissions. The current alias pattern is unnecessarily complex.Apply this diff:
updateMapping: (state, action) => { if (action.payload?.data) { const { id, role } = action.payload.data const index = state.items.findIndex((element) => element.role === role) + if (index === -1) return - const permissions = state.items[index].permissions - permissions.splice(id, 1) - const changedPermissions = state.items - changedPermissions[index].permissions = permissions - state.items = [...changedPermissions] + state.items[index].permissions.splice(id, 1) } },admin-ui/plugins/services/redux/api/LdapApi.js (1)
50-58: Minor readability tweak fortestLdapConfigoptions constructionYou can simplify the options creation while keeping behavior identical:
- testLdapConfig = (input) => { - const options = {} - options['gluuLdapConfiguration'] = input + testLdapConfig = (input) => { + const options = { + gluuLdapConfiguration: input, + }This avoids the intermediate empty object + mutation and makes the payload shape clearer at a glance.
admin-ui/app/components/dashboard-style-airframe/side-menu-animate.ts (1)
77-86: Duplicatedestroymethod definition
SideMenuAnimate.prototype.destroyis defined twice with identical bodies. While this doesn’t change behavior (the second overrides the first with the same implementation), it’s redundant and slightly confusing.Consider removing one of the duplicate definitions to reduce noise:
-SideMenuAnimate.prototype.destroy = function () { - this._nodesObserver.disconnect() -} - /** * Disconnects the observer */ SideMenuAnimate.prototype.destroy = function () { this._nodesObserver.disconnect() }admin-ui/app/components/dashboard-style-airframe/slim-menu-animate.ts (1)
46-83: Unreachable mouse‑out animation codeInside
mouseOutHandler, there is an unconditionalreturnbefore the timeline is constructed:this.mouseOutHandler = function () { if (_this._animationsEnabled()) { const triggerElement = this const subMenuElement = triggerElement.querySelector(':scope > .sidebar-submenu') return const timeline = anime.timeline({ /* ... */ }) // ... } }As written, the mouse‑out animation never runs, and everything after
returnis dead code. If the intent is to have no mouse‑out animation, consider deleting the unused block; if you do want the closing animation, drop thereturn(and re‑verify styles).- const subMenuElement = triggerElement.querySelector(':scope > .sidebar-submenu') - return - const timeline = anime + const subMenuElement = triggerElement.querySelector(':scope > .sidebar-submenu') + const timeline = anime .timeline({ targets: subMenuElement, duration: ANIMATION_DURATION, // ... })admin-ui/plugins/auth-server/components/AuthN/AuthNListPage.js (1)
117-130: Stale closure risk:scriptsLoadingandlistnot in dependency array.The effect references
scriptsLoadingandlistbut only depends on[scripts]. This can cause stale values. Use functional state update to avoid needinglistin deps:useEffect(() => { - setList({ ...list, scripts: [] }) if (scripts.length > 0 && !scriptsLoading) { const getEnabledscripts = scripts.filter((item) => item.enabled === true) if (getEnabledscripts?.length > 0) { const updateScriptsItems = getEnabledscripts.map((item) => ({ ...item, name: 'myAuthnScript', acrName: item.name, })) - setList({ ...list, scripts: updateScriptsItems }) + setList((prev) => ({ ...prev, scripts: updateScriptsItems })) + } else { + setList((prev) => ({ ...prev, scripts: [] })) } + } else { + setList((prev) => ({ ...prev, scripts: [] })) } - }, [scripts]) + }, [scripts, scriptsLoading])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (32)
admin-ui/app/components/dashboard-style-airframe/side-menu-animate.ts(3 hunks)admin-ui/app/components/dashboard-style-airframe/sidebar-entry-animate.ts(5 hunks)admin-ui/app/components/dashboard-style-airframe/slim-menu-animate.ts(4 hunks)admin-ui/app/components/dashboard-style-airframe/slim-sidebar-animate.ts(2 hunks)admin-ui/app/locales/en/translation.json(7 hunks)admin-ui/app/locales/es/translation.json(7 hunks)admin-ui/app/locales/fr/translation.json(7 hunks)admin-ui/app/locales/pt/translation.json(7 hunks)admin-ui/app/redux/api/LicenseApi.ts(2 hunks)admin-ui/app/routes/Apps/Gluu/GluuPropertyItem.tsx(1 hunks)admin-ui/plugins/PluginMenuResolver.js(2 hunks)admin-ui/plugins/admin/components/CustomScripts/CustomScriptAddPage.tsx(1 hunks)admin-ui/plugins/admin/components/CustomScripts/CustomScriptEditPage.tsx(1 hunks)admin-ui/plugins/admin/components/CustomScripts/CustomScriptForm.tsx(10 hunks)admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx(1 hunks)admin-ui/plugins/admin/components/CustomScripts/PersonAuthenticationFields.tsx(1 hunks)admin-ui/plugins/admin/components/CustomScripts/ScriptListPage.tsx(0 hunks)admin-ui/plugins/admin/components/CustomScripts/ScriptListPage.tsx(1 hunks)admin-ui/plugins/admin/components/CustomScripts/constants.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/helper/utils.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/hooks/auditUtils.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/hooks/index.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptActions.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/types/forms.ts(1 hunks)admin-ui/plugins/admin/components/Webhook/WebhookURLChecker.js(1 hunks)admin-ui/plugins/admin/redux/features/apiRoleSlice.js(1 hunks)admin-ui/plugins/admin/redux/features/mappingSlice.js(2 hunks)admin-ui/plugins/auth-server/components/AuthN/AuthNListPage.js(2 hunks)admin-ui/plugins/scripts/plugin-metadata.ts(3 hunks)admin-ui/plugins/services/Components/Configuration/LdapAddPage.js(1 hunks)admin-ui/plugins/services/redux/api/LdapApi.js(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/CustomScripts/hooks/auditUtils.ts
🧬 Code graph analysis (9)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (11)
admin-ui/plugins/auth-server/components/AuthN/AuthNListPage.js (3)
useTranslation(25-25)dispatch(26-26)theme(30-30)admin-ui/plugins/services/Components/Configuration/LdapAddPage.js (1)
dispatch(10-10)admin-ui/app/utils/pagingUtils.ts (1)
getPagingSize(1-24)admin-ui/plugins/admin/components/CustomScripts/constants.ts (1)
DEFAULT_SCRIPT_TYPE(1-1)admin-ui/plugins/user-management/types/CommonTypes.ts (1)
ThemeContext(13-17)admin-ui/plugins/admin/components/CustomScripts/hooks/index.ts (3)
useCustomScriptsByType(3-3)useCustomScriptTypes(8-8)useDeleteCustomScript(7-7)admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (3)
useCustomScriptsByType(68-84)useCustomScriptTypes(190-225)useDeleteCustomScript(155-188)admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js (1)
errorMessage(100-100)admin-ui/app/customColors.ts (1)
customColors(4-25)admin-ui/plugins/services/Components/Configuration/LdapListPage.js (1)
tableOptions(251-257)admin-ui/app/components/Sidebar/types.ts (1)
MenuItem(2-10)
admin-ui/plugins/admin/components/CustomScripts/types/forms.ts (2)
admin-ui/plugins/admin/redux/features/types/customScript.ts (1)
CustomScriptItem(29-46)admin-ui/plugins/admin/components/CustomScripts/types/customScript.ts (1)
CustomScriptItem(29-49)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptEditPage.tsx (3)
admin-ui/app/helpers/navigation.ts (2)
useAppNavigation(66-95)ROUTES(97-97)admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (2)
useCustomScript(86-95)useUpdateCustomScript(123-153)admin-ui/plugins/admin/components/CustomScripts/types/actions.ts (1)
SubmitData(11-14)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptForm.tsx (5)
admin-ui/plugins/admin/components/CustomScripts/types/forms.ts (1)
CustomScriptFormProps(6-11)admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (1)
useCustomScriptTypes(190-225)admin-ui/plugins/admin/components/CustomScripts/PersonAuthenticationFields.tsx (1)
PersonAuthenticationFields(18-82)admin-ui/plugins/admin/components/CustomScripts/helper/utils.ts (1)
getModuleProperty(6-13)admin-ui/plugins/admin/components/CustomScripts/constants.ts (2)
PROGRAMMING_LANGUAGES(17-20)LOCATION_TYPE_OPTIONS(22-25)
admin-ui/plugins/admin/components/CustomScripts/helper/utils.ts (2)
admin-ui/plugins/admin/components/CustomScripts/types/forms.ts (1)
FormValues(17-32)admin-ui/app/utils/Util.ts (1)
filterEmptyObjects(82-84)
admin-ui/plugins/admin/components/CustomScripts/hooks/auditUtils.ts (2)
admin-ui/app/utils/TokenController.ts (1)
addAdditionalData(49-124)admin-ui/app/redux/api/backend-api.ts (1)
postUserAction(51-69)
admin-ui/plugins/auth-server/components/AuthN/AuthNListPage.js (2)
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (1)
useCustomScriptsByType(68-84)admin-ui/plugins/admin/components/CustomScripts/constants.ts (1)
DEFAULT_SCRIPT_TYPE(1-1)
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (3)
admin-ui/plugins/admin/redux/sagas/WebhookSaga.ts (1)
triggerWebhook(279-368)admin-ui/plugins/admin/components/CustomScripts/hooks/auditUtils.ts (1)
logAuditAction(41-60)admin-ui/plugins/admin/components/CustomScripts/constants.ts (1)
SCRIPT_CACHE_CONFIG(3-9)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptAddPage.tsx (2)
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (1)
useCreateCustomScript(97-121)admin-ui/plugins/admin/components/CustomScripts/types/actions.ts (1)
SubmitData(11-14)
🪛 Biome (2.1.2)
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptActions.ts
[error] 30-30: This hook is being called from a nested function, but all hooks must be called unconditionally from the top-level component.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 37-37: This hook is being called from a nested function, but all hooks must be called unconditionally from the top-level component.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 41-41: This hook is being called from a nested function, but all hooks must be called unconditionally from the top-level component.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ 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)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (49)
admin-ui/plugins/PluginMenuResolver.js (1)
39-39: Use ofconstforpluginRoutesis correct and idiomaticBoth
processRoutesandprocessRoutesSynconly mutatepluginRoutesvia.pushand never reassign the variable, so switching fromlettoconstis safe and clarifies intent without changing behavior.Also applies to: 85-85
admin-ui/app/locales/en/translation.json (7)
51-51: ✓ Approved: New "retry" action added.Clear, concise localization entry supporting retry functionality for async operations.
Also applies to: 51-51
103-103: ✓ Approved: New "search" field added to dashboard/fields.Supports search UI component in dashboard context, consistent with field naming conventions.
Also applies to: 103-103
416-419: ✓ Approved: New pagination and SAML ACR field labels added.Lines 416-419 add well-organized localization keys:
saml_acrs: Aligns with existing SAML field namingrows_per_page,of,more_than: Standard pagination/display textTerminology is appropriate for table UI controls.
823-834: ✓ Approved: Script management CRUD operation messages added.Comprehensive set of 12 localization keys (lines 823–834) covering script lifecycle:
- Success messages:
script_added_successfully,script_deleted_successfully,script_updated_successfully- Error messages:
error_adding_script,error_deleting_script,error_updating_script- Status/state messages:
loading_script,error_loading_script,error_loading_scripts,script_not_found,no_scripts_foundTerminology is consistent with existing message patterns (e.g.,
invalid_script_data). All keys support the PR's migration to React Query–based script management.
875-875: ✓ Approved: Tooltip for sort order toggling added.Clear, instructive tooltip for ascending/descending sort functionality. Placement in tooltips section is appropriate.
Also applies to: 875-875
926-926: ✓ Approved: Script search placeholder added.Placeholder text "Search scripts by name or description" is user-friendly and follows existing placeholder conventions (e.g., line 925
search_pattern).Also applies to: 926-926
1069-1069: ✓ Approved: Storage location type options added.Lines 1069 and 1075 add
databaseandfileoptions, supporting script storage location selection. Placement in options section and terminology are consistent with existing patterns.Also applies to: 1075-1075
admin-ui/app/locales/es/translation.json (7)
51-51: ✓ Approved: Spanish translation for "retry" action.Translation "Reintentar" is appropriate and idiomatic for Spanish-speaking users.
Also applies to: 51-51
103-103: ✓ Approved: Spanish translation for "search" field.Translation "Buscar" aligns with existing Spanish field translations (e.g., line 36
search: "Buscar").Also applies to: 103-103
416-419: ✓ Approved: Spanish translations for pagination and SAML fields.Translations are consistent with existing Spanish terminology:
saml_acrs: "SAML ACRS" (parallels English)rows_per_page: "Filas por página" (standard Spanish for table pagination)of: "de" (preposition used in Spanish pagination e.g., "1 de 10")more_than: "más de" (comparative form in Spanish)All translations follow existing conventions observed in the file.
822-833: ✓ Approved: Spanish translations for script management messages.All 12 script operation messages are appropriately translated:
- Success messages use "exitosamente" (successfully)
- Error messages use "Error al" (Error... / Error when...) pattern consistent with line 816–821
- Loading states: "Cargando script..." aligns with existing loading patterns (e.g., line 837
fetching_project_details)- Status messages follow natural Spanish phrasing
Terminology mirrors English structure for consistency across UI.
871-871: ✓ Approved: Spanish tooltip for sort order toggling.Translation "Alternar orden (ascendente/descendente)" clearly conveys toggle functionality and is consistent with existing Spanish tooltip style.
Also applies to: 871-871
919-919: ✓ Approved: Spanish placeholder for script search.Placeholder "Buscar scripts por nombre o descripción" is idiomatic, user-friendly, and matches the structure of the English version (line 926).
Also applies to: 919-919
1062-1062: ✓ Approved: Spanish translations for storage location options.
database: "Base de datos" (standard Spanish term for database)file: "Archivo" (standard Spanish term for file)Both translations are concise and consistent with existing Spanish option terminology in the file.
Also applies to: 1068-1068
admin-ui/app/locales/fr/translation.json (1)
168-168: Translation keys properly added and appropriately localized for French.All new script-management-related keys have been added consistently across the French translation file with contextually appropriate French translations. The messages maintain clarity and follow established patterns in the codebase (e.g., script operation status messages like
script_added_successfully,error_adding_script). JSON structure is valid with no duplicates.Also applies to: 179-179, 531-534, 759-770, 812-812, 860-860
admin-ui/app/locales/pt/translation.json (1)
165-165: Translation keys properly added and appropriately localized for Portuguese.All new script-management-related keys have been added consistently across the Portuguese translation file, mirroring the French file additions with contextually appropriate Portuguese translations. The terminology remains consistent with the French translations where applicable (e.g., untranslated "script" terms). JSON structure is valid with no duplicates.
Also applies to: 176-176, 544-547, 754-765, 807-807, 853-853
admin-ui/app/routes/Apps/Gluu/GluuPropertyItem.tsx (1)
77-77: LGTM! Conditional margin improves alignment.The change correctly applies top margin only when input labels are present, ensuring proper vertical alignment of the remove button with labeled input fields. When labels are absent, the unnecessary margin is removed, maintaining consistent spacing.
Note: This UI alignment fix appears unrelated to the main PR objective of migrating SCRIPTS pages from Redux to React Query with TypeScript-generated types.
admin-ui/app/redux/api/LicenseApi.ts (1)
33-33: LGTM! Correct immutability improvement.Changing
retriesfromlettoconstis correct—the outer variable is only read once to initialize the recursive call and never reassigned. The parameter inmakeRequestshadows this variable and handles the incrementing.Also applies to: 64-64
admin-ui/plugins/services/Components/Configuration/LdapAddPage.js (1)
25-31: Const binding formessageis safe and clarifies intent
messageis never reassigned, so switching toconstis correct and keeps the function behavior unchanged.admin-ui/plugins/services/redux/api/LdapApi.js (1)
18-27: Immutability cleanup inupdateLdapConfig/addLdapConfiglooks goodUsing
const optionshere is appropriate since the binding is never reassigned and the behavior of both methods is preserved.Also applies to: 30-39
admin-ui/app/components/dashboard-style-airframe/side-menu-animate.ts (1)
3-52:animeimport and local const/let usage are correctThe switch to
constfor the anime import,option, and other locals plusletforactiveAnimationpreserves the existing animation behavior while tightening scoping. No functional regressions stand out.admin-ui/app/components/dashboard-style-airframe/slim-sidebar-animate.ts (1)
2-267: Let/const refactor maintains sidebar animation behaviorThe refactor to use
constfor config and DOM/query results andletfortimelineStage1,timelineStage2, andisAnimatingis consistent and preserves the existing two‑stage animation logic. No new scoping or hoisting issues are introduced.admin-ui/app/components/dashboard-style-airframe/sidebar-entry-animate.ts (1)
3-97: executeAnimation refactor to const/let is safeUsing
constforconfig,sidebarElement, the section arrays, andtimeline, plusletforsidebarMenuSectionandsideMenuEntries, keeps the original animation flow and side effects while improving scoping. No functional differences are introduced.admin-ui/app/components/dashboard-style-airframe/slim-menu-animate.ts (1)
3-44: Const/let usage and animation constants look goodUsing
constforanime, theANIMATION_*constants,_thiscaptures, and timeline/element references appropriately narrows mutability while keeping the hover‑in animation behavior unchanged.Also applies to: 87-123
admin-ui/plugins/admin/components/CustomScripts/hooks/auditUtils.ts (1)
6-26: LGTM!Good defensive coding with proper null checks for required authentication data before constructing the audit log.
admin-ui/plugins/admin/components/CustomScripts/types/forms.ts (2)
7-8: Type inconsistency:itemacceptsCustomScriptbuthandleSubmitonly acceptsCustomScriptItem.The
itemprop now acceptsCustomScript | CustomScriptItem, buthandleSubmitdata parameter still only accepts{ customScript: CustomScriptItem }. If aCustomScriptis passed asitemand submitted, the types would be inconsistent.Consider whether
handleSubmitshould also acceptCustomScript:- handleSubmit: (data: { customScript: CustomScriptItem }) => void | Promise<void> + handleSubmit: (data: { customScript: CustomScriptItem | CustomScript }) => void | Promise<void>Or verify that form transformation always produces
CustomScriptItemregardless of input type.
10-14: LGTM!The
isSubmittingprop and broadenedrowtype align well with the async mutation flow introduced in the edit/add pages.admin-ui/plugins/auth-server/components/AuthN/AuthNListPage.js (1)
44-48: LGTM!The migration from Redux selectors to
useCustomScriptsByTypehook is implemented correctly. The optional chaining onscriptsResponse?.entriesproperly handles the loading state.admin-ui/plugins/admin/components/CustomScripts/CustomScriptEditPage.tsx (4)
28-55: LGTM!The mounted flag pattern provides reasonable protection against dispatching actions after component unmount, especially for navigation. The separation of success and error effects is clean.
57-75: LGTM!The submission flow correctly validates input, extracts the action message for audit logging, and delegates error display to the mutation state effect.
77-106: LGTM!Comprehensive loading and error state handling with appropriate fallback messages.
108-121: LGTM!The form is correctly wired with the mutation pending state for both the loader and the
isSubmittingprop.admin-ui/plugins/admin/components/CustomScripts/CustomScriptAddPage.tsx (2)
50-68: LGTM!The submission flow correctly validates, extracts the action message, and calls the mutation.
74-78: LGTM!Passing an empty object for the add flow is appropriate, assuming
CustomScriptFormhandles default values internally.admin-ui/plugins/admin/components/CustomScripts/constants.ts (1)
1-25: LGTM!Constants are well-organized with sensible cache timing values. The separation of configuration, options arrays, and localization keys follows good practices for maintainability.
admin-ui/plugins/scripts/plugin-metadata.ts (2)
2-2: LGTM!Import updated correctly to reference the new
CustomScriptListPagecomponent, aligning with the migration from Redux to React Query.
39-40: Clean Redux removal.Emptying the
reducersandsagasarrays completes the migration away from Redux for script management while maintaining the plugin metadata interface.admin-ui/plugins/admin/components/CustomScripts/CustomScriptForm.tsx (4)
50-52: LGTM!The
isEditModederivation anduseCustomScriptTypeshook integration are clean. Preventing script type modification in edit mode is the correct business logic.
292-310: LGTM!The MUI Alert component with error styling and action button is well-implemented. The conditional rendering based on
scriptError?.stackTraceprovides clear error visibility.
360-365: Good component extraction.Extracting
PersonAuthenticationFieldsreduces form complexity and improves maintainability. Props are correctly passed including thegetModulePropertyhelper.
426-431: LGTM!Using centralized
PROGRAMMING_LANGUAGESconstant instead of inline options improves maintainability and consistency.admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (5)
50-52: LGTM!The
ScriptTableRowinterface extendingCustomScriptwith an index signature is necessary for MaterialTable's dynamic property access.
150-169: LGTM!The delete confirmation handler properly uses
mutateAsyncwith try-catch, shows appropriate toast messages, and cleans up state on success. Error message extraction handles both Error instances and fallback translation.
311-374: LGTM!Actions are correctly gated by permission checks (
canWrite,canRead,canDelete). The conditional action list construction ensures users only see operations they're authorized to perform.
524-545: LGTM!MaterialTable integration is well-configured with custom Container, detail panel for inline editing, and proper localization. The loading state correctly combines
isLoadinganddeleteMutation.isPending.
546-564: LGTM!TablePagination is properly configured with server-side pagination controls, i18n support for labels, and appropriate styling. The
labelDisplayedRowsfunction handles both known and unknown counts gracefully.admin-ui/plugins/admin/components/CustomScripts/PersonAuthenticationFields.tsx (1)
28-51: LGTM!The SAML ACRS multi-select implementation is correct. The type assertion on line 39 is necessary due to reactstrap's typing limitations for select elements.
admin-ui/plugins/admin/components/CustomScripts/hooks/index.ts (1)
1-13: LGTM!Clean barrel export consolidating the Custom Scripts hooks API for easier imports by consumers.
admin-ui/plugins/admin/components/CustomScripts/CustomScriptAddPage.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/CustomScriptAddPage.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/CustomScriptEditPage.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/CustomScriptForm.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/PersonAuthenticationFields.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (7)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (1)
442-456: InputProps usage flagged in previous review.A past review comment already suggested migrating from
InputPropstoslotProps.inputfor MUI v5 forward compatibility.admin-ui/plugins/admin/components/CustomScripts/PersonAuthenticationFields.tsx (1)
53-71: Hardcoded strings should use i18n.The label "Interactive" (line 54) and option labels "Web", "Native", "Both methods" (lines 68-70) are hardcoded while other UI text uses
t()for internationalization.admin-ui/plugins/admin/components/CustomScripts/hooks/auditUtils.ts (1)
33-33: Double type cast suggests store type mismatch.The
as unknown as RootStatepattern bypasses type checking. Consider updating the store's type definitions to returnRootStatedirectly.admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptActions.ts (1)
73-78: MissingqueryFninprefetchQuerywill cause runtime failure.
prefetchQueryrequires aqueryFnto fetch the data. Without it, React Query will throw an error unless this query has been previously cached.Apply this diff to add the missing
queryFn:getCustomScriptByType: (payload: { action: LegacyScriptAction }) => { const { type, ...params } = payload.action queryClient.prefetchQuery({ queryKey: getGetConfigScriptsByTypeQueryKey(type, params), + queryFn: () => getConfigScriptsByType(type, params), }) },Note: You'll need to import the
getConfigScriptsByTypefunction fromJansConfigApi.admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (3)
90-90: Variable shadowing:triggerWebhookshadows imported action creator.The local constant
triggerWebhookreturned fromuseWebhookTrigger()shadows the importedtriggerWebhookaction creator from line 21, reducing readability.Consider renaming the local binding:
- const triggerWebhook = useWebhookTrigger() + const webhookTrigger = useWebhookTrigger()Then update usages at lines 100, 131, and 169 to use the new name.
103-103: AvoidJSON.parse(JSON.stringify(...))for cloning.This cloning approach loses
Dateobjects,undefinedvalues,Map/Set, and functions. Consider usingstructuredClone()for a more robust deep clone.Apply this diff:
- action_data: JSON.parse(JSON.stringify(variables.data)) as Record<string, unknown>, + action_data: structuredClone(variables.data) as Record<string, unknown>,Also applies to line 134 in
useUpdateCustomScript.
224-227: Returning hook references may lead to Rules of Hooks violations.Returning hook references (
useCustomScripts,useCustomScriptsByType,useCustomScript,useCustomScriptTypes) fromuseCustomScriptOperationsencourages consumers to call these hooks conditionally or in loops, violating React's Rules of Hooks.Consider either:
- Remove these from the return object and have consumers import hooks directly
- Call these hooks at the top level and return their results
For example, if consumers need dynamic script fetching, consider:
return { - useScripts: useCustomScripts, - useScriptsByType: useCustomScriptsByType, - useScript: useCustomScript, - useScriptTypes: useCustomScriptTypes, createScript: createMutation, updateScript: updateMutation, deleteScript: deleteMutation, scriptTypes: scriptTypesQuery.data || [], scriptTypesLoading: scriptTypesQuery.isLoading, }Consumers should import and call
useCustomScripts,useCustomScriptsByType, etc. directly at the component's top level.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (17)
admin-ui/app/utils/TokenController.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/CustomScriptAddPage.tsx(1 hunks)admin-ui/plugins/admin/components/CustomScripts/CustomScriptDetailPage.tsx(2 hunks)admin-ui/plugins/admin/components/CustomScripts/CustomScriptEditPage.tsx(1 hunks)admin-ui/plugins/admin/components/CustomScripts/CustomScriptForm.tsx(10 hunks)admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx(1 hunks)admin-ui/plugins/admin/components/CustomScripts/PersonAuthenticationFields.tsx(1 hunks)admin-ui/plugins/admin/components/CustomScripts/helper/utils.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/hooks/auditUtils.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/hooks/index.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptActions.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/hooks/useMutationEffects.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/types/actions.ts(0 hunks)admin-ui/plugins/admin/components/CustomScripts/types/forms.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/types/index.ts(0 hunks)admin-ui/plugins/admin/components/CustomScripts/types/state.ts(0 hunks)
💤 Files with no reviewable changes (3)
- admin-ui/plugins/admin/components/CustomScripts/types/index.ts
- admin-ui/plugins/admin/components/CustomScripts/types/state.ts
- admin-ui/plugins/admin/components/CustomScripts/types/actions.ts
🧰 Additional context used
🧠 Learnings (2)
📚 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/CustomScripts/hooks/auditUtils.ts
📚 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/CustomScripts/hooks/auditUtils.tsadmin-ui/plugins/admin/components/CustomScripts/CustomScriptAddPage.tsxadmin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptActions.ts
🧬 Code graph analysis (8)
admin-ui/plugins/admin/components/CustomScripts/types/forms.ts (2)
admin-ui/plugins/admin/redux/features/types/customScript.ts (1)
CustomScriptItem(29-46)admin-ui/plugins/admin/components/CustomScripts/types/customScript.ts (1)
CustomScriptItem(29-49)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptEditPage.tsx (2)
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (2)
useCustomScript(77-86)useUpdateCustomScript(113-142)admin-ui/plugins/admin/components/CustomScripts/hooks/useMutationEffects.ts (1)
useMutationEffects(15-52)
admin-ui/plugins/admin/components/CustomScripts/hooks/auditUtils.ts (2)
admin-ui/app/utils/TokenController.ts (2)
AdditionalPayload(10-21)addAdditionalData(49-124)admin-ui/app/redux/api/backend-api.ts (1)
postUserAction(51-69)
admin-ui/plugins/admin/components/CustomScripts/PersonAuthenticationFields.tsx (4)
admin-ui/plugins/admin/components/CustomScripts/types/forms.ts (1)
FormValues(16-31)admin-ui/plugins/admin/components/CustomScripts/constants.ts (1)
SAML_ACRS_OPTIONS(11-15)admin-ui/plugins/admin/components/CustomScripts/helper/utils.ts (1)
getModuleProperty(6-13)admin-ui/app/customColors.ts (1)
customColors(4-25)
admin-ui/plugins/admin/components/CustomScripts/hooks/useMutationEffects.ts (2)
admin-ui/plugins/admin/components/CustomScripts/hooks/index.ts (1)
useMutationEffects(15-15)admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js (1)
errorMessage(100-100)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptAddPage.tsx (3)
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (1)
useCreateCustomScript(88-111)admin-ui/plugins/admin/components/CustomScripts/hooks/useMutationEffects.ts (1)
useMutationEffects(15-52)admin-ui/plugins/admin/components/CustomScripts/types/actions.ts (1)
SubmitData(5-7)
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (3)
admin-ui/plugins/admin/redux/sagas/WebhookSaga.ts (1)
triggerWebhook(279-368)admin-ui/plugins/admin/components/CustomScripts/constants.ts (1)
SCRIPT_CACHE_CONFIG(3-9)admin-ui/plugins/admin/components/CustomScripts/hooks/auditUtils.ts (1)
logAuditAction(28-47)
admin-ui/plugins/admin/components/CustomScripts/helper/utils.ts (2)
admin-ui/plugins/admin/components/CustomScripts/types/forms.ts (1)
FormValues(16-31)admin-ui/app/utils/Util.ts (1)
filterEmptyObjects(82-84)
🔇 Additional comments (33)
admin-ui/plugins/admin/components/CustomScripts/types/forms.ts (1)
6-14: Type union approach is reasonable for migration.The union types
CustomScriptItem | CustomScriptenable gradual migration from manually-created types to generated types. The async return type addition forhandleSubmitcorrectly supports mutation-based flows.Note: Components consuming these props may need type guards when accessing
CustomScriptItem-specific fields likeaction_message,script_path, orlocation_typethat don't exist onCustomScript.admin-ui/plugins/admin/components/CustomScripts/CustomScriptDetailPage.tsx (1)
16-116: LGTM!The shorthand fragment syntax (
<>...</>) is idiomatic React and equivalent to the explicitReact.Fragment. No behavioral changes.admin-ui/app/utils/TokenController.ts (1)
10-21: LGTM!Exporting
AdditionalPayloadenables its use in the new audit utilities without duplicating the type definition.admin-ui/plugins/admin/components/CustomScripts/CustomScriptAddPage.tsx (2)
46-46: Empty object passed asitemprop.Passing
{}asitemis appropriate for the "Add" flow. However, sinceitemis typed asCustomScriptItem | CustomScript, TypeScript should accept an empty object. EnsureCustomScriptFormhandles the empty/undefined field cases gracefully.
30-36: I'll help you verify the review comment about unused variables and type assertions. Let me investigate the CustomScript type definition and the code context.
<function_calls>
#!/bin/bashFind CustomScript type definition
rg -n "type CustomScript|interface CustomScript" --type ts --type tsx -A 15
</function_calls>
#!/bin/bashLocate the file under review
find . -type f -name "CustomScriptAddPage.tsx" 2>/dev/null
</function_calls>
<function_calls>
#!/bin/bashRead the CustomScriptAddPage.tsx file to see imports and full context
file_path=$(find . -type f -name "CustomScriptAddPage.tsx" 2>/dev/null | head -1)
if [ -n "$file_path" ]; then
wc -l "$file_path"
fi
</function_calls>
<function_calls>
#!/bin/bashRead first 80 lines of the file to see imports and component structure
file_path=$(find . -type f -name "CustomScriptAddPage.tsx" 2>/dev/null | head -1)
if [ -n "$file_path" ]; then
head -80 "$file_path" | cat -n
fi
</function_calls>
<function_calls>
#!/bin/bashSearch for where CustomScript is imported from
rg -n "import.*CustomScript|from.*CustomScript" --type ts --type tsx | head -20
</function_calls>
<function_calls>
#!/bin/bashLook for type definition files or generated API types
find . -type f ( -name ".d.ts" -o -name "types" -o -name "api" ) -path "/CustomScript*" 2>/dev/null | head -20
</function_calls>
<function_calls>
#!/bin/bashMore comprehensive search for CustomScript definition
rg -n "CustomScript\s*[=:{]" --type ts --type tsx | head -30
</function_calls>admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (3)
150-169: Delete confirmation handler is well-structured.The
handleDeleteConfirmproperly validatesitemToDelete?.inumbefore proceeding, uses async/await with proper error handling, and provides user feedback via toast notifications. The cleanup of modal state on success is correct.
402-426: Good error boundary pattern.The error handling renders a user-friendly message with the actual error details and provides a retry action. This is a solid pattern for handling query failures.
568-578: Delete dialog correctly guarded.The
GluuDialogis properly conditionally rendered only whencanDelete && itemToDeleteare truthy, preventing unnecessary renders and ensuring the dialog has valid data.admin-ui/plugins/admin/components/CustomScripts/hooks/auditUtils.ts (3)
1-4: Good: Proper import ofAdditionalPayloadtype.The import from
Utils/TokenControllercorrectly reuses the existing type definition, avoiding duplication.
6-26: LGTM: Defensive audit log creation.The
createAuditLogfunction properly validates required auth data before creating the log and returnsnullwith a warning when data is missing.
42-46: Good: HandlespostUserActionreturning-1on failure.The error handling correctly checks for the
-1return value frompostUserActionand logs an appropriate error message.admin-ui/plugins/admin/components/CustomScripts/CustomScriptEditPage.tsx (3)
1-12: LGTM: Clean imports and hook-based architecture.Good migration from Redux state to React Query hooks. The imports are well-organized and the component now uses proper data-fetching patterns.
18-20: Good: Proper use ofuseSearchParams.Using React Router's
useSearchParamsinstead of directly accessingwindow.location.searchensures reactive updates on navigation.
50-79: LGTM: Proper loading and error state handling.The component correctly handles loading states with a loader UI and provides meaningful error messages when script fetching fails.
admin-ui/plugins/admin/components/CustomScripts/hooks/useMutationEffects.ts (1)
8-13: LGTM: Well-typed generic interface.The interface properly exposes type parameters for flexibility and provides sensible defaults.
admin-ui/plugins/admin/components/CustomScripts/helper/utils.ts (3)
6-13: LGTM: Defensive property lookup with broadened signature.The function properly handles undefined
propertiesand the broadened type signature accommodates bothModuleProperty[]andSimpleCustomProperty[].
28-28: Good: Uses nullish coalescing forlevel.Using
??instead of||correctly preserves a validlevelof0.
31-33: Good: Correct type cast forconfigurationProperties.The cast to
ConfigurationProperty[]aligns with theFormValuesinterface definition.admin-ui/plugins/admin/components/CustomScripts/PersonAuthenticationFields.tsx (2)
11-16: LGTM: Well-defined props interface.The interface clearly defines the component's dependencies and uses proper TypeScript types.
73-77: LGTM: Validation error display.Properly shows formik validation errors for moduleProperties when touched.
admin-ui/plugins/admin/components/CustomScripts/CustomScriptForm.tsx (8)
38-38: Good: Removed unusedisSubmittingprop.The component now correctly uses only
formik.isSubmittingfor loading state.
45-47: LGTM: Proper edit mode detection and hook usage.The
isEditModeflag correctly determines if editing an existing script, anduseCustomScriptTypesreplaces Redux state access.
287-305: LGTM: Error alert with action button.Good use of MUI Alert with an action button for showing script errors. The styling is consistent with the custom color scheme.
354-361: LGTM: Clean component extraction.Extracting
PersonAuthenticationFieldsimproves maintainability and reduces the complexity of this form component.
380-380: LGTM: Prevent script type changes in edit mode.Disabling
scriptTypewhenisEditModeis true prevents potentially breaking changes to existing scripts.
421-426: LGTM: Using constants for programming languages.Replacing inline options with
PROGRAMMING_LANGUAGESconstant improves maintainability.
452-456: LGTM: Localized location type options.Using
LOCATION_TYPE_OPTIONSwitht(location.key)properly supports i18n for location type labels.
141-159: I'll verify the review comment about the potential stale closure in theuseCallbackdependency array. Let me start by examining the codebase for similar patterns and understanding how Formik is used in this project.
<function_calls>
#!/bin/bashCheck if there are similar patterns in the codebase for comparison
rg -n "useCallback.*formik.(values|setFieldValue)" --type=ts --type=tsx -C2
</function_calls>
<function_calls>
#!/bin/bashLet's examine the file in question to understand the full context
wc -l admin-ui/plugins/admin/components/CustomScripts/CustomScriptForm.tsx
</function_calls>
<function_calls>
#!/bin/bashRead the specific section and surrounding context
sed -n '120,170p' admin-ui/plugins/admin/components/CustomScripts/CustomScriptForm.tsx
</function_calls>
<function_calls>
#!/bin/bashLook for how formik is used in this file
rg -n "formik." admin-ui/plugins/admin/components/CustomScripts/CustomScriptForm.tsx -C1 | head -50
</function_calls>admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptActions.ts (1)
14-60: LGTM: Well-structured action hook.The
useCustomScriptActionshook provides a clean API for CRUD operations with appropriate status flags and refetch helpers. The async wrappers and invalidation strategies follow React Query best practices.admin-ui/plugins/admin/components/CustomScripts/hooks/index.ts (1)
1-15: LGTM: Clean barrel export pattern.The consolidated export surface provides a clean public API for CustomScripts hooks, making them easier to consume across the application.
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (3)
26-48: LGTM: Clean interfaces and safe webhook wrapper.The
ScriptTypeandWebhookPayloadinterfaces are well-defined. TheuseWebhookTriggerhook provides a safe wrapper with appropriate error handling.
50-86: LGTM: Consistent fetch hooks with proper caching.The fetch hooks (
useCustomScripts,useCustomScriptsByType,useCustomScript) follow a consistent pattern with auth token checks and appropriate cache configurations.
180-215: LGTM: Robust script type transformation with defensive checks.The
useCustomScriptTypeshook includes thorough null/undefined checks and handles various formatting cases (underscore-delimited, simple types). The extended cache times are appropriate for infrequently changing data.
admin-ui/plugins/admin/components/CustomScripts/CustomScriptEditPage.tsx
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/hooks/useMutationEffects.ts
Outdated
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/PersonAuthenticationFields.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
admin-ui/plugins/admin/components/CustomScripts/PersonAuthenticationFields.tsx (2)
54-70: Hardcoded strings should use i18n.The label "Interactive" (line 54) and option labels "Web", "Native", "Both methods" (lines 68-70) are hardcoded while other UI text in this component uses
t()for internationalization.Apply this diff and add corresponding translation keys to locale files:
<FormGroup row> - <GluuLabel label="Interactive" required doc_category={SCRIPT} doc_entry="usage_type" /> + <GluuLabel label="fields.interactive" required doc_category={SCRIPT} doc_entry="usage_type" /> <Col sm={9}> <InputGroup> <CustomInput // ... > <option value="">{t('options.choose')}...</option> - <option value="interactive">Web</option> - <option value="service">Native</option> - <option value="both">Both methods</option> + <option value="interactive">{t('options.web')}</option> + <option value="service">{t('options.native')}</option> + <option value="both">{t('options.both_methods')}</option> </CustomInput>
38-41: Type casting workaround for select element.The double cast through
unknownis a workaround for the component library's typing. If theInputcomponent supportsChangeEvent<HTMLSelectElement>fortype="select", prefer using that directly.admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (1)
439-452: Consider migrating fromInputPropstoslotProps.input.In MUI v5,
InputPropsis being phased out in favor ofslotProps.inputfor forward compatibility with MUI v6.admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (1)
88-111: Variable shadowing:triggerWebhookshadows imported action creator.The local
triggerWebhookfromuseWebhookTrigger()shadows the importedtriggerWebhookat line 21. This was flagged in a previous review.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
admin-ui/app/context/theme/config.ts(4 hunks)admin-ui/plugins/admin/components/CustomScripts/CustomScriptAddPage.tsx(1 hunks)admin-ui/plugins/admin/components/CustomScripts/CustomScriptEditPage.tsx(1 hunks)admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx(1 hunks)admin-ui/plugins/admin/components/CustomScripts/PersonAuthenticationFields.tsx(1 hunks)admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/hooks/useMutationEffects.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/types/actions.ts(0 hunks)admin-ui/plugins/admin/components/CustomScripts/types/customScript.ts(0 hunks)admin-ui/plugins/admin/components/CustomScripts/types/forms.ts(1 hunks)
💤 Files with no reviewable changes (2)
- admin-ui/plugins/admin/components/CustomScripts/types/actions.ts
- admin-ui/plugins/admin/components/CustomScripts/types/customScript.ts
🧰 Additional context used
🧠 Learnings (1)
📚 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/CustomScripts/CustomScriptAddPage.tsxadmin-ui/plugins/admin/components/CustomScripts/hooks/useMutationEffects.ts
🧬 Code graph analysis (7)
admin-ui/app/context/theme/config.ts (1)
admin-ui/app/customColors.ts (1)
customColors(4-25)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptEditPage.tsx (3)
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (2)
useCustomScript(77-86)useUpdateCustomScript(113-142)admin-ui/plugins/admin/components/CustomScripts/hooks/useMutationEffects.ts (1)
useMutationEffects(15-46)admin-ui/plugins/admin/components/CustomScripts/types/actions.ts (1)
SubmitData(3-5)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptAddPage.tsx (2)
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (1)
useCreateCustomScript(88-111)admin-ui/plugins/admin/components/CustomScripts/hooks/useMutationEffects.ts (1)
useMutationEffects(15-46)
admin-ui/plugins/admin/components/CustomScripts/PersonAuthenticationFields.tsx (4)
admin-ui/plugins/admin/components/CustomScripts/types/forms.ts (1)
FormValues(14-29)admin-ui/plugins/admin/components/CustomScripts/constants.ts (1)
SAML_ACRS_OPTIONS(11-15)admin-ui/plugins/admin/components/CustomScripts/helper/utils.ts (1)
getModuleProperty(6-13)admin-ui/app/customColors.ts (1)
customColors(4-25)
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (4)
admin-ui/plugins/admin/components/CustomScripts/types/customScript.ts (1)
ScriptType(22-25)admin-ui/plugins/admin/redux/sagas/WebhookSaga.ts (1)
triggerWebhook(279-368)admin-ui/plugins/admin/components/CustomScripts/constants.ts (1)
SCRIPT_CACHE_CONFIG(3-9)admin-ui/plugins/admin/components/CustomScripts/hooks/auditUtils.ts (1)
logAuditAction(28-47)
admin-ui/plugins/admin/components/CustomScripts/hooks/useMutationEffects.ts (2)
admin-ui/plugins/admin/components/CustomScripts/hooks/index.ts (1)
useMutationEffects(15-15)admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js (1)
errorMessage(100-100)
admin-ui/plugins/admin/components/CustomScripts/types/forms.ts (2)
admin-ui/plugins/admin/components/CustomScripts/types/customScript.ts (1)
CustomScriptItem(27-47)admin-ui/plugins/admin/redux/features/types/customScript.ts (1)
CustomScriptItem(29-46)
🔇 Additional comments (24)
admin-ui/app/context/theme/config.ts (1)
8-47: LGTM! ConsistentborderColoraddition across all themes.The new
borderColorproperty is correctly added to all four theme configurations with the same value (customColors.lightGray). SincegetThemeColorreturnstypeof themeConfig[keyof typeof themeConfig], the return type automatically includesborderColor, enabling type-safe access without casting in consumers likeCustomScriptListPage.tsx.admin-ui/plugins/admin/components/CustomScripts/hooks/useMutationEffects.ts (1)
34-45: Error handling logic is improved; same re-render concern applies.The error message extraction now properly handles
Errorinstances, objects with amessageproperty, and falls back to the translated error message. This addresses the previous review feedback well.The same potential duplicate-trigger concern from the success effect applies here. Consider applying the same ref-based guard pattern.
admin-ui/plugins/admin/components/CustomScripts/PersonAuthenticationFields.tsx (1)
11-23: Well-structured props interface and component setup.The component correctly encapsulates person authentication-specific fields, properly typed with
FormikProps<FormValues>. The extraction ofgetModulePropertyas a prop allows flexibility in how module properties are accessed.admin-ui/plugins/admin/components/CustomScripts/CustomScriptAddPage.tsx (2)
16-22: Clean mutation integration with reusable effects hook.The
useMutationEffectshook cleanly separates the success/error side effects from the page component. The mutation is properly connected with appropriate translation keys for success and error messages.
30-41: Based on my verification, I can now provide the rewritten review comment:
Type assertion bypasses TypeScript validation, but form validation mitigates the risk.
The assertion
scriptData as CustomScriptbypasses TypeScript's structural checks. However, this is less critical than initially suggested because the form validation schema (Yup) enforces that required fields (name,scriptType,programmingLanguage,level) are present before submission reaches this point.The destructuring removes only UI-specific fields (
action_message,script_path,location_type), not core required fields. The real concern is type safety rather than runtime integrity—CustomScriptItem(local type) has all optional properties, whileCustomScript(from JansConfigApi) likely expects required fields.Consider strengthening the type relationship instead of adding redundant runtime checks:
- await createMutation.mutateAsync({ - data: scriptData as CustomScript, - actionMessage: action_message, - }) + // Type is ensured by form validation; consider updating CustomScriptItem + // interface to reflect required fields or using a type guard at form level + await createMutation.mutateAsync({ + data: scriptData as CustomScript, + actionMessage: action_message, + })Alternatively, define an intermediate type that explicitly reflects what CustomScript requires:
type ValidatedCustomScript = Required<Pick<CustomScript, 'name' | 'scriptType' | 'programmingLanguage' | 'level'>> & Omit<CustomScript, 'name' | 'scriptType' | 'programmingLanguage' | 'level'>admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (6)
79-97: Permission handling is properly structured.Direct constant assignment for
scriptsResourceIdandscriptScopesaddresses the prior review feedback. TheuseMemohooks for permission checks are appropriate since they depend on hook functions. TheauthorizeHelpereffect correctly runs when dependencies change.
147-166: Delete confirmation flow handles success/error appropriately.The delete flow correctly:
- Guards against missing
inum- Closes the modal and clears state on success
- Extracts error messages with proper fallback
- Uses local toast dispatch (appropriate since modal state needs updating on success)
308-371: Table actions properly gated by permissions.Actions are conditionally added based on
canWrite,canRead, andcanDeletepermissions. The type assertionsrowData as CustomScriptare necessary due to MaterialTable's union type (T | T[]) for row data in action handlers, and are safe here since selection is disabled.
399-423: Good error state handling with recovery option.The error UI properly extracts error messages and provides a retry button for user recovery. The error message extraction pattern is consistent with other error handling in this PR.
521-542: MaterialTable integration is correct.The table setup properly configures:
- Loading state combining query and delete mutation pending states
- Custom container removing elevation
- Detail panel for expandable row details
- Localization for empty state message
543-561: Pagination correctly configured with theme-aware styling.The
borderColoraccess (themeColors.borderColor ?? '#e0e0e0') is now clean after the theme config changes. Pagination state is properly connected with callbacks for page/rows-per-page changes.admin-ui/plugins/admin/components/CustomScripts/types/forms.ts (2)
5-6: Type inconsistency betweenitemandhandleSubmitparameter.The
itemprop acceptsCustomScript, buthandleSubmitexpects{ customScript: CustomScriptItem }. If aCustomScriptobject has properties not present inCustomScriptItem(or vice versa), the form may submit incomplete or mistyped data.Consider aligning the types:
export interface CustomScriptFormProps { item: CustomScriptItem | CustomScript - handleSubmit: (data: { customScript: CustomScriptItem }) => void | Promise<void> + handleSubmit: (data: { customScript: CustomScriptItem | CustomScript }) => void | Promise<void> viewOnly?: boolean }Alternatively, ensure the form internally normalizes
CustomScripttoCustomScriptItembefore invokinghandleSubmit.
10-12: LGTM!The
rowtype update toCustomScriptItem | CustomScriptaligns with theitemprop change and supports the new data flow from API hooks.admin-ui/plugins/admin/components/CustomScripts/CustomScriptEditPage.tsx (5)
1-12: LGTM!Clean import refactoring that removes Redux dependencies in favor of the new hook-based architecture. The imports are well-organized and include proper type imports.
22-30: LGTM!Good use of the custom hooks. The
useMutationEffectshook provides standardized toast notifications and navigation, keeping the component clean.
67-84: LGTM!Defensive error handling with proper fallback messages. The error message extraction handles various error shapes appropriately.
86-94: LGTM!Clean render structure with the loader appropriately tied to the mutation's pending state. The form correctly receives the fetched script data.
46-49: I'll verify the safety of the type cast in the custom script handling code. Let me start by exploring the type definitions involved.
<function_calls>
#!/bin/bashFind CustomScript and CustomScriptItem type definitions
rg -n "export (interface|type) CustomScript" --type ts -C 5
</function_calls>Awaiting results to examine the type definitions...
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (6)
1-24: LGTM!Well-organized imports covering React Query, Redux integration, API hooks, and audit utilities. The import structure supports the hook-based architecture cleanly.
35-48: LGTM!The
useWebhookTriggerhook properly memoizes the callback withuseCallbackand includesdispatchin the dependency array.
50-86: LGTM!Well-structured data fetching hooks with proper auth-token gating and cache configuration. The
enabledconditions prevent unnecessary API calls when required parameters are missing.
158-168: LGTM!Safe
queryKeyhandling with proper type guards before callingstartsWith. This addresses the previous review concern about unsafe casting.
183-218: LGTM!Well-implemented
selectfunction with defensive handling for edge cases (null data, empty strings, underscore-separated names). The longer stale time is appropriate for script types which change infrequently.
226-237: The concern about Rules of Hooks violations is not applicable—useCustomScriptOperationsis unused dead code.Investigation reveals that:
useCustomScriptOperationsis never called anywhere in the codebase- No component imports or invokes this hook
- All components import individual hooks directly (
useCustomScriptsByType,useDeleteCustomScript, etc.) instead- The returned hook functions are never executed, so there is no possibility of conditional or loop-based invocation
While returning hooks from hooks can theoretically violate React's Rules of Hooks if misused, this code path is not taken in practice. The hook is exported but unused, making it dead code rather than a functional concern.
Consider removing
useCustomScriptOperationsentirely, or document its purpose if it's intended for future use or as a convenience API for external consumers.
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts
Outdated
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts
Outdated
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/hooks/useMutationEffects.ts
Outdated
Show resolved
Hide resolved
syntrydy
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (1)
143-180: Variable shadowing; query invalidation looks good.Line 145: Same variable shadowing issue — rename to
webhookTriggerfor consistency.The predicate-based query invalidation (lines 157-167) correctly uses type guards to safely handle
queryKey, addressing previous unsafe casting concerns.Apply this diff:
export function useDeleteCustomScript() { const queryClient = useQueryClient() - const triggerWebhook = useWebhookTrigger() + const webhookTrigger = useWebhookTrigger() const baseMutation = useDeleteConfigScriptsByInum() return { ...baseMutation, mutateAsync: async (variables: { inum: string; actionMessage?: string }) => { const { actionMessage, ...baseVariables } = variables const result = await baseMutation.mutateAsync(baseVariables) await Promise.all([ queryClient.invalidateQueries({ queryKey: getGetConfigScriptsQueryKey() }), queryClient.invalidateQueries({ predicate: (query) => { const queryKey = query.queryKey if (!Array.isArray(queryKey) || typeof queryKey[0] !== 'string') { return false } const key = queryKey[0] return ( key.startsWith('/api/v1/config/scripts/type/') || key.startsWith('/api/v1/config/scripts/') ) }, }), ]) - triggerWebhook({ createdFeatureValue: { inum: variables.inum } }) + webhookTrigger({ createdFeatureValue: { inum: variables.inum } }) await logAuditAction(DELETION, SCRIPT, { action: { action_data: { inum: variables.inum } }, message: actionMessage, }) return result }, } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/hooks/useMutationEffects.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
admin-ui/plugins/admin/components/CustomScripts/hooks/useMutationEffects.ts (1)
admin-ui/plugins/admin/components/CustomScripts/hooks/index.ts (1)
useMutationEffects(15-15)
⏰ 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)
- GitHub Check: label PR
🔇 Additional comments (5)
admin-ui/plugins/admin/components/CustomScripts/hooks/useMutationEffects.ts (1)
1-64: LGTM! Mutation effects implementation is solid.The hook correctly addresses previous concerns:
- Uses persistent refs (
successHandledRef,errorHandledRef) to prevent duplicate side effects during re-renders- Properly resets handled flags when mutation returns to idle state
- Enhanced error type checking handles
Errorinstances, objects withmessageproperties, and falls back to translated error messages- Dependencies are correctly specified in both effects
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (4)
1-32: LGTM! Imports and type definitions are well-organized.The
ScriptTypeis correctly imported from the types file rather than redefined, addressing the previous duplicate type concern.
34-47: LGTM! Webhook trigger with proper error handling.The utility hook correctly wraps webhook dispatch in error handling to prevent failures from disrupting mutation flows.
49-85: LGTM! Query hooks are properly configured.All three fetch hooks correctly:
- Gate queries on auth token presence
- Use consistent cache configuration from
SCRIPT_CACHE_CONFIG- Comply with React Query v5 (no deprecated callbacks)
182-217: LGTM! Script types hook with safe data transformation.The
useCustomScriptTypeshook correctly:
- Gates on auth token
- Safely handles empty/invalid data
- Transforms script type identifiers to user-friendly names
- Uses appropriate cache configuration
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (4)
admin-ui/plugins/admin/redux/sagas/WebhookSaga.ts (1)
triggerWebhook(279-368)admin-ui/plugins/admin/components/CustomScripts/constants.ts (1)
SCRIPT_CACHE_CONFIG(3-9)admin-ui/plugins/admin/components/CustomScripts/hooks/auditUtils.ts (1)
logAuditAction(28-47)admin-ui/plugins/admin/components/CustomScripts/types/customScript.ts (1)
ScriptType(22-25)
🔇 Additional comments (1)
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (1)
1-232: Excellent refactoring—all previous critical issues addressed! ✅The implementation now follows React Query v5 best practices and all major concerns from previous reviews have been resolved:
- ✅ Removed deprecated
onSuccess/onErrorcallbacks- ✅ Fixed variable shadowing (
webhookTriggerinstead oftriggerWebhook)- ✅ Replaced brittle
JSON.parse(JSON.stringify())withstructuredClone()- ✅ Added proper type guards for
queryKeyaccess (lines 158-161)- ✅ Using canonical query key getters (
getGetConfigScriptsByInumQueryKey)- ✅ Eliminated duplicate
ScriptTypedefinition by importing from types- ✅ Removed hook references from
useCustomScriptOperationsreturn (Rules of Hooks compliant)The hooks are well-structured with:
- Proper auth gating on all queries
- Appropriate cache configuration (staleTime/gcTime)
- Comprehensive cache invalidation strategies
- Webhook triggering with error handling
- Audit logging for all mutations
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
- Can we remove the horizontal scroll.
https://www.loom.com/share/872fe718d6414024b911147d10ccadbc - Heading are not visible
- Selected rows fields are not visible
4. Texts in the table footer are not visible:
5. I have enabled `Twillo_sms` script but changes are not reflected on Scripts list page. Please make sure all the changed fields should be correctly reflected on list page after saving.
- I have added a webhook on add/edit of script. The webhooks dialog opened after editing a script has disabled
acceptbutton.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (1)
435-457: TextField adornments still using deprecatedInputPropsAPI (optional).MUI v5 favors
slotProps.inputoverInputPropsfor start/end adornments. This is already noted in an earlier review; consider migrating when convenient to stay aligned with the newer API surface.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx(1 hunks)admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts(1 hunks)
⏰ 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)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (1)
230-243: AggregateduseCustomScriptOperationssurface looks good and focused.The operations hook cleanly bundles the three mutations plus script type data/loading flags without re‑exposing any hook references, which avoids Rules‑of‑Hooks pitfalls and keeps the API small and purposeful.
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (1)
452-474: PreferslotProps.inputoverInputPropson TextField (duplicate of prior nit).MUI v5 has been moving away from
InputPropsonTextFieldin favor ofslotProps.input. For forward compatibility, consider migrating the adornments:- <TextField + <TextField size="small" label={t('fields.search')} placeholder={t('placeholders.search_scripts')} value={pattern} onChange={handlePatternChange} onKeyDown={handlePatternKeyDown} - InputProps={{ - startAdornment: ( - <InputAdornment position="start"> - <SearchIcon fontSize="small" /> - </InputAdornment> - ), - endAdornment: pattern && ( - <InputAdornment position="end"> - <IconButton size="small" onClick={handleSearchClear} edge="end"> - <ClearIcon fontSize="small" /> - </IconButton> - </InputAdornment> - ), - }} + slotProps={{ + input: { + startAdornment: ( + <InputAdornment position="start"> + <SearchIcon fontSize="small" /> + </InputAdornment> + ), + endAdornment: pattern && ( + <InputAdornment position="end"> + <IconButton size="small" onClick={handleSearchClear} edge="end"> + <ClearIcon fontSize="small" /> + </IconButton> + </InputAdornment> + ), + }, + }} sx={{ minWidth: 300 }} />This keeps behavior identical while aligning with the newer MUI API.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx(1 hunks)
🔇 Additional comments (4)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (4)
78-97: Permissions and query wiring look consistent with objectives.Using
ADMIN_UI_RESOURCES.ScriptswithCEDAR_RESOURCE_SCOPES,hasCedar*Permission, andauthorizeHelperplusGluuViewWrappergives clear, centralized access control for the page, anduseCustomScriptsByTypeis parameterized cleanly withlimit,startIndex,pattern,sortBy, andsortOrder. This matches the move away from Redux toward typed hooks and looks solid.Also applies to: 99-118, 449-449
141-165: Ensure delete mutation also refreshes the list.
handleDeleteConfirmshows a toast and closes the dialog on success, but the list is only refreshed when the user explicitly clicks the refresh icon. IfuseDeleteCustomScriptdoes not already invalidate the relevant query, the deleted script may remain visible until a manual refresh.Please confirm that
useDeleteCustomScriptis invalidating the same query key used byuseCustomScriptsByType(or callrefetchon success here) so the table always reflects the current backend state after a delete.Also applies to: 590-600
176-200: ConfirmsortOrderstring values match the API contract.You’re sending
sortOrderas'ascending' | 'descending'whensortByis set. This is fine as long as the generated client / backend expects those exact string values (not e.g.'asc' | 'desc').Please double‑check the generated TypeScript client types or API docs for the sort order enum and adjust the toggling logic if necessary.
Also applies to: 104-110
211-324: Table configuration, theming, and detail panel integration look well-structured.The column definitions, action wiring, rowStyle highlighting for errored/disabled scripts, theme‑aware header and pagination styling, and the
detailPanelintegration withCustomScriptDetailPageare all cohesive and readable. This is a nice, idiomatic use of@material-table/coreand MUI theming in this context.Also applies to: 391-417, 541-562, 563-586
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (1)
453-475: Migrate TextField adornments fromInputPropstoslotProps.input(MUI v5+)The search
TextFieldstill usesInputPropsfor adornments, which MUI v5 is steering away from in favor ofslotProps.input. This is the same concern raised in a prior review on this file.You can future‑proof this by shifting the adornments into
slotProps.input:- <TextField + <TextField size="small" label={t('fields.search')} placeholder={t('placeholders.search_scripts')} value={pattern} onChange={handlePatternChange} onKeyDown={handlePatternKeyDown} - InputProps={{ - startAdornment: ( - <InputAdornment position="start"> - <SearchIcon fontSize="small" /> - </InputAdornment> - ), - endAdornment: pattern && ( - <InputAdornment position="end"> - <IconButton size="small" onClick={handleSearchClear} edge="end"> - <ClearIcon fontSize="small" /> - </IconButton> - </InputAdornment> - ), - }} + slotProps={{ + input: { + startAdornment: ( + <InputAdornment position="start"> + <SearchIcon fontSize="small" /> + </InputAdornment> + ), + endAdornment: pattern && ( + <InputAdornment position="end"> + <IconButton size="small" onClick={handleSearchClear} edge="end"> + <ClearIcon fontSize="small" /> + </IconButton> + </InputAdornment> + ), + }, + }} sx={{ minWidth: 300 }} />If MUI’s typings complain, you may need to slightly adjust the
slotProps.inputtype (e.g., with a narrow cast) based on the installed@mui/materialversion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (6)
admin-ui/plugins/admin/components/CustomScripts/types/customScript.ts (1)
ScriptError(18-20)admin-ui/app/utils/pagingUtils.ts (1)
getPagingSize(1-24)admin-ui/plugins/admin/components/CustomScripts/constants.ts (1)
DEFAULT_SCRIPT_TYPE(1-1)admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (3)
useCustomScriptsByType(61-74)useCustomScriptTypes(193-228)useDeleteCustomScript(154-191)admin-ui/plugins/admin/components/CustomScripts/hooks/index.ts (3)
useCustomScriptsByType(3-3)useCustomScriptTypes(8-8)useDeleteCustomScript(7-7)admin-ui/app/customColors.ts (1)
customColors(4-25)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx
Outdated
Show resolved
Hide resolved
| @@ -1,11 +1,11 @@ | |||
| // @ts-nocheck | |||
There was a problem hiding this comment.
we shall remove this, otherwise compiler wont be able to check the types of this file
| @@ -1,6 +1,6 @@ | |||
| // @ts-nocheck | |||
There was a problem hiding this comment.
we shall remove this, otherwise compiler wont be able to check the types of this file
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
admin-ui/app/utils/Util.ts (1)
86-96: Consider trimming and conditionally includingdescriptionRight now
descriptionis forwarded as‑is and the property is always present (possiblyundefined), while the type marks it as optional. For consistency withkey/valueand to better reflect the optional contract, you can trim and only include it when defined/non‑empty.-export const mapPropertyToKeyValue = (prop: { - key?: string - value?: string - value1?: string - value2?: string - description?: string -}): { key: string; value: string; description?: string } => { - const key = (prop.key ?? prop.value1 ?? '').trim() - const value = (prop.value ?? prop.value2 ?? '').trim() - return { key, value, description: prop.description } -} +export const mapPropertyToKeyValue = (prop: { + key?: string + value?: string + value1?: string + value2?: string + description?: string +}): { key: string; value: string; description?: string } => { + const key = (prop.key ?? prop.value1 ?? '').trim() + const value = (prop.value ?? prop.value2 ?? '').trim() + const description = prop.description?.trim() + + return description !== undefined && description !== '' + ? { key, value, description } + : { key, value } +}admin-ui/plugins/admin/components/CustomScripts/CustomScriptForm.tsx (3)
61-75: Verify property normalization for Custom/Module properties (likely linked to “empty custom properties” bug)
transformPropertyForApinormalizes both{ key, value }and{ value1, value2 }into{ value1, value2, hide, description }, whileconfigurationPropertiesOptions/modulePropertiesOptionsderive options fromformik.values.*(produced bytransformToFormValues) and feed them intoGluuProperties. IftransformToFormValuesdoes not perform the inverse normalization (i.e., consistently mapping bothkey/valueandvalue1/value2from the TS client into the shape expected bymapPropertyToKeyValue/GluuProperties), some properties can show with empty keys/values or not show at all, which matches the QA report.Please double-check that
transformToFormValuesis symmetric withtransformPropertyForApiforConfigurationPropertyandModuleProperty, so legacy data using either field pair renders correctly in the form. I can help sketch a symmetric mapping if it’s not already there.Also applies to: 263-269, 505-522
219-227: Clearingscript_pathleaves stalelocationPath/location_pathmodule propertyWhen the user clears the script path input,
scriptPathChangeearly-returns on a falsy value and never clearslocationPathor the'location_path'entry inmoduleProperties, so those stay at the last non-empty value whilescript_pathbecomes''. Even if validation typically prevents saving an empty path forfilelocation type, this leaves inconsistent form state and can be surprising.You can fix this by explicitly clearing
locationPathand removing the module property whenvalueis empty:const scriptPathChange = useCallback( (value: string): void => { - if (!value) return - formik.setFieldValue('locationPath', value) - removeModuleProperty('location_path') - updateModuleProperty('location_path', value) + if (!value) { + formik.setFieldValue('locationPath', undefined) + removeModuleProperty('location_path') + return + } + + formik.setFieldValue('locationPath', value) + removeModuleProperty('location_path') + updateModuleProperty('location_path', value) }, [formik.setFieldValue, removeModuleProperty, updateModuleProperty], )Also applies to: 471-481
564-576:disableApplylogic prevents users from submitting Add Script formOn the Add Script page, multiple required fields start empty (
name,scriptType,script_pathwhenlocation_typedefaults to'file'), triggering validation failures on mount. The conditiondisableApply={!formik.isValid || !formik.dirty}keeps the Apply button permanently disabled until all validation errors clear, even though inline validation errors are displayed for each field.Two improvements:
Add
location_typeto the validation schema. The field is currently used in conditional.when()checks (lines 16, 20 invalidations.ts) but is not defined inYup.object(), which can cause inconsistent validation tracking.Consider loosening the Apply button gate. If you want to allow submission and let the API reject invalid data, or to rely on inline error displays rather than blocking the button:
- disableApply={!formik.isValid || !formik.dirty} + disableApply={!formik.dirty}This removes the isValid check and still prevents no-op submissions (when nothing has changed). Validation errors will still display inline, and the API can provide feedback on any remaining issues.
♻️ Duplicate comments (1)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (1)
424-446: (Repeat nit) Consider migrating TextField adornments toslotProps.inputMUI v5 is steering toward
slotProps.inputinstead ofInputPropsonTextField. Refactoring the search field adornments intoslotProps={{ input: { startAdornment, endAdornment } }}would align this with the newer API and improve forward compatibility, but it’s purely optional and cosmetic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
admin-ui/app/routes/Apps/Gluu/GluuPropertyItem.tsx(2 hunks)admin-ui/app/utils/Util.ts(1 hunks)admin-ui/plugins/admin/components/CustomScripts/CustomScriptForm.tsx(11 hunks)admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/CustomScripts/CustomScriptListPage.tsx
🧬 Code graph analysis (2)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptForm.tsx (7)
admin-ui/plugins/admin/components/CustomScripts/types/forms.ts (1)
CustomScriptFormProps(4-8)admin-ui/app/helpers/navigation.ts (1)
useAppNavigation(66-95)admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (1)
useCustomScriptTypes(193-228)admin-ui/app/customColors.ts (1)
customColors(4-25)admin-ui/plugins/admin/components/CustomScripts/PersonAuthenticationFields.tsx (1)
PersonAuthenticationFields(18-82)admin-ui/plugins/admin/components/CustomScripts/helper/utils.ts (1)
getModuleProperty(6-13)admin-ui/plugins/admin/components/CustomScripts/constants.ts (2)
PROGRAMMING_LANGUAGES(17-20)LOCATION_TYPE_OPTIONS(22-25)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (5)
admin-ui/plugins/admin/components/CustomScripts/types/customScript.ts (1)
ScriptError(18-20)admin-ui/app/utils/pagingUtils.ts (1)
getPagingSize(1-24)admin-ui/plugins/admin/components/CustomScripts/constants.ts (1)
DEFAULT_SCRIPT_TYPE(1-1)admin-ui/plugins/admin/components/CustomScripts/hooks/index.ts (3)
useCustomScriptsByType(3-3)useCustomScriptTypes(8-8)useDeleteCustomScript(7-7)admin-ui/app/customColors.ts (1)
customColors(4-25)
🔇 Additional comments (6)
admin-ui/app/routes/Apps/Gluu/GluuPropertyItem.tsx (2)
78-78: LGTM! Layout refinement improves alignment.The conditional margin application correctly aligns the remove button with the input fields when labels are present, improving the visual layout consistency.
48-49: Remove the unnecessaryproperty.descriptioncheck or properly add it to the Property type definition.The placeholder logic accesses
property.description, but this field does not exist in the Property type definitions used by GluuProperties. ThePropertytype only includeskey/value(orsource/destinationfor multi-properties), soproperty.descriptionis alwaysundefinedand the fallback tovaluePlaceholderis used regardless.Either:
- Remove the
property.descriptioncheck entirely if it's not needed- Or add
description?: stringto the Property type definitions if description should be supported across all property objects (particularly for configurationProperties which lack this field in the Script types)admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (2)
140-160: Delete flow and error handling look solidThe delete confirmation handler correctly guards on
itemToDelete.inum, awaits the mutation, shows success/error toasts, and reliably closes/clears dialog state. The addedconsole.errorpreserves full error context without changing UX, which is useful for debugging multi-step failures. Based on learnings, keeping this logging here is appropriate.
513-549: Detail panel and delete dialog state handling look correctThe detail panel now passes the row object directly into
CustomScriptDetailPage, matching the expected row shape, and the delete dialog’s close handler clears bothmodalanditemToDelete, avoiding stale selection and ensuring clean unmounting when closed. No issues spotted here.admin-ui/plugins/admin/components/CustomScripts/CustomScriptForm.tsx (2)
12-15: Error alert + modal wiring looks correctThe conditional rendering of the error Alert and the lazy-loaded error modal is coherent: the Alert only shows when a stack trace exists, and the “show error” action cleanly opens the modal via
showErrorModal. No issues spotted here.Also applies to: 276-305
27-32: Typed script types + centralized select options are a solid improvementUsing
useCustomScriptTypeswith anisEditModeguard forscriptType, and driving language/location selects fromPROGRAMMING_LANGUAGESandLOCATION_TYPE_OPTIONS, together with thePersonAuthenticationFieldsconditional onscriptTypeState, makes this form more consistent and maintainable. This aligns well with the PR goal of moving to TS-generated types.Also applies to: 45-48, 271-273, 354-361, 363-400, 402-460, 421-427, 452-456
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (1)
424-446: MigrateTextFieldadornments toslotPropsfor MUI v5+ forward compatibilityThe search
TextFieldstill usesInputPropsforstartAdornment/endAdornment. In MUI v5.15.x this works, but the library is nudging towardsslotProps.inputfor long‑term support.You can switch to:
- <TextField + <TextField size="small" label={t('fields.search')} placeholder={t('placeholders.search_scripts')} value={pattern} onChange={handlePatternChange} onKeyDown={handlePatternKeyDown} - InputProps={{ - startAdornment: ( - <InputAdornment position="start"> - <SearchIcon fontSize="small" /> - </InputAdornment> - ), - endAdornment: pattern && ( - <InputAdornment position="end"> - <IconButton size="small" onClick={handleSearchClear} edge="end"> - <ClearIcon fontSize="small" /> - </IconButton> - </InputAdornment> - ), - }} + slotProps={{ + input: { + startAdornment: ( + <InputAdornment position="start"> + <SearchIcon fontSize="small" /> + </InputAdornment> + ), + endAdornment: pattern && ( + <InputAdornment position="end"> + <IconButton size="small" onClick={handleSearchClear} edge="end"> + <ClearIcon fontSize="small" /> + </IconButton> + </InputAdornment> + ), + }, + }} sx={{ minWidth: 300 }} />This keeps behavior identical while aligning with MUI’s newer API.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/CustomScripts/CustomScriptListPage.tsx
🧬 Code graph analysis (1)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (6)
admin-ui/plugins/admin/components/CustomScripts/types/customScript.ts (1)
ScriptError(18-20)admin-ui/app/utils/pagingUtils.ts (1)
getPagingSize(1-24)admin-ui/plugins/admin/components/CustomScripts/constants.ts (1)
DEFAULT_SCRIPT_TYPE(1-1)admin-ui/plugins/user-management/types/CommonTypes.ts (1)
ThemeContext(13-17)admin-ui/plugins/admin/components/CustomScripts/hooks/useCustomScriptApi.ts (3)
useCustomScriptsByType(61-74)useCustomScriptTypes(193-228)useDeleteCustomScript(154-191)admin-ui/app/customColors.ts (1)
customColors(4-25)
🔇 Additional comments (2)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (2)
196-283: Columns rely onscriptErrorpresence; ensure upstream mapping is consistentThe
ScriptTableRowinterface adds an optionalscriptError?: ScriptError, androwStyle/name column conditionally render based onscriptError?.stackTrace. For scripts where custom properties and errors come from the backend, you’ll want to ensure the list API (or any transformation layer) always setsscriptErrorconsistently (e.g.,undefinedvs missing vs empty object).If some scripts don’t surface custom error info while others do, double‑check the mapping logic in the API layer (or query
selectoption) to confirm that:
scriptErroris populated when the backend returns error metadata.- It’s omitted/undefined when there truly is no error.
This may help with the “Custom Properties not appearing” investigation if the same mapping is responsible for both properties and error state.
389-416: Error UI still renders underGluuViewWrapper; confirm behavior for unauthorized usersThe error branch wraps the error card in
<GluuViewWrapper canShow={canRead}>and returns it whenevererroris truthy. This means:
- If
canReadisfalsebut the hook still reports an error (e.g., auth failure), the outer React tree for the page still renders, andGluuViewWrapperis responsible for hiding the content.- The high‑level intent seems to be “no read permission → no page content”, while “read permission + fetch error → show error card”.
If
GluuViewWrapperfully hides children whencanShow={false}, this is fine. If it instead displays some generic “no permission” message, that message will share layout with the error card wrapper. Given prior review comments about aligning error UI with permission gating, it’s worth double‑checking at runtime that:
- A user without read permission does not see the generic “error loading scripts” card, and
- Authorized users with a failing API call do see the card as intended.
If there’s any ambiguity, an explicit
if (!canRead) return <GluuViewWrapper canShow={false}>before the error branch would make the behavior unambiguous.
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx
Show resolved
Hide resolved
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx (2)
431-444: Consider usingslotPropsinstead ofInputProps.The TextField still uses the older
InputPropsAPI. As noted in previous reviews, MUI v5 is transitioning toslotProps.inputfor better forward compatibility.
69-70: Redundant modal state increases coupling.The component maintains both
modalanditemToDeleteto control dialog visibility. As noted in previous reviews, the presence ofitemToDeletealone could drive the dialog's open state, eliminating the need for a separatemodalboolean.Also applies to: 536-548
Also applies to: 536-548
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
admin-ui/app/routes/Apps/Gluu/GluuProperties.tsx(3 hunks)admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/CustomScripts/CustomScriptListPage.tsx
admin-ui/plugins/admin/components/CustomScripts/CustomScriptListPage.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/app/routes/Apps/Gluu/GluuProperties.tsx(2 hunks)
⏰ 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)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
admin-ui/app/routes/Apps/Gluu/GluuProperties.tsx (2)
68-80: Refactoring successfully eliminates duplication.The call to
syncFormikPropertiescorrectly replaces the inline Formik synchronization logic, as suggested in the previous review. The pattern is consistent across all three handlers (addProperty,changeProperty,removeProperty).However, verify that the type safety issues flagged in the helper function don't impact the user-reported bugs.
15-34: The identified type assertion issue does not cause the reported user bugs; the data transformation flow is functionally correct despite the type safety concern.While line 23 of
syncFormikPropertiescontains an unsafe cast of properties toKeyValuePropertywithout validation, this code path is never executed in current usage. The component defaults (isKeys=true,multiProperties=false) cause the function to execute lines 26-30 instead, which correctly transforms the data to API format{value1, value2}.The complete data flow works correctly:
- API provides:
{value1, value2, hide?, description?}mapPropertyToKeyValueconverts to:{key, value, description?}syncFormikPropertiestransforms back to:{value1, value2}(via lines 26-30)transformPropertyForApion submit handles both formats properlyThe user-reported bugs likely have different root causes.
configurationPropertiesis notably absent from the Formik validation schema (unlikemoduleProperties), which could contribute to form behavior issues, but not through the type assertion mechanism identified in the review.The code could be improved by:
- Adding validation schema for
configurationProperties- Explicitly passing
isKeysandmultiPropertiesprops to document intent- Adding type guards to prevent unsafe casts
However, the current implementation functions correctly for its intended use case.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
admin-ui/app/routes/Apps/Gluu/GluuProperties.tsx (1)
71-73: Duplicate: Missing Formik sync already flagged.This useEffect only updates local state but doesn't sync to Formik. The issue and solution are already covered in the previous comment on lines 21-41.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/app/routes/Apps/Gluu/GluuProperties.tsx(2 hunks)
🔇 Additional comments (1)
admin-ui/app/routes/Apps/Gluu/GluuProperties.tsx (1)
15-20: LGTM: Type guards correctly implemented.The type guard functions use proper TypeScript patterns with the
inoperator and type predicates.
|
) * fix(admin-ui): Use typescript generated client for Webhooks pages #2433 * fix(admin-ui): Code review #2433 * fix(admin-ui): Code review #2433 * fix(admin-ui): Code review #2433 * fix(admin-ui): Code review #2433 * fix(admin-ui): Code review #2433 * fix(admin-ui): Code review #2433 * fix(admin-ui): Code review #2433 * fix(admin-ui): code review #2478 * fix(admin-ui): code review #2433 * fix(admin-ui): code review #2433 * fix(admin-ui): code review #2433 * fix(admin-ui): code review #2433 * fix(admin-ui): code review #2433 * fix(admin-ui): code review #2433 * fix(admin-ui): code review #2433 * fix(admin-ui): code review #2433
4 participants
#2464
Closes #2464
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.