From 15d310469c65f4c01cc24342f6b508703e2ca0d6 Mon Sep 17 00:00:00 2001 From: Smartsheet-JB-Brown Date: Mon, 14 Apr 2025 23:23:39 -0700 Subject: [PATCH] initialize first load with data on browse --- .../package-manager/PackageManagerView.tsx | 5 + .../PackageManagerViewStateManager.ts | 13 +- .../__tests__/PackageManagerView.test.tsx | 172 ++++++------------ .../PackageManagerViewStateManager.test.ts | 62 ++++++- 4 files changed, 128 insertions(+), 124 deletions(-) diff --git a/webview-ui/src/components/package-manager/PackageManagerView.tsx b/webview-ui/src/components/package-manager/PackageManagerView.tsx index 29764895bc..dea1379bb6 100644 --- a/webview-ui/src/components/package-manager/PackageManagerView.tsx +++ b/webview-ui/src/components/package-manager/PackageManagerView.tsx @@ -27,6 +27,11 @@ const PackageManagerView: React.FC = ({ onDone }) => { }) }, [state.allItems]) + // Fetch items on mount + useEffect(() => { + manager.transition({ type: "FETCH_ITEMS" }) + }, [manager]) + // Compute all available tags const allTags = Array.from(new Set(state.allItems.flatMap((item) => item.tags || []))).sort() diff --git a/webview-ui/src/components/package-manager/PackageManagerViewStateManager.ts b/webview-ui/src/components/package-manager/PackageManagerViewStateManager.ts index b4338f9d2b..e7bbf963f7 100644 --- a/webview-ui/src/components/package-manager/PackageManagerViewStateManager.ts +++ b/webview-ui/src/components/package-manager/PackageManagerViewStateManager.ts @@ -46,6 +46,7 @@ export class PackageManagerViewStateManager { private readonly FETCH_TIMEOUT = 30000 // 30 seconds private readonly FILTER_DEBOUNCE = 300 // 300 milliseconds private stateChangeHandlers: Set = new Set() + private sourcesModified = false // Track if sources have been modified constructor() { this.state = { @@ -212,7 +213,16 @@ export class PackageManagerViewStateManager { this.notifyStateChange() if (tab === "browse") { - void this.transition({ type: "FETCH_ITEMS" }) + // Always fetch when switching to browse if sources were modified + if (this.sourcesModified) { + this.sourcesModified = false // Reset the flag + void this.transition({ type: "FETCH_ITEMS" }) + } else { + // Only fetch if we don't have any items yet + if (this.state.allItems.length === 0) { + void this.transition({ type: "FETCH_ITEMS" }) + } + } } break } @@ -297,6 +307,7 @@ export class PackageManagerViewStateManager { // If all sources are removed, add the default source 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.notifyStateChange() vscode.postMessage({ diff --git a/webview-ui/src/components/package-manager/__tests__/PackageManagerView.test.tsx b/webview-ui/src/components/package-manager/__tests__/PackageManagerView.test.tsx index 64b8a2cf93..e73ea1d08f 100644 --- a/webview-ui/src/components/package-manager/__tests__/PackageManagerView.test.tsx +++ b/webview-ui/src/components/package-manager/__tests__/PackageManagerView.test.tsx @@ -94,15 +94,52 @@ describe("PackageManagerView", () => { }) }) - it("should initialize with empty states", async () => { + it("should automatically fetch items on mount", async () => { render() - // Should show empty state message initially - expect(screen.getByText("No package manager items found")).toBeInTheDocument() + // Should immediately trigger a fetch + expect(mockPostMessage).toHaveBeenCalledWith({ + type: "fetchPackageManagerItems", + bool: true, + }) - // Simulate receiving empty items + // Should show loading state + expect(screen.getByText("Loading items...")).toBeInTheDocument() + + // Simulate receiving items + await act(async () => { + window.dispatchEvent( + new MessageEvent("message", { + data: { + type: "state", + state: { + packageManagerItems: mockItems, + isFetching: false, + activeTab: "browse", + refreshingUrls: [], + sources: [], + filters: { type: "", search: "", tags: [] }, + sortConfig: { by: "name", order: "asc" }, + }, + }, + }), + ) + }) + + // Should show items + expect(screen.getByText("2 items total")).toBeInTheDocument() + expect(screen.getByText("Test Package")).toBeInTheDocument() + expect(screen.getByText("Another Package")).toBeInTheDocument() + }) + + it("should show empty state when fetch returns no items", async () => { + render() + + // Should show loading state while fetching + expect(screen.getByText("Loading items...")).toBeInTheDocument() + + // Simulate receiving empty items from fetch await act(async () => { - // First trigger fetch request which sets isFetching to true window.dispatchEvent( new MessageEvent("message", { data: { @@ -125,69 +162,13 @@ describe("PackageManagerView", () => { expect(screen.getByText("No package manager items found")).toBeInTheDocument() }) - it("should handle state updates correctly", async () => { - render() - - // Initial state should show empty state - expect(screen.getByText("No package manager items found")).toBeInTheDocument() - - // Simulate state update - await act(async () => { - // First trigger fetch request which sets isFetching to true - window.dispatchEvent( - new MessageEvent("message", { - data: { - type: "state", - state: { - packageManagerItems: [], - isFetching: true, - activeTab: "browse", - refreshingUrls: [], - sources: [], - filters: { type: "", search: "", tags: [] }, - sortConfig: { by: "name", order: "asc" }, - }, - }, - }), - ) - }) - - // Wait for loading state to appear - await screen.findByText("Loading items...") - - // Complete the state update - await act(async () => { - window.dispatchEvent( - new MessageEvent("message", { - data: { - type: "state", - state: { - packageManagerItems: mockItems, - isFetching: false, - activeTab: "browse", - refreshingUrls: [], - sources: [], - filters: { type: "", search: "", tags: [] }, - sortConfig: { by: "name", order: "asc" }, - }, - }, - }), - ) - }) - - // Then wait for items to appear - await screen.findByText("2 items total") - expect(screen.getByText("Test Package")).toBeInTheDocument() - expect(screen.getByText("Another Package")).toBeInTheDocument() - }) - it("should handle filter state transitions", async () => { render() - // Initial state should show empty state - expect(screen.getByText("No package manager items found")).toBeInTheDocument() + // Should show loading state initially + expect(screen.getByText("Loading items...")).toBeInTheDocument() - // Load initial items + // Simulate receiving items await act(async () => { window.dispatchEvent( new MessageEvent("message", { @@ -248,11 +229,12 @@ describe("PackageManagerView", () => { expect(screen.getByText("Test Package")).toBeInTheDocument() expect(screen.queryByText("Another Package")).not.toBeInTheDocument() }) + it("should handle tab switching correctly", async () => { render() - // Initial state should show empty state - expect(screen.getByText("No package manager items found")).toBeInTheDocument() + // Should show loading state initially + expect(screen.getByText("Loading items...")).toBeInTheDocument() // Load initial items await act(async () => { @@ -316,12 +298,11 @@ describe("PackageManagerView", () => { expect(screen.getByText("Another Package")).toBeInTheDocument() }) - // Set shorter timeout for faster failure during debugging it("should handle source changes correctly", async () => { render() - // Initial state should show empty state - expect(screen.getByText("No package manager items found")).toBeInTheDocument() + // Should show loading state initially + expect(screen.getByText("Loading items...")).toBeInTheDocument() // Ensure state is updated and synchronized await act(async () => { @@ -353,7 +334,6 @@ describe("PackageManagerView", () => { // Wait for sources view to render await screen.findByText("Configure Package Manager Sources") - // Add new source // Add new source const urlInput = screen.getByPlaceholderText(/^Git repository URL/) fireEvent.change(urlInput, { target: { value: "https://github.com/test/repo" } }) @@ -377,34 +357,12 @@ describe("PackageManagerView", () => { bool: true, }) }) + it("should preserve filter state during tab switches", async () => { render() - // Set initial state - await act(async () => { - window.dispatchEvent( - new MessageEvent("message", { - data: { - type: "state", - state: { - packageManagerItems: [], - isFetching: false, - activeTab: "browse", - refreshingUrls: [], - sources: [], - filters: { type: "", search: "", tags: [] }, - sortConfig: { by: "name", order: "asc" }, - }, - }, - }), - ) - }) - - // Wait for initial render to complete - await screen.findByText("No package manager items found") - - // Wait for initial render to complete - await screen.findByText("No package manager items found") + // Should show loading state initially + expect(screen.getByText("Loading items...")).toBeInTheDocument() // Load initial items with explicit state transitions await act(async () => { @@ -476,6 +434,7 @@ describe("PackageManagerView", () => { await screen.findByText("1 item total") expect(screen.getByText("Test Package")).toBeInTheDocument() expect(screen.queryByText("Another Package")).not.toBeInTheDocument() + // Update search input and filter state const searchInput = screen.getByPlaceholderText("Search package manager items...") fireEvent.change(searchInput, { target: { value: "test" } }) @@ -503,31 +462,10 @@ describe("PackageManagerView", () => { }), ) }) + // Verify filtered text appears (handle both singular and plural cases) await screen.findByText(/1 item.*found.*filtered|1 items.*found.*filtered/) - // Switch to sources tab - await act(async () => { - window.dispatchEvent( - new MessageEvent("message", { - data: { - type: "state", - state: { - packageManagerItems: [mockItems[0]], - isFetching: false, - activeTab: "browse", - refreshingUrls: [], - sources: [], - filters: { type: "", search: "test", tags: [] }, - sortConfig: { by: "name", order: "asc" }, - isFiltered: true, - filteredCount: 1, - }, - }, - }), - ) - }) - // Switch to sources tab const sourcesTab = screen.getByRole("button", { name: "Sources" }) fireEvent.click(sourcesTab) 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 6b70664bb3..70eed975d0 100644 --- a/webview-ui/src/components/package-manager/__tests__/PackageManagerViewStateManager.test.ts +++ b/webview-ui/src/components/package-manager/__tests__/PackageManagerViewStateManager.test.ts @@ -85,7 +85,7 @@ describe("PackageManagerViewStateManager", () => { expect(state.sources).toEqual([ { url: "https://github.com/RooVetGit/Roo-Code-Packages", - name: "Roo Code Package Manager Template", + name: "Roo Code", enabled: true, }, ]) @@ -96,7 +96,7 @@ describe("PackageManagerViewStateManager", () => { sources: [ { url: "https://github.com/RooVetGit/Roo-Code-Packages", - name: "Roo Code Package Manager Template", + name: "Roo Code", enabled: true, }, ], @@ -272,7 +272,7 @@ describe("PackageManagerViewStateManager", () => { expect(state.sources).toEqual([ { url: "https://github.com/RooVetGit/Roo-Code-Packages", - name: "Roo Code Package Manager Template", + name: "Roo Code", enabled: true, }, ]) @@ -283,7 +283,7 @@ describe("PackageManagerViewStateManager", () => { sources: [ { url: "https://github.com/RooVetGit/Roo-Code-Packages", - name: "Roo Code Package Manager Template", + name: "Roo Code", enabled: true, }, ], @@ -474,7 +474,8 @@ describe("PackageManagerViewStateManager", () => { expect(state.activeTab).toBe("sources") }) - it("should trigger fetch when switching to browse tab", async () => { + it("should trigger fetch when switching to browse tab with no items", async () => { + jest.clearAllMocks() // Clear mock to ignore initialize() call await manager.transition({ type: "SET_ACTIVE_TAB", payload: { tab: "browse" }, @@ -486,6 +487,55 @@ describe("PackageManagerViewStateManager", () => { }) }) + it("should not trigger fetch when switching to browse tab with existing items", async () => { + jest.clearAllMocks() // Clear mock to ignore initialize() call + + // Add some items first + await manager.transition({ + type: "FETCH_COMPLETE", + payload: { items: [createTestItem()] }, + }) + + // Switch to browse tab + await manager.transition({ + type: "SET_ACTIVE_TAB", + payload: { tab: "browse" }, + }) + + expect(vscode.postMessage).not.toHaveBeenCalledWith({ + type: "fetchPackageManagerItems", + bool: true, + }) + }) + + it("should trigger fetch when switching to browse tab after source modification", async () => { + jest.clearAllMocks() // Clear mock to ignore initialize() call + + // Add some items first + await manager.transition({ + type: "FETCH_COMPLETE", + payload: { items: [createTestItem()] }, + }) + + // Modify sources + await manager.transition({ + type: "UPDATE_SOURCES", + payload: { sources: [{ url: "https://github.com/test/repo1", enabled: true }] }, + }) + + // Switch to browse tab + await manager.transition({ + type: "SET_ACTIVE_TAB", + payload: { tab: "browse" }, + }) + + // Should trigger fetch due to source modification + expect(vscode.postMessage).toHaveBeenCalledWith({ + type: "fetchPackageManagerItems", + bool: true, + }) + }) + it("should not trigger fetch when switching to sources tab", async () => { await manager.transition({ type: "SET_ACTIVE_TAB", @@ -570,7 +620,7 @@ describe("PackageManagerViewStateManager", () => { sources: [ { url: "https://github.com/RooVetGit/Roo-Code-Packages", - name: "Roo Code Package Manager Template", + name: "Roo Code", enabled: true, }, ],