feat(admin-ui): revamp SCIM module as per Figma#2702
Merged
Conversation
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
…ex into admin-ui-issue-2630
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>
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:
📝 WalkthroughWalkthroughReplaced react-redux hooks with app-specific hooks across numerous files; revamped SCIM module (validation schema, form layout, commit flow, types, styles); consolidated webhook helper utilities and user-management helper barrel; adjusted UI/form styling constants and added localization and short-code entries. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Client
participant ScimForm as ScimConfiguration
participant Validation as getScimConfigurationSchema
participant Operations as buildScimChangedFieldOperations
participant CommitDialog as GluuWebhookCommitDialog
participant Webhook as triggerScimWebhook
participant API as Backend/API
User->>ScimForm: Submit form
ScimForm->>Validation: Validate formValues
Validation-->>ScimForm: Validation result
alt valid
ScimForm->>Operations: Build changed operations
Operations-->>ScimForm: operations[]
ScimForm->>CommitDialog: Open dialog with operations
User->>CommitDialog: Confirm
CommitDialog->>Webhook: triggerScimWebhook(payload, feature)
Webhook->>API: Send webhook payload
API-->>Webhook: Response
Webhook-->>CommitDialog: Success
CommitDialog-->>ScimForm: Close / success
else invalid
Validation-->>ScimForm: errors
ScimForm-->>User: show validation errors
end
sequenceDiagram
participant Component as UserForm
participant Helper as user-management/helper
participant Audit as AuditLogger
participant Webhook as triggerUserWebhook
participant Trigger as triggerWebhookForFeature
participant Redux as Store
Component->>Helper: call logUserUpdate(...)
Helper-->>Audit: prepare audit payload
Audit->>Redux: dispatch audit action
Component->>Helper: call triggerUserWebhook(data, feature)
Helper->>Trigger: triggerWebhookForFeature(record, feature)
Trigger->>Redux: dispatch webhook trigger
Redux-->>Component: update/confirm
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mo-auto
added
comp-admin-ui
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
admin-ui/app/routes/Apps/Gluu/GluuSessionTimeout.tsx (1)
41-41:⚠️ Potential issue | 🟡 MinorTypo in audit log message.
"mannually" should be "manually".
✏️ Proposed fix
- message: isTimedOut ? 'User session timed out' : 'User logged out mannually', + message: isTimedOut ? 'User session timed out' : 'User logged out manually',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/routes/Apps/Gluu/GluuSessionTimeout.tsx` at line 41, Fix the typo in the audit log message where the logout path uses "mannually" — locate the message assignment that sets message: isTimedOut ? 'User session timed out' : 'User logged out mannually' (in GluuSessionTimeout) and change the string to 'User logged out manually' so the audit entry reads correctly for the non-timeout branch.admin-ui/plugins/scim/helper/utils.ts (1)
46-66:⚠️ Potential issue | 🟠 MajorEmit a removal when a user clears an existing value.
Once
normalizedNewbecomesundefined, this helper returns without generating any patch. Clearing an optional SCIM setting therefore leaves the old server value in place after save.🩹 Suggested fix
if (normalizedOriginal !== normalizedNew) { if (normalizedNew === undefined || normalizedNew === null) { - return + if (normalizedOriginal !== undefined && normalizedOriginal !== null) { + patches.push({ + op: 'remove', + path: `/${path}`, + }) + } + return } const operation = normalizedOriginal === undefined ? 'add' : 'replace' patches.push({ op: operation, path: `/${path}`, - value: newValue, + value: normalizedNew, }) }Based on learnings, ensure Yup schema marks fields as nullable when the backend DTOs permit nulls and do not mark them as required. The same nullable-field contract needs to be preserved when generating patches.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/scim/helper/utils.ts` around lines 46 - 66, The helper addPatchIfDifferent currently returns early when normalizedNew is undefined/null, preventing a patch to remove an existing value; update it so that when normalizedOriginal !== normalizedNew and normalizedNew is undefined or null you push a patch with op: 'remove' and path: `/${path}` (no value field for remove), otherwise keep the existing logic to emit 'add' or 'replace' using the normalizedNew/newValue as appropriate; update addPatchIfDifferent to use normalizedOriginal/normalizedNew for comparisons and emit the 'remove' operation when a user clears a value to ensure the backend sees the deletion.admin-ui/plugins/scim/components/ScimPage.tsx (1)
79-104:⚠️ Potential issue | 🟠 MajorUse
previousConfigas optimistic-update base to avoid stale cache writes.On Line 79 you correctly snapshot
previousConfig, but the optimistic patch reduce currently starts fromscimConfiguration(Line 103). If that value is stale, optimistic cache state can regress newer data.Suggested fix
- if (previousConfig && scimConfiguration) { + if (previousConfig) { queryClient.setQueryData(getGetScimConfigQueryKey(), () => { return variables.data.reduce( (updated, patch: JsonPatch) => { if (typeof patch.path !== 'string' || !patch.path.startsWith('/')) { return updated } const key = patch.path.substring(1) switch (patch.op) { case 'replace': case 'add': return { ...updated, [key]: patch.value } case 'remove': { const { [key]: omitted, ...rest } = updated void omitted return rest } default: return updated } }, - { ...scimConfiguration }, + { ...previousConfig }, ) }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/scim/components/ScimPage.tsx` around lines 79 - 104, The optimistic-update currently reduces patches starting from scimConfiguration which can be stale; change the base state in the queryClient.setQueryData call to use the previously snapped previousConfig (fallback to scimConfiguration if previousConfig is undefined) so the reducer in variables.data.reduce applies patches on previousConfig instead of scimConfiguration; update the code around queryClient.setQueryData/getGetScimConfigQueryKey to initialize the reduce with previousConfig || scimConfiguration (referencing previousConfig, scimConfiguration, variables.data.reduce, JsonPatch).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin-ui/app/cedarling/hooks/useCedarling.ts`:
- Line 33: Move the static Set creation out of the component so it isn't
recreated on every render: define a top-level constant (e.g., executeUrls = new
Set([SSA_ADMIN, SSA_DEVELOPER, SCIM_BULK, REVOKE_SESSION, OPENID])) above the
useCedarling hook and replace the in-hook declaration with a reference to that
top-level executeUrls; update any dependency arrays that previously referenced
the in-scope variable to use the top-level constant (no changes needed to hook
logic or the SSA_*/OPENID constants).
In `@admin-ui/app/routes/Apps/Gluu/MultiValueSelectCard.tsx`:
- Around line 149-160: The remove button inside the conditional block calls
onRemoveField even when the card is disabled; update the button rendered in the
component (the element using className removeFieldButton inside the wrapper/div
that returns when onRemoveField is provided) to respect the disabled prop by
disabling the button (add disabled attribute) and short-circuiting the onClick
(or wrap onClick to no-op) when disabled is true, and ensure aria-disabled or
appropriate accessibility attributes are set so a read-only card cannot be
removed.
- Line 15: The combobox lacks a programmatic label and the wrapper remove button
ignores the disabled state: preserve/pass the original name prop so the label
can get an id and be referenced, add an id to the label and attach
aria-labelledby to the input via the renderInput implementation (keep using the
existing renderInput method to set aria-labelledby), and set disabled={disabled}
on the wrapper remove button that calls onRemoveField so it respects the same
disabled gating used by handleAdd, handleRemove, and handleRemoveByName.
In `@admin-ui/app/routes/Apps/Gluu/styles/GluuInputRow.style.ts`:
- Around line 16-19: The current style rule that reads '& input:focus, &
input:focus-visible, & input:active' removes visible focus indicators; remove
':focus-visible' from that suppression and instead add a dedicated rule for '&
input:focus-visible' that preserves an accessible focus indicator (e.g., restore
outline/boxShadow or a visible ring). Update the style object in
GluuInputRow.style.ts so the shared suppression only applies to ':focus' and
':active' if needed, and add a separate '& input:focus-visible' entry that sets
an appropriate visible outline/boxShadow for keyboard focus.
In `@admin-ui/app/routes/Apps/Gluu/styles/MultiValueSelectCard.style.ts`:
- Around line 50-71: The current focus selector '&.Mui-focused,
&.Mui-focusVisible' on the .MuiOutlinedInput-root removes the browser focus
indicator and leaves border/background identical to the resting state; update
that selector to present a visible, high-contrast focus ring for keyboard users
(e.g., add an outline or focus shadow such as outline: '2px solid
<accessibleColor>' or boxShadow: '0 0 0 3px <focusRingColor>' and/or change
borderColor to a clearly contrasting color), ensure the rule applies when
.Mui-focused or .Mui-focusVisible is present, and include !important if needed
to override inherited styles so the combobox shows a visible focus state for
keyboard navigation.
In `@admin-ui/app/routes/License/hooks/useLicenseDetails.ts`:
- Around line 79-85: Replace the mutation-based audit composition with an
immutable object spread: instead of calling createAuditLog(currentState) and
then mutating audit.action/resource/message, create a new object by spreading
createAuditLog(currentState) and adding action: DELETION, resource: API_LICENSE,
and message: pendingMessageRef.current, then pass that newAudit to
postUserAction; keep references to createAuditLog, DELETION, API_LICENSE,
pendingMessageRef, and postUserAction to locate the change.
In `@admin-ui/plugins/admin/components/Settings/SettingsPage.style.ts`:
- Around line 136-148: The validation messages are positioned absolutely which
removes them from the document flow and causes overlap; in the CSS rules around
'& .form-group div[class*="col"]' and '& [data-field-error]' remove or avoid
'position: absolute' on the '[data-field-error]' selector and instead keep it in
the normal flow (e.g., make it display:block/inline-block and use marginTop or
paddingTop to create spacing) while preserving the font size override; this
ensures long or localized errors expand the layout rather than overlap adjacent
fields.
- Around line 158-184: The current focus selectors in SettingsPage.style.ts
remove browser focus indicators (selectors: '& input:focus, & select:focus, &
.custom-select:focus' and '& input:focus-visible, & select:focus-visible'),
which makes keyboard focus invisible; instead, stop clearing outline/boxShadow
and implement an accessible focus style: remove the "!important" removals and
add a visible focus ring (e.g., a contrasting border color and/or subtle
boxShadow with an outline-offset) for the same selectors so keyboard users can
see focus; ensure focus-visible still applies to show the ring only for keyboard
focus and retain existing background/color values in those rules.
In `@admin-ui/plugins/scim/components/ScimConfiguration.tsx`:
- Around line 58-67: handleCancel and handleFormSubmit currently list
formik.resetForm and formik.handleSubmit in their dependency arrays which can
break Formik's internal registration and focus behavior; change both callbacks
to depend on the whole formik object instead (use formik in the dependency
arrays for useCallback for handleCancel and handleFormSubmit) so that the hooks
re-evaluate correctly when Formik's instance changes, referencing the existing
handleCancel, handleFormSubmit, formik.resetForm, and formik.handleSubmit
identifiers when making the update.
In `@admin-ui/plugins/scim/components/ScimFieldRenderer.tsx`:
- Around line 27-103: The FieldConfig.disabled flag is only applied in the
'text' branch; update the other renderer branches so the disabled contract is
consistent: pass disabled={disabled} into GluuInputRow in the 'number' case,
into GluuSelectRow in the 'select' case, and into GluuToggleRow in the 'toggle'
case (and if those components' prop types don't include disabled, add it to
their props/interfaces and handle it in their implementations).
In `@admin-ui/plugins/scim/components/styles/ScimFormPage.style.ts`:
- Around line 179-187: The focus override for the selector '& input:focus, &
input:focus-visible, & input:active, & select:focus, & select:focus-visible, &
select:active' removes the browser focus ring and makes focused inputs
indistinguishable; update the rule in ScimFormPage.style.ts to preserve or add a
visible, high-contrast focus indicator (for example by removing the 'outline:
none !important' and 'boxShadow: none !important' lines or replacing them with a
clear accessible outline/box-shadow such as a 2px solid or 3px glow using a
theme contrast color) and ensure the border/box-shadow color contrasts with the
background so keyboard users can see focus.
In `@admin-ui/plugins/scim/helper/validations.ts`:
- Around line 30-57: The numeric schema entries (maxCount, bulkMaxOperations,
bulkMaxPayloadSize, metricReporterInterval, metricReporterKeepDataDays) must
accept blank strings from transformToFormValues() without becoming NaN; update
each Yup.number() chain to be nullable and convert empty-string inputs to null
(e.g. append .nullable().transform((value, original) => original === '' ? null :
value)) and do not mark them as required so the form can be cleared safely;
apply this change to the listed fields in validations.ts.
---
Outside diff comments:
In `@admin-ui/app/routes/Apps/Gluu/GluuSessionTimeout.tsx`:
- Line 41: Fix the typo in the audit log message where the logout path uses
"mannually" — locate the message assignment that sets message: isTimedOut ?
'User session timed out' : 'User logged out mannually' (in GluuSessionTimeout)
and change the string to 'User logged out manually' so the audit entry reads
correctly for the non-timeout branch.
In `@admin-ui/plugins/scim/components/ScimPage.tsx`:
- Around line 79-104: The optimistic-update currently reduces patches starting
from scimConfiguration which can be stale; change the base state in the
queryClient.setQueryData call to use the previously snapped previousConfig
(fallback to scimConfiguration if previousConfig is undefined) so the reducer in
variables.data.reduce applies patches on previousConfig instead of
scimConfiguration; update the code around
queryClient.setQueryData/getGetScimConfigQueryKey to initialize the reduce with
previousConfig || scimConfiguration (referencing previousConfig,
scimConfiguration, variables.data.reduce, JsonPatch).
In `@admin-ui/plugins/scim/helper/utils.ts`:
- Around line 46-66: The helper addPatchIfDifferent currently returns early when
normalizedNew is undefined/null, preventing a patch to remove an existing value;
update it so that when normalizedOriginal !== normalizedNew and normalizedNew is
undefined or null you push a patch with op: 'remove' and path: `/${path}` (no
value field for remove), otherwise keep the existing logic to emit 'add' or
'replace' using the normalizedNew/newValue as appropriate; update
addPatchIfDifferent to use normalizedOriginal/normalizedNew for comparisons and
emit the 'remove' operation when a user clears a value to ensure the backend
sees the deletion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2ba61525-7d92-43bc-89df-45dddcf5710f
📒 Files selected for processing (41)
admin-ui/app/cedarling/hooks/useCedarling.tsadmin-ui/app/components/App/PermissionsPolicyInitializer.tsxadmin-ui/app/constants/ui.tsadmin-ui/app/locales/en/translation.jsonadmin-ui/app/locales/es/translation.jsonadmin-ui/app/routes/Apps/Gluu/GluuAppSidebar.tsxadmin-ui/app/routes/Apps/Gluu/GluuRemovableSelectRow.tsxadmin-ui/app/routes/Apps/Gluu/GluuSessionTimeout.tsxadmin-ui/app/routes/Apps/Gluu/GluuWebhookErrorDialog.tsxadmin-ui/app/routes/Apps/Gluu/MultiValueSelectCard.tsxadmin-ui/app/routes/Apps/Gluu/styles/GluuInputRow.style.tsadmin-ui/app/routes/Apps/Gluu/styles/MultiValueSelectCard.style.tsadmin-ui/app/routes/Apps/Gluu/types/MultiValueSelectCard.types.tsadmin-ui/app/routes/License/hooks/useLicenseDetails.tsadmin-ui/app/routes/components/Dropdowns/DropdownProfile.tsxadmin-ui/app/utils/LicenseScreens/GenerateLicenseCard.tsxadmin-ui/app/utils/UploadSSA.tsxadmin-ui/plugins/admin/components/Assets/AssetForm.tsxadmin-ui/plugins/admin/components/Assets/JansAssetListPage.tsxadmin-ui/plugins/admin/components/Assets/hooks/useAssetMutations.tsadmin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsxadmin-ui/plugins/admin/components/Mapping/RolePermissionCard.tsxadmin-ui/plugins/admin/components/Mapping/hooks/useMappingApi.tsadmin-ui/plugins/admin/components/Mapping/styles/MappingPage.style.tsadmin-ui/plugins/admin/components/Settings/SettingsPage.style.tsadmin-ui/plugins/admin/components/Webhook/hooks/useWebhookMutations.tsadmin-ui/plugins/admin/helper/shortCodes.jsonadmin-ui/plugins/admin/helper/utils.tsadmin-ui/plugins/scim/components/ScimConfiguration.tsxadmin-ui/plugins/scim/components/ScimFieldRenderer.tsxadmin-ui/plugins/scim/components/ScimPage.tsxadmin-ui/plugins/scim/components/constants.tsadmin-ui/plugins/scim/components/styles/ScimFormPage.style.tsadmin-ui/plugins/scim/components/types/index.tsadmin-ui/plugins/scim/helper/constants.tsadmin-ui/plugins/scim/helper/index.tsadmin-ui/plugins/scim/helper/schema.tsadmin-ui/plugins/scim/helper/utils.tsadmin-ui/plugins/scim/helper/validations.tsadmin-ui/plugins/scim/types/index.tsadmin-ui/plugins/user-management/components/UserClaimEntry.tsx
💤 Files with no reviewable changes (3)
- admin-ui/plugins/scim/components/types/index.ts
- admin-ui/plugins/user-management/components/UserClaimEntry.tsx
- admin-ui/plugins/scim/helper/schema.ts
admin-ui/app/routes/Apps/Gluu/styles/MultiValueSelectCard.style.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
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 (1)
admin-ui/plugins/scim/components/ScimPage.tsx (1)
83-105:⚠️ Potential issue | 🟠 MajorUse cached
previousConfigas the optimistic-update base.The optimistic reducer currently patches
scimConfigurationfrom closure state instead of the snapshot returned bygetQueryData. That can apply patches to stale data and produce incorrect optimistic UI state.♻️ Proposed fix
- if (previousConfig && scimConfiguration) { + if (previousConfig) { queryClient.setQueryData(getGetScimConfigQueryKey(), () => { return variables.data.reduce( (updated, patch: JsonPatch) => { if (typeof patch.path !== 'string' || !patch.path.startsWith('/')) { return updated } const key = patch.path.substring(1) switch (patch.op) { case 'replace': case 'add': return { ...updated, [key]: patch.value } case 'remove': { const { [key]: omitted, ...rest } = updated void omitted return rest } default: return updated } }, - { ...scimConfiguration }, + { ...previousConfig }, ) }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/scim/components/ScimPage.tsx` around lines 83 - 105, The optimistic-update reducer is using the closure value scimConfiguration instead of the cached snapshot previousConfig returned by getQueryData, which can produce stale updates; change the setQueryData updater so it seeds the reducer with { ...previousConfig } (or previousConfig directly) as the base instead of { ...scimConfiguration }, keeping the rest of the reducer logic (variables.data, JsonPatch handling, ops 'add'|'replace'|'remove') intact and still using getGetScimConfigQueryKey() for the query key.
♻️ Duplicate comments (1)
admin-ui/plugins/scim/components/ScimFieldRenderer.tsx (1)
98-105:⚠️ Potential issue | 🟠 Major
disabledis not honored for toggle fields.
GluuToggleRowdisables viaviewOnly, so passingdisabledhere leaves toggle fields editable.♻️ Proposed fix
<GluuToggleRow label={label} name={name} formik={formik} lsize={LABEL_SIZE} rsize={INPUT_SIZE} - disabled={disabled} + viewOnly={disabled} doc_category={DOC_CATEGORY} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/scim/components/ScimFieldRenderer.tsx` around lines 98 - 105, The GluuToggleRow toggle remains editable because it uses viewOnly rather than disabled; update ScimFieldRenderer's GluuToggleRow invocation to pass the viewOnly prop (e.g., viewOnly={disabled}) so the toggle honors the disabled state (you can keep or remove the existing disabled={disabled} prop, but ensure viewOnly is set based on the disabled variable).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin-ui/plugins/scim/components/styles/ScimFormPage.style.ts`:
- Around line 131-135: The styles currently override focus outlines in the rule
that applies createFormInputFocusStyles(inputColors) and sets outlineOffset: '0
!important'; update this so the explicit outline disabling is removed and the
focus indicator remains robust: either remove any 'outline: none' logic inside
createFormInputFocusStyles and drop the outlineOffset override, or replace it
with a clear, high-contrast box-shadow (and keep outline preserved) so keyboard
focus is visibly distinct for inputs/selects; locate createFormInputFocusStyles
and the CSS rule that spreads its result to make the change.
In `@admin-ui/plugins/scim/helper/validations.ts`:
- Around line 5-13: Rename the misleading function isSecureUrl to isValidHttpUrl
to reflect that it accepts both http and https; update the function declaration
(isSecureUrl -> isValidHttpUrl) and replace all usages and exports/imports
referencing isSecureUrl in this file so callers (including the existing
reference in the same module) use the new name; keep the implementation
unchanged so it still validates http: and https: protocols.
In `@admin-ui/plugins/user-management/components/UserForm.style.ts`:
- Around line 65-70: The spread of createFormInputAutofillStyles(inputColors)
already sets backgroundColor and color with !important, so remove the redundant
overrides of backgroundColor and color (which reference inputBg and
themeColors.fontColor) from the autofill selector in UserForm.style.ts, or if
the !important override is intentional keep them but add a one-line comment
above that selector explaining why the explicit overrides are required for
specificity; target the block that begins with '& input:-webkit-autofill...' and
adjust accordingly.
---
Outside diff comments:
In `@admin-ui/plugins/scim/components/ScimPage.tsx`:
- Around line 83-105: The optimistic-update reducer is using the closure value
scimConfiguration instead of the cached snapshot previousConfig returned by
getQueryData, which can produce stale updates; change the setQueryData updater
so it seeds the reducer with { ...previousConfig } (or previousConfig directly)
as the base instead of { ...scimConfiguration }, keeping the rest of the reducer
logic (variables.data, JsonPatch handling, ops 'add'|'replace'|'remove') intact
and still using getGetScimConfigQueryKey() for the query key.
---
Duplicate comments:
In `@admin-ui/plugins/scim/components/ScimFieldRenderer.tsx`:
- Around line 98-105: The GluuToggleRow toggle remains editable because it uses
viewOnly rather than disabled; update ScimFieldRenderer's GluuToggleRow
invocation to pass the viewOnly prop (e.g., viewOnly={disabled}) so the toggle
honors the disabled state (you can keep or remove the existing
disabled={disabled} prop, but ensure viewOnly is set based on the disabled
variable).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e1b63dda-ed1d-46d9-853c-36879c2a4b95
📒 Files selected for processing (22)
admin-ui/app/cedarling/hooks/useCedarling.tsadmin-ui/app/routes/Apps/Gluu/GluuWebhookCommitDialog.tsxadmin-ui/app/routes/Apps/Gluu/styles/GluuInputRow.style.tsadmin-ui/app/routes/License/hooks/useLicenseDetails.tsadmin-ui/app/styles/formStyles.tsadmin-ui/app/utils/triggerWebhookForFeature.tsadmin-ui/plugins/scim/components/ScimConfiguration.tsxadmin-ui/plugins/scim/components/ScimFieldRenderer.tsxadmin-ui/plugins/scim/components/ScimPage.tsxadmin-ui/plugins/scim/components/styles/ScimFormPage.style.tsadmin-ui/plugins/scim/helper/utils.tsadmin-ui/plugins/scim/helper/validations.tsadmin-ui/plugins/user-management/components/PasswordChangeModal.style.tsadmin-ui/plugins/user-management/components/PasswordChangeModal.tsxadmin-ui/plugins/user-management/components/User2FADevicesModal.tsxadmin-ui/plugins/user-management/components/UserAddPage.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/helper/index.tsadmin-ui/plugins/user-management/helper/utils.tsadmin-ui/plugins/user-management/hooks/useUserMutations.ts
Comment on lines
+131
to
+135
| '& input:focus, & input:focus-visible, & input:active, & select:focus, & select:focus-visible, & select:active': | ||
| { | ||
| ...createFormInputFocusStyles(inputColors), | ||
| outlineOffset: '0 !important', | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check what createFormInputFocusStyles provides for focus styling
# Expect: The utility should define visible focus indicators (outline, border, or box-shadow)
ast-grep --pattern 'export const createFormInputFocusStyles = $_'
# Also search for the implementation
rg -n -A 15 'createFormInputFocusStyles' --type ts -g '!*.d.ts'Repository: GluuFederation/flex
Length of output: 11581
Remove explicit outline disabling for WCAG 2.4.7 compliance.
While createFormInputFocusStyles does provide a visible focus indicator via a border, it explicitly removes the outline (outline: 'none !important') and box-shadow. WCAG 2.4.7 requires a visible focus indicator; relying solely on a border may not provide sufficient contrast or be recognized universally. Either keep the outline or add a more prominent box-shadow to ensure robust keyboard navigation accessibility across different user preferences and assistive technologies.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@admin-ui/plugins/scim/components/styles/ScimFormPage.style.ts` around lines
131 - 135, The styles currently override focus outlines in the rule that applies
createFormInputFocusStyles(inputColors) and sets outlineOffset: '0 !important';
update this so the explicit outline disabling is removed and the focus indicator
remains robust: either remove any 'outline: none' logic inside
createFormInputFocusStyles and drop the outlineOffset override, or replace it
with a clear, high-contrast box-shadow (and keep outline preserved) so keyboard
focus is visibly distinct for inputs/selects; locate createFormInputFocusStyles
and the CSS rule that spreads its result to make the change.
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin-ui/plugins/scim/helper/validations.ts`:
- Around line 21-25: The Yup validation for applicationUrl uses test name
'valid-secure-url' while the validator function is isValidHttpUrl (which accepts
both http and https); rename the test key to reflect the actual check (e.g.,
'valid-http-url' or 'valid-url') or adjust the validator to enforce HTTPS if you
intend 'secure' behavior; update the test declaration referencing applicationUrl
and isValidHttpUrl so the test name matches the validation semantics.
In `@admin-ui/plugins/user-management/components/UserForm.style.ts`:
- Line 29: Replace the hardcoded numeric padding in the form group override by
using the centralized design token: change the call that creates
formGroupWithErrors (createFormGroupOverrides({ columnPaddingBottom: 16 })) to
use SPACING (e.g., SPACING[someKey] or SPACING.x) instead of 16; update the
import to ensure SPACING is imported where formGroupWithErrors is defined and
pick the appropriate spacing token that matches 16px in the design system so
columnPaddingBottom references SPACING rather than a literal number.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4f0979af-aa09-4a16-a3ea-7356a0ad63e9
📒 Files selected for processing (2)
admin-ui/plugins/scim/helper/validations.tsadmin-ui/plugins/user-management/components/UserForm.style.ts
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
duttarnab
requested changes
Mar 16, 2026
Contributor
duttarnab
left a comment
duttarnab
left a comment
There was a problem hiding this comment.
Disable External Logger configurationandExternal Logger Configurationfields are missing.
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
There was a problem hiding this comment.
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/scim/helper/utils.ts (1)
60-70:⚠️ Potential issue | 🟠 MajorClearing a previously set field is currently impossible in patch generation.
At lines 61–63, empty/null new values are ignored. If a user clears an existing value (for example
externalLoggerConfiguration), no JSON Patch is emitted, so the backend keeps stale data. This pattern is already handled correctly in other parts of the codebase (e.g., SmtpEditPage.tsx). Emit aremoveoperation when the original value exists and the new value is empty/null.Proposed fix
if (normalizedOriginal !== normalizedNew) { if (normalizedNew === undefined || normalizedNew === null) { - return + if (normalizedOriginal !== undefined && normalizedOriginal !== null) { + patches.push({ + op: 'remove', + path: `/${path}`, + }) + } + return } const operation = normalizedOriginal === undefined ? 'add' : 'replace' patches.push({ op: operation, path: `/${path}`, - value: newValue, + value: normalizedNew, }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/scim/helper/utils.ts` around lines 60 - 70, The current logic ignores empty/null new values and never emits a JSON Patch to clear an existing field; change the branch that checks normalizedNew so that when normalizedNew is null/undefined and normalizedOriginal is present you push a { op: 'remove', path: `/${path}` } patch (no value), otherwise return; keep the existing flow for non-empty new values (compute operation = normalizedOriginal === undefined ? 'add' : 'replace' and push with value: newValue). Refer to variables/functions: normalizedOriginal, normalizedNew, patches, operation, path, newValue in utils.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@admin-ui/plugins/scim/helper/utils.ts`:
- Around line 60-70: The current logic ignores empty/null new values and never
emits a JSON Patch to clear an existing field; change the branch that checks
normalizedNew so that when normalizedNew is null/undefined and
normalizedOriginal is present you push a { op: 'remove', path: `/${path}` }
patch (no value), otherwise return; keep the existing flow for non-empty new
values (compute operation = normalizedOriginal === undefined ? 'add' : 'replace'
and push with value: newValue). Refer to variables/functions:
normalizedOriginal, normalizedNew, patches, operation, path, newValue in
utils.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8329b59a-a0ff-49ac-979f-90ec02e184fb
📒 Files selected for processing (9)
admin-ui/app/locales/en/translation.jsonadmin-ui/app/locales/es/translation.jsonadmin-ui/app/locales/fr/translation.jsonadmin-ui/app/locales/pt/translation.jsonadmin-ui/app/routes/Apps/Gluu/styles/GluuInputRow.style.tsadmin-ui/plugins/scim/components/constants.tsadmin-ui/plugins/scim/helper/utils.tsadmin-ui/plugins/scim/helper/validations.tsadmin-ui/plugins/scim/types/index.ts
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin-ui/plugins/scim/helper/utils.ts`:
- Around line 33-35: The cast indicates AppConfiguration3 is missing the
disableExternalLoggerConfiguration property; update the generated
AppConfiguration3 type to include disableExternalLoggerConfiguration (optional
and typed consistently with other boolean flags, e.g., boolean | undefined or
JsonValue depending on generator expectations), then remove the ad-hoc cast and
read config.disableExternalLoggerConfiguration directly where used (notably in
the toBooleanValue call in utils.ts and the similar access in
createJsonPatchFromDifferences) so the code uses the typed property and avoids
unsafe Record<string, JsonValue | undefined> casting.
In
`@admin-ui/plugins/smtp-management/__tests__/components/SmtpManagement/SmtpEditPage.test.tsx`:
- Around line 22-27: The test mocks use lowercase resource IDs which diverge
from production; update the jest.mock for ADMIN_UI_RESOURCES to use the real
PascalCase keys/values (e.g., 'SMTP' and 'Webhooks') so case-sensitive
comparisons behave like production, and adjust the CEDAR_RESOURCE_SCOPES mock to
provide non-empty scope arrays (e.g., populate smtp scopes) so smtpScopes.length
> 0 triggers authorizeHelper in the component; modify mocks referencing
ADMIN_UI_RESOURCES and CEDAR_RESOURCE_SCOPES and re-run tests to ensure
authorizeHelper and related authorization branches are exercised.
In
`@admin-ui/plugins/smtp-management/__tests__/components/SmtpManagement/SmtpForm.test.tsx`:
- Around line 18-23: The test mocks use lowercase resource keys causing casing
mismatch; update the mocked constants so ADMIN_UI_RESOURCES uses 'SMTP' and
'Webhooks' (not 'smtp'/'webhooks') and make the CEDAR_RESOURCE_SCOPES mock use
the same production-casing keys (e.g., CEDAR_RESOURCE_SCOPES: { SMTP: [],
Webhooks: [] }) so tests align with production values referenced by the code
under test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 50d785f5-1724-4983-9299-6a078e363647
📒 Files selected for processing (3)
admin-ui/plugins/scim/helper/utils.tsadmin-ui/plugins/smtp-management/__tests__/components/SmtpManagement/SmtpEditPage.test.tsxadmin-ui/plugins/smtp-management/__tests__/components/SmtpManagement/SmtpForm.test.tsx
admin-ui/plugins/smtp-management/__tests__/components/SmtpManagement/SmtpEditPage.test.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/smtp-management/__tests__/components/SmtpManagement/SmtpForm.test.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
|
duttarnab
approved these changes
Mar 17, 2026
moabu
approved these changes
Mar 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
4 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.
feat(admin-ui): revamp SCIM module as per Figma (#2635)
Summary
This PR revamps the SCIM module in the Admin UI to align with the latest Figma designs as part of the ongoing Admin UI redesign initiative.
Changes Included
Result
Parent Initiative
This work is part of the following parent feature:
feat(admin-ui): Revamp the existing design of the admin-ui according to the new design🔗 Ticket
Closes: #2635
Summary by CodeRabbit
New Features
UI Improvements
Bug Fixes
Localization