fix(admin-ui): resolve navigation hang when redirecting to SSA page (#2672)#2675
Merged
fix(admin-ui): resolve navigation hang when redirecting to SSA page (#2672)#2675
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Cursor IDE ignore entries; tightens license/error handling in LicenseSaga (including backend-status mapping and explicit missing-license flow); restructures ApiKeyRedirect loader conditional; introduces BackendApi types and updates usages; adds one-time dispatch guard in AppAuthProvider; and adjusts SSA upload dropzone styling. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Admin UI (ApiKeyRedirect)
participant Saga as LicenseSaga
participant API as Backend API
participant Store as Redux Store
UI->>Saga: trigger license/config check
Saga->>API: fetch API access token / license
API-->>Saga: success or ApiErrorLike
alt success with valid license
Saga->>Store: dispatch license-present/valid responses
else no valid license or error
Saga->>Store: dispatch retrieveLicenseKeyResponse (missing)
Saga->>Store: dispatch checkLicensePresentResponse(false)
Saga->>Store: dispatch generateTrialLicenseResponse(null)
Saga->>Store: dispatch setBackendStatus (if ApiErrorLike)
end
UI->>Store: read license/config and backendStatus
UI-->>UI: evaluate showRedirectingLoader and render UploadSSA / ApiKey / loader
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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. ✨ 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 |
mo-auto
added
comp-admin-ui
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin-ui/app/redux/sagas/LicenseSaga.ts`:
- Around line 123-125: The no-key branch in LicenseSaga dispatches
retrieveLicenseKeyResponse and checkLicensePresentResponse but does not clear
generateTrialLicenseResponse, leaving stale trial-license state; update the else
branch in LicenseSaga to also dispatch generateTrialLicenseResponse with a
cleared/failed payload (e.g., isTrialGenerated: false and/or null payload) so
any previous successful trial generation is reset when no valid license key is
found, ensuring Redux state keys generateTrialLicenseResponse,
retrieveLicenseKeyResponse, and checkLicensePresentResponse are all consistent.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
admin-ui/.gitignoreadmin-ui/app/redux/sagas/LicenseSaga.tsadmin-ui/app/utils/ApiKeyRedirect.tsxadmin-ui/app/utils/styles/UploadSSA.style.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin-ui/app/redux/sagas/LicenseSaga.ts`:
- Around line 40-45: getBackendStatusFromError currently yields null for
errorMessage when response.data is missing (e.g. network/CORS failures); update
getBackendStatusFromError to preserve a fallback message by checking
err?.message or String(error) (or a hardcoded fallback like "Network error")
when err.response?.data?.responseMessage and err.response?.data?.message are
absent, and return that non-null string as errorMessage while keeping the rest
of the returned shape (active: false, statusCode detection) intact.
- Around line 50-55: After successfully fetching the token in the defaultToken
flow (the call to fetchApiTokenWithDefaultScopes and the subsequent yield
put(setApiDefaultToken(defaultToken))), reset the transient backend error state
by dispatching setBackendStatus with active: true, errorMessage: null,
statusCode: null so prior failures don't leave the UI in a "backend down" state;
add this yield put immediately after setApiDefaultToken(defaultToken) in
LicenseSaga and keep the existing catch handling that logs via devLogger.error
and uses getBackendStatusFromError.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin-ui/app/utils/AppAuthProvider.tsx`:
- Around line 219-221: The promise chain in AppAuthProvider.tsx that handles
fetchUserInformation returns early when value === -1, causing a silent failure;
update the .then handler (where value is checked and assigned to ujwt) to handle
the -1 sentinel by setting an error state or dispatching a user-visible
notification and ensuring auth state is updated (e.g., set isAuthenticated false
or call logout) so the component/UI doesn't stay stuck; modify the .then for
FetchUserInfoResult and related state setters or auth dispatchers in the
AppAuthProvider component to surface the error and short-circuit to a clear
fallback path instead of returning silently.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
admin-ui/app/redux/api/backend-api.tsadmin-ui/app/redux/api/types/BackendApi.tsadmin-ui/app/utils/AppAuthProvider.tsx
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
admin-ui/app/utils/ApiKeyRedirect.tsx (1)
75-81: 🧹 Nitpick | 🔵 TrivialConsider extracting nested ternary for readability.
The 4-level nested ternary is difficult to follow and maintain. Consider refactoring to a helper function or extracting the logic into a clearly named variable.
♻️ Suggested refactor using a helper function
+ const renderMainContent = () => { + if (isConfigValid === false) { + return backendStatus.active ? <UploadSSA /> : null + } + if (!isTimeout && isUnderThresholdLimit && shouldShowApiKey) { + return <ApiKey /> + } + return null + } + return ( <React.Fragment> <Container> - {isConfigValid === false && backendStatus.active ? ( - <UploadSSA /> - ) : isConfigValid === false ? null : !isTimeout && isUnderThresholdLimit ? ( - shouldShowApiKey ? ( - <ApiKey /> - ) : null - ) : null} + {renderMainContent()} {!backendStatus.active && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin-ui/app/utils/ApiKeyRedirect.tsx` around lines 75 - 81, The JSX contains a hard-to-read 4-level nested ternary rendering expression around isConfigValid, backendStatus.active, isTimeout, isUnderThresholdLimit and shouldShowApiKey that returns UploadSSA, ApiKey or null; extract this logic into a clearly named helper function or boolean variables (e.g., computeApiKeyRender() or const shouldRenderUploadSSA / const shouldRenderApiKey) and replace the nested ternary with a single, descriptive expression (or call to the helper) so that the rendering decisions for UploadSSA and ApiKey are easy to follow and test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@admin-ui/app/utils/ApiKeyRedirect.tsx`:
- Around line 75-81: The JSX contains a hard-to-read 4-level nested ternary
rendering expression around isConfigValid, backendStatus.active, isTimeout,
isUnderThresholdLimit and shouldShowApiKey that returns UploadSSA, ApiKey or
null; extract this logic into a clearly named helper function or boolean
variables (e.g., computeApiKeyRender() or const shouldRenderUploadSSA / const
shouldRenderApiKey) and replace the nested ternary with a single, descriptive
expression (or call to the helper) so that the rendering decisions for UploadSSA
and ApiKey are easy to follow and test.
duttarnab
previously approved these changes
Feb 26, 2026
|
duttarnab
approved these changes
Feb 26, 2026
moabu
approved these changes
Feb 26, 2026
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
4 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.
fix(admin-ui): resolve navigation hang when redirecting to SSA page (#2672)
Summary
The Admin UI was getting stuck while navigating to the SSA page, preventing proper application launch in nightly builds. The UI would hang during routing and the SSA page would not load.
Problem
Root Cause
Fix Details
Result
🔗 Ticket
Closes: #2672
Summary by CodeRabbit
Bug Fixes
Style
Chores
Chores