fix(admin-ui): Use typescript generated client for Services plugin #2564
Conversation
syntrydy
added
comp-admin-ui
|
Warning Rate limit exceeded@syntrydy has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 12 minutes and 41 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces handwritten Redux slices/sagas and promise-based API wrappers in the Services plugin with TypeScript-generated JansConfigApi + React Query + Jotai; adds typed TSX pages/components, Formik/Yup forms, audit hooks, atoms, and i18n keys; removes legacy Redux wiring, saga tests, and JS API clients. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI (Page / Form)
participant RQ as React Query (mutation)
participant API as JansConfig API (generated client)
participant Audit as Audit Logger (use*Audit)
participant QC as QueryClient (cache)
participant Toast as Toasts
rect `#E8F6FF`
UI->>RQ: trigger mutation (create/update/delete/test)
end
RQ->>API: call generated endpoint
alt success
API-->>RQ: 200 + data
RQ->>Audit: log action (CREATE/UPDATE/DELETE)
Audit-->>RQ: ack
RQ->>QC: invalidate/refresh queries
QC-->>UI: refetched data
RQ->>Toast: show success
else error
API-->>RQ: error
RQ->>Toast: show error
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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. 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}
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 (1)
admin-ui/plugins/auth-server/components/AuthN/AuthNListPage.tsx (1)
127-141: Object reference stability may cause unnecessary effect triggers.The
ldapvariable is recreated on every render due to line 63 (const ldap = (ldapData || []) as AuthNItem[]), which will trigger this effect on every render even when the actual data hasn't changed. Consider memoizing theldapvalue or usingldapDatadirectly in the dependency array.🔎 Proposed fix to stabilize dependencies
- const ldap = (ldapData || []) as AuthNItem[] - const loading = ldapLoading + const ldap = useMemo(() => (ldapData || []) as AuthNItem[], [ldapData]) + const loading = ldapLoadingThen update the effect dependency:
- }, [ldap, loading]) + }, [ldap, loading])With
useMemo, theldapreference will only change whenldapDatachanges.
🤖 Fix all issues with AI Agents
In @admin-ui/plugins/auth-server/components/AuthN/AuthNListPage.tsx:
- Around line 60-64: The code casts ldapData to AuthNItem[] which hides a type
mismatch—use the correct GluuLdapConfiguration type returned by
useGetConfigDatabaseLdap instead of forcing AuthNItem: replace the cast on
ldapData and type the ldap variable as GluuLdapConfiguration[] (or import
GluuLdapConfiguration and assign ldapData to that type), or add a small mapper
function to convert GluuLdapConfiguration -> AuthNItem and call it (reference
ldapData, ldapLoading, useGetConfigDatabaseLdap, AuthNItem,
GluuLdapConfiguration, and LdapListPage.tsx for the correct usage example).
In @admin-ui/plugins/services/Components/Configuration/CacheInMemory.tsx:
- Around line 9-11: The CacheInMemoryProps interface in CacheInMemory.tsx is
missing the config: InMemoryConfiguration property present in the shared
interface in types.ts; either import and use the shared interface or add the
missing property to match. Update CacheInMemory.tsx to import the shared
interface (e.g., CacheInMemoryProps from ./types) and replace the local
interface, or modify the local CacheInMemoryProps to include config:
InMemoryConfiguration so it aligns with the shared type used elsewhere
(referencing CacheInMemoryProps, CacheFormValues, and InMemoryConfiguration).
In @admin-ui/plugins/services/Components/Configuration/CacheMemcached.tsx:
- Around line 13-16: Replace the local CacheMemcachedProps interface in
CacheMemcached.tsx by importing the existing CacheMemcachedProps from ./types.ts
and remove the duplicated interface declaration; ensure the import brings in the
same symbols (CacheMemcachedProps and any dependent types like
MemcachedConfiguration or CacheFormValues if needed) and update any references
in the file to use the imported interface.
In @admin-ui/plugins/services/Components/Configuration/CacheNative.tsx:
- Around line 10-12: Remove the local duplicate interface CacheNativeProps and
import the exported CacheNativeProps from types.ts (which includes
NativePersistenceConfiguration); then update the component signature to use the
imported CacheNativeProps (so the component accepts the same shape as declared
in types.ts) and remove any local-only definition; if the component truly only
needs formik, ensure the exported interface in types.ts is adjusted there
(remove config) or narrow the prop usage to match the exported type, and update
CachePage.tsx usages accordingly to satisfy the imported type.
In @admin-ui/plugins/services/Components/Configuration/CachePage.tsx:
- Line 333: Remove the empty <FormGroup row></FormGroup> used solely for spacing
inside the CachePage component and replace it with intentional layout spacing:
either apply a margin/padding CSS class or inline style to the surrounding
element (or add a named spacer CSS class) so the spacing intent is explicit;
update the CachePage TSX where the FormGroup is used and remove the empty
FormGroup element, adjusting the container (or a nearby element) to use a
margin/padding rule instead.
- Around line 219-293: The onSubmit handler can log success even if a later
mutation fails; ensure the audit log only runs after all mutations complete
successfully by moving the logCacheUpdate call to after the patchCacheMutation
and all provider-specific mutateAsync awaits, or by checking that each await
(putNativeMutation.mutateAsync, putMemoryMutation.mutateAsync,
putRedisMutation.mutateAsync, putMemcachedMutation.mutateAsync,
patchCacheMutation.mutateAsync) resolved without throwing before calling
logCacheUpdate; alternatively wrap each mutation in its own try/catch and
rethrow on failure so the outer catch prevents logging — update onSubmit to call
logCacheUpdate only after all awaits succeed.
- Around line 158-181: INITIAL_VALUES is recreated on every render causing
Formik (when using enableReinitialize) to reinitialize unnecessarily; wrap the
INITIAL_VALUES object in a React.useMemo (e.g., const INITIAL_VALUES =
useMemo(() => ({ ... }), [cacheData, cacheMemData, cacheMemoryData,
cacheRedisData, cacheNativeData])) so it only changes when its dependent cache*
data objects change, and then pass that memoized INITIAL_VALUES into Formik.
- Around line 104-146: The four PUT mutation hooks (putMemoryMutation,
putMemcachedMutation, putNativeMutation, putRedisMutation) duplicate identical
onSuccess/onError callbacks; extract a shared pair of handlers (e.g.,
handlePutSuccess and handlePutError) that call dispatch(updateToast(true,
'success')) and dispatch(updateToast(true, 'danger')) respectively, then pass
these handlers into each
usePutConfigCacheInMemory/usePutConfigCacheMemcached/usePutConfigCacheNativePersistence/usePutConfigCacheRedis
hook via their mutation config to remove the duplication.
In @admin-ui/plugins/services/Components/Configuration/CacheRedis.tsx:
- Around line 14-17: The CacheRedisProps interface is being redeclared in
CacheRedis.tsx; remove the local declaration and import CacheRedisProps from the
existing types module (where it's declared as CacheRedisProps in types.ts) so
the component uses the shared interface; update the import list in
CacheRedis.tsx to include CacheRedisProps and delete the duplicate interface
block to avoid drift.
In @admin-ui/plugins/services/Components/Configuration/CouchbaseItem.tsx:
- Around line 12-16: The local CouchbaseItemProps interface in CouchbaseItem.tsx
conflicts with the shared type in types.ts: it uses formik:
FormikProps<CouchbaseConfiguration[]> and declares index while types.ts defines
formik: FormikProps<CouchbaseFormValues> and omits index; to fix, either import
and reuse the shared interface from types.ts (or import CouchbaseFormValues and
change the local prop to formik: FormikProps<CouchbaseFormValues> and add index
to the shared type) so both files agree, or update types.ts to include the index
property and/or accept CouchbaseConfiguration[]—ensure CouchbaseItemProps (or
the imported/shared type) is the single source of truth and update any
references to FormikProps type accordingly.
In @admin-ui/plugins/services/Components/Configuration/CouchbasePage.tsx:
- Around line 83-90: The list rendering in CouchbasePage.tsx uses the array
index as the React key which can break reconciliation; change the key on the
CouchbaseItem in the couchbase.map to use a stable unique identifier from the
couchbase item (e.g., couchbaseItem.configId or another unique property on
CouchbaseConfiguration) instead of index, and if items might lack that property
ensure the data model provides a stable id (or generate/persist one when items
are created) so CouchbaseItem receives key={couchbaseItem.configId} (and keep
passing index as the index prop only if still needed).
- Around line 53-64: handleSubmit currently updates Couchbase configs
sequentially and aborts on the first failure; change it to run all updates
concurrently with Promise.allSettled so each config is attempted and you can
gather which edits succeeded or failed. For each item in values call
editMutation.mutateAsync and logCouchbaseUpdate, collect the results from
Promise.allSettled, then process the settled results to log or surface which
config IDs succeeded and which failed (including error details) and return or
display an aggregate success/failure summary; keep references to handleSubmit,
editMutation.mutateAsync and logCouchbaseUpdate when implementing this change.
In @admin-ui/plugins/services/Components/Configuration/hooks/index.ts:
- Around line 82-95: The delete audit helpers (logLdapDelete and logSqlDelete)
currently only send configId; change their signatures to accept the full deleted
object (e.g., ldapConfig / sqlConfig) and include that object in the
logAuditUserAction call (either as message detail and/or a dedicated payload
field) so the complete deleted state is recorded; update callers to pass the
full object instead of just configId, and ensure the dependency arrays and types
for useCallback (token, userinfo, client_id) remain correct.
- Around line 34-237: The four hooks (useLdapAudit, useSqlAudit, useCacheAudit,
useCouchbaseAudit) duplicate selecting auth state; extract that into a shared
hook (e.g., useAuthContext) that returns token, client_id, and userinfo, then
replace the repeated lines in each audit hook with a call to useAuthContext;
update dependencies of useCallback closures to reference token, client_id, and
userinfo from useAuthContext to keep behavior identical.
- Around line 40-95: Wrap the calls to logAuditUserAction inside try-catch in
each of the three hooks (logLdapCreate, logLdapUpdate, logLdapDelete) so audit
failures are fire-and-forget: call logAuditUserAction as before but catch any
thrown error and handle it silently or log it (e.g., console.error or existing
logger) without rethrowing, ensuring the UI flow is not disrupted by audit
errors.
- Around line 17-32: Replace the duplicated local interfaces by importing the
canonical types: remove the local AuthState and RootState declarations and
import AuthRootState (or the exact named exports) from
admin-ui/app/utils/types.ts; update any references in this file to use the
imported AuthRootState (or AuthState/RootState names exported there) so the
reducer selector/type annotations match the central type (including required
fields like location) and avoid making fields optional locally.
In @admin-ui/plugins/services/Components/Configuration/LdapAddPage.tsx:
- Around line 49-54: The defaultConfigurations object is created inside the
LdapAddPage component causing it to be re-instantiated on every render; move it
to a module-level constant (e.g., export const DEFAULT_LDAP_CONFIG or const
DEFAULT_CONFIG) at the top of the file and replace the in-component usage with
that constant so the component (and any tests) can import/reuse the canonical
default configuration instead of recreating it each render.
In @admin-ui/plugins/services/Components/Configuration/LdapDetailPage.tsx:
- Around line 89-95: In LdapDetailPage remove the unintended raw boolean
rendering by deleting the `{row.useSSL}` expression so only the Badge remains;
keep the existing Badge usage and getBadgeTheme(row.useSSL) and continue to
display `{row.useSSL ? t('options.true') : t('options.false')}` inside the
Badge.
- Around line 8-11: Remove the duplicate LdapDetailPageProps interface from
LdapDetailPage.tsx and instead import the existing type from types.ts; replace
the local interface usage with the imported type (keep referencing
GluuLdapConfiguration and the testLdapConnection signature) and update the
component props annotation to use the imported LdapDetailPageProps (or directly
use the imported types) so the file uses the single source-of-truth type from
types.ts.
- Around line 127-128: Replace the hardcoded boolean strings in the
LdapDetailPage JSX with i18n keys: change the Label that renders
row.useAnonymousBind (currently using 'true' : 'false') to use the translation
function, e.g. row.useAnonymousBind ? t('options.true') : t('options.false') (or
t('options.enabled') if that matches the existing local convention), so the
display is consistent with other boolean labels in LdapDetailPage.
In @admin-ui/plugins/services/Components/Configuration/LdapForm.tsx:
- Around line 48-52: The helper function named toogle is a typo and unclear;
rename it to a descriptive name like activateValidation (or enableInit) and
update all references to it so its purpose—calling setInit(true) on first
interaction when init is false—is explicit; ensure the function signature
(activateValidation(): void) and any invocations (e.g., onClick/onChange
handlers) replace toogle, and run a quick search for the old symbol to update
usages consistent with the same fix applied in SqlForm.tsx.
- Around line 69-75: The submitForm function is using direct DOM access
(getElementsByClassName and click) which is fragile; replace that with Formik's
programmatic submission: obtain the Formik context (via useFormikContext or by
accepting the formik bag/submitForm prop) and call formik.submitForm() instead
of clicking the DOM button, and then call toggle at the appropriate time (either
before or after awaiting formik.submitForm() to preserve the original UX).
Update the submitForm function to reference submitForm from Formik (or accept it
as an argument) and remove direct DOM manipulation.
- Around line 103-133: The normalization of servers and baseDNs is duplicated
between onSubmit and checkLdapConnection; extract this into one or two helper
functions (e.g., normalizeServers(items: (string|ServerItem)[]): string[] and
normalizeBaseDNs(items: (string|BaseDnItem)[]): string[]) or a single generic
normalizeList helper, implement the same typeof/object check and property access
used now, and replace the inline map calls in both onSubmit and
checkLdapConnection to call the new helper(s) so the logic is centralized and
reusable.
In @admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx:
- Line 106: The state variable item is initialized to an empty object that may
not satisfy the GluuLdapConfiguration type; change its type to
GluuLdapConfiguration | null and initialize with null (update const [item,
setItem] = useState<GluuLdapConfiguration | null>(null)), then update any
usages—especially the deletion handler and the confirmation dialog code that
reads item (e.g., functions named like onDeleteClick, handleDelete, confirm
dialog render) to null-check item before accessing its properties and to handle
the null case (disable buttons or early-return) so runtime errors are avoided.
- Line 80: The code currently asserts persistenceData as PersistenceInfo and
reads persistenceType which can mask API shape mismatches; add a runtime type
guard (e.g., isPersistenceInfo(obj): obj is PersistenceInfo) and use it when
deriving persistenceType (replace the cast in the expression that defines
persistenceType) or explicitly check for null/undefined before accessing
persistenceType so you only read the property when the runtime shape matches;
update references to persistenceData/persistenceType in LdapListPage.tsx to use
the guard/check and handle the fallback (undefined or safe default) when the
check fails.
In @admin-ui/plugins/services/Components/Configuration/SqlDetailPage.tsx:
- Around line 8-11: The local interface SqlDetailPageProps declares
testSqlConnection as optional but the shared type in ./types.ts defines it as
required; reconcile them by either making testSqlConnection optional in the
shared type or removing the optional modifier locally so both use the same
contract—update the SqlDetailPageProps or the shared interface in ./types.ts
(lines around the testSqlConnection definition) so the signature for
testSqlConnection(row: SqlConfiguration) is consistent across the codebase.
In @admin-ui/plugins/services/Components/Configuration/SqlEditPage.tsx:
- Around line 68-72: SqlEditPage passes a shallow-copied item to SqlForm which
can allow nested mutations to affect atom state; change the prop to pass a deep
clone like LdapEditPage does using cloneDeep(item) so Formik edits cannot mutate
the original atom. Locate SqlEditPage where SqlForm is instantiated (props:
item, handleSubmit, isLoading) and replace the shallow spread ({ ...item }) with
a deep clone via cloneDeep(item) and ensure cloneDeep is imported from lodash
(or your project utility) to keep behavior consistent with LdapEditPage.
In @admin-ui/plugins/services/Components/Configuration/SqlForm.tsx:
- Around line 59-65: Replace hardcoded validation messages in the
validationSchema with translated strings by using the translation function from
useTranslation (t); update the Yup validators in validationSchema (e.g.,
configId, userName, userPassword, connectionUri, schemaName) to call
t('validation.required') for required messages and
t('validation.min_characters', { count: 2 }) for min() messages so all
validation text is localized and consistent with the component's use of
useTranslation.
- Around line 37-43: The submitForm function currently uses
document.getElementsByClassName('UserActionSubmitButton') and click(), which
bypasses React/Formik; change it to call Formik's submitForm (e.g.
formik.submitForm()) or invoke the submit handler passed via props instead of
DOM click, and ensure submitForm is defined where it can access the formik
instance (move its declaration to after the formik hook/variable or accept
formik/submit handler as a parameter/prop); keep the existing toggle() call but
remove any direct DOM manipulation of UserActionSubmitButton.
- Around line 27-31: Rename the misspelled function to a clear name like
activateValidation (replace function toogle() with function
activateValidation()) and update all callers (e.g., onKeyUp={toogle}) to
onKeyUp={activateValidation}; ensure you don't conflict with the existing toggle
function (keep the original toggle intact) and adjust any exports or bindings
that referenced toogle so the new name is used everywhere.
In @admin-ui/plugins/services/Components/Configuration/SqlListPage.tsx:
- Around line 36-40: AlertState currently types severity as '' which is invalid
for MUI/GluuAlert; change AlertState.severity to be nullable (severity:
'success' | 'error' | 'warning' | 'info' | null) and update where the state is
initialized and reset (e.g., initial alert state and any setAlert/resetAlert
calls) to use null instead of '' so GluuAlert receives either a valid severity
string or null.
In @admin-ui/plugins/services/Components/Configuration/types.ts:
- Around line 13-17: SqlFormProps currently declares handleSubmit as a union ({
sql: SqlConfiguration } | SqlConfiguration) but SqlForm only ever passes the
wrapped shape; update the exported SqlFormProps.handleSubmit to accept only the
wrapped object (data: { sql: SqlConfiguration }) => void to match SqlForm's
implementation, keeping the create flag handling unchanged; alternatively, if
you prefer the form to accept both shapes, modify the SqlForm component
(SqlForm) to detect and normalize an unwrapped SqlConfiguration into { sql: ...
} before calling handleSubmit, but prefer the first approach for consistency.
In @admin-ui/plugins/user-management/components/UserEditPage.tsx:
- Around line 53-57: The persistence fetch currently uses a type assertion and
ignores loading/error states; replace the assertion by using the properly
generated TypeScript return type from useGetPropertiesPersistence so
persistenceData has a real type and derive persistenceType as
persistenceData?.persistenceType; handle isLoading and isError from
useGetPropertiesPersistence to show a loading indicator or error message (and
prevent form submit while loading) so lines referring to persistenceType
(including the logic at lines 99-101) are safe; also propagate the same typed
value into the loader update at line 126 and surface the error (e.g., show an
inline alert) when isError is true instead of silently treating persistenceType
as undefined.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (50)
admin-ui/plugins/auth-server/components/AuthN/AuthNListPage.tsxadmin-ui/plugins/services/Components/Configuration/CacheInMemory.tsxadmin-ui/plugins/services/Components/Configuration/CacheMemcached.tsxadmin-ui/plugins/services/Components/Configuration/CacheNative.tsxadmin-ui/plugins/services/Components/Configuration/CachePage.jsadmin-ui/plugins/services/Components/Configuration/CachePage.tsxadmin-ui/plugins/services/Components/Configuration/CacheRedis.tsxadmin-ui/plugins/services/Components/Configuration/CouchbaseItem.tsxadmin-ui/plugins/services/Components/Configuration/CouchbasePage.jsadmin-ui/plugins/services/Components/Configuration/CouchbasePage.tsxadmin-ui/plugins/services/Components/Configuration/LdapAddPage.jsadmin-ui/plugins/services/Components/Configuration/LdapAddPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapDetailPage.jsadmin-ui/plugins/services/Components/Configuration/LdapDetailPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapEditPage.jsadmin-ui/plugins/services/Components/Configuration/LdapEditPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapForm.tsxadmin-ui/plugins/services/Components/Configuration/LdapListPage.tsxadmin-ui/plugins/services/Components/Configuration/PersistenceDetail.tsxadmin-ui/plugins/services/Components/Configuration/SqlAddPage.jsadmin-ui/plugins/services/Components/Configuration/SqlAddPage.tsxadmin-ui/plugins/services/Components/Configuration/SqlDetailPage.tsxadmin-ui/plugins/services/Components/Configuration/SqlEditPage.jsadmin-ui/plugins/services/Components/Configuration/SqlEditPage.tsxadmin-ui/plugins/services/Components/Configuration/SqlForm.jsadmin-ui/plugins/services/Components/Configuration/SqlForm.tsxadmin-ui/plugins/services/Components/Configuration/SqlListPage.jsadmin-ui/plugins/services/Components/Configuration/SqlListPage.tsxadmin-ui/plugins/services/Components/Configuration/atoms/index.tsadmin-ui/plugins/services/Components/Configuration/hooks/index.tsadmin-ui/plugins/services/Components/Configuration/types.tsadmin-ui/plugins/services/__tests__/api/Ldap.test.jsadmin-ui/plugins/services/plugin-metadata.jsadmin-ui/plugins/services/redux/api/CacheApi.jsadmin-ui/plugins/services/redux/api/CouchbaseApi.jsadmin-ui/plugins/services/redux/api/LdapApi.jsadmin-ui/plugins/services/redux/api/PersistenceConfigApi.jsadmin-ui/plugins/services/redux/api/SqlApi.jsadmin-ui/plugins/services/redux/api/index.jsadmin-ui/plugins/services/redux/features/cacheSlice.jsadmin-ui/plugins/services/redux/features/couchbaseSlice.jsadmin-ui/plugins/services/redux/features/ldapSlice.jsadmin-ui/plugins/services/redux/features/persistenceTypeSlice.jsadmin-ui/plugins/services/redux/features/sqlSlice.jsadmin-ui/plugins/services/redux/sagas/CacheSaga.jsadmin-ui/plugins/services/redux/sagas/CouchbaseSaga.jsadmin-ui/plugins/services/redux/sagas/LdapSaga.jsadmin-ui/plugins/services/redux/sagas/PersistenceTypeSaga.jsadmin-ui/plugins/services/redux/sagas/SqlSaga.jsadmin-ui/plugins/user-management/components/UserEditPage.tsx
💤 Files with no reviewable changes (26)
- admin-ui/plugins/services/Components/Configuration/CouchbasePage.js
- admin-ui/plugins/services/redux/api/PersistenceConfigApi.js
- admin-ui/plugins/services/redux/api/SqlApi.js
- admin-ui/plugins/services/Components/Configuration/SqlEditPage.js
- admin-ui/plugins/services/Components/Configuration/LdapEditPage.js
- admin-ui/plugins/services/tests/api/Ldap.test.js
- admin-ui/plugins/services/Components/Configuration/CachePage.js
- admin-ui/plugins/services/redux/sagas/CacheSaga.js
- admin-ui/plugins/services/redux/sagas/CouchbaseSaga.js
- admin-ui/plugins/services/redux/features/persistenceTypeSlice.js
- admin-ui/plugins/services/Components/Configuration/SqlAddPage.js
- admin-ui/plugins/services/Components/Configuration/SqlListPage.js
- admin-ui/plugins/services/Components/Configuration/SqlForm.js
- admin-ui/plugins/services/Components/Configuration/LdapDetailPage.js
- admin-ui/plugins/services/redux/sagas/PersistenceTypeSaga.js
- admin-ui/plugins/services/redux/features/couchbaseSlice.js
- admin-ui/plugins/services/redux/sagas/SqlSaga.js
- admin-ui/plugins/services/redux/api/LdapApi.js
- admin-ui/plugins/services/redux/features/sqlSlice.js
- admin-ui/plugins/services/Components/Configuration/LdapAddPage.js
- admin-ui/plugins/services/redux/api/CacheApi.js
- admin-ui/plugins/services/redux/features/ldapSlice.js
- admin-ui/plugins/services/redux/api/index.js
- admin-ui/plugins/services/redux/api/CouchbaseApi.js
- admin-ui/plugins/services/redux/sagas/LdapSaga.js
- admin-ui/plugins/services/redux/features/cacheSlice.js
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-01-06T13:45:19.278Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts:4-101
Timestamp: 2026-01-06T13:45:19.278Z
Learning: In admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts, ensure Yup schema marks fields as nullable when the backend DTOs permit nulls and do not mark them as required. Align frontend validation with backend validation to prevent conflicts when editing existing configurations with null values; apply this pattern consistently to other Admin UI validation files where the backend is the source of truth.
Applied to files:
admin-ui/plugins/services/Components/Configuration/atoms/index.tsadmin-ui/plugins/services/Components/Configuration/hooks/index.tsadmin-ui/plugins/services/Components/Configuration/types.ts
📚 Learning: 2025-11-06T08:23:20.948Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2418
File: admin-ui/plugins/user-management/components/UserForm.tsx:290-297
Timestamp: 2025-11-06T08:23:20.948Z
Learning: In the UserForm component (admin-ui/plugins/user-management/components/UserForm.tsx), the Back button should fall back to '/user/usersmanagement' when browser history is unavailable, not '/home/dashboard', as this keeps users in the user management context. This aligns with the new requirement that users should be redirected to the respective listing screen of the component rather than the dashboard.
Applied to files:
admin-ui/plugins/user-management/components/UserEditPage.tsx
📚 Learning: 2025-12-05T18:24:41.713Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2498
File: admin-ui/plugins/saml/components/SamlConfigurationForm.tsx:30-30
Timestamp: 2025-12-05T18:24:41.713Z
Learning: In the GluuFederation/flex admin-ui codebase, `SetTitle` (imported from 'Utils/SetTitle') is a custom React hook that should be called at the component's top level, not inside `useEffect` or other hooks, because it internally manages side effects for setting page titles.
Applied to files:
admin-ui/plugins/services/Components/Configuration/PersistenceDetail.tsxadmin-ui/plugins/services/Components/Configuration/LdapForm.tsxadmin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Applied to files:
admin-ui/plugins/services/Components/Configuration/hooks/index.ts
🧬 Code graph analysis (14)
admin-ui/plugins/services/Components/Configuration/LdapDetailPage.tsx (3)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
LdapDetailPageProps(19-22)admin-ui/app/customColors.ts (1)
customColors(4-25)admin-ui/plugins/admin/redux/features/mappingSlice.js (1)
index(47-47)
admin-ui/plugins/services/Components/Configuration/SqlAddPage.tsx (3)
admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useSqlAudit(100-156)admin-ui/app/helpers/navigation.ts (1)
ROUTES(204-204)admin-ui/app/components/index.tsx (2)
Container(62-62)CardBody(44-44)
admin-ui/plugins/services/Components/Configuration/CacheNative.tsx (1)
admin-ui/plugins/services/Components/Configuration/types.ts (2)
CacheNativeProps(45-48)CacheFormValues(55-78)
admin-ui/plugins/services/Components/Configuration/CachePage.tsx (2)
admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useCacheAudit(158-186)admin-ui/plugins/services/Components/Configuration/types.ts (1)
CacheFormValues(55-78)
admin-ui/plugins/services/Components/Configuration/SqlForm.tsx (4)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
SqlFormProps(24-28)admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (1)
handleSubmit(145-162)admin-ui/app/components/index.tsx (5)
Form(68-68)FormGroup(70-70)Col(60-60)Input(72-72)InputGroup(73-73)admin-ui/app/customColors.ts (1)
customColors(4-25)
admin-ui/plugins/auth-server/components/AuthN/AuthNListPage.tsx (1)
admin-ui/plugins/auth-server/components/AuthN/atoms/authNAtoms.ts (1)
AuthNItem(11-34)
admin-ui/plugins/services/Components/Configuration/CouchbaseItem.tsx (2)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
CouchbaseItemProps(80-83)admin-ui/app/components/index.tsx (1)
Badge(37-37)
admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
admin-ui/app/utils/AuditLogger.ts (1)
logAuditUserAction(25-66)
admin-ui/plugins/services/Components/Configuration/CacheMemcached.tsx (1)
admin-ui/plugins/services/Components/Configuration/types.ts (2)
CacheMemcachedProps(40-43)CacheFormValues(55-78)
admin-ui/plugins/services/Components/Configuration/SqlDetailPage.tsx (2)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
SqlDetailPageProps(30-33)admin-ui/app/components/index.tsx (3)
Row(95-95)Col(60-60)Container(62-62)
admin-ui/plugins/services/Components/Configuration/CacheInMemory.tsx (1)
admin-ui/plugins/services/Components/Configuration/types.ts (2)
CacheInMemoryProps(35-38)CacheFormValues(55-78)
admin-ui/plugins/services/Components/Configuration/LdapForm.tsx (3)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
LdapFormProps(13-17)admin-ui/app/components/index.tsx (1)
Form(68-68)admin-ui/app/customColors.ts (1)
customColors(4-25)
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx (3)
admin-ui/plugins/services/Components/Configuration/atoms/index.ts (1)
currentLdapItemAtom(4-4)admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useLdapAudit(34-98)admin-ui/plugins/services/Components/Configuration/types.ts (1)
PersistenceInfo(104-112)
admin-ui/plugins/services/Components/Configuration/CacheRedis.tsx (1)
admin-ui/plugins/services/Components/Configuration/types.ts (2)
CacheRedisProps(50-53)CacheFormValues(55-78)
🔇 Additional comments (28)
admin-ui/plugins/user-management/components/UserEditPage.tsx (2)
99-101: Verify business logic when persistenceType is unavailable.The conditional spread safely handles
undefinedpersistenceType (nothing is spread if the condition is false). However, consider whether the form should be submittable before persistence configuration loads.If
persistenceTypeis still loading or failed to load, the form will submit withoutcustomObjectClassesfor LDAP configurations, which may cause issues downstream.Consider blocking form submission until persistence data is available:
const submitData = useCallback( async (values: UserEditFormValues, modifiedFields: ModifiedFields, userMessage: string) => { + if (!persistenceType) { + dispatch(updateToast(true, 'warning', t('messages.persistence_config_required'))) + return + } const baseCustomAttributes = buildCustomAttributesFromValues(values, personAttributes)
1-138: Note: File location vs PR objectives.This file is in the
user-managementplugin, while the PR objectives state the intent is to "use TypeScript-generated client for the Services plugin."If this change is intentionally applying the same refactoring pattern across multiple plugins, that's fine. Otherwise, verify that changes to user-management components are within the intended scope of this PR.
admin-ui/plugins/services/plugin-metadata.js (1)
94-95: Clean removal of Redux wiring.Empty arrays for
reducersandsagascorrectly reflect the migration to React Query and Jotai for state management. The route configurations and permission checks remain properly intact.admin-ui/plugins/services/Components/Configuration/atoms/index.ts (1)
1-6: LGTM!The Jotai atoms are well-typed and follow best practices. Using
nullas the initial value with explicit type annotations (GluuLdapConfiguration | null) correctly represents the "no item selected" state.admin-ui/plugins/services/Components/Configuration/SqlAddPage.tsx (2)
21-37: Well-structured mutation with proper error handling.Good patterns observed:
- Cache invalidation on success ensures UI reflects the new data
- Audit logging is wrapped in try-catch to prevent blocking navigation on logging failures
- Clear separation of success/error toast handling
48-48: No issues found. The empty object{}is safely handled bySqlForm, which uses a defensive initialization pattern with the||operator to provide sensible defaults (empty strings, empty arrays,false) for all fields when they are undefined. This design explicitly supports creating new SQL configurations from scratch, and validation only occurs during form submission, not initialization.admin-ui/plugins/services/Components/Configuration/CacheRedis.tsx (1)
19-22: Good defensive coding with safe theme fallback.The optional chaining with a default value (
theme?.state?.theme || 'darkBlue') properly handles cases where the theme context might not be fully initialized.admin-ui/plugins/services/Components/Configuration/SqlEditPage.tsx (1)
51-61: Good null state handling.Displaying a user-friendly message when no configuration is selected is appropriate. This prevents errors if users navigate directly to the edit page without selecting an item first.
admin-ui/plugins/services/Components/Configuration/LdapEditPage.tsx (2)
43-51: Flexible data handling accommodates multiple submission formats.The handler properly handles both wrapped (
{ ldap: ... }) and unwrapped data formats, which is useful ifLdapFormmay submit data in different structures. TheisEmptycheck provides additional validation before mutation.
25-41: Consistent mutation pattern with proper error isolation.The try-catch around audit logging (lines 30-34) ensures that logging failures don't block the user flow. Cache invalidation and navigation proceed regardless of audit success.
admin-ui/plugins/services/Components/Configuration/CouchbasePage.tsx (1)
39-39: Type assertion indicates mismatch between hook return type and PersistenceInfo interface.
useGetPropertiesPersistence(from the external 'JansConfigApi' package) appears to return a type incompatible with the locally-definedPersistenceInfo. The cast on line 39 is necessary to satisfy TypeScript, but this pattern is inconsistent across the codebase (UserEditPage uses a narrower type, PersistenceDetail avoids casting). Verify the actual return type ofuseGetPropertiesPersistenceand either update the types.ts file to match or adjust the assertion to align with the hook's documented schema.admin-ui/plugins/services/Components/Configuration/CouchbaseItem.tsx (1)
225-226: Good security improvement for password field.Adding
type="password"masks the password input, which is a security best practice for sensitive fields.admin-ui/plugins/services/Components/Configuration/SqlListPage.tsx (1)
68-74: LGTM - React Query integration follows best practices.Good use of
staleTimefor caching, proper query key management for cache invalidation, and clean separation of data fetching concerns. The mutation handlers appropriately dispatch toasts and invalidate queries.admin-ui/plugins/services/Components/Configuration/PersistenceDetail.tsx (2)
42-46: LGTM - Query gated by permission with conditional fetching.Good pattern using
enabled: canReadPersistenceto prevent unnecessary API calls when the user lacks read permission. ThestaleTimeconfiguration is consistent with other files.
40-40: Correct usage ofSetTitle.
SetTitleis properly called at the component level, not insideuseEffect, which aligns with its design as a custom hook that internally manages side effects. Based on learnings from similar components in this codebase.admin-ui/plugins/services/Components/Configuration/CacheNative.tsx (1)
1-8: Good migration from PropTypes to TypeScript.Clean replacement of runtime PropTypes validation with compile-time TypeScript interfaces. The explicit
ReactElementreturn type improves type safety.admin-ui/plugins/services/Components/Configuration/LdapAddPage.tsx (1)
21-37: LGTM - Clean mutation setup with proper cache invalidation.Good pattern with:
- Cache invalidation via
queryClient.invalidateQueries- Toast notifications for user feedback
- Audit logging with error handling
- Navigation on success
admin-ui/plugins/services/Components/Configuration/CacheMemcached.tsx (1)
18-21: LGTM!The typed function signature with
ReactElementreturn type and the safe theme access with optional chaining and default fallback are good improvements for type safety and robustness.admin-ui/plugins/services/Components/Configuration/CacheInMemory.tsx (1)
13-36: LGTM!The typed function signature and ReactElement return type improve type safety. The component implementation is clean and correctly uses the formik prop for form state management.
admin-ui/plugins/services/Components/Configuration/CachePage.tsx (1)
44-66: LGTM!Good implementation of permission-based access control using Cedarling hooks. The memoized permission checks and proper authorization flow are well structured.
admin-ui/plugins/services/Components/Configuration/SqlDetailPage.tsx (1)
91-103: LGTM!The conditional test button rendering with proper type and click handler is well implemented. The theme-based styling maintains consistency with the rest of the UI.
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx (2)
177-251: LGTM!The action generation logic correctly handles typed arrays and properly narrows types when handling both single items and arrays from material-table callbacks. The dependency array for the useEffect is comprehensive.
264-278: LGTM!The async deletion flow with proper try-catch error handling and audit logging is well implemented. The navigation and modal toggle after deletion maintain good UX.
admin-ui/plugins/services/Components/Configuration/SqlForm.tsx (1)
45-71: LGTM!The Formik setup with validation schema and form values is well structured. The
enableReinitializeoption correctly handles item prop changes.admin-ui/plugins/services/Components/Configuration/LdapForm.tsx (2)
54-63: LGTM!The test mutation hook setup with proper success and error toast notifications is well implemented. The integration with the dispatch for toast updates maintains consistency with other forms in the codebase.
137-137: LGTM!Good approach combining
isLoadingprop withtestMutation.isPendingto show the loader during both initial load and connection testing.admin-ui/plugins/services/Components/Configuration/types.ts (2)
1-112: Well-structured type definitions.The type definitions are well-organized with clear separation of concerns between form props, detail page props, and form values. The integration with
FormikPropsfollows standard patterns.
55-78: No action needed. The optional fields inCacheFormValues(and similar form value types) are correctly designed for their use case. UnlikeLdapFormandSqlFormwhich enforce strict upfront validation,CachePageintentionally uses no validation schema because it sends type-specific configurations to separate endpoints based on the selectedcacheProviderType. All fields exceptcacheProviderTypeare appropriately optional, matching the dynamic nature of multi-type cache configuration and aligning with backend DTO expectations.
admin-ui/plugins/auth-server/components/AuthN/AuthNListPage.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/services/Components/Configuration/CacheInMemory.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/services/Components/Configuration/CacheMemcached.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/services/Components/Configuration/CacheNative.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/services/Components/Configuration/CachePage.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
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/services/Components/Configuration/CouchbaseItem.tsx (1)
49-75: Avoidlength && map(...)here and prefer stable keys.The pattern:
{couchbase.servers.length && couchbase.servers.map((server, idx) => ( <Badge key={idx} ...>{server}</Badge> ))}(and similarly for
buckets) can render a stray0when the array is empty, and index keys are brittle under reordering.Consider:
{couchbase.servers.length > 0 && couchbase.servers.map((server) => ( <Badge key={server} color={`primary-${selectedTheme}`} className="ms-1"> {server} </Badge> ))}and an analogous change for
buckets, using the bucket name as the key.admin-ui/plugins/auth-server/components/AuthN/AuthNListPage.tsx (1)
190-201: Undefined variableloadingwill cause a build/runtime error.The code references
loadingon lines 191, 200, and 211, but this variable is not defined. The LDAP loading state is namedldapLoading(line 59). This will cause a build failure or runtime reference error.🔎 Proposed fix
const tableData = useMemo(() => { - if (loading || scriptsLoading || acrsLoading) { + if (ldapLoading || scriptsLoading || acrsLoading) { return [] } if (isBuiltIn) { return BUILT_IN_ACRS as AuthNItem[] } return [...list.ldap, ...list.scripts].sort( (item1, item2) => (item1.level || 0) - (item2.level || 0), ) - }, [loading, scriptsLoading, acrsLoading, isBuiltIn, list.ldap, list.scripts]) + }, [ldapLoading, scriptsLoading, acrsLoading, isBuiltIn, list.ldap, list.scripts])And at line 211:
- isLoading={loading || scriptsLoading || acrsLoading} + isLoading={ldapLoading || scriptsLoading || acrsLoading}
🤖 Fix all issues with AI agents
In @admin-ui/plugins/services/Components/Configuration/CouchbasePage.tsx:
- Line 39: Replace the direct type assertion when deriving persistenceType with
the same type-guard pattern used in SqlListPage.tsx: import or replicate the
isPersistenceInfo guard and use it to check persistenceData before accessing
persistenceType (e.g., if (isPersistenceInfo(persistenceData)) { const
persistenceType = persistenceData.persistenceType ... }). Update any code paths
that assume PersistenceInfo so they only run when the guard passes to ensure
type safety and consistency with SqlListPage.tsx.
In @admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx:
- Around line 37-43: The type guard isPersistenceInfo currently treats empty
objects as valid PersistenceInfo which lets persistenceType be undefined later;
either make the guard strict by removing the empty-object clause so it only
returns true when 'persistenceType' in data (ensuring callers can rely on that
property), or if permissive behavior is needed, rename the function to
mayBePersistenceInfo and update all call sites to reflect the weaker guarantee;
update usages that assume persistenceType exists (e.g., where you access
persistenceType on the returned type) to handle undefined when you keep the
permissive guard.
In @admin-ui/plugins/services/Components/Configuration/SqlEditPage.tsx:
- Around line 25-49: handleSubmit currently strips action_message from data.sql
before calling editMutation.mutate, and the onSuccess audit call in
usePutConfigDatabaseSql hardcodes 'SQL configuration updated', so any
user-provided action_message is lost; change handleSubmit to include
action_message in the mutation variables (e.g., pass { data: cleanData, meta: {
action_message } } or include action_message on the payload) and update the
editMutation onSuccess handler where logSqlUpdate is called
(logSqlUpdate(variables.data, variables.action_message ?? 'SQL configuration
updated') or equivalent) so the supplied action_message is forwarded to
logSqlUpdate with the current string as a fallback.
In @admin-ui/plugins/services/Components/Configuration/types.ts:
- Around line 10-25: The props interfaces are inconsistent: LdapFormProps
exposes a createLdap?: boolean mode flag while SqlFormProps exposes isLoading?:
boolean; harmonize them by choosing a consistent prop pattern (either both
accept a mode flag like createMode/create?: boolean or both accept isLoading and
an optional mode flag) and update the interfaces and any components that consume
them (LdapFormProps, SqlFormProps and their handleSubmit signatures) to match
that pattern; ensure both form components receive the same optional flags (e.g.,
isLoading?: boolean and isCreate?: boolean) and update usages to pass the new
props accordingly.
- Around line 50-73: CacheFormValues currently mixes all provider-specific
fields into one flat interface; refactor it into a discriminated union keyed on
cacheProviderType (e.g., 'IN_MEMORY', 'MEMCACHED', 'REDIS',
'NATIVE_PERSISTENCE') so each variant only exposes the relevant fields
(memoryDefaultPutExpiration for IN_MEMORY,
memCacheServers/maxOperationQueueLength/bufferSize/memDefaultPutExpiration/connectionFactoryType
for MEMCACHED,
redisProviderType/servers/password/sentinelMasterGroupName/sslTrustStoreFilePath/redisDefaultPutExpiration/useSSL/maxIdleConnections/maxTotalConnections/connectionTimeout/soTimeout/maxRetryAttempts
for REDIS, and
nativeDefaultPutExpiration/defaultCleanupBatchSize/deleteExpiredOnGetRequest for
NATIVE_PERSISTENCE); update any code using CacheFormValues (forms, validators,
switch/case or type guards) to narrow by cacheProviderType so TypeScript
enforces correct fields.
In @admin-ui/plugins/user-management/components/UserEditPage.tsx:
- Around line 30-40: Replace the local PersistenceInfo interface and
isPersistenceInfo type guard with the canonical type used by the codebase
(import the PersistenceInfo or the return type of useGetPropertiesPersistence
from services/Components/Configuration/types.ts or from the hook itself) and
then remove or tighten the runtime guard; update references to the local
PersistenceInfo and isPersistenceInfo in UserEditPage.tsx to use the imported
type (or the hook's return type) and, if a runtime check is still required,
narrow isPersistenceInfo to exclude arrays and non-plain objects so it matches
the generated type shape.
- Around line 76-80: The effect handling persistenceError in UserEditPage
currently only logs to console; update it to surface failures to users by using
the existing useGetPropertiesPersistence result (persistenceError) to call
updateToast with a dedicated i18n key (e.g., 'userEdit.persistenceLoadFailed')
and/or render an inline Alert component near the form when persistenceError is
set, and also disable the form submit (or show a non-interactive submit state)
while persistenceError is present to prevent silent LDAP-related behavior
changes; locate the useEffect that references persistenceError and adjust it to
dispatch updateToast and set a local state flag (or derive from
persistenceError) used by the form submit button and alert rendering.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (24)
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/plugins/auth-server/components/AuthN/AuthNListPage.tsxadmin-ui/plugins/services/Components/Configuration/CacheInMemory.tsxadmin-ui/plugins/services/Components/Configuration/CacheMemcached.tsxadmin-ui/plugins/services/Components/Configuration/CacheNative.tsxadmin-ui/plugins/services/Components/Configuration/CachePage.tsxadmin-ui/plugins/services/Components/Configuration/CacheRedis.tsxadmin-ui/plugins/services/Components/Configuration/CouchbaseItem.tsxadmin-ui/plugins/services/Components/Configuration/CouchbasePage.tsxadmin-ui/plugins/services/Components/Configuration/LdapAddPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapDetailPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapForm.tsxadmin-ui/plugins/services/Components/Configuration/LdapListPage.tsxadmin-ui/plugins/services/Components/Configuration/SqlDetailPage.tsxadmin-ui/plugins/services/Components/Configuration/SqlEditPage.tsxadmin-ui/plugins/services/Components/Configuration/SqlForm.tsxadmin-ui/plugins/services/Components/Configuration/SqlListPage.tsxadmin-ui/plugins/services/Components/Configuration/hooks/index.tsadmin-ui/plugins/services/Components/Configuration/types.tsadmin-ui/plugins/services/redux/audit/Resources.jsadmin-ui/plugins/user-management/components/UserEditPage.tsx
💤 Files with no reviewable changes (1)
- admin-ui/plugins/services/redux/audit/Resources.js
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.
Applied to files:
admin-ui/plugins/services/Components/Configuration/SqlListPage.tsx
📚 Learning: 2025-11-06T08:23:20.948Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2418
File: admin-ui/plugins/user-management/components/UserForm.tsx:290-297
Timestamp: 2025-11-06T08:23:20.948Z
Learning: In the UserForm component (admin-ui/plugins/user-management/components/UserForm.tsx), the Back button should fall back to '/user/usersmanagement' when browser history is unavailable, not '/home/dashboard', as this keeps users in the user management context. This aligns with the new requirement that users should be redirected to the respective listing screen of the component rather than the dashboard.
Applied to files:
admin-ui/plugins/services/Components/Configuration/SqlListPage.tsxadmin-ui/plugins/user-management/components/UserEditPage.tsx
📚 Learning: 2025-11-03T08:47:33.943Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/services/Components/Configuration/SqlListPage.tsx
📚 Learning: 2025-11-10T14:18:58.310Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
Applied to files:
admin-ui/plugins/services/Components/Configuration/SqlListPage.tsxadmin-ui/plugins/services/Components/Configuration/CouchbasePage.tsxadmin-ui/plugins/services/Components/Configuration/CachePage.tsxadmin-ui/plugins/services/Components/Configuration/hooks/index.ts
📚 Learning: 2025-12-05T18:24:41.713Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2498
File: admin-ui/plugins/saml/components/SamlConfigurationForm.tsx:30-30
Timestamp: 2025-12-05T18:24:41.713Z
Learning: In the GluuFederation/flex admin-ui codebase, `SetTitle` (imported from 'Utils/SetTitle') is a custom React hook that should be called at the component's top level, not inside `useEffect` or other hooks, because it internally manages side effects for setting page titles.
Applied to files:
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapForm.tsx
📚 Learning: 2025-11-26T12:25:34.923Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2475
File: admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js:111-216
Timestamp: 2025-11-26T12:25:34.923Z
Learning: In the admin-ui codebase, avoid suggesting comments for small functionalities or straightforward layout decisions in smaller components. Comments should be reserved for more complex or non-obvious logic.
Applied to files:
admin-ui/plugins/services/Components/Configuration/CachePage.tsx
📚 Learning: 2026-01-06T13:45:19.278Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts:4-101
Timestamp: 2026-01-06T13:45:19.278Z
Learning: In the admin-ui Config API Properties forms (ApiConfigForm component), frontend Yup validation uses `.nullable()` for all fields without `.required()` because the backend DTOs are the source of truth and enforce validation. The frontend validation should match what the backend accepts to avoid conflicts when editing existing configurations that may have null values.
Applied to files:
admin-ui/plugins/services/Components/Configuration/SqlForm.tsx
📚 Learning: 2026-01-06T13:45:19.278Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts:4-101
Timestamp: 2026-01-06T13:45:19.278Z
Learning: In admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts, ensure Yup schema marks fields as nullable when the backend DTOs permit nulls and do not mark them as required. Align frontend validation with backend validation to prevent conflicts when editing existing configurations with null values; apply this pattern consistently to other Admin UI validation files where the backend is the source of truth.
Applied to files:
admin-ui/plugins/services/Components/Configuration/types.tsadmin-ui/plugins/services/Components/Configuration/hooks/index.ts
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Applied to files:
admin-ui/plugins/services/Components/Configuration/hooks/index.ts
🧬 Code graph analysis (15)
admin-ui/plugins/services/Components/Configuration/SqlListPage.tsx (5)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
PersistenceInfo(100-108)admin-ui/plugins/services/Components/Configuration/atoms/index.ts (1)
currentSqlItemAtom(6-6)admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useSqlAudit(107-173)admin-ui/app/utils/pagingUtils.ts (1)
getPagingSize(1-24)admin-ui/app/customColors.ts (1)
customColors(4-25)
admin-ui/plugins/services/Components/Configuration/CacheMemcached.tsx (1)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
CacheMemcachedProps(36-39)
admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
PersistenceInfo(100-108)
admin-ui/plugins/services/Components/Configuration/SqlDetailPage.tsx (2)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
SqlDetailPageProps(27-30)admin-ui/app/components/index.tsx (3)
Row(95-95)Col(60-60)Container(62-62)
admin-ui/plugins/services/Components/Configuration/CacheInMemory.tsx (1)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
CacheInMemoryProps(32-34)
admin-ui/plugins/services/Components/Configuration/CouchbasePage.tsx (2)
admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useCouchbaseAudit(206-260)admin-ui/plugins/services/Components/Configuration/types.ts (1)
PersistenceInfo(100-108)
admin-ui/plugins/services/Components/Configuration/SqlEditPage.tsx (5)
admin-ui/plugins/services/Components/Configuration/atoms/index.ts (1)
currentSqlItemAtom(6-6)admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useSqlAudit(107-173)admin-ui/app/helpers/navigation.ts (1)
ROUTES(204-204)admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (1)
handleSubmit(145-162)admin-ui/app/components/index.tsx (2)
Container(62-62)CardBody(44-44)
admin-ui/plugins/services/Components/Configuration/CachePage.tsx (2)
admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useCacheAudit(175-204)admin-ui/plugins/services/Components/Configuration/types.ts (1)
CacheFormValues(50-73)
admin-ui/plugins/services/Components/Configuration/LdapDetailPage.tsx (3)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
LdapDetailPageProps(16-19)admin-ui/app/components/index.tsx (6)
Container(62-62)FormGroup(70-70)Row(95-95)Col(60-60)Label(75-75)Badge(37-37)admin-ui/app/customColors.ts (1)
customColors(4-25)
admin-ui/plugins/services/Components/Configuration/LdapAddPage.tsx (3)
admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useLdapAudit(31-105)admin-ui/app/helpers/navigation.ts (1)
ROUTES(204-204)admin-ui/app/components/index.tsx (2)
Container(62-62)CardBody(44-44)
admin-ui/plugins/services/Components/Configuration/SqlForm.tsx (4)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
SqlFormProps(21-25)admin-ui/app/components/index.tsx (5)
Form(68-68)FormGroup(70-70)Col(60-60)Input(72-72)InputGroup(73-73)admin-ui/app/customColors.ts (1)
customColors(4-25)admin-ui/plugins/services/Components/Configuration/SqlForm.js (1)
SqlForm(14-263)
admin-ui/plugins/services/Components/Configuration/CouchbaseItem.tsx (2)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
CouchbaseItemProps(75-79)admin-ui/app/components/index.tsx (1)
Badge(37-37)
admin-ui/plugins/services/Components/Configuration/hooks/index.ts (2)
admin-ui/app/utils/types.ts (1)
AuthRootState(38-54)admin-ui/app/utils/AuditLogger.ts (1)
logAuditUserAction(25-66)
admin-ui/plugins/services/Components/Configuration/CacheNative.tsx (1)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
CacheNativeProps(41-43)
admin-ui/plugins/services/Components/Configuration/CacheRedis.tsx (1)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
CacheRedisProps(45-48)
🔇 Additional comments (56)
admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
150-155: IncludingloadingPersistenceinGluuLoaderlooks goodBlocking the loader on
loadingPersistencein addition toloadingAttributesandisSubmittingaccurately reflects the page’s real loading state and avoids partially initialized submits.admin-ui/app/locales/pt/translation.json (1)
684-686: New Portuguese validation/partial-update messages are consistentThe added
partial_update_failure,min_characters, andfield_requiredmessages look correct, use{{count}}consistently, and align with the semantics used in other locales.admin-ui/app/locales/es/translation.json (1)
842-844: Spanish messages for partial update and field validation are well-formedThe new
partial_update_failure,min_characters, andfield_requiredentries read naturally and match the expected interpolation pattern with{{count}}, keeping behavior in sync with other locales.admin-ui/app/locales/en/translation.json (1)
840-842: New validation and error messages look consistent.
partial_update_failure,min_characters, andfield_requiredfollow existing naming, phrasing, and placeholder conventions; no issues from a localization or UX perspective.admin-ui/app/locales/fr/translation.json (1)
689-691: French translations preserve meaning and placeholders.The three added entries accurately mirror the EN messages and keep the
{{count}}interpolation intact; no further changes needed.admin-ui/plugins/services/Components/Configuration/SqlListPage.tsx (4)
249-263: Navigation flow after deletion looks correct now.The
navigateBackcall is now inside thetryblock (line 255), so users stay on the page when deletion fails. This addresses the previous review concern.However, note that
toggle()on line 260 runs unconditionally, closing the dialog even on failure. This seems intentional for UX (close confirmation dialog regardless of outcome), but verify this is the desired behavior.
44-48: AlertState severity type looks correct.Using
undefinedinstead of empty string for severity is appropriate since MUI Alert acceptsundefined(which omits the severity styling). This is a cleaner approach than the empty string that was flagged in the previous review.
36-42: Type guard implementation is sound.The
isPersistenceInfoguard correctly handles both populated and empty response objects from the API. Empty objects will result inpersistenceTypebeingundefined, which will cause the fallback Alert to display at line 317.
280-315: MaterialTable integration is well-structured.The defensive array check on line 297 properly handles potential undefined/non-array responses. Permission-gated actions and detail panel integration are correctly implemented.
admin-ui/plugins/services/Components/Configuration/CouchbasePage.tsx (2)
43-72: Batch submission with partial failure handling looks correct.The
Promise.allSettledimplementation properly handles all configurations and provides differentiated feedback: success toast when all pass, danger toast when all fail, and warning toast with partial failure message when some succeed. This addresses the previous review concern about sequential mutations.
91-98: Key usage is improved.Using
configId ?? indexas the key is better than justindex. This addresses the previous review concern about stable keys.admin-ui/plugins/services/Components/Configuration/LdapAddPage.tsx (2)
15-20: Default configuration extracted to module level.This addresses the previous review suggestion. The constant is now defined outside the component, preventing recreation on each render.
46-54: Data normalization in handleSubmit is well-implemented.The function correctly handles both wrapped (
{ ldap: ... }) and unwrapped data formats, and properly strips theaction_messagefield before mutation. This flexibility accommodates different form submission patterns.admin-ui/plugins/auth-server/components/AuthN/AuthNListPage.tsx (1)
124-141: Mapper function properly addresses the type mismatch.The
mapLdapToAuthNItemfunction correctly transformsGluuLdapConfigurationtoAuthNItem, avoiding the unsafe type cast that was flagged in the previous review. The empty dependency array is appropriate since the function has no external dependencies.admin-ui/plugins/services/Components/Configuration/CacheMemcached.tsx (1)
1-14: TypeScript migration completed correctly.The component now imports
CacheMemcachedPropsfrom the shared types file (addressing the previous review comment), uses proper TypeScript typing for the function signature, and includes defensive theme access with a fallback. PropTypes removal is appropriate with TypeScript typing in place.admin-ui/plugins/services/Components/Configuration/LdapDetailPage.tsx (3)
1-8: TypeScript typing correctly uses shared interface.The component imports
LdapDetailPagePropsfrom the shared types file, addressing the previous review comment about interface duplication. The function signature is properly typed.
82-89: Boolean display is now properly rendered.The
useSSLfield now only renders through the Badge component with proper i18n translations. The previous issue of raw boolean rendering before the Badge has been resolved.
119-127: Consistent i18n usage foruseAnonymousBind.This addresses the previous review comment about inconsistent boolean display. Now uses
t('options.true')/t('options.false')like other boolean fields in the component.admin-ui/plugins/services/Components/Configuration/SqlDetailPage.tsx (2)
1-11: TypeScript typing correctly uses shared interface.The component imports
SqlDetailPagePropsfrom the shared types file. The previous concern about interface mismatch has been resolved—types.ts(lines 26-29 in relevant snippets) now definestestSqlConnectionas optional, matching this component's usage pattern.
86-98: Conditional test button rendering is well-implemented.The test connection button is correctly rendered only when
testSqlConnectionis provided, properly handling the optional prop from the interface. The inline arrow function() => testSqlConnection(row)is appropriate here since it's a simple handler.admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx (7)
45-63: LGTM! Comprehensive action typing.The
AlertState,ActionObject, andActionTypeinterfaces provide strong type safety for MaterialTable actions and alert state management. The union type forActionTypeproperly handles both static and dynamic (row-dependent) actions.
80-90: Good use of type guard for persistence data.The code properly applies the
isPersistenceInfoguard and safely extractspersistenceTypewith optional chaining. The guard protects against unexpected API response shapes, and the conditional rendering (line 333) correctly handles when persistence type is not LDAP.
134-165: LGTM! Mutation setup follows best practices.The mutations properly invalidate queries on success and provide user feedback via toasts and alerts. The separation of concerns between global toasts (for delete) and local alerts (for test) is appropriate.
187-261: LGTM! Action generation handles permissions and types correctly.The dynamic action generation properly:
- Guards each action behind appropriate Cedar permissions
- Handles both single and array
rowDatawith explicitArray.isArraychecks- Uses internationalized tooltip strings
- Includes
queryClientin dependencies for the refresh action's query invalidation
274-297: LGTM! Proper null-safety and error handling.The deletion confirmation handler correctly:
- Checks
item?.configIdbefore proceeding (handles null state)- Awaits both mutation and audit logging
- Catches and logs errors without disrupting the UI
- Navigates back after successful deletion
The test connection function appropriately resets alert state before initiating the test.
327-357: LGTM! Loading states and table rendering are properly handled.The component correctly:
- Aggregates loading states from data fetching and mutations (line 327)
- Uses
GluuLoaderto block UI during persistence type loading- Provides safe defaults for table data (
Array.isArray(ldapConfigurations) ? ldapConfigurations : [])- Conditionally renders based on
persistenceType === 'ldap'
65-65: Good explicit return type.The function signature explicitly declares
ReactElementas the return type, improving type safety and documentation. This aligns with the TypeScript migration goals of the PR.admin-ui/plugins/services/Components/Configuration/SqlForm.tsx (6)
1-25: LGTM! Clean TypeScript migration.All past review concerns have been addressed:
- ✅
tooglerenamed toactivateValidation(line 22)- ✅ Validation messages use
t()for i18n (lines 34-45)- ✅ Function signatures properly typed with
SqlFormPropsand returnReactElementThe type imports are correctly separated using the
typekeyword.
32-46: Validation schema properly internationalized.The validation schema now uses
t()for all error messages, addressing the previous review comment. The required fields align with the SQL configuration requirements.Based on learnings: The schema doesn't use
.nullable()on fields, which is appropriate if these fields are truly required by the backend SQL configuration DTO. Ensure this matches the backend validation.
48-68: LGTM! Formik configuration follows best practices.The form setup includes:
- Safe default values for all fields (using
|| ''or|| []or|| false)enableReinitialize: trueto handle prop changes- Proper merging of
itemandvaluesto preserve additional properties- Correct payload structure
{ sql: result }matching the API contract
70-73: Excellent fix for form submission.The
handleFormSubmitfunction now properly uses Formik'ssubmitForm()method instead of DOM manipulation, addressing the previous review concern. The modal is closed before submission, which is a reasonable UX flow.
150-167: Good handling of type compatibility.The form properly:
- Casts
formiktoFormikContextType<unknown>forGluuTypeAheadcompatibility (line 155)- Stringifies array errors with
String(formik.errors.connectionUri)(line 165) to prevent React rendering errorsThese are pragmatic solutions for working with the existing
GluuTypeAheadcomponent's type expectations.
272-278: LGTM! Commit dialog properly wired.The
GluuCommitDialogintegration correctly:
- Uses the new
handleFormSubmitfunction for theonAccepthandler- Passes the modal state and formik context
- Provides a confirmation step before submission
admin-ui/plugins/services/Components/Configuration/CachePage.tsx (7)
44-66: LGTM! Proper setup and permission handling.The component correctly:
- Uses
useMemoto avoid recreating resource IDs and scopes- Calls
SetTitleat the top level (per learnings about it being a custom hook)- Checks Cedar permissions and returns
nullearly if read permission is denied
92-106: Cleaner mutation setup than suggested.The mutation hooks are now defined without inline callbacks (except
patchCacheMutation's query invalidation), with success/error handling centralized in theonSubmithandler (lines 246, 255). This is actually cleaner than extracting shared callbacks, as it keeps the mutation logic together and avoids spreading error handling across multiple locations.
118-144: Good memoization of initial values.The
initialValuesare now properly memoized withuseMemoand appropriate dependencies, addressing the previous review comment. This prevents unnecessary Formik reinitializations when the component re-renders for unrelated reasons.
182-258: LGTM! Proper sequential mutation handling.The
onSubmithandler correctly:
- Executes all mutations sequentially with
await- Only dispatches success toast and logs audit after all mutations complete (lines 246-253)
- Catches any mutation failures and dispatches danger toast without logging success (lines 254-257)
This addresses the previous concern about partial failure logging. The sequential execution ensures atomicity in user perception (though not transactional at the API level).
262-297: LGTM! Provider type selection properly wired.The cache provider type selector correctly:
- Updates both local state (for immediate UI rendering) and Formik values
- Conditionally renders the appropriate provider-specific form component
- Uses i18n for option labels
The dual state management pattern is appropriate here for responsive UI updates.
298-310: LGTM! Proper form footer with appropriate guards.The form footer correctly:
- Uses explicit spacing with a div (line 298) instead of an empty
FormGroup- Gates the apply button on permission (
canWriteCache), validation (formik.isValid), and changes (formik.dirty)- Shows/hides buttons based on permissions
- Disables cancel when form is pristine
157-176: LGTM! Comprehensive loading state management.The component properly:
- Aggregates all mutation states into
isMutating(lines 157-162)- Blocks UI during both data loading and mutations (line 172)
- Returns
nullearly if read permission is denied (lines 164-166)- Provides loading feedback throughout async operations
admin-ui/plugins/services/Components/Configuration/LdapForm.tsx (7)
35-45: Excellent extraction of normalization logic.The normalization helper functions address the previous review comment about duplicated logic. Both
normalizeServersandnormalizeBaseDNsare properly typed and will be reused inonSubmitandcheckLdapConnection, eliminating code duplication.
60-75: LGTM! Function naming and mutation setup improved.The changes address previous review comments:
- ✅
tooglerenamed toactivateValidation(line 60) with clear intent- ✅ Test mutation integrated with proper success/error handlers using i18n messages
The mutation pattern follows the PR's broader migration from Redux actions to React Query mutations.
103-112: LGTM! onSubmit uses extracted normalization helpers.The submit handler correctly:
- Uses
normalizeServersandnormalizeBaseDNshelper functions (addressing previous duplication concern)- Merges
itemandvaluesto preserve additional properties- Properly types the result as
GluuLdapConfiguration- Conditionally wraps in
{ ldap: result }for create operations
119-131: Excellent refactoring of form submission and test connection.Both functions now follow best practices:
- ✅
handleFormSubmitusesformik.submitForm()instead of DOM manipulation (addressing previous review comment)- ✅
checkLdapConnectionuses the extracted normalization helpers for consistency- Both properly construct typed
GluuLdapConfigurationobjects
134-221: LGTM! Consistent field validation activation.The form fields correctly:
- Use
activateValidation(not the oldtoogle) foronKeyUphandlers- Include test mutation state in the loader (line 135)
- Disable
configIdfield when editing (line 157)- Show validation errors with proper color styling
264-296: LGTM! Proper type handling for GluuTypeAhead.The TypeAhead fields correctly:
- Cast
formiktoFormikContextType<unknown>for component compatibility (lines 268, 288)- Stringify validation errors with
String()to prevent React rendering errors (lines 277, 294)- Maintain proper required field indicators
This pattern is consistent with the SqlForm.tsx approach.
361-366: LGTM! Commit dialog properly integrated.The
GluuCommitDialogcorrectly uses the newhandleFormSubmitfunction, completing the refactoring away from DOM manipulation. All past review comments have been addressed in this file.admin-ui/plugins/services/Components/Configuration/hooks/index.ts (6)
5-29: Excellent refactoring of auth state selection.The code addresses previous review comments:
- ✅
AuthRootStateis imported from the centralUtils/typesfile (line 5) instead of being duplicated locally- ✅ Common auth state selection is extracted into
useAuditAuthhelper (lines 18-29)- Proper use of
useMemowith granular dependencies prevents unnecessary recomputationThis DRY refactoring will be reused across all four audit hooks.
31-105: Excellent implementation of LDAP audit hooks.All previous review comments have been addressed:
- ✅ Uses extracted
useAuditAuthhelper (line 32) eliminating duplication- ✅ All audit functions wrapped in try-catch blocks (lines 40-54, 65-79, 86-99) ensuring fire-and-forget behavior
- ✅
logLdapDeleteaccepts fullldapobject and includes it in the payload (line 95), not justconfigIdThe error handling pattern ensures audit failures don't disrupt user workflows.
107-173: LGTM! SQL audit hooks follow the same improved pattern.The SQL audit hooks mirror the LDAP implementation:
- ✅ Uses
useAuditAuthhelper- ✅ All functions wrapped in try-catch for fire-and-forget behavior
- ✅
logSqlDeleteaccepts and includes fullsqlobject payload (line 163)Consistent implementation across resource types improves maintainability.
175-204: LGTM! Cache audit hook properly implemented.The cache audit hook:
- ✅ Uses
useAuditAuthhelper for consistency- ✅ Wraps audit call in try-catch (lines 184-198)
- Uses
PATCHaction (appropriate for cache configuration updates)- Uses
cacheProviderTypeasperformedOnidentifier (reasonable since cache config is a singleton)
206-260: LGTM! Couchbase audit hooks complete the set.The Couchbase audit hooks:
- ✅ Use
useAuditAuthhelper- ✅ Both create and update wrapped in try-catch blocks
- Follow the same patterns as LDAP and SQL hooks
- Use
configIdasperformedOnidentifierThe module provides comprehensive audit coverage for all four resource types with consistent error handling.
1-260: Excellent audit hooks module implementation.This module successfully addresses all previous review comments and provides:
- ✅ Centralized auth state selection with
useAuditAuth- ✅ Imported types from central location (
AuthRootStatefromUtils/types)- ✅ Fire-and-forget error handling in all audit functions
- ✅ Full payload logging (including in delete operations)
The consistent patterns across all four hooks (LDAP, SQL, Cache, Couchbase) make the codebase maintainable and predictable. This is a solid foundation for the Services plugin's audit logging.
admin-ui/plugins/services/Components/Configuration/types.ts (3)
1-8: LGTM! Clean imports aligned with PR objectives.The imports properly leverage the TypeScript-generated client types from JansConfigApi and integrate with Formik for type-safe form handling.
100-108: LGTM! Appropriate optional fields for read-only metadata.All fields in
PersistenceInfoare appropriately marked optional since this appears to be read-only metadata about the persistence layer that may not always be fully populated.
81-98: No action needed. TheCouchbaseFormValuesinterface is not used by the Couchbase form components.CouchbasePage.tsxusesCouchbaseConfigurationfrom the API client instead, andCouchbaseItem.tsxis a read-only display component with no client-side validation. UnlikeSqlForm.tsxandLdapForm.tsxwhich have explicit Yup validation schemas, there is no frontend validation for Couchbase. The form submission directly passes data to the backend API, which is responsible for validation.Likely an incorrect or invalid review comment.
admin-ui/plugins/services/Components/Configuration/CouchbasePage.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
Outdated
Show resolved
Hide resolved
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 (1)
admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
128-136: Missing revokeSessionMutation in useCallback dependencies.The
submitDatacallback usesrevokeSessionMutationbut it's not included in the dependency array. While mutation objects from React Query are stable, ESLint exhaustive-deps rules typically flag this.🔧 Suggested fix
[personAttributes, userDetails, persistenceType, standardFields, updateUserMutation], + // revokeSessionMutation is stable but add for completeness if linter requires )
🤖 Fix all issues with AI agents
In @admin-ui/plugins/services/Components/Configuration/CouchbasePage.tsx:
- Around line 84-100: The component renders items from the raw `couchbase` prop
while Formik is initialized from that same prop with `enableReinitialize`, which
can cause the UI to reset mid-edit; change the rendering to use Formik's current
state by iterating `formik.values` (not `couchbase`) when mapping to
`CouchbaseItem`, keep passing `formik`, `index`, and `couchbase` prop data from
the corresponding `formik.values[index]`, and ensure `initialValues` and
`onSubmit` (e.g., `handleSubmit`) remain unchanged so edits are preserved and
reinitialization behaves consistently.
In @admin-ui/plugins/services/Components/Configuration/LdapEditPage.tsx:
- Around line 55-65: The LdapEditPage currently renders a static placeholder
when item is falsy; instead redirect to the LDAP list for better UX. Replace the
early return that renders the Container/Card with a navigation action: inside
LdapEditPage use the router navigation hook (e.g., useNavigate or useHistory
depending on project router version) and in a useEffect when item is null call
navigate('/admin/ldap' or the app’s LDAP list route) so the page redirects
automatically; remove the static placeholder return and ensure any cleanup or
focus logic remains intact.
In @admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx:
- Around line 37-41: Change the AlertState severity union to match SqlListPage
by using undefined instead of an empty string: update the interface declaration
(AlertState) to severity: 'success' | 'error' | 'warning' | 'info' | undefined,
and then replace any initial or reset uses that assign '' with undefined (e.g.,
the initial alert state and all setAlert/reset calls around the places that
previously used '' such as the reset logic in LdapListPage) so GluuAlert
receives either a valid MUI severity or undefined.
- Around line 266-280: The onDeletionConfirmed handler currently calls
navigateBack(ROUTES.LDAP_LIST) and toggle() unconditionally, so users are
redirected even when deleteMutation.mutateAsync or logLdapDelete fails; move the
navigateBack and toggle calls into the success path (inside the try block) and
keep the catch block to handle/log the error (e.g., show toast via existing
error handling) so navigation only happens after successful delete and audit
logging; update the dependency list if needed and reference the
onDeletionConfirmed function, deleteMutation.mutateAsync, logLdapDelete,
navigateBack, and toggle when making the change.
In @admin-ui/plugins/services/Components/Configuration/SqlAddPage.tsx:
- Around line 40-48: The handleSubmit function duplicates an action_message
extraction pattern that appears across pages; extract this logic into a shared
utility (e.g., extractActionMessage) that accepts the form payload and a default
string and returns { cleanData, message }; update handleSubmit to call this
utility to set actionMessageRef.current and pass cleanData into
addMutation.mutate (retain use of addMutation.mutate and actionMessageRef) so
the extraction is centralized and reusable across SqlAddPage, LdapEditPage, etc.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
admin-ui/app/locales/en/translation.jsonadmin-ui/plugins/services/Components/Configuration/CachePage.tsxadmin-ui/plugins/services/Components/Configuration/CouchbasePage.tsxadmin-ui/plugins/services/Components/Configuration/LdapAddPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapEditPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapForm.tsxadmin-ui/plugins/services/Components/Configuration/LdapListPage.tsxadmin-ui/plugins/services/Components/Configuration/SqlAddPage.tsxadmin-ui/plugins/services/Components/Configuration/SqlEditPage.tsxadmin-ui/plugins/services/Components/Configuration/SqlListPage.tsxadmin-ui/plugins/services/Components/Configuration/types.tsadmin-ui/plugins/user-management/components/UserEditPage.tsx
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2026-01-07T09:26:24.613Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx:112-172
Timestamp: 2026-01-07T09:26:24.613Z
Learning: In React components using Formik, do not extract individual Formik methods (such as setValues, setTouched, validateForm) or properties (such as touched, errors) into separate variables/refs to use in dependency arrays. Keeping the entire formik object in the dependency array is the recommended pattern, as extracting methods separately can cause issues with form focusing and break Formik's internal field registration and focus management.
Applied to files:
admin-ui/plugins/services/Components/Configuration/SqlEditPage.tsxadmin-ui/plugins/services/Components/Configuration/CachePage.tsxadmin-ui/plugins/services/Components/Configuration/LdapForm.tsx
📚 Learning: 2025-11-06T08:23:20.948Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2418
File: admin-ui/plugins/user-management/components/UserForm.tsx:290-297
Timestamp: 2025-11-06T08:23:20.948Z
Learning: In the UserForm component (admin-ui/plugins/user-management/components/UserForm.tsx), the Back button should fall back to '/user/usersmanagement' when browser history is unavailable, not '/home/dashboard', as this keeps users in the user management context. This aligns with the new requirement that users should be redirected to the respective listing screen of the component rather than the dashboard.
Applied to files:
admin-ui/plugins/user-management/components/UserEditPage.tsxadmin-ui/plugins/services/Components/Configuration/SqlListPage.tsx
📚 Learning: 2025-11-10T14:18:58.310Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
Applied to files:
admin-ui/plugins/user-management/components/UserEditPage.tsxadmin-ui/plugins/services/Components/Configuration/SqlListPage.tsxadmin-ui/plugins/services/Components/Configuration/CouchbasePage.tsxadmin-ui/plugins/services/Components/Configuration/CachePage.tsx
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Applied to files:
admin-ui/plugins/user-management/components/UserEditPage.tsx
📚 Learning: 2025-12-05T18:24:41.713Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2498
File: admin-ui/plugins/saml/components/SamlConfigurationForm.tsx:30-30
Timestamp: 2025-12-05T18:24:41.713Z
Learning: In the GluuFederation/flex admin-ui codebase, `SetTitle` (imported from 'Utils/SetTitle') is a custom React hook that should be called at the component's top level, not inside `useEffect` or other hooks, because it internally manages side effects for setting page titles.
Applied to files:
admin-ui/plugins/user-management/components/UserEditPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapListPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapForm.tsx
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.
Applied to files:
admin-ui/plugins/services/Components/Configuration/SqlListPage.tsx
📚 Learning: 2025-11-03T08:47:33.943Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/services/Components/Configuration/SqlListPage.tsx
📚 Learning: 2025-11-26T12:25:34.923Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2475
File: admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js:111-216
Timestamp: 2025-11-26T12:25:34.923Z
Learning: In the admin-ui codebase, avoid suggesting comments for small functionalities or straightforward layout decisions in smaller components. Comments should be reserved for more complex or non-obvious logic.
Applied to files:
admin-ui/plugins/services/Components/Configuration/CachePage.tsx
📚 Learning: 2026-01-07T09:56:02.063Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx:320-322
Timestamp: 2026-01-07T09:56:02.063Z
Learning: In the admin-ui Config API Properties forms (ApiConfigForm component in admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx), the error display pattern using `String(fieldError)` for top-level properties is acceptable because the DTO structure and validation schema design ensure that top-level errors are string messages, not nested error objects. Nested field validation is handled within JsonPropertyBuilderConfigApi components themselves.
Applied to files:
admin-ui/plugins/services/Components/Configuration/types.ts
📚 Learning: 2026-01-06T13:45:19.278Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts:4-101
Timestamp: 2026-01-06T13:45:19.278Z
Learning: In admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts, ensure Yup schema marks fields as nullable when the backend DTOs permit nulls and do not mark them as required. Align frontend validation with backend validation to prevent conflicts when editing existing configurations with null values; apply this pattern consistently to other Admin UI validation files where the backend is the source of truth.
Applied to files:
admin-ui/plugins/services/Components/Configuration/types.ts
🧬 Code graph analysis (6)
admin-ui/plugins/services/Components/Configuration/LdapAddPage.tsx (3)
admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useLdapAudit(31-105)admin-ui/app/helpers/navigation.ts (1)
ROUTES(204-204)admin-ui/app/components/index.tsx (2)
Container(62-62)CardBody(44-44)
admin-ui/plugins/user-management/components/UserEditPage.tsx (1)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
isPersistenceInfo(190-194)
admin-ui/plugins/services/Components/Configuration/CouchbasePage.tsx (2)
admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useCouchbaseAudit(206-260)admin-ui/plugins/services/Components/Configuration/types.ts (1)
isPersistenceInfo(190-194)
admin-ui/plugins/services/Components/Configuration/CachePage.tsx (2)
admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useCacheAudit(175-204)admin-ui/plugins/services/Components/Configuration/types.ts (5)
CacheFormValues(83-110)isNativePersistenceCache(131-135)isInMemoryCache(113-117)isRedisCache(125-129)isMemcachedCache(119-123)
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx (4)
admin-ui/plugins/services/Components/Configuration/atoms/index.ts (1)
currentLdapItemAtom(4-4)admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useLdapAudit(31-105)admin-ui/plugins/services/Components/Configuration/types.ts (1)
isPersistenceInfo(190-194)admin-ui/app/utils/pagingUtils.ts (1)
getPagingSize(1-24)
admin-ui/plugins/services/Components/Configuration/LdapForm.tsx (3)
admin-ui/plugins/services/Components/Configuration/types.ts (1)
LdapFormProps(10-15)admin-ui/app/components/index.tsx (2)
Form(68-68)FormGroup(70-70)admin-ui/app/customColors.ts (1)
customColors(4-25)
🔇 Additional comments (18)
admin-ui/app/locales/en/translation.json (1)
840-842: LGTM! Well-structured translation entries for the new error handling patterns.The new messages provide clear, actionable feedback:
partial_update_failurehandles batch operation scenariosmin_charactersuses proper i18next interpolation with{{count}}- Persistence-related messages provide both summary and detail levels for appropriate UI contexts
Also applies to: 893-894
admin-ui/plugins/services/Components/Configuration/CouchbasePage.tsx (1)
45-74: Good implementation of partial failure handling with Promise.allSettled.The batch update logic correctly:
- Handles all configurations in parallel
- Distinguishes between full success, full failure, and partial failure states
- Logs partial failures with error details
- Uses the new
partial_update_failuretranslation keyadmin-ui/plugins/services/Components/Configuration/CachePage.tsx (2)
119-145: Good use of useMemo for initialValues.The memoization correctly depends on all cache data sources, preventing unnecessary Formik reinitializations on unrelated re-renders.
261-290: Provider selector handles state synchronization correctly.The dual-state approach (local
cacheProviderType+ Formik's field value) ensures the UI updates immediately while keeping form state in sync. TheuseEffectat line 113-117 initializes from server data appropriately.admin-ui/plugins/services/Components/Configuration/types.ts (3)
74-110: Good dual-approach for cache form types.The design provides both:
CacheFormValuesUnion(lines 75-79) for strict discriminated union type safetyCacheFormValues(lines 83-110) for Formik compatibility with all fields optionalThe comment at lines 81-82 clearly documents this intentional design decision. The type guards (lines 112-135) enable safe narrowing at runtime.
190-193: Type guard correctly excludes arrays.The
isPersistenceInfoguard properly checks!Array.isArray(data), which is important since some API responses could return arrays. This prevents false positives when narrowing types.
10-26: Props interfaces now have consistent isLoading optional flag.Both
LdapFormProps(line 14) andSqlFormProps(line 25) now includeisLoading?: boolean. ThecreateLdapflag inLdapFormPropsserves a different purpose (mode distinction) and its absence inSqlFormPropsis acceptable if SQL doesn't need create/edit mode differentiation at the form level.admin-ui/plugins/user-management/components/UserEditPage.tsx (2)
55-72: Persistence handling implementation addresses all prior concerns.The implementation correctly:
- Uses the shared
isPersistenceInfotype guard instead of type assertions (line 62-64)- Tracks loading state with
loadingPersistence(line 57)- Handles errors with toast notification (lines 68-72)
- Uses
useRefto prevent duplicate toasts on re-renders (line 66)
142-147: Good UX pattern for surfacing persistence errors.The inline Alert provides persistent visibility of the error state, while the toast provides immediate notification. This dual approach ensures users are aware of the degraded functionality (LDAP-specific settings may not apply) without blocking form usage.
admin-ui/plugins/services/Components/Configuration/SqlAddPage.tsx (2)
22-38: Solid mutation setup with proper error handling.The mutation correctly:
- Shows toast feedback for both success and error states
- Invalidates query cache to refresh the list
- Wraps audit logging in try/catch to prevent audit failures from blocking navigation
- Uses
navigateBackfor proper navigation flow
50-50: SqlForm properly handles empty initial configurations. The component initializes all form fields with sensible defaults (empty strings, empty arrays, or false) and enforces validation on required fields (configId, userName, userPassword, connectionUri, schemaName). Formik prevents form submission until all required fields are valid, and error messages are displayed to guide users when fields are incomplete. No further action needed.admin-ui/plugins/services/Components/Configuration/LdapEditPage.tsx (2)
44-53: Correct handling of LdapFormProps union type.The
handleSubmitfunction properly handles both variants of the union type ({ ldap: GluuLdapConfiguration } | GluuLdapConfiguration) by checking for theldapproperty at line 46. This aligns with theLdapFormPropsinterface definition in types.ts.
72-76: Good use of cloneDeep to prevent atom mutation.Using
cloneDeep(item)ensures the form operates on an independent copy, preventing accidental mutations of the Jotai atom's value. This is especially important since Formik mutates its values object during form interactions.admin-ui/plugins/services/Components/Configuration/SqlEditPage.tsx (1)
18-81: LGTM! Past review concerns have been addressed.The component correctly implements the SQL configuration edit flow with proper state management, audit logging via
actionMessageRef, and deep cloning to prevent atom mutation. The mutation setup, cache invalidation, and navigation flow are consistent with other edit pages in this PR.admin-ui/plugins/services/Components/Configuration/SqlListPage.tsx (1)
53-323: LGTM! Past review concerns have been addressed.The SQL list page properly implements data fetching, permission-based actions, mutations with cache invalidation, and a test connection flow. The deletion handler correctly navigates only on success (line 247 inside try block), and
AlertStatenow usesundefinedfor severity instead of an empty string.admin-ui/plugins/services/Components/Configuration/LdapAddPage.tsx (1)
15-72: LGTM! Past review concerns have been addressed.The LDAP add page correctly implements the creation flow with a module-level default configuration constant (lines 15-20), proper mutation handling, cache invalidation, audit logging via
actionMessageRef, and data normalization before submission.admin-ui/plugins/services/Components/Configuration/LdapForm.tsx (2)
29-125: LGTM! Past review concerns have been addressed.The form correctly extracts normalization helpers (lines 29-39), renames the validation trigger to
activateValidation(line 54), and usesformik.submitForm()directly (line 115) instead of DOM manipulation. The test mutation integration and loading state handling are properly implemented.
262-288: Formik type casts and error stringification are appropriate.The casts to
FormikContextType<unknown>(lines 262, 282) accommodateGluuTypeAhead's typing constraints, and theString()calls (lines 271, 288) safely handle non-string error messages (e.g., array validation errors from Yup) for consistent rendering.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx:
- Around line 266-280: The onDeletionConfirmed callback currently calls toggle()
unconditionally after attempting deletion; move the toggle() call inside the try
block (after navigateBack(ROUTES.LDAP_LIST)) so the dialog only closes on
successful deletion. Update the onDeletionConfirmed function (references:
onDeletionConfirmed, deleteMutation.mutateAsync, logLdapDelete, navigateBack,
toggle) to call toggle() after successful await calls and keep the catch block
unchanged so the dialog remains open on error to allow retry.
In @admin-ui/plugins/services/Components/Configuration/types.ts:
- Around line 103-125: The current type guards accept CacheFormValues and return
intersections; update them to accept CacheFormValuesUnion and return the
specific provider types to leverage discriminated union narrowing: change each
function signature (isInMemoryCache, isMemcachedCache, isRedisCache,
isNativePersistenceCache) to take values: CacheFormValuesUnion and have their
return type be the corresponding provider type (InMemoryCacheFormValues,
MemcachedCacheFormValues, RedisCacheFormValues,
NativePersistenceCacheFormValues) while keeping the same predicate body
(checking values.cacheProviderType).
- Around line 78-79: The CacheFormValues type weakly types cacheProviderType as
"CacheProviderType | string"; change it to strictly use CacheProviderType
(remove the "| string") to restore type safety and handle unknown API values
explicitly elsewhere, or if you must accept unknowns, extend the
CacheProviderType union to include those values and add an exhaustive handler
where cacheProviderType is consumed (e.g., in form parsing or switch statements)
to avoid silent runtime errors; update the interface name CacheFormValues and
the property cacheProviderType accordingly and adjust any callers that relied on
arbitrary strings to validate/normalize unknown provider values.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
admin-ui/plugins/services/Components/Configuration/CouchbasePage.tsxadmin-ui/plugins/services/Components/Configuration/LdapAddPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapEditPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapListPage.tsxadmin-ui/plugins/services/Components/Configuration/SqlAddPage.tsxadmin-ui/plugins/services/Components/Configuration/SqlEditPage.tsxadmin-ui/plugins/services/Components/Configuration/types.ts
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-11-10T14:18:58.310Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
Applied to files:
admin-ui/plugins/services/Components/Configuration/CouchbasePage.tsxadmin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
📚 Learning: 2026-01-07T09:26:24.613Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx:112-172
Timestamp: 2026-01-07T09:26:24.613Z
Learning: In React components using Formik, do not extract individual Formik methods (such as setValues, setTouched, validateForm) or properties (such as touched, errors) into separate variables/refs to use in dependency arrays. Keeping the entire formik object in the dependency array is the recommended pattern, as extracting methods separately can cause issues with form focusing and break Formik's internal field registration and focus management.
Applied to files:
admin-ui/plugins/services/Components/Configuration/CouchbasePage.tsxadmin-ui/plugins/services/Components/Configuration/SqlEditPage.tsx
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.
Applied to files:
admin-ui/plugins/services/Components/Configuration/LdapEditPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
📚 Learning: 2025-11-06T08:23:20.948Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2418
File: admin-ui/plugins/user-management/components/UserForm.tsx:290-297
Timestamp: 2025-11-06T08:23:20.948Z
Learning: In the UserForm component (admin-ui/plugins/user-management/components/UserForm.tsx), the Back button should fall back to '/user/usersmanagement' when browser history is unavailable, not '/home/dashboard', as this keeps users in the user management context. This aligns with the new requirement that users should be redirected to the respective listing screen of the component rather than the dashboard.
Applied to files:
admin-ui/plugins/services/Components/Configuration/LdapEditPage.tsxadmin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
📚 Learning: 2025-11-03T08:47:33.943Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
📚 Learning: 2025-12-05T18:24:41.713Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2498
File: admin-ui/plugins/saml/components/SamlConfigurationForm.tsx:30-30
Timestamp: 2025-12-05T18:24:41.713Z
Learning: In the GluuFederation/flex admin-ui codebase, `SetTitle` (imported from 'Utils/SetTitle') is a custom React hook that should be called at the component's top level, not inside `useEffect` or other hooks, because it internally manages side effects for setting page titles.
Applied to files:
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
📚 Learning: 2026-01-07T09:56:02.063Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx:320-322
Timestamp: 2026-01-07T09:56:02.063Z
Learning: In the admin-ui Config API Properties forms (ApiConfigForm component in admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx), the error display pattern using `String(fieldError)` for top-level properties is acceptable because the DTO structure and validation schema design ensure that top-level errors are string messages, not nested error objects. Nested field validation is handled within JsonPropertyBuilderConfigApi components themselves.
Applied to files:
admin-ui/plugins/services/Components/Configuration/types.ts
📚 Learning: 2026-01-06T13:45:19.278Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts:4-101
Timestamp: 2026-01-06T13:45:19.278Z
Learning: In admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts, ensure Yup schema marks fields as nullable when the backend DTOs permit nulls and do not mark them as required. Align frontend validation with backend validation to prevent conflicts when editing existing configurations with null values; apply this pattern consistently to other Admin UI validation files where the backend is the source of truth.
Applied to files:
admin-ui/plugins/services/Components/Configuration/types.ts
🧬 Code graph analysis (5)
admin-ui/plugins/services/Components/Configuration/CouchbasePage.tsx (1)
admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useCouchbaseAudit(206-260)
admin-ui/plugins/services/Components/Configuration/SqlEditPage.tsx (6)
admin-ui/plugins/services/Components/Configuration/atoms/index.ts (1)
currentSqlItemAtom(6-6)admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useSqlAudit(107-173)admin-ui/app/helpers/navigation.ts (1)
ROUTES(204-204)admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (1)
handleSubmit(145-162)admin-ui/plugins/services/Components/Configuration/types.ts (1)
extractActionMessage(186-195)admin-ui/app/components/index.tsx (2)
Container(62-62)CardBody(44-44)
admin-ui/plugins/services/Components/Configuration/LdapAddPage.tsx (5)
admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useLdapAudit(31-105)admin-ui/app/helpers/navigation.ts (1)
ROUTES(204-204)admin-ui/plugins/auth-server/components/Configuration/ConfigPage.js (1)
handleSubmit(145-162)admin-ui/plugins/services/Components/Configuration/types.ts (1)
extractActionMessage(186-195)admin-ui/app/components/index.tsx (2)
Container(62-62)CardBody(44-44)
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx (5)
admin-ui/plugins/services/Components/Configuration/atoms/index.ts (1)
currentLdapItemAtom(4-4)admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useLdapAudit(31-105)admin-ui/plugins/services/Components/Configuration/types.ts (1)
isPersistenceInfo(180-184)admin-ui/app/utils/pagingUtils.ts (1)
getPagingSize(1-24)admin-ui/app/helpers/navigation.ts (1)
ROUTES(204-204)
admin-ui/plugins/services/Components/Configuration/SqlAddPage.tsx (1)
admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useSqlAudit(107-173)
🔇 Additional comments (17)
admin-ui/plugins/services/Components/Configuration/SqlAddPage.tsx (1)
41-50: LGTM! Action message extraction now uses centralized utility.The previous concern about duplicating the
action_messageextraction pattern has been properly addressed. The code now uses theextractActionMessageutility function (line 14) fromtypes.ts, making this pattern reusable across all add/edit pages in the Services plugin.admin-ui/plugins/services/Components/Configuration/LdapEditPage.tsx (1)
57-65: LGTM! Auto-redirect implemented for better UX.The previous suggestion to redirect instead of showing a placeholder has been properly implemented. The page now uses
useEffectto automatically navigate toROUTES.LDAP_LISTwhen no item is selected (lines 57-61), with an earlynullreturn to prevent flickering. This keeps users in the LDAP management context, consistent with learned patterns from the codebase.Based on learnings, this aligns with the requirement that users should be redirected to the respective listing screen rather than showing a static message.
admin-ui/plugins/services/Components/Configuration/CouchbasePage.tsx (3)
45-74: Excellent! Parallel updates with comprehensive failure handling.The previous concern about sequential mutations causing inconsistent state has been properly resolved. The code now uses
Promise.allSettled(lines 48-54) to process all configuration updates in parallel, with robust handling for:
- Full success (line 62)
- Full failure (line 64)
- Partial failure with warning toast (line 66) and detailed error logging (lines 67-70)
This ensures all configurations are attempted and provides clear feedback about outcomes.
91-99: LGTM! Now rendering from Formik state.The code now correctly renders from
formik.values(lines 91-92) instead of the rawcouchbasequery data. This prevents mid-edit resets when server data changes withenableReinitialize, preserving user edits as intended.Additionally, the component now uses a stable key pattern
couchbaseItem.configId ?? index(line 94) for proper React reconciliation.
39-41: LGTM! Type guard applied for consistency.The code now uses the
isPersistenceInfotype guard (imported fromtypes.tson line 21) instead of a direct type assertion, providing type safety consistent withSqlListPage.tsxand other components in the Services plugin.admin-ui/plugins/services/Components/Configuration/SqlEditPage.tsx (2)
45-54: LGTM! Action message properly wired to audit logging.The previous concern about custom
action_messagevalues being lost has been properly addressed. The code now:
- Extracts the message via
extractActionMessage(lines 47-50)- Stores it in
actionMessageRef.current(line 51)- Passes it to
logSqlUpdatein the mutation'sonSuccesshandler (line 33)This ensures user-provided audit context flows through to the audit log while falling back to the default message when not provided.
71-75: LGTM! Deep cloning applied for consistency.The code now uses
cloneDeep(item)(line 72) when passing the configuration toSqlForm, consistent withLdapEditPage. This prevents Formik mutations from affecting the atom state, ensuring proper isolation between form state and application state.admin-ui/plugins/services/Components/Configuration/LdapAddPage.tsx (1)
16-21: LGTM! Default configuration now a module-level constant.The previous suggestion to extract
defaultConfigurationsas a module-level constant has been properly implemented. TheDEFAULT_LDAP_CONFIGURATIONconstant (lines 16-21):
- Prevents recreation on each render
- Uses clear uppercase naming convention
- Is properly typed as
GluuLdapConfiguration- Can be easily imported for testing or reuse
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx (5)
37-41: AlertState severity type properly aligned.The
AlertStateinterface now correctly usesundefinedinstead of an empty string for the severity type, which aligns with MUI's expected severity values and is consistent with the SqlListPage pattern.
72-82: Well-structured data fetching with proper type guarding.Good use of React Query for data fetching with appropriate
staleTimeconfiguration. TheisPersistenceInfotype guard properly validates the persistence data before accessingpersistenceType, addressing potential runtime type mismatches.
107-108: State initialization properly typed.Good improvement -
itemstate is now correctly typed asGluuLdapConfiguration | nullwithnullinitialization, which properly represents the absence of a selected item.
179-253: Actions array well-structured with proper permission gating.The dynamic action generation correctly handles MaterialTable's signature where
rowDatacan be either a single item or an array. The permission-based action filtering and query invalidation for refresh are properly implemented.
319-344: Loading state and data handling properly implemented.Good implementation combining multiple loading states (
loading,deleteMutation.isPending,testMutation.isPending) into a singleisLoadingflag. The data array validation on line 344 ensuresldapConfigurationsis safely passed to MaterialTable even if undefined.admin-ui/plugins/services/Components/Configuration/types.ts (4)
10-26: Form props properly typed with isLoading support.
SqlFormProps.handleSubmitnow correctly accepts only the wrapped object form{ sql: SqlConfiguration }, aligning the type definition with actual usage. Both form props includeisLoadingfor consistent loading state handling.
33-76: Discriminated union pattern well-implemented for cache providers.The
CacheFormValuesUniontype with provider-specific interfaces (InMemoryCacheFormValues,MemcachedCacheFormValues, etc.) enables compile-time enforcement of which fields are valid for each provider. This is a much cleaner approach than a flat interface with all optional fields.
127-168: Component props properly typed for Formik integration.Cache and Couchbase component props are well-structured with appropriate Formik bindings. The
CouchbaseItemPropscorrectly usesFormikProps<CouchbaseConfiguration[]>for array-based form handling.
180-195: Type guard and utility function well-implemented.
isPersistenceInfocorrectly validates that data is a non-array object containingpersistenceTypebefore type narrowing. TheextractActionMessageutility cleanly separates the audit message from form data using object destructuring.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx:
- Around line 168-171: handleLdapDelete uses toggle() but it's missing from the
useCallback dependency array; update the useCallback for handleLdapDelete to
include toggle (and any other stable references like setItem if not guaranteed
stable) so the closure stays current — locate the useCallback that defines
handleLdapDelete and add toggle to its dependency array.
- Around line 291-293: The useEffect that calls setAlertObj({ severity:
undefined, message: '', show: false }) on mount is redundant because alertObj is
already initialized with those values; remove the useEffect block (the call to
useEffect that references setAlertObj) to avoid unnecessary mount-side effects
and keep the initial state in the alertObj declaration.
In @admin-ui/plugins/services/Components/Configuration/types.ts:
- Around line 180-184: The isPersistenceInfo type guard can return true for
objects where persistenceType is undefined; update isPersistenceInfo to check
that data is an object and that the persistenceType property exists and is of
the expected type (e.g., typeof (data as any).persistenceType === 'string' or
!== undefined) before narrowing to PersistenceInfo; reference the
isPersistenceInfo function and the PersistenceInfo type to locate the check and
add the extra typeof/property existence validation.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
admin-ui/plugins/services/Components/Configuration/CachePage.tsxadmin-ui/plugins/services/Components/Configuration/LdapListPage.tsxadmin-ui/plugins/services/Components/Configuration/SqlListPage.tsxadmin-ui/plugins/services/Components/Configuration/types.ts
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-11-10T14:18:58.310Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
Applied to files:
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsxadmin-ui/plugins/services/Components/Configuration/CachePage.tsxadmin-ui/plugins/services/Components/Configuration/SqlListPage.tsx
📚 Learning: 2025-12-05T18:24:41.713Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2498
File: admin-ui/plugins/saml/components/SamlConfigurationForm.tsx:30-30
Timestamp: 2025-12-05T18:24:41.713Z
Learning: In the GluuFederation/flex admin-ui codebase, `SetTitle` (imported from 'Utils/SetTitle') is a custom React hook that should be called at the component's top level, not inside `useEffect` or other hooks, because it internally manages side effects for setting page titles.
Applied to files:
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
📚 Learning: 2026-01-07T09:26:24.613Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx:112-172
Timestamp: 2026-01-07T09:26:24.613Z
Learning: In React components using Formik, do not extract individual Formik methods (such as setValues, setTouched, validateForm) or properties (such as touched, errors) into separate variables/refs to use in dependency arrays. Keeping the entire formik object in the dependency array is the recommended pattern, as extracting methods separately can cause issues with form focusing and break Formik's internal field registration and focus management.
Applied to files:
admin-ui/plugins/services/Components/Configuration/CachePage.tsx
📚 Learning: 2025-11-26T12:25:34.923Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2475
File: admin-ui/plugins/auth-server/components/Clients/ClientDetailPage.js:111-216
Timestamp: 2025-11-26T12:25:34.923Z
Learning: In the admin-ui codebase, avoid suggesting comments for small functionalities or straightforward layout decisions in smaller components. Comments should be reserved for more complex or non-obvious logic.
Applied to files:
admin-ui/plugins/services/Components/Configuration/CachePage.tsx
📚 Learning: 2025-11-05T19:57:35.143Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2415
File: admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx:148-153
Timestamp: 2025-11-05T19:57:35.143Z
Learning: In the SSA Add page (admin-ui/plugins/auth-server/components/Ssa/SsaAddPage.tsx), the Back button should fall back to '/auth-server/config/ssa' when browser history is unavailable, not '/home/dashboard', as this keeps users in the SSA management context.
Applied to files:
admin-ui/plugins/services/Components/Configuration/SqlListPage.tsx
📚 Learning: 2025-11-06T08:23:20.948Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2418
File: admin-ui/plugins/user-management/components/UserForm.tsx:290-297
Timestamp: 2025-11-06T08:23:20.948Z
Learning: In the UserForm component (admin-ui/plugins/user-management/components/UserForm.tsx), the Back button should fall back to '/user/usersmanagement' when browser history is unavailable, not '/home/dashboard', as this keeps users in the user management context. This aligns with the new requirement that users should be redirected to the respective listing screen of the component rather than the dashboard.
Applied to files:
admin-ui/plugins/services/Components/Configuration/SqlListPage.tsx
📚 Learning: 2025-11-03T08:47:33.943Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2404
File: admin-ui/app/routes/Apps/Gluu/Gluuformfooter.tsx:70-77
Timestamp: 2025-11-03T08:47:33.943Z
Learning: In the admin-ui forms (Gluuformfooter component), the Back button should always navigate to '/home/dashboard' rather than using history-based navigation (navigate(-1)), as clarified by faisalsiddique4400 in PR #2404.
Applied to files:
admin-ui/plugins/services/Components/Configuration/SqlListPage.tsx
📚 Learning: 2026-01-07T09:56:02.063Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx:320-322
Timestamp: 2026-01-07T09:56:02.063Z
Learning: In the admin-ui Config API Properties forms (ApiConfigForm component in admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx), the error display pattern using `String(fieldError)` for top-level properties is acceptable because the DTO structure and validation schema design ensure that top-level errors are string messages, not nested error objects. Nested field validation is handled within JsonPropertyBuilderConfigApi components themselves.
Applied to files:
admin-ui/plugins/services/Components/Configuration/types.ts
📚 Learning: 2026-01-06T13:45:19.278Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts:4-101
Timestamp: 2026-01-06T13:45:19.278Z
Learning: In admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts, ensure Yup schema marks fields as nullable when the backend DTOs permit nulls and do not mark them as required. Align frontend validation with backend validation to prevent conflicts when editing existing configurations with null values; apply this pattern consistently to other Admin UI validation files where the backend is the source of truth.
Applied to files:
admin-ui/plugins/services/Components/Configuration/types.ts
🧬 Code graph analysis (1)
admin-ui/plugins/services/Components/Configuration/CachePage.tsx (2)
admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useCacheAudit(175-204)admin-ui/plugins/services/Components/Configuration/types.ts (6)
CacheProviderType(33-33)CacheFormValues(78-101)isNativePersistenceCache(121-125)isInMemoryCache(103-107)isRedisCache(115-119)isMemcachedCache(109-113)
🔇 Additional comments (17)
admin-ui/plugins/services/Components/Configuration/types.ts (4)
1-31: LGTM! Clean interface definitions for LDAP and SQL forms.The prop interfaces are well-typed and properly leverage the generated JansConfigApi types. The
SqlFormProps.handleSubmitnow correctly accepts only the wrapped object shape{ sql: SqlConfiguration }, addressing the previous inconsistency.
33-76: Well-structured discriminated union for cache provider types.The
CacheFormValuesUniondiscriminated union with provider-specific interfaces is a good pattern that enables compile-time type safety when narrowing bycacheProviderType.
78-101: CacheFormValues flat interface is acceptable for Formik compatibility.While a discriminated union would be ideal for type safety, Formik forms typically work better with a flat interface where all fields exist (even if some are undefined based on the selected provider). The type guards below provide runtime narrowing when needed.
186-195: Utility function is clean and well-typed.The
extractActionMessagehelper correctly separates the action message from the payload data using rest destructuring and provides proper typing with generics.admin-ui/plugins/services/Components/Configuration/SqlListPage.tsx (5)
36-51: LGTM! Well-defined local types for component state.
AlertStatecorrectly usesundefinedinstead of empty string for the initial/reset severity state. TheActionObjectandActionTypetypes properly handle both static and dynamic action definitions for MaterialTable.
241-255: Navigation after successful deletion is now correct.The
toggle()andnavigateBack()calls are inside the try block, so they only execute on successful deletion. On error, the dialog remains open and the error is logged, allowing the user to retry.
122-132: Consider adding query invalidation on delete error for consistency.The delete mutation's
onErrorhandler dispatches a toast but doesn't invalidate queries. While the mutation error itself prevents stale state, you may want consistency with refresh behavior if partial operations could occur.
68-78: Verify staleTime aligns with data freshness requirements.Both queries use
staleTime: 30000(30 seconds). This is reasonable for configuration data that changes infrequently, but ensure this matches the expected update frequency in production.
272-322: LGTM! Clean table rendering with proper data guards.The component correctly guards against non-array data with
Array.isArray(sqlConfigurations) ? sqlConfigurations : []and shows an appropriate message when the persistence type is not SQL.admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx (4)
37-55: LGTM! Types align with SqlListPage pattern.
AlertStatenow correctly usesundefinedfor the null-state value, consistent withSqlListPage. The action types are properly defined.
107-115: LGTM! Item state correctly typed as nullable.Using
GluuLdapConfiguration | nullwithnullinitialization is the correct pattern for optional selection state, addressing the previous concern about empty object initialization.
266-280: Deletion flow correctly places toggle inside try block.The
toggle()call is now inside the try block, so the dialog only closes on successful deletion. This addresses the previous concern about unconditional dialog closure.
321-367: LGTM! Clean render logic with proper guards.The component properly gates LDAP-specific content based on
persistenceType, uses theisPersistenceInfotype guard for safe access, and handles loading states appropriately.admin-ui/plugins/services/Components/Configuration/CachePage.tsx (4)
69-91: LGTM! Data fetching with proper defaults and loading coordination.The multiple cache configuration hooks are well-organized with consistent
staleTimeand proper permission gating viaenabled: canReadCache. The consolidatedloadingstate correctly aggregates all loading states.
119-145: LGTM! Initial values properly memoized.The
initialValuesobject is now wrapped inuseMemowith appropriate dependencies, preventing unnecessary Formik reinitializations.
188-234: Type guards used correctly but only one mutation executes per submit.The type guards (
isNativePersistenceCache,isInMemoryCache, etc.) correctly narrow the form values. However, since only one provider can be active at a time (based oncacheProviderType), only one of these if-blocks will execute per submission. This is correct behavior but could be clearer with an explicit switch statement or early returns.
261-324: LGTM! Form rendering with proper permission-based controls.The form correctly disables/hides controls based on
canWriteCache, uses the commit dialog pattern for confirmation, and handles cancel/reset appropriately. The provider-type selector properly synchronizes local state with Formik state.
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
Outdated
Show resolved
Hide resolved
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx:
- Around line 168-180: The handleLdapDelete callback currently references toggle
before toggle is declared, causing a temporal dead zone error; fix it by moving
the toggle declaration (the useCallback that calls setModal) above
handleLdapDelete so toggle is defined before use, keep toggle's dependency array
as [] and retain handleLdapDelete's dependency on toggle, and ensure
handleGoToLdapAddPage and other callbacks remain unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsxadmin-ui/plugins/services/Components/Configuration/types.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-10T14:18:58.310Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
Applied to files:
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
📚 Learning: 2025-12-05T18:24:41.713Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2498
File: admin-ui/plugins/saml/components/SamlConfigurationForm.tsx:30-30
Timestamp: 2025-12-05T18:24:41.713Z
Learning: In the GluuFederation/flex admin-ui codebase, `SetTitle` (imported from 'Utils/SetTitle') is a custom React hook that should be called at the component's top level, not inside `useEffect` or other hooks, because it internally manages side effects for setting page titles.
Applied to files:
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
📚 Learning: 2026-01-07T09:56:02.063Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx:320-322
Timestamp: 2026-01-07T09:56:02.063Z
Learning: In the admin-ui Config API Properties forms (ApiConfigForm component in admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx), the error display pattern using `String(fieldError)` for top-level properties is acceptable because the DTO structure and validation schema design ensure that top-level errors are string messages, not nested error objects. Nested field validation is handled within JsonPropertyBuilderConfigApi components themselves.
Applied to files:
admin-ui/plugins/services/Components/Configuration/types.ts
📚 Learning: 2026-01-06T13:45:19.278Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2561
File: admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts:4-101
Timestamp: 2026-01-06T13:45:19.278Z
Learning: In admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/validations.ts, ensure Yup schema marks fields as nullable when the backend DTOs permit nulls and do not mark them as required. Align frontend validation with backend validation to prevent conflicts when editing existing configurations with null values; apply this pattern consistently to other Admin UI validation files where the backend is the source of truth.
Applied to files:
admin-ui/plugins/services/Components/Configuration/types.ts
🧬 Code graph analysis (1)
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx (3)
admin-ui/plugins/services/Components/Configuration/atoms/index.ts (1)
currentLdapItemAtom(4-4)admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useLdapAudit(31-105)admin-ui/plugins/services/Components/Configuration/types.ts (1)
isPersistenceInfo(180-186)
🪛 Biome (2.1.2)
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
[error] 173-173: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
🔇 Additional comments (9)
admin-ui/plugins/services/Components/Configuration/types.ts (5)
1-8: LGTM! Clean imports from generated client and Formik.The imports correctly reference the TypeScript-generated
JansConfigApitypes and Formik'sFormikPropsfor form state typing.
33-77: Well-structured discriminated union for cache provider types.The cache provider types are properly implemented as a discriminated union keyed on
cacheProviderType. Each variant (InMemoryCacheFormValues,MemcachedCacheFormValues,RedisCacheFormValues,NativePersistenceCacheFormValues) exposes only the relevant fields, providing compile-time type safety. TheCacheFormValuesUniontype correctly combines these variants.
78-101: CacheFormValues serves as a practical flat interface for Formik state.While a previous review suggested refactoring to use the discriminated union exclusively, maintaining this flat interface is reasonable for Formik form state management where all fields need to be accessible. The discriminated union (
CacheFormValuesUnion) and type guards provide type-safe narrowing when needed.Note:
cacheProviderTypeon line 79 now correctly uses the strictCacheProviderTypeunion instead ofCacheProviderType | string, which addresses the earlier type safety concern.
180-186: Type guard now includes proper runtime validation.The
isPersistenceInfofunction correctly validates that:
- Data is not null
- Data is an object (not an array)
- The
persistenceTypeproperty exists and is a stringThis addresses the earlier concern about the guard accepting objects where
persistenceTypecould be undefined.
188-197: Clean utility for extracting action messages from API responses.The
extractActionMessagefunction provides a type-safe way to separate audit messages from response data. The generic constraint ensures only objects with an optionalaction_messageproperty are accepted, and the return type correctly omits that property from the cleaned data.admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx (4)
72-82: Good integration of React Query with proper type guarding.The data fetching uses React Query hooks with
staleTimefor cache optimization. TheisPersistenceInfotype guard from./typesis correctly used to safely extractpersistenceType, handling the case where the API response shape might not match expectations.
126-157: Well-structured mutation hooks with proper side effects.The delete and test mutations are properly configured with:
onSuccess: Toast notification and query invalidation for delete; alert state update for testonError: Error toast/alert handling- Proper cleanup of
testRunningstate in both success and error pathsThis follows React Query best practices for mutation side effects.
269-283: Deletion flow correctly handles success path.The
toggle()call is now inside the try block afternavigateBack(), ensuring the modal only closes on successful deletion. This addresses the earlier concern about unconditional dialog closure.Note: The catch block logs the error but doesn't close the dialog, allowing users to retry—this is intentional UX behavior.
318-318: Good consolidated loading state.Combining
loading,deleteMutation.isPending, andtestMutation.isPendinginto a singleisLoadingvariable provides a unified loading indicator for the table, improving UX by showing loading state during any async operation.
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx:
- Line 110: Remove the unused local state testRunning declared via useState in
the LdapListPage component and delete all calls to its updater setTestRunning
(the calls currently around the LDAP test flow); rely on testMutation.isPending
for loading state instead and ensure no other code reads testRunning or expects
it to exist.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-10T14:18:58.310Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:107-109
Timestamp: 2025-11-10T14:18:58.310Z
Learning: In the Cedarling configuration page (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), console.error calls should be kept in catch blocks when there are multiple operations in the try block, as they help print the whole error object with details for debugging unexpected errors.
Applied to files:
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
📚 Learning: 2025-12-05T18:24:41.713Z
Learnt from: faisalsiddique4400
Repo: GluuFederation/flex PR: 2498
File: admin-ui/plugins/saml/components/SamlConfigurationForm.tsx:30-30
Timestamp: 2025-12-05T18:24:41.713Z
Learning: In the GluuFederation/flex admin-ui codebase, `SetTitle` (imported from 'Utils/SetTitle') is a custom React hook that should be called at the component's top level, not inside `useEffect` or other hooks, because it internally manages side effects for setting page titles.
Applied to files:
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
🧬 Code graph analysis (1)
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx (7)
admin-ui/plugins/services/Components/Configuration/atoms/index.ts (1)
currentLdapItemAtom(4-4)admin-ui/plugins/services/Components/Configuration/hooks/index.ts (1)
useLdapAudit(31-105)admin-ui/plugins/services/Components/Configuration/types.ts (1)
isPersistenceInfo(180-186)admin-ui/app/utils/pagingUtils.ts (1)
getPagingSize(1-24)admin-ui/app/helpers/navigation.ts (1)
ROUTES(204-204)admin-ui/app/customColors.ts (1)
customColors(4-25)admin-ui/app/components/index.tsx (1)
CardBody(44-44)
🔇 Additional comments (7)
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx (7)
37-55: Well-structured type definitions.The
AlertStateinterface now correctly usesundefinedinstead of an empty string for the null severity state, and theActionObject/ActionTypedefinitions properly type the Material-Table actions with support for both static and row-dependent actions.
80-82: Type guard properly addresses previous type safety concerns.Using
isPersistenceInfo(a proper type guard that checks forpersistenceTypeas a string property) eliminates the type assertion issue flagged in previous reviews. The conditional narrowing is now type-safe and explicit.
126-157: Mutations properly configured with React Query.Both the delete and test mutations correctly handle success/error states with toast notifications and query invalidation. The pattern follows React Query best practices.
168-176: Previous temporal dead zone issue resolved.The
togglecallback is now correctly declared beforehandleLdapDelete, eliminating the "use before declaration" error from previous reviews. The dependency array properly includestoggle.
269-283: Deletion handler correctly sequenced.The
onDeletionConfirmedcallback now correctly placestoggle()andnavigateBack()inside thetryblock, ensuring the modal closes and navigation occurs only on successful deletion. Theconsole.errorin the catch block is appropriate here because multiple async operations are performed in the try block.Based on learnings, console.error calls should be kept when there are multiple operations in the try block for debugging unexpected errors.
294-316: Table configuration and memoized components properly implemented.The table options correctly apply theme colors and uppercase transformation with proper TypeScript const assertions. Both
PaperContainerandDetailPanelare appropriately memoized with their dependencies.
320-366: Render logic is robust and well-typed.The conditional rendering based on
persistenceType, defensiveArray.isArraycheck, combined loading states, and properly typed column definitions create a reliable UI. TheSetTitlehook is correctly called at the component's top level (line 124), as per learnings.
admin-ui/plugins/services/Components/Configuration/LdapListPage.tsx
Outdated
Show resolved
Hide resolved
|
4 participants
#2563
Closes #2563
Summary by CodeRabbit
New Features
Improvements
Localization
✏️ Tip: You can customize this high-level summary in your review settings.