From 309c8894834ab18dc9a22d2d21c98fa0e2b7fe52 Mon Sep 17 00:00:00 2001 From: Smartsheet-JB-Brown Date: Mon, 14 Apr 2025 23:47:52 -0700 Subject: [PATCH] deleting a source causes browse refresh --- .../PackageManagerViewStateManager.ts | 16 +- .../PackageManagerViewStateManager.test.ts | 137 +++++++++++++++--- 2 files changed, 133 insertions(+), 20 deletions(-) diff --git a/webview-ui/src/components/package-manager/PackageManagerViewStateManager.ts b/webview-ui/src/components/package-manager/PackageManagerViewStateManager.ts index e7bbf963f7..4c3ebebdbc 100644 --- a/webview-ui/src/components/package-manager/PackageManagerViewStateManager.ts +++ b/webview-ui/src/components/package-manager/PackageManagerViewStateManager.ts @@ -308,15 +308,20 @@ export class PackageManagerViewStateManager { const updatedSources = sources.length === 0 ? [DEFAULT_PACKAGE_MANAGER_SOURCE] : sources this.state.sources = updatedSources this.sourcesModified = true // Set the flag when sources are modified + this.state.isFetching = false // Reset fetching state when sources change this.notifyStateChange() + // Send sources update to extension vscode.postMessage({ type: "packageManagerSources", sources: updatedSources, } as WebviewMessage) + // Schedule fetch for next tick to ensure sources message is sent first if (this.state.activeTab === "browse") { - void this.transition({ type: "FETCH_ITEMS" }) + setTimeout(() => { + void this.transition({ type: "FETCH_ITEMS" }) + }, 0) } break } @@ -351,12 +356,21 @@ export class PackageManagerViewStateManager { isFetching: message.state?.isFetching, itemCount: message.state?.packageManagerItems?.length, firstItem: message.state?.packageManagerItems?.[0], + sources: message.state?.sources, currentState: { isFetching: this.state.isFetching, itemCount: this.state.allItems.length, + sources: this.state.sources, }, }) + // Update sources from either sources or packageManagerSources in state + if (message.state?.sources || message.state?.packageManagerSources) { + const sources = message.state.packageManagerSources || message.state.sources + this.state.sources = sources?.length > 0 ? sources : [DEFAULT_PACKAGE_MANAGER_SOURCE] + this.notifyStateChange() + } + if (message.state?.isFetching) { console.log("State indicates fetching, transitioning to FETCH_ITEMS") void this.transition({ diff --git a/webview-ui/src/components/package-manager/__tests__/PackageManagerViewStateManager.test.ts b/webview-ui/src/components/package-manager/__tests__/PackageManagerViewStateManager.test.ts index 70eed975d0..b683a85da7 100644 --- a/webview-ui/src/components/package-manager/__tests__/PackageManagerViewStateManager.test.ts +++ b/webview-ui/src/components/package-manager/__tests__/PackageManagerViewStateManager.test.ts @@ -268,25 +268,19 @@ describe("PackageManagerViewStateManager", () => { // Wait for state to settle jest.runAllTimers() - const state = manager.getState() - expect(state.sources).toEqual([ - { - url: "https://github.com/RooVetGit/Roo-Code-Packages", - name: "Roo Code", - enabled: true, - }, - ]) + // Get all calls to postMessage + const calls = (vscode.postMessage as jest.Mock).mock.calls + const sourcesMessages = calls.filter((call) => call[0].type === "packageManagerSources") + const lastSourcesMessage = sourcesMessages[sourcesMessages.length - 1] - // Should send the final sources state to webview with default source - expect(vscode.postMessage).toHaveBeenLastCalledWith({ + // Verify state has default source + const state = manager.getState() + expect(state.sources).toEqual([DEFAULT_PACKAGE_MANAGER_SOURCE]) + + // Verify the last sources message was sent with default source + expect(lastSourcesMessage[0]).toEqual({ type: "packageManagerSources", - sources: [ - { - url: "https://github.com/RooVetGit/Roo-Code-Packages", - name: "Roo Code", - enabled: true, - }, - ], + sources: [DEFAULT_PACKAGE_MANAGER_SOURCE], }) }) @@ -596,6 +590,44 @@ describe("PackageManagerViewStateManager", () => { // Filter behavior tests are already covered in the previous describe block describe("Source Management", () => { + beforeEach(() => { + // Mock setTimeout to execute immediately + jest.useFakeTimers() + }) + + afterEach(() => { + jest.useRealTimers() + }) + + it("should reset isFetching after source deletion", async () => { + // Start with two sources + const sources = [ + { url: "https://github.com/test/repo1", enabled: true }, + { url: "https://github.com/test/repo2", enabled: true }, + ] + + await manager.transition({ + type: "UPDATE_SOURCES", + payload: { sources }, + }) + + // Set isFetching to true + await manager.transition({ type: "FETCH_ITEMS" }) + + // Delete one source + await manager.transition({ + type: "UPDATE_SOURCES", + payload: { sources: [sources[0]] }, + }) + + // Run any pending timers + jest.runAllTimers() + + // Verify isFetching was reset + const state = manager.getState() + expect(state.isFetching).toBe(false) + }) + it("should re-add default source when all sources are removed", async () => { // Add some test sources const sources = [ @@ -608,14 +640,24 @@ describe("PackageManagerViewStateManager", () => { payload: { sources }, }) + // Clear mock to ignore previous messages + ;(vscode.postMessage as jest.Mock).mockClear() + // Remove all sources await manager.transition({ type: "UPDATE_SOURCES", payload: { sources: [] }, }) - // Verify that the default source was automatically re-added - expect(vscode.postMessage).toHaveBeenLastCalledWith({ + // Run any pending timers before checking messages + jest.runAllTimers() + + // Get all calls to postMessage + const calls = (vscode.postMessage as jest.Mock).mock.calls + const sourcesMessage = calls.find((call) => call[0].type === "packageManagerSources") + + // Verify that the sources message was sent with default source + expect(sourcesMessage[0]).toEqual({ type: "packageManagerSources", sources: [ { @@ -730,6 +772,63 @@ describe("PackageManagerViewStateManager", () => { }) describe("Message Handling", () => { + it("should restore sources from packageManagerSources on webview launch", () => { + const savedSources = [ + { + url: "https://github.com/RooVetGit/Roo-Code-Packages", + name: "Roo Code", + enabled: true, + }, + { + url: "https://github.com/test/custom-repo", + name: "Custom Repo", + enabled: true, + }, + ] + + // Simulate VS Code restart by sending initial state with saved sources + manager.handleMessage({ + type: "state", + state: { packageManagerSources: savedSources }, + }) + + const state = manager.getState() + expect(state.sources).toEqual(savedSources) + }) + + it("should use default source when state message has no sources", () => { + manager.handleMessage({ + type: "state", + state: { packageManagerItems: [] }, + }) + + const state = manager.getState() + expect(state.sources).toEqual([DEFAULT_PACKAGE_MANAGER_SOURCE]) + }) + + it("should update sources when receiving state message", () => { + const customSources = [ + { + url: "https://github.com/test/repo1", + name: "Test Repo 1", + enabled: true, + }, + { + url: "https://github.com/test/repo2", + name: "Test Repo 2", + enabled: true, + }, + ] + + manager.handleMessage({ + type: "state", + state: { sources: customSources }, + }) + + const state = manager.getState() + expect(state.sources).toEqual(customSources) + }) + it("should handle state message with package manager items", () => { const testItems = [createTestItem()]