feat(admin-ui): revamp User Management module as per Figma#2695
Merged
Conversation
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:
📝 WalkthroughWalkthroughAdds theming tokens and color tokens for Available Claims, removes the attributes Redux slice and saga, introduces a MultiValueSelectCard and many theme-aware style hooks, replaces several modals with portal-based implementations (webhook/commit/password), refactors user-management forms and list/table flows, expands types/hooks, updates locales, and adds tests and test helpers. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User (UI)
participant C as UserFormCommitDialog
participant H as useWebhookDialogAction
participant M as Webhook Modal (portal)
participant S as Server/API
U->>C: Click "Apply"
C->>H: Open webhook flow (request check)
H->>M: Render portal modal (overlay + table)
M->>U: Show webhook options
U->>M: Accept webhook trigger
M->>H: Confirm acceptance
H->>S: Call webhook trigger API
S-->>H: Respond (success/failure)
H-->>C: webhookCheckComplete
C->>U: Show commit dialog
U->>C: Confirm commit
C->>S: Submit user update API
S-->>C: Update response
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![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: 31
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
admin-ui/plugins/user-management/components/User2FADevicesModal.tsx (1)
199-225:⚠️ Potential issue | 🟠 MajorDon't remove OTP rows locally before the update succeeds.
The OTP row is filtered out immediately after
updateUserData()is fired. If the PUT fails, the device disappears from the modal even though the backend still has it.Move the local
setOTPDevicesListupdate into the success path, or add rollback logic inonError.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/components/User2FADevicesModal.tsx` around lines 199 - 225, The current handleRemove2Fa optimistically calls setOTPDevicesList right after updateUserData, causing the UI to drop the OTP row even if the PUT fails; change this so the local state is updated only on a successful server response—either move the setOTPDevicesList call into the success callback/then-handler of updateUserData (or the onSuccess of the mutation wrapper) or implement onError rollback logic that restores the previous OTP list; refer to handleRemove2Fa, updateUserData, setOTPDevicesList and the jansOTPDevices construction to locate where to apply the change.admin-ui/plugins/user-management/types/CommonTypes.ts (1)
1-8:⚠️ Potential issue | 🔴 CriticalImport statement must be at the top of the file.
The
importstatement on line 6 appears after theRoleinterface declaration. TypeScript requires all imports to be at the top of the file before any other declarations. This will cause a compilation error.🐛 Proposed fix to move import to top
+import type { JsonValue } from 'Routes/Apps/Gluu/types/common' + +export type FormFieldValue = JsonValue | undefined + export interface Role { role: string inum?: string } -import type { JsonValue } from 'Routes/Apps/Gluu/types/common' - -export type FormFieldValue = JsonValue | undefined - export interface UserFormValues {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/types/CommonTypes.ts` around lines 1 - 8, Move the import of JsonValue to the top of the file before any declarations: place "import type { JsonValue } from 'Routes/Apps/Gluu/types/common'" above the export interface Role declaration so TypeScript imports come first; then ensure the Role interface and the export type FormFieldValue remain unchanged and reference JsonValue correctly.admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
89-103:⚠️ Potential issue | 🟠 MajorMove session-revocation logic before the success navigation.
The update success handler redirects immediately via
navigateBack(), which is synchronous, while the revoke-session calls only run afterwards in thesubmitDatacallback. The user sees the success message and leaves the page before session revocation is attempted, making failures here silent and the old session potentially still active.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/components/UserEditPage.tsx` around lines 89 - 103, The session-revocation must run and be awaited before navigating away on a successful update: move the revoke-session logic that currently lives in submitData into the updateUserMutation onSuccess handler (the same place that calls logUserUpdate and triggerUserWebhook), call the existing revoke function (e.g., revokeUserSessions/revokeSessions) before invoking navigateBack(ROUTES.USER_MANAGEMENT), await it, and handle errors (dispatch an error toast or log) so failures are not silent.admin-ui/plugins/user-management/helper/userAuditHelpers.ts (2)
28-37:⚠️ Potential issue | 🔴 CriticalRedact passwords inside
modifiedFieldstoo.
AuditPayloadnow carries nestedmodifiedFields, butredactSensitiveData()only masks top-level password fields. This PR's user form recordsuserPassword/userConfirmPasswordinmodifiedFields, so those values can still reach audit logs in plaintext.🛡️ Proposed fix
const redactSensitiveData = (payload: AuditPayload): void => { + if (payload.modifiedFields) { + for (const key of ['userPassword', 'userConfirmPassword', USER_PASSWORD_ATTR]) { + if (key in payload.modifiedFields) { + payload.modifiedFields[key] = '[REDACTED]' + } + } + } + if (payload.userPassword) { payload.userPassword = '[REDACTED]' } if (payload.userConfirmPassword) { payload.userConfirmPassword = '[REDACTED]'Also applies to: 40-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/helper/userAuditHelpers.ts` around lines 28 - 37, The redactSensitiveData function in userAuditHelpers.ts currently only masks top-level password properties (userPassword, userConfirmPassword) but must also redact those keys when they appear inside modifiedFields (AuditPayload.modifiedFields) or any nested object there; update redactSensitiveData (or a helper it calls) to walk modifiedFields and replace values for keys "userPassword" and "userConfirmPassword" (and any other sensitive keys used elsewhere) with a mask, handling string or string[] values and preserving structure, so audit logs never contain plaintext passwords.
37-50:⚠️ Potential issue | 🟠 MajorDo not overwrite every JSON patch with a fake password change.
Any truthy
jsonPatchStringis replaced with a hard-coded/userPasswordpatch, even when the user only changedstatus, or another field. That destroys the real audit trail and can falsely imply a password update. Parse the patch and redact only sensitive operations.🧾 Proposed fix
if (payload.jsonPatchString) { - payload.jsonPatchString = '[{"op":"replace","path":"/userPassword","value":"[REDACTED]"}]' + try { + const sensitivePaths = new Set(['/userPassword', '/userConfirmPassword']) + const operations = JSON.parse(payload.jsonPatchString) as Array<{ + op?: string + path?: string + value?: unknown + }> + + payload.jsonPatchString = JSON.stringify( + operations.map((operation) => + operation.path && sensitivePaths.has(operation.path) + ? { ...operation, value: '[REDACTED]' } + : operation, + ), + ) + } catch { + payload.jsonPatchString = '[REDACTED]' + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/helper/userAuditHelpers.ts` around lines 37 - 50, redactSensitiveData currently replaces any truthy jsonPatchString with a hard-coded password patch, which destroys the real audit trail; update redactSensitiveData to parse payload.jsonPatchString as JSON (in the redactSensitiveData function), iterate over the patch array and only replace the value for operations whose path targets the sensitive field(s) (e.g., "/userPassword" or other password-related paths) with "[REDACTED]", leaving all other ops unchanged, and fall back to a safe behavior (e.g., leave the original string or set to a generic "[REDACTED]" marker) if parsing fails; also keep existing logic for payload.userPassword and payload.userConfirmPassword redaction.admin-ui/plugins/user-management/components/UserForm.tsx (2)
188-203:⚠️ Potential issue | 🟠 MajorRemove reverted fields from
modifiedFields.Both modified-fields helpers only drop entries when the new value is empty. If a user changes a field and then restores the original non-empty value,
modifiedFieldsstill reports it as changed, so Apply/Cancel stay enabled and the commit dialog/audit payload can include changes that no longer exist. Compare against the initial form value before keeping the entry.Also applies to: 225-240
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/components/UserForm.tsx` around lines 188 - 203, updateModifiedFields currently only removes entries when the new value is empty, so revert-to-original non-empty values remain marked; modify updateModifiedFields to also compare the incoming value against the form's initial value (e.g., initialValues or initialFormValues) and remove the key from setModifiedFields when they are equal (use deep equality for arrays/strings/booleans), otherwise keep/update the entry; apply the same change to the other modified-fields helper referenced in the diff so both functions consult initial form state and drop entries when the new value matches the original.
60-67:⚠️ Potential issue | 🟠 MajorStop memoizing
personAttributesbehind a partial key.This key only tracks the array length and the first attribute name. If any other attribute metadata changes,
memoizedPersonAttributesstays stale and the form keeps using outdated claim definitions for initialization, reset, and remove flows. Either depend on the full prop here or derive the key from all attributes that affect form behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/components/UserForm.tsx` around lines 60 - 67, The memoization uses a partial key (personAttributesKey) so memoizedPersonAttributes can become stale; update the memoization to depend on the full personAttributes data instead of only length and first name. Replace the derived key approach for personAttributesKey/memoizedPersonAttributes by either directly using personAttributes in the useMemo dependency array or computing a stable key that serializes all relevant fields (e.g., mapping the attributes to the exact fields used by the form) and use that key so memoizedPersonAttributes always updates when any attribute metadata that affects initialization/reset/remove changes.
🤖 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/locales/es/translation.json`:
- Line 16: The value for the translation key "add_attribute_mapping" contains an
accidental concatenation ("AceptarAgregar mapeo de atributos"); update the
translation JSON entry for the "add_attribute_mapping" key to the correct
Spanish string "Agregar mapeo de atributos" (replace the current value so the
key remains "add_attribute_mapping" and only the value changes).
In `@admin-ui/app/locales/pt/translation.json`:
- Line 343: The translation for the key "firstName" in the locales file
currently reuses the generic "Nome"; change the value for the "firstName" key in
admin-ui/app/locales/pt/translation.json to a distinct Portuguese label such as
"Primeiro nome" (or "Nome próprio") so first-name fields are distinguishable
from the generic "name" label.
In `@admin-ui/app/routes/Apps/Gluu/GluuRemovableInputRow.tsx`:
- Around line 105-106: The onKeyDown currently calls handler() for any key;
update GluuRemovableInputRow's keyboard handling so onKeyDown inspects the
KeyboardEvent and only invokes handler when the key is Enter or Space (e.g.,
e.key === 'Enter' || e.key === ' '), and call e.preventDefault() for Space to
avoid page scroll; keep the existing onClick handler unchanged.
In `@admin-ui/app/routes/Apps/Gluu/MultiValueSelectCard.tsx`:
- Around line 132-145: The remove-field element in the MultiValueSelectCard
component uses a div with role="button" (classes.removeFieldButton) which lacks
native button semantics; replace that div with a real <button type="button">
element, move the onClick={onRemoveField} and remove the onKeyDown handler
(native button handles Enter/Space), keep the aria-label and className for
styling, and remove role and tabIndex attributes so the remover is accessible
and behaves correctly.
- Around line 84-124: The Add/Remove controls and tag delete buttons still allow
mutations even when the component is read-only because the `disabled` state is
only applied to the autocomplete; update the mutation paths so they respect the
same `disabled` flag: pass the `disabled` prop (or computed isDisabled boolean)
into the GluuButton instances (the Add button and Remove button) and into each
tag's remove button, set the native disabled attribute and appropriate
aria-disabled, and guard the handlers `handleAdd`, `handleRemove`, and
`handleRemoveByName` to no-op if disabled to prevent programmatic clicks from
mutating state.
In `@admin-ui/app/routes/Apps/Gluu/styles/GluuCommitDialog.style.ts`:
- Line 51: Replace the inline arithmetic "magic number" adjustments with named
local constants or explanatory comments in GluuCommitDialog.style.ts: define
top-of-file constants (e.g., CONTENT_BUTTONS_PADDING =
CEDARLING_CONFIG_SPACING.BUTTONS_MT + 5, CHECKBOX_LABEL_GAP_ADJUST =
MAPPING_SPACING.CHECKBOX_LABEL_GAP - 1, BORDER_RADIUS_SMALL_ADJUST =
BORDER_RADIUS.SMALL - 2, CHECKBOX_LABEL_GAP_PLUS =
MAPPING_SPACING.CHECKBOX_LABEL_GAP + 3) and use those constants where padding,
gap, and radius are set, or add short comments next to each occurrence
referencing why the adjustment is needed; update occurrences that reference
CEDARLING_CONFIG_SPACING.BUTTONS_MT, MAPPING_SPACING.CHECKBOX_LABEL_GAP, and
BORDER_RADIUS.SMALL so the intent and maintainability are clear.
In `@admin-ui/app/routes/Apps/Gluu/styles/GluuInputRow.style.ts`:
- Around line 15-20: The current colWrapper style removes input focus indicators
by setting outline and boxShadow to none; update the '& input:focus, &
input:active' rule inside GluuInputRow.style.ts to provide a visible, accessible
focus style (e.g., a clear high-contrast outline or box-shadow ring) instead of
removing it so keyboard users retain a location cue; locate the colWrapper
definition and replace the suppression with a descriptive focus style applied to
the same selector so inputs inside colWrapper show a distinct focus indicator.
In `@admin-ui/app/utils/hooks/useWebhookDialogAction.tsx`:
- Around line 141-148: Add proper modal focus management to the dialog: set
aria-modal="true" on the container, attach a ref (e.g., modalRef) and on open
call modalRef.current.focus() while storing document.activeElement to restore on
close, and ensure the container remains focusable (tabIndex={-1}) so the Escape
handler always fires; implement focus trapping inside handleModalKeyDown (or a
new trapKeydown handler) to intercept Tab and Shift+Tab and cycle focus among
focusable elements within the dialog, and restore focus to the previously
focused element when the dialog unmounts/ closes. Use the existing
handleModalKeyDown name or add a helper that runs onKeyDown for the same element
and keep aria-labelledby="webhook-modal-title" as-is.
In `@admin-ui/app/utils/styles/WebhookTriggerModal.style.ts`:
- Around line 60-64: Replace the hardcoded 'minWidth: 650' in the table wrapper
style inside WebhookTriggerModal.style.ts with a named constant (e.g.,
WEBHOOK_TABLE_MIN_WIDTH); declare and export that constant at the top of the
file (or alongside other style constants) and use it in place of the literal
value so the table width is configurable/maintainable, mirroring the approach
used for MODAL_WIDTH in GluuCommitDialog.style.ts; update any imports or
references if you move the constant to a shared constants file.
- Line 69: Replace the magic "+ 3" in the gap expression with a named constant
(e.g., CHECKBOX_LABEL_EXTRA_GAP = 3) or add an inline comment explaining why 3px
is needed; update the gap to use MAPPING_SPACING.CHECKBOX_LABEL_GAP +
CHECKBOX_LABEL_EXTRA_GAP and export or colocate the constant near the
WebhookTriggerModal style so future maintainers see the intent.
In `@admin-ui/plugins/user-management/components/AvailableClaimsPanel.tsx`:
- Around line 50-62: The current visibleOptions memo returns [] when
searchClaims is empty, which clears the panel; change the behavior so an empty
search shows all eligible claims by replacing the early `if (searchTrimmed ===
'') return []` logic with returning personAttributes filtered by the same
eligibility checks (status active, not in USED_CLAIMS, not already in
selectedClaims). Update the matching logic in the filter (inside visibleOptions)
so matchesSearch is true for empty search (e.g., treat empty searchLower as a
wildcard) or compute the full eligibility filter and only apply
name.includes(searchLower) when searchLower is non-empty; keep references to
visibleOptions, searchClaims, personAttributes, selectedClaims, and USED_CLAIMS.
In `@admin-ui/plugins/user-management/components/PasswordChangeModal.style.ts`:
- Around line 72-89: Add a visible keyboard focus state to the toggleButton
style so keyboard users see when the control is focused: update the toggleButton
style (the object keyed by "toggleButton") to include a focus rule such as
'&:focus, &:focus-visible' that sets a clear visual indicator (e.g., outline:
'2px solid <accessible color>' or boxShadow using an accessible theme color like
themeColors.primary) and remove/reduce default outline if necessary; ensure the
rule maintains existing hover/opacity behavior and uses themeColors for
consistent styling.
- Around line 62-65: The current '&:focus' rule in PasswordChangeModal.style.ts
removes the outline/boxShadow without an alternative visual cue; update the
focus styling for the input/style object that contains '&:focus' to add a subtle
but high-contrast indicator (for example change borderColor to a theme focus
color and/or add a faint boxShadow like '0 0 0 3px rgba(...)') and prefer using
':focus-visible' alongside ':focus' so keyboard focus is highlighted while mouse
interactions remain unchanged; keep outline: 'none' if you add the alternative
indicator and ensure the chosen color meets contrast/accessibility guidelines.
In `@admin-ui/plugins/user-management/components/PasswordChangeModal.tsx`:
- Around line 292-299: The password visibility toggle in PasswordChangeModal
currently uses tabIndex={-1} which removes it from keyboard focus; update the
toggle button(s) (the element using class formClasses.toggleButton and the state
handlers showPassword / setShowPassword) to be keyboard-accessible by removing
tabIndex={-1} or setting tabIndex={0} so the control is in the tab order; apply
the same change to the other toggle instance around lines 327-334 that uses the
same class/handlers.
- Around line 176-186: The effect currently caches passwordFormik.resetForm in
resetFormRef which can cause Formik focus/registration bugs; remove resetFormRef
entirely and call passwordFormik.resetForm() directly inside the useEffect
cleanup branch that runs when isOpen is false, and update the dependency array
to include passwordFormik (matching how handleCancel and handleFormSubmit use
it) so the effect depends on the full passwordFormik object rather than an
extracted method.
In `@admin-ui/plugins/user-management/components/User2FADevicesModal.style.ts`:
- Around line 19-21: The variable accentGreen (assigned from
customColors.claimsSelectionBorder) is poorly named for its limited use in
pagination button colors; rename it to a clearer identifier like
paginationAccentColor and update all references where accentGreen is used (in
this file's styling for pagination buttons) so the assignment reads const
paginationAccentColor = customColors.claimsSelectionBorder and styling uses
paginationAccentColor instead of accentGreen.
- Around line 24-27: Replace hardcoded borderRadius values (currently set to 12)
with the shared constant BORDER_RADIUS.DEFAULT: update the call to
getCardBorderStyle({ isDark, borderRadius: 12 }) to use borderRadius:
BORDER_RADIUS.DEFAULT and similarly replace any other occurrences in this file
where 12 is used for border radius (e.g., styles that set borderRadius
directly). Add the import for BORDER_RADIUS from its module at the top of the
file and ensure references are updated consistently across modalCardBorderStyle
and other style definitions in this component.
In `@admin-ui/plugins/user-management/components/User2FADevicesModal.tsx`:
- Around line 302-306: The GluuTable is hardcoded with loading={false}, causing
the table to show an empty state while the FIDO2 query is still fetching;
replace the literal with the actual fetch state from the FIDO2 query hook (e.g.,
use the isLoading or isFetching boolean returned by the hook that provides
paginatedData) so GluuTable's loading prop reflects the real request status
(update the GluuTable usage in User2FADevicesModal.tsx to pass that hook's
loading flag instead of false).
In `@admin-ui/plugins/user-management/components/UserAddPage.tsx`:
- Around line 63-76: submitData is dropping the form's last/surname and middle
name when building the create payload; include the missing fields (e.g., sn and
middleName or the exact property names used elsewhere) into submitableValues
before calling createUserMutation.mutate so those values aren't lost, and ensure
types align with CustomUser (update the CustomUser shape or cast if necessary);
locate submitData and buildCustomAttributesFromValues to add sn: values.sn || ''
and middleName: values.middleName || '' (or the exact field names used in this
module) to the payload.
In `@admin-ui/plugins/user-management/components/UserClaimEntry.tsx`:
- Around line 58-64: The placeholder strings in UserClaimEntry.tsx (the
conditional using isRolesUnavailable and rolesLoading to choose 'Loading
roles...', 'Failed to load roles', or 'Search Here') are hard-coded; route them
through the app i18n instead — import/consume the project's translation helper
(e.g., useTranslation or i18n.t) and replace those literals with translated
keys/strings (e.g., t('userManagement.loadingRoles'),
t('userManagement.failedLoadRoles'), t('userManagement.searchHere')) so the
placeholder expression uses t(...) for each branch while keeping the same
isRolesUnavailable and rolesLoading logic.
- Around line 39-45: handleMultiValueChange closes over the modifiedFields
variable which can cause lost updates when multiple updates happen quickly;
change the setModifiedFields call inside handleMultiValueChange to use a
functional updater that receives the previous state and returns the new state
(i.e., setModifiedFields(prev => ({ ...prev, [data.name]: next }))) and keep the
rest of the callback logic (formik.setFieldValue and formik.setFieldTouched)
intact; update the dependency array if needed to remove modifiedFields so the
callback no longer closes over the stale value.
In `@admin-ui/plugins/user-management/components/UserDetailViewPage.style.ts`:
- Around line 4-7: The interface UserDetailViewPageStylesParams currently
declares an unused isDark property; remove isDark from
UserDetailViewPageStylesParams (leaving themeColors: ThemeConfig) and update any
references that construct or type this params object (e.g., places that pass an
object to UserDetailViewPageStyles or similar style factory) so they no longer
include isDark, ensuring the style factory/type signatures and imports remain
consistent.
In `@admin-ui/plugins/user-management/components/UserDetailViewPage.tsx`:
- Around line 54-63: The fields array in UserDetailViewPage (created inside the
useMemo) currently shows the raw internal label "jansAdminUIRole:"; replace that
hardcoded string with a localized label by calling t(...) (e.g.,
t('fields.jansAdminUIRole') or an appropriate key) so the entry becomes { label:
`${t('...')}:`, value: roleValue }; update any i18n resource key accordingly and
ensure the dependency array still includes t and roleValue.
In `@admin-ui/plugins/user-management/components/UserForm.tsx`:
- Line 329: In the UserForm component, replace hardcoded placeholder and status
strings ("Enter Here", "Search Here", "Loading roles...", "Failed to load
roles") with i18n lookups using the existing translation keys used elsewhere in
the form (use the same placeholder keys for input placeholders and add new keys
for role loading/error messages), and update the locales files to include the
new keys (e.g., userForm.placeholder.*, userForm.roles.loading,
userForm.roles.error) so all occurrences in UserForm.tsx use t('...') instead of
literal strings.
In `@admin-ui/plugins/user-management/components/UserFormCommitDialog.tsx`:
- Around line 38-39: The commit dialog is shown before the async webhook
decision settles because webhookModal defaults to false; introduce an explicit
"webhookCheckComplete" boolean (component state or derived prop) and set it true
only when useWebhookDialogAction finishes resolving (or immediately if
canReadWebhooks is false/not applicable), then change the showCommitDialog logic
to use modal && webhookCheckComplete && !showWebhookFirst so GluuCommitDialog
only renders after the webhook check has settled; update useWebhookDialogAction
invocation to set webhookCheckComplete on both success/failure paths to avoid
race conditions with GluuCommitDialog.
In `@admin-ui/plugins/user-management/components/UserFormPage.style.ts`:
- Line 15: The call to getCardBorderStyle({ isDark }) omits the borderRadius and
can diverge from the explicit BORDER_RADIUS.DEFAULT used in formCard; update the
cardBorderStyle invocation to include borderRadius: BORDER_RADIUS.DEFAULT (e.g.,
getCardBorderStyle({ isDark, borderRadius: BORDER_RADIUS.DEFAULT })) so
cardBorderStyle and the formCard borderRadius are consistent, or if the explicit
override on formCard was intentional remove its borderRadius to rely solely on
getCardBorderStyle.
In `@admin-ui/plugins/user-management/components/UserListPage.style.ts`:
- Around line 42-66: The tableCard style duplicates borderRadius already
provided by cardBorderStyle; remove the explicit 'borderRadius':
BORDER_RADIUS.DEFAULT from the tableCard object (or keep it only if an
intentional override is required) so that cardBorderStyle supplies the radius
consistently—update the tableCard definition in UserListPage.style.ts to
eliminate the redundant borderRadius entry while leaving cardBorderStyle,
tableCard, and other keys intact.
- Around line 24-35: The searchCard style duplicates borderRadius:
BORDER_RADIUS.DEFAULT even though cardBorderStyle (from getCardBorderStyle)
already applies borderRadius; remove the explicit borderRadius entry in the
searchCard object to avoid redundancy and rely on cardBorderStyle, keeping the
reference to BORDER_RADIUS.DEFAULT only where getCardBorderStyle is invoked.
In `@admin-ui/plugins/user-management/helper/validations.ts`:
- Around line 122-132: The toDateLike function's parameter should be typed as
unknown (not object) and narrowed inside the function to correctly handle
primitives/string/number/Date; update toDateLike to accept unknown, check types
using typeof and instanceof, and handle the { value } shape safely by iterating
or recursing with a depth limit and/or a visited Set to prevent infinite
recursion or stack overflow (e.g., bail after a max depth or when revisiting the
same wrapper), while keeping the return type string | number | Date | null;
update usages that call toDateLike (rawDate computation that uses attrValues and
attrSingleValue) if necessary to satisfy the new signature.
In `@admin-ui/plugins/user-management/hooks/useUserMutations.ts`:
- Around line 31-33: The audit logging catch currently only prints to console
when isDevelopment, which hides production failures; update the catch on
logUserDeletion(inum, userData) to always record the failure for production by
(1) calling the app's monitoring/error-reporting function (e.g.,
captureException/reportError/monitoringClient) with the auditError and context
(inum, userData), and (2) dispatching a user-facing non-blocking warning (e.g.,
showToast or dispatch({ type: 'toast', level: 'warning', ... })) so operators
are alerted; keep the existing console.error for dev but ensure both monitoring
and a toast are invoked unconditionally in the catch block.
In `@admin-ui/plugins/user-management/utils/userFormUtils.ts`:
- Around line 147-155: The boolean coercion incorrectly treats any object
wrapper as truthy; update the boolValue calculation in userFormUtils.ts (where
firstValue is derived from customAttr.values and used when isBoolean) to handle
object-shaped values explicitly: if typeof firstValue === 'object' and
firstValue !== null, check for a boolean-carrying property (e.g., 'value' or
'val') and use that boolean if present (preserving actual false), otherwise fall
back to the existing string/Boolean handling; ensure this change keeps the
existing branches for typeof firstValue === 'boolean' and typeof firstValue ===
'string' and only special-cases object wrappers so wrapped false is not coerced
to true.
---
Outside diff comments:
In `@admin-ui/plugins/user-management/components/User2FADevicesModal.tsx`:
- Around line 199-225: The current handleRemove2Fa optimistically calls
setOTPDevicesList right after updateUserData, causing the UI to drop the OTP row
even if the PUT fails; change this so the local state is updated only on a
successful server response—either move the setOTPDevicesList call into the
success callback/then-handler of updateUserData (or the onSuccess of the
mutation wrapper) or implement onError rollback logic that restores the previous
OTP list; refer to handleRemove2Fa, updateUserData, setOTPDevicesList and the
jansOTPDevices construction to locate where to apply the change.
In `@admin-ui/plugins/user-management/components/UserEditPage.tsx`:
- Around line 89-103: The session-revocation must run and be awaited before
navigating away on a successful update: move the revoke-session logic that
currently lives in submitData into the updateUserMutation onSuccess handler (the
same place that calls logUserUpdate and triggerUserWebhook), call the existing
revoke function (e.g., revokeUserSessions/revokeSessions) before invoking
navigateBack(ROUTES.USER_MANAGEMENT), await it, and handle errors (dispatch an
error toast or log) so failures are not silent.
In `@admin-ui/plugins/user-management/components/UserForm.tsx`:
- Around line 188-203: updateModifiedFields currently only removes entries when
the new value is empty, so revert-to-original non-empty values remain marked;
modify updateModifiedFields to also compare the incoming value against the
form's initial value (e.g., initialValues or initialFormValues) and remove the
key from setModifiedFields when they are equal (use deep equality for
arrays/strings/booleans), otherwise keep/update the entry; apply the same change
to the other modified-fields helper referenced in the diff so both functions
consult initial form state and drop entries when the new value matches the
original.
- Around line 60-67: The memoization uses a partial key (personAttributesKey) so
memoizedPersonAttributes can become stale; update the memoization to depend on
the full personAttributes data instead of only length and first name. Replace
the derived key approach for personAttributesKey/memoizedPersonAttributes by
either directly using personAttributes in the useMemo dependency array or
computing a stable key that serializes all relevant fields (e.g., mapping the
attributes to the exact fields used by the form) and use that key so
memoizedPersonAttributes always updates when any attribute metadata that affects
initialization/reset/remove changes.
In `@admin-ui/plugins/user-management/helper/userAuditHelpers.ts`:
- Around line 28-37: The redactSensitiveData function in userAuditHelpers.ts
currently only masks top-level password properties (userPassword,
userConfirmPassword) but must also redact those keys when they appear inside
modifiedFields (AuditPayload.modifiedFields) or any nested object there; update
redactSensitiveData (or a helper it calls) to walk modifiedFields and replace
values for keys "userPassword" and "userConfirmPassword" (and any other
sensitive keys used elsewhere) with a mask, handling string or string[] values
and preserving structure, so audit logs never contain plaintext passwords.
- Around line 37-50: redactSensitiveData currently replaces any truthy
jsonPatchString with a hard-coded password patch, which destroys the real audit
trail; update redactSensitiveData to parse payload.jsonPatchString as JSON (in
the redactSensitiveData function), iterate over the patch array and only replace
the value for operations whose path targets the sensitive field(s) (e.g.,
"/userPassword" or other password-related paths) with "[REDACTED]", leaving all
other ops unchanged, and fall back to a safe behavior (e.g., leave the original
string or set to a generic "[REDACTED]" marker) if parsing fails; also keep
existing logic for payload.userPassword and payload.userConfirmPassword
redaction.
In `@admin-ui/plugins/user-management/types/CommonTypes.ts`:
- Around line 1-8: Move the import of JsonValue to the top of the file before
any declarations: place "import type { JsonValue } from
'Routes/Apps/Gluu/types/common'" above the export interface Role declaration so
TypeScript imports come first; then ensure the Role interface and the export
type FormFieldValue remain unchanged and reference JsonValue correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 04d0792d-9767-43b9-b03c-d82ac20394ac
📒 Files selected for processing (58)
admin-ui/app/context/theme/config.tsadmin-ui/app/customColors.tsadmin-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/redux/actions/index.tsadmin-ui/app/redux/features/attributesSlice.tsadmin-ui/app/redux/reducers/index.tsadmin-ui/app/redux/sagas/AttributesSaga.tsadmin-ui/app/redux/sagas/index.tsadmin-ui/app/redux/types/index.tsadmin-ui/app/routes/Apps/Gluu/GluuInputRow.tsxadmin-ui/app/routes/Apps/Gluu/GluuRemovableInputRow.tsxadmin-ui/app/routes/Apps/Gluu/GluuTabs.tsxadmin-ui/app/routes/Apps/Gluu/GluuViewDetailsModal.tsxadmin-ui/app/routes/Apps/Gluu/MultiValueSelectCard.tsxadmin-ui/app/routes/Apps/Gluu/styles/GluuCommitDialog.style.tsadmin-ui/app/routes/Apps/Gluu/styles/GluuInputRow.style.tsadmin-ui/app/routes/Apps/Gluu/styles/MultiValueSelectCard.style.tsadmin-ui/app/routes/Apps/Gluu/types/GluuCommitDialog.tsadmin-ui/app/routes/Apps/Gluu/types/MultiValueSelectCard.types.tsadmin-ui/app/utilities.tsxadmin-ui/app/utils/hooks/useWebhookDialogAction.tsxadmin-ui/app/utils/styles/WebhookTriggerModal.style.tsadmin-ui/plugins/admin/components/Assets/types/FormTypes.tsadmin-ui/plugins/auth-server/components/Sessions/SessionListPage.tsxadmin-ui/plugins/saml/components/WebsiteSsoIdentityProviderForm.tsxadmin-ui/plugins/saml/components/WebsiteSsoServiceProviderForm.tsxadmin-ui/plugins/user-management/components/AvailableClaimsPanel.style.tsadmin-ui/plugins/user-management/components/AvailableClaimsPanel.tsxadmin-ui/plugins/user-management/components/PasswordChangeModal.style.tsadmin-ui/plugins/user-management/components/PasswordChangeModal.tsxadmin-ui/plugins/user-management/components/User2FADevicesModal.style.tsadmin-ui/plugins/user-management/components/User2FADevicesModal.tsxadmin-ui/plugins/user-management/components/UserAddPage.tsxadmin-ui/plugins/user-management/components/UserClaimEntry.tsxadmin-ui/plugins/user-management/components/UserDetailViewPage.style.tsadmin-ui/plugins/user-management/components/UserDetailViewPage.tsxadmin-ui/plugins/user-management/components/UserDeviceDetailViewPage.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/components/UserFormCommitDialog.tsxadmin-ui/plugins/user-management/components/UserFormPage.style.tsadmin-ui/plugins/user-management/components/UserList.tsxadmin-ui/plugins/user-management/components/UserListPage.style.tsadmin-ui/plugins/user-management/helper/userAuditHelpers.tsadmin-ui/plugins/user-management/helper/userWebhookHelpers.tsadmin-ui/plugins/user-management/helper/validations.tsadmin-ui/plugins/user-management/hooks/index.tsadmin-ui/plugins/user-management/hooks/useUserMutations.tsadmin-ui/plugins/user-management/types/CommonTypes.tsadmin-ui/plugins/user-management/types/ComponentTypes.tsadmin-ui/plugins/user-management/types/ErrorTypes.tsadmin-ui/plugins/user-management/types/index.tsadmin-ui/plugins/user-management/utils/attributeTransformUtils.tsadmin-ui/plugins/user-management/utils/userFormUtils.ts
💤 Files with no reviewable changes (5)
- admin-ui/app/redux/sagas/index.ts
- admin-ui/app/redux/actions/index.ts
- admin-ui/app/redux/reducers/index.ts
- admin-ui/app/redux/sagas/AttributesSaga.ts
- admin-ui/app/redux/features/attributesSlice.ts
faisalsiddique4400
changed the title
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
…ex into admin-ui-issue-2630
Contributor
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
admin-ui/plugins/saml/components/WebsiteSsoIdentityProviderForm.tsx (1)
438-575: 🧹 Nitpick | 🔵 TrivialConsider standardizing error display logic across all fields.
The form uses two different patterns for showing validation errors:
- Fields like
name,displayName, anddescriptionuse theshouldShowErrorcallback, which shows errors when:error && (touched || submitCount > 0 || value has content)- Fields in the manual metadata section use inline Boolean expressions, which show errors when:
error && (touched || submitCount > 0)The
shouldShowErrorcallback has an additional condition that displays errors when a value has content, even if untouched. The inline expressions lack this third condition, creating inconsistent error display behavior.♻️ Refactor to use shouldShowError consistently
Replace all inline Boolean
showErrorexpressions with calls to theshouldShowErrorcallback. For example:<GluuInputRow label="fields.idp_entity_id" name="idpEntityId" value={formik.values.idpEntityId || ''} formik={formik} required={!formik.values.metaDataFileImportedFlag} lsize={4} rsize={8} - showError={Boolean( - formik.errors.idpEntityId && - (formik.touched.idpEntityId || formik.submitCount > 0), - )} + showError={shouldShowError('idpEntityId')} errorMessage={formik.errors.idpEntityId} disabled={viewOnly} doc_category={DOC_SECTION} doc_entry="entityId" />Apply the same pattern to all other fields using inline Boolean expressions (nameIDPolicyFormat, singleSignOnServiceUrl, singleLogoutServiceUrl, signingCertificate, encryptionPublicKey, principalAttribute, principalType).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/saml/components/WebsiteSsoIdentityProviderForm.tsx` around lines 438 - 575, The error display is inconsistent: replace the inline Boolean showError expressions for the manual metadata fields with the existing shouldShowError helper so all fields use the same logic. For each GluuSelectRow/GluuInputRow that currently uses showError={Boolean(...)} (specifically nameIDPolicyFormat, singleSignOnServiceUrl, singleLogoutServiceUrl, signingCertificate, encryptionPublicKey, principalAttribute, principalType and any similar fields), change showError to call shouldShowError with the field name (e.g., showError={shouldShowError('singleSignOnServiceUrl')}) so the same touched/submitCount/value-present logic in shouldShowError is applied consistently across WebsiteSsoIdentityProviderForm.admin-ui/plugins/auth-server/components/Sessions/SessionListPage.tsx (1)
576-707:⚠️ Potential issue | 🟡 MinorFinish localizing the rest of this toolbar/filter block.
Line 588 now uses
t('fields.username'), but the same UI still renders hard-coded English at Line 635 (Columns), Line 676 (Search Filter), Line 682 (None), and Line 707 (Value). That leaves the revamped page partially untranslated in non-English locales.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/auth-server/components/Sessions/SessionListPage.tsx` around lines 576 - 707, The toolbar/filter block in SessionListPage.tsx still contains hard-coded English strings ("Columns", "Search Filter", "None", "Value"); update those UI labels to use the i18n translator t(...) like the other fields (e.g., replace the MaterialButton text in the handleColumnMenuOpen block, the TextField select label inside the filter (select for searchFilter), the MenuItem with value "" and the fallback TextField label "Value") and add corresponding translation keys to your locale resource files so the new keys render in non-English locales.admin-ui/app/locales/es/translation.json (1)
1074-1107:⚠️ Potential issue | 🟡 MinorAdd the missing
placeholders.search_heretranslation.
UserClaimEntrynow readsplaceholders.search_here, but this locale only definessearchandsearch_claims_here, so that placeholder stays untranslated in Spanish.♻️ Suggested fix
"placeholders": { "id": "Introduce el id", "search": "Buscar", + "search_here": "Buscar aquí", "action_commit_message": "Indica el motivo de este cambio",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/locales/es/translation.json` around lines 1074 - 1107, Add the missing Spanish translation for the new key placeholders.search_here referenced by UserClaimEntry: add an entry "placeholders.search_here": "Buscar aquí" (or equivalent Spanish phrasing) into the translation JSON alongside the existing "search" and "search_claims_here" keys so the placeholder is properly localized.admin-ui/plugins/user-management/components/User2FADevicesModal.tsx (1)
87-95:⚠️ Potential issue | 🟠 MajorRollback the optimistic OTP removal on update failure.
Line 224 removes the OTP row locally before the PUT completes, but this error path only shows a toast. If the update fails, the modal stays in a "deleted" state even though the server kept the device.
admin-ui/plugins/user-management/helper/userAuditHelpers.ts (1)
52-65:⚠️ Potential issue | 🟠 MajorPreserve the original
customAttributes.valuesshape when redacting.This turns each value into
{ value: '[REDACTED]' }and then hides the schema change with the cast on Line 65. If the audit payload serializer/backend still expects the normal scalar array, password-related audit logs will start failing. Keep the redacted array in the same shape as the source data.Suggested fix
if (payload.customAttributes && payload.customAttributes.length > 0) { payload.customAttributes = payload.customAttributes.map((attr) => { if (isSensitiveCustomAttr(attr.name)) { const redactedValues = attr.values && Array.isArray(attr.values) - ? attr.values.map(() => ({ value: '[REDACTED]' })) + ? attr.values.map(() => '[REDACTED]') : undefined return { ...attr, values: redactedValues, } } return attr - }) as CustomUser['customAttributes'] + }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/helper/userAuditHelpers.ts` around lines 52 - 65, The redaction logic in the payload.customAttributes mapping (inside the block that checks payload.customAttributes and uses isSensitiveCustomAttr) changes the shape of attr.values to an array of { value: '[REDACTED]' } objects and then force-casts it to CustomUser['customAttributes']; instead, preserve the original values shape by mapping each existing scalar value to the string '[REDACTED]' (or leaving undefined if attr.values was not an array), so attr.values remains an array of strings (or undefined) and no cast is required.admin-ui/plugins/user-management/components/UserForm.tsx (1)
60-67:⚠️ Potential issue | 🟠 MajorDrop the custom
personAttributesKeymemoization.This key only tracks
lengthand the first attribute name, so changes to any other attribute definition leavememoizedPersonAttributes, initial values, and selected claims stale. The parent already memoizespersonAttributes; use that array directly, or derive a key from the full collection if you truly need one.🩹 Proposed fix
- const personAttributesKey = useMemo( - () => - personAttributes.length > 0 - ? `${personAttributes.length}-${personAttributes[0]?.name || ''}` - : 'empty', - [personAttributes.length, personAttributes[0]?.name], - ) - const memoizedPersonAttributes = useMemo(() => personAttributes, [personAttributesKey]) + const memoizedPersonAttributes = personAttributes🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/components/UserForm.tsx` around lines 60 - 67, The custom personAttributesKey and its weak tracking cause stale memoizedPersonAttributes; remove personAttributesKey and change memoizedPersonAttributes to use the full personAttributes dependency (e.g., replace the two useMemo calls with a single line that either returns personAttributes directly or uses useMemo(() => personAttributes, [personAttributes])); update any code that references memoizedPersonAttributes to keep behavior unchanged.admin-ui/plugins/user-management/components/UserAddPage.tsx (1)
48-53:⚠️ Potential issue | 🟠 MajorDon't let audit logging block the successful create flow.
By the time this callback runs, the user has already been created.
await logUserCreation(...)can still prevent cache invalidation and navigation after the success toast fires, leaving the UI stranded on the form even though the create succeeded. Treat audit logging as a separately handled side effect instead of gating the main success path on it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/components/UserAddPage.tsx` around lines 48 - 53, In the onSuccess callback, don't block the main success path on audit logging: remove the await that gates the flow (the call to logUserCreation in onSuccess) and invoke logUserCreation as a fire-and-forget side effect (e.g., call it without awaiting and attach error handling so it won't throw back into onSuccess); keep dispatch(updateToast(...)), triggerUserWebhook(...), queryClient.invalidateQueries(getGetUserQueryKey()), and navigateBack(ROUTES.USER_MANAGEMENT) on the main path so cache invalidation and navigation always run even if logUserCreation fails.
♻️ Duplicate comments (2)
admin-ui/app/routes/Apps/Gluu/MultiValueSelectCard.tsx (1)
139-150:⚠️ Potential issue | 🟠 MajorDisabled cards can still remove the whole field.
The add/remove/tag actions now honor
disabled, but the field-level remove button does not, so a read-only card can still mutate the form state.Proposed fix
<button type="button" className={classes.removeFieldButton} onClick={onRemoveField} + disabled={disabled} aria-label="Remove field" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/routes/Apps/Gluu/MultiValueSelectCard.tsx` around lines 139 - 150, The remove-field button in MultiValueSelectCard.tsx currently always renders and calls onRemoveField even when the component is disabled; change the button to honor the disabled prop by adding the disabled attribute and preventing the click handler from invoking onRemoveField when disabled (e.g. replace onClick={onRemoveField} with onClick={() => { if (!disabled) onRemoveField(); }}), and update ARIA state (aria-disabled or remove aria-label) as appropriate so classes.removeFieldButton and onRemoveField are no longer active when disabled.admin-ui/plugins/user-management/components/AvailableClaimsPanel.tsx (1)
50-62:⚠️ Potential issue | 🟠 MajorEmpty searches should restore the full available-claims list.
Clearing the query still returns
[], so the panel goes blank and admins cannot browse/select claims again until they type another character. An empty query should mean “show all eligible claims.”Suggested fix
const visibleOptions = useMemo(() => { const searchTrimmed = searchClaims.trim() - if (searchTrimmed === '') return [] const searchLower = searchTrimmed.toLowerCase() return personAttributes.filter((data: PersonAttribute) => { const name = data.displayName?.toLowerCase() || '' const alreadyAddedClaim = selectedClaims.some((el: PersonAttribute) => el.name === data.name) const isActive = data.status?.toLowerCase() === 'active' const notUsed = !USED_CLAIMS.has(data.name) - const matchesSearch = name.includes(searchLower) + const matchesSearch = searchLower === '' || name.includes(searchLower) return Boolean(isActive && notUsed && matchesSearch && !alreadyAddedClaim) }) }, [personAttributes, searchClaims, selectedClaims])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/components/AvailableClaimsPanel.tsx` around lines 50 - 62, The current useMemo (visibleOptions) returns an empty array when searchClaims is empty; change the logic so an empty search returns all eligible claims instead of []; specifically, in the visibleOptions computation (useMemo) when searchTrimmed === '' return personAttributes.filter(...) applying the same eligibility checks (isActive: data.status?.toLowerCase() === 'active', notUsed: !USED_CLAIMS.has(data.name), and not alreadyAddedClaim: !selectedClaims.some(el => el.name === data.name)) rather than []. Preserve the existing matchesSearch check only for non-empty queries (i.e., compute matchesSearch = name.includes(searchLower) and include it in the predicate when searchTrimmed !== '') so admins see the full available list on empty queries.
🤖 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/locales/es/translation.json`:
- Around line 589-590: There are two definitions of the fields.username key in
translation.json causing a shadowed entry; remove the earlier/first
fields.username definition and keep only the intended one ("Nombre de usuario")
so the fields object contains a single username property, and verify no other
duplicate keys exist in the same fields block.
In `@admin-ui/app/locales/pt/translation.json`:
- Around line 926-930: The translation for the key "audit_logging_failed"
currently contains a hardcoded "Usuário excluído com sucesso" message; change
this to an operation-neutral Portuguese string (e.g., "A ação foi concluída, mas
o registro de auditoria falhou.") or alternatively split into explicit keys per
operation (e.g., "audit_logging_failed_create", "audit_logging_failed_update",
"audit_logging_failed_delete") and update all usages to reference the new keys;
ensure consistency between the key name(s) and the code paths that display the
toast so the message matches the actual CRUD operation.
In `@admin-ui/app/routes/Apps/Gluu/GluuInputRow.tsx`:
- Around line 151-156: Replace the hardcoded aria-label on the password toggle
button with a localized, state-specific label that changes based on the current
customType (e.g., when customType indicates password is hidden use the "show"
label, and when visible use the "hide" label). Use your app's i18n helper (e.g.,
t or translate) inside the component to produce keys like "password.show" and
"password.hide", update the button with aria-label={t(customType === 'password'
? 'password.show' : 'password.hide')} (or equivalent), and keep the existing
onClick handler (setVisibility) and className (classes.passwordToggle). Ensure
the translation keys exist and that the toggle reflects the new label whenever
customType changes.
In `@admin-ui/app/routes/Apps/Gluu/GluuRemovableInputRow.tsx`:
- Around line 97-117: The clickable remove control currently uses a div with
role="button" and custom onKeyDown which lacks tab focus and an accessible name;
replace that div with a semantic <button type="button"> (preserving the inline
style spread of applicationStyle.removableInputRow, width/height/padding, and
the inner <i> icon), remove the custom onKeyDown (native button handles
Enter/Space), keep the onClick calling handler, and add an aria-label (e.g.,
aria-label="Remove") so screen readers can announce the control; ensure you
still reference themeColors.fontColor for the icon styling.
In `@admin-ui/app/routes/Apps/Gluu/GluuViewDetailsModal.tsx`:
- Line 9: The component GluuViewDetailsModal currently falls back to a
2FA-specific title (t('messages.2FA_details')) when the title prop is omitted;
update the component so it no longer uses a 2FA-specific default: in the
GluuViewDetailsModal props (title?: string) and where title is used (lines
around 24-35), remove the t('messages.2FA_details') fallback and replace it with
a generic fallback (e.g. t('messages.details')) or require title when
customHeader is not provided; ensure references are to the title prop, the
GluuViewDetailsModal component, and the customHeader prop so consumers receive a
correct, generic header.
In `@admin-ui/app/routes/Apps/Gluu/MultiValueSelectCard.tsx`:
- Around line 21-22: The component MultiValueSelectCard currently hard-codes
English strings (e.g., default placeholder = 'Search Here', button labels, and
remove aria-labels); update it so default UI text is localized by either calling
the translation function t(...) where defaults are defined or by accepting
localized label props (e.g., placeholderLabel, addButtonLabel, removeAriaLabel)
and falling back to t('...') when props are not provided; change all occurrences
referenced in this file (including lines around the placeholder default and the
button/aria-label usages at the blocks around 100-113 and 128-147) to use these
localized values so the component displays translated text in non-English
locales.
- Around line 38-47: handleAdd currently accepts any typed string and your
Autocomplete flips between controlled/uncontrolled; update handleAdd to reject
additions when allowCustom is false unless the trimmed pendingValue exists in
options (e.g., if (!allowCustom && !options.includes(trimmed)) return), and keep
the existing duplicate/empty checks; additionally ensure the Autocomplete
inputValue prop is only controlled when allowCustom is true (pass allowCustom ?
pendingValue : undefined) to avoid controlled/uncontrolled flips—apply the same
validation logic to the other add/submit paths referenced (lines around 71-79
and 91-93) so only allowed option values can be added when allowCustom is false,
referencing handleAdd, pendingValue, selectedItems, options, allowCustom, and
the Autocomplete inputValue prop.
In `@admin-ui/plugins/user-management/components/AvailableClaimsPanel.tsx`:
- Around line 72-82: The search input in AvailableClaimsPanel is unlabeled for
assistive tech; update the input with an accessible name by either adding a
visible <label> tied to id "availableClaimsSearch" or adding an aria-label
(e.g., aria-label={t('placeholders.search_claims_here')}) so screen readers can
identify it; keep existing id, value (searchClaims), and onChange
(handleSearchChange) intact and ensure the label text is localized via t(...) if
you add a visible label.
In `@admin-ui/plugins/user-management/components/PasswordChangeModal.tsx`:
- Around line 292-297: The aria-label strings for the password visibility toggle
in PasswordChangeModal.tsx are hard-coded; update them to use the translation
function (e.g., t(...)) so they are localized. In the toggle button where
setShowPassword and showPassword are used (className formClasses.toggleButton),
replace the literal 'Hide password' / 'Show password' with t('...') keys and add
the corresponding locale entries; do the same for the second toggle instance
around the other toggle (the block using setShowPassword/showPassword at the
later occurrence).
- Around line 235-257: The dialog container in PasswordChangeModal (modalContent
/ commitClasses.modalContainer) must implement full focus management like
useWebhookDialogAction: on open save document.activeElement, set focus to the
dialog container (e.g., via ref on the div), add a keydown listener to trap Tab
(cycle focus among focusable elements inside the dialog) and intercept Escape to
call handleCancel (or submitChangePassword/handleCommitDialogCancel as
appropriate), and on close remove the listener and restore the previously
focused element; mirror the exact focus/ref/key handling logic from
useWebhookDialogAction (useEffect that runs when isOpen changes, a ref for the
modal container, Tab-trap logic, Escape handling, and cleanup/restoration) and
wire the existing handlers (handleCancel, handleOverlayKeyDown,
handleModalKeyDown) to use that centralized focus-management implementation.
- Around line 76-83: The success toast and modal-close calls inside the mutation
onSuccess (where dispatch(updateToast(...)), setPasswordModal(false), toggle(),
setPassword(''), and onSuccess?.()) are executed before the session revocation
steps; move those UI success actions to after the revocation calls (the
functions that perform session revocation referenced later in this file) so the
toast/close only happen when revoke operations complete successfully, and if
revocation fails surface a warning/error (dispatch a warning/error toast) and
keep the modal open (or prevent closing) so the user can retry; ensure you
locate and update the mutation onSuccess handler and the revocation functions so
success UI is only triggered after revokeSessions (and any revoke-by-cookie or
revoke-by-user-id helpers) resolve.
In `@admin-ui/plugins/user-management/components/User2FADevicesModal.style.ts`:
- Around line 55-60: The .btn-close styles force a white close icon via color:
customColors.white and filter: 'brightness(0) invert(1)', which hides the glyph
when the header background (modalBg) is light; remove the hard-coded white/color
filter or make it conditional: either drop the color and filter so the close
button inherits color (use color: 'inherit' or no color) or apply the white
color/filter only when the header is dark (e.g., check the header theme/variable
used to render the modal header and add a dark-header selector or flag). Update
the rule for '.btn-close' in User2FADevicesModal.style.ts to stop forcing white
on light headers and ensure the close icon remains visible.
In `@admin-ui/plugins/user-management/components/UserClaimEntry.tsx`:
- Around line 74-75: In UserClaimEntry.tsx the single-value label JSX is falling
back to an empty string instead of the claim name; update the label props to use
data.displayName || data.name (instead of ''), so single-value items show
data.name when displayName is missing; apply the same change to the other
occurrence near the multi-value branch (both label usages in the UserClaimEntry
component).
In `@admin-ui/plugins/user-management/components/UserDetailViewPage.tsx`:
- Around line 9-10: Remove the unnecessary DOMPurify sanitization: delete the
sanitizeValue helper and stop calling it where label/values are rendered
(references to sanitizeValue in UserDetailViewPage and the render at the
locations noted). Instead render the raw string values as plain JSX text nodes
(let React handle escaping). Ensure any tests or usages that import/call
sanitizeValue are updated/removed and no other code depends on the DOMPurify
call.
In `@admin-ui/plugins/user-management/components/UserEditPage.tsx`:
- Around line 89-98: The onSuccess handler in updateUserMutation (usePutUser)
currently shows the success toast and navigates away before session revocation
completes; move the forced session revocation call used in submitData (the
revoke/sessions mutate or revokeForcedSessions/revokeUserSessions function
invoked after mutateAsync) into this onSuccess path and await it before
dispatching updateToast, closing modal, triggering webhooks, invalidating
queries, or calling navigateBack; also handle and log errors from the revocation
(not silently) so a revoke failure is visible, and apply the same change to the
similar handler around lines 142-158.
In `@admin-ui/plugins/user-management/components/UserFormCommitDialog.tsx`:
- Around line 12-18: The dialog always hardcodes adminUiFeatures.users_edit when
calling useWebhookDialogAction, preventing the create flow from using the
correct webhook feature; add a prop (e.g., webhookFeature: keyof typeof
adminUiFeatures or webhookFeature: string) to UserFormCommitDialogProps and
thread it through the component, then replace the direct
adminUiFeatures.users_edit reference in the component where
useWebhookDialogAction is invoked with the new webhookFeature prop so callers
(UserForm add vs edit) can pass the appropriate feature; update all call-sites
to provide the correct feature value.
In `@admin-ui/plugins/user-management/components/UserList.tsx`:
- Around line 75-80: The pageNumber state can remain stale when the result set
becomes empty; inside the logic that computes effectivePage (used for GluuTable)
ensure you also update/clamp the stateful pageNumber variable to effectivePage
regardless of totalItems value — i.e., after computing effectivePage (and not
only when totalItems > 0) set pageNumber = effectivePage so the startIndex
passed into useGetUser ({ startIndex: pageNumber * limit }) stays in sync with
the displayed page.
In `@admin-ui/plugins/user-management/helper/userAuditHelpers.ts`:
- Around line 159-161: The catch block in the user deletion audit helper
(userAuditHelpers.ts) currently logs the error and rethrows it, which breaks the
delete flow; change the handler so it only logs the failure (include error
details) and swallows the exception instead of rethrowing — i.e., remove the
throw in the catch for the user-deletion audit function (the catch that contains
console.error('Failed to log user deletion:', error)) so the helper becomes
best-effort and lets the caller decide how to surface partial-success.
In `@admin-ui/plugins/user-management/hooks/useUserMutations.ts`:
- Around line 30-31: The console.error in the catch for logUserDeletion is
leaking the full CustomUser (userData) to client logs; change the error logging
to include only the error (auditError) and a stable identifier (inum) and remove
the full userData payload—update the catch in useUserMutations.ts where
logUserDeletion(inum, userData) is awaited so it logs something like "Audit
logging failed", auditError, { inum } (or any minimal non-sensitive fields), but
do not include userData or any email/claims.
- Around line 28-44: The deletion mutation (mutation.mutateAsync) should be
isolated from post-delete side effects so a failure in logUserDeletion,
triggerUserWebhook, queryUtils.invalidateQueriesByKey, or
callbacksRef.current?.onSuccess?.() doesn’t get reported as a deletion failure:
wrap mutation.mutateAsync({ inum }) in its own try/catch and treat that catch as
the true delete failure (dispatch error toast, call
callbacksRef.current?.onError, rethrow or return as currently intended); after a
successful mutateAsync, run logUserDeletion, triggerUserWebhook,
queryUtils.invalidateQueriesByKey, dispatch(updateToast(... success ...)), and
callbacksRef.current?.onSuccess?.() each inside their own try/catch blocks (or
at least catch and handle errors as non-fatal warnings) so side-effect errors
log/audit or show a warning toast but do not trigger the delete failure path or
call onError.
In `@admin-ui/plugins/user-management/utils/userFormUtils.ts`:
- Around line 40-45: The ternary that computes rawDate uses a truthy check on
attrSingleValue which drops numeric zero; update the condition in the rawDate
assignment to use an explicit null/undefined check (e.g., attrSingleValue !=
null) so numeric values like 0 are passed into toDateLike; locate the rawDate
expression (and related variables attrValues, attrSingleValue, toDateLike) and
replace the truthy check with a null/undefined check to ensure zeros reach
toDateLike.
---
Outside diff comments:
In `@admin-ui/app/locales/es/translation.json`:
- Around line 1074-1107: Add the missing Spanish translation for the new key
placeholders.search_here referenced by UserClaimEntry: add an entry
"placeholders.search_here": "Buscar aquí" (or equivalent Spanish phrasing) into
the translation JSON alongside the existing "search" and "search_claims_here"
keys so the placeholder is properly localized.
In `@admin-ui/plugins/auth-server/components/Sessions/SessionListPage.tsx`:
- Around line 576-707: The toolbar/filter block in SessionListPage.tsx still
contains hard-coded English strings ("Columns", "Search Filter", "None",
"Value"); update those UI labels to use the i18n translator t(...) like the
other fields (e.g., replace the MaterialButton text in the handleColumnMenuOpen
block, the TextField select label inside the filter (select for searchFilter),
the MenuItem with value "" and the fallback TextField label "Value") and add
corresponding translation keys to your locale resource files so the new keys
render in non-English locales.
In `@admin-ui/plugins/saml/components/WebsiteSsoIdentityProviderForm.tsx`:
- Around line 438-575: The error display is inconsistent: replace the inline
Boolean showError expressions for the manual metadata fields with the existing
shouldShowError helper so all fields use the same logic. For each
GluuSelectRow/GluuInputRow that currently uses showError={Boolean(...)}
(specifically nameIDPolicyFormat, singleSignOnServiceUrl,
singleLogoutServiceUrl, signingCertificate, encryptionPublicKey,
principalAttribute, principalType and any similar fields), change showError to
call shouldShowError with the field name (e.g.,
showError={shouldShowError('singleSignOnServiceUrl')}) so the same
touched/submitCount/value-present logic in shouldShowError is applied
consistently across WebsiteSsoIdentityProviderForm.
In `@admin-ui/plugins/user-management/components/UserAddPage.tsx`:
- Around line 48-53: In the onSuccess callback, don't block the main success
path on audit logging: remove the await that gates the flow (the call to
logUserCreation in onSuccess) and invoke logUserCreation as a fire-and-forget
side effect (e.g., call it without awaiting and attach error handling so it
won't throw back into onSuccess); keep dispatch(updateToast(...)),
triggerUserWebhook(...), queryClient.invalidateQueries(getGetUserQueryKey()),
and navigateBack(ROUTES.USER_MANAGEMENT) on the main path so cache invalidation
and navigation always run even if logUserCreation fails.
In `@admin-ui/plugins/user-management/components/UserForm.tsx`:
- Around line 60-67: The custom personAttributesKey and its weak tracking cause
stale memoizedPersonAttributes; remove personAttributesKey and change
memoizedPersonAttributes to use the full personAttributes dependency (e.g.,
replace the two useMemo calls with a single line that either returns
personAttributes directly or uses useMemo(() => personAttributes,
[personAttributes])); update any code that references memoizedPersonAttributes
to keep behavior unchanged.
In `@admin-ui/plugins/user-management/helper/userAuditHelpers.ts`:
- Around line 52-65: The redaction logic in the payload.customAttributes mapping
(inside the block that checks payload.customAttributes and uses
isSensitiveCustomAttr) changes the shape of attr.values to an array of { value:
'[REDACTED]' } objects and then force-casts it to
CustomUser['customAttributes']; instead, preserve the original values shape by
mapping each existing scalar value to the string '[REDACTED]' (or leaving
undefined if attr.values was not an array), so attr.values remains an array of
strings (or undefined) and no cast is required.
---
Duplicate comments:
In `@admin-ui/app/routes/Apps/Gluu/MultiValueSelectCard.tsx`:
- Around line 139-150: The remove-field button in MultiValueSelectCard.tsx
currently always renders and calls onRemoveField even when the component is
disabled; change the button to honor the disabled prop by adding the disabled
attribute and preventing the click handler from invoking onRemoveField when
disabled (e.g. replace onClick={onRemoveField} with onClick={() => { if
(!disabled) onRemoveField(); }}), and update ARIA state (aria-disabled or remove
aria-label) as appropriate so classes.removeFieldButton and onRemoveField are no
longer active when disabled.
In `@admin-ui/plugins/user-management/components/AvailableClaimsPanel.tsx`:
- Around line 50-62: The current useMemo (visibleOptions) returns an empty array
when searchClaims is empty; change the logic so an empty search returns all
eligible claims instead of []; specifically, in the visibleOptions computation
(useMemo) when searchTrimmed === '' return personAttributes.filter(...) applying
the same eligibility checks (isActive: data.status?.toLowerCase() === 'active',
notUsed: !USED_CLAIMS.has(data.name), and not alreadyAddedClaim:
!selectedClaims.some(el => el.name === data.name)) rather than []. Preserve the
existing matchesSearch check only for non-empty queries (i.e., compute
matchesSearch = name.includes(searchLower) and include it in the predicate when
searchTrimmed !== '') so admins see the full available list on empty queries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ab6133e2-7b9d-4d51-ae1b-ff8b26bd9f14
📒 Files selected for processing (58)
admin-ui/app/context/theme/config.tsadmin-ui/app/customColors.tsadmin-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/redux/actions/index.tsadmin-ui/app/redux/features/attributesSlice.tsadmin-ui/app/redux/reducers/index.tsadmin-ui/app/redux/sagas/AttributesSaga.tsadmin-ui/app/redux/sagas/index.tsadmin-ui/app/redux/types/index.tsadmin-ui/app/routes/Apps/Gluu/GluuInputRow.tsxadmin-ui/app/routes/Apps/Gluu/GluuRemovableInputRow.tsxadmin-ui/app/routes/Apps/Gluu/GluuTabs.tsxadmin-ui/app/routes/Apps/Gluu/GluuViewDetailsModal.tsxadmin-ui/app/routes/Apps/Gluu/MultiValueSelectCard.tsxadmin-ui/app/routes/Apps/Gluu/styles/GluuCommitDialog.style.tsadmin-ui/app/routes/Apps/Gluu/styles/GluuInputRow.style.tsadmin-ui/app/routes/Apps/Gluu/styles/MultiValueSelectCard.style.tsadmin-ui/app/routes/Apps/Gluu/types/GluuCommitDialog.tsadmin-ui/app/routes/Apps/Gluu/types/MultiValueSelectCard.types.tsadmin-ui/app/utilities.tsxadmin-ui/app/utils/hooks/useWebhookDialogAction.tsxadmin-ui/app/utils/styles/WebhookTriggerModal.style.tsadmin-ui/plugins/admin/components/Assets/types/FormTypes.tsadmin-ui/plugins/auth-server/components/Sessions/SessionListPage.tsxadmin-ui/plugins/saml/components/WebsiteSsoIdentityProviderForm.tsxadmin-ui/plugins/saml/components/WebsiteSsoServiceProviderForm.tsxadmin-ui/plugins/user-management/components/AvailableClaimsPanel.style.tsadmin-ui/plugins/user-management/components/AvailableClaimsPanel.tsxadmin-ui/plugins/user-management/components/PasswordChangeModal.style.tsadmin-ui/plugins/user-management/components/PasswordChangeModal.tsxadmin-ui/plugins/user-management/components/User2FADevicesModal.style.tsadmin-ui/plugins/user-management/components/User2FADevicesModal.tsxadmin-ui/plugins/user-management/components/UserAddPage.tsxadmin-ui/plugins/user-management/components/UserClaimEntry.tsxadmin-ui/plugins/user-management/components/UserDetailViewPage.style.tsadmin-ui/plugins/user-management/components/UserDetailViewPage.tsxadmin-ui/plugins/user-management/components/UserDeviceDetailViewPage.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/components/UserFormCommitDialog.tsxadmin-ui/plugins/user-management/components/UserFormPage.style.tsadmin-ui/plugins/user-management/components/UserList.tsxadmin-ui/plugins/user-management/components/UserListPage.style.tsadmin-ui/plugins/user-management/helper/userAuditHelpers.tsadmin-ui/plugins/user-management/helper/userWebhookHelpers.tsadmin-ui/plugins/user-management/helper/validations.tsadmin-ui/plugins/user-management/hooks/index.tsadmin-ui/plugins/user-management/hooks/useUserMutations.tsadmin-ui/plugins/user-management/types/CommonTypes.tsadmin-ui/plugins/user-management/types/ComponentTypes.tsadmin-ui/plugins/user-management/types/ErrorTypes.tsadmin-ui/plugins/user-management/types/index.tsadmin-ui/plugins/user-management/utils/attributeTransformUtils.tsadmin-ui/plugins/user-management/utils/userFormUtils.ts
💤 Files with no reviewable changes (5)
- admin-ui/app/redux/actions/index.ts
- admin-ui/app/redux/sagas/index.ts
- admin-ui/app/redux/sagas/AttributesSaga.ts
- admin-ui/app/redux/features/attributesSlice.ts
- admin-ui/app/redux/reducers/index.ts
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
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 (5)
admin-ui/plugins/user-management/helper/userAuditHelpers.ts (3)
114-140: 🧹 Nitpick | 🔵 TrivialRemove unused
dataparameter.Same issue as
logUserCreation- thedataparameter inlogUserUpdateis never used.♻️ Proposed fix
-export const logUserUpdate = async (data: CustomUser, payload: CustomUser): Promise<void> => { +export const logUserUpdate = async (payload: CustomUser): Promise<void> => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/helper/userAuditHelpers.ts` around lines 114 - 140, The function logUserUpdate contains an unused parameter data; remove it from the signature and any related type annotations (change export const logUserUpdate = async (data: CustomUser, payload: CustomUser) to only accept payload), and then update all call sites that pass the extra data argument to call logUserUpdate(payload) instead; verify there are no remaining references to the removed parameter and adjust imports/types if needed (follow the same change applied to logUserCreation for consistency).
186-215: 🧹 Nitpick | 🔵 TrivialType casting mismatch at line 196.
The
payloadparameter is typed asRecord<string, string | string[] | boolean | object | object[]>but is spread intoAuditPayloadwhich extendsCustomUser. This loose typing could mask missing required fields or structural mismatches.Consider either:
- Narrowing the
payloadparameter type to match expected audit payload structure- Using a type guard before spreading, or
- Creating a separate type for password change audit payloads
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/helper/userAuditHelpers.ts` around lines 186 - 215, The payload parameter of logPasswordChange is too loosely typed and is being spread into AuditPayload (which extends CustomUser), risking missing required fields; fix this by defining a dedicated PasswordChangeAuditPayload type that matches the fields expected by AuditPayload for password-change events (or create a type guard that validates payload shape), update the logPasswordChange signature to accept PasswordChangeAuditPayload (or narrow with the guard before spreading), then use that strongly-typed variable when calling redactSensitiveData and logAuditUserAction (references: logPasswordChange, AuditPayload, redactSensitiveData, logAuditUserAction) so TypeScript ensures structural correctness rather than relying on an unsafe cast.
86-112: 🧹 Nitpick | 🔵 TrivialRemove unused
dataparameter fromlogUserCreationandlogUserUpdate.Both functions accept a
dataparameter that is never used in the function body. Only thepayloadparameter is referenced. This adds confusion and maintenance burden.Update the following callers:
admin-ui/plugins/user-management/components/UserAddPage.tsx:50— Removedataargument fromlogUserCreationcalladmin-ui/plugins/user-management/components/UserEditPage.tsx:123— Removedataargument fromlogUserUpdatecalladmin-ui/plugins/user-management/components/User2FADevicesModal.tsx:89— Removedataargument fromlogUserUpdatecall♻️ Proposed fixes
-export const logUserCreation = async (data: CustomUser, payload: CustomUser): Promise<void> => { +export const logUserCreation = async (payload: CustomUser): Promise<void> => {-export const logUserUpdate = async (data: CustomUser, payload: CustomUser): Promise<void> => { +export const logUserUpdate = async (payload: CustomUser): Promise<void> => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/helper/userAuditHelpers.ts` around lines 86 - 112, Remove the unused first parameter named data from the function signatures of logUserCreation and logUserUpdate (so they only accept payload: CustomUser) and update their internal types/usages accordingly (no other logic changes). Then update all call sites to stop passing the extra data argument: change the calls in UserAddPage (logUserCreation), UserEditPage (logUserUpdate), and User2FADevicesModal (logUserUpdate) to invoke the functions with only the payload argument. Ensure TypeScript types/exports still compile after updating the function signatures.admin-ui/app/locales/pt/translation.json (1)
1021-1053:⚠️ Potential issue | 🟡 MinorAdd
placeholders.search_hereto the PT locale.
MultiValueSelectCardnow falls back tot('placeholders.search_here'), but this file only addssearchandsearch_claims_here. Portuguese screens will show the raw key or an English fallback.💬 Suggested fix
"placeholders": { "id": "Enter id", "search": "Pesquisar", + "search_here": "Pesquisar aqui", "action_commit_message": "Forneça o motivo desta mudança",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/locales/pt/translation.json` around lines 1021 - 1053, The PT locale is missing the placeholders.search_here key used by MultiValueSelectCard, so add a new entry "placeholders.search_here" to translation.json (alongside the existing "search" and "search_claims_here") with the Portuguese text (e.g., "Pesquisar aqui") so the component no longer displays the raw key or English fallback.admin-ui/app/locales/es/translation.json (1)
1079-1113:⚠️ Potential issue | 🟡 MinorAdd the missing
placeholders.search_hereentry.
UserClaimEntry.tsxnow callst('placeholders.search_here'), but this locale only definessearchandsearch_claims_here. Spanish users will fall back to another locale or the raw key on that path.🌐 Proposed fix
"search_pattern": "Patrón de búsqueda", + "search_here": "Buscar aquí", "search_claims_here": "Buscar reclamaciones aquí",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/locales/es/translation.json` around lines 1079 - 1113, The Spanish locale is missing the placeholders.search_here key referenced from UserClaimEntry.tsx; add a new JSON entry "placeholders.search_here": "Buscar aquí" (or equivalent Spanish text) into the translation object alongside the existing keys (e.g., near "search" / "search_claims_here") so t('placeholders.search_here') resolves for Spanish users.
♻️ Duplicate comments (5)
admin-ui/plugins/user-management/utils/userFormUtils.ts (1)
147-165:⚠️ Potential issue | 🟠 MajorUnwrap nested primitive values before boolean coercion.
Lines 155-162 only preserve wrapped booleans. Payloads like
{ value: 'false' },{ val: 'false' }, or{ value: 0 }still fall through toBoolean(firstValue), which evaluates the wrapper object as truthy and preselects the wrong boolean claim.🛠 Suggested fix
- const firstValue = customAttr.values[0] as string | boolean | object + const rawFirstValue = customAttr.values[0] as + | string + | number + | boolean + | { value?: string | number | boolean | null; val?: string | number | boolean | null } + const firstValue = + rawFirstValue && typeof rawFirstValue === 'object' + ? 'value' in rawFirstValue + ? rawFirstValue.value + : 'val' in rawFirstValue + ? rawFirstValue.val + : rawFirstValue + : rawFirstValue @@ - } else if (typeof firstValue === 'object') { - const obj = firstValue as Record<string, string | number | boolean> - boolValue = - 'value' in obj && typeof obj.value === 'boolean' - ? obj.value - : 'val' in obj && typeof obj.val === 'boolean' - ? obj.val - : Boolean(firstValue) + } else if (typeof firstValue === 'number') { + boolValue = firstValue !== 0 } else { boolValue = Boolean(firstValue) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/utils/userFormUtils.ts` around lines 147 - 165, The boolean coercion in userFormUtils.ts for customAttr.values[0] (firstValue) only treats value/val when they are already boolean and falls back to Boolean(firstValue), which treats wrapper objects like {value: 'false'} as truthy; update the branch inside isBoolean to, when typeof firstValue === 'object', extract obj = firstValue as Record<string, string|number|boolean> and if 'value' or 'val' exists, coerce that inner primitive to boolean (handle booleans directly, strings by comparing lowercased 'true' or numeric strings/number 0 as false, and numbers by checking !== 0) and assign to boolValue; only fall back to Boolean(firstValue) if no inner primitive is present.admin-ui/app/routes/Apps/Gluu/MultiValueSelectCard.tsx (1)
152-160:⚠️ Potential issue | 🟡 MinorHonor
disabledfor the field-level remover too.The Add/Remove/tag paths now respect
disabled, but Lines 156-160 still let a disabled card remove itself viaonRemoveField.🛠 Suggested fix
<button type="button" className={classes.removeFieldButton} - onClick={onRemoveField} + onClick={disabled ? undefined : onRemoveField} + disabled={disabled} aria-label={t('actions.remove')} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/routes/Apps/Gluu/MultiValueSelectCard.tsx` around lines 152 - 160, The remove button in MultiValueSelectCard still calls onRemoveField even when the component is disabled; update the JSX for the remove button (the element using classes.removeFieldButton inside the MultiValueSelectCard component) to honor the disabled prop by adding the disabled attribute (and aria-disabled if you want) and guard the onClick so it does nothing when disabled (e.g., only call onRemoveField when !disabled). Ensure the visual and accessibility states reflect disabled as well.admin-ui/plugins/user-management/components/PasswordChangeModal.tsx (1)
247-267:⚠️ Potential issue | 🟠 MajorThis portal dialog still needs real focus management.
The container never takes focus on open, does not trap
Tab, and does not restore focus on close. Keyboard users can still tab behind the overlay, andEscapeonly works when focus happens to stay inside this subtree.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/components/PasswordChangeModal.tsx` around lines 247 - 267, The modal lacks focus management: when opening PasswordChangeModal it should save the previously focused element, move focus into the modal container (the element using classNames `${commitClasses.modalContainer} ${formClasses.modalContainer}`), trap Tab/Shift+Tab so focus cannot leave the dialog, handle Escape to call handleCancel regardless of where focus is, and on close restore focus to the saved element; implement this by adding a useEffect in PasswordChangeModal that watches passwordModal, captures document.activeElement before open, sets focus to the modal container (use a ref), attaches a keydown listener on the container or document to intercept Tab (manage focusable elements inside the container) and Escape (call handleCancel), and cleans up listeners and restores focus when passwordModal becomes false.admin-ui/plugins/user-management/components/AvailableClaimsPanel.tsx (1)
50-62:⚠️ Potential issue | 🟠 MajorEmpty search should restore the claims list.
Line 52 still returns
[]for an empty query, so the panel renders blank on first load and the clear button wipes the list instead of restoring eligible claims.Suggested fix
const visibleOptions = useMemo(() => { const searchTrimmed = searchClaims.trim() - if (searchTrimmed === '') return [] const searchLower = searchTrimmed.toLowerCase() return personAttributes.filter((data: PersonAttribute) => { const name = data.displayName?.toLowerCase() || '' const alreadyAddedClaim = selectedClaims.some((el: PersonAttribute) => el.name === data.name) const isActive = data.status?.toLowerCase() === 'active' const notUsed = !USED_CLAIMS.has(data.name) - const matchesSearch = name.includes(searchLower) + const matchesSearch = searchLower === '' || name.includes(searchLower) return Boolean(isActive && notUsed && matchesSearch && !alreadyAddedClaim) }) }, [personAttributes, searchClaims, selectedClaims])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/components/AvailableClaimsPanel.tsx` around lines 50 - 62, The useMemo for visibleOptions currently returns [] when searchClaims is empty, causing the panel to be blank; update the logic in the visibleOptions computation (inside the useMemo for visibleOptions in AvailableClaimsPanel.tsx) so that when searchTrimmed === '' you still return the eligible claims by applying the same filters (isActive, notUsed, not already in selectedClaims using USED_CLAIMS and selectedClaims checks) but without the matchesSearch check, i.e. remove the early empty-array return and ensure filtering by status, USED_CLAIMS, and selectedClaims remains applied to personAttributes.admin-ui/app/locales/fr/translation.json (1)
1029-1062:⚠️ Potential issue | 🟡 MinorAdd the missing
placeholders.search_herekey.
UserForm.tsxnow usest('placeholders.search_here')for the role picker, but this locale only addssearchandsearch_claims_here, so French users will not get a localized placeholder there.Suggested fix
"placeholders": { "id": "Enter id", "search": "Recherche", + "search_here": "Rechercher ici", "action_commit_message": "Fournir la raison de ce changement",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/locales/fr/translation.json` around lines 1029 - 1062, Add the missing translation key placeholders.search_here to the French locale so t('placeholders.search_here') used in UserForm.tsx resolves; edit admin-ui/app/locales/fr/translation.json and add a "placeholders.search_here" entry with an appropriate French string (e.g., a short placeholder like "Rechercher" or "Tapez pour rechercher") alongside the existing keys so the role picker displays a localized placeholder.
🤖 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/routes/Apps/Gluu/GluuInputRow.tsx`:
- Around line 153-158: The visibility toggle button remains interactive when the
input is disabled; update the button in GluuInputRow (the element using
classes.passwordToggle and onClick={setVisibility}) to respect the input's
disabled state by passing the disabled prop (and optionally aria-disabled) so
the button is non-interactive when the input is disabled, and ensure
setVisibility is not invoked when disabled (e.g., rely on the button's disabled
behavior or guard inside setVisibility).
In `@admin-ui/app/routes/Apps/Gluu/GluuRemovableInputRow.tsx`:
- Around line 99-120: Simplify the button JSX by passing the click handler
directly instead of wrapping it in an extra arrow function: replace onClick={()
=> handler()} with onClick={handler}; while you're here, you can also remove the
unnecessary React.CSSProperties cast on applicationStyle.removableInputRow if
the type already matches, inside GluuRemovableInputRow (look for the button
element and the applicationStyle.removableInputRow usage).
In `@admin-ui/plugins/user-management/__tests__/components/UserList.test.tsx`:
- Around line 18-20: The test for UserList currently doesn't set up a user with
write permission, so make the permission precondition explicit by seeding or
mocking a write-capable user before rendering; update the test that renders
<UserList /> (in UserList.test.tsx) to either provide a user object with write
permission via the Wrapper or mock the permission-check utility (e.g., the
function/component used to determine write access) so the assertion that "Add
user" is present is tied to an explicit write permission state rather than the
default harness.
In
`@admin-ui/plugins/user-management/__tests__/components/userManagementTestUtils.tsx`:
- Around line 91-105: The current createUserManagementTestWrapper captures
queryClientToUse at wrapper-creation time causing shared React Query cache
between tests; fix by creating the QueryClient inside the returned Wrapper
component instead of outside: in createUserManagementTestWrapper's inner
function (Wrapper) compute const queryClientToUse = client ??
createUserManagementQueryClient() each render/instantiation so
QueryClientProvider gets a fresh QueryClient per wrapper instance, preserving
the optional client parameter and keeping usage of QueryClientProvider and
createUserManagementQueryClient consistent.
In `@admin-ui/plugins/user-management/components/PasswordChangeModal.tsx`:
- Around line 138-145: The code currently returns early when userDetails?.dn is
missing (userDn) which skips the subsequent audit logging even though the
password patch succeeded; instead of returning, change the degraded-success path
to log the missing DN (console.warn or process log), dispatch the warning toast
(updateToast) as now, but do NOT return — allow the function to continue so the
audit log call (the audit/logging logic after the revoke/session block) still
runs; ensure any revoke-session call that uses userDn is guarded (only called if
userDn exists) while the overall success flow and audit logging always execute.
- Around line 189-195: When the parent modal closes the current effect resets
the form but forgets to reset the commit/confirm state; add a call to reset the
password confirmation modal (e.g., setPasswordModal(false) or the equivalent
state setter for passwordModal) inside the useEffect branch that runs when
!isOpen so the confirm/commit step is cleared along with formikRef.resetForm(),
setPassword(''), setShowPassword(false), and setShowConfirmPassword(false).
In `@admin-ui/plugins/user-management/components/User2FADevicesModal.style.ts`:
- Around line 84-97: The wrapper rule currently sets '& > div, & > div >
div:last-child { overflowX: "visible" }' which allows wide table content to
spill out; change that to allow horizontal scrolling by setting overflowX to
'auto' (or 'scroll' if you prefer) on the wrapper selector, keep the table rule
'& table { tableLayout: "auto"; minWidth: 0 }' as-is so long content can
wrap/measure, and ensure the GluuTable wrapper rule '& > div > div:last-child'
retains its border/background—i.e., replace overflowX: 'visible' with overflowX:
'auto' for the selectors mentioned so the table becomes horizontally scrollable
inside the modal.
In `@admin-ui/plugins/user-management/components/UserDetailViewPage.tsx`:
- Line 55: Replace the hardcoded English fallback in the label object inside
UserDetailViewPage.tsx: instead of t('fields.lastName', { defaultValue: 'Last
Name' }), use the existing surname translation as the fallback by setting
defaultValue to t('fields.sn') (e.g., t('fields.lastName', { defaultValue:
t('fields.sn') })) so locales that lack fields.lastName will fall back to the
already-translated fields.sn key; update the label entry that pairs with value:
lastName accordingly.
In `@admin-ui/plugins/user-management/components/UserEditPage.tsx`:
- Around line 103-119: The current branch handling session revocation (guarded
by anyKeyPresent and using revokeSessionMutation.mutateAsync and
AXIOS_INSTANCE.delete) calls dispatch(updateToast(...)) for a warning and then
returns, which aborts the overall success flow after the user update; instead,
log the error and dispatch a neutral warning toast but do NOT return—allow the
function to continue to the success toast, audit/webhook follow-ups, cache
invalidation, and navigation. Update the warning copy to neutral wording that
covers non-password changes (e.g., “Session revocation failed; changes saved”)
and apply the same change to the duplicate branch later in the file that uses
the same revokeSessionMutation/AXIOS_INSTANCE pattern.
- Around line 121-125: logUserUpdate can reject and block subsequent steps; wrap
the await logUserUpdate(…) call in a try/catch so audit logging failures are
handled but don't stop the flow. Specifically, call await logUserUpdate(data,
variables.data) inside a try block and in catch dispatch an error toast using
messages.audit_logging_failed (e.g., dispatch(updateToast(true, 'error',
t('messages.audit_logging_failed')))), then continue to call
triggerUserWebhook(data), queryClient.invalidateQueries({ queryKey:
getGetUserQueryKey() }) and any navigation. Keep existing
dispatch(setWebhookModal(false)) and the success toast before the try/catch
as-is.
In `@admin-ui/plugins/user-management/components/UserForm.tsx`:
- Around line 526-539: The Change Password button in UserForm is forcing a
light-theme background via backgroundColor={customColors.white}, which breaks
theme-aware styling; remove the backgroundColor prop from the GluuButton in the
changePasswordRow (the GluuButton instance using className
"gluu-change-password-btn" and classes.changePasswordButton) so it falls back to
theme tokens (themeColors/themeTokens) for background, text, and border; if a
temporary override is absolutely required, add an inline comment referencing the
reason and planned removal date, otherwise delete the hard-coded white override.
In `@admin-ui/plugins/user-management/hooks/useUserMutations.ts`:
- Around line 44-66: The three catch blocks use the wrong toast text key; update
each to dispatch a more specific toast for the actual failure: in the catch for
triggerUserWebhook(userData) replace the 'messages.audit_logging_failed' key
with a webhook-specific key (e.g., 'messages.webhook_trigger_failed'), in the
catch for await queryUtils.invalidateQueriesByKey(...) replace it with an
invalidation-specific key (e.g., 'messages.query_invalidation_failed'), and in
the catch wrapping callbacksRef.current?.onSuccess?.() use a callback-specific
key (e.g., 'messages.post_delete_callback_failed'); keep the same
dispatch(updateToast(true, ...)) shape and the existing console.error lines, and
ensure those new i18n keys exist in your translations.
---
Outside diff comments:
In `@admin-ui/app/locales/es/translation.json`:
- Around line 1079-1113: The Spanish locale is missing the
placeholders.search_here key referenced from UserClaimEntry.tsx; add a new JSON
entry "placeholders.search_here": "Buscar aquí" (or equivalent Spanish text)
into the translation object alongside the existing keys (e.g., near "search" /
"search_claims_here") so t('placeholders.search_here') resolves for Spanish
users.
In `@admin-ui/app/locales/pt/translation.json`:
- Around line 1021-1053: The PT locale is missing the placeholders.search_here
key used by MultiValueSelectCard, so add a new entry "placeholders.search_here"
to translation.json (alongside the existing "search" and "search_claims_here")
with the Portuguese text (e.g., "Pesquisar aqui") so the component no longer
displays the raw key or English fallback.
In `@admin-ui/plugins/user-management/helper/userAuditHelpers.ts`:
- Around line 114-140: The function logUserUpdate contains an unused parameter
data; remove it from the signature and any related type annotations (change
export const logUserUpdate = async (data: CustomUser, payload: CustomUser) to
only accept payload), and then update all call sites that pass the extra data
argument to call logUserUpdate(payload) instead; verify there are no remaining
references to the removed parameter and adjust imports/types if needed (follow
the same change applied to logUserCreation for consistency).
- Around line 186-215: The payload parameter of logPasswordChange is too loosely
typed and is being spread into AuditPayload (which extends CustomUser), risking
missing required fields; fix this by defining a dedicated
PasswordChangeAuditPayload type that matches the fields expected by AuditPayload
for password-change events (or create a type guard that validates payload
shape), update the logPasswordChange signature to accept
PasswordChangeAuditPayload (or narrow with the guard before spreading), then use
that strongly-typed variable when calling redactSensitiveData and
logAuditUserAction (references: logPasswordChange, AuditPayload,
redactSensitiveData, logAuditUserAction) so TypeScript ensures structural
correctness rather than relying on an unsafe cast.
- Around line 86-112: Remove the unused first parameter named data from the
function signatures of logUserCreation and logUserUpdate (so they only accept
payload: CustomUser) and update their internal types/usages accordingly (no
other logic changes). Then update all call sites to stop passing the extra data
argument: change the calls in UserAddPage (logUserCreation), UserEditPage
(logUserUpdate), and User2FADevicesModal (logUserUpdate) to invoke the functions
with only the payload argument. Ensure TypeScript types/exports still compile
after updating the function signatures.
---
Duplicate comments:
In `@admin-ui/app/locales/fr/translation.json`:
- Around line 1029-1062: Add the missing translation key
placeholders.search_here to the French locale so t('placeholders.search_here')
used in UserForm.tsx resolves; edit admin-ui/app/locales/fr/translation.json and
add a "placeholders.search_here" entry with an appropriate French string (e.g.,
a short placeholder like "Rechercher" or "Tapez pour rechercher") alongside the
existing keys so the role picker displays a localized placeholder.
In `@admin-ui/app/routes/Apps/Gluu/MultiValueSelectCard.tsx`:
- Around line 152-160: The remove button in MultiValueSelectCard still calls
onRemoveField even when the component is disabled; update the JSX for the remove
button (the element using classes.removeFieldButton inside the
MultiValueSelectCard component) to honor the disabled prop by adding the
disabled attribute (and aria-disabled if you want) and guard the onClick so it
does nothing when disabled (e.g., only call onRemoveField when !disabled).
Ensure the visual and accessibility states reflect disabled as well.
In `@admin-ui/plugins/user-management/components/AvailableClaimsPanel.tsx`:
- Around line 50-62: The useMemo for visibleOptions currently returns [] when
searchClaims is empty, causing the panel to be blank; update the logic in the
visibleOptions computation (inside the useMemo for visibleOptions in
AvailableClaimsPanel.tsx) so that when searchTrimmed === '' you still return the
eligible claims by applying the same filters (isActive, notUsed, not already in
selectedClaims using USED_CLAIMS and selectedClaims checks) but without the
matchesSearch check, i.e. remove the early empty-array return and ensure
filtering by status, USED_CLAIMS, and selectedClaims remains applied to
personAttributes.
In `@admin-ui/plugins/user-management/components/PasswordChangeModal.tsx`:
- Around line 247-267: The modal lacks focus management: when opening
PasswordChangeModal it should save the previously focused element, move focus
into the modal container (the element using classNames
`${commitClasses.modalContainer} ${formClasses.modalContainer}`), trap
Tab/Shift+Tab so focus cannot leave the dialog, handle Escape to call
handleCancel regardless of where focus is, and on close restore focus to the
saved element; implement this by adding a useEffect in PasswordChangeModal that
watches passwordModal, captures document.activeElement before open, sets focus
to the modal container (use a ref), attaches a keydown listener on the container
or document to intercept Tab (manage focusable elements inside the container)
and Escape (call handleCancel), and cleans up listeners and restores focus when
passwordModal becomes false.
In `@admin-ui/plugins/user-management/utils/userFormUtils.ts`:
- Around line 147-165: The boolean coercion in userFormUtils.ts for
customAttr.values[0] (firstValue) only treats value/val when they are already
boolean and falls back to Boolean(firstValue), which treats wrapper objects like
{value: 'false'} as truthy; update the branch inside isBoolean to, when typeof
firstValue === 'object', extract obj = firstValue as Record<string,
string|number|boolean> and if 'value' or 'val' exists, coerce that inner
primitive to boolean (handle booleans directly, strings by comparing lowercased
'true' or numeric strings/number 0 as false, and numbers by checking !== 0) and
assign to boolValue; only fall back to Boolean(firstValue) if no inner primitive
is present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 22902473-aefc-467c-b4e2-45e2349c79a4
📒 Files selected for processing (27)
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/redux/types/index.tsadmin-ui/app/routes/Apps/Gluu/GluuInputRow.tsxadmin-ui/app/routes/Apps/Gluu/GluuRemovableInputRow.tsxadmin-ui/app/routes/Apps/Gluu/GluuViewDetailsModal.tsxadmin-ui/app/routes/Apps/Gluu/MultiValueSelectCard.tsxadmin-ui/app/utilities.tsxadmin-ui/plugins/admin/helper/shortCodes.jsonadmin-ui/plugins/admin/helper/utils.tsadmin-ui/plugins/user-management/__tests__/components/UserAddPage.test.tsxadmin-ui/plugins/user-management/__tests__/components/UserEditPage.test.tsxadmin-ui/plugins/user-management/__tests__/components/UserList.test.tsxadmin-ui/plugins/user-management/__tests__/components/userManagementTestUtils.tsxadmin-ui/plugins/user-management/components/AvailableClaimsPanel.tsxadmin-ui/plugins/user-management/components/PasswordChangeModal.tsxadmin-ui/plugins/user-management/components/User2FADevicesModal.style.tsadmin-ui/plugins/user-management/components/UserClaimEntry.tsxadmin-ui/plugins/user-management/components/UserDetailViewPage.tsxadmin-ui/plugins/user-management/components/UserEditPage.tsxadmin-ui/plugins/user-management/components/UserForm.tsxadmin-ui/plugins/user-management/components/UserFormCommitDialog.tsxadmin-ui/plugins/user-management/helper/userAuditHelpers.tsadmin-ui/plugins/user-management/hooks/useUserMutations.tsadmin-ui/plugins/user-management/utils/userFormUtils.ts
admin-ui/plugins/user-management/__tests__/components/userManagementTestUtils.tsx
Show resolved
Hide resolved
admin-ui/plugins/user-management/components/UserDetailViewPage.tsx
Outdated
Show resolved
Hide resolved
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 (2)
admin-ui/plugins/user-management/components/UserForm.tsx (1)
59-66: 🧹 Nitpick | 🔵 TrivialUnstable memo dependency may cause unnecessary re-renders or stale values.
The
personAttributesKeymemo depends onpersonAttributes[0]?.namedirectly, but the dependency array only includespersonAttributes.lengthandpersonAttributes[0]?.name. If the first element's name changes but the array length stays the same, React's dependency check will work correctly. However, accessingpersonAttributes[0]without checking length first could throw if called before the guard.More importantly,
memoizedPersonAttributeson line 66 depends onpersonAttributesKeywhich is a string—this doesn't prevent the actual array reference from changing on every render if the parent re-renders with a new array instance. ThepersonAttributesKeyapproach only gates re-computation but the actual memo returns the same potentially-stale array.🔧 Suggested simplification
- const personAttributesKey = useMemo( - () => - personAttributes.length > 0 - ? `${personAttributes.length}-${personAttributes[0]?.name || ''}` - : 'empty', - [personAttributes.length, personAttributes[0]?.name], - ) - const memoizedPersonAttributes = useMemo(() => personAttributes, [personAttributesKey]) + const memoizedPersonAttributes = useMemo( + () => personAttributes, + // eslint-disable-next-line react-hooks/exhaustive-deps + [personAttributes.length, personAttributes[0]?.name], + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/components/UserForm.tsx` around lines 59 - 66, The current memoization is unsafe: personAttributesKey reads personAttributes[0] without guarding length and using a string key doesn't prevent the array's contents from changing or avoid re-renders when a new array instance is passed. Fix by first guarding any index access (only read personAttributes[0]?.name after checking personAttributes.length>0) and change memoization to derive dependencies from the array contents instead of a single string key — for example, compute a stable signature like personAttributes.map(p=>p.name).join('|') (or another deterministic reducer of relevant fields) and use that signature plus personAttributes.length in the useMemo dependency for memoizedPersonAttributes; ensure memoizedPersonAttributes uses useMemo(() => personAttributes, [personAttributes.length, signature]) so it only updates when actual contents change.admin-ui/plugins/user-management/components/PasswordChangeModal.tsx (1)
97-125:⚠️ Potential issue | 🟠 MajorUse the repo’s existing
customAttributesvalue shape here.
admin-ui/plugins/user-management/helper/userAuditHelpers.ts:40-67treatscustomAttributes[].valuesas objects with avaluefield, but this patch writesvalues: [password]and replaces/values/0. That gives newly added password attributes a different shape than the rest of the user-management flow expects.Proposed fix
const patchOperations: UserPatchRequest = passwordAttrIndex !== undefined && passwordAttrIndex >= 0 ? { jsonPatchString: JSON.stringify([ { op: 'replace', - path: `/customAttributes/${passwordAttrIndex}/values/0`, + path: `/customAttributes/${passwordAttrIndex}/values/0/value`, value: password, }, ]), } : { jsonPatchString: JSON.stringify([ { op: 'add', path: '/customAttributes/-', value: { name: 'userPassword', multiValued: false, - values: [password], + values: [{ value: password }], }, }, ]), }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/components/PasswordChangeModal.tsx` around lines 97 - 125, The patch currently writes plain strings for password values which conflicts with the rest of the codebase that expects customAttributes[].values to be objects with a value field; update both the "add" and "replace" branches so they produce the same shape: for the add branch set values to an array of objects [{ value: password }], and for the replace branch set the patch operation's value to an object { value: password } (or alternatively change the replace path to target /values/0/value and keep the replacement as the string), and ensure the auditPayload customAttributes entry follows the same shape as used elsewhere (use objects with value fields) so functions referenced in userAuditHelpers (and variables patchOperations, passwordAttrIndex, auditPayload, userDetails) see a consistent customAttributes.values structure.
♻️ Duplicate comments (2)
admin-ui/plugins/user-management/__tests__/components/userManagementTestUtils.tsx (1)
96-99: 🧹 Nitpick | 🔵 TrivialGood fix for QueryClient caching—consider
useStatefor stability.Moving the
QueryClientcreation inside the component viauseMemoaddresses the original concern about shared cache across tests. However,useStatewith a lazy initializer is the more idiomatic pattern for objects that must remain stable across re-renders, as React may drop memoized values in future versions.♻️ Optional: Use useState for guaranteed stability
return function Wrapper({ children }: { children: React.ReactNode }) { - const queryClientToUse = React.useMemo( - () => client ?? createUserManagementQueryClient(), - [client], - ) + const [queryClientToUse] = React.useState( + () => client ?? createUserManagementQueryClient(), + ) return (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/__tests__/components/userManagementTestUtils.tsx` around lines 96 - 99, The QueryClient is created with React.useMemo which can be dropped by future React behavior; switch to React.useState with a lazy initializer to guarantee stability across re-renders. Replace the use of useMemo for queryClientToUse (and its fallback to createUserManagementQueryClient()) with a useState(() => client ?? createUserManagementQueryClient()) pattern so the created QueryClient instance is stable, while still using the provided client when present.admin-ui/plugins/user-management/components/PasswordChangeModal.tsx (1)
251-273:⚠️ Potential issue | 🟠 MajorThis portal dialog still isn’t modal for keyboard users.
role="dialog"andtabIndex={-1}alone do not move focus into the dialog, trapTab, or restore focus on close. Users can still tab behind the overlay, and Escape only works when one of these elements happens to own focus. Please wire the same focus-handoff/trap/restore behavior used by the other portal dialogs, and addaria-modal="true"here as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/components/PasswordChangeModal.tsx` around lines 251 - 273, The PasswordChangeModal dialog lacks proper focus management and aria-modal; update the PasswordChangeModal so that when isOpen becomes true it programmatically moves focus into the dialog (store and later restore document.activeElement), adds aria-modal="true" on the dialog container, and installs a Tab/Shift+Tab focus trap and Escape handler that mirrors the behavior used by your other portal dialogs (reuse the same focus-trap/restore utilities or hook used elsewhere). Ensure the same handlers (e.g., handleCancel, handleModalKeyDown, submitChangePassword, and the GluuCommitDialog interaction) continue to work with the trap and that focus is restored on close or when the commit dialog cancels.
🤖 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/locales/es/translation.json`:
- Around line 1081-1084: Add the missing Spanish translation for the key
placeholders.search_here used by MultiValueSelectCard.tsx: open the locales ES
translation.json and inside the "placeholders" object add "search_here": "Buscar
aquí" (or an approved Spanish variant like "Buscar..." ) so the component
(MultiValueSelectCard) no longer falls back to the raw key when rendering in
Spanish.
In `@admin-ui/app/locales/pt/translation.json`:
- Around line 6-10: Remove duplicate root-level keys "Username" and "Nickname"
and consolidate translations to use the canonical field keys used elsewhere
(fields.username and fields.nickName) so there is a single source of truth;
update the JSON by deleting or merging the root entries "Username" and
"Nickname" and ensure any references in the UI use fields.username or
fields.nickName consistently, keeping only distinct keys where Portuguese
wording must differ.
In `@admin-ui/plugins/admin/redux/sagas/WebhookSaga.ts`:
- Around line 105-108: The early-return branch that runs when
!enabledFeatureWebhooks.length || !featureToTrigger currently resets
setFeatureToTrigger('') and setTriggerWebhookResponse('') but leaves
webhookTriggerErrors and showErrorModal stale; update this branch to also clear
the full webhook error state by dispatching the actions that reset
webhookTriggerErrors (e.g., setWebhookTriggerErrors([]) or the app's equivalent)
and setShowErrorModal(false) so the store matches the centralized cleanup used
in useWebhookDialogAction (and keep setFeatureToTrigger and
setTriggerWebhookResponse as-is).
In `@admin-ui/plugins/user-management/__tests__/components/UserList.test.tsx`:
- Around line 19-31: Add a negative unit test that mocks useCedarling to return
hasCedarWritePermission: jest.fn(() => false) (keep hasCedarReadPermission and
other hooks as needed), renders the UserList component with the same Wrapper,
and asserts that the "Add user" button is not present (use queryByText or
equivalent). Target the existing test file and mirror the positive test
structure (reference useCedarling, UserList, Wrapper, and the "Add user" button
text) so the new test verifies the permission-denied scenario.
- Around line 10-11: The tests currently instantiate a shared Redux store and
Wrapper at module scope (createUserManagementTestStore and
createUserManagementTestWrapper), which can cause state leakage between tests;
move the creation of store and Wrapper into a per-test lifecycle so each test
gets a fresh instance—e.g., create new store and Wrapper inside a beforeEach or
inside each test so any dispatches do not affect other tests, and update
references in tests to use the locally-scoped store/Wrapper.
In
`@admin-ui/plugins/user-management/__tests__/components/userManagementTestUtils.tsx`:
- Line 80: Remove the noReducer placeholder if it isn't required by any
selectors/components used in these tests; locate the Redux root used in the test
utils (the noReducer entry in the reducer map) and delete that key, or if some
selectors expect that slice shape, replace the placeholder with a minimal
documented stub and add a one-line comment explaining why it exists (reference:
noReducer in
admin-ui/plugins/user-management/__tests__/components/userManagementTestUtils.tsx).
- Around line 61-73: Extend the mock reducer states to match the real slices:
update defaultWebhookReducerState (used in tests) to include
triggerWebhookMessage, webhookTriggerErrors, triggerPayload, featureToTrigger,
and showErrorModal with appropriate initial values/types, and update
defaultAuthReducerState to include isAuthenticated, userinfo_jwt, issuer,
codeChallenge, codeChallengeMethod, codeVerifier, backendStatus, loadingConfig,
idToken, jwtToken, userInum, isUserInfoFetched, and hasSession with correct
default values/types; modify the test utilities where defaultWebhookReducerState
and defaultAuthReducerState are defined so tests get full-shaped states that
mirror the actual reducers (preserve existing config, location, userinfo
fields).
In `@admin-ui/plugins/user-management/components/PasswordChangeModal.tsx`:
- Around line 138-168: The code lets the success path run even when userDn is
missing; update the missing-DN branch in PasswordChangeModal so it does not fall
through to full success: set showSuccessAndClose = false (or return early) when
userDetails?.dn is falsy so the success toast, setPasswordModal(false),
toggle(), setPassword(''), and onSuccess() calls are skipped; locate the
userDn/check block and the variables showSuccessAndClose, revokeSessionMutation,
AXIOS_INSTANCE, updateToast, setPasswordModal, toggle, setPassword, and
onSuccess to implement this change.
In `@admin-ui/plugins/user-management/components/UserDetailViewPage.tsx`:
- Line 77: The memo wrapper around UserDetailViewPage is ineffective because
UserList.tsx creates a new prop object each render via row={{ rowData: row }},
defeating shallow comparison; either remove React.memo by changing the export
from export default React.memo(UserDetailViewPage) to export default
UserDetailViewPage, or instead stabilize the prop at the call site (e.g., pass a
stable reference or memoize the expanded row object in renderExpandedRow) so the
prop identity doesn't change on every render; pick one approach and apply it to
the symbols UserDetailViewPage and the renderExpandedRow prop usage in
UserList.tsx.
In `@admin-ui/plugins/user-management/components/UserEditPage.tsx`:
- Around line 46-49: Guard against themeState being undefined by importing
DEFAULT_THEME and using it as a fallback when reading themeState.theme: replace
direct accesses to themeState.theme in the useMemo call for themeColors, the
isDark computation, and the useStyles call with (themeState?.theme ??
DEFAULT_THEME); keep getThemeColor, THEME_DARK, useTheme, useStyles, themeColors
and isDark identifiers the same so the rest of the component is unchanged.
In `@admin-ui/plugins/user-management/components/UserForm.tsx`:
- Around line 68-72: Guard against themeState being undefined when calling
useTheme: change how selectedTheme is derived to fall back to DEFAULT_THEME if
themeState is falsy (useTheme -> state: themeState, selectedTheme), then pass
selectedTheme into getThemeColor and compare to THEME_DARK; update imports to
ensure DEFAULT_THEME is available and keep existing references to getThemeColor,
THEME_DARK, and useStyles intact so useStyles({ isDark, themeColors }) receives
a valid theme.
- Around line 488-508: The role selector is only shown when roleClaim exists in
selectedClaims; to ensure admins can always assign roles, either remove the
conditional so the MultiValueSelectCard (labelled by JANS_ADMIN_UI_ROLE_ATTR,
value selectedRoles, onChange handleRoleChange, options rolesToBeShown) is
rendered unconditionally (still disabled when rolesLoading || rolesError) or
ensure the form initialization auto-inserts a roleClaim for
JANS_ADMIN_UI_ROLE_ATTR into selectedClaims when missing (so roleClaim is
present); update the component logic that derives roleClaim from selectedClaims
or the form init code to implement one of these fixes and keep the existing
props (allowCustom, placeholder, doc_category) intact.
---
Outside diff comments:
In `@admin-ui/plugins/user-management/components/PasswordChangeModal.tsx`:
- Around line 97-125: The patch currently writes plain strings for password
values which conflicts with the rest of the codebase that expects
customAttributes[].values to be objects with a value field; update both the
"add" and "replace" branches so they produce the same shape: for the add branch
set values to an array of objects [{ value: password }], and for the replace
branch set the patch operation's value to an object { value: password } (or
alternatively change the replace path to target /values/0/value and keep the
replacement as the string), and ensure the auditPayload customAttributes entry
follows the same shape as used elsewhere (use objects with value fields) so
functions referenced in userAuditHelpers (and variables patchOperations,
passwordAttrIndex, auditPayload, userDetails) see a consistent
customAttributes.values structure.
In `@admin-ui/plugins/user-management/components/UserForm.tsx`:
- Around line 59-66: The current memoization is unsafe: personAttributesKey
reads personAttributes[0] without guarding length and using a string key doesn't
prevent the array's contents from changing or avoid re-renders when a new array
instance is passed. Fix by first guarding any index access (only read
personAttributes[0]?.name after checking personAttributes.length>0) and change
memoization to derive dependencies from the array contents instead of a single
string key — for example, compute a stable signature like
personAttributes.map(p=>p.name).join('|') (or another deterministic reducer of
relevant fields) and use that signature plus personAttributes.length in the
useMemo dependency for memoizedPersonAttributes; ensure memoizedPersonAttributes
uses useMemo(() => personAttributes, [personAttributes.length, signature]) so it
only updates when actual contents change.
---
Duplicate comments:
In
`@admin-ui/plugins/user-management/__tests__/components/userManagementTestUtils.tsx`:
- Around line 96-99: The QueryClient is created with React.useMemo which can be
dropped by future React behavior; switch to React.useState with a lazy
initializer to guarantee stability across re-renders. Replace the use of useMemo
for queryClientToUse (and its fallback to createUserManagementQueryClient())
with a useState(() => client ?? createUserManagementQueryClient()) pattern so
the created QueryClient instance is stable, while still using the provided
client when present.
In `@admin-ui/plugins/user-management/components/PasswordChangeModal.tsx`:
- Around line 251-273: The PasswordChangeModal dialog lacks proper focus
management and aria-modal; update the PasswordChangeModal so that when isOpen
becomes true it programmatically moves focus into the dialog (store and later
restore document.activeElement), adds aria-modal="true" on the dialog container,
and installs a Tab/Shift+Tab focus trap and Escape handler that mirrors the
behavior used by your other portal dialogs (reuse the same focus-trap/restore
utilities or hook used elsewhere). Ensure the same handlers (e.g., handleCancel,
handleModalKeyDown, submitChangePassword, and the GluuCommitDialog interaction)
continue to work with the trap and that focus is restored on close or when the
commit dialog cancels.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9ad6f09b-0570-4103-8ce0-816edf4dfe07
📒 Files selected for processing (15)
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/GluuInputRow.tsxadmin-ui/app/routes/Apps/Gluu/GluuRemovableInputRow.tsxadmin-ui/plugins/admin/redux/sagas/WebhookSaga.tsadmin-ui/plugins/user-management/__tests__/components/UserList.test.tsxadmin-ui/plugins/user-management/__tests__/components/userManagementTestUtils.tsxadmin-ui/plugins/user-management/components/PasswordChangeModal.tsxadmin-ui/plugins/user-management/components/User2FADevicesModal.style.tsadmin-ui/plugins/user-management/components/UserDetailViewPage.tsxadmin-ui/plugins/user-management/components/UserEditPage.tsxadmin-ui/plugins/user-management/components/UserForm.tsxadmin-ui/plugins/user-management/hooks/useUserMutations.ts
admin-ui/plugins/user-management/__tests__/components/UserList.test.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/user-management/components/PasswordChangeModal.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
Contributor
There was a problem hiding this comment.
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 (3)
admin-ui/app/locales/pt/translation.json (1)
1023-1055:⚠️ Potential issue | 🟡 MinorAdd
placeholders.search_hereto the Portuguese locale.The new searchable multi-select flow expects
placeholders.search_here; without it, Portuguese screens will fall back to the raw key.💬 Suggested fix
"placeholders": { "id": "Enter id", "search": "Pesquisar", + "search_here": "Pesquisar aqui", "action_commit_message": "Forneça o motivo desta mudança",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/locales/pt/translation.json` around lines 1023 - 1055, Add the missing translation key placeholders.search_here to the Portuguese locale so the multi-select search UI doesn't fall back to the raw key; locate the Portuguese translation object (near keys like "search" and "search_pattern") and add an entry for "placeholders.search_here" with the Portuguese string (e.g., "Pesquisar aqui") ensuring it matches the JSON structure and quotation/commas style used by the existing keys.admin-ui/plugins/user-management/components/UserForm.tsx (1)
59-65: 🧹 Nitpick | 🔵 TrivialUnstable memoization key may cause unnecessary re-renders.
The
personAttributesKeydepends onpersonAttributes[0]?.name, but accessing index0directly is fragile. If the array order changes while keeping the same content, the key changes unnecessarily. More importantly,memoizedPersonAttributesat line 66 depends on this key but returns the samepersonAttributesreference, which defeats the purpose of memoization.♻️ Suggested simplification
- const personAttributesKey = useMemo( - () => - personAttributes.length > 0 - ? `${personAttributes.length}-${personAttributes[0]?.name || ''}` - : 'empty', - [personAttributes.length, personAttributes[0]?.name], - ) - const memoizedPersonAttributes = useMemo(() => personAttributes, [personAttributesKey]) + const memoizedPersonAttributes = useMemo(() => personAttributes, [personAttributes])If referential stability is needed, consider using a deep comparison or derive the key from all attribute names.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/components/UserForm.tsx` around lines 59 - 65, The memo key personAttributesKey (and subsequent memoizedPersonAttributes) is unstable because it only inspects personAttributes[0]?.name and returns the same array reference, defeating memoization; update the key to derive from all attribute identities (e.g., join of personAttributes.map(a => a.name) or JSON.stringify of the names) and change memoizedPersonAttributes to use useMemo with that derived key so it returns a stable reference only when the full set/order of attribute names actually changes; reference personAttributesKey, memoizedPersonAttributes and personAttributes when making the change.admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
162-180:⚠️ Potential issue | 🟡 MinorRemove the
onErrortoast dispatch from the mutation config and add explicit error handling insubmitData.The mutation's
onErrorhandler dispatches an error toast, but according to codebase patterns (evident in SAML plugin mutations), update mutation hooks should not dispatch error toasts internally—callers should handle errors. Additionally,submitDatalacks explicit try/catch, making error handling implicit and harder to trace.The
GluuCommitDialog.handleAccepthas a try/finally but no catch, so errors frommutateAsyncpropagate uncaught. Wrap themutateAsynccall insubmitDatawith try/catch, remove theonErrortoast dispatch from the mutation config, and let the catch block handle the error notification.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/plugins/user-management/components/UserEditPage.tsx` around lines 162 - 180, Remove the onError toast dispatch from the update mutation configuration (updateUserMutation) so the mutation hook does not emit UI notifications itself; instead, wrap the call to updateUserMutation.mutateAsync inside submitData with an explicit try/catch: perform the mutateAsync in try, handle success as before, and in catch dispatch the error toast (and any logging) so the caller controls UI feedback; ensure GluuCommitDialog.handleAccept still preserves its finally block behavior (do not remove the existing try/finally) but let submitData surface handled errors (i.e., do not leave mutateAsync unwrapped so exceptions no longer propagate uncaught).
🤖 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/locales/pt/translation.json`:
- Around line 936-941: Add the missing Portuguese translation key
messages.session_revoke_failed to the Portuguese translation JSON so
PasswordChangeModal.tsx (which references messages.session_revoke_failed at
lines where the modal shows revoke failures) no longer falls back; insert a new
entry with the key "session_revoke_failed" and the Portuguese message (e.g.,
"Falha ao revogar a sessão.") alongside the other keys such as
"session_revoke_failed_changes_saved" and "revokeUserSession" in the same
object.
In
`@admin-ui/plugins/user-management/__tests__/components/userManagementTestUtils.tsx`:
- Line 96: Remove the extraneous authState: undefined property from the test
default auth state in userManagementTestUtils.tsx so the test initial state
matches the production authSlice initialState; leave AuthState optional and use
the setOAuthState action to set authState in tests when needed, ensuring the
exported test state object no longer includes authState by default.
In `@admin-ui/plugins/user-management/components/PasswordChangeModal.tsx`:
- Around line 122-125: The auditPayload's customAttributes entry lacks the
values field expected by redactSensitiveData in userAuditHelpers.ts; update the
customAttributes object inside auditPayload (the one currently { name:
'userPassword', multiValued: false }) to include a values array (e.g., values:
[] or values: ['']) so the shape matches the expected { name, values,
multiValued } structure and the redaction logic will handle it correctly.
In `@admin-ui/plugins/user-management/components/UserDetailViewPage.tsx`:
- Around line 22-28: Replace the hard-coded string 'jansAdminUIRole' in the
useMemo block that computes roleValue with the shared constant exported from
admin-ui/plugins/user-management/common/Constants.ts: import the role attribute
constant and use it in the attrs.find predicate (and anywhere else this literal
appears) so the lookup in roleAttr (and the types referenced like
CustomAttrWithValues and rowData) uses the single source-of-truth constant
instead of a duplicated string.
In `@admin-ui/plugins/user-management/components/UserForm.tsx`:
- Around line 131-139: The submitForm callback currently returns
Promise.resolve() when isSubmitting is true but otherwise returns whatever
onSubmitData returns, producing inconsistent return types; make submitForm
consistently return Promise<void> by converting it to an async function (async
(usermessage) => { if (isSubmitting) return; await onSubmitData(formik.values,
modifiedFields, usermessage); }) so it always returns a Promise<void> and
preserves the original call to onSubmitData; update the useCallback reference to
submitForm and keep dependencies [onSubmitData, formik.values, modifiedFields,
isSubmitting].
---
Outside diff comments:
In `@admin-ui/app/locales/pt/translation.json`:
- Around line 1023-1055: Add the missing translation key
placeholders.search_here to the Portuguese locale so the multi-select search UI
doesn't fall back to the raw key; locate the Portuguese translation object (near
keys like "search" and "search_pattern") and add an entry for
"placeholders.search_here" with the Portuguese string (e.g., "Pesquisar aqui")
ensuring it matches the JSON structure and quotation/commas style used by the
existing keys.
In `@admin-ui/plugins/user-management/components/UserEditPage.tsx`:
- Around line 162-180: Remove the onError toast dispatch from the update
mutation configuration (updateUserMutation) so the mutation hook does not emit
UI notifications itself; instead, wrap the call to
updateUserMutation.mutateAsync inside submitData with an explicit try/catch:
perform the mutateAsync in try, handle success as before, and in catch dispatch
the error toast (and any logging) so the caller controls UI feedback; ensure
GluuCommitDialog.handleAccept still preserves its finally block behavior (do not
remove the existing try/finally) but let submitData surface handled errors
(i.e., do not leave mutateAsync unwrapped so exceptions no longer propagate
uncaught).
In `@admin-ui/plugins/user-management/components/UserForm.tsx`:
- Around line 59-65: The memo key personAttributesKey (and subsequent
memoizedPersonAttributes) is unstable because it only inspects
personAttributes[0]?.name and returns the same array reference, defeating
memoization; update the key to derive from all attribute identities (e.g., join
of personAttributes.map(a => a.name) or JSON.stringify of the names) and change
memoizedPersonAttributes to use useMemo with that derived key so it returns a
stable reference only when the full set/order of attribute names actually
changes; reference personAttributesKey, memoizedPersonAttributes and
personAttributes when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2c8efc04-dbf8-4331-8539-3c85e5fe4c8e
📒 Files selected for processing (9)
admin-ui/app/locales/es/translation.jsonadmin-ui/app/locales/pt/translation.jsonadmin-ui/plugins/admin/redux/sagas/WebhookSaga.tsadmin-ui/plugins/user-management/__tests__/components/UserList.test.tsxadmin-ui/plugins/user-management/__tests__/components/userManagementTestUtils.tsxadmin-ui/plugins/user-management/components/PasswordChangeModal.tsxadmin-ui/plugins/user-management/components/UserDetailViewPage.tsxadmin-ui/plugins/user-management/components/UserEditPage.tsxadmin-ui/plugins/user-management/components/UserForm.tsx
admin-ui/plugins/user-management/__tests__/components/userManagementTestUtils.tsx
Show resolved
Hide resolved
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 (2)
admin-ui/app/routes/Apps/Gluu/GluuInputRow.tsx (1)
150-178:⚠️ Potential issue | 🟠 MajorRemove the stale second password toggle.
This branch already renders the visibility control inside
passwordInputWrapper, but Lines 168-178 still append the old toggle for every password field. That leaves duplicate buttons, breaks the new layout, and keeps a second hardcoded Englisharia-label.🛠️ Proposed fix
- {type === 'password' && ( - <button - type="button" - className={classes.passwordToggle} - onClick={setVisibility} - aria-label="Toggle password visibility" - disabled={disabled} - > - {customType === 'text' ? <Visibility /> : <VisibilityOff />} - </button> - )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/routes/Apps/Gluu/GluuInputRow.tsx` around lines 150 - 178, The second, stale password visibility button block should be removed to avoid duplicate toggles and the hardcoded English aria-label; delete the conditional JSX block that starts with "{type === 'password' && (" and renders a button using classes.passwordToggle, onClick={setVisibility}, disabled={disabled}, and aria-label="Toggle password visibility", since the visibility control is already rendered inside the passwordInputWrapper (where setVisibility, customType and the i18n aria-label via t(...) are used).admin-ui/app/locales/fr/translation.json (1)
1065-1098:⚠️ Potential issue | 🟡 MinorAdd the missing
placeholders.search_herekey.This block was updated for the new search UI, but it does not include
placeholders.search_here, which was added inadmin-ui/app/locales/en/translation.json. French users will fall back to default/raw text anywhere that new placeholder is used.Proposed fix
"id": "Enter id", "search": "Recherche", + "search_here": "Rechercher ici", "action_commit_message": "Fournir la raison de ce changement", "activate_ldap_configuration": "Activer la configuration LDAP",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/locales/fr/translation.json` around lines 1065 - 1098, Add the missing translation key placeholders.search_here to the French translations so the new search UI doesn't fall back to raw text; update the JSON block (near other search-related keys like "search" and "search_pattern") by adding a "placeholders.search_here" entry with the appropriate French text (e.g., "Rechercher ici") to mirror the key present in admin-ui/app/locales/en/translation.json.
♻️ Duplicate comments (1)
admin-ui/app/locales/pt/translation.json (1)
968-970:⚠️ Potential issue | 🟡 MinorStill missing
messages.session_revoke_failedin Portuguese.This block adds
session_revoke_failed_changes_saved, but the intermediatesession_revoke_failedkey is still absent. The password-change flow can still fall back to the raw key or another locale when session revocation fails after a successful password update.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/locales/pt/translation.json` around lines 968 - 970, Add the missing translation key "session_revoke_failed" alongside the existing "session_revoke_failed_changes_saved" and "revokeUserSession" entries; set its value to a concise Portuguese message like "Falha ao revogar a sessão." so the password-change flow can use a localized fallback when session revocation fails (ensure the key name is exactly "session_revoke_failed").
🤖 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/app/locales/fr/translation.json`:
- Around line 1065-1098: Add the missing translation key
placeholders.search_here to the French translations so the new search UI doesn't
fall back to raw text; update the JSON block (near other search-related keys
like "search" and "search_pattern") by adding a "placeholders.search_here" entry
with the appropriate French text (e.g., "Rechercher ici") to mirror the key
present in admin-ui/app/locales/en/translation.json.
In `@admin-ui/app/routes/Apps/Gluu/GluuInputRow.tsx`:
- Around line 150-178: The second, stale password visibility button block should
be removed to avoid duplicate toggles and the hardcoded English aria-label;
delete the conditional JSX block that starts with "{type === 'password' && ("
and renders a button using classes.passwordToggle, onClick={setVisibility},
disabled={disabled}, and aria-label="Toggle password visibility", since the
visibility control is already rendered inside the passwordInputWrapper (where
setVisibility, customType and the i18n aria-label via t(...) are used).
---
Duplicate comments:
In `@admin-ui/app/locales/pt/translation.json`:
- Around line 968-970: Add the missing translation key "session_revoke_failed"
alongside the existing "session_revoke_failed_changes_saved" and
"revokeUserSession" entries; set its value to a concise Portuguese message like
"Falha ao revogar a sessão." so the password-change flow can use a localized
fallback when session revocation fails (ensure the key name is exactly
"session_revoke_failed").
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4b8e3a70-a441-4516-a61d-c414feaaed91
📒 Files selected for processing (7)
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/redux/types/index.tsadmin-ui/app/routes/Apps/Gluu/GluuInputRow.tsxadmin-ui/app/routes/Apps/Gluu/styles/GluuInputRow.style.ts
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>
duttarnab
requested changes
Mar 12, 2026
Contributor
There was a problem hiding this comment.
- This error should be handled silently without showing any toast because this error comes when the modified user is not already signed-in.
- The password change dialog is not closed after the password is successfully changed.
/webhook/users_editis called twice to fetch the webhooks related to user_edit.
4. `/webhook/users_edit` is just used to fetch and display the webhooks on dialog. There is no use to call this endpoint after the save operation.
- Webhooks are not triggered by calling
/admin-ui/webhook/trigger/{featureId}after the user save/delete operation. - This message should be highlighted on change of password, role or status
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>
|
duttarnab
approved these changes
Mar 16, 2026
moabu
approved these changes
Mar 16, 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 User Management module as per Figma (#2630)
Summary
This PR revamps the User Management module 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: #2630
Summary by CodeRabbit
New Features
Improvements
Tests