From 6b1014528c1504bd497919c9b3e75373ae9a5507 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 20 Aug 2025 13:42:28 +0000 Subject: [PATCH 1/5] fix: fix workspaces pagination --- site/src/api/api.ts | 4 +- site/src/api/queries/workspaces.ts | 11 ++- .../WorkspacesPage/WorkspacesPage.test.tsx | 78 +++++++++++++++++++ 3 files changed, 85 insertions(+), 8 deletions(-) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 966c8902c3e73..7bad235d6bf25 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -1187,9 +1187,9 @@ class ApiMethods { }; getWorkspaces = async ( - options: TypesGen.WorkspacesRequest, + req: TypesGen.WorkspacesRequest, ): Promise => { - const url = getURLWithSearchParams("/api/v2/workspaces", options); + const url = getURLWithSearchParams("/api/v2/workspaces", req); const response = await this.axios.get(url); return response.data; }; diff --git a/site/src/api/queries/workspaces.ts b/site/src/api/queries/workspaces.ts index bcfb07b75452b..1c3e82a8816c2 100644 --- a/site/src/api/queries/workspaces.ts +++ b/site/src/api/queries/workspaces.ts @@ -139,15 +139,14 @@ async function findMatchWorkspace(q: string): Promise { } } -function workspacesKey(config: WorkspacesRequest = {}) { - const { q, limit } = config; - return ["workspaces", { q, limit }] as const; +function workspacesKey(req: WorkspacesRequest = {}) { + return ["workspaces", req] as const; } -export function workspaces(config: WorkspacesRequest = {}) { +export function workspaces(req: WorkspacesRequest = {}) { return { - queryKey: workspacesKey(config), - queryFn: () => API.getWorkspaces(config), + queryKey: workspacesKey(req), + queryFn: () => API.getWorkspaces(req), } as const satisfies QueryOptions; } diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx index 988e9a5385098..9cfd9a7dc96d0 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx @@ -305,6 +305,84 @@ describe("WorkspacesPage", () => { MockStoppedWorkspace.latest_build.template_version_id, ); }); + + it("correctly handles pagination by including pagination parameters in query key", async () => { + // Create enough workspaces to require pagination + const totalWorkspaces = 50; + const workspacesPage1 = Array.from({ length: 25 }, (_, i) => ({ + ...MockWorkspace, + id: `page1-workspace-${i}`, + name: `page1-workspace-${i}`, + })); + const workspacesPage2 = Array.from({ length: 25 }, (_, i) => ({ + ...MockWorkspace, + id: `page2-workspace-${i}`, + name: `page2-workspace-${i}`, + })); + + const getWorkspacesSpy = jest.spyOn(API, "getWorkspaces"); + + // Mock responses for both pages + getWorkspacesSpy + .mockResolvedValueOnce({ + workspaces: workspacesPage1, + count: totalWorkspaces, + }) + .mockResolvedValueOnce({ + workspaces: workspacesPage2, + count: totalWorkspaces, + }); + + const user = userEvent.setup(); + renderWithAuth(); + + // Wait for first page to load + await waitFor(() => { + expect(screen.getByText("page1-workspace-0")).toBeInTheDocument(); + }); + + // Verify first API call was made with correct pagination parameters + expect(getWorkspacesSpy).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + q: "owner:me", + offset: 0, + limit: 25, + }), + ); + + // Navigate to page 2 + const nextPageButton = screen.getByRole("button", { name: /next page/i }); + await user.click(nextPageButton); + + // Wait for second page to load + await waitFor(() => { + expect(screen.getByText("page2-workspace-0")).toBeInTheDocument(); + }); + + // Verify second API call was made with updated pagination parameters + expect(getWorkspacesSpy).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + q: "owner:me", + offset: 25, + limit: 25, + }), + ); + + // Verify the calls were made with different parameters (fixing the pagination bug) + expect(getWorkspacesSpy).toHaveBeenCalledTimes(2); + + // Ensure first page content is no longer visible + expect(screen.queryByText("page1-workspace-0")).not.toBeInTheDocument(); + + // Verify the key difference: the offset changed between calls + const firstCallArgs = getWorkspacesSpy.mock.calls[0][0]; + const secondCallArgs = getWorkspacesSpy.mock.calls[1][0]; + expect(firstCallArgs.offset).toBe(0); + expect(secondCallArgs.offset).toBe(25); + expect(firstCallArgs.offset).not.toBe(secondCallArgs.offset); + }); }); const getWorkspaceCheckbox = (workspace: Workspace) => { From c63fc7da8ecec700075b192e30ac7a3fed9b7bb1 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 20 Aug 2025 14:40:44 +0000 Subject: [PATCH 2/5] improve tests --- .../WorkspacesPage/WorkspacesPage.test.tsx | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx index 9cfd9a7dc96d0..4fc97d9bdecc4 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx @@ -307,7 +307,6 @@ describe("WorkspacesPage", () => { }); it("correctly handles pagination by including pagination parameters in query key", async () => { - // Create enough workspaces to require pagination const totalWorkspaces = 50; const workspacesPage1 = Array.from({ length: 25 }, (_, i) => ({ ...MockWorkspace, @@ -322,7 +321,6 @@ describe("WorkspacesPage", () => { const getWorkspacesSpy = jest.spyOn(API, "getWorkspaces"); - // Mock responses for both pages getWorkspacesSpy .mockResolvedValueOnce({ workspaces: workspacesPage1, @@ -336,12 +334,10 @@ describe("WorkspacesPage", () => { const user = userEvent.setup(); renderWithAuth(); - // Wait for first page to load await waitFor(() => { expect(screen.getByText("page1-workspace-0")).toBeInTheDocument(); }); - // Verify first API call was made with correct pagination parameters expect(getWorkspacesSpy).toHaveBeenNthCalledWith( 1, expect.objectContaining({ @@ -351,16 +347,13 @@ describe("WorkspacesPage", () => { }), ); - // Navigate to page 2 const nextPageButton = screen.getByRole("button", { name: /next page/i }); await user.click(nextPageButton); - // Wait for second page to load await waitFor(() => { expect(screen.getByText("page2-workspace-0")).toBeInTheDocument(); }); - // Verify second API call was made with updated pagination parameters expect(getWorkspacesSpy).toHaveBeenNthCalledWith( 2, expect.objectContaining({ @@ -370,18 +363,7 @@ describe("WorkspacesPage", () => { }), ); - // Verify the calls were made with different parameters (fixing the pagination bug) - expect(getWorkspacesSpy).toHaveBeenCalledTimes(2); - - // Ensure first page content is no longer visible expect(screen.queryByText("page1-workspace-0")).not.toBeInTheDocument(); - - // Verify the key difference: the offset changed between calls - const firstCallArgs = getWorkspacesSpy.mock.calls[0][0]; - const secondCallArgs = getWorkspacesSpy.mock.calls[1][0]; - expect(firstCallArgs.offset).toBe(0); - expect(secondCallArgs.offset).toBe(25); - expect(firstCallArgs.offset).not.toBe(secondCallArgs.offset); }); }); From 7c4ce4da7b573f70d159e760708adc57c5f7cfee Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 20 Aug 2025 18:57:17 +0000 Subject: [PATCH 3/5] Make tests more reliable --- .../WorkspacesPage/WorkspacesPage.test.tsx | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx index 4fc97d9bdecc4..04246ad686bf3 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx @@ -321,15 +321,20 @@ describe("WorkspacesPage", () => { const getWorkspacesSpy = jest.spyOn(API, "getWorkspaces"); - getWorkspacesSpy - .mockResolvedValueOnce({ - workspaces: workspacesPage1, - count: totalWorkspaces, - }) - .mockResolvedValueOnce({ - workspaces: workspacesPage2, - count: totalWorkspaces, - }); + getWorkspacesSpy.mockImplementation(({ offset }) => { + if (offset === 0) { + return Promise.resolve({ + workspaces: workspacesPage1, + count: totalWorkspaces, + }); + } else if (offset === 25) { + return Promise.resolve({ + workspaces: workspacesPage2, + count: totalWorkspaces, + }); + } + return Promise.reject(new Error("Unexpected offset")); + }); const user = userEvent.setup(); renderWithAuth(); From ed86e7f50c297be6b196de5c9c340c5f74090d47 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 20 Aug 2025 19:01:57 +0000 Subject: [PATCH 4/5] Fix lint --- .../WorkspacesPage/WorkspacesPage.test.tsx | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx index 04246ad686bf3..e2016113ba7f9 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx @@ -322,18 +322,20 @@ describe("WorkspacesPage", () => { const getWorkspacesSpy = jest.spyOn(API, "getWorkspaces"); getWorkspacesSpy.mockImplementation(({ offset }) => { - if (offset === 0) { - return Promise.resolve({ - workspaces: workspacesPage1, - count: totalWorkspaces, - }); - } else if (offset === 25) { - return Promise.resolve({ - workspaces: workspacesPage2, - count: totalWorkspaces, - }); + switch (offset) { + case 0: + return Promise.resolve({ + workspaces: workspacesPage1, + count: totalWorkspaces, + }); + case 25: + return Promise.resolve({ + workspaces: workspacesPage2, + count: totalWorkspaces, + }); + default: + return Promise.reject(new Error("Unexpected offset")); } - return Promise.reject(new Error("Unexpected offset")); }); const user = userEvent.setup(); From 275116b3621bf23c604cae70f40d0948fb5b1c9e Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 21 Aug 2025 16:53:55 +0000 Subject: [PATCH 5/5] Improve tests --- .../WorkspacesPage/WorkspacesPage.test.tsx | 26 +++++++------------ .../pages/WorkspacesPage/WorkspacesPage.tsx | 3 ++- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx index e2016113ba7f9..b80da553de6d6 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx @@ -345,14 +345,11 @@ describe("WorkspacesPage", () => { expect(screen.getByText("page1-workspace-0")).toBeInTheDocument(); }); - expect(getWorkspacesSpy).toHaveBeenNthCalledWith( - 1, - expect.objectContaining({ - q: "owner:me", - offset: 0, - limit: 25, - }), - ); + expect(getWorkspacesSpy).toHaveBeenLastCalledWith({ + q: "owner:me", + offset: 0, + limit: 25, + }); const nextPageButton = screen.getByRole("button", { name: /next page/i }); await user.click(nextPageButton); @@ -361,14 +358,11 @@ describe("WorkspacesPage", () => { expect(screen.getByText("page2-workspace-0")).toBeInTheDocument(); }); - expect(getWorkspacesSpy).toHaveBeenNthCalledWith( - 2, - expect.objectContaining({ - q: "owner:me", - offset: 25, - limit: 25, - }), - ); + expect(getWorkspacesSpy).toHaveBeenLastCalledWith({ + q: "owner:me", + offset: 25, + limit: 25, + }); expect(screen.queryByText("page1-workspace-0")).not.toBeInTheDocument(); }); diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx index 62ed7bfed7fe4..0488fc0730e5d 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx @@ -116,7 +116,8 @@ const WorkspacesPage: FC = () => { }); const workspacesQueryOptions = workspaces({ - ...pagination, + limit: pagination.limit, + offset: pagination.offset, q: filterState.filter.query, }); const { data, error, refetch } = useQuery({