diff --git a/site/src/api/queries/chats.test.ts b/site/src/api/queries/chats.test.ts index fc51f4602f..76da66347c 100644 --- a/site/src/api/queries/chats.test.ts +++ b/site/src/api/queries/chats.test.ts @@ -10,6 +10,7 @@ import { chatCostUsersKey, chatKey, chatsKey, + infiniteChats, unarchiveChat, } from "./chats"; @@ -17,6 +18,7 @@ vi.mock("api/api", () => ({ API: { archiveChat: vi.fn(), unarchiveChat: vi.fn(), + getChats: vi.fn(), getChatCostSummary: vi.fn(), getChatCostUsers: vi.fn(), }, @@ -328,3 +330,72 @@ describe("chat cost query factories", () => { expect(API.getChatCostUsers).toHaveBeenCalledWith(params); }); }); + +describe("infiniteChats", () => { + const PAGE_LIMIT = 50; + + describe("getNextPageParam", () => { + it("returns undefined when lastPage has fewer items than the limit", () => { + const { getNextPageParam } = infiniteChats(); + const lastPage = Array.from({ length: PAGE_LIMIT - 1 }, (_, i) => + makeChat(`chat-${i}`), + ); + expect(getNextPageParam(lastPage, [lastPage])).toBeUndefined(); + }); + + it("returns pages.length + 1 when lastPage has exactly the limit", () => { + const { getNextPageParam } = infiniteChats(); + const lastPage = Array.from({ length: PAGE_LIMIT }, (_, i) => + makeChat(`chat-${i}`), + ); + const pages = [lastPage]; + expect(getNextPageParam(lastPage, pages)).toBe(pages.length + 1); + }); + }); + + describe("queryFn", () => { + it("computes offset 0 for pageParam 0", async () => { + vi.mocked(API.getChats).mockResolvedValue([]); + const { queryFn } = infiniteChats(); + await queryFn({ pageParam: 0 }); + expect(API.getChats).toHaveBeenCalledWith({ + limit: PAGE_LIMIT, + offset: 0, + }); + }); + + it("computes offset 0 for pageParam <= 0", async () => { + vi.mocked(API.getChats).mockResolvedValue([]); + const { queryFn } = infiniteChats(); + await queryFn({ pageParam: -1 }); + expect(API.getChats).toHaveBeenCalledWith({ + limit: PAGE_LIMIT, + offset: 0, + }); + }); + + it("computes correct offset for subsequent pages", async () => { + vi.mocked(API.getChats).mockResolvedValue([]); + const { queryFn } = infiniteChats(); + + await queryFn({ pageParam: 2 }); + expect(API.getChats).toHaveBeenCalledWith({ + limit: PAGE_LIMIT, + offset: PAGE_LIMIT, + }); + + await queryFn({ pageParam: 3 }); + expect(API.getChats).toHaveBeenCalledWith({ + limit: PAGE_LIMIT, + offset: PAGE_LIMIT * 2, + }); + }); + + it("throws when pageParam is not a number", () => { + const { queryFn } = infiniteChats(); + expect(() => queryFn({ pageParam: "bad" })).toThrow( + "pageParam must be a number", + ); + }); + }); +}); diff --git a/site/src/pages/AgentsPage/AgentsSidebar.test.tsx b/site/src/pages/AgentsPage/AgentsSidebar.test.tsx new file mode 100644 index 0000000000..a8c8bad04a --- /dev/null +++ b/site/src/pages/AgentsPage/AgentsSidebar.test.tsx @@ -0,0 +1,276 @@ +import { + MockAppearanceConfig, + MockBuildInfo, + MockDefaultOrganization, + MockEntitlements, + MockUserOwner, +} from "testHelpers/entities"; +import { act, render } from "@testing-library/react"; +import type * as TypesGen from "api/typesGenerated"; +import type { Chat } from "api/typesGenerated"; +import { ThemeOverride } from "contexts/ThemeProvider"; +import { DashboardContext } from "modules/dashboard/DashboardProvider"; +import type { FC, PropsWithChildren } from "react"; +import { QueryClient, QueryClientProvider } from "react-query"; +import { MemoryRouter } from "react-router"; +import themes, { DEFAULT_THEME } from "theme"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { AgentsSidebar } from "./AgentsSidebar"; + +// ---- IntersectionObserver mock ---- + +type IOCallback = (entries: Array<{ isIntersecting: boolean }>) => void; +let observerCallback: IOCallback | null = null; +let observeCount = 0; + +class MockIntersectionObserver { + observe = vi.fn(() => { + observeCount++; + }); + disconnect = vi.fn(); + unobserve = vi.fn(); + + constructor(cb: IOCallback) { + observerCallback = cb; + } +} + +// ---- Auth mock ---- + +vi.mock("hooks", async () => { + const actual = await vi.importActual("hooks"); + return { + ...actual, + useAuthenticated: () => ({ + user: MockUserOwner, + permissions: {}, + signOut: vi.fn(), + }), + }; +}); + +// ---- Helpers ---- + +const oneWeekAgo = new Date(Date.now() - 7 * 24 * 60 * 60 * 1000).toISOString(); + +const buildChat = (overrides: Partial = {}): Chat => ({ + id: "chat-default", + owner_id: "owner-1", + title: "Agent", + status: "completed", + last_model_config_id: "model-1", + created_at: oneWeekAgo, + updated_at: oneWeekAgo, + archived: false, + last_error: null, + ...overrides, +}); + +const dashboardValue = { + entitlements: MockEntitlements, + experiments: [] as TypesGen.Experiment[], + appearance: MockAppearanceConfig, + buildInfo: MockBuildInfo, + organizations: [MockDefaultOrganization], + showOrganizations: false, + canViewOrganizationSettings: false, +}; + +const Wrapper: FC = ({ children }) => { + const queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false, refetchOnWindowFocus: false }, + }, + }); + return ( + + + + + {children} + + + + + ); +}; + +const defaultProps: React.ComponentProps = { + chats: [buildChat({ id: "chat-1", title: "Chat One" })], + chatErrorReasons: {}, + modelOptions: [], + modelConfigs: [], + onArchiveAgent: vi.fn(), + onUnarchiveAgent: vi.fn(), + onArchiveAndDeleteWorkspace: vi.fn(), + onNewAgent: vi.fn(), + isCreating: false, + archivedFilter: "active" as const, +}; + +// ---- Tests ---- + +describe("AgentsSidebar load-more behavior", () => { + beforeEach(() => { + observerCallback = null; + observeCount = 0; + vi.stubGlobal("IntersectionObserver", MockIntersectionObserver); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("calls onLoadMore when the sentinel becomes visible", () => { + const onLoadMore = vi.fn(); + render( + + + , + ); + + act(() => { + observerCallback?.([{ isIntersecting: true }]); + }); + + expect(onLoadMore).toHaveBeenCalledTimes(1); + }); + + it("does NOT call onLoadMore when isFetchingNextPage is true", () => { + const onLoadMore = vi.fn(); + render( + + + , + ); + + act(() => { + observerCallback?.([{ isIntersecting: true }]); + }); + + expect(onLoadMore).not.toHaveBeenCalled(); + }); + + it("does NOT recreate the observer when re-rendered with a new onLoadMore reference", () => { + const onLoadMore1 = vi.fn(); + const { rerender } = render( + + + , + ); + + const countAfterMount = observeCount; + + // Re-render with a brand-new function reference, which was the + // original bug trigger. + const onLoadMore2 = vi.fn(); + rerender( + + + , + ); + + // The observer should NOT have been torn down and recreated. + expect(observeCount).toBe(countAfterMount); + + // The new callback should still be the one invoked. + act(() => { + observerCallback?.([{ isIntersecting: true }]); + }); + expect(onLoadMore1).not.toHaveBeenCalled(); + expect(onLoadMore2).toHaveBeenCalledTimes(1); + }); + + it("does NOT spam onLoadMore across multiple re-renders", () => { + const onLoadMore = vi.fn(); + const { rerender } = render( + + + , + ); + + // Sentinel becomes visible once. + act(() => { + observerCallback?.([{ isIntersecting: true }]); + }); + expect(onLoadMore).toHaveBeenCalledTimes(1); + + // Parent re-renders many times with new inline arrow callbacks + // (the pattern that caused the original bug). + for (let i = 0; i < 10; i++) { + rerender( + + onLoadMore()} + /> + , + ); + } + + // Re-renders alone should NOT trigger additional onLoadMore calls; + // only a new IntersectionObserver entry should. + expect(onLoadMore).toHaveBeenCalledTimes(1); + }); + + it("resumes loading after isFetchingNextPage goes from true to false", () => { + const onLoadMore = vi.fn(); + const { rerender } = render( + + + , + ); + + // Blocked while fetching. + act(() => { + observerCallback?.([{ isIntersecting: true }]); + }); + expect(onLoadMore).not.toHaveBeenCalled(); + + // Fetch completes. + rerender( + + + , + ); + + // Observer fires again while sentinel is still visible. + act(() => { + observerCallback?.([{ isIntersecting: true }]); + }); + expect(onLoadMore).toHaveBeenCalledTimes(1); + }); + + it("does NOT render the sentinel when hasNextPage is false", () => { + const onLoadMore = vi.fn(); + render( + + + , + ); + + // No observer should have been created since the sentinel + // is not rendered. + expect(observeCount).toBe(0); + }); +}); diff --git a/site/src/pages/AgentsPage/AgentsSidebar.tsx b/site/src/pages/AgentsPage/AgentsSidebar.tsx index 6881bc1d78..d0a602bf9a 100644 --- a/site/src/pages/AgentsPage/AgentsSidebar.tsx +++ b/site/src/pages/AgentsPage/AgentsSidebar.tsx @@ -903,22 +903,39 @@ const LoadMoreSentinel: FC<{ isFetchingNextPage?: boolean; }> = ({ onLoadMore, isFetchingNextPage }) => { const sentinelRef = useRef(null); + const onLoadMoreRef = useRef(onLoadMore); + const isFetchingNextPageRef = useRef(isFetchingNextPage); + + // Keep refs in sync with the latest prop values so the + // observer callback always reads current state without + // needing to tear down and re-create the observer. + useEffect(() => { + onLoadMoreRef.current = onLoadMore; + }, [onLoadMore]); + + useEffect(() => { + isFetchingNextPageRef.current = isFetchingNextPage; + }, [isFetchingNextPage]); useEffect(() => { const el = sentinelRef.current; - if (!el || !onLoadMore) return; + if (!el) return; const observer = new IntersectionObserver( (entries) => { - if (entries[0]?.isIntersecting) { - onLoadMore(); + if ( + entries[0]?.isIntersecting && + !isFetchingNextPageRef.current && + onLoadMoreRef.current + ) { + onLoadMoreRef.current(); } }, { threshold: 0 }, ); observer.observe(el); return () => observer.disconnect(); - }, [onLoadMore]); + }, []); return (