Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 59 additions & 4 deletions src/core/config/__tests__/importExport.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -731,9 +731,12 @@ describe("importExport", () => {
{ name: "valid-profile", id: "valid-id", apiProvider: "openai" as ProviderName },
])

const seenImportedAt: Array<number | undefined> = []
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)
Expand Down Expand Up @@ -766,15 +769,67 @@ 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()
showInfoMessageSpy.mockRestore()
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<number | undefined> = []
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" }])

Expand Down
1 change: 1 addition & 0 deletions src/core/config/importExport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
19 changes: 18 additions & 1 deletion webview-ui/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const App = () => {
const {
didHydrateState,
showWelcome,
settingsImportedAt,
shouldShowAnnouncement,
telemetrySetting,
telemetryKey,
Expand All @@ -67,6 +68,7 @@ const App = () => {

const [showAnnouncement, setShowAnnouncement] = useState(false)
const [tab, setTab] = useState<Tab>("chat")
const handledImportRef = useRef<number | undefined>(undefined)

const [deleteMessageDialogState, setDeleteMessageDialogState] = useState<DeleteMessageDialogState>({
isOpen: false,
Expand Down Expand Up @@ -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])
Comment thread
roomote[bot] marked this conversation as resolved.

useEffect(() => {
if (didHydrateState) {
telemetryClient.updateTelemetryState(telemetrySetting, telemetryKey, machineId)
Expand Down Expand Up @@ -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 ? (
<WelcomeView />
) : (
<>
Expand Down
192 changes: 192 additions & 0 deletions webview-ui/src/__tests__/App.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ vi.mock("@src/components/settings/SettingsView", () => ({
},
}))

vi.mock("@src/components/welcome/WelcomeViewProvider", () => ({
__esModule: true,
default: function WelcomeView() {
return <div data-testid="welcome-view">Welcome View</div>
},
}))

vi.mock("@src/components/history/HistoryView", () => ({
__esModule: true,
default: function HistoryView({ onDone }: { onDone: () => void }) {
Expand Down Expand Up @@ -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(<AppWithProviders />)

Expand All @@ -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(<AppWithProviders />)

expect(screen.getByTestId("welcome-view")).toBeInTheDocument()
expect(screen.queryByTestId("settings-view")).not.toBeInTheDocument()
})

it("switches to settings view when receiving settingsButtonClicked action", async () => {
render(<AppWithProviders />)

Expand All @@ -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(<AppWithProviders />)

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(<AppWithProviders />)

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(<AppWithProviders />)

if (action) {
act(() => {
triggerMessage(action)
})
}

if (action === "historyButtonClicked") {
expect(screen.getByTestId("welcome-view")).toBeInTheDocument()
}

state.settingsImportedAt = Date.now()
rerender(<AppWithProviders />)

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(<AppWithProviders />)

act(() => {
triggerMessage(action)
})

expect(await screen.findByTestId(viewId)).toBeInTheDocument()
expect(screen.queryByTestId("welcome-view")).not.toBeInTheDocument()

state.settingsImportedAt = Date.now()
rerender(<AppWithProviders />)

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(<AppWithProviders />)

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(<AppWithProviders />)

Expand Down
Loading