feat(admin-ui): revamp Audit Logs and reusable Table component as per Figma (#2646)#2654
Merged
feat(admin-ui): revamp Audit Logs and reusable Table component as per Figma (#2646)#2654
Conversation
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 new reusable UI components (GluuDatePicker, GluuTable, GluuSearchToolbar, GluuRefreshButton), replaces many MUI DatePicker usages with GluuDatePicker, removes legacy license Redux/saga/api in favor of a react-query hook, revamps Audit page to use GluuTable+GluuSearchToolbar, and updates theme, utilities, and translations. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Toolbar as GluuSearchToolbar
participant Page as AuditListPage
participant Query as AuditQuery
participant Server as Server
rect rgba(135,206,235,0.5)
User->>Toolbar: set filters / dates / click refresh
Toolbar->>Page: emit filters, dateRange, onRefresh
Page->>Query: call/refetch with params
Query->>Server: HTTP request (params)
Server-->>Query: returns rows + total
Query-->>Page: data
Page->>User: render GluuTable (rows, pagination)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mo-auto
added
comp-admin-ui
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 34
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/License/LicenseDetailsPage.test.tsx (1)
73-81: 🧹 Nitpick | 🔵 TrivialTest only verifies static label text, not dynamic license values.
The test asserts that labels like "Product Name" and "License Type" are present, but doesn't verify the actual values from
mockLicense(e.g.,'Gluu Admin UI','TIME_LIMITED'). Consider adding assertions for rendered values to ensure the hook data actually flows through to the UI.admin-ui/plugins/admin/components/Audit/AuditListPage.tsx (1)
234-249:⚠️ Potential issue | 🟡 MinorBeware of expandedRows state persisting across pagination with non-global row keys.
getAuditRowKeyreturnsrow.idwhich is page-relative (pageNumber * limit + idx + 1), so ids repeat across pages. While React reconciliation is fine since the entire dataset replaces on page change, theexpandedRowsSet in GluuTable will persist between pages. If a row withid=1is expanded on page 1, navigating to page 2 will auto-expand its row withid=1due to the Set lookup, creating unintended expansion behavior.Use a globally unique key for
getAuditRowKey(e.g., combining page number with index, or using a unique property from the audit entry if available) to avoid this state bleed.
🤖 Fix all issues with AI agents
In `@admin-ui/app/components/GluuDatePicker/GluuDatePicker.style.ts`:
- Around line 111-136: The spread of the parameter common (typed SxProps<Theme>)
inside buildTextFieldSx is unsafe because SxProps can be an
array/callback/false; either narrow buildCommonInputStyles to return a plain
Record<string, unknown> (or SxObject) or coerce/assert common to an object
before spreading; update the buildCommonInputStyles return type (or the call
site) to a concrete object type and/or add an explicit type assertion/cast when
spreading common in buildTextFieldSx to ensure TypeScript knows it's a plain
object and avoid unsafe spreads.
- Around line 195-234: Rename the exported composite hook useStyles to a clearer
name like useDatePickerStyles to avoid confusion with makeStyles-generated
hooks; update the export and all internal usages/tests to the new name, ensuring
the function signature and returned object remain unchanged; locate the function
that calls buildPickerThemeColors, useLayoutStyles, buildCommonInputStyles,
buildTextFieldSx, buildPopperSx, and buildDatePickerRootSx and rename that
symbol and its imports/exports accordingly so callers import useDatePickerStyles
instead of useStyles.
- Around line 75-93: titleLabel and titleLabelGrid duplicate most style
properties; extract a shared base (e.g., titleLabelBase) containing fontFamily,
fontSize, fontWeight, color, letterSpacing, display and then create titleLabel
and titleLabelGrid by spreading that base and overriding marginBottom (0 for
titleLabel, '6px' for titleLabelGrid). Update the object where titleLabel and
titleLabelGrid are defined so both reference the shared base to eliminate
duplication.
In `@admin-ui/app/components/GluuDatePicker/GluuDatePicker.tsx`:
- Line 81: The GluuDatePicker's value prop currently forces a default of today
via value={props.value ?? createDate()}, preventing an empty/unselected state;
change the logic in GluuDatePicker to allow null/undefined by removing the
createDate() fallback (use props.value as-is or explicitly allow null) and
ensure the component's prop type/interface for value and any internal handlers
in GluuDatePicker accept and correctly handle null/undefined (and not convert it
to a Date), so optional date use-cases can represent an unselected state.
- Around line 91-111: The memo comparator inside GluuDatePicker (the custom
areEqual function comparing prevProps/nextProps) currently compares single-mode
props but omits minDate and maxDate; update the single-mode branch (where
prev/next are cast to GluuDatePickerSingleProps) to include comparisons for
prev.minDate === next.minDate and prev.maxDate === next.maxDate (using the same
equality style as other primitive/nullable props, e.g., direct === or
null-coalescing pattern) so changes to minDate/maxDate trigger a re-render.
- Around line 152-166: The component currently calls createDate() inside
renderPicker for maxDate which creates a new Dayjs object on every render and
breaks referential stability; fix this by memoizing "today" once at the
component level (e.g., useMemo or useRef inside GluuDatePicker component) and
replace createDate() in renderPicker's maxDate expression with that memoized
today variable (affecting renderPicker, maxDate, and any references to
createDate() used as the "today" value).
- Around line 77-89: The single-date rendering in GluuDatePicker returns a bare
DatePicker (value={props.value ?? createDate()}, onChange={props.onChange},
etc.) but unlike the range path it isn't wrapped in a LocalizationProvider; wrap
that single-mode return in <LocalizationProvider dateAdapter={AdapterDayjs}>
(the same provider used by the range path) so the DatePicker uses the correct
adapter and avoids runtime errors if no ancestor provider exists.
In `@admin-ui/app/components/GluuDatePicker/types.ts`:
- Around line 21-33: The GluuDatePicker props are inconsistent: move the shared
props `isDark` and the date-format property to the common GluuDatePickerBase (or
rename `dateFormat` to `format` across all interfaces) so both
GluuDatePickerRangeProps and GluuDatePickerSingleProps consume the same keys;
update GluuDatePickerBase to include `isDark?: boolean` and a single unified
format property (either `format?: string` or `dateFormat?: string`) and remove
the duplicate from GluuDatePickerRangeProps, then adjust any usages of
`format`/`dateFormat` and `isDark` to the new common name.
In `@admin-ui/app/components/GluuSearchToolbar/GluuRefreshButton.tsx`:
- Around line 8-22: GluuRefreshButtonProps exposes both variant and outlined
which duplicate control of the outlined visual state; remove the outlined prop
and the outlined override to avoid confusion: delete the outlined?: boolean
entry from GluuRefreshButtonProps and change any logic that computes isOutlined
(currently using outlinedProp ?? variant === 'outlined') to derive solely from
variant (e.g., isOutlined = variant === 'outlined'); also remove any usages/read
of outlinedProp elsewhere in GluuRefreshButton.tsx so the button's visual state
is driven only by variant.
- Around line 69-72: The GluuRefreshButton currently renders a Font Awesome <i>
element (`className="fa fa-refresh"`) which mixes icon libraries; replace it
with MUI's RefreshIcon (import RefreshIcon from '@mui/icons-material/Refresh')
inside the GluuRefreshButton component so the codebase stays consistent and
avoids the extra dependency, preserve the conditional spinning behavior by
mapping `loading` to a CSS class or the icon's `sx`/`style` (e.g., apply a
transform/animation when `loading`), and keep the existing sizing by using the
`fontSize` prop or `sx={{ fontSize: fontSizes.md }}`; update any imports to
remove Font Awesome usage and ensure the component still uses the same `loading`
and `fontSizes` symbols.
- Around line 46-53: The code reads themeColors.formFooter.back.* directly when
computing backgroundColor, textColor, and borderColor (see variables
backgroundColor, textColor, borderColor), which can throw if formFooter or back
is undefined; update those expressions to use optional chaining
(themeColors.formFooter?.back?.backgroundColor, ?.textColor, ?.borderColor) and
provide the existing fallbacks (isOutlined ? ... :
themeColors.formFooter?.back?....) so the values safely resolve when
formFooter/back is missing.
In `@admin-ui/app/components/GluuSearchToolbar/GluuSearchToolbar.style.ts`:
- Around line 70-73: The placeholder styling in GluuSearchToolbar.style.ts uses
'&::placeholder' with color: inputColor and opacity: 1, making placeholders
indistinguishable from user input; update the '&::placeholder' rule to use a
muted value (e.g., a dedicated placeholder color variable or theme hint color)
or reduce opacity (e.g., 0.5) instead of inputColor and opacity: 1 so the
placeholder is visually distinct from typed text.
In `@admin-ui/app/components/GluuSearchToolbar/GluuSearchToolbar.tsx`:
- Around line 196-212: The primary action in GluuSearchToolbar currently
defaults to AddIcon when primaryAction.icon is missing (primaryAction.icon ??
<AddIcon ...>), which can be semantically wrong for a search toolbar; update
GluuSearchToolbar to use a neutral/default search-oriented icon or make the icon
required: either replace the AddIcon fallback with a SearchIcon (or another
neutral icon) or remove the hardcoded fallback and enforce consumers to pass
primaryAction.icon (validate in GluuSearchToolbar and render nothing or a
default SVG when absent). Locate the fallback expression (primaryAction.icon) in
GluuSearchToolbar and adjust it, and ensure AuditListPage and other consumers
pass explicit icons if you choose to require them.
- Around line 97-118: The search input in GluuSearchToolbar lacks an accessible
name when searchLabel is falsy; update the input element (the <input> inside
GluuSearchToolbar using props searchLabel, searchPlaceholder, searchValue,
onSearch, handleKeyDown and classes.searchInput) to include an aria-label that
falls back to a sensible value (for example use searchLabel || searchPlaceholder
|| a generic string like "search") so the control is accessible when the visible
label is omitted.
- Around line 120-159: The labels rendered with <GluuText> in
GluuSearchToolbar.tsx are not associated with their controls; update the filter
and dateInput rendering to generate a unique id (e.g., `${filter.key}-select` /
`${dateInput.key}-date`), set that id on the corresponding <select> and <input>,
and replace or wrap <GluuText variant="span"> with a <label htmlFor={id}> (or
add aria-labelledby on the control referencing the label id) so screen readers
can associate the labels with the form controls; apply the same pattern for both
filters (filter.key, filter.onChange) and dateInputs (dateInput.key,
dateInput.onChange).
In `@admin-ui/app/components/GluuSearchToolbar/types.ts`:
- Around line 35-62: GluuSearchToolbarProps currently requires onSearch which
forces consumers to implement a search handler even when they don't use the
search UI; make onSearch optional by updating the GluuSearchToolbarProps
interface so onSearch?: (value: string) => void and ensure any internal usage in
the GluuSearchToolbar component (e.g., where onSearch is called) guards for
undefined before invoking it (use conditional calls or provide a noop fallback)
and update related handlers or tests to handle the optional callback.
In `@admin-ui/app/components/GluuTable/GluuTable.style.ts`:
- Around line 155-170: The paginationSelect style removes the focus outline
leaving no visible focus indicator and harming keyboard accessibility; update
the paginationSelect style (the paginationSelect object) to provide a clear,
high-contrast focus state instead of removing outline — e.g., within the
existing '&:focus' add a visible ring or border change using an accessible color
from themeColors (or themeColors.primary), such as setting a 2px
solid/high-contrast border or a subtle boxShadow ring, ensure outline remains
none only if the new visual cue is present, and keep borderRadius and spacing
consistent so keyboard users can see the focused select.
In `@admin-ui/app/components/GluuTable/GluuTable.tsx`:
- Around line 189-193: Replace the imperative hover handling in GluuTable (the
tr using onMouseEnter/onMouseLeave and classList.add/remove) with a CSS :hover
rule in the module styles by removing the onMouseEnter/onMouseLeave handlers and
adding a nested '&:hover' style to the classes.row definition that applies the
same styles as classes.rowHover; update references to classes.rowHover in the
component to rely on the single classes.row hover rule so no DOM classList
manipulation occurs.
- Around line 266-270: The pagination string can render "1-0 of 0" when
pagination.totalItems is 0; update the rendering in GluuTable.tsx to compute a
guarded start value (e.g., const start = pagination.totalItems === 0 ? 0 :
pagination.page * pagination.rowsPerPage + 1) and use that start and the
existing end calculation (Math.min((pagination.page + 1) *
pagination.rowsPerPage, pagination.totalItems)) in the span instead of the
current unguarded expression so the component displays "0-0 of 0" or "0 of 0"
correctly when totalItems is zero.
- Around line 76-104: The action buttons rendered by renderActionCell lack an
accessible name; update the button element inside the actions.map in
GluuTable.tsx to include an aria-label (e.g., aria-label={action.tooltip ||
action.ariaLabel || action.label}) so screen readers will announce the button
purpose, and if the actions objects supply a stable unique id prefer using that
as the React key (e.g., action.id) instead of idx when available; keep the
existing title for tooltip but ensure aria-label is present for accessibility.
- Around line 194-203: The expandable cell in GluuTable is only clickable and
lacks keyboard support; update the <td> rendering (the element using expandable,
classes.cellExpand, ExpandMoreIcon and isExpanded) to be focusable and
keyboard-operable: add tabIndex={0}, role="button", and
aria-expanded={isExpanded}, and implement an onKeyDown handler that listens for
Enter and Space and calls toggleRow(rowKey) (prevent default for Space to avoid
page scroll); ensure existing onClick(toggleRow(rowKey)) behavior and the expand
icon class logic remain unchanged to preserve visual state.
- Around line 132-140: Replace the span used for sortable headers with a native
button element to satisfy useSemanticElements: when isSortable is true, render a
<button> (styled via classes.sortableHeader) instead of <span role="button">,
remove manual role, tabIndex and onKeyDown Enter handling, and keep the onClick
handler calling handleSort(col.key); ensure sortableHeader CSS resets default
button styles (background/border/padding/font/color) so appearance remains
unchanged; update any references in GluuTable.tsx around isSortable,
classes.sortableHeader, handleSort, and col.key accordingly.
In `@admin-ui/app/components/GluuTable/types.ts`:
- Around line 5-13: The ColumnDef interface currently types key as a plain
string which allows invalid keys; change the typing to constrain keys to the
properties of T (e.g. use Extract<keyof T, string> or keyof T & string) and
update the render signature to be generic over that key so value is correctly
typed (introduce a generic K extends Extract<keyof T, string> or similar and use
ColumnDef<T, K> with key: K and render: (value: T[K], row: T, rowIndex: number)
=> ReactNode) to get stronger compile-time checking while preserving row typing.
In `@admin-ui/app/routes/License/hooks/useLicenseDetails.ts`:
- Around line 27-38: transformLicenseResponse currently strips all double-quote
characters from fields (companyName, customerFirstName, customerLastName,
customerEmail) which may remove intentional quotes inside values; update the
sanitization to remove only surrounding quotes by replacing .replace(/"/g, '')
with a replace using the surrounding-quotes regex (e.g., .replace(/^"|"$/g, '')
) for each referenced field in transformLicenseResponse so internal quotes
remain intact and undefined/null still fall back to '' as before.
- Around line 95-101: The useCallback for resetLicense depends on the entire
mutation object which changes identity; update the dependency to reference the
stable mutation.mutate function instead. In the function defined as resetLicense
(which sets pendingMessageRef.current and calls mutation.mutate()), replace the
dependency array [mutation] with [mutation.mutate] (or directly use mutate from
useLicenseConfigDelete if available) so resetLicense is not recreated when
mutation state (e.g., isPending) changes.
- Around line 72-80: The onSuccess handler in useLicenseDetails.ts currently
awaits postUserAction(audit) which can throw and block the subsequent toast and
onResetSuccess; modify the onSuccess implementation to ensure audit posting
cannot prevent the success path by either wrapping the await
postUserAction(audit) call in a try-catch (logging errors but not rethrowing) or
firing it off without awaiting
(Promise.resolve().then(()=>postUserAction(audit)).catch(()=>{/*log*/})),
keeping the rest of the onSuccess flow (queryClient.invalidateQueries, toast
success, and calling onResetSuccess) guaranteed to run; reference the onSuccess
closure, createAuditLog(currentState), addAdditionalData(audit, DELETION,
API_LICENSE, {}), pendingMessageRef.current, and postUserAction(audit) when
making the change.
- Around line 84-88: The onError handler for useLicenseConfigDelete currently
only handles 403 via isFourZeroThreeError and dispatch(auditLogoutLogs(...)),
swallowing all other errors; modify the onError callback so that after checking
for isFourZeroThreeError(error) it also shows a user-facing error toast for all
non-403 failures (using the app’s toast/notification utility) and optionally log
the error to console or monitoring; keep the existing dispatch(auditLogoutLogs)
behavior for 403s and ensure the non-403 branch does not silently return
(display the toast with a clear message using the project’s notification helper
and include error.message/details).
- Line 19: The code incorrectly imports a narrow RootState from
'@/redux/sagas/types/audit' and uses an unsafe cast for store.getState(); update
useLicenseDetails.ts to import the full application RootState from
'@/redux/types' (or create a dedicated AuditState type) and replace the
double-cast on the result of store.getState() used in createAuditLog so
TypeScript can verify the shape; ensure createAuditLog only reads
state.authReducer (or map the full state to an explicit audit slice) and adjust
the import and type annotations accordingly (references: useLicenseDetails.ts,
createAuditLog, store.getState, RootState).
In `@admin-ui/plugins/admin/components/Audit/AuditListPage.tsx`:
- Line 327: The component is wrapped with React.memo but as a route-level page
component (AuditListPage) that receives no props this memoization is pointless;
remove the memo wrapper and export the component directly by replacing "export
default React.memo(AuditListPage)" with a plain default export of AuditListPage
so the file exports "AuditListPage" without React.memo.
- Around line 76-88: The two independent initializations for
startDateStr/endDateStr and queryParams (using createDate() and
subtractDate(...)) can diverge; extract the initial start and end Dayjs values
into a single stable source (e.g., a useMemo or top-level constant like
initialStartDate and initialEndDate) and use those variables to seed useState
for startDateStr, endDateStr and queryParams (GetAuditDataParams) so
dateConverter(subtractDate(...)), toIsoDate(...) and
queryParams.start_date/queryParams.end_date all derive from the same initial
Dayjs instances.
- Line 94: Replace the unnecessary useMemo wrapper around the boolean OR: the
expression const loading = useMemo(() => isLoading || isFetching, [isLoading,
isFetching]) should be simplified to a plain constant; remove useMemo and assign
loading directly from isLoading || isFetching (referencing the loading variable
and the isLoading/isFetching sources in AuditListPage and the component where
they are defined).
- Around line 139-143: handleRefresh currently sends an empty pattern to the API
via buildQueryParams(limit, 0, '', filterState) but doesn't update the UI state,
causing the search input to still show the old text; update handleRefresh to
either clear the pattern state by calling setPattern('') before setQueryParams
(to match the empty pattern you send) or preserve the current input by passing
pattern instead of '' to buildQueryParams; if you use pattern or setPattern
inside the callback, also include pattern and setPattern in the useCallback
dependency array so handleRefresh stays consistent.
In `@admin-ui/plugins/auth-server/components/Sessions/SessionListPage.tsx`:
- Around line 695-700: The GluuDatePicker here is passing the display format via
the format prop; for range-mode consistency change that prop to dateFormat so it
uses the GluuDatePickerRangeProps shape. Replace
format={DATE_FORMATS.DATE_PICKER_DISPLAY_US} with
dateFormat={DATE_FORMATS.DATE_PICKER_DISPLAY_US} on the GluuDatePicker (the
component using value={filterState.date} and onChange={handleDateChange}) so it
aligns with other range usages and leverages dateFormat ?? format fallback
correctly.
In `@admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx`:
- Around line 344-351: The GluuDatePicker is receiving a redundant label prop
causing the expiration date text to render twice; keep the surrounding
LocalizationProvider but remove the label prop from GluuDatePicker (used inside
the LocalizationProvider where GluuLabel already renders the field label).
Locate the LocalizationProvider/ GluuDatePicker block (and the GluuLabel for
expirationDate) and delete the label={t('fields.expiration_date')} prop on
GluuDatePicker so the UI uses only GluuLabel for the field label.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/plugins/auth-server/components/Sessions/SessionListPage.tsx (1)
390-390:⚠️ Potential issue | 🟡 MinorCSV download filename says "client-tokens" but this is the Sessions page.
The filename should reflect the content being exported (sessions, not client tokens). This looks like a copy-paste artifact from
ClientActiveTokens.Proposed fix
- link.setAttribute('download', `client-tokens.csv`) + link.setAttribute('download', `sessions.csv`)
🤖 Fix all issues with AI agents
In `@admin-ui/app/components/GluuDatePicker/GluuDatePicker.tsx`:
- Around line 91-113: The single-mode memo comparator inside the GluuDatePicker
memo (the arrow function that checks isGluuDatePickerRangeProps and compares
GluuDatePickerSingleProps) currently only compares prev.format === next.format;
update it to compare the effective format the component uses by replacing that
comparison with (prev.dateFormat ?? prev.format) === (next.dateFormat ??
next.format) so changes to dateFormat trigger re-renders (this mirrors the range
comparator and the displayFormat logic).
In `@admin-ui/app/components/GluuDatePicker/types.ts`:
- Around line 3-10: Remove the redundant dateFormat declaration from
GluuDatePickerRangeProps since GluuDatePickerRangeProps already extends
GluuDatePickerBase which declares dateFormat?: string; locate the type
GluuDatePickerRangeProps and delete its duplicate dateFormat property so the
shared format prop remains only on GluuDatePickerBase, then run type checks to
ensure nothing else relied on a separate range-specific declaration.
- Line 6: GluuDatePickerBase declares an isDark prop that is never used by
GluuDatePicker.tsx (the component computes isDarkTheme from theme context) yet
rangePropsEqual still compares a.isDark === b.isDark; either remove isDark from
the GluuDatePickerBase type and from equality checks (rangePropsEqual) to avoid
dead/incorrect comparisons, or update GluuDatePicker.tsx to respect props.isDark
(use props.isDark instead of/in addition to the theme-derived isDarkTheme) and
document its precedence; locate the type in types.ts
(GluuDatePickerBase/isDark), the comparator rangePropsEqual, and the component
GluuDatePicker.tsx to make the consistent change.
In `@admin-ui/app/components/GluuSearchToolbar/GluuSearchToolbar.tsx`:
- Around line 86-92: The primaryButtonColors useMemo reads
themeColors.formFooter.apply.backgroundColor and .textColor without optional
chaining which can crash if formFooter or apply is undefined; update the
selector inside useMemo (primaryButtonColors) to use optional chaining
(themeColors.formFooter?.apply?.backgroundColor and
themeColors.formFooter?.apply?.textColor) and provide sensible fallbacks if
needed, ensuring the change is applied to the useMemo callback that defines
primaryButtonColors.
In `@admin-ui/app/components/GluuSearchToolbar/types.ts`:
- Around line 44-56: Extract the inline dateRange object type into a reusable
named interface (e.g., DateRangeConfig) and replace the inline definition in the
GluuSearchToolbar props with that interface; ensure the new interface matches
the existing shape (startDate, endDate, onStartDateChange, onEndDateChange,
optional onStartDateAccept/onEndDateAccept, dateFormat, layout, labelAsTitle,
inputHeight) and consider aligning or reusing GluuDatePickerRangeProps where
possible to avoid duplicate types.
In `@admin-ui/app/components/GluuTable/GluuTable.style.ts`:
- Around line 116-128: The actionButton style lacks keyboard focus styling;
update the actionButton rule (symbol: actionButton) to add :focus and
:focus-visible states that mirror the pagination select focus indicator (use a
visible outline or box-shadow and ensure outlineOffset and borderRadius align
with existing styling), and keep the existing hover transition; ensure
color/contrast matches themeColors.fontColor and that the focus style is removed
on :focus:not(:focus-visible) if needed for mouse users.
In `@admin-ui/app/components/GluuTable/GluuTable.tsx`:
- Around line 76-106: The renderActionCell useCallback references the i18n
translator t (used as t('fields.actions')) but t is not included in the
dependency array of useCallback; update the dependency list for renderActionCell
to include t so the aria-label fallback updates when language changes (keep
existing deps actions and classes and add t) and ensure any lint rules are
satisfied for renderActionCell, actions, classes, and t.
- Around line 188-206: The expandable cell in GluuTable currently makes a <td>
interactive (role="button", tabIndex, keyboard handlers) which is semantically
incorrect; change the markup so the <td> is non-interactive and contains a
native <button> for the expand control (move aria-expanded, onClick, onKeyDown
and event logic from the <td> to the <button>, keep calling toggleRow(rowKey)),
apply the existing classes (e.g., classes.cellExpand, classes.expandIcon,
classes.expandIconOpen) and sx styling to the button and ExpandMoreIcon so
visual/layout are preserved, and remove role/tabIndex from the <td> to restore
correct HTML semantics.
In `@admin-ui/app/routes/License/hooks/useLicenseDetails.ts`:
- Around line 98-100: In useLicenseDetails.ts update the translation key from
'error_processiong_request' to the correct 'error_processing_request' where
message is assigned (the expression using error instanceof Error and t(...)),
and then update all i18n translation files (e.g., translation.json entries
referenced by the t function) to rename the key accordingly so both code and
locales use 'error_processing_request'; ensure you update any other usages of
the misspelled key across the codebase and run i18n linting/tests.
In `@admin-ui/app/utils/regex.ts`:
- Around line 21-23: The function regexForBracedKey constructs a RegExp from a
dynamic key which can introduce ReDoS or unexpected behavior if key contains
regex metacharacters; to fix, escape all regex-special characters in the key
before interpolating it into the pattern (i.e., transform the incoming key in
regexForBracedKey to a safely-escaped form using a standard escape routine for
characters like . * + ? ^ $ { } ( ) | [ ] \ ) and then build new
RegExp(`\\{${escapedKey}\\}`, 'g') so the generated regex matches the literal
braced key.
In `@admin-ui/plugins/admin/components/Audit/AuditListPage.tsx`:
- Around line 55-56: The module-level constants initialStartDate and
initialEndDate are computed at import time causing stale defaults; move their
computation into the AuditListPage component (or into the useState initializers)
so that createDate() and subtractDate(...) run when the component mounts (e.g.,
compute newStart = subtractDate(createDate(), 14, 'day') and newEnd =
createDate() inside AuditListPage or use a lazy initializer for the state that
references those values) to ensure the default end date reflects "today."
In `@admin-ui/plugins/admin/helper/utils.ts`:
- Around line 17-23: There are two duplicate functions implementing the same
logic: getNestedValue and getNestedProperty; remove one and consolidate usages
to the remaining symbol (e.g., keep getNestedValue), then replace all references
of the removed function (getNestedProperty) with the chosen function name
throughout the file and delete the redundant implementation; ensure the kept
function signature and behavior remain unchanged (input types Record<string,
JsonValue> and path: string, returning JsonValue | undefined) so callers
continue to work.
- Around line 41-47: The utility clearInputById performs direct DOM mutation and
can desync React-controlled inputs; replace it with a React-friendly API: remove
direct getElementById usage in clearInputById and instead provide a function
that either (a) accepts a React state setter (e.g., setValue: (v:string)=>void)
and calls setValue('') or (b) accepts an input ref
(React.RefObject<HTMLInputElement>) and calls ref.current?.focus() or
ref.current!.value = '' only for uncontrolled refs; update call sites to pass
the component's state setter or ref and rename the function to something like
clearControlledInput to make intent explicit. Ensure the unique symbol
clearInputById is changed or overloaded so all usages are updated to the new
signature to avoid direct DOM writes.
- Line 25: The hasValue<T> type guard currently uses a falsy check (!!value)
which incorrectly excludes valid falsy values like 0, '', false, and NaN while
claiming to return NonNullable<T>; change the runtime check in hasValue to
explicitly test for null and undefined (value !== null && value !== undefined)
so the type guard only excludes null/undefined while preserving valid falsy
values.
In `@admin-ui/plugins/schema/components/Person/AttributeViewPage.tsx`:
- Line 11: The import REGEX_LEADING_COLON in AttributeViewPage.tsx is unused and
not exported from `@/utils/regex`; either remove this unused import or define and
export REGEX_LEADING_COLON in your regex module and replace the inline /^:/
usage with that constant. Locate the import statement for REGEX_LEADING_COLON
and the inline regex usage (the /^:/ in AttributeViewPage.tsx) and either delete
the import if you keep the inline regex, or add/export REGEX_LEADING_COLON from
`@/utils/regex` and update the code to use REGEX_LEADING_COLON instead of /^:/ to
keep things consistent.
duttarnab
requested changes
Feb 12, 2026
Contributor
duttarnab
left a comment
duttarnab
left a comment
There was a problem hiding this comment.
- The title of the Audit Log screen should be
Admin UI Audit Log. - The Date range also should have time selection.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Fix all issues with AI agents
In `@admin-ui/app/components/GluuSearchToolbar/GluuSearchToolbar.tsx`:
- Around line 36-38: GluuSearchToolbar currently defaults searchPlaceholder to
the hardcoded English string 'Search Pattern'; change this so the default uses
the translation system instead of a literal or make the prop required: update
the GluuSearchToolbar component to set the default for searchPlaceholder via the
i18n translator (e.g., call the component's t function or useTranslation hook)
or remove the default and mark searchPlaceholder as required on
GluuSearchToolbarProps so callers must supply a translated string; locate the
searchPlaceholder default in the GluuSearchToolbar function and replace it
accordingly.
- Around line 65-71: The defaultDates object in GluuSearchToolbar is memoized
with an empty dependency array so createDate() runs only once at mount, causing
the "end" date to become stale in long-lived SPA sessions; replace the
useMemo(() => ({ start: subtractDate(createDate(), 14, 'day'), end: createDate()
}), []) usage so defaultDates is recomputed when needed — either remove useMemo
and compute the dates on render or derive them in an effect that runs on each
mount/activation; update references to defaultDates accordingly (functions:
defaultDates, createDate, subtractDate, useMemo) so the "end" date reflects the
current day rather than the original mount day.
- Line 184: The backgroundColor fallback can throw if themeColors.card is
undefined; update the expression used in the GluuSearchToolbar backgroundColor
prop so the fallback uses optional chaining (e.g., use
themeColors.settings?.cardBackground ?? themeColors.card?.background) so
accessing card.background is safe — locate the backgroundColor prop in
GluuSearchToolbar.tsx and change the fallback from themeColors.card.background
to themeColors.card?.background (or supply a sensible default after that).
In `@admin-ui/app/components/GluuTable/GluuTable.style.ts`:
- Around line 35-44: The wrapper style currently sets overflowX: 'hidden' which
can clip table content on narrow viewports; change the overflowX value in the
wrapper style object to 'auto' so horizontal scrollbars appear when needed
(update the wrapper style's overflowX property accordingly; look for the wrapper
object in GluuTable.style.ts).
- Around line 105-112: The cellExpand style currently sets display: 'flex' on
the <td>, which breaks table layout; remove display:'flex' (and any
flex-specific alignItems/justifyContent) from the cellExpand rule in
GluuTable.style.ts and instead add a new class (e.g., cellExpandInner) that
applies the flex layout; then in GluuTable.tsx wrap the <GluuButton> (or
whatever content uses classes.cellExpand) with a <div
className={classes.cellExpandInner}> so the <td> retains table-cell behavior
while the inner div manages flex alignment.
In `@admin-ui/app/components/GluuTable/GluuTable.tsx`:
- Around line 18-29: T_KEYS contains references to fields.expand_row and
messages.collapse/messages.expand that are missing from the locale files, so add
these keys to the translation JSONs (at least
admin-ui/app/locales/en/translation.json and other locale files) with
appropriate localized strings; update the JSON entries matching the nested
structure (e.g., "fields": { "expand_row": "…" } and "messages": { "collapse":
"…", "expand": "…" }) so GluuTable's use of T_KEYS and its defaultValue
fallbacks no longer rely on hardcoded English.
- Around line 33-61: expandedRows (useState) is never cleared when the data prop
changes, causing stale expansions to carry over between datasets; add an effect
that watches data (and optionally getRowKey) and calls setExpandedRows(new
Set()) to reset the expandedRows whenever the table data reference changes so
expansions don't persist across pages or refetches.
In `@admin-ui/app/routes/License/hooks/useLicenseDetails.ts`:
- Line 79: The current double-cast in useLicenseDetails (store.getState() as
unknown as RootState) exists because the Redux store was created with
`@ts-nocheck` and not typed; remove the `@ts-nocheck` in the store module and update
the store creation to use the generic configureStore<RootState>({...}) (and
export the RootState type from that module) so getState() is correctly inferred;
then remove the cast in useLicenseDetails and rely on the now-typed
store.getState() return type.
In `@admin-ui/app/utils/pagingUtils.ts`:
- Around line 38-53: The savePagingSize function currently accepts any positive
integer but getDefaultPagingSize ignores values not in ROWS_PER_PAGE_OPTIONS;
update savePagingSize to validate the computed validSize against
ROWS_PER_PAGE_OPTIONS before persisting (use the same constant/array used by
getDefaultPagingSize), and only call localStorage.setItem(STORAGE_KEY,
String(validSize)) when the size is a member of ROWS_PER_PAGE_OPTIONS; if the
size is invalid, log a warning and return without writing.
In `@admin-ui/plugins/admin/components/Audit/AuditListPage.style.ts`:
- Around line 21-32: The styles for searchCard (and nearby rules) use hardcoded
paddings ('24px 20px' and '20px') and fragile nth-child column paddings; replace
those literal values with the shared spacing constants (e.g.,
SPACING.CARD_PADDING and other SPACING values) and update searchCard to compute
padding using those constants, and remove/replace the nth-child-based
column-specific rules with class-based selectors or column-specific class names
(instead of nth-child) so column reordering/addition won't break layout; update
references in AuditListPage.style.ts (searchCard, any nth-child rules) to use
SPACING constants and stable selectors.
In `@admin-ui/plugins/admin/components/Audit/AuditListPage.tsx`:
- Around line 210-217: The dateBadgeColors useMemo currently accesses
themeColors.formFooter.apply.* directly which can crash if formFooter or apply
is undefined; update the accessor in the useMemo for dateBadgeColors (the
useMemo block that creates backgroundColor, textColor, borderColor) to use
optional chaining like themeColors.formFooter?.apply?.backgroundColor,
themeColors.formFooter?.apply?.textColor, and
themeColors.formFooter?.apply?.borderColor so missing theme segments return
undefined instead of throwing; keep the same dependency array ([themeColors])
and preserve the useMemo wrapper and variable name dateBadgeColors.
- Around line 369-377: Add a short inline comment next to the GluuTable usage
explaining why loading is explicitly set to false (because the outer GluuLoader
handles the blocking overlay for the whole page) so future maintainers won't
remove or change it; reference the GluuTable component prop (loading={false}) in
AuditListPage and mention the surrounding GluuLoader responsibility and that
table-level loading states are intentionally suppressed (keep the comment
adjacent to the GluuTable element that uses columns, data/auditRows,
getAuditRowKey, and pagination).
In `@admin-ui/plugins/admin/components/Settings/SettingsPage.style.ts`:
- Around line 148-151: The disabled select styling (`& select:disabled`) is
incomplete compared to the disabled input rule (`& input:disabled`); update the
`& select:disabled` rule to explicitly set `backgroundColor`, `border`, and
`color` with `!important` (matching the `& input:disabled` overrides) in
addition to `opacity` and `cursor` so browser UA styles can't gray or alter
disabled selects inconsistently.
In `@admin-ui/plugins/admin/helper/utils.ts`:
- Line 15: The type JsonValue currently omits arrays causing array-valued JSON
to be mis-typed; update the type definitions so arrays are allowed by changing
JsonValue to include JsonValue[] and ensure JsonObject's value type references
JsonValue (so nested objects/arrays are both supported), then run a quick
typecheck and review usages in getNestedValue and webhookOutputObject to confirm
they accept arrays for traversal and output.
- Around line 86-101: The replacement loop uses the original templateValue for
every placeholder, so earlier replacements get lost; change the logic in the
block handling REGEX_BRACED_PLACEHOLDER to accumulate replacements by reading
the current value from webhook.httpRequestBody[key] (or initialize a local
variable like currentValue = templateValue) and then for each placeholder run
.replace against that currentValue, assigning the result back to
webhook.httpRequestBody[key] (and to currentValue) and updating
shortcodeValueMap[placeholderKey] accordingly; reference the symbols
REGEX_BRACED_PLACEHOLDER, templateValue, webhook.httpRequestBody[key],
regexForBracedKey, shortcodeValueMap, getNestedValue, and createdFeatureValue
when making the change.
admin-ui/app/components/GluuSearchToolbar/GluuSearchToolbar.tsx
Outdated
Show resolved
Hide resolved
admin-ui/app/components/GluuSearchToolbar/GluuSearchToolbar.tsx
Outdated
Show resolved
Hide resolved
admin-ui/app/components/GluuSearchToolbar/GluuSearchToolbar.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@admin-ui/app/components/GluuTable/GluuTable.style.ts`:
- Around line 223-240: Replace the current focus rule on paginationSelect (the
"&:focus" block) with a ":focus-visible" rule to match the actionButton
pattern—i.e., change the selector to "&:focus-visible" and keep the same
outline, boxShadow (using paginationAccent) and border styling so the keyboard
focus ring remains but mouse clicks don't show it; ensure you update the
paginationSelect block in GluuTable.style.ts accordingly.
In `@admin-ui/app/components/GluuTable/GluuTable.tsx`:
- Around line 333-367: Extract the repeated page-bound checks into local
booleans: define const isFirstPage = pagination.page === 0 and const isLastPage
= (pagination.page + 1) * pagination.rowsPerPage >= pagination.totalItems near
the top of the pagination block, and then replace the three occurrences in the
"next" GluuButton (className, disabled, textColor) and the two occurrences in
the "previous" GluuButton with those variables; keep existing references to
pagination.onPageChange, classes.paginationButtonDisabled, themeColors and
T_KEYS.MESSAGES_PREVIOUS_PAGE / T_KEYS.MESSAGES_NEXT_PAGE unchanged so behavior
and labels remain identical.
In `@admin-ui/plugins/admin/components/Audit/AuditListPage.style.ts`:
- Line 48: In AuditListPage.style.ts update the CSS for the container that
currently sets 'overflowX': 'hidden' (same area referenced in
GluuTable.style.ts) to use 'overflowX': 'auto' instead so the table shows a
horizontal scrollbar on narrow viewports instead of silently clipping content;
locate the style object or export where 'overflowX' is set and replace 'hidden'
with 'auto' so scrollbars appear only when needed.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
admin-ui/app/components/GluuButton/GluuButton.tsx (1)
53-109:⚠️ Potential issue | 🟡 Minor
disableHoverStylesmissing fromuseMemodependency array.
disableHoverStylesis consumed at Line 58 (keepBgOnHover) and Line 74 (resolveBackgroundColor) but is not listed in the dependency array (Lines 90–109). If this prop were toggled at runtime, the memoized style would be stale.Proposed fix
], [ sizeConfig, outlined, isHovered, isDisabled, block, themeColors, isDark, backgroundColor, textColor, borderColor, borderRadius, fontSize, fontWeight, padding, minHeight, useOpacityOnHover, hoverOpacity, + disableHoverStyles, style, ])admin-ui/app/locales/es/translation.json (1)
8-8:⚠️ Potential issue | 🟡 MinorPre-existing concatenation bug in
add_attribute_mapping.The value
"AceptarAgregar mapeo de atributos"appears to be an accidental merge of"Aceptar"(Accept) and"Agregar mapeo de atributos"(Add Attribute Mapping). This predates this PR but will display incorrectly.Proposed fix
- "add_attribute_mapping": "AceptarAgregar mapeo de atributos", + "add_attribute_mapping": "Agregar mapeo de atributos",
🤖 Fix all issues with AI agents
In `@admin-ui/app/components/GluuSearchToolbar/GluuSearchToolbar.tsx`:
- Around line 80-87: handleKeyDown currently closes over searchValue and lists
it in the useCallback deps, causing the callback to be recreated on every
keystroke; to fix, stop capturing the changing state: create a ref (e.g.,
searchValueRef) that you update whenever searchValue changes (or from the
onChange handler) and then change handleKeyDown to read searchValueRef.current
instead, keeping useCallback deps to [onSearchSubmit] only; alternatively, if
feasible, change onSearchSubmit to not take an argument and let the parent read
its own state so handleKeyDown can call onSearchSubmit() without referencing
searchValue.
In `@admin-ui/app/components/GluuTable/GluuTable.style.ts`:
- Line 28: The style file computes paginationAccent by directly dereferencing
themeColors.formFooter.back.backgroundColor which can throw if formFooter or
back is undefined; change the paginationAccent assignment to use optional
chaining (themeColors.formFooter?.back?.backgroundColor) and provide a sensible
fallback (e.g., a default color or themeColors.primary) so it matches the
defensive access used in GluuTable.tsx and cannot crash during style
computation.
- Around line 204-213: The paginationBar style duplicates the wrapper's full
border causing a double border; edit the paginationBar object (not wrapper) to
remove the full border property and replace it with only a top border: remove
the existing border: `1px solid ${rowBorder}` and add `borderTop: \`1px solid
${rowBorder}\`` so you keep the top separator while preserving
`borderBottomLeftRadius`, `borderBottomRightRadius`, `backgroundColor: rowBg`,
and other layout props in paginationBar.
In `@admin-ui/app/components/GluuTable/GluuTable.tsx`:
- Around line 229-232: The aria-label currently always resolves to
T_KEYS.FIELDS_EXPAND_ROW (so defaultValue toggles but translation doesn't);
update the aria-label logic in GluuTable.tsx to pick the correct translation key
based on isExpanded, e.g., call t(isExpanded ? MESSAGES_COLLAPSE :
MESSAGES_EXPAND) (the same keys used for title), so the label toggles between
"Collapse row" and "Expand row" in the current locale; ensure you reference the
same t, isExpanded, MESSAGES_COLLAPSE and MESSAGES_EXPAND symbols already
present in the component.
In `@admin-ui/app/routes/License/hooks/useLicenseDetails.ts`:
- Around line 75-106: The onSuccess block calls postUserAction and currently
logs errors unconditionally via console.error('License reset audit post
failed:', e) while onError only logs when process.env.NODE_ENV ===
'development'; make these consistent by gating the onSuccess console.error
behind the same NODE_ENV === 'development' check (i.e., in the onSuccess handler
around the catch for postUserAction), keeping all other logic (createAuditLog,
addAdditionalData, pendingMessageRef, dispatch(updateToast), onResetSuccessRef)
unchanged.
In `@admin-ui/plugins/admin/components/Audit/AuditListPage.tsx`:
- Around line 60-63: The useMemo call creating canReadAuditLogs unnecessarily
includes the module-level constant AUDIT_LOGS_RESOURCE_ID in its dependency
array; remove AUDIT_LOGS_RESOURCE_ID from the deps so the hook only depends on
hasCedarReadPermission (i.e., change the dependency array for useMemo to
[hasCedarReadPermission]) to avoid a redundant dependency while keeping
hasCedarReadPermission as the actual reactive dependency in AuditListPage.tsx.
duttarnab
requested changes
Feb 13, 2026
Contributor
duttarnab
left a comment
duttarnab
left a comment
There was a problem hiding this comment.
The date-time range search and long pattern will be include once the API supports date-time range search.
duttarnab
previously approved these changes
Feb 13, 2026
|
moabu
approved these changes
Feb 13, 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 Audit Logs and reusable Table component as per Figma (#2646)
Summary
This PR revamps the Audit Logs screen and refactors the reusable Table component to align with the latest Figma designs as part of the ongoing Admin UI redesign initiative.
Changes Included
Audit Logs
Reusable Table Component
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: #2646
Summary by CodeRabbit
New Features
Improvements
Translations
Removals