Skip to content

Conversation

@faisalsiddique4400
Copy link
Contributor

@faisalsiddique4400 faisalsiddique4400 commented Dec 3, 2025

fix(admin-ui): roles not removable from user profile and apply action not triggered (#2494)

Summary

User roles were not updating correctly when removing roles from the profile, and the Apply button did not trigger changes if fields were removed. These issues were identified in Flex 5.14.0.

Fix Summary

  • Handled state updates properly when removing roles from the user object.
  • Fixed form change detection logic to enable Apply when fields are removed.
  • Updated the role list binding to ensure UI reflects add/remove actions immediately.
  • Ensured correct payload is sent during update operations.

Result

Users can now add or remove roles successfully, and the Apply button activates as expected when modifications are made.

🔗 Ticket

Closes: #2494

Summary by CodeRabbit

  • New Features

    • Expandable display for multi-valued attributes with "Show More"/"Show Less" and clearer empty-value badges.
    • Forms now accept and display boolean field values alongside strings.
  • Bug Fixes

    • More reliable form change detection, Apply button disabling, and submission flow to prevent unintended submits.
    • Safer redaction and handling of sensitive custom attributes in audit payloads.
  • Improvements

    • Centralized attribute transformation and validation for consistent behavior and fewer edge cases.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 3, 2025

📝 Walkthrough

Walkthrough

Centralizes attribute transformation and form validation into new utils, expands form value types to include booleans, updates components to use typed ModifiedFields and shared helpers, and strengthens GluuCommitDialog typing and array/boolean rendering.

Changes

Cohort / File(s) Summary
GluuCommitDialog array rendering
admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx, admin-ui/app/routes/Apps/Gluu/types.ts
Tightened prop typing (GluuCommitDialogProps, FormikProps<UserEditFormValues>), introduced JsonValue alias, safer theme/selectors, and improved array/boolean/empty-value rendering with collapse and Show More/Show Less behavior.
Form components (centralized attribute logic)
admin-ui/plugins/user-management/components/UserAddPage.tsx, admin-ui/plugins/user-management/components/UserEditPage.tsx, admin-ui/plugins/user-management/components/UserForm.tsx
Replaced in-file custom-attribute logic with buildCustomAttributesFromValues, updateCustomAttributesWithModifiedFields, getStandardFieldValues; adopted ModifiedFields typing; refactored submit/apply flows and attribute hydration.
New utilities & barrel export
admin-ui/plugins/user-management/utils/attributeTransformUtils.ts, admin-ui/plugins/user-management/utils/formValidationUtils.ts, admin-ui/plugins/user-management/utils/index.ts, admin-ui/plugins/user-management/utils/userFormUtils.ts
Added attribute transform helpers (normalize, build/update Jans custom attributes), form validation utilities (buildFormOperations, shouldDisableApplyButton, hasFormChanges), extended getStringValue/setupCustomAttributes for booleans, and re-exported via new utils index.
Type updates (form & modified fields)
admin-ui/plugins/user-management/types/ComponentTypes.ts, admin-ui/plugins/user-management/types/CommonTypes.ts, admin-ui/plugins/user-management/types/index.ts
Introduced UserEditFormValues, ModifiedFieldValue, ModifiedFields; allowed booleans in UserFormValues, FormOperation, and FormValueEntry; consolidated type definitions and updated props/signatures to use ModifiedFields.
Validation & hooks import consolidation
admin-ui/plugins/user-management/helper/validations.ts, admin-ui/plugins/user-management/hooks/usePasswordChange.ts, admin-ui/plugins/user-management/hooks/usePasswordValidation.ts, admin-ui/plugins/user-management/schemas/userFormSchema.ts
Redirected imports to utils barrel (e.g., validatePassword, getStringValue) and refined single-valued attribute handling to preserve booleans/strings rather than always JSON-stringifying.
Audit & redux typing
admin-ui/plugins/user-management/helper/userAuditHelpers.ts, admin-ui/app/redux/sagas/types/audit.ts, admin-ui/plugins/user-management/types/UserApiTypes.ts
Added CustomObjectAttribute import, extended AuditPayload (passwords, jsonPatchString), added AuditLogHeaders and webhookReducer to RootState, and minor comment cleanups.

Sequence Diagram(s)

sequenceDiagram
    participant UI as User Form (UserAdd/Edit/Form)
    participant Utils as user-management/utils
    participant API as JansConfigApi / Mutation

    rect rgb(210,230,255)
    Note over UI,Utils: Build payload from form state
    UI->>Utils: getStandardFieldValues(values, standardFields)
    UI->>Utils: buildCustomAttributesFromValues(values, personAttributes)
    Utils-->>UI: standardFieldsObj, customAttributes[]
    end

    rect rgb(230,255,230)
    Note over UI,Utils: Apply incremental edits
    UI->>Utils: updateCustomAttributesWithModifiedFields(existingAttrs, modifiedFields, personAttributes, originalUser)
    Utils-->>UI: reconciled customAttributes[]
    end

    rect rgb(255,240,220)
    Note over UI,API: Persist changes
    UI->>API: mutation.mutate({ ...standardFieldsObj, customAttributes })
    API-->>UI: Response (success / error)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60–90 minutes

  • Pay extra attention to:
    • attributeTransformUtils.ts (multi-valued/boolean/date normalization, edge cases)
    • ComponentTypes.ts (type propagation, union changes and downstream effects)
    • UserForm.tsx / UserEditPage.tsx / UserAddPage.tsx (Formik typing, ModifiedFields wiring, apply/submit flow)
    • utils/index.ts (barrel exports and potential import cycles)
    • GluuCommitDialog.tsx (new rendering helpers and JsonValue typing)

Possibly related PRs

Suggested labels

kind-bug

Suggested reviewers

  • syntrydy
  • moabu
  • duttarnab

Poem

🐇 I hopped through fields and nudged each type,

Arrays folded neatly, booleans tucked right,
Helpers stitched the attributes tight,
Commits now render with clearer light,
A carrot-coded cheer tonight 🥕

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Several changes extend beyond the immediate role removal fix: updates to Redux audit types, GluuCommitDialog typing, custom attribute transformation utilities, and form validation refactoring. While these support the core functionality, they represent significant architectural refactoring not strictly necessary for the stated issue. Clarify whether TypeScript refactoring (attribute transform utils, Redux audit types, form validation) and GluuCommitDialog changes are separate concerns that should be addressed in a dedicated PR, or document how they directly enable the role removal fix.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'Fix(admin-ui): roles not removable from user profile and apply action not triggered' clearly identifies the two main issues being addressed: role removal functionality and Apply button triggering. It is specific, concise, and directly summarizes the primary fixes.
Linked Issues check ✅ Passed The PR addresses the linked issue #2494 by implementing multiple fixes: proper state updates for role removal, form change-detection logic for Apply button triggering, UI binding updates for role operations, and correct update payloads. These changes align with the objectives of allowing role modification and ensuring persistence.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch admin-ui-issue-2494

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8a7483f and ee0f47b.

📒 Files selected for processing (2)
  • admin-ui/app/redux/sagas/types/audit.ts (2 hunks)
  • admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (6 hunks)
🧰 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/app/redux/sagas/types/audit.ts
📚 Learning: 2025-11-26T12:25:34.923Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2475
File: admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js:111-216
Timestamp: 2025-11-26T12:25:34.923Z
Learning: In the admin-ui codebase, avoid suggesting comments for small functionalities or straightforward layout decisions in smaller components. Comments should be reserved for more complex or non-obvious logic.

Applied to files:

  • admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx
🧬 Code graph analysis (2)
admin-ui/app/redux/sagas/types/audit.ts (1)
admin-ui/plugins/admin/redux/features/WebhookSlice.ts (1)
  • WebhookSliceState (6-27)
admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (4)
admin-ui/app/routes/Apps/Gluu/types.ts (1)
  • GluuCommitDialogProps (55-66)
admin-ui/plugins/admin/components/Mapping/MappingItem.js (2)
  • theme (8-8)
  • selectedTheme (9-9)
admin-ui/plugins/user-management/types/CommonTypes.ts (1)
  • ThemeContext (10-14)
admin-ui/app/redux/sagas/types/audit.ts (1)
  • RootState (41-44)
🔇 Additional comments (2)
admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (2)

47-52: Theme and selector typing changes are correct and align with store typings

Using theme?.state.theme || 'light' safely handles missing context and keeps a clear default, and typing isOpen as number | null prevents accidental boolean/string misuse for the collapse index.

The typed selector:

const { loadingWebhooks, webhookModal } = useSelector(
  (state: RootState) => state.webhookReducer,
)

is a good use of your centralized RootState from redux/sagas/types/audit, and matches the newly added webhookReducer entry there. No issues from a typing or behavior standpoint.


113-133: Array/boolean/object rendering now correctly surfaces role removals and complex values

The combination of renderArrayValue and the updated operations.map branch looks solid:

  • Empty arrays render as an explicit "" badge, which will make “all roles removed” clearly visible in the commit dialog.
  • Non‑empty arrays show the first two entries by default, with the rest available via a Collapse and a Show More/Show Less toggle, avoiding overflow while still exposing full detail when needed.
  • Booleans are rendered as stringified badges, and ''/null/undefined consistently show as "", giving a clear “cleared value” signal.
  • Plain objects are rendered via JSON.stringify, so you no longer get [object Object] in the UI.
  • The isOpen index plus key from operations.map provide a stable per‑row expansion toggle.

This directly supports the PR goal of making role removals (and other cleared multi‑value fields) visible and understandable in the commit dialog. The early if (!modal) return <></> keeps the component inert when closed and matches the existing modal gating pattern.

Also applies to: 135-137, 184-215


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mo-auto mo-auto added the comp-admin-ui Component affected by issue or PR label Dec 3, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

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/user-management/components/UserForm.tsx (1)

102-119: Potential stale state issue in handleCancel.

The handleCancel callback passes selectedClaims to setupCustomAttributes, but the state reset to [] happens on the next line (line 107). This means setupCustomAttributes receives the old selectedClaims array, which may cause the list to re-populate incorrectly.

Additionally, including selectedClaims in the dependency array (line 119) recreates this callback on every claim selection change, which may be unnecessary.

Consider using a ref or restructuring the flow:

   const handleCancel = useCallback(() => {
     const resetValues = initializeCustomAttributes(userDetails || null, memoizedPersonAttributes)
     formik.resetForm({ values: resetValues })
     setModifiedFields({})
     if (userDetails) {
-      setSelectedClaims([])
-      setupCustomAttributes(
-        userDetails,
-        memoizedPersonAttributes,
-        selectedClaims,
-        setSelectedClaims,
-      )
+      const freshClaims: PersonAttribute[] = []
+      setupCustomAttributes(
+        userDetails,
+        memoizedPersonAttributes,
+        freshClaims,
+        setSelectedClaims,
+      )
       initializedRef.current = userDetails.inum || 'new'
     } else {
       setSelectedClaims([])
       initializedRef.current = 'new'
     }
-  }, [formik, userDetails, memoizedPersonAttributes, selectedClaims, setSelectedClaims])
+  }, [formik, userDetails, memoizedPersonAttributes, setSelectedClaims])
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 923ba6f and 4adaddd.

📒 Files selected for processing (17)
  • admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (1 hunks)
  • admin-ui/plugins/user-management/components/UserAddPage.tsx (2 hunks)
  • admin-ui/plugins/user-management/components/UserEditPage.tsx (2 hunks)
  • admin-ui/plugins/user-management/components/UserForm.tsx (7 hunks)
  • admin-ui/plugins/user-management/helper/userAuditHelpers.ts (2 hunks)
  • admin-ui/plugins/user-management/helper/validations.ts (2 hunks)
  • admin-ui/plugins/user-management/hooks/usePasswordChange.ts (1 hunks)
  • admin-ui/plugins/user-management/hooks/usePasswordValidation.ts (1 hunks)
  • admin-ui/plugins/user-management/schemas/userFormSchema.ts (1 hunks)
  • admin-ui/plugins/user-management/types/CommonTypes.ts (1 hunks)
  • admin-ui/plugins/user-management/types/ComponentTypes.ts (2 hunks)
  • admin-ui/plugins/user-management/types/UserApiTypes.ts (0 hunks)
  • admin-ui/plugins/user-management/types/index.ts (0 hunks)
  • admin-ui/plugins/user-management/utils/attributeTransformUtils.ts (1 hunks)
  • admin-ui/plugins/user-management/utils/formValidationUtils.ts (1 hunks)
  • admin-ui/plugins/user-management/utils/index.ts (1 hunks)
  • admin-ui/plugins/user-management/utils/userFormUtils.ts (2 hunks)
💤 Files with no reviewable changes (2)
  • admin-ui/plugins/user-management/types/UserApiTypes.ts
  • admin-ui/plugins/user-management/types/index.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-06T08:23:20.948Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2418
File: admin-ui/plugins/user-management/components/UserForm.tsx:290-297
Timestamp: 2025-11-06T08:23:20.948Z
Learning: In the UserForm component (admin-ui/plugins/user-management/components/UserForm.tsx), the Back button should fall back to '/user/usersmanagement' when browser history is unavailable, not '/home/dashboard', as this keeps users in the user management context. This aligns with the new requirement that users should be redirected to the respective listing screen of the component rather than the dashboard.

Applied to files:

  • admin-ui/plugins/user-management/components/UserForm.tsx
  • admin-ui/plugins/user-management/components/UserEditPage.tsx
  • admin-ui/plugins/user-management/types/ComponentTypes.ts
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.

Applied to files:

  • admin-ui/plugins/user-management/helper/userAuditHelpers.ts
🧬 Code graph analysis (6)
admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (9)
admin-ui/plugins/saml/components/TrustRelationshipList.js (2)
  • item (30-30)
  • selectedTheme (26-26)
admin-ui/plugins/saml/components/SamlIdentityList.js (2)
  • item (49-49)
  • selectedTheme (52-52)
admin-ui/plugins/services/Components/Configuration/LdapListPage.js (2)
  • item (79-79)
  • selectedTheme (90-90)
admin-ui/plugins/services/Components/Configuration/SqlListPage.js (2)
  • item (77-77)
  • selectedTheme (86-86)
admin-ui/plugins/auth-server/components/Clients/ClientListPage.js (2)
  • item (99-99)
  • selectedTheme (57-57)
admin-ui/plugins/admin/components/Mapping/MappingItem.js (1)
  • selectedTheme (9-9)
admin-ui/plugins/auth-server/components/AuthN/AuthNListPage.js (1)
  • selectedTheme (30-30)
admin-ui/plugins/auth-server/components/Clients/ClientWizardForm.js (1)
  • selectedTheme (61-61)
admin-ui/plugins/services/Components/Configuration/PersistenceDetail.js (1)
  • selectedTheme (22-22)
admin-ui/plugins/user-management/components/UserForm.tsx (3)
admin-ui/plugins/user-management/types/ComponentTypes.ts (1)
  • ModifiedFields (16-16)
admin-ui/plugins/user-management/utils/formValidationUtils.ts (2)
  • buildFormOperations (17-23)
  • shouldDisableApplyButton (3-15)
admin-ui/plugins/user-management/utils/userFormUtils.ts (1)
  • setupCustomAttributes (105-152)
admin-ui/plugins/user-management/components/UserAddPage.tsx (2)
admin-ui/plugins/user-management/types/ComponentTypes.ts (1)
  • ModifiedFields (16-16)
admin-ui/plugins/user-management/utils/attributeTransformUtils.ts (1)
  • buildCustomAttributesFromValues (118-158)
admin-ui/plugins/user-management/utils/attributeTransformUtils.ts (3)
admin-ui/plugins/user-management/types/ComponentTypes.ts (4)
  • FormValueEntry (118-124)
  • ModifiedFieldValue (15-15)
  • UserEditFormValues (5-13)
  • ModifiedFields (16-16)
admin-ui/plugins/user-management/common/Constants.ts (2)
  • JANS_ADMIN_UI_ROLE_ATTR (4-4)
  • BIRTHDATE_ATTR (6-6)
admin-ui/plugins/user-management/types/UserApiTypes.ts (2)
  • PersonAttribute (5-5)
  • CustomUser (4-4)
admin-ui/plugins/user-management/utils/formValidationUtils.ts (1)
admin-ui/plugins/user-management/types/ComponentTypes.ts (1)
  • ModifiedFields (16-16)
admin-ui/plugins/user-management/types/ComponentTypes.ts (3)
admin-ui/plugins/auth-server/components/Scopes/types/formTypes.ts (1)
  • ModifiedFields (17-19)
admin-ui/plugins/user-management/types/UserApiTypes.ts (2)
  • PersonAttribute (5-5)
  • CustomUser (4-4)
admin-ui/plugins/user-management/types/CommonTypes.ts (1)
  • UserFormValues (6-8)
⏰ 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). (2)
  • GitHub Check: Analyze (javascript)
  • GitHub Check: sonar scan (admin-ui)
🔇 Additional comments (29)
admin-ui/plugins/user-management/types/CommonTypes.ts (1)

6-8: LGTM!

The addition of boolean to the UserFormValues index signature aligns with the corresponding UserEditFormValues interface in ComponentTypes.ts (line 12), ensuring consistent typing across form-related types. This supports the PR's goal of properly handling boolean attributes during role modifications.

admin-ui/plugins/user-management/types/ComponentTypes.ts (5)

18-25: LGTM!

The UserClaimEntryProps interface correctly uses the new ModifiedFields type for state management, ensuring consistent typing between the modified fields tracking and the state setter.


27-35: LGTM!

The updated onSubmitData signature properly uses UserEditFormValues and ModifiedFields, providing type-safe form submission handling that supports the PR's goal of tracking field modifications for proper role updates.


43-47: LGTM!

Adding boolean to the FormOperation.value type is consistent with the broader changes supporting boolean attributes in form operations.


118-124: LGTM!

The FormValueEntry union type extension to include boolean maintains consistency with the other type updates in this file.


5-16: ModifiedFields type is correctly defined for the actual usage pattern.

The new type definitions are sound. While ModifiedFields here (string | string[] | boolean) differs from the auth-server plugin definition which includes undefined | null, this reflects intentional design differences:

  • In user-management, field removal/clearing uses empty values: '' for strings, [] for arrays, and false for booleans
  • modifiedFields tracks explicitly set values via replace operations, not field deletions via nullification
  • The current type signature accurately matches this implementation

No changes needed.

admin-ui/plugins/user-management/utils/attributeTransformUtils.ts (7)

1-19: LGTM!

The imports and type definitions are well-organized. The ValueExtractionRecord interface properly captures the expected shape of complex form entries.


21-34: LGTM!

The extractValueFromEntry function correctly handles the special case for jansAdminUIRole attribute and gracefully falls back through role, value, label, and attribute name keys. This is essential for the PR's role removal fix.


36-53: LGTM!

Good use of strict date parsing with moment(normalized, 'YYYY-MM-DD', true) to ensure only valid dates are returned for the birthdate attribute.


80-94: LGTM!

The helper functions isBooleanAttribute, isMultiValuedAttribute, and hadOriginalValue are clean, focused utilities with appropriate null-safety.


96-110: LGTM!

The processMultiValuedField and processSingleValuedField functions correctly differentiate between multi-valued and single-valued attribute processing, with proper boolean handling in the single-valued case.


118-158: LGTM!

The buildCustomAttributesFromValues function comprehensively handles multi-valued, boolean, and single-valued attributes. The filtering at line 138 ensures empty values are excluded from multi-valued attributes.


211-218: LGTM!

The getStandardFieldValues function is a clean utility for extracting standard fields with proper normalization.

admin-ui/plugins/user-management/hooks/usePasswordValidation.ts (1)

4-4: LGTM!

The import path update to use the barrel export (../utils) aligns with the new utility module organization introduced in this PR.

admin-ui/plugins/user-management/hooks/usePasswordChange.ts (1)

13-13: LGTM!

Consistent with the barrel export pattern adopted in usePasswordValidation.ts.

admin-ui/plugins/user-management/schemas/userFormSchema.ts (1)

4-4: LGTM!

Import path updated to use the new barrel export, maintaining consistency with the refactored utility structure.

admin-ui/plugins/user-management/utils/userFormUtils.ts (1)

131-144: Boolean attribute handling correctly filters false values.

The logic properly handles boolean attributes by parsing values from various types (boolean, string, other) and excluding attributes with false values from the selected claims. This aligns with the PR objective to properly handle role/attribute removal.

admin-ui/plugins/user-management/helper/validations.ts (2)

6-6: LGTM!

Import path updated to use the centralized barrel export.


161-176: Proper type preservation for boolean and string attributes.

The explicit type checking ensures booleans remain as boolean type and strings remain as string type in the form values, rather than being incorrectly stringified. This is essential for correct form state management and the Apply button change detection.

admin-ui/plugins/user-management/utils/formValidationUtils.ts (3)

3-15: Verify: Apply button enabled with invalid form when modifiedFields exist.

The logic on line 13 only disables the button if the form is invalid AND there are no modifiedFields. This means the Apply button can be enabled even when the form is invalid, as long as modifiedFields has entries.

Is this intentional? If a user removes a role (adding to modifiedFields) but has invalid data in other fields, should they be able to submit?

If validation should always be enforced, consider:

   if (isSubmitting || !isFormChanged) return true
-  if (!hasModifiedFields && !isValid) return true
+  if (!isValid) return true
   return false

17-23: LGTM!

Clean utility for building JSON Patch-style operations from modified fields.


25-27: LGTM!

Simple and effective change detection combining Formik's dirty state with manual field tracking.

admin-ui/plugins/user-management/components/UserEditPage.tsx (2)

97-99: Verify modifiedFields payload format.

The transformation creates an array of single-key objects:

[{ key1: value1 }, { key2: value2 }]

This format seems unusual. Typically, modified fields would be either a flat object { key1: value1, key2: value2 } or an array of operation objects [{ path: 'key1', value: value1 }, ...].

Confirm this matches the expected backend API format. If a flat object is expected:

-          modifiedFields: Object.keys(modifiedFields).map((key) => ({
-            [key]: modifiedFields[key],
-          })),
+          modifiedFields: modifiedFields,

77-109: Clean refactoring to centralized utilities.

The submission logic is now more maintainable with clear separation: build base attributes, update with modifications, extract standard fields. The useCallback memoization with proper dependencies is correctly implemented.

admin-ui/plugins/user-management/components/UserAddPage.tsx (1)

48-65: LGTM!

The _modifiedFields parameter is correctly prefixed to indicate it's intentionally unused in the add flow (new users don't have prior state to modify). The submission logic cleanly uses the centralized buildCustomAttributesFromValues utility.

admin-ui/plugins/user-management/components/UserForm.tsx (3)

162-192: Core fix for role removal - properly tracks modifications.

This is the key fix for the PR objective. The removeSelectedClaimsFromState function now:

  1. Sets type-appropriate empty values (array [], boolean false, string '')
  2. Updates modifiedFields state to track the removal
  3. Triggers the Apply button by making the form appear "changed"

This ensures role removals are detected by the change-tracking logic and the Apply button activates accordingly.


457-462: Correct usage of the centralized shouldDisableApplyButton utility.

The Apply button state is now determined by combining Formik's built-in state (dirty, isValid) with the manual modifiedFields tracking, ensuring the button enables when roles are removed.


140-160: Initialization guard prevents infinite loops.

The initializedRef pattern with the combined ${userKey}-${attrsKey} key correctly prevents re-running the setup effect unnecessarily while still responding to meaningful changes (user or attributes data changes).

admin-ui/plugins/user-management/utils/index.ts (1)

1-3: Clean barrel export setup.

This centralizes utility exports effectively. All three modules export distinct names with no collisions detected.

duttarnab
duttarnab previously approved these changes Dec 3, 2025
moabu
moabu previously approved these changes Dec 4, 2025
@faisalsiddique4400 faisalsiddique4400 dismissed stale reviews from moabu and duttarnab via 8d073a7 December 4, 2025 09:52
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (1)

119-134: Array rendering helper is solid; watch out for non-unique badge keys

renderBadges/renderArrayValue nicely isolate the array rendering and handle the “empty array” case in a way that will make removed multi-valued fields (like roles) visible in the commit dialog, which aligns well with the PR objective.

One minor concern: key={String(data)} in renderBadges can easily collide for duplicate values or objects ([object Object]), which may cause React to reuse DOM nodes in surprising ways. If values can contain duplicates or objects, consider switching to an index‑based or composite key (e.g., pass a baseKey into renderBadges and use ${baseKey}-${idx}) to ensure uniqueness.

Also applies to: 135-155

♻️ Duplicate comments (1)
admin-ui/plugins/user-management/utils/attributeTransformUtils.ts (1)

183-183: Shallow copy mutates original attribute objects.

As noted in the previous review, line 183 creates a shallow copy with [...customAttributes], but modifications to existingAttr.values (lines 197, 211) mutate the original objects. If immutability is required, deep-clone the attributes:

-  const result = [...customAttributes]
+  const result = customAttributes.map(attr => ({ ...attr }))

Also applies to: 196-197, 208-211

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4adaddd and 8d073a7.

📒 Files selected for processing (4)
  • admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (5 hunks)
  • admin-ui/plugins/user-management/helper/userAuditHelpers.ts (3 hunks)
  • admin-ui/plugins/user-management/utils/attributeTransformUtils.ts (1 hunks)
  • admin-ui/plugins/user-management/utils/userFormUtils.ts (2 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/user-management/helper/userAuditHelpers.ts
🧬 Code graph analysis (3)
admin-ui/plugins/user-management/helper/userAuditHelpers.ts (2)
admin-ui/plugins/user-management/common/Constants.ts (1)
  • USER_PASSWORD_ATTR (7-7)
admin-ui/plugins/user-management/types/UserApiTypes.ts (1)
  • CustomUser (4-4)
admin-ui/plugins/user-management/utils/attributeTransformUtils.ts (3)
admin-ui/plugins/user-management/types/ComponentTypes.ts (3)
  • FormValueEntry (118-124)
  • ModifiedFieldValue (15-15)
  • ModifiedFields (16-16)
admin-ui/plugins/user-management/common/Constants.ts (2)
  • JANS_ADMIN_UI_ROLE_ATTR (4-4)
  • BIRTHDATE_ATTR (6-6)
admin-ui/plugins/user-management/types/UserApiTypes.ts (2)
  • PersonAttribute (5-5)
  • CustomUser (4-4)
admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (2)
admin-ui/app/routes/Apps/Gluu/types.ts (1)
  • GluuCommitDialogProps (54-65)
admin-ui/plugins/user-management/types/CommonTypes.ts (1)
  • ThemeContext (10-14)
🔇 Additional comments (8)
admin-ui/plugins/user-management/helper/userAuditHelpers.ts (3)

41-45: Good improvement: Centralized sensitive attribute list.

The introduction of SENSITIVE_CUSTOM_ATTRS and isSensitiveCustomAttr helper addresses extensibility concerns. Adding new sensitive attributes is now straightforward.


74-89: Redaction logic is sound with proper null guards.

The redaction correctly:

  1. Guards against missing/empty customAttributes
  2. Preserves non-sensitive attributes unchanged
  3. Maps sensitive attribute values to '[REDACTED]' while maintaining array structure
  4. Uses proper type casting for the final result

The as CustomUser['customAttributes'] cast at line 88 aligns the return type with the expected interface.


47-47: Type inference relies on conditional type extraction.

The CustomAttributeValueItem type uses conditional inference to extract the element type from CustomObjectAttribute['values']. This is a clean approach, but if values is undefined or not an array in the upstream type definition, this resolves to never.

#!/bin/bash
# Verify CustomObjectAttribute['values'] type structure
rg -n "CustomObjectAttribute" --type ts -A 10 | head -60
admin-ui/plugins/user-management/utils/userFormUtils.ts (1)

18-26: Array handling now returns first element.

The updated getStringValue addresses the previous review concern by returning value[0] ?? '' for arrays. Boolean values returning empty string is appropriate since this function is for string extraction.

admin-ui/plugins/user-management/utils/attributeTransformUtils.ts (4)

1-11: Well-organized utility module with clear separation of concerns.

The module cleanly extracts attribute transformation logic into focused, testable functions. The imports are appropriate and the type definitions align with the broader codebase patterns.


65-84: Boolean handling now implemented correctly.

The normalizeFieldValue function now explicitly handles booleans (lines 75-77), converting them to 'true' or 'false' strings. This addresses the previous review concern about boolean values becoming empty strings.


134-174: buildCustomAttributesFromValues is well-structured.

The function correctly:

  1. Uses the attribute map for O(1) lookups
  2. Handles multi-valued vs single-valued attributes appropriately
  3. Filters empty values from multi-valued arrays
  4. Normalizes boolean and date values correctly

227-234: Clean utility for extracting standard field values.

The getStandardFieldValues function properly normalizes form values for standard fields using Object.fromEntries with a clean mapping pattern.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c55303b and 8a7483f.

📒 Files selected for processing (1)
  • admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-26T12:25:34.923Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2475
File: admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js:111-216
Timestamp: 2025-11-26T12:25:34.923Z
Learning: In the admin-ui codebase, avoid suggesting comments for small functionalities or straightforward layout decisions in smaller components. Comments should be reserved for more complex or non-obvious logic.

Applied to files:

  • admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx
🔇 Additional comments (4)
admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (4)

47-50: Good defensive coding and type safety improvements.

The fallback to 'light' theme when context is unavailable and the explicit number | null typing for isOpen both strengthen the component's reliability.


106-142: Rendering helpers correctly handle all value types, including empty arrays.

The renderBadges and renderArrayValue functions properly handle:

  • Empty arrays → empty quotes badge (critical for role removal visibility)
  • Non-empty arrays → first 2 items with expand/collapse for longer lists
  • All values → JSON stringified for accurate display

This directly addresses the PR objective of making role removals visible in the commit dialog.


193-227: Operations rendering comprehensively handles all value types.

The updated rendering logic correctly distinguishes and displays:

  • Arrays → renderArrayValue with empty-array support
  • Booleans → stringified badge
  • Empty/null/undefined → empty quotes badge
  • Objects → JSON stringified badge
  • Primitives → stringified badge

This ensures all changes, including empty arrays from role removals, are visible to users as intended by the PR.


25-28: Local JsonValue type definition is appropriate; no shared type to import exists.

The GluuCommitDialogProps and RootState imports are correct. The local JsonValue type on line 28 should remain as defined—no shared JsonValue or GluuInputType types exist in the repository. While admin-ui/plugins/user-management/types/ComponentTypes.ts contains a FormValueEntry type, it differs structurally (non-recursive, lacks number type) and is unsuitable for this use case. The recursive JsonValue definition here is necessary for handling nested audit data structures.

duttarnab
duttarnab previously approved these changes Dec 5, 2025
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 5, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp-admin-ui Component affected by issue or PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(admin-ui): Unable to remove roles from user's profile

5 participants