Merge pull request 'fix(admin): refetch Inventory when its tab becomes visible (#327)' (#328) from bugfix/327-inventory-stale-after-publish into main

Reviewed-on: #328

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.