Improve unit test coverage of Applications -> Token Settings#1950
Open
brionmario wants to merge 1 commit intoasgardeo:mainfrom
Open
Improve unit test coverage of Applications -> Token Settings#1950brionmario wants to merge 1 commit intoasgardeo:mainfrom
Applications -> Token Settings#1950brionmario wants to merge 1 commit intoasgardeo:mainfrom
Conversation
|
Warning Rate limit exceeded
⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds and updates tests for OAuth2/PKCE, scope management, and JWT preview; preserves existing token subfields when updating OAuth token config; adds token-validation tab auto-switching with inline error indicators; and renames one i18n key. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
brionmario
added
the
skip-changelog
![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
🧹 Nitpick comments (1)
frontend/apps/thunder-console/src/features/applications/components/edit-application/advanced-settings/__tests__/EditAdvancedSettings.test.tsx (1)
227-228: Use fully shaped OAuth2 fixtures instead of force-casts to strengthen type-safety.The test fixtures at lines 227–228 and 253–254 cast partial objects to
OAuth2Config, omitting the requiredresponseTypesfield. TypeScript's interface definition shows bothgrantTypesandresponseTypesare required (not optional). This force-casting weakens type-safety and bypasses contract validation that could catch real regressions.Add the missing
responseTypesfield to match the interface contract:
- For
authorization_codegrant type, useresponseTypes: ['code']- For
implicitgrant type, useresponseTypes: ['token']Proposed fix
- inboundAuthConfig: [ - {type: 'oauth2', config: {grantTypes: ['authorization_code']} as OAuth2Config} as InboundAuthConfig, - ], + inboundAuthConfig: [ + { + type: 'oauth2', + config: { + grantTypes: ['authorization_code'], + responseTypes: ['code'], + }, + } as InboundAuthConfig, + ], @@ - const editedInbound: InboundAuthConfig[] = [ - {type: 'oauth2', config: {grantTypes: ['implicit']} as OAuth2Config} as InboundAuthConfig, - ]; + const editedInbound: InboundAuthConfig[] = [ + { + type: 'oauth2', + config: { + grantTypes: ['implicit'], + responseTypes: ['token'], + }, + } as InboundAuthConfig, + ];Also applies to: 253–254
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/apps/thunder-console/src/features/applications/components/edit-application/advanced-settings/__tests__/EditAdvancedSettings.test.tsx` around lines 227 - 228, The OAuth2 test fixtures in EditAdvancedSettings.test.tsx are being force-cast to OAuth2Config while omitting required fields; update the fixtures that set {type: 'oauth2', config: {grantTypes: ['authorization_code']} as OAuth2Config} and the similar implicit fixture to include the required responseTypes property (use responseTypes: ['code'] for authorization_code and responseTypes: ['token'] for implicit) so the objects fully conform to the OAuth2Config interface instead of relying on force-casts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/__tests__/JwtPreview.test.tsx`:
- Around line 105-135: The tests currently only check for no-throws and DOM
presence but must assert side effects: spy/mock the editor instance used by
JwtPreview to verify that editor.deltaDecorations is called on mount and again
on rerender when defaultClaims changes (reference JwtPreview and
deltaDecorations), and ensure the content change listener's disposable.dispose
is invoked on unmount (reference the content listener/disposable and the unmount
helper). Update the tests to inject or mock the Monaco editor instance, attach
spies to deltaDecorations and the disposable.dispose method, then assert those
spies were called at the appropriate stages (initial render, rerender with new
defaultClaims, and unmount).
In
`@frontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/__tests__/ScopeMapper.test.tsx`:
- Around line 160-192: The keyboard-selection assertions in the ScopeMapper
tests are too broad because they only check presence of 'given_name' which may
already appear in the Available Attributes panel; update the two tests
("pressing Enter on a scope in the left panel selects it" and "pressing Space on
a scope in the left panel selects it") to assert selection-specific UI: after
simulating keyDown on the profile button (use screen.getByRole('button', {name:
/profile/i}) as already used), verify that 'given_name' appears specifically
inside the Mapped/Right panel (or that 'email' is no longer shown in the Mapped
panel), e.g., query the mapped panel container (the element rendered by
ScopeMapper that holds mapped claims) and assert within that container rather
than using global getByText; this ensures the key press actually moved the
mapping.
In `@frontend/packages/thunder-i18n/src/locales/en-US.ts`:
- Around line 1271-1273: The locale file is missing the translation key
applications:edit.token.validation_error used by TokenValidationSection.tsx for
the tab error indicator aria-label; add an entry 'edit.token.validation_error':
'<appropriate English message>' to the en-US locale object (near the other
edit.token.* keys such as 'edit.token.inherit_from_id_token' and
'edit.token.user_info.inherit_hint') so TokenValidationSection.tsx can read a
proper human-readable string for the aria-label.
---
Nitpick comments:
In
`@frontend/apps/thunder-console/src/features/applications/components/edit-application/advanced-settings/__tests__/EditAdvancedSettings.test.tsx`:
- Around line 227-228: The OAuth2 test fixtures in EditAdvancedSettings.test.tsx
are being force-cast to OAuth2Config while omitting required fields; update the
fixtures that set {type: 'oauth2', config: {grantTypes: ['authorization_code']}
as OAuth2Config} and the similar implicit fixture to include the required
responseTypes property (use responseTypes: ['code'] for authorization_code and
responseTypes: ['token'] for implicit) so the objects fully conform to the
OAuth2Config interface instead of relying on force-casts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d02c0e23-185d-4bf5-8c72-53b794aa6704
📒 Files selected for processing (8)
frontend/apps/thunder-console/src/features/applications/components/edit-application/advanced-settings/__tests__/EditAdvancedSettings.test.tsxfrontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/EditTokenSettings.tsxfrontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/TokenValidationSection.tsxfrontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/__tests__/JwtPreview.test.tsxfrontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/__tests__/ScopeMapper.test.tsxfrontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/__tests__/ScopeSection.test.tsxfrontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/__tests__/ScopeSelector.test.tsxfrontend/packages/thunder-i18n/src/locales/en-US.ts
...atures/applications/components/edit-application/token-settings/__tests__/JwtPreview.test.tsx
Outdated
Show resolved
Hide resolved
...tures/applications/components/edit-application/token-settings/__tests__/ScopeMapper.test.tsx
Show resolved
Hide resolved
brionmario
force-pushed
the
feat-homepage
branch
3 times, most recently
from
ba31172 to
1fe5107
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/__tests__/ScopeSection.test.tsx (1)
154-157: Assert mock invocation before indexingmock.calls.Line 154 directly accesses the first call payload; adding an invocation assertion first gives clearer failure output when callback wiring breaks.
Suggested tweak
- const updatedClaims = onScopeClaimsChange.mock.calls[0][0] as ScopeClaims; + expect(onScopeClaimsChange).toHaveBeenCalledTimes(1); + const updatedClaims = onScopeClaimsChange.mock.calls[0][0] as ScopeClaims;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/__tests__/ScopeSection.test.tsx` around lines 154 - 157, Add an assertion that the mock callback was invoked before indexing into its calls: verify onScopeClaimsChange was called (e.g., expect(onScopeClaimsChange).toHaveBeenCalled()) immediately before retrieving updatedClaims from onScopeClaimsChange.mock.calls[0][0] so tests fail with a clear message when the handler wasn't called.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/TokenValidationSection.tsx`:
- Around line 129-156: The Tab labels currently put aria-label on the decorative
dot Box which won't reliably expose the error to screen readers; instead, when
hasAccessTokenError or hasIdTokenError is true, include the translated error
text from t('applications:edit.token.validation_error', 'Validation error') in
the Tab's accessible name (e.g., append it to the span that contains the tab
text or set the Tab's aria-label to "<tab text> - Validation error") and mark
the dot Box as decorative by removing its aria-label and adding
aria-hidden="true" or role="presentation" so it doesn't duplicate or steal
accessibility semantics.
---
Nitpick comments:
In
`@frontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/__tests__/ScopeSection.test.tsx`:
- Around line 154-157: Add an assertion that the mock callback was invoked
before indexing into its calls: verify onScopeClaimsChange was called (e.g.,
expect(onScopeClaimsChange).toHaveBeenCalled()) immediately before retrieving
updatedClaims from onScopeClaimsChange.mock.calls[0][0] so tests fail with a
clear message when the handler wasn't called.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fd3af4ba-f17b-47c5-8379-8217598b0414
📒 Files selected for processing (8)
frontend/apps/thunder-console/src/features/applications/components/edit-application/advanced-settings/__tests__/EditAdvancedSettings.test.tsxfrontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/EditTokenSettings.tsxfrontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/TokenValidationSection.tsxfrontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/__tests__/JwtPreview.test.tsxfrontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/__tests__/ScopeMapper.test.tsxfrontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/__tests__/ScopeSection.test.tsxfrontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/__tests__/ScopeSelector.test.tsxfrontend/packages/thunder-i18n/src/locales/en-US.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/tests/ScopeMapper.test.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/EditTokenSettings.tsx
- frontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/tests/JwtPreview.test.tsx
- frontend/apps/thunder-console/src/features/applications/components/edit-application/token-settings/tests/ScopeSelector.test.tsx
- frontend/apps/thunder-console/src/features/applications/components/edit-application/advanced-settings/tests/EditAdvancedSettings.test.tsx
.../features/applications/components/edit-application/token-settings/TokenValidationSection.tsx
Show resolved
Hide resolved
brionmario
force-pushed
the
feat-homepage
branch
from
1fe5107 to
814d4b5
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1950 +/- ##
==========================================
+ Coverage 90.02% 90.15% +0.12%
==========================================
Files 879 879
Lines 56803 56826 +23
==========================================
+ Hits 51139 51231 +92
+ Misses 4147 4079 -68
+ Partials 1517 1516 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rajithacharith
approved these changes
Mar 26, 2026
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
DonOmalVindula
approved these changes
Mar 26, 2026
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This pull request enhances the test coverage and improves user experience for the OAuth2 application editing and token settings features. The main changes include new and expanded test cases for advanced settings, scope mapping, and JWT preview components, as well as UI improvements for error visibility and accessibility in token validation and scope mapping.
Test Coverage Improvements:
ScopeSectioncomponent, verifying correct rendering, prop passing, and callback behavior when scopes are added or removed, including proper cleanup of scope claims.EditAdvancedSettingsto cover OAuth2 config change handling and correct merging and prioritization of inbound auth configs.JwtPreviewtests to cover Monaco editor integration, decoration application, prop changes, and cleanup on unmount. [1] [2]ScopeMapper, ensuring scopes can be selected via Enter and Space keys.User Experience and Accessibility Enhancements:
TokenValidationSectionto automatically switch tabs to the token type with validation errors and visually indicate errors on tabs with a red dot, increasing error discoverability and accessibility. [1] [2] [3]slotPropsfor minimum value enforcement, aligning with UI library best practices.Bug Fixes:
Test Utility Updates:
JwtPreview.Minor Updates:
fireEvent,InboundAuthConfig) in test files to support new test cases. [1] [2] [3]These changes collectively strengthen the reliability, usability, and accessibility of the application editing experience for OAuth2 applications.
Approach
N/A
Related Issues
Related PRs
Token&Advancedsections #1942Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
Bug Fixes
New Features
Tests
Localization