-
Notifications
You must be signed in to change notification settings - Fork 22
fix(admin-ui): Use typescript generated client for Auth Server Config API pages #2455
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughReplaces Redux-based Config API wiring with TypeScript hook-based clients and prop-driven components: removes Redux slice, saga, and API wrapper; adds typed hooks, type re-exports, YAML ambient declarations, refactors form and JSON property builder to TypeScript, and updates plugin metadata. (31 words) Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ConfigApiPage as ConfigApiPage (New)
participant ApiConfigForm
participant Hooks as Custom Hooks
participant API as API Layer
User->>ConfigApiPage: Mount
ConfigApiPage->>Hooks: useGetConfigApiProperties()
Hooks->>API: GET /config/api
API-->>Hooks: configuration
Hooks-->>ConfigApiPage: data + isLoading
ConfigApiPage->>ApiConfigForm: render with configuration
User->>ApiConfigForm: submit(patches,message)
ApiConfigForm->>ConfigApiPage: onSubmit(patches,message)
ConfigApiPage->>Hooks: usePatchConfigApiProperties(patches)
Hooks->>API: PATCH /config/api
API-->>Hooks: success / error
alt success
Hooks-->>ConfigApiPage: success
ConfigApiPage->>Hooks: useConfigApiActions().logConfigApiUpdate(...)
Hooks->>API: POST /audit
ConfigApiPage-->>User: show success toast
else error
Hooks-->>ConfigApiPage: error
ConfigApiPage-->>User: show error card/toast
end
sequenceDiagram
participant OldFlow as Redux Flow (Removed)
participant NewFlow as Hooks Flow (New)
rect rgb(240,220,220)
Note over OldFlow: Dispatch -> Saga -> API -> Reducer -> Component select
OldFlow->>OldFlow: dispatch(getConfigApiConfiguration)
OldFlow->>OldFlow: saga handles fetch + audit
OldFlow->>OldFlow: dispatch(response) -> update slice
end
rect rgb(220,240,220)
Note over NewFlow: Hook-based direct data + local state
NewFlow->>NewFlow: useGetConfigApiProperties() -> returns data/isLoading
NewFlow->>NewFlow: onSubmit -> usePatchConfigApiProperties -> audit via useConfigApiActions
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 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/auth-server/components/Configuration/ConfigApiConfiguration/JsonPropertyBuilderConfigApi.tsx (1)
11-37: Schema-based type guards risk unsound narrowing; consider loosening the TS predicates.The helper functions
_isBoolean,_isString, andisStringArrayact as TS type guards but also treat the JSON schema as authoritative:function _isBoolean(item: unknown, schema?: JsonPropertyBuilderConfigApiProps['schema']): item is boolean { return typeof item === 'boolean' || schema?.type === 'boolean' }Because
schema?.type === 'boolean'doesn’t inspect the actual runtime value, TypeScript will happily treatpropValueas abooleaneven if it’s still a string like'true'. The same applies to strings and string arrays, which can lead to incorrect assumptions aboutpropValue’s shape inside the guarded branches.To keep the runtime behavior while avoiding misleading narrowing, you could:
- Keep the current logic but change the return type of these helpers from
item is ...toboolean, or- Split runtime type checks from schema checks, e.g. a true type guard for the value and a separate
isBooleanFieldBySchemafunction used purely for rendering decisions.Also,
_isNumbercurrently treatsbigintas anumberin its predicate; ifpropValuecan never be a bigint from this API, you might simplify it to only accepttypeof item === 'number'to keep the guard strictly accurate.admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx (1)
81-98: Typed access intoconfiguration/schemaworks; you can tighten key typingThe cast
configuration[propKey as keyof ApiAppConfiguration]and the narrowedschema[propKey]are reasonable, but you can make this safer and avoid the cast by typing the keys:- {Object.keys(configuration).map((propKey) => { + {(Object.keys(configuration) as (keyof ApiAppConfiguration)[]).map((propKey) => { if (generateLabel(propKey)) { return ( <JsonPropertyBuilderConfigApi key={propKey} propKey={propKey} - propValue={configuration[propKey as keyof ApiAppConfiguration]} + propValue={configuration[propKey]} lSize={6} handler={patchHandler} schema={ - schema[propKey] as { type?: string; items?: { type?: string; enum?: string[] } } + schema[propKey as string] as { + type?: string + items?: { type?: string; enum?: string[] } + } }This keeps TS aligned with the actual
ApiAppConfigurationkeys and minimizes unsafe casting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
admin-ui/app/types/yaml.d.ts(1 hunks)admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx(3 hunks)admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.js(0 hunks)admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx(1 hunks)admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/JsonPropertyBuilderConfigApi.tsx(9 hunks)admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/index.ts(1 hunks)admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts(1 hunks)admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/componentTypes.ts(1 hunks)admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/configApiTypes.ts(1 hunks)admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/index.ts(1 hunks)admin-ui/plugins/auth-server/plugin-metadata.js(0 hunks)admin-ui/plugins/auth-server/redux/api/ConfigurationApi.js(0 hunks)admin-ui/plugins/auth-server/redux/features/configApiSlice.js(0 hunks)admin-ui/plugins/auth-server/redux/sagas/configApiSaga.js(0 hunks)
💤 Files with no reviewable changes (5)
- admin-ui/plugins/auth-server/plugin-metadata.js
- admin-ui/plugins/auth-server/redux/api/ConfigurationApi.js
- admin-ui/plugins/auth-server/redux/sagas/configApiSaga.js
- admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.js
- admin-ui/plugins/auth-server/redux/features/configApiSlice.js
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Applied to files:
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts
📚 Learning: 2025-11-07T12:17:39.857Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:43-149
Timestamp: 2025-11-07T12:17:39.857Z
Learning: In the Cedarling configuration UI PR (#2378), the `configApiPolicyStoreUrl` field is intentionally out of scope. It relates to config API configuration and will be covered in a separate PR. The current PR focuses on the Admin UI policy store URL (`auiPolicyStoreUrl`).
Applied to files:
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx
🔇 Additional comments (8)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1)
9-38: Audit hook wiring looks correct; verify payload sensitivity and simplify ModifiedFields.The hook is cleanly wired: it derives auth context from Redux and exposes a focused
logConfigApiUpdatehelper which callers can wrap in their own error handling.Two points to consider:
- If
ApiAppConfigurationincludes any sensitive fields (e.g. client secrets, tokens), logging the full object aspayloadmight be more than you need. It may be safer to either:
- log only the minimal subset of fields required for auditability, or
- ensure
logAuditUserActioncentrally redacts sensitive keys before persistence.ModifiedFieldscan be expressed more idiomatically as a type alias:-export interface ModifiedFields { - [key: string]: unknown -} +export type ModifiedFields = Record<string, unknown>admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/index.ts (1)
1-1: Hooks barrel is concise and correct.Re-exporting
useConfigApiActionshere keeps the Config API hooks discoverable without adding complexity.admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/index.ts (1)
1-2: Type barrel is well-structured.Aggregating
configApiTypesandcomponentTypesbehind this index keeps downstream imports clean and localized to a single entry point.admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/configApiTypes.ts (1)
1-3: Type re-exports correctly encapsulate JansConfigApi.This file cleanly centralizes the external
ApiAppConfigurationandJsonPatchtypes, which makes it easier to adjust if the generated client ever changes its signatures.admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (1)
20-52: Config API page wiring looks solid; please verify mutation/query integration with the generated client.The page composes the generated hooks and the new audit hook cleanly:
- Initial load via
useGetConfigApiProperties.- Updates via
usePatchConfigApiProperties().mutateAsync({ data: patches }).- Non-fatal audit logging via
logConfigApiUpdate(updatedConfig, message, { requestBody: patches }).- Clear separation between initial-load errors (
error) and save-time errors (errorMessage+ toasts).Two things worth double‑checking against the generated JansConfigApi hooks:
- That
usePatchConfigApiPropertieseither:
- invalidates or updates the underlying
useGetConfigApiPropertiesquery soconfigurationstays in sync after a save, or- otherwise ensures the next render reflects the updated configuration.
- That the
JsonPatchtype imported from'./types'matches the expected request body shape forusePatchConfigApiProperties(i.e.,data: JsonPatch[]is exactly what the generated client expects).If both are true, this page should behave as intended without additional glue code.
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx (3)
6-11: Imports forAPI_CONFIG_WRITEand typed config/patch models look consistent
API_CONFIG_WRITEis correctly used both inauthorizeand permission checks, and theApiAppConfiguration/JsonPatchtype imports align with the new prop-driven API. No issues here.
37-47: UpdateduseEffectdependency onauthorizeis appropriateUsing
[authorize]in the dependency array avoids the stale-closure pattern while still effectively behaving like a “run on mount” effect for a stable hook API. The authorization error logging is fine for now.
57-63: Consider clearingpatches/operationsafter a successful submitDelegating to
onSubmit(patches, userMessage)is correct, butpatchesandoperationsare never cleared. If this form stays mounted after a successful update, any subsequent changes will send both the new and all previously applied patches again.To avoid reapplying stale operations, you can clear state after a successful submit:
const submitForm = useCallback( async (userMessage: string) => { toggle() - await onSubmit(patches, userMessage) + await onSubmit(patches, userMessage) + setPatches([]) + setOperations([]) }, - [toggle, onSubmit, patches], + [toggle, onSubmit, patches], )If the parent unmounts/remounts
ApiConfigFormafter submit, this is less critical, but it’s safer to handle cleanup locally.
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx
Show resolved
Hide resolved
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ApiConfigForm.tsx
Show resolved
Hide resolved
.../plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/componentTypes.ts
Outdated
Show resolved
Hide resolved
duttarnab
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
On point one: Current: "Config API Configuration" (uppercase 'C') Target: "Config API configuration" (lowercase 'c') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1)
admin-ui/app/utils/AuditLogger.ts (1)
logAuditUserAction(25-66)
🔇 Additional comments (3)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (3)
9-11: LGTM!The
ModifiedFieldsinterface provides appropriate flexibility for tracking varied field changes in audit logs.
13-18: LGTM!The Redux selector logic correctly extracts authentication context needed for audit logging.
36-39: LGTM!The return structure follows standard React hook patterns.
...ins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts
Outdated
Show resolved
Hide resolved
...ins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx(1 hunks)admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Applied to files:
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts
🧬 Code graph analysis (2)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (2)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1)
useConfigApiActions(11-36)admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/configApiTypes.ts (1)
JsonPatch(3-3)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1)
admin-ui/app/utils/AuditLogger.ts (1)
logAuditUserAction(25-66)
🔇 Additional comments (2)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (1)
49-71: Loading/error and render flow looks solidCombining query and mutation flags into a single
loadingprop forGluuLoader, short‑circuiting on initialerrorwith an inline card, and conditionally renderingApiConfigFormplus anerrorMessagealert gives a clear UX and keeps concerns separated. No issues here.admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1)
7-35: Audit logging hook implementation is correct and prior issues are resolved
useConfigApiActionsnow cleanly derivestoken,client_id, anduserinfofromauthReducerand wrapslogAuditUserActionwith the right metadata (UPDATE, resource"Config API configuration",performedOn: 'config-api', andmodifiedFieldswithout overridingpayload). The previous problems with an unusedconfigparameter and conflictingpayloadconstruction are gone, and theuseCallbackdependency list matches the captured values.
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Applied to files:
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.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/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx
🧬 Code graph analysis (1)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (3)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1)
useConfigApiActions(11-36)admin-ui/plugins/admin/redux/sagas/ApiPermissionSaga.js (1)
errorMessage(100-100)admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/configApiTypes.ts (1)
JsonPatch(3-3)
🔇 Additional comments (1)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (1)
13-23: Config page flow and audit handling look correct and address prior gating concernThe overall wiring (fetch via
useGetConfigApiProperties, PATCH viausePatchConfigApiProperties, then non-blockinglogConfigApiUpdatewith a warning toast on audit failure) is solid and fixes the earlier issue where audit logging depended on the response body. Keepingconsole.errorin both catch blocks is also good for debugging when multiple operations happen in the same try.Based on learnings
Also applies to: 24-44, 58-69
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-07T12:55:26.241Z
Learnt from: kdhttps
Repo: GluuFederation/flex PR: 2378
File: admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx:93-104
Timestamp: 2025-11-07T12:55:26.241Z
Learning: In the Cedarling configuration feature (admin-ui/plugins/admin/components/Cedarling/CedarlingConfigPage.tsx), the sync role to scopes mappings operation depends on the `auiPolicyStoreUrl`. The backend fetches content from this URL and syncs roles based on the response. Therefore, the audit log for the sync operation should include the `auiPolicyStoreUrl` in its payload to provide proper audit context.
Applied to files:
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.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/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx
🧬 Code graph analysis (1)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (2)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/hooks/useConfigApiActions.ts (1)
useConfigApiActions(11-36)admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/types/configApiTypes.ts (1)
JsonPatch(3-3)
🔇 Additional comments (3)
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx (3)
1-22: LGTM! Clean setup with proper TypeScript typing.The imports and component initialization follow React best practices. The hooks are properly initialized, types are imported, and the i18n setup is correct.
46-56: LGTM! Proper loading state combination and improved error handling.The loading state correctly combines both the mutation pending state and the initial query loading state. The error handling now properly extracts the error message using
instanceof Errorcheck, addressing the previous review concern aboutString(error)potentially rendering[object Object].
58-69: LGTM! Proper render flow with both transient and persistent error feedback.The component correctly:
- Wraps content in
GluuLoaderfor loading states- Conditionally renders the form only when configuration data is available
- Provides both immediate feedback via toast notifications and persistent error display via the alert div
The dual error display (toast + persistent alert) is a good UX pattern that provides immediate feedback while maintaining visibility of errors after the toast disappears.
admin-ui/plugins/auth-server/components/Configuration/ConfigApiConfiguration/ConfigApiPage.tsx
Outdated
Show resolved
Hide resolved
|
|




#2449
Closes #2453
Summary by CodeRabbit
New Features
Refactor
Other
✏️ Tip: You can customize this high-level summary in your review settings.