fix(admin-ui): Use typescript generated client on Audit pages #2509
Conversation
syntrydy
added
comp-admin-ui
📝 WalkthroughWalkthroughReplaces the JS/Redux audit implementation with a TypeScript hook-based AuditListPage: removes audit Redux slice, saga, and API wrapper; adds a TSX AuditListPage, type definitions, and new localization keys across en/es/fr/pt locales. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as AuditListPage (TSX)
participant Hook as useGetAuditData (TS hook / Generated client)
participant Server as Audit Data API
Note over UI,Hook: User enters filters / triggers search
UI->>Hook: request audit data (pattern,start,end,limit,offset,token)
Hook->>Server: GET /audit-data (with token)
alt 200 OK
Server-->>Hook: audit entries + totals
Hook-->>UI: parsed response
UI->>UI: render table, update pagination
else 401 / token expired
Server-->>Hook: 401
Hook->>Hook: refresh token (internal helper)
Hook->>Server: retry GET with refreshed token
Server-->>Hook: audit entries
Hook-->>UI: parsed response
else Error
Server-->>Hook: error
Hook-->>UI: error -> show error message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30–40 minutes
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2025-11-07T12:55:26.241ZApplied to files:
📚 Learning: 2025-11-26T12:25:34.923ZApplied to files:
🔇 Additional comments (4)
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 |
syntrydy
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
admin-ui/app/locales/en/translation.json(6 hunks)admin-ui/app/locales/es/translation.json(6 hunks)admin-ui/app/locales/fr/translation.json(3 hunks)admin-ui/app/locales/pt/translation.json(3 hunks)admin-ui/app/redux/reducers/index.ts(0 hunks)admin-ui/plugins/admin/components/Audit/AuditListPage.js(0 hunks)admin-ui/plugins/admin/components/Audit/AuditListPage.tsx(1 hunks)admin-ui/plugins/admin/components/Audit/types/index.ts(1 hunks)admin-ui/plugins/admin/plugin-metadata.ts(1 hunks)admin-ui/plugins/admin/redux/api/AuditApi.js(0 hunks)admin-ui/plugins/admin/redux/features/auditSlice.js(0 hunks)admin-ui/plugins/admin/redux/sagas/AuditSaga.js(0 hunks)
💤 Files with no reviewable changes (5)
- admin-ui/plugins/admin/components/Audit/AuditListPage.js
- admin-ui/plugins/admin/redux/api/AuditApi.js
- admin-ui/plugins/admin/redux/sagas/AuditSaga.js
- admin-ui/plugins/admin/redux/features/auditSlice.js
- admin-ui/app/redux/reducers/index.ts
🧰 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/admin/components/Audit/AuditListPage.tsx
🧬 Code graph analysis (1)
admin-ui/plugins/admin/components/Audit/AuditListPage.tsx (2)
admin-ui/app/customColors.ts (1)
customColors(4-25)admin-ui/plugins/admin/components/Audit/types/index.ts (2)
AuditSearchProps(11-24)AuditRow(3-9)
🔇 Additional comments (6)
admin-ui/plugins/admin/plugin-metadata.ts (2)
200-205: Route configuration correctly preserved.The AuditListPage route remains properly configured with appropriate permissions and resource keys. This is correct since the PR refactors the implementation from Redux to TypeScript hooks while preserving the feature.
207-216: LGTM! Clean removal of audit Redux infrastructure.The audit reducer and saga have been correctly removed from the plugin configuration. The remaining reducers and sagas for other features are properly configured. The removed Redux files (auditSlice.js and AuditSaga.js) are deleted from the codebase, and the new AuditListPage is properly using the JansConfigApi instead of Redux infrastructure.
admin-ui/app/locales/es/translation.json (1)
64-65: Translation keys appropriately added to support audit feature.All new entries follow the existing structure and are semantically correct for the audit pages' functional needs (pagination, date validation, filtering guidance, error handling). Spanish translations maintain proper grammar and preserve template placeholders for dynamic values.
Also applies to: 170-170, 180-180, 779-780, 801-803, 829-830, 834-834
admin-ui/app/locales/en/translation.json (2)
64-65: English translation keys are appropriate and consistent with Spanish equivalents.All new additions match the structure and intent of the Spanish translations. Messages use clear, idiomatic English suitable for the audit feature. Template placeholders are preserved correctly.
Also applies to: 170-170, 180-180, 779-780, 801-801, 803-803, 829-830, 834-834
1-1: Note: French (fr) and Portuguese (pt) translation files not provided.The enriched summary indicates that French and Portuguese locales also received similar additions for audit feature support. Consider verifying those files separately to ensure consistency across all four supported locales (en, es, fr, pt).
admin-ui/plugins/admin/components/Audit/AuditListPage.tsx (1)
1-124: Overall audit list implementation is clean and correctly wired to the TS client.The page composes filters, data fetching (
useGetAuditData), and server‑side pagination coherently:
- Query params are centralized in
queryParamsand always built viabuildQueryParams, avoiding ad‑hoc URL construction.- Date handling with Dayjs and the helper predicates (
hasBothDates,isStartAfterEnd,isValidDate,dateConverter) is clear and keeps the search-disable logic readable.- The
AuditSearchsubcomponent keeps the main page lean and encapsulates keyboard handling, page-size select, and refresh/search controls.- MaterialTable is used as a presentational shell over an already paginated dataset (
auditRows), with a customPaginationcomponent that drives server‑side pagination viaonPageChange/onRowsPerPageChange.No blocking correctness, performance, or concurrency issues stand out here.
Also applies to: 164-383, 397-595
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
admin-ui/plugins/admin/components/Audit/AuditListPage.tsx (1)
535-549: Excellent! Validation messages address previous review feedback.The validation messages for both
isStartAfterEndandhasOnlyOneDateare now properly displayed, addressing the concern raised in the previous review about the Search button being silently disabled. Users now receive clear feedback about why they cannot search.admin-ui/plugins/admin/components/Audit/types/index.ts (1)
1-20: LGTM! Clean and minimal type definitions.The type definitions are well-structured and all fields are actively used in
AuditListPage.tsx:
AuditRowfields (id, serial, log) are all consumed by the table componentAuditSearchPropsaccurately reflects the search component's interface- Proper use of Dayjs types for date handling
Note: The previous review's concern about unused
timestampandcontentfields does not apply to the current code, as these fields are not present in the interface.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
admin-ui/app/locales/en/translation.json(6 hunks)admin-ui/app/locales/es/translation.json(6 hunks)admin-ui/app/locales/fr/translation.json(3 hunks)admin-ui/app/locales/pt/translation.json(3 hunks)admin-ui/plugins/admin/components/Audit/AuditListPage.tsx(1 hunks)admin-ui/plugins/admin/components/Audit/types/index.ts(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/admin/components/Audit/AuditListPage.tsxadmin-ui/app/locales/fr/translation.jsonadmin-ui/plugins/admin/components/Audit/types/index.ts
📚 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/admin/components/Audit/AuditListPage.tsx
🔇 Additional comments (16)
admin-ui/app/locales/en/translation.json (3)
64-65: New action keys look good.The
add_custom_attributesandrefreshkeys are clearly defined and ready for UI consumption.
170-170: Field keys for pagination and audit UI are correctly placed.
log_entryandpage_sizesit appropriately in the fields section alongside related metadata keys.Also applies to: 180-180
779-781: Audit messages are now correctly placed inmessagessection.The migration from the problematic
titlessection (flagged in past review) to the correctmessagessection is confirmed. All audit-related keys (showing_entries,start_date_after_end,both_dates_required,adjust_filters,audit_description,error_loading_data,try_again_later,no_data) are now properly co-located where the UI component will look them up. Template variable syntax ({{start}},{{end}},{{total}}) is consistent and correct.Also applies to: 802-807
admin-ui/app/locales/fr/translation.json (2)
671-672: Field keys for audit UI properly translated.The
log_entryandpage_sizekeys are correctly added to the Frenchfieldssection with appropriate translations that align with the English versions.
800-807: Audit messages correctly repositioned and fully translated.This confirms that the past review issue—audit messages placed under
titlesinstead ofmessages—has been resolved. All eight audit-related keys are now in the correctmessagessection with complete French translations. Template variables are consistent with English ({{start}},{{end}},{{total}}), and the message copy is idiomatic French (e.g., "Affichage de {{start}} à {{end}} sur {{total}} entrées" accurately mirrors the pagination layout).admin-ui/app/locales/pt/translation.json (2)
794-802: Audit i18n keys correctly positioned in messages namespace.The namespace correction from the previous review has been applied: all audit-related i18n keys (
error_loading_data,try_again_later,no_data,adjust_filters,audit_description,showing_entries,start_date_after_end,both_dates_required) are now correctly placed in themessagessection, aligning with the audit UI component expectations (messages.*references).
171-172: Action and field i18n keys added appropriately.New keys—
refreshaction andlog_entry/page_sizefield labels—have been added with semantically correct Portuguese translations and are positioned in the appropriate sections.Also applies to: 665-667
admin-ui/app/locales/es/translation.json (2)
779-803: Verify audit key consistency: "no_data" vs "no_data_found" naming divergence.Seven of eight audit message keys are present in the Spanish
messagessection. However, the Portuguese file includes ano_datakey (line 797 in pt file), but the Spanish file appears to useno_data_found(line 861) instead. If the audit UI component referencesmessages.no_data, the Spanish locale will show a missing translation or fall back to the key name.Confirm whether:
- Spanish should also have a
no_datakey (aligned with Portuguese), or- The audit UI intentionally uses a different key name in different locales (inconsistent), or
- "no_data_found" is intentionally substituted for "no_data"
Also applies to: 830-831
64-65: Action and field i18n keys correctly added to Spanish locale.New keys (
refresh,log_entry,page_size) are present with appropriate Spanish translations and correct section placement, consistent with the Portuguese file structure.Also applies to: 170-170, 180-180
admin-ui/plugins/admin/components/Audit/AuditListPage.tsx (7)
1-114: LGTM! Clean imports and well-structured component definitions.The imports, constants, and LoadingSkeleton component are well-organized and appropriate for the functionality. The use of MUI components, dayjs for date handling, and Material Table is consistent with the rest of the codebase.
116-151: LGTM! Well-implemented empty and error states.The EmptyState and ErrorState components provide clear user feedback with appropriate icons and messages. The ErrorState's retry functionality is a good UX pattern.
153-271: LGTM! Well-designed search component with good UX.The AuditSearch component includes thoughtful features like Enter-key search, animated refresh button, and proper disabled states. The keyboard accessibility is a good addition.
273-336: LGTM! Sound validation and query construction logic.The state management and filter validation logic is well-implemented:
- Properly handles pattern-only searches
- Requires both dates for date-range filtering
- Validates start date before end date
- The
buildQueryParamsfunction correctly builds API parameters based on current filters
338-364: LGTM! Properly memoized event handlers.All event handlers are correctly implemented with appropriate useCallback dependencies. The logic properly resets pagination when changing page size and rebuilds query parameters as needed.
459-505: LGTM! Well-configured MaterialTable with proper pagination.The MaterialTable implementation is solid:
- Custom pagination component correctly wired to state
- PAGE_SIZE_OPTIONS properly used for rows-per-page dropdown
- Styling is consistent with theme colors
- Conditional row striping improves readability
508-570: LGTM! Complete and well-structured main render.The main component render includes:
- Clear page title and description
- Search component with proper props
- Validation feedback messages
- Data table with appropriate states
- Entry count display
The component provides a complete and user-friendly audit log viewing experience.
|
4 participants
#2508
Closes #2508
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.