diff --git a/frontend/src/admin/Admin.tsx b/frontend/src/admin/Admin.tsx index ab5daef..955572f 100755 --- a/frontend/src/admin/Admin.tsx +++ b/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 = { 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([]); const [modalOpen, setModalOpen] = useState(false); const [editingItem, setEditingItem] = useState(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 ( @@ -493,9 +521,10 @@ export default function Admin() { }, + { key: 'inventory', label: 'Inventory', children: }, { key: 'categories', label: 'Categories', children: }, { key: 'tags', label: 'Tags', children: }, { key: 'upload-links', label: 'Upload links', children: }, diff --git a/frontend/tests/e2e/admin-draft-queue.spec.ts b/frontend/tests/e2e/admin-draft-queue.spec.ts index 58cbdd9..2403682 100644 --- a/frontend/tests/e2e/admin-draft-queue.spec.ts +++ b/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(); + }); }); diff --git a/frontend/tests/e2e/pages/AdminPage.ts b/frontend/tests/e2e/pages/AdminPage.ts index c700f2d..65ab194 100644 --- a/frontend/tests/e2e/pages/AdminPage.ts +++ b/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.