fix(admin): refetch Inventory when its tab becomes visible (#327)

Two items published from the Review queue did not appear in Inventory. Publishing was never at fault: the items were published, the rows were right, and the API returned them. The Inventory tab was showing a list it had fetched earlier and never refreshed.

antd keeps a tab pane mounted once it has been rendered, and Inventory is the default tab, so its pane mounts at page load whether or not anyone looks at it. Its only fetch runs from an effect depending on the filters, so it fires on mount and when a filter changes and at no other time. Open the admin, switch to the Review queue, publish, switch back, and nothing has re-run — the table still holds the list built before the submissions existed. A browser reload shows them, which is what makes this read as publishing being broken rather than as a stale table.

The tab is controlled now and tells Inventory whether it is the one on screen, and Inventory refetches when that becomes true. Guarded on visibility rather than fetching unconditionally, because the pane lives for the life of the page and would otherwise keep refetching while hidden.

destroyInactiveTabPane on the Tabs would also have fixed it, by remounting, and was rejected: it discards every tab's state on every switch — filters, scroll position, a half-filled form — and refetches all of them repeatedly, which is a much larger behavioural change than this bug is worth.

The existing publish test passes against the broken behaviour, and that is the reason this went unnoticed. It asserts against GET /api/admin/items, and the API was always correct. The new test asserts through the UI and never reloads the page: Inventory is opened before publishing, so its list is fetched while the item still carries its submission-timestamp name, and the assertion afterwards looks for the name given at publish — which a stale list cannot contain. A reload anywhere in it would make it pass against the bug it exists to catch.

AdminPage's comment claimed only the active panel is mounted. It is only the active panel that is visible; the rest stay in the DOM. That mistaken belief is the shape of this bug, so the comment now says which it is and why it matters.

Not fixed here: Categories, Tags, Upload links and Customers are mounted-once children of the same Tabs and are all changeable from elsewhere, so they very likely share this. Recorded on the issue rather than assumed to be fine.

Verified: tsc clean for src and tests, lint 0 errors, production build green. The new test needs CI to run — the e2e suite wants a browser and a database this machine cannot provide.

Closes #327

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

frontend/src/admin/Admin.tsx

@@ -51,7 +51,22 @@ const { Title } = Typography;
 // not a state of its own worth drawing the eye to.
 const STATUS_TAG_COLORS: Record<string, string> = { sold: 'red', reserved: 'orange', pending: 'default' };
 
-function Inventory() {
+/**
+ * `active` is whether the Inventory tab is the one on screen (#327).
+ *
+ * antd keeps a tab pane mounted once it has been rendered, and this pane is
+ * rendered at page load because Inventory is the default tab. So switching away,
+ * publishing something from the Review queue, and switching back re-runs no
+ * effect and refetches nothing: the table goes on showing the list it built when
+ * the admin was first opened. It read as publishing being broken, because the
+ * item really was published and really was absent from the list.
+ *
+ * Refetching when the tab becomes visible is the narrow fix. Setting
+ * `destroyInactiveTabPane` on the Tabs would also work, by remounting — but it
+ * discards every tab's state on every switch, filters and half-filled forms
+ * included, which is a much larger change than this bug is worth.
+ */
+function Inventory({ active }: Readonly<{ active: boolean }>) {
   const [items, setItems] = useState<Item[]>([]);
   const [modalOpen, setModalOpen] = useState(false);
   const [editingItem, setEditingItem] = useState<Item | null>(null);
@@ -108,8 +123,17 @@ function Inventory() {
   ]).catch(() => message.error('Could not load categories and tags')), []);
 
   // Refetch whenever the filters change — filtering is server-side so the
-  // result stays correct regardless of how many items exist.
-  useEffect(() => { void load(filters); }, [load, filters]);
+  // result stays correct regardless of how many items exist — and whenever this
+  // tab becomes the visible one, because anything published, discarded or
+  // edited from a sibling tab happened while this list sat untouched (#327).
+  //
+  // Guarded on `active` rather than fetching unconditionally: this pane stays
+  // mounted for the life of the page, so without the guard every filter change
+  // would still refetch while the tab is hidden and nobody is looking.
+  useEffect(() => {
+    if (!active) return;
+    void load(filters);
+  }, [active, load, filters]);
   useEffect(() => { void loadOptions(); }, [loadOptions]);
 
   function applyFilters(next: ItemFilters) { setFilters(next); }
@@ -481,6 +505,10 @@ function Inventory() {
 export default function Admin() {
   const { mode, toggle } = useThemeMode();
   const { token } = theme.useToken();
+  // Controlled rather than defaultActiveKey, so a pane can be told whether it is
+  // the one on screen. Panes stay mounted here, so "visible" is not something a
+  // child can work out for itself (#327).
+  const [activeTab, setActiveTab] = useState('inventory');
 
   return (
     <Layout style={{ minHeight: '100vh' }}>
@@ -493,9 +521,10 @@ export default function Admin() {
       </Header>
       <Content style={{ padding: 24 }}>
         <Tabs
-          defaultActiveKey="inventory"
+          activeKey={activeTab}
+          onChange={setActiveTab}
           items={[
-            { key: 'inventory', label: 'Inventory', children: <Inventory /> },
+            { key: 'inventory', label: 'Inventory', children: <Inventory active={activeTab === 'inventory'} /> },
             { key: 'categories', label: 'Categories', children: <Categories /> },
             { key: 'tags', label: 'Tags', children: <Tags /> },
             { key: 'upload-links', label: 'Upload links', children: <UploadLinks /> },

frontend/tests/e2e/admin-draft-queue.spec.ts

@@ -144,4 +144,53 @@ test.describe('The review queue', () => {
     await card.getByRole('button', { name: 'Rotate right' }).click();
     expect((await rotated).status()).toBe(204);
   });
+
+  /**
+   * The bug reported from QA in #327: two items were published and did not
+   * appear in Inventory.
+   *
+   * The publish itself was never at fault, which is why the test above passes —
+   * it asserts against `GET /api/admin/items`, and the API was always right.
+   * What was wrong was the Inventory tab: antd keeps a pane mounted once it has
+   * been rendered, so a tab opened at page load and returned to later refetches
+   * nothing and shows the list it built the first time.
+   *
+   * So this asserts through the UI and, crucially, **never reloads the page**.
+   * Opening Inventory before publishing is what arms it: the list is fetched
+   * while the item still has its submission-timestamp name, and the assertion
+   * afterwards looks for the name given at publish — which a stale list cannot
+   * contain. A `page.reload()` anywhere in here would make it pass against the
+   * broken behaviour.
+   */
+  test('an item published from the queue appears in Inventory without a reload', async ({
+    page,
+    admin
+  }) => {
+    const note = `Tab staleness ${RUN}`;
+    const publishedName = `Vase ${RUN}`;
+    await submitAnItem(page, note);
+
+    // Inventory first, so its pane is mounted and its list fetched before the
+    // publish happens. This is the state a real admin is in: the tab has been
+    // open since they arrived.
+    await admin.open('Inventory');
+    await expect(admin.activeTable).toBeVisible();
+
+    await admin.openTab('Review queue');
+    const card = page.locator('.ant-card').filter({ hasText: note });
+    await expect(card).toBeVisible();
+
+    await card.getByLabel('Name').fill(publishedName);
+    // Setting a price confirms it, so publishing does not stop on the
+    // unconfirmed-price dialog the test above covers.
+    await card.getByLabel('Price').fill('42');
+    await card.getByRole('button', { name: 'Publish', exact: true }).click();
+    await expect(card.getByText('you set this price')).toBeVisible();
+
+    await admin.openTab('Inventory');
+
+    // Sorted by created_at descending and submitted moments ago, so it is on the
+    // first page rather than somewhere in the accumulated dev database.
+    await expect(admin.activePanel.getByText(publishedName)).toBeVisible();
+  });
 });

frontend/tests/e2e/pages/AdminPage.ts

@@ -13,10 +13,17 @@ export type AdminTab =
 /**
  * The admin shell: the tab strip and the panel it swaps.
  *
- * Only the active tab's panel is mounted, which is what keeps the locators
+ * Only the active tab's panel is *visible*, which is what keeps the locators
  * inside each panel unambiguous — the email editors used to be stacked and a
  * locator for "Save" matched all six.
  *
+ * Visible, not mounted. antd keeps a pane in the DOM once it has been rendered
+ * and only hides it, so scoping to `.ant-tabs-tabpane-active` is what makes
+ * these locators work — the inactive panels are still there. This comment used
+ * to say "mounted", and that mistake is the shape of #327: a tab that never
+ * unmounts also never re-runs its effects, so Inventory went on showing a list
+ * it had fetched before anything was published into it.
+ *
  * `activePanel` exists because several specs reached for
  * `.ant-tabs-tabpane-active .ant-table` and similar to scope themselves to the
  * visible panel. That knowledge belongs here rather than in five spec files.