Fix Console app shell stuck on loading skeleton after clicking root org name in breadcrumb#10470
Fix Console app shell stuck on loading skeleton after clicking root org name in breadcrumb#10470SujanSanjula96 wants to merge 3 commits into
Conversation
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe breadcrumb switch handler now clears its loading flag in a finally block. The super-breadcrumb rendering path now uses a single branch and only attaches its click handler when the breadcrumb list has more than one item. ChangesOrganization switch breadcrumb behavior
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@features/admin.organizations.v1/__tests__/organization-switch-breadcrumb.test.tsx`:
- Line 128: The breadcrumb tests are using visible text lookup for the clickable
organization item, which should be replaced with stable `data-componentid`-based
selection. Update the affected assertions in
`organization-switch-breadcrumb.test.tsx` to query the breadcrumb DOM nodes via
the relevant `data-componentid` selectors instead of `screen.findByText`, and
apply the same change to the other referenced lookup spots in this test file so
the selectors match the repo’s testing convention.
- Line 143: The mock call assertions in organization-switch-breadcrumb.test.tsx
use unsafe any typing for
updateOrganizationSwitchRequestLoadingStateMock.mock.calls; replace those array
typings with a proper tuple/interface type derived from the mock signature, or
use unknown plus narrowing in the assertions. Update the repeated call-site
variables in the test to use the same strongly typed mock-call shape so the
checks remain type-safe without any.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 67f6cae2-a7eb-4d02-804b-f22933366217
📒 Files selected for processing (3)
.changeset/fix-console-org-switch-stuck-skeleton.mdfeatures/admin.organizations.v1/__tests__/organization-switch-breadcrumb.test.tsxfeatures/admin.organizations.v1/components/organization-switch/organization-switch-breadcrumb.tsx
| test("UTC-7610.1 - clears the org-switch loading flag after a successful root-org switch", async () => { | ||
| renderBreadcrumb(); | ||
|
|
||
| const rootOrgItem: HTMLElement = await screen.findByText("Root Organization"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use data-componentid selectors instead of text-based lookup.
These tests locate the clickable breadcrumb by visible text. For this repo’s test pattern, select DOM nodes via data-componentid to keep selectors stable and guideline-compliant.
As per coding guidelines, “Use data-componentid for DOM element selection in tests (not id or CSS classes)” and “Do not use data-testid.”
Also applies to: 158-158, 177-177
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@features/admin.organizations.v1/__tests__/organization-switch-breadcrumb.test.tsx`
at line 128, The breadcrumb tests are using visible text lookup for the
clickable organization item, which should be replaced with stable
`data-componentid`-based selection. Update the affected assertions in
`organization-switch-breadcrumb.test.tsx` to query the breadcrumb DOM nodes via
the relevant `data-componentid` selectors instead of `screen.findByText`, and
apply the same change to the other referenced lookup spots in this test file so
the selectors match the repo’s testing convention.
Sources: Coding guidelines, Learnings
|
|
||
| // Regression guard: the LAST call must be `false` — the flag must not be | ||
| // left stuck on `true` (the pre-fix bug that froze the app shell). | ||
| const calls: any[][] = updateOrganizationSwitchRequestLoadingStateMock.mock.calls; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace any in mock call typing.
Line 143, Line 168, and Line 188 use any for mock call arrays; this violates the TS guideline and weakens type safety in assertions.
As per coding guidelines, “Never use any type; use proper types or unknown with type guards instead.”
Also applies to: 168-168, 188-188
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@features/admin.organizations.v1/__tests__/organization-switch-breadcrumb.test.tsx`
at line 143, The mock call assertions in organization-switch-breadcrumb.test.tsx
use unsafe any typing for
updateOrganizationSwitchRequestLoadingStateMock.mock.calls; replace those array
typings with a proper tuple/interface type derived from the mock signature, or
use unknown plus narrowing in the assertions. Update the repeated call-site
variables in the test to use the same strongly typed mock-call shape so the
checks remain type-safe without any.
Source: Coding guidelines
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10470 +/- ##
========================================
Coverage 72.72% 72.73%
========================================
Files 469 469
Lines 70888 70908 +20
Branches 484 240 -244
========================================
+ Hits 51555 51572 +17
- Misses 19037 19225 +188
+ Partials 296 111 -185 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
features/admin.organizations.v1/__tests__/organization-switch-breadcrumb-root-consistency.test.tsx (1)
116-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
data-componentidselectors instead of text queries.Querying
"bt.com"makes these assertions depend on display text rather than the component contract. Please select the breadcrumb item viadata-componentidso the test follows the repo standard and stays resilient to label changes. As per coding guidelines, "Usedata-componentidfor DOM element selection in tests (notidor CSS classes) — useIdentifiableComponentInterfaceto provide this attribute." Based on learnings, "prefer using the data-componentid attribute for component identification in tests."Also applies to: 136-136
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/admin.organizations.v1/__tests__/organization-switch-breadcrumb-root-consistency.test.tsx` at line 116, The breadcrumb test is selecting the root organization item by visible text, which ties the assertion to label content instead of the component contract. Update the relevant queries in organization-switch-breadcrumb-root-consistency.test.tsx to use the breadcrumb item’s data-componentid, matching the repo standard for test selectors. Locate the affected assertions around rootOrgItem and replace the text-based lookup with the component identifier provided by the breadcrumb/IdentifiableComponentInterface implementation so the test remains resilient to label changes.Sources: Coding guidelines, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@features/admin.organizations.v1/__tests__/organization-switch-breadcrumb-root-consistency.test.tsx`:
- Around line 88-91: The root-path breadcrumb test is only verifying that
clicking does not switch to the same organization, but it never proves the click
actually reaches and opens the tenant dropdown. Update the `tenant-dropdown`
mock and the related assertions in
`organization-switch-breadcrumb-root-consistency.test.tsx` so the mocked
`TenantDropdown` exposes a positive open signal when its trigger is activated,
and assert that signal alongside the existing no-self-switch checks. Use the
`organization-switch-breadcrumb.tsx` behavior and the mocked `tenant-dropdown`
component to confirm the breadcrumb click bubbles through and opens the
dropdown.
---
Nitpick comments:
In
`@features/admin.organizations.v1/__tests__/organization-switch-breadcrumb-root-consistency.test.tsx`:
- Line 116: The breadcrumb test is selecting the root organization item by
visible text, which ties the assertion to label content instead of the component
contract. Update the relevant queries in
organization-switch-breadcrumb-root-consistency.test.tsx to use the breadcrumb
item’s data-componentid, matching the repo standard for test selectors. Locate
the affected assertions around rootOrgItem and replace the text-based lookup
with the component identifier provided by the
breadcrumb/IdentifiableComponentInterface implementation so the test remains
resilient to label changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 0d6012c2-7a36-4b05-b5a5-655ba413e221
📒 Files selected for processing (3)
.changeset/fix-console-org-switch-root-name-click-consistency.mdfeatures/admin.organizations.v1/__tests__/organization-switch-breadcrumb-root-consistency.test.tsxfeatures/admin.organizations.v1/components/organization-switch/organization-switch-breadcrumb.tsx
✅ Files skipped from review due to trivial changes (1)
- .changeset/fix-console-org-switch-root-name-click-consistency.md
🚧 Files skipped from review as they are similar to previous changes (1)
- features/admin.organizations.v1/components/organization-switch/organization-switch-breadcrumb.tsx
| vi.mock("@wso2is/admin.tenants.v1/components/dropdown/tenant-dropdown", () => ({ | ||
| __esModule: true, | ||
| default: (props: { dropdownTrigger: React.ReactNode }) => <div>{ props.dropdownTrigger }</div> | ||
| })); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The root-path regression test never proves the click reaches the dropdown.
The tenant-dropdown mock is a passive wrapper, so this test only proves "no self-switch". If the breadcrumb click stops bubbling and becomes a no-op, it still passes, even though the root-click contract in organization-switch-breadcrumb.tsx is to bubble up and open the dropdown. Please assert a positive dropdown-open signal here as well.
Also applies to: 110-123
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@features/admin.organizations.v1/__tests__/organization-switch-breadcrumb-root-consistency.test.tsx`
around lines 88 - 91, The root-path breadcrumb test is only verifying that
clicking does not switch to the same organization, but it never proves the click
actually reaches and opens the tenant dropdown. Update the `tenant-dropdown`
mock and the related assertions in
`organization-switch-breadcrumb-root-consistency.test.tsx` so the mocked
`TenantDropdown` exposes a positive open signal when its trigger is activated,
and assert that signal alongside the existing no-self-switch checks. Use the
`organization-switch-breadcrumb.tsx` behavior and the mocked `tenant-dropdown`
component to confirm the breadcrumb click bubbles through and opens the
dropdown.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
features/admin.organizations.v1/__tests__/organization-switch-breadcrumb-root-consistency.test.tsx (1)
162-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
data-componentidinstead of.closest(".section")in these tests.These assertions currently depend on Semantic UI’s wrapper DOM, so harmless markup changes can break the tests without changing behavior. Query the crumb via its
data-componentidand assert on that node directly. As per coding guidelines, "Usedata-componentidfor DOM element selection in tests (notidor CSS classes)" and "Do not usedata-testid— usedata-componentidviaIdentifiableComponentInterfaceinstead." Based on learnings, "prefer using the data-componentid attribute for component identification in tests."Also applies to: 188-189
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/admin.organizations.v1/__tests__/organization-switch-breadcrumb-root-consistency.test.tsx` around lines 162 - 163, The breadcrumb root assertions in organization-switch-breadcrumb-root-consistency.test.tsx are coupled to Semantic UI wrapper markup via .closest(".section"). Update the tests to locate the crumb using its data-componentid and assert against that element directly instead of traversing CSS classes. Make the same change in the related root crumb checks in the same test file so the queries stay stable even if wrapper DOM changes.Sources: Coding guidelines, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@features/admin.organizations.v1/__tests__/organization-switch-breadcrumb-root-consistency.test.tsx`:
- Around line 162-163: The breadcrumb root assertions in
organization-switch-breadcrumb-root-consistency.test.tsx are coupled to Semantic
UI wrapper markup via .closest(".section"). Update the tests to locate the crumb
using its data-componentid and assert against that element directly instead of
traversing CSS classes. Make the same change in the related root crumb checks in
the same test file so the queries stay stable even if wrapper DOM changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 6ffd78cc-78d7-482d-a756-8052ad49a733
📒 Files selected for processing (3)
.changeset/fix-console-org-switch-root-name-click-consistency.mdfeatures/admin.organizations.v1/__tests__/organization-switch-breadcrumb-root-consistency.test.tsxfeatures/admin.organizations.v1/components/organization-switch/organization-switch-breadcrumb.tsx
✅ Files skipped from review due to trivial changes (1)
- .changeset/fix-console-org-switch-root-name-click-consistency.md
🚧 Files skipped from review as they are similar to previous changes (1)
- features/admin.organizations.v1/components/organization-switch/organization-switch-breadcrumb.tsx
…rganization name in breadcrumb
a756bc8 to
1455f6b
Compare
1455f6b to
c589d56
Compare
1 participant
Summary
Clicking the root organization name in the Console header breadcrumb switcher left the app shell permanently stuck in a loading-skeleton state — the left side panel and org-switcher header never resolved after the click. A manual browser reload was the only workaround. This is a master-branch fix for
wso2/identity-apps.Root Cause
handleOrganizationSwitchinorganization-switch-breadcrumb.tsxcallsupdateOrganizationSwitchRequestLoadingState(true)before the re-sign-in sequence to widen the skeleton window (intentional, from a prior commit). On the root-org self-click path the handler'stryblock completes and falls through with no(false)reset, leaving the shared flag permanentlytrue. The consumers —dashboard-layout.tsx(side panel) andheader.tsx(org-switcher) — keep rendering the skeleton indefinitely because the flag never clears. The emptycatchswallows any error, so the error path had the same stuck-flag problem.Changes
features/admin.organizations.v1/components/organization-switch/organization-switch-breadcrumb.tsxfinally { updateOrganizationSwitchRequestLoadingState(false); }to the existingtry/catchinhandleOrganizationSwitch(+2 lines). The(true)set beforeonSignInis preserved — the wider skeleton window for genuine cross-org switches is unchanged. The flag is now always cleared on both success and error paths..changeset/fix-console-org-switch-stuck-skeleton.md"@wso2is/console": patchfeatures/admin.organizations.v1/__tests__/organization-switch-breadcrumb.test.tsxfalseafter switch — covering the bug scenario (success path), error path, and balance (everytruefollowed by afalse). Tests fail on unpatched code and pass after the fix.Verification
Runtime verification was performed against a freshly-extracted
wso2is-7.4.0-SNAPSHOTpack patched with the rebuilt@wso2is/consolewebapp (branchfix/issue-7610, HEADd82253ee), using Playwright (headless Chromium):Root-org self-click (the bug) — FIXED. After the click the shell stayed fully healthy with no manual reload:
hasSidePanel: true, hasWelcome: true, skeletons: 0. On the unpatched pack the same step producedhasSidePanel: false, skeletons: stuck.Root → sub-org switch — PASS. Transient skeleton appeared mid-switch (wider window preserved from prior commit), then fully resolved at the sub-org console URL. Side panel + header showing sub-org context.
Sub-org → root switch — PASS, no bounce-back. Final URL had no
/o/segment; full root-org chrome restored. The bounce-back guard indashboard-layout.tsx(from PR Fix sub-org to root org switch bounce-back #10281) was not disturbed.Server logs: no fix-related errors. Console HTTP 200 throughout.
Test Evidence
.ai/reproduce-7610.mjs(primary bug path),.ai/verify-7610-crossorg.mjs(cross-org regression)features/admin.organizations.v1/__tests__/organization-switch-breadcrumb.test.tsx— confirmed fail on unpatched, pass on patchedHow to Test
For runtime verification: deploy the patched console to a
wso2is-7.4.0-SNAPSHOTpack, create a tenant, log in as the tenant owner, click the root org name in the header breadcrumb, and confirm the app shell (side panel + org switcher) resolves without a manual page reload.