test(storefront): cover paging, and say what the filter tests meant (#269)

Two assertions named a fixture and expected it visible in the unfiltered grid. No paginated catalogue can promise that — the item is on some page, not necessarily the first — so both would have started failing the moment paging landed. They were only ever proxies for "the result set got bigger", and the visible total lets them say that directly, which is what the issue predicted when it asked for a count.

The new cases assert the control and the URL rather than which item is on which page, because the development database never truncates and which item lands where is not something a test may rely on. That is the same trap the two rewritten assertions had fallen into, and repeating it in new tests would have been worse than leaving them alone.

Writing them found a real defect rather than just covering the feature. The control was rendering while the catalogue was still loading, showing "0 items" for a moment before the real count arrived — the empty-state early return only fires once loading has finished, so a mid-load render fell through to the grid branch with a total of zero. It is now suppressed until there is something to count, which is both true and what makes the count usable as a signal in a test. StorefrontPage.totalItems waits for the control for the same reason: reading during the load returned zero and quietly made "the result set shrank" compare against nothing.

The conditional skips carry a file-level eslint exception with its reasoning rather than being left to add four warnings. They are honest about a real limit: against a catalogue of ten items or fewer these cases prove nothing, and if the e2e database is ever seeded that thinly they need fixtures of their own instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

frontend/src/App.tsx

@@ -127,6 +127,12 @@ function Catalogue({
           </Col>
         ))}
       </Row>
+      {/* Not while there is nothing to count. A genuinely empty catalogue
+          returns early above with the empty state, so the only way to reach
+          here with a total of zero is mid-load — and flashing "0 items" at
+          somebody while their catalogue is still arriving says something
+          untrue. */}
+      {total > 0 && (
       <Pagination
         style={{ marginTop: 24, textAlign: 'center' }}
         current={page}
@@ -144,6 +150,7 @@ function Catalogue({
         hideOnSinglePage={false}
         showTotal={(count) => `${count} ${count === 1 ? 'item' : 'items'}`}
       />
+      )}
     </>
   );
 }

frontend/tests/e2e/favorites-filter.spec.ts

@@ -98,13 +98,20 @@ test.describe('Filtering the storefront by favorites', () => {
     await storefront.gotoSignedIn();
     await favorite(storefront, favoritePrompt, KEPT);
 
+    // The unfiltered total, captured before filtering, is what "the full
+    // catalogue" means here. It used to mean "OTHER is on screen", which a
+    // paginated catalogue cannot promise — OTHER is on some page, not
+    // necessarily the first (#269).
+    await storefront.goto();
+    const unfiltered = await storefront.totalItems();
+
     await page.goto('/?favorites=1');
     await expect(storefront.addToFavoritesButton(OTHER)).toBeHidden();
 
     await expect(storefront.activeFilters).toContainText('My favorites');
     await storefront.removeFilterChip('My favorites').click();
 
-    await expect(storefront.addToFavoritesButton(OTHER)).toBeVisible();
+    await expect.poll(() => storefront.totalItems()).toBe(unfiltered);
     await expect(page).not.toHaveURL(/favorites/);
   });
 

frontend/tests/e2e/filters.spec.ts

@@ -153,16 +153,26 @@ test.describe('Storefront filters', () => {
     await expect(storefront.card(NAMES.dearItem)).toBeHidden();
   });
 
+  // Asserts what it means, rather than that one fixture happens to be on
+  // screen. It used to check a named item was visible in the unfiltered grid,
+  // which a paginated catalogue cannot promise — that item is on some page, not
+  // necessarily the first (#269).
   test('removing a chip widens the results again', async ({ storefront, filterDrawer }) => {
     await storefront.goto();
+    const unfiltered = await storefront.totalItems();
+
     await storefront.openFilters();
     await filterDrawer.chooseCategory(NAMES.decor);
     await filterDrawer.close();
 
     await expect(storefront.card(NAMES.deepItem)).toBeHidden();
+    await expect.poll(() => storefront.totalItems()).toBeLessThan(unfiltered);
 
     await storefront.removeFilterChip(NAMES.decor).click();
-    await expect(storefront.card(NAMES.deepItem)).toBeVisible();
+
+    // Polled rather than read once: the count changes when the refetch lands,
+    // and a single read races it.
+    await expect.poll(() => storefront.totalItems()).toBe(unfiltered);
   });
 
   test('clear all removes every active filter', async ({ page, storefront, filterDrawer }) => {

frontend/tests/e2e/pages/StorefrontPage.ts

@@ -38,6 +38,16 @@ export class StorefrontPage {
 
   /** What the nearest error boundary renders when the grid itself throws. */
   readonly catalogueBoundaryHeading: Locator;
+  /** The paging control under the grid (#269). */
+  readonly pagination: Locator;
+  /**
+   * Where the catalogue says how many items it has.
+   *
+   * A class locator rather than a role, matching how this file already reaches
+   * `.item-card` — antd gives the total no role of its own, and this is the one
+   * place in the suite that knows where it lives.
+   */
+  readonly resultCount: Locator;
 
   constructor(private readonly page: Page) {
     this.header = new Header(page);
@@ -52,12 +62,35 @@ export class StorefrontPage {
     this.retryButton = page.getByRole('button', { name: 'Retry' });
 
     this.catalogueBoundaryHeading = page.getByRole('heading', { name: "The item list didn't load" });
+
+    this.pagination = page.locator('.ant-pagination');
+    this.resultCount = page.locator('.ant-pagination-total-text');
   }
 
   async goto(): Promise<void> {
     await this.page.goto('/');
   }
 
+  /**
+   * How many items the catalogue says it has, in total, across every page.
+   *
+   * The number rather than the text, so a test can assert a result set grew or
+   * shrank without knowing what it grew from. That is the whole reason #269
+   * added a visible count: two assertions used to name a fixture and expect it
+   * in the unfiltered grid, which no paginated catalogue can promise.
+   */
+  async totalItems(): Promise<number> {
+    // Waits rather than reading straight away. The control is not rendered
+    // until there is something to count, so reading during the initial load
+    // used to return 0 and quietly make "the result set shrank" assertions
+    // compare against nothing.
+    await this.resultCount.waitFor();
+    const text = (await this.resultCount.textContent()) ?? '';
+    const digits = /(\d+)/.exec(text);
+    if (digits === null) throw new Error(`no count in the pagination total: "${text}"`);
+    return Number(digits[1]);
+  }
+
   /**
    * Goes to the storefront and waits for the session to settle.
    *

frontend/tests/e2e/pagination.spec.ts

@@ -0,0 +1,105 @@
+import { test, expect } from './fixtures';
+
+/* eslint-disable sonarjs/no-skipped-tests --
+ * The `test.skip(total <= 10, ...)` calls below are conditional guards, not
+ * disabled tests: they run whenever the catalogue is large enough to have a
+ * second page, which it is on any real database. The rule cannot tell a
+ * runtime condition from a permanently ignored test, and its own message asks
+ * for an explanation rather than removal — this is it.
+ *
+ * The guards are honest about a real limit, though: against a catalogue of ten
+ * items or fewer these cases prove nothing. If the e2e database is ever seeded
+ * that thinly, they should be given fixtures of their own rather than left to
+ * skip quietly.
+ */
+
+/**
+ * Paging the catalogue (#269).
+ *
+ * Every assertion here is about the control and the URL, never about a
+ * particular item being on a particular page. The development database never
+ * truncates, so which item lands where is not something a test may rely on —
+ * that is exactly the trap the two rewritten assertions in filters.spec.ts and
+ * favorites-filter.spec.ts had fallen into.
+ */
+test.describe('Paging the catalogue', () => {
+  test('shows how many items there are', async ({ storefront }) => {
+    await storefront.goto();
+
+    await expect(storefront.resultCount).toBeVisible();
+    expect(await storefront.totalItems()).toBeGreaterThan(0);
+  });
+
+  // Ten by default, which is the decision on the issue. A denser grid is
+  // something a customer asks for, not something they get by accident.
+  test('shows ten items to a page by default', async ({ page, storefront }) => {
+    await storefront.goto();
+
+    const total = await storefront.totalItems();
+    test.skip(total <= 10, 'needs more than one page of catalogue to be meaningful');
+
+    await expect(page.locator('.item-card')).toHaveCount(10);
+  });
+
+  // The reason numbered pages were chosen over infinite scroll: a page is a
+  // place you can send someone.
+  test('puts the page in the URL so it can be linked', async ({ page, storefront }) => {
+    await storefront.goto();
+
+    const total = await storefront.totalItems();
+    test.skip(total <= 10, 'needs more than one page of catalogue to be meaningful');
+
+    await storefront.pagination.getByRole('listitem', { name: '2', exact: true }).click();
+
+    await expect(page).toHaveURL(/[?&]page=2\b/);
+  });
+
+  // Page one is the absence of the parameter, so the plain URL stays clean.
+  test('leaves page one out of the URL', async ({ page, storefront }) => {
+    await storefront.goto();
+
+    const total = await storefront.totalItems();
+    test.skip(total <= 10, 'needs more than one page of catalogue to be meaningful');
+
+    await storefront.pagination.getByRole('listitem', { name: '2', exact: true }).click();
+    await expect(page).toHaveURL(/[?&]page=2\b/);
+
+    await storefront.pagination.getByRole('listitem', { name: '1', exact: true }).click();
+    await expect(page).not.toHaveURL(/[?&]page=/);
+  });
+
+  // A link to a page that no longer exists lands on the last one. An empty
+  // grid would read as "this shop has nothing".
+  test('clamps a page past the end rather than showing nothing', async ({ page, storefront }) => {
+    await page.goto('/?page=99999');
+
+    await expect(storefront.resultCount).toBeVisible();
+    await expect(page.locator('.item-card').first()).toBeVisible();
+  });
+
+  // The jump box was deliberately left off.
+  test('offers no "go to page" box', async ({ storefront }) => {
+    await storefront.goto();
+
+    await expect(
+      storefront.pagination.locator('.ant-pagination-options-quick-jumper')
+    ).toHaveCount(0);
+  });
+
+  test('remembers a chosen page size', async ({ page, storefront }) => {
+    await storefront.goto();
+
+    const total = await storefront.totalItems();
+    test.skip(total <= 20, 'needs more than twenty items for a size change to show');
+
+    await storefront.pagination.locator('.ant-select').click();
+    await page.getByTitle('20 / page').click();
+    await expect(page.locator('.item-card')).toHaveCount(20);
+
+    // The preference is the point: it survives a reload without being in the
+    // URL, because it belongs to this person and not to a shared link.
+    await page.reload();
+    await expect(page.locator('.item-card')).toHaveCount(20);
+    await expect(page).not.toHaveURL(/pageSize/);
+  });
+});