-
Notifications
You must be signed in to change notification settings - Fork 19
Fix(admin-ui): roles not removable from user profile and apply action not triggered #2496
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughCentralizes attribute transformation and form validation into new utils, expands form value types to include booleans, updates components to use typed Changes
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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2025-11-07T12:55:26.241ZApplied to files:
📚 Learning: 2025-11-26T12:25:34.923ZApplied to files:
🧬 Code graph analysis (2)admin-ui/app/redux/sagas/types/audit.ts (1)
admin-ui/app/routes/Apps/Gluu/GluuCommitDialog.tsx (4)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 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 inhandleCancel.The
handleCancelcallback passesselectedClaimstosetupCustomAttributes, but the state reset to[]happens on the next line (line 107). This meanssetupCustomAttributesreceives the oldselectedClaimsarray, which may cause the list to re-populate incorrectly.Additionally, including
selectedClaimsin 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
📒 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.tsxadmin-ui/plugins/user-management/components/UserEditPage.tsxadmin-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
booleanto theUserFormValuesindex signature aligns with the correspondingUserEditFormValuesinterface inComponentTypes.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
UserClaimEntryPropsinterface correctly uses the newModifiedFieldstype for state management, ensuring consistent typing between the modified fields tracking and the state setter.
27-35: LGTM!The updated
onSubmitDatasignature properly usesUserEditFormValuesandModifiedFields, providing type-safe form submission handling that supports the PR's goal of tracking field modifications for proper role updates.
43-47: LGTM!Adding
booleanto theFormOperation.valuetype is consistent with the broader changes supporting boolean attributes in form operations.
118-124: LGTM!The
FormValueEntryunion type extension to includebooleanmaintains consistency with the other type updates in this file.
5-16:ModifiedFieldstype is correctly defined for the actual usage pattern.The new type definitions are sound. While
ModifiedFieldshere (string | string[] | boolean) differs from the auth-server plugin definition which includesundefined | null, this reflects intentional design differences:
- In user-management, field removal/clearing uses empty values:
''for strings,[]for arrays, andfalsefor booleansmodifiedFieldstracks 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
ValueExtractionRecordinterface properly captures the expected shape of complex form entries.
21-34: LGTM!The
extractValueFromEntryfunction correctly handles the special case forjansAdminUIRoleattribute and gracefully falls back throughrole,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, andhadOriginalValueare clean, focused utilities with appropriate null-safety.
96-110: LGTM!The
processMultiValuedFieldandprocessSingleValuedFieldfunctions correctly differentiate between multi-valued and single-valued attribute processing, with proper boolean handling in the single-valued case.
118-158: LGTM!The
buildCustomAttributesFromValuesfunction 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
getStandardFieldValuesfunction 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
falsevalues 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
booleantype and strings remain asstringtype 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 whenmodifiedFieldsexist.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 asmodifiedFieldshas 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: VerifymodifiedFieldspayload 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
useCallbackmemoization with proper dependencies is correctly implemented.admin-ui/plugins/user-management/components/UserAddPage.tsx (1)
48-65: LGTM!The
_modifiedFieldsparameter 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 centralizedbuildCustomAttributesFromValuesutility.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
removeSelectedClaimsFromStatefunction now:
- Sets type-appropriate empty values (array
[], booleanfalse, string'')- Updates
modifiedFieldsstate to track the removal- 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 centralizedshouldDisableApplyButtonutility.The Apply button state is now determined by combining Formik's built-in state (
dirty,isValid) with the manualmodifiedFieldstracking, ensuring the button enables when roles are removed.
140-160: Initialization guard prevents infinite loops.The
initializedRefpattern 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.
8d073a7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 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/renderArrayValuenicely 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)}inrenderBadgescan easily collide for duplicate values or objects ([object Object]), which may cause React to reuse DOM nodes in surprising ways. Ifvaluescan contain duplicates or objects, consider switching to an index‑based or composite key (e.g., pass abaseKeyintorenderBadgesand 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 toexistingAttr.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
📒 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_ATTRSandisSensitiveCustomAttrhelper addresses extensibility concerns. Adding new sensitive attributes is now straightforward.
74-89: Redaction logic is sound with proper null guards.The redaction correctly:
- Guards against missing/empty
customAttributes- Preserves non-sensitive attributes unchanged
- Maps sensitive attribute values to
'[REDACTED]'while maintaining array structure- 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
CustomAttributeValueItemtype uses conditional inference to extract the element type fromCustomObjectAttribute['values']. This is a clean approach, but ifvaluesisundefinedor not an array in the upstream type definition, this resolves tonever.#!/bin/bash # Verify CustomObjectAttribute['values'] type structure rg -n "CustomObjectAttribute" --type ts -A 10 | head -60admin-ui/plugins/user-management/utils/userFormUtils.ts (1)
18-26: Array handling now returns first element.The updated
getStringValueaddresses the previous review concern by returningvalue[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
normalizeFieldValuefunction 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:buildCustomAttributesFromValuesis well-structured.The function correctly:
- Uses the attribute map for O(1) lookups
- Handles multi-valued vs single-valued attributes appropriately
- Filters empty values from multi-valued arrays
- Normalizes boolean and date values correctly
227-234: Clean utility for extracting standard field values.The
getStandardFieldValuesfunction properly normalizes form values for standard fields usingObject.fromEntrieswith a clean mapping pattern.
admin-ui/plugins/user-management/utils/attributeTransformUtils.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/app/routes/Apps/Gluu/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 explicitnumber | nulltyping forisOpenboth strengthen the component's reliability.
106-142: Rendering helpers correctly handle all value types, including empty arrays.The
renderBadgesandrenderArrayValuefunctions 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 →
renderArrayValuewith 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
GluuCommitDialogPropsandRootStateimports are correct. The localJsonValuetype on line 28 should remain as defined—no sharedJsonValueorGluuInputTypetypes exist in the repository. Whileadmin-ui/plugins/user-management/types/ComponentTypes.tscontains aFormValueEntrytype, it differs structurally (non-recursive, lacksnumbertype) and is unsuitable for this use case. The recursiveJsonValuedefinition here is necessary for handling nested audit data structures.
|



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
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
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.