diff --git a/src/core/config/__tests__/importExport.spec.ts b/src/core/config/__tests__/importExport.spec.ts index dcd7f0089c..74bb44aff4 100644 --- a/src/core/config/__tests__/importExport.spec.ts +++ b/src/core/config/__tests__/importExport.spec.ts @@ -731,9 +731,12 @@ describe("importExport", () => { { name: "valid-profile", id: "valid-id", apiProvider: "openai" as ProviderName }, ]) + const seenImportedAt: Array = [] const mockProvider = { - settingsImportedAt: 0, - postStateToWebview: vi.fn().mockResolvedValue(undefined), + settingsImportedAt: undefined as number | undefined, + postStateToWebview: vi.fn().mockImplementation(async () => { + seenImportedAt.push(mockProvider.settingsImportedAt) + }), } const showWarningMessageSpy = vi.spyOn(vscode.window, "showWarningMessage").mockResolvedValue(undefined) @@ -766,8 +769,10 @@ describe("importExport", () => { ) expect(showInfoMessageSpy).not.toHaveBeenCalled() - // Provider state should still be updated - expect(mockProvider.settingsImportedAt).toBeGreaterThan(0) + // Provider state should be delivered once, then cleared. + expect(seenImportedAt).toHaveLength(1) + expect(seenImportedAt[0]).toBeGreaterThan(0) + expect(mockProvider.settingsImportedAt).toBeUndefined() expect(mockProvider.postStateToWebview).toHaveBeenCalled() showWarningMessageSpy.mockRestore() @@ -775,6 +780,56 @@ describe("importExport", () => { consoleWarnSpy.mockRestore() }) + it("clears settingsImportedAt after posting the imported state so later launches do not replay it", async () => { + const filePath = "/mock/path/settings.json" + const mockFileContent = JSON.stringify({ + providerProfiles: { + currentApiConfigName: "valid-profile", + apiConfigs: { + "valid-profile": { + apiProvider: "openai" as ProviderName, + apiKey: "test-key", + id: "valid-id", + }, + }, + }, + globalSettings: { mode: "code" }, + }) + + ;(fs.readFile as Mock).mockResolvedValue(mockFileContent) + ;(fs.access as Mock).mockResolvedValue(undefined) + + mockProviderSettingsManager.export.mockResolvedValue({ + currentApiConfigName: "default", + apiConfigs: { default: { apiProvider: "anthropic" as ProviderName, id: "default-id" } }, + }) + mockProviderSettingsManager.listConfig.mockResolvedValue([ + { name: "valid-profile", id: "valid-id", apiProvider: "openai" as ProviderName }, + ]) + + const seenImportedAt: Array = [] + const mockProvider = { + settingsImportedAt: undefined as number | undefined, + postStateToWebview: vi.fn().mockImplementation(async () => { + seenImportedAt.push(mockProvider.settingsImportedAt) + }), + } + + await importSettingsWithFeedback( + { + providerSettingsManager: mockProviderSettingsManager, + contextProxy: mockContextProxy, + customModesManager: mockCustomModesManager, + provider: mockProvider, + }, + filePath, + ) + + expect(seenImportedAt).toHaveLength(1) + expect(seenImportedAt[0]).toBeGreaterThan(0) + expect(mockProvider.settingsImportedAt).toBeUndefined() + }) + it("should handle multiple profiles with mixed valid and invalid providers", async () => { ;(vscode.window.showOpenDialog as Mock).mockResolvedValue([{ fsPath: "/mock/path/settings.json" }]) diff --git a/src/core/config/importExport.ts b/src/core/config/importExport.ts index 050a43191e..c2adb982c6 100644 --- a/src/core/config/importExport.ts +++ b/src/core/config/importExport.ts @@ -327,6 +327,7 @@ export const importSettingsWithFeedback = async ( if (result.success) { provider.settingsImportedAt = Date.now() await provider.postStateToWebview() + provider.settingsImportedAt = undefined const warnings = "warnings" in result ? result.warnings : undefined // Show warnings if any profiles had issues but were still imported (with modifications) diff --git a/webview-ui/src/App.tsx b/webview-ui/src/App.tsx index f02da9a34a..6f39e806d0 100644 --- a/webview-ui/src/App.tsx +++ b/webview-ui/src/App.tsx @@ -54,6 +54,7 @@ const App = () => { const { didHydrateState, showWelcome, + settingsImportedAt, shouldShowAnnouncement, telemetrySetting, telemetryKey, @@ -67,6 +68,7 @@ const App = () => { const [showAnnouncement, setShowAnnouncement] = useState(false) const [tab, setTab] = useState("chat") + const handledImportRef = useRef(undefined) const [deleteMessageDialogState, setDeleteMessageDialogState] = useState({ isOpen: false, @@ -169,6 +171,19 @@ const App = () => { } }, [shouldShowAnnouncement, tab]) + useEffect(() => { + const isRecoverableTab = tab === "settings" || tab === "marketplace" + + if (showWelcome && settingsImportedAt && settingsImportedAt !== handledImportRef.current) { + handledImportRef.current = settingsImportedAt + if (!isRecoverableTab) { + setCurrentSection("providers") + setCurrentMarketplaceTab(undefined) + setTab("settings") + } + } + }, [showWelcome, settingsImportedAt, tab]) + useEffect(() => { if (didHydrateState) { telemetryClient.updateTelemetryState(telemetrySetting, telemetryKey, machineId) @@ -214,7 +229,9 @@ const App = () => { // Do not conditionally load ChatView, it's expensive and there's state we // don't want to lose (user input, disableInput, askResponse promise, etc.) - return showWelcome ? ( + const isSetupGatedTab = showWelcome && tab !== "settings" && tab !== "marketplace" + + return isSetupGatedTab ? ( ) : ( <> diff --git a/webview-ui/src/__tests__/App.spec.tsx b/webview-ui/src/__tests__/App.spec.tsx index 29d2c41977..137bed5d70 100644 --- a/webview-ui/src/__tests__/App.spec.tsx +++ b/webview-ui/src/__tests__/App.spec.tsx @@ -47,6 +47,13 @@ vi.mock("@src/components/settings/SettingsView", () => ({ }, })) +vi.mock("@src/components/welcome/WelcomeViewProvider", () => ({ + __esModule: true, + default: function WelcomeView() { + return
Welcome View
+ }, +})) + vi.mock("@src/components/history/HistoryView", () => ({ __esModule: true, default: function HistoryView({ onDone }: { onDone: () => void }) { @@ -181,6 +188,15 @@ describe("App", () => { window.dispatchEvent(messageEvent) } + const createSetupIncompleteState = () => ({ + didHydrateState: true, + showWelcome: true, + shouldShowAnnouncement: false, + experiments: {}, + language: "en", + telemetrySetting: "enabled", + }) + it("shows chat view by default", () => { render() @@ -189,6 +205,22 @@ describe("App", () => { expect(chatView.getAttribute("data-hidden")).toBe("false") }, 10000) + it("shows welcome view when setup is incomplete", () => { + mockUseExtensionState.mockReturnValue({ + didHydrateState: true, + showWelcome: true, + shouldShowAnnouncement: false, + experiments: {}, + language: "en", + telemetrySetting: "enabled", + }) + + render() + + expect(screen.getByTestId("welcome-view")).toBeInTheDocument() + expect(screen.queryByTestId("settings-view")).not.toBeInTheDocument() + }) + it("switches to settings view when receiving settingsButtonClicked action", async () => { render() @@ -203,6 +235,166 @@ describe("App", () => { expect(chatView.getAttribute("data-hidden")).toBe("true") }) + it.each([ + ["settings", "settings-view"], + ["marketplace", "marketplace-view"], + ])("still switches to %s while welcome gating is active", async (action, testId) => { + mockUseExtensionState.mockReturnValue({ + didHydrateState: true, + showWelcome: true, + shouldShowAnnouncement: false, + experiments: {}, + language: "en", + telemetrySetting: "enabled", + }) + + render() + + act(() => { + triggerMessage(`${action}ButtonClicked`) + }) + + expect(await screen.findByTestId(testId)).toBeInTheDocument() + expect(screen.queryByTestId("welcome-view")).not.toBeInTheDocument() + }) + + it("keeps history behind the welcome gate while setup is incomplete", () => { + mockUseExtensionState.mockReturnValue({ + didHydrateState: true, + showWelcome: true, + shouldShowAnnouncement: false, + experiments: {}, + language: "en", + telemetrySetting: "enabled", + }) + + render() + + act(() => { + triggerMessage("historyButtonClicked") + }) + + expect(screen.getByTestId("welcome-view")).toBeInTheDocument() + expect(screen.queryByTestId("history-view")).not.toBeInTheDocument() + }) + + it.each([ + { label: "chat", action: undefined }, + { label: "history", action: "historyButtonClicked" }, + ])("redirects to providers settings when an import fires from the $label tab", async ({ action }) => { + const state = { + ...createSetupIncompleteState(), + settingsImportedAt: undefined as number | undefined, + } + + mockUseExtensionState.mockImplementation(() => state) + + const { rerender } = render() + + if (action) { + act(() => { + triggerMessage(action) + }) + } + + if (action === "historyButtonClicked") { + expect(screen.getByTestId("welcome-view")).toBeInTheDocument() + } + + state.settingsImportedAt = Date.now() + rerender() + + expect(await screen.findByTestId("settings-view")).toBeInTheDocument() + expect(screen.queryByTestId("welcome-view")).not.toBeInTheDocument() + }) + + it.each([ + { + label: "settings before returning to chat", + action: "settingsButtonClicked", + viewId: "settings-view", + nextAction: undefined, + }, + { + label: "settings before switching to history", + action: "settingsButtonClicked", + viewId: "settings-view", + nextAction: "historyButtonClicked", + }, + { + label: "marketplace before returning to chat", + action: "marketplaceButtonClicked", + viewId: "marketplace-view", + nextAction: undefined, + }, + { + label: "marketplace before switching to history", + action: "marketplaceButtonClicked", + viewId: "marketplace-view", + nextAction: "historyButtonClicked", + }, + ])( + "consumes imported settings without a later redirect when already on $label", + async ({ action, viewId, nextAction }) => { + const state = { + ...createSetupIncompleteState(), + settingsImportedAt: undefined as number | undefined, + } + + mockUseExtensionState.mockImplementation(() => state) + + const { rerender } = render() + + act(() => { + triggerMessage(action) + }) + + expect(await screen.findByTestId(viewId)).toBeInTheDocument() + expect(screen.queryByTestId("welcome-view")).not.toBeInTheDocument() + + state.settingsImportedAt = Date.now() + rerender() + + const currentView = await screen.findByTestId(viewId) + expect(currentView).toBeInTheDocument() + + if (nextAction) { + act(() => { + triggerMessage(nextAction) + }) + } else { + act(() => { + currentView.click() + }) + } + + expect(screen.getByTestId("welcome-view")).toBeInTheDocument() + expect(screen.queryByTestId("settings-view")).not.toBeInTheDocument() + expect(screen.queryByTestId("marketplace-view")).not.toBeInTheDocument() + }, + ) + + it("does not bounce back to settings after the import redirect has already fired", async () => { + const importedAt = Date.now() + + mockUseExtensionState.mockReturnValue({ + ...createSetupIncompleteState(), + settingsImportedAt: importedAt, + }) + + render() + + const settingsView = await screen.findByTestId("settings-view") + expect(settingsView).toBeInTheDocument() + + act(() => { + settingsView.click() + }) + + expect(screen.getByTestId("welcome-view")).toBeInTheDocument() + expect(screen.queryByTestId("settings-view")).not.toBeInTheDocument() + }) + it("switches to history view when receiving historyButtonClicked action", async () => { render()