chore(admin-ui): removal two more redundant UI dependencies (#2878)#2879
Merged
Conversation
…ion + single-source action catalog (#2872) Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
…2874) Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR removes the ChangesAdmin UI dependency removal and control migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
mo-auto
added
comp-admin-ui
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
admin-ui/plugins/auth-server/components/OidcClients/components/ClientTokensPanel.tsx (1)
272-287:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPersist the toggle’s boolean state, not the checkbox value string.
Line 285 stores
e.target.value, butGluuToggleRowis backed by a checkbox input. That yields the string value ("on"), not the checked state, soRUN_INTROSPECTION_SCRIPT_BEFORE_JWT_CREATIONstops matching the boolean contract used byvalue={Boolean(...)}and by the downstream payload.Suggested fix
handler={(e) => { setModifiedFields((prev) => ({ ...prev, [CLIENT_TOKEN_MODIFIED_FIELDS.RUN_INTROSPECTION_SCRIPT_BEFORE_JWT_CREATION]: - e.target.value, + e.target.checked, })) }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@admin-ui/plugins/auth-server/components/OidcClients/components/ClientTokensPanel.tsx` around lines 272 - 287, The toggle handler for GluuToggleRow is storing e.target.value ("on") instead of the checkbox boolean, breaking boolean contracts; update the handler passed to GluuToggleRow to setModifiedFields with the checkbox state (use e.target.checked or Boolean(e.target.checked)) for CLIENT_TOKEN_MODIFIED_FIELDS.RUN_INTROSPECTION_SCRIPT_BEFORE_JWT_CREATION so the stored value matches the value prop derived from formik and the downstream payload expectations (also ensure formik is updated consistently if needed).admin-ui/app/routes/Apps/Gluu/GluuAutocomplete.tsx (1)
277-291:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse exact-match duplicate checks for Enter-created values.
The Enter path rejects any input that is merely contained in an existing label because it uses
includes(...). That diverges fromfilterOptions(...), which uses exact equality, so keyboard submit can fail for a value that the dropdown correctly offers as a new selection.Suggested fix
if ( trimmed && - !optionValues.some((o) => - getDisplayLabel(o).toLowerCase().includes(trimmed.toLowerCase()), - ) && + !optionValues.some( + (o) => getDisplayLabel(o).toLowerCase() === trimmed.toLowerCase(), + ) && !selectedItems.some( (s) => getDisplayLabel(s).toLowerCase() === trimmed.toLowerCase(), ) ) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@admin-ui/app/routes/Apps/Gluu/GluuAutocomplete.tsx` around lines 277 - 291, The Enter key path in the onKeyDown handler for allowCustom currently rejects inputs when the trimmed value is merely contained in an existing label because it uses getDisplayLabel(o).toLowerCase().includes(trimmed.toLowerCase()); change this to an exact case-insensitive equality check (compare getDisplayLabel(o).toLowerCase() === trimmed.toLowerCase()) and similarly for the selectedItems check so the Enter-created value uses the same exact-match logic as filterOptions; keep using inputValue.trim(), then call onChange([...selectedItems, trimmed]) and setInputValue('') as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@admin-ui/app/constants/site.ts`:
- Line 1: The SITE_DESCRIPTION constant currently uses "admin Ui" with incorrect
capitalization; update the string value of SITE_DESCRIPTION to use "admin UI"
(i.e., change 'Jans-server admin Ui' to 'Jans-server admin UI') so the meta
description displays the correct capitalization.
In `@admin-ui/plugins/auth-server/components/Authentication/Acrs/AcrsForm.tsx`:
- Around line 361-399: The LDAP autocomplete inputs (GluuAutocomplete for
name="servers" and name="baseDNs") currently only call formik.setFieldValue so
their touched flags never get set, preventing errors and disabling Apply; update
the onChange handlers to also mark the field touched (e.g., call
formik.setFieldTouched('servers', true) and formik.setFieldTouched('baseDNs',
true) or use formik.setTouched with the appropriate key) immediately after
calling formik.setFieldValue so formik.touched.servers and
formik.touched.baseDNs become true and validation/errors display.
In
`@admin-ui/plugins/auth-server/components/Authentication/types/authenticationTypes.ts`:
- Around line 88-94: AcrsFormValues.configurationProperties is redeclaring an
inline shape that diverges from the shared ConfigurationProperty type; replace
the inline Array<{id?: string; key?: string; value?: string; value1?: string;
value2?: string}> with the shared ConfigurationProperty[] type (import
ConfigurationProperty if needed), ensure the shared property includes the
optional hide field used elsewhere, and update usages in AcrsForm,
getPropertiesConfig, and transformConfigurationProperties to rely on the unified
ConfigurationProperty shape so the three consumers share one contract.
---
Outside diff comments:
In `@admin-ui/app/routes/Apps/Gluu/GluuAutocomplete.tsx`:
- Around line 277-291: The Enter key path in the onKeyDown handler for
allowCustom currently rejects inputs when the trimmed value is merely contained
in an existing label because it uses
getDisplayLabel(o).toLowerCase().includes(trimmed.toLowerCase()); change this to
an exact case-insensitive equality check (compare
getDisplayLabel(o).toLowerCase() === trimmed.toLowerCase()) and similarly for
the selectedItems check so the Enter-created value uses the same exact-match
logic as filterOptions; keep using inputValue.trim(), then call
onChange([...selectedItems, trimmed]) and setInputValue('') as before.
In
`@admin-ui/plugins/auth-server/components/OidcClients/components/ClientTokensPanel.tsx`:
- Around line 272-287: The toggle handler for GluuToggleRow is storing
e.target.value ("on") instead of the checkbox boolean, breaking boolean
contracts; update the handler passed to GluuToggleRow to setModifiedFields with
the checkbox state (use e.target.checked or Boolean(e.target.checked)) for
CLIENT_TOKEN_MODIFIED_FIELDS.RUN_INTROSPECTION_SCRIPT_BEFORE_JWT_CREATION so the
stored value matches the value prop derived from formik and the downstream
payload expectations (also ensure formik is updated consistently if needed).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8f28681c-2670-4f88-b70f-5544ba9a1fd0
📒 Files selected for processing (66)
admin-ui/app/components/GluuTable/GluuTable.tsxadmin-ui/app/components/GluuTable/constants.tsadmin-ui/app/components/Layout/Layout.tsxadmin-ui/app/components/icons/index.tsadmin-ui/app/config/site.tsadmin-ui/app/constants/index.tsadmin-ui/app/constants/site.tsadmin-ui/app/constants/ui.tsadmin-ui/app/layout/components/DefaultSidebar.tsxadmin-ui/app/layout/default.tsxadmin-ui/app/routes/Apps/Gluu/GluuAutocomplete.tsxadmin-ui/app/routes/Apps/Gluu/GluuBooleanSelectBox.tsxadmin-ui/app/routes/Apps/Gluu/GluuInlineInput.tsxadmin-ui/app/routes/Apps/Gluu/GluuPermissionModal.tsxadmin-ui/app/routes/Apps/Gluu/GluuSecretDetail.tsxadmin-ui/app/routes/Apps/Gluu/GluuSuspenseLoader.tsxadmin-ui/app/routes/Apps/Gluu/GluuToggle.tsxadmin-ui/app/routes/Apps/Gluu/GluuTypeAhead.tsxadmin-ui/app/routes/Apps/Gluu/GluuTypeAheadForDn.tsxadmin-ui/app/routes/Apps/Gluu/GluuTypeAheadWithAdd.tsxadmin-ui/app/routes/Apps/Gluu/Tests/GluuAutocomplete.test.tsxadmin-ui/app/routes/Apps/Gluu/Tests/GluuSecretDetail.test.tsxadmin-ui/app/routes/Apps/Gluu/Tests/GluuToggle.test.tsxadmin-ui/app/routes/Apps/Gluu/Tests/GluuToggleRow.test.tsxadmin-ui/app/routes/Apps/Gluu/Tests/GluuTypeAhead.test.tsxadmin-ui/app/routes/Apps/Gluu/Tests/GluuTypeAheadForDn.test.tsxadmin-ui/app/routes/Apps/Gluu/Tests/GluuTypeAheadWithAdd.test.tsxadmin-ui/app/routes/Apps/Gluu/styles/GluuPermissionModal.style.tsadmin-ui/app/routes/Apps/Gluu/styles/GluuSecretDetail.style.tsadmin-ui/app/routes/Apps/Gluu/styles/GluuToggle.style.tsadmin-ui/app/routes/Apps/Gluu/styles/GluuTypeAhead.style.tsadmin-ui/app/routes/Apps/Gluu/styles/GluuTypeAheadWithAdd.style.tsadmin-ui/app/routes/Apps/Gluu/types/GluuComponentPropsTypes.tsadmin-ui/app/routes/Apps/Gluu/types/GluuPermissionModal.types.tsadmin-ui/app/routes/Apps/Gluu/types/GluuToggle.types.tsadmin-ui/app/routes/Apps/Gluu/types/GluuTypeAhead.types.tsadmin-ui/app/routes/Apps/Gluu/types/GluuTypeAheadForDn.types.tsadmin-ui/app/routes/Apps/Gluu/types/GluuTypeAheadWithAdd.types.tsadmin-ui/app/routes/Apps/Gluu/types/index.tsadmin-ui/app/styles/plugins/_react-bootstrap-typeahead.scssadmin-ui/app/styles/plugins/_react-toggle.scssadmin-ui/app/styles/plugins/plugins.cssadmin-ui/app/styles/plugins/plugins.scssadmin-ui/app/utils/__tests__/zip.test.tsadmin-ui/app/utils/menuFilters.tsadmin-ui/app/utils/zip.tsadmin-ui/package.jsonadmin-ui/plugins/admin/components/Assets/AssetForm.tsxadmin-ui/plugins/admin/components/Webhook/WebhookForm.tsxadmin-ui/plugins/auth-server/components/Authentication/Acrs/Acrs.tsxadmin-ui/plugins/auth-server/components/Authentication/Acrs/AcrsEditPage.tsxadmin-ui/plugins/auth-server/components/Authentication/Acrs/AcrsForm.style.tsadmin-ui/plugins/auth-server/components/Authentication/Acrs/AcrsForm.tsxadmin-ui/plugins/auth-server/components/Authentication/AgamaFlows/AgamaFlows.tsxadmin-ui/plugins/auth-server/components/Authentication/Aliases/Aliases.tsxadmin-ui/plugins/auth-server/components/Authentication/Authentication.tsxadmin-ui/plugins/auth-server/components/Authentication/DefaultAcr/DefaultAcr.tsxadmin-ui/plugins/auth-server/components/Authentication/types/authenticationTypes.tsadmin-ui/plugins/auth-server/components/OidcClients/components/ClientTokensPanel.tsxadmin-ui/plugins/saml/components/WebsiteSsoIdentityProviderForm.tsxadmin-ui/plugins/saml/components/WebsiteSsoServiceProviderForm.tsxadmin-ui/plugins/scripts/components/CustomScriptForm.tsxadmin-ui/plugins/smtp/components/SmtpForm.tsxadmin-ui/plugins/user-claims/__tests__/components/UserClaimsListPage.test.tsxadmin-ui/plugins/user-claims/__tests__/utils/attributes.test.tsadmin-ui/plugins/user-claims/utils/attributes.ts
💤 Files with no reviewable changes (31)
- admin-ui/app/routes/Apps/Gluu/styles/GluuSecretDetail.style.ts
- admin-ui/app/config/site.ts
- admin-ui/plugins/user-claims/tests/utils/attributes.test.ts
- admin-ui/plugins/user-claims/utils/attributes.ts
- admin-ui/app/routes/Apps/Gluu/Tests/GluuTypeAhead.test.tsx
- admin-ui/app/routes/Apps/Gluu/Tests/GluuTypeAheadWithAdd.test.tsx
- admin-ui/app/styles/plugins/_react-bootstrap-typeahead.scss
- admin-ui/app/components/icons/index.ts
- admin-ui/app/routes/Apps/Gluu/GluuTypeAheadForDn.tsx
- admin-ui/app/routes/Apps/Gluu/GluuTypeAheadWithAdd.tsx
- admin-ui/app/styles/plugins/_react-toggle.scss
- admin-ui/app/routes/Apps/Gluu/Tests/GluuTypeAheadForDn.test.tsx
- admin-ui/app/routes/Apps/Gluu/types/GluuTypeAheadWithAdd.types.ts
- admin-ui/app/routes/Apps/Gluu/GluuTypeAhead.tsx
- admin-ui/app/routes/Apps/Gluu/types/GluuTypeAheadForDn.types.ts
- admin-ui/app/routes/Apps/Gluu/styles/GluuPermissionModal.style.ts
- admin-ui/app/routes/Apps/Gluu/styles/GluuTypeAheadWithAdd.style.ts
- admin-ui/app/routes/Apps/Gluu/types/GluuTypeAhead.types.ts
- admin-ui/app/routes/Apps/Gluu/types/GluuComponentPropsTypes.ts
- admin-ui/app/routes/Apps/Gluu/GluuBooleanSelectBox.tsx
- admin-ui/app/routes/Apps/Gluu/types/GluuPermissionModal.types.ts
- admin-ui/app/layout/default.tsx
- admin-ui/app/routes/Apps/Gluu/Tests/GluuSecretDetail.test.tsx
- admin-ui/app/routes/Apps/Gluu/styles/GluuTypeAhead.style.ts
- admin-ui/app/routes/Apps/Gluu/GluuPermissionModal.tsx
- admin-ui/app/routes/Apps/Gluu/types/index.ts
- admin-ui/app/styles/plugins/plugins.css
- admin-ui/app/styles/plugins/plugins.scss
- admin-ui/app/routes/Apps/Gluu/GluuSecretDetail.tsx
- admin-ui/app/routes/Apps/Gluu/GluuSuspenseLoader.tsx
- admin-ui/package.json
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
admin-ui/plugins/auth-server/components/Authentication/types/authenticationTypes.ts (1)
9-13:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve
hidein the centralized property contract.
AuthNItem.configurationPropertiesstill modelshide?: boolean, butAcrsFormValues.configurationPropertiesis now narrowed toPropertyConfig[]without that field. Downstream,getPropertiesConfig()dropshideandtransformConfigurationProperties()rewrites every row tohide: false, so opening and saving an ACR with hidden properties silently unhides them.♻️ Proposed fix
export type ConfigurationProperty = { key?: string value?: string value1?: string value2?: string hide?: boolean } -export type PropertyConfig = { +export type PropertyConfig = ConfigurationProperty & { id: string key: string value: string -} +}Then keep that field during the round-trip in
admin-ui/plugins/auth-server/components/Authentication/Acrs/helper/acrUtils.ts:return entry.configurationProperties.map((e) => ({ id: crypto.randomUUID(), key: e.key || e.value1 || '', value: e.value || e.value2 || '', + hide: e.hide ?? false, })) @@ .map((e) => ({ value1: e.key || e.value1 || '', value2: e.value || e.value2 || '', - hide: false, + hide: e.hide ?? false, }))Also applies to: 94-94
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@admin-ui/plugins/auth-server/components/Authentication/types/authenticationTypes.ts` around lines 9 - 13, PropertyConfig currently lacks the optional hide flag causing hidden properties to be lost; add hide?: boolean to the PropertyConfig type and update the round-trip logic so hide is preserved: adjust AuthNItem.configurationProperties / AcrsFormValues.configurationProperties typing to include hide, and modify getPropertiesConfig() and transformConfigurationProperties() so they carry through existing row.hide (do not unconditionally set hide: false) and only default to false when hide is undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@admin-ui/app/components/Layout/Layout.tsx`:
- Around line 91-92: The default pageDescription value in the Layout component
is incorrect; update the literal assigned to pageDescription in Layout.tsx to
exactly match the previous siteDescription constant ("Jans-server admin Ui") so
capitalization is consistent—locate the pageDescription property in the exported
metadata/object in Layout.tsx and change its string to match the siteDescription
constant.
---
Duplicate comments:
In
`@admin-ui/plugins/auth-server/components/Authentication/types/authenticationTypes.ts`:
- Around line 9-13: PropertyConfig currently lacks the optional hide flag
causing hidden properties to be lost; add hide?: boolean to the PropertyConfig
type and update the round-trip logic so hide is preserved: adjust
AuthNItem.configurationProperties / AcrsFormValues.configurationProperties
typing to include hide, and modify getPropertiesConfig() and
transformConfigurationProperties() so they carry through existing row.hide (do
not unconditionally set hide: false) and only default to false when hide is
undefined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e2266653-3632-40e6-847d-9b659507dfa0
📒 Files selected for processing (6)
admin-ui/app/components/Layout/Layout.tsxadmin-ui/app/routes/Apps/Gluu/GluuAutocomplete.tsxadmin-ui/plugins/auth-server/components/Authentication/Acrs/AcrsForm.tsxadmin-ui/plugins/auth-server/components/Authentication/Acrs/helper/acrUtils.tsadmin-ui/plugins/auth-server/components/Authentication/types/authenticationTypes.tsadmin-ui/plugins/auth-server/components/OidcClients/components/ClientTokensPanel.tsx
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/plugins/auth-server/components/Authentication/Acrs/AcrsEditPage.tsx (1)
235-241:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLog the full error object, not just the message, for diagnostic stack traces.
Line 237 logs only
error.message, but the catch block filters for non-API errors (!('response' in error)). Per codebase convention, non-network client-side errors should include the full stack trace for diagnostics.🛡️ Proposed fix
} catch (error) { if (error instanceof Error && !('response' in error)) { - logger.error('Unexpected error during form submission:', error.message) + logger.error('Unexpected error during form submission:', error instanceof Error ? error : String(error)) handleError(error) } setIsSubmitting(false) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@admin-ui/plugins/auth-server/components/Authentication/Acrs/AcrsEditPage.tsx` around lines 235 - 241, The catch block in the error handling section is logging only error.message for non-API errors, but the full error object should be logged to include stack trace information for diagnostic purposes. In the catch block where the error instanceof Error check is performed, change the logger.error call to pass the full error object instead of just error.message so that the stack trace is captured according to codebase conventions for client-side errors.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@admin-ui/plugins/auth-server/components/Authentication/AgamaFlows/AgamaFlows.tsx`:
- Line 377: The error parameter logged in the FileReader.onerror handler at line
377 is a ProgressEvent object, not an Error object. Instead of logging the
ProgressEvent directly, access the actual error information from the
FileReader's error property. Modify the logger.error call to inspect and log the
FileReader's error details (such as event.target.error or this.error depending
on context) to provide meaningful error information for debugging, following the
DOM operation patterns established in PR 2876.
---
Outside diff comments:
In
`@admin-ui/plugins/auth-server/components/Authentication/Acrs/AcrsEditPage.tsx`:
- Around line 235-241: The catch block in the error handling section is logging
only error.message for non-API errors, but the full error object should be
logged to include stack trace information for diagnostic purposes. In the catch
block where the error instanceof Error check is performed, change the
logger.error call to pass the full error object instead of just error.message so
that the stack trace is captured according to codebase conventions for
client-side errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1ae31c0e-735e-42a9-9618-c17eeb82a08c
📒 Files selected for processing (8)
admin-ui/app/components/Layout/Layout.tsxadmin-ui/app/layout/default.tsxadmin-ui/app/utils/menuFilters.tsadmin-ui/plugins/admin/components/Webhook/WebhookForm.tsxadmin-ui/plugins/auth-server/components/Authentication/Acrs/AcrsEditPage.tsxadmin-ui/plugins/auth-server/components/Authentication/AgamaFlows/AgamaFlows.tsxadmin-ui/plugins/auth-server/components/Authentication/Aliases/Aliases.tsxadmin-ui/plugins/auth-server/components/Authentication/DefaultAcr/DefaultAcr.tsx
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/plugins/auth-server/components/Authentication/Acrs/AcrsEditPage.tsx (1)
235-241:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLog the full error object, not just the message, for diagnostic stack traces.
Line 237 logs only
error.message, but the catch block filters for non-API errors (!('response' in error)). Per codebase convention, non-network client-side errors should include the full stack trace for diagnostics.🛡️ Proposed fix
} catch (error) { if (error instanceof Error && !('response' in error)) { - logger.error('Unexpected error during form submission:', error.message) + logger.error('Unexpected error during form submission:', error instanceof Error ? error : String(error)) handleError(error) } setIsSubmitting(false) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@admin-ui/plugins/auth-server/components/Authentication/Acrs/AcrsEditPage.tsx` around lines 235 - 241, The catch block in the error handling section is logging only error.message for non-API errors, but the full error object should be logged to include stack trace information for diagnostic purposes. In the catch block where the error instanceof Error check is performed, change the logger.error call to pass the full error object instead of just error.message so that the stack trace is captured according to codebase conventions for client-side errors.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@admin-ui/plugins/auth-server/components/Authentication/AgamaFlows/AgamaFlows.tsx`:
- Line 377: The error parameter logged in the FileReader.onerror handler at line
377 is a ProgressEvent object, not an Error object. Instead of logging the
ProgressEvent directly, access the actual error information from the
FileReader's error property. Modify the logger.error call to inspect and log the
FileReader's error details (such as event.target.error or this.error depending
on context) to provide meaningful error information for debugging, following the
DOM operation patterns established in PR 2876.
---
Outside diff comments:
In
`@admin-ui/plugins/auth-server/components/Authentication/Acrs/AcrsEditPage.tsx`:
- Around line 235-241: The catch block in the error handling section is logging
only error.message for non-API errors, but the full error object should be
logged to include stack trace information for diagnostic purposes. In the catch
block where the error instanceof Error check is performed, change the
logger.error call to pass the full error object instead of just error.message so
that the stack trace is captured according to codebase conventions for
client-side errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1ae31c0e-735e-42a9-9618-c17eeb82a08c
📒 Files selected for processing (8)
admin-ui/app/components/Layout/Layout.tsxadmin-ui/app/layout/default.tsxadmin-ui/app/utils/menuFilters.tsadmin-ui/plugins/admin/components/Webhook/WebhookForm.tsxadmin-ui/plugins/auth-server/components/Authentication/Acrs/AcrsEditPage.tsxadmin-ui/plugins/auth-server/components/Authentication/AgamaFlows/AgamaFlows.tsxadmin-ui/plugins/auth-server/components/Authentication/Aliases/Aliases.tsxadmin-ui/plugins/auth-server/components/Authentication/DefaultAcr/DefaultAcr.tsx
🛑 Comments failed to post (1)
admin-ui/plugins/auth-server/components/Authentication/AgamaFlows/AgamaFlows.tsx (1)
377-377:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFileReader error handling should inspect error object, not log the ProgressEvent directly.
Line 377 logs
errorfromFileReader.onerror, which is aProgressEvent, not anError. Per established patterns for DOM operations (learnings from PR 2876), this should destructure or inspect the error object:reader.onerror = (error) => { - logger.error('Error reading SHA256 file:', error) + const readerError = reader.error + logger.error('Error reading SHA256 file:', readerError instanceof Error ? readerError : String(readerError)) toast.error('Failed to read SHA256 file') }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.reader.onerror = (error) => { const readerError = reader.error logger.error('Error reading SHA256 file:', readerError instanceof Error ? readerError : String(readerError)) toast.error('Failed to read SHA256 file') }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@admin-ui/plugins/auth-server/components/Authentication/AgamaFlows/AgamaFlows.tsx` at line 377, The error parameter logged in the FileReader.onerror handler at line 377 is a ProgressEvent object, not an Error object. Instead of logging the ProgressEvent directly, access the actual error information from the FileReader's error property. Modify the logger.error call to inspect and log the FileReader's error details (such as event.target.error or this.error depending on context) to provide meaningful error information for debugging, following the DOM operation patterns established in PR 2876.Source: Learnings
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
duttarnab
previously approved these changes
Jun 16, 2026
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@admin-ui/plugins/user-management/components/UserForm.tsx`:
- Around line 187-193: The isTrackedChange function incorrectly marks all arrays
as tracked changes due to the condition Array.isArray(value) at line 189, which
returns true even for empty arrays. This causes empty arrays to always be
considered modified even when the initial value is also an empty array. Modify
the condition to check if the array is non-empty before marking it as a tracked
change, or better yet, remove the Array.isArray check entirely and let the
comparison logic handle array equality by checking if the current value differs
meaningfully from the initial value. This fix should be applied in the
isTrackedChange callback to ensure empty arrays are only marked as changes when
the initial value is actually different.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c41b3cef-a01b-4d7f-8b61-5e37e9a3522a
📒 Files selected for processing (19)
admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsxadmin-ui/app/routes/Apps/Gluu/GluuRemovableInputRow.tsxadmin-ui/app/routes/Apps/Gluu/GluuSelectRow.tsxadmin-ui/app/routes/Apps/Gluu/styles/GluuRemovableInputRow.style.tsadmin-ui/app/routes/Apps/Gluu/types/GluuCommitDialog.types.tsadmin-ui/app/routes/Apps/Gluu/types/GluuSelectRow.types.tsadmin-ui/app/utils/logLevel.tsadmin-ui/plugins/admin/components/Settings/SettingsPage.tsxadmin-ui/plugins/scripts/components/CustomScriptForm.tsxadmin-ui/plugins/scripts/components/styles/CustomScriptFormPage.style.tsadmin-ui/plugins/user-claims/components/styles/UserClaimsFormPage.style.tsadmin-ui/plugins/user-management/common/Constants.tsadmin-ui/plugins/user-management/components/AvailableClaimsPanel.tsxadmin-ui/plugins/user-management/components/UserClaimEntry.tsxadmin-ui/plugins/user-management/components/UserEditPage.tsxadmin-ui/plugins/user-management/components/UserForm.style.tsadmin-ui/plugins/user-management/components/UserForm.tsxadmin-ui/plugins/user-management/utils/attributeTransformUtils.tsadmin-ui/plugins/user-management/utils/userFormUtils.ts
💤 Files with no reviewable changes (1)
- admin-ui/plugins/user-management/components/UserEditPage.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@admin-ui/plugins/user-management/components/UserForm.tsx`:
- Around line 187-191: The isTrackedChange function currently marks a field as
modified whenever the new value is non-empty, without comparing it to the actual
initial value. This causes false positives when a user reverts a field to its
original non-empty value. Fix this by comparing the current value parameter
against formik.initialValues[name] for equality instead of just checking if
values are empty. The function should return true only when the value has
actually changed from its initial state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 88e30078-384a-4ec4-b315-ba187788f19f
📒 Files selected for processing (3)
admin-ui/app/routes/Apps/Gluu/GluuWebhookExecutionDialog.tsxadmin-ui/app/routes/Apps/Gluu/styles/GluuWebhookExecutionDialog.style.tsadmin-ui/plugins/user-management/components/UserForm.tsx
💤 Files with no reviewable changes (1)
- admin-ui/app/routes/Apps/Gluu/styles/GluuWebhookExecutionDialog.style.ts
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
|
duttarnab
approved these changes
Jun 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
chore(admin-ui): remove two more redundant UI dependencies (#2878)
Summary
Continues the dependency and dead-code cleanup from #2874. Removes two redundant
UI libraries (
react-bootstrap-typeahead,react-toggle) by leaning onalready-bundled MUI and a native control, relocates the stray site constants into
app/constants/, and clears out dead components the cleanup surfaced. Nouser-facing behavior changes.
Dependencies removed
react-bootstrap-typeahead->GluuAutocomplete(in-house wrapper over MUIAutocomplete, already bundled)react-toggle+@types/react-toggle->GluuToggle(native checkbox + CSS)Changes
GluuAutocomplete(ACR mapping, SAML SP form,GluuInlineInputarray mode); deletedGluuTypeAhead,GluuTypeAheadForDn,GluuTypeAheadWithAddwith their styles, types, and tests.react-togglecall sites toGluuToggle/GluuToggleRow(Assets, Webhook, SMTP, Custom Script, SAML IdP).GluuTogglefully controlled: dropped its internal state and sync effect sovalueis the single source of truth (behavior unchanged; updated its tests to a controlled harness)._react-bootstrap-typeahead.scss,_react-toggle.scss, and the staleplugins.css, plus theirplugins.scssimports.app/config/site.ts->app/constants/site.tsand updated its import.Dead code removed (surfaced during cleanup)
GluuBooleanSelectBox(single caller + a never-used<select>branch) ->GluuToggleRow.GluuSecretDetail,GluuSuspenseLoader(a one-line<GluuSpinner/>wrapper), andGluuPermissionModal(+ its style and type) - unused, no production callers.How to test (manual QA)
Log in first; each control was reworked internally and should behave as before.
Verification
Ticket
Closes: #2878
Summary by CodeRabbit