Fix back button in artifact creation forms navigating to home instead of Add Artifact page#1959
Fix back button in artifact creation forms navigating to home instead of Add Artifact page#1959SasinduDilshara wants to merge 2 commits intomainfrom
Conversation
…#820) Two defects were causing the "Go Back" button in artifact creation forms to navigate to the Project Overview (home) instead of the Add Artifact page: 1. goBack() in rpc-manager.ts had an incorrect guard that called navigate(projectUri) with no history entry when the current view was a Form, causing navigation to the default/home view. Fix: remove the guard — always pop history and navigate to the popped entry. 2. The Add Artifact view never called addToHistory() before opening a creation form via executeCommand(), so the history stack had no Add Artifact entry to pop back to. Fix: add addToHistory({ location: { view: MACHINE_VIEW.ADD_ARTIFACT } }) at the top of handleClick() in AddArtifact/index.tsx. Also adds: - E2E back-navigation test (navigationTests/backNavigation.spec.ts) - Unit tests for the History stack (test/suite/history.test.ts) - Registers the new E2E test suite in test.list.ts Fixes wso2/mi-vscode#820 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Issue Analysis ArtifactIssue Analysis — [Issue #820]: Back button in artifact creation forms directs to the home page instead of the Add Artifact pageClassification
Reproducibility
Root Cause AnalysisThere are two interrelated defects that together cause the bug: Defect 1 —
|
Fix Plan — Issue #820Dev Test Result
Changes Made
Known IssuesNone — dev test passed cleanly. The fix covers all artifact types (the |
Fix Verification ReportIssue: wso2/mi-vscode#820 Reproduction Steps Executed
ResultAfter clicking "Go Back" from the API creation form, the webview navigated back to the Add Artifact page (tab title "Add Artifact", showing the "Create an Integration" artifact cards: API, Automation, Event Integration, + View More). Before the fix, clicking "Go Back" would have navigated to the Project Overview (home) page instead. The fix correctly restores the expected back-navigation behavior. Evidenceverify-820-05-add-artifact-view.pngStarting state — "Add Artifact" page open with artifact cards (API, Automation, Event Integration, + View More button). verify-820-07-api-form.pngAfter clicking the API card — "API Form" tab is active, showing the Create API form with Name*, Context*, Version Type fields and breadcrumb verify-820-08-after-back.pngAfter clicking "Go Back" — the webview shows the "Add Artifact" page with "Create an Integration" cards (API, Automation, Event Integration). This is the CORRECT behavior. The tab title reads "Add Artifact". verify-820-09-final-state.pngFinal confirmed state — "Add Artifact" page remains visible. Navigation correctly returned to Add Artifact (not Project Overview). Fix SummaryTwo code changes were applied to
The fixed VSIX ( |
|
|
📝 WalkthroughWalkthroughThe changes address Issue Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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.
🧹 Nitpick comments (1)
workspaces/mi/mi-extension/src/test/e2e-playwright-tests/navigationTests/backNavigation.spec.ts (1)
93-94: Replace fixed sleeps with deterministic polling to improve test stability.Lines 93, 123, and 160 use
waitForTimeout(2000)before asserting webview title. This fixed sleep pattern is brittle under CI variance. Replace withexpect.poll()to wait on actual state, which the project already uses in Utils.ts:Suggested refactor
- await page.page.waitForTimeout(2000); - const { title: iframeTitle } = await page.getCurrentWebview(); - console.log(`After Go Back from API Form: iframeTitle="${iframeTitle}"`); - // Bug: before fix this was MACHINE_VIEW.Overview — now must be ADD_ARTIFACT. - expect(iframeTitle).toBe(MACHINE_VIEW.ADD_ARTIFACT); + await expect.poll(async () => { + const { title } = await page.getCurrentWebview(); + return title; + }, { timeout: 10000 }).toBe(MACHINE_VIEW.ADD_ARTIFACT);Apply similar changes at lines 123-124 and 160-161.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-extension/src/test/e2e-playwright-tests/navigationTests/backNavigation.spec.ts` around lines 93 - 94, Tests use fixed sleeps via await page.page.waitForTimeout(2000) before reading the webview title which is brittle; replace each sleep + immediate getCurrentWebview call with a deterministic poll using expect.poll to wait for the webview title to reach the expected value (e.g., expect.poll(async () => (await page.getCurrentWebview()).title, { timeout: <sensible-ms> }).toEqual(expectedTitle)). Update the occurrences that call page.page.waitForTimeout and then const { title: iframeTitle } = await page.getCurrentWebview() to use this polling pattern (see existing expect.poll usage in Utils.ts for the exact style and timeout configuration).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@workspaces/mi/mi-extension/src/test/e2e-playwright-tests/navigationTests/backNavigation.spec.ts`:
- Around line 93-94: Tests use fixed sleeps via await
page.page.waitForTimeout(2000) before reading the webview title which is
brittle; replace each sleep + immediate getCurrentWebview call with a
deterministic poll using expect.poll to wait for the webview title to reach the
expected value (e.g., expect.poll(async () => (await
page.getCurrentWebview()).title, { timeout: <sensible-ms>
}).toEqual(expectedTitle)). Update the occurrences that call
page.page.waitForTimeout and then const { title: iframeTitle } = await
page.getCurrentWebview() to use this polling pattern (see existing expect.poll
usage in Utils.ts for the exact style and timeout configuration).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2f5c1c67-09c2-420b-a037-f180f0664d69
📒 Files selected for processing (6)
workspaces/mi/mi-extension/package.jsonworkspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.tsworkspaces/mi/mi-extension/src/test/e2e-playwright-tests/navigationTests/backNavigation.spec.tsworkspaces/mi/mi-extension/src/test/e2e-playwright-tests/test.list.tsworkspaces/mi/mi-extension/src/test/suite/history.test.tsworkspaces/mi/mi-visualizer/src/views/AddArtifact/index.tsx
3 participants
Summary
Fixes wso2/mi-vscode#820
When a user navigates to the Add Artifact page and clicks an artifact type card (e.g. API), the resulting creation form shows two navigation buttons — "Go Back" (←) and "Home" (⌂). Clicking "Go Back" was incorrectly navigating to the Project Overview (home page) instead of returning to the Add Artifact page — making both buttons behave identically.
Root Cause — Two Interrelated Defects
Defect 1 —
goBack()navigates to Home when current view is a Form(
workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts)When the current view is a creation form, the
elsebranch callednavigate(projectUri)with no history entry, which always resolves to the default/home view.Defect 2 — Add Artifact view never pushed itself to history before opening a form
(
workspaces/mi/mi-visualizer/src/views/AddArtifact/index.tsx)When a user clicked an artifact card,
handleClick()fired a VS Code command to open the creation form but never calledaddToHistory(). So the history stack had no Add Artifact entry to pop back to.Fix
rpc-manager.tsif (!view?.includes("Form"))guard —goBack()now always pops the history stack and navigates to the popped entry.AddArtifact/index.tsxaddToHistory({ location: { view: MACHINE_VIEW.ADD_ARTIFACT } })at the top ofhandleClick(), before anyexecuteCommandcall.The
addToHistorycall is placed before theif/else ifchain so it applies to all artifact types (APIs, endpoints, sequences, templates, message stores, etc.).Tests Added
navigationTests/backNavigation.spec.ts): Opens a project, navigates to Add Artifact, clicks the API card, then verifies the "Go Back" button returns to the Add Artifact page (not Project Overview). Also verifies the "Home" button still navigates to Project Overview.test/suite/history.test.ts): Tests theHistorystack behaviour — push/pop round-trips, LIFO order, the fixed flow (AddArtifact entry in stack before form), and the pre-fix scenario (documents the bug).test.list.ts.Test Plan
addToHistoryis called before the artifact-type branch, not inside each branch🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Chores