Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/fix-content-list-stable-total.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"emdash": patch
"@emdash-cms/admin": patch
---

Fix the admin collection list pagination denominator so it no longer grows in increments of 5 as the user pages forward.

The `GET /_emdash/api/content/{collection}` response now includes a `total` field with the full filtered row count (independent of `limit`). The admin uses it as the pagination denominator, so a 143-entry collection reads `1/8` on page 1 instead of `1/5 → 5/10 → 10/15 → …` as successive API pages load.

The `total` field is optional; pre-upgrade clients that ignore it still work, and the admin falls back to the loaded-item count when an older server doesn't return it.

Also handles the edge case where the current page exceeds `totalPages` after filtering or deletion — the admin clamps the active page so the table doesn't render empty while waiting for a refetch.
103 changes: 82 additions & 21 deletions packages/admin/src/components/ContentList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ export interface ContentListProps {
*/
sort?: ContentListSort;
onSortChange?: (sort: ContentListSort) => void;
/**
* Total rows matching the current filters (ignoring pagination). When
* set, the pagination denominator reflects this stable count instead of
* growing as more API pages are fetched.
*/
total?: number;
}

type ViewTab = "all" | "trash";
Expand Down Expand Up @@ -104,6 +110,7 @@ export function ContentList({
urlPattern,
sort,
onSortChange,
total,
}: ContentListProps) {
const { t } = useLingui();
const [activeTab, setActiveTab] = React.useState<ViewTab>("all");
Expand All @@ -122,17 +129,37 @@ export function ContentList({
return items.filter((item) => getItemTitle(item).toLowerCase().includes(query));
}, [items, searchQuery]);

const totalPages = Math.max(1, Math.ceil(filteredItems.length / PAGE_SIZE));
const paginatedItems = filteredItems.slice(page * PAGE_SIZE, (page + 1) * PAGE_SIZE);
// When the server reports a total, it's the source of truth for the
// denominator. Otherwise fall back to the size of the (possibly partial)
// client list, matching pre-refactor behavior. Client-side search always
// defers to `filteredItems` because `total` reflects the unfiltered set.
const effectiveTotal = typeof total === "number" && !searchQuery ? total : filteredItems.length;
const totalPages = Math.max(1, Math.ceil(effectiveTotal / PAGE_SIZE));

// Clamp the current page in case filters collapse the count (user was on
// page 5 of 10, then typed a query narrowing to 1 page). Without clamping
// we'd render an empty table until the next refetch.
const clampedPage = Math.min(page, totalPages - 1);
const paginatedItems = filteredItems.slice(
clampedPage * PAGE_SIZE,
(clampedPage + 1) * PAGE_SIZE,
);

// Auto-fetch next API page when user reaches the last client-side page.
// skip when a search query is active
// filteredItems shrinking would otherwise collapse totalPages to 1 and trigger a spurious fetch
// Auto-fetch the next API page when the user is on a client page whose
// items haven't been loaded yet. Skip during client-side search because
// filtering can collapse `filteredItems` below the loaded count and
// trigger a spurious fetch.
//
// Safety: relies on `onLoadMore` being deduped against concurrent calls.
// The router wires this to TanStack Query's `fetchNextPage`, which is
// idempotent while a fetch is in flight.
React.useEffect(() => {
if (page >= totalPages - 1 && hasMore && onLoadMore && !searchQuery) {
if (!hasMore || !onLoadMore || searchQuery) return;
const loadedPages = Math.ceil(filteredItems.length / PAGE_SIZE);
if (clampedPage >= loadedPages - 1) {
onLoadMore();
}
}, [page, totalPages, hasMore, onLoadMore, searchQuery]);
}, [clampedPage, filteredItems.length, hasMore, onLoadMore, searchQuery]);

return (
<div className="space-y-4">
Expand Down Expand Up @@ -287,34 +314,31 @@ export function ContentList({
{totalPages > 1 && (
<div className="flex items-center justify-between">
<span className="text-sm text-kumo-subtle">
{searchQuery
? plural(filteredItems.length, {
one: `# item matching "${searchQuery}"`,
other: `# items matching "${searchQuery}"`,
})
: plural(filteredItems.length, {
one: `#${hasMore ? "+" : ""} item`,
other: `#${hasMore ? "+" : ""} items`,
})}
{renderItemCount({
searchQuery,
filteredCount: filteredItems.length,
total,
hasMore,
})}
</span>
<div className="flex items-center gap-2">
<Button
variant="outline"
shape="square"
disabled={page === 0}
onClick={() => setPage(page - 1)}
disabled={clampedPage === 0}
onClick={() => setPage(clampedPage - 1)}
aria-label={t`Previous page`}
>
<CaretPrev className="h-4 w-4" aria-hidden="true" />
</Button>
<span className="text-sm">
{page + 1} / {totalPages}
{clampedPage + 1} / {totalPages}
</span>
<Button
variant="outline"
shape="square"
disabled={page >= totalPages - 1}
onClick={() => setPage(page + 1)}
disabled={clampedPage >= totalPages - 1}
onClick={() => setPage(clampedPage + 1)}
aria-label={t`Next page`}
>
<CaretNext className="h-4 w-4" aria-hidden="true" />
Expand Down Expand Up @@ -459,6 +483,43 @@ function SortableTh({ field, sort, onSortChange, label }: SortableThProps) {
);
}

/**
* Render the row-count line above pagination. The rules are:
* - A search query always wins — say how many matches there are.
* - When the server reported a total, use it (no `+` suffix needed —
* we know the count).
* - Otherwise fall back to the pre-refactor behavior: loaded count,
* with `+` when there are more pages the user hasn't fetched yet.
*/
function renderItemCount({
searchQuery,
filteredCount,
total,
hasMore,
}: {
searchQuery: string;
filteredCount: number;
total: number | undefined;
hasMore: boolean | undefined;
}): string {
if (searchQuery) {
return plural(filteredCount, {
one: `# item matching "${searchQuery}"`,
other: `# items matching "${searchQuery}"`,
});
}
if (typeof total === "number") {
return plural(total, {
one: `# item`,
other: `# items`,
});
}
return plural(filteredCount, {
one: `#${hasMore ? "+" : ""} item`,
other: `#${hasMore ? "+" : ""} items`,
});
}

interface ContentListItemProps {
item: ContentItem;
collection: string;
Expand Down
5 changes: 5 additions & 0 deletions packages/admin/src/lib/api/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ export async function throwResponseError(res: Response, fallback: string): Promi
export interface FindManyResult<T> {
items: T[];
nextCursor?: string;
/**
* Total number of rows matching the filters (ignoring pagination).
* Optional because older servers may not return it.
*/
total?: number;
}

/**
Expand Down
6 changes: 6 additions & 0 deletions packages/admin/src/router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,11 @@ function ContentListPage() {
return data?.pages.flatMap((page) => page.items) || [];
}, [data]);

// Server returns `total` on every page; the first page is authoritative
// because filters don't change within a fetch cycle. Fall back to the
// loaded count so old servers (pre-total) still render a denominator.
const total = data?.pages[0]?.total ?? items.length;

if (!manifest) {
return <LoadingScreen />;
}
Expand Down Expand Up @@ -435,6 +440,7 @@ function ContentListPage() {
urlPattern={collectionConfig.urlPattern}
sort={sort}
onSortChange={setSort}
total={total}
/>
);
}
Expand Down
19 changes: 19 additions & 0 deletions packages/admin/tests/components/ContentList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,25 @@ describe("ContentList", () => {
// Post 0 should not be visible
expect(screen.getByText("Post 0").query()).toBeNull();
});

// Regression: before this change `totalPages` was derived only from
// loaded items, so the denominator grew in increments of 5 (API
// fetches 100, page size 20 → 5 client pages per fetch). When the
// parent supplies an authoritative `total`, the denominator must
// reflect it from the first render.
it("uses `total` as a stable denominator instead of items.length", async () => {
// Only the first 20 items have been loaded, but the server knows
// there are 143 total.
const items = Array.from({ length: 20 }, (_, i) =>
makeItem({ id: `item_${i}`, data: { title: `Post ${i}` } }),
);
const screen = await render(
<ContentList {...defaultProps} items={items} total={143} hasMore={true} />,
);

// 143 / 20 = 8 pages. The denominator should read 8, not "/5".
await expect.element(screen.getByText("1 / 8")).toBeInTheDocument();
});
});

describe("sortable headers", () => {
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/api/handlers/content.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ export async function handleContentList(
data: {
items: result.items,
nextCursor: result.nextCursor,
total: result.total,
},
};
} catch (error) {
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/api/schemas/content.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ export const contentListResponseSchema = z
.object({
items: z.array(contentItemSchema),
nextCursor: z.string().optional(),
total: z.number().int().nonnegative().optional(),
})
.meta({ id: "ContentListResponse" });

Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/api/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ import type { ContentItem } from "../database/repositories/types.js";
export interface ListResponse<T> {
items: T[];
nextCursor?: string;
/**
* Total number of rows matching the filters, ignoring pagination. Used
* by the admin to render a stable pagination denominator. Optional so
* callers that don't surface a row count don't pay for the extra query.
*/
total?: number;
}

/**
Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/database/repositories/content.ts
Original file line number Diff line number Diff line change
Expand Up @@ -518,12 +518,16 @@ export class ContentRepository {
.orderBy("id", safeOrderDirection === "ASC" ? "asc" : "desc")
.limit(limit + 1);

const rows = await query.execute();
// Run the page fetch and the unbounded count together — the UI needs
// both to render a stable denominator, and issuing them in parallel
// on SQLite is essentially free.
const [rows, total] = await Promise.all([query.execute(), this.count(type, options.where)]);
const hasMore = rows.length > limit;
const items = rows.slice(0, limit);

const mappedResult: FindManyResult<ContentItem> = {
items: items.map((row) => this.mapRow(type, row as Record<string, unknown>)),
total,
};

if (hasMore && items.length > 0) {
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/database/repositories/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ export interface FindManyOptions {
export interface FindManyResult<T> {
items: T[];
nextCursor?: string; // Base64-encoded JSON: {orderValue: string, id: string}
/**
* Total number of rows matching the where clause (ignoring pagination).
* Optional because not every caller needs it; repositories that compute
* it should set it so the UI can render a stable pagination denominator.
*/
total?: number;
}

/** Encode a cursor from order value + id */
Expand Down
33 changes: 33 additions & 0 deletions packages/core/tests/database/repositories/content.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,39 @@ describe("ContentRepository", () => {
).rejects.toThrow(EmDashValidationError);
});
});

describe("total", () => {
// Regression guard for the admin "denominator grows as you page
// forward" bug: each list response must include the full count so
// the UI doesn't have to reverse-engineer it from accumulated
// pages.
it("reports total rows regardless of limit", async () => {
const result = await repo.findMany("post", { limit: 2 });

expect(result.items).toHaveLength(2);
expect(result.total).toBe(5);
});

it("total respects the where clause", async () => {
const result = await repo.findMany("post", {
limit: 2,
where: { status: "published" },
});

expect(result.total).toBe(3);
});

it("total stays stable across cursor pages", async () => {
const page1 = await repo.findMany("post", { limit: 2 });
const page2 = await repo.findMany("post", {
limit: 2,
cursor: page1.nextCursor,
});

expect(page1.total).toBe(5);
expect(page2.total).toBe(5);
});
});
});

describe("update", () => {
Expand Down
28 changes: 28 additions & 0 deletions packages/core/tests/unit/api/content-handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,3 +305,31 @@ describe("Content Handlers — auto-slug generation", () => {
});
});
});

describe("Content Handlers — list total", () => {
let db: Kysely<Database>;

beforeEach(async () => {
db = await setupTestDatabaseWithCollections();
// Seed enough items that limit-based pagination kicks in and we can
// assert total > items.length.
for (let i = 0; i < 8; i++) {
const result = await handleContentCreate(db, "post", {
data: { title: `Post ${i}` },
});
if (!result.success) throw new Error("seed failed");
}
});

afterEach(async () => {
await teardownTestDatabase(db);
});

it("returns total independent of limit", async () => {
const result = await handleContentList(db, "post", { limit: 2 });

expect(result.success).toBe(true);
expect(result.data?.items).toHaveLength(2);
expect(result.data?.total).toBe(8);
});
});
Loading