fix(admin-ui): Use typescript generated client for Auth Server Keys pages#2452
fix(admin-ui): Use typescript generated client for Auth Server Keys pages#2452
Conversation
|
Warning Rate limit exceeded@moabu has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 34 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 (4)
📝 WalkthroughWalkthroughRefactors Auth Server Keys pages from Redux-based sagas/slices/APIs to TypeScript-generated client usage via new hooks and typed components; removes legacy Redux wiring and tests, adds Jest module aliases, adds JWK types/constants/hooks, and replaces old JwksPage with a JwkListPage/JwkItem implementation and tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as KeysPage / JwkListPage
participant Hook as useJwkApi
participant Client as JansConfigApi (generated)
participant OldRedux as (was) jwksSaga/store
rect rgb(220,240,255)
UI->>Hook: call useJwkApi()
Hook->>Client: useGetConfigJwks / generated client call
Client-->>Hook: jwks data / error
Hook-->>UI: { jwks, isLoading, error, refetch }
end
rect rgb(255,230,220)
note right of OldRedux: Previously\nUI -> dispatch getJwks()\nSaga -> call JwksApi -> dispatch response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 3
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/Configuration/Keys/Jwks/JwkItem.tsx (1)
114-122: CRITICAL: x5c array is not being rendered correctly.The
x5cfield is an array of strings (as defined in the mock data and JSONWebKey type), but line 120 usesdefaultValue={item.x5c ?? ''}. When an array is used as a string value, JavaScript will calltoString()on it, resulting in a comma-separated string rather than a properly formatted certificate chain.Additionally, the test at KeysPage.test.tsx line 47 expects only the first element:
firstKey?.x5c?.[0], which contradicts this implementation.Apply this diff to handle the x5c array properly:
<Col sm={11}> <Input id="x5c" type="textarea" data-testid="x5c" name="x5c" readOnly - defaultValue={item.x5c ?? ''} + defaultValue={Array.isArray(item.x5c) ? item.x5c[0] ?? '' : ''} /> </Col>Or, if you want to display all certificates in the chain:
- defaultValue={item.x5c ?? ''} + defaultValue={Array.isArray(item.x5c) ? item.x5c.join('\n\n') : ''}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (21)
admin-ui/jest.config.js(1 hunks)admin-ui/plugins/auth-server/__tests__/api/Jwks.test.js(0 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsx(12 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkListPage.test.tsx(1 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkListPage.tsx(1 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwksPage.js(0 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwksPage.test.js(0 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/KeysPage.test.tsx(1 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/KeysPage.tsx(3 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/__fixtures__/jwkTestData.ts(2 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/constants.ts(1 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/hooks/index.ts(1 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/hooks/useJwkActions.ts(1 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/hooks/useJwkApi.ts(1 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/types/index.ts(1 hunks)admin-ui/plugins/auth-server/components/Configuration/Keys/types/jwkTypes.ts(1 hunks)admin-ui/plugins/auth-server/plugin-metadata.js(0 hunks)admin-ui/plugins/auth-server/redux/api/JwksApi.js(0 hunks)admin-ui/plugins/auth-server/redux/audit/Resources.js(1 hunks)admin-ui/plugins/auth-server/redux/features/jwksSlice.js(0 hunks)admin-ui/plugins/auth-server/redux/sagas/JwksSaga.js(0 hunks)
💤 Files with no reviewable changes (7)
- admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwksPage.test.js
- admin-ui/plugins/auth-server/redux/api/JwksApi.js
- admin-ui/plugins/auth-server/plugin-metadata.js
- admin-ui/plugins/auth-server/redux/features/jwksSlice.js
- admin-ui/plugins/auth-server/tests/api/Jwks.test.js
- admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwksPage.js
- admin-ui/plugins/auth-server/redux/sagas/JwksSaga.js
🧰 Additional context used
🧬 Code graph analysis (6)
admin-ui/plugins/auth-server/components/Configuration/Keys/hooks/useJwkActions.ts (1)
admin-ui/plugins/auth-server/components/Configuration/Keys/constants.ts (1)
ROUTES(3-5)
admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsx (2)
admin-ui/plugins/auth-server/components/Configuration/Keys/types/jwkTypes.ts (1)
JwkItemProps(7-10)admin-ui/plugins/auth-server/components/Configuration/Keys/constants.ts (1)
DATE_FORMAT(1-1)
admin-ui/plugins/auth-server/components/Configuration/Keys/hooks/useJwkApi.ts (1)
admin-ui/plugins/auth-server/components/Configuration/Keys/types/jwkTypes.ts (1)
UseJwkApiResult(13-18)
admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkListPage.tsx (1)
admin-ui/plugins/auth-server/components/Configuration/Keys/hooks/useJwkApi.ts (1)
useJwkApi(5-22)
admin-ui/plugins/auth-server/components/Configuration/Keys/KeysPage.test.tsx (3)
admin-ui/plugins/auth-server/components/Configuration/Keys/__fixtures__/jwkTestData.ts (3)
mockJwksConfig(3-20)mockJwksConfigWithZeroExp(22-34)mockEmptyJwksConfig(36-38)admin-ui/plugins/auth-server/components/Configuration/Keys/constants.ts (1)
DATE_FORMAT(1-1)admin-ui/plugins/auth-server/components/Configuration/Keys/hooks/useJwkApi.ts (1)
useJwkApi(5-22)
admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkListPage.test.tsx (2)
admin-ui/plugins/auth-server/components/Configuration/Keys/__fixtures__/jwkTestData.ts (1)
mockJwksConfig(3-20)admin-ui/plugins/auth-server/components/Configuration/Keys/constants.ts (1)
DATE_FORMAT(1-1)
🔇 Additional comments (19)
admin-ui/plugins/auth-server/redux/audit/Resources.js (1)
8-8: LGTM!The new
JWKconstant follows the established pattern for audit resource identifiers and integrates cleanly with the existing constants.admin-ui/plugins/auth-server/components/Configuration/Keys/types/index.ts (1)
1-1: LGTM!This barrel file follows TypeScript best practices for organizing and consolidating type exports.
admin-ui/plugins/auth-server/components/Configuration/Keys/hooks/index.ts (1)
1-2: LGTM!This barrel file follows React and TypeScript best practices for consolidating hook exports.
admin-ui/plugins/auth-server/components/Configuration/Keys/constants.ts (1)
1-5: LGTM!The constants are properly defined with
as constassertions, ensuring type safety and immutability. The date format and route paths are appropriate.admin-ui/plugins/auth-server/components/Configuration/Keys/hooks/useJwkApi.ts (1)
5-22: LGTM!The hook implementation follows React best practices:
- Properly uses the generated API client
- Includes error logging for debugging
- Normalizes errors to a consistent
Errortype- Returns a well-typed interface matching
UseJwkApiResultadmin-ui/plugins/auth-server/components/Configuration/Keys/hooks/useJwkActions.ts (1)
5-15: LGTM!The hook correctly uses
useCallbackto memoize the navigation function, with the appropriate dependency array. The implementation is clean and focused.admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkListPage.test.tsx (2)
12-22: LGTM!The mocking strategy correctly isolates the component under test by providing controlled hook return values. The mock setup is comprehensive and matches the expected hook interfaces.
36-51: LGTM!The test comprehensively validates the component's rendering:
- Verifies title presence
- Checks all key fields are populated from mock data
- Correctly handles conditional
expfield rendering with proper date formattingadmin-ui/jest.config.js (3)
16-16: LGTM!The
@/alias is a common pattern for absolute imports and properly maps to theapp/directory.
19-19: LGTM!The lowercase
contextalias complements the existing uppercaseContextalias (line 18). Since JavaScript module resolution is case-sensitive, these do not conflict.
26-26: LGTM!The
JansConfigApialias correctly maps to the generated TypeScript client file, enabling tests to resolve the new imports.admin-ui/plugins/auth-server/components/Configuration/Keys/KeysPage.tsx (1)
1-30: LGTM! Clean refactoring to use the new JwkListPage component.The changes successfully replace the old component with the new typed implementation, add proper TypeScript annotations, and include a displayName for better debugging. The refactoring aligns with the PR objectives of moving away from Redux-based manual types to TypeScript-generated client usage.
admin-ui/plugins/auth-server/components/Configuration/Keys/__fixtures__/jwkTestData.ts (1)
1-38: LGTM! Well-structured test fixtures.The mock data properly covers multiple test scenarios (normal, zero expiration, and empty states) and correctly types
x5cas an array, which aligns with the WebKeysConfiguration type from the generated client. The fixtures provide good coverage for the test cases.admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkListPage.tsx (2)
27-41: LGTM! Proper error state handling.The error state rendering correctly displays localized error messages with a fallback, ensuring users receive meaningful feedback when API calls fail.
43-57: LGTM! Well-structured loading and empty states.The component properly handles loading state with GluuLoader and displays an informative alert when no JWKs are found. The memoized jwkItems list ensures efficient rendering.
admin-ui/plugins/auth-server/components/Configuration/Keys/types/jwkTypes.ts (1)
1-18: LGTM! Clean type definitions.The file properly re-exports generated types from JansConfigApi and defines clear interfaces for component props and hook results. The type-only imports ensure optimal tree-shaking.
admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkItem.tsx (3)
36-81: LGTM! Proper use of readOnly and nullish coalescing.The changes from
disabledtoreadOnlyare semantically correct for display-only fields, and the nullish coalescing operator (??) properly handles undefined values with empty string fallbacks. This improves both the user experience and code clarity.Also applies to: 90-107, 119-120
12-12: LGTM! Proper memoization and displayName.The component is correctly memoized using
React.memowith explicit typing, and the displayName is set for better debugging in React DevTools.Also applies to: 173-173
10-10: The double type assertion for Accordion is correct and necessary.The Accordion component uses a compound component pattern where sub-components (Header, Body, Indicator) are attached as static properties at runtime in
index.ts. TypeScript cannot infer these runtime-attached properties, so the double assertionas unknown as AccordionComponentis the appropriate pattern to align the function's type with the interface that includes these static members. This is a standard TypeScript pattern for compound components and does not indicate a type incompatibility, version mismatch, or incorrect type definitions.
admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkListPage.tsx
Outdated
Show resolved
Hide resolved
| }) | ||
| expect(screen.getByText(/JSON Web Keys/)).toBeInTheDocument() | ||
| const firstKey = mockJwksConfig.keys?.[0] | ||
| expect(screen.getByTestId('x5c')).toHaveValue(firstKey?.x5c?.[0] ?? '') |
There was a problem hiding this comment.
🧩 Analysis chain
Verify x5c field handling - test expectation may not match implementation.
The test expects x5c to display only the first element of the array (firstKey?.x5c?.[0]), but the implementation in JwkItem.tsx line 120 uses defaultValue={item.x5c ?? ''}. Since x5c is typed as an array in the mock data (lines 13-15 of jwkTestData.ts), this would stringify the entire array rather than showing just the first element.
Run the following script to verify how x5c is handled in the JSONWebKey type and check for other usages:
🏁 Script executed:
#!/bin/bash
# Description: Verify x5c type definition and usage patterns
# Check JSONWebKey type definition for x5c field
ast-grep --pattern $'interface JSONWebKey {
$$$
x5c$_
$$$
}'
# Search for x5c usage patterns in the codebase
rg -nP --type=ts --type=tsx -C3 'x5c' -g '!**/node_modules/**' -g '!**/dist/**'Length of output: 27
🏁 Script executed:
#!/bin/bash
# Description: Verify x5c type definition and usage patterns - fixed
# Search for JSONWebKey type definition
rg -nP --type=ts -C5 'interface JSONWebKey|type JSONWebKey' -g '!**/node_modules/**' -g '!**/dist/**'
# Search for x5c usage patterns more broadly
rg -nP -C3 'x5c' -g '!**/node_modules/**' -g '!**/dist/**' | head -100
# Check JwkItem.tsx specifically for x5c handling around line 120
fd -e tsx -e ts 'JwkItem' -x cat {} \; 2>/dev/null | grep -n -A10 -B10 'x5c' || echo "File not found or pattern not matched"
# Check the test file around line 47
fd -e test.tsx 'KeysPage' -x cat {} \; 2>/dev/null | sed -n '40,60p' || echo "Test file not found"Length of output: 299173
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== search: getByTestId('x5c') in admin-ui ==="
rg -n -C3 --hidden --glob '!**/node_modules/**' "getByTestId\\(.{0,20}['\"]x5c['\"]" admin-ui || true
echo
echo "=== search: defaultValue={item.x5c in admin-ui ==="
rg -n -C3 --hidden --glob '!**/node_modules/**' "defaultValue\\s*=\\s*\\{\\s*item\\.x5c" admin-ui || true
echo
echo "=== search: x5c occurrences in admin-ui (context) ==="
rg -n -C3 --hidden --glob '!**/node_modules/**' "\\bx5c\\b" admin-ui || true
echo
echo "=== search: JSONWebKey type definitions in repo ==="
rg -n -C3 --hidden --glob '!**/node_modules/**' "interface\\s+JSONWebKey|type\\s+JSONWebKey|\\bJSONWebKey\\b" || trueLength of output: 26702
Align x5c rendering with tests — component renders entire array but tests expect the first element.
JwkItem.tsx uses defaultValue={item.x5c ?? ''} (renders the full array) while KeysPage.test.tsx (line 47) and JwkListPage.test.tsx (line 43) assert screen.getByTestId('x5c') equals firstKey?.x5c?.[0]; either change the component to render item.x5c?.[0] (or explicitly join/format the array) or update the tests to match the intended UI output.
🤖 Prompt for AI Agents
In admin-ui/plugins/auth-server/components/Configuration/Keys/KeysPage.test.tsx
around line 47 the test asserts that the x5c input contains only the first
element of the JWK array, but the component JwkItem.tsx sets
defaultValue={item.x5c ?? ''} which renders the entire array; to fix, make the
rendering and tests consistent: either update JwkItem.tsx to render
item.x5c?.[0] (or join/format the array to a single string) so the input shows
the first element, or change this test (and JwkListPage.test.tsx) to expect the
full array representation produced by the component (e.g., join with commas or
JSON.stringify) — pick one approach and update the component or tests
accordingly so the rendered value matches the asserted value.
admin-ui/plugins/auth-server/components/Configuration/Keys/KeysPage.test.tsx
Show resolved
Hide resolved
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
|
Docstrings generation was requested by @moabu. * #2452 (comment) The following files were modified: * `admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkListPage.tsx` * `admin-ui/plugins/auth-server/components/Configuration/Keys/KeysPage.tsx` * `admin-ui/plugins/auth-server/components/Configuration/Keys/hooks/useJwkActions.ts`
Docstrings generation was requested by @moabu. * #2452 (comment) The following files were modified: * `admin-ui/plugins/auth-server/components/Configuration/Keys/Jwks/JwkListPage.tsx` * `admin-ui/plugins/auth-server/components/Configuration/Keys/KeysPage.tsx` * `admin-ui/plugins/auth-server/components/Configuration/Keys/hooks/useJwkActions.ts` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
4 participants
#2449
Closes #2449
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores