fix(ui): address review on orgs-projects url state

Keep /projects list paging as a pushed history entry, validate the
project key table page size against its offered options, and clear the
key table params through the table-state setters instead of a copied
key list.
This commit is contained in:
ryan-crabbe-berri 2026-09-16 10:25:27 -07:00
parent df2dd9b7f2
commit ef8e066c77
6 changed files with 104 additions and 20 deletions

View file

@ -189,6 +189,20 @@ describe("OrganizationsPanel - org detail deep link (?org=)", () => {
await expectQueryString("?org=org-plain");
});
it("closing the org detail keeps the list's search, filter, sort and page in the URL", async () => {
renderPanel({
searchParams:
"?org_search=Acme&filter_org_id=org-7&sort_by=spend&sort_order=asc&page=2&org=org-x&org_tab=members",
});
act(() => mockOrgInfoView.mock.calls.at(-1)?.[0].onClose());
await expectQueryString("?org_search=Acme&filter_org_id=org-7&sort_by=spend&sort_order=asc&page=2");
expect(onUrlUpdate).toHaveBeenCalledTimes(1);
expect(screen.getByPlaceholderText("Search by Organization Name")).toHaveValue("Acme");
expect(useOrganizationsSpy).toHaveBeenLastCalledWith({ org_id: "org-7", org_alias: "Acme" });
});
it("closing the org detail drops ?org_tab= together with ?org=", async () => {
renderPanel({ searchParams: "?org=org-from-url&org_tab=members" });

View file

@ -100,6 +100,34 @@ describe("ProjectKeysSection URL state (keys_ prefix)", () => {
expect(screen.getByTestId("pagination-page")).toHaveTextContent("Page 2 of 5");
});
it("should cap an oversized ?keys_page_size= at the largest offered page size", () => {
mockUseKeys.mockReturnValue(fortyTwoKeys);
renderWithProviders(<ProjectKeysSection projectId="proj-1" />, { searchParams: "?keys_page_size=500" });
expect(mockUseKeys).toHaveBeenLastCalledWith(1, 25, expect.anything());
expect(screen.getByTestId("pagination-page")).toHaveTextContent("Page 1 of 2");
});
it("should fall back to the default page size for a ?keys_page_size= outside the offered options", () => {
mockUseKeys.mockReturnValue(fortyTwoKeys);
renderWithProviders(<ProjectKeysSection projectId="proj-1" />, { searchParams: "?keys_page_size=7" });
expect(mockUseKeys).toHaveBeenLastCalledWith(1, 5, expect.anything());
expect(screen.getByTestId("pagination-page")).toHaveTextContent("Page 1 of 9");
});
it("should drop an unsupported ?keys_page_size= when the user pages forward", async () => {
const user = userEvent.setup();
mockUseKeys.mockReturnValue(fortyTwoKeys);
const onUrlUpdate = vi.fn<OnUrlUpdateFunction>();
renderWithProviders(<ProjectKeysSection projectId="proj-1" />, { searchParams: "?keys_page_size=7", onUrlUpdate });
await user.click(screen.getByTestId("pagination-next"));
await waitFor(() => expect(onUrlUpdate.mock.calls.at(-1)?.[0].queryString).toBe("?keys_page=2"));
expect(mockUseKeys).toHaveBeenLastCalledWith(2, 5, expect.anything());
});
it("should write the key name filter to ?keys_search= and return the keys to their first page", async () => {
mockUseKeys.mockReturnValue(fortyTwoKeys);
const onUrlUpdate = vi.fn<OnUrlUpdateFunction>();

View file

@ -8,7 +8,7 @@ import { KeyResponse } from "@/components/key_team_helpers/key_list";
import { DataTable } from "@/components/shared/DataTable";
import { getProjectKeysTableColumns } from "./ProjectKeysTableColumns";
import { PROJECT_KEYS_DEFAULT_PAGE_SIZE } from "./useProjectsUrlState";
import { PROJECT_KEYS_PAGE_SIZE_OPTIONS } from "./useProjectsUrlState";
interface ProjectKeysTableProps {
keys: KeyResponse[];
@ -19,8 +19,6 @@ interface ProjectKeysTableProps {
onPaginationChange: OnChangeFn<PaginationState>;
}
const PAGE_SIZE_OPTIONS = [PROJECT_KEYS_DEFAULT_PAGE_SIZE, 10, 25];
function EmptyState() {
return (
<div className="flex flex-col items-center gap-1 py-6">
@ -52,7 +50,7 @@ export function ProjectKeysTable({
pagination={pagination}
onPaginationChange={onPaginationChange}
rowCount={totalCount}
pageSizeOptions={PAGE_SIZE_OPTIONS}
pageSizeOptions={PROJECT_KEYS_PAGE_SIZE_OPTIONS}
isLoading={isLoading}
isError={isError}
loadingMessage="Loading keys…"

View file

@ -209,6 +209,7 @@ describe("ProjectsPage", () => {
expect(screen.getByTestId("pagination-page")).toHaveTextContent("Page 1 of 1");
});
await waitFor(() => expect(onUrlUpdate.mock.calls.at(-1)?.[0].queryString).toBe("?project_search=Project+01"));
expect(onUrlUpdate).toHaveBeenCalledTimes(2);
});
it("should restore the search box and filtered list from a ?project_search= deep link", () => {
@ -280,7 +281,8 @@ describe("ProjectsPage", () => {
const onUrlUpdate = vi.fn<(event: UrlUpdateEvent) => void>();
mockUseProjects.mockReturnValue({ data: mockProjects, isLoading: false });
renderWithProviders(<ProjectsPage />, {
searchParams: "?page=2&project_search=Project&project=proj-1&keys_page=3&keys_page_size=10&keys_search=prod",
searchParams:
"?page=2&project_search=Project&project=proj-1&keys_page=3&keys_page_size=10&keys_search=prod&keys_sort_by=spend&keys_sort_order=asc",
onUrlUpdate,
});

View file

@ -73,7 +73,7 @@ describe("ProjectsTable pagination URL state", () => {
expect(screen.getByTestId("pagination-range")).toHaveTextContent("Showing 11-14 of 14");
});
it("should write ?page=2 to the URL when the next page control is clicked", async () => {
it("should push ?page=2 onto history when the next page control is clicked", async () => {
const user = userEvent.setup();
const onUrlUpdate = vi.fn();
renderTable({ onUrlUpdate });
@ -84,6 +84,7 @@ describe("ProjectsTable pagination URL state", () => {
const [update] = onUrlUpdate.mock.calls[0];
expect(update.searchParams.get("page")).toBe("2");
expect(update.searchParams.has("page_size")).toBe(false);
expect(update.options.history).toBe("push");
expect(firstDataRow().getByText("Project 11")).toBeInTheDocument();
});
@ -147,6 +148,7 @@ describe("ProjectsTable pagination URL state", () => {
const lastUpdate = onUrlUpdate.mock.calls.at(-1)?.[0];
expect(lastUpdate.searchParams.get("page")).toBeNull();
expect(lastUpdate.searchParams.get("page_size")).toBe("25");
expect(lastUpdate.options.history).toBe("push");
});
it("should apply both params from a ?page=2&page_size=25 deep link so the restored view matches", () => {

View file

@ -1,12 +1,11 @@
import { functionalUpdate, type OnChangeFn, type PaginationState } from "@tanstack/react-table";
import { useUrlTableState, type UrlTableState, type UrlTableStateOptions } from "@/components/shared/DataTable";
import { parseAsString, useQueryStates } from "nuqs";
import { useCallback } from "react";
import { parseAsInteger, useQueryStates } from "nuqs";
import { useCallback, useMemo } from "react";
export const PROJECTS_DEFAULT_PAGE_SIZE = 10;
export const PROJECT_KEYS_DEFAULT_PAGE_SIZE = 5;
const PROJECT_KEYS_URL_PREFIX = "keys_";
const TABLE_STATE_URL_KEYS = ["search", "sort_by", "sort_order", "page", "page_size"] as const;
export const PROJECT_KEYS_PAGE_SIZE_OPTIONS = [PROJECT_KEYS_DEFAULT_PAGE_SIZE, 10, 25];
const PROJECTS_TABLE_STATE_OPTIONS: UrlTableStateOptions<never> = {
sortFields: [],
@ -16,24 +15,65 @@ const PROJECTS_TABLE_STATE_OPTIONS: UrlTableStateOptions<never> = {
urlKeys: { search: "project_search" },
};
const PROJECTS_PAGE_PARAMS = {
page: parseAsInteger.withDefault(1),
page_size: parseAsInteger.withDefault(PROJECTS_DEFAULT_PAGE_SIZE),
};
const PROJECT_KEYS_TABLE_STATE_OPTIONS: UrlTableStateOptions<never> = {
sortFields: [],
defaultSort: { id: "created_at", desc: true },
defaultPageSize: PROJECT_KEYS_DEFAULT_PAGE_SIZE,
maxPageSize: 25,
maxPageSize: Math.max(...PROJECT_KEYS_PAGE_SIZE_OPTIONS),
filterColumns: [],
keyPrefix: PROJECT_KEYS_URL_PREFIX,
keyPrefix: "keys_",
};
const PROJECT_KEYS_URL_STATE = Object.fromEntries(
TABLE_STATE_URL_KEYS.map((key) => [`${PROJECT_KEYS_URL_PREFIX}${key}`, parseAsString]),
);
export function useProjectsTableState(): UrlTableState {
const tableState = useUrlTableState(PROJECTS_TABLE_STATE_OPTIONS);
const [, setPageParams] = useQueryStates(PROJECTS_PAGE_PARAMS, { history: "push" });
const { pagination } = tableState;
export const useProjectsTableState = (): UrlTableState => useUrlTableState(PROJECTS_TABLE_STATE_OPTIONS);
const onPaginationChange = useCallback<OnChangeFn<PaginationState>>(
(updaterOrValue) => {
const next = functionalUpdate(updaterOrValue, pagination);
void setPageParams({ page: next.pageIndex + 1, page_size: next.pageSize });
},
[pagination, setPageParams],
);
export const useProjectKeysTableState = (): UrlTableState => useUrlTableState(PROJECT_KEYS_TABLE_STATE_OPTIONS);
return useMemo(() => ({ ...tableState, onPaginationChange }), [tableState, onPaginationChange]);
}
export function useProjectKeysTableState(): UrlTableState {
const tableState = useUrlTableState(PROJECT_KEYS_TABLE_STATE_OPTIONS);
const { pagination: urlPagination, onPaginationChange: writePagination } = tableState;
const pageSize = PROJECT_KEYS_PAGE_SIZE_OPTIONS.includes(urlPagination.pageSize)
? urlPagination.pageSize
: PROJECT_KEYS_DEFAULT_PAGE_SIZE;
const pagination = useMemo<PaginationState>(
() => ({ pageIndex: urlPagination.pageIndex, pageSize }),
[urlPagination.pageIndex, pageSize],
);
const onPaginationChange = useCallback<OnChangeFn<PaginationState>>(
(updaterOrValue) => writePagination(functionalUpdate(updaterOrValue, pagination)),
[pagination, writePagination],
);
return useMemo(
() => ({ ...tableState, pagination, onPaginationChange }),
[tableState, pagination, onPaginationChange],
);
}
export function useClearProjectKeysTableState(): () => void {
const [, setProjectKeysUrlState] = useQueryStates(PROJECT_KEYS_URL_STATE);
return useCallback(() => void setProjectKeysUrlState(null), [setProjectKeysUrlState]);
const { setSearch, onSortingChange, onColumnFiltersChange, onPaginationChange } = useProjectKeysTableState();
return useCallback(() => {
setSearch("");
onSortingChange([]);
onColumnFiltersChange([]);
onPaginationChange({ pageIndex: 0, pageSize: PROJECT_KEYS_DEFAULT_PAGE_SIZE });
}, [setSearch, onSortingChange, onColumnFiltersChange, onPaginationChange]);
}