diff --git a/.changeset/10519-pageview-refresh-in-place.md b/.changeset/10519-pageview-refresh-in-place.md new file mode 100644 index 0000000000..88aecdd85e --- /dev/null +++ b/.changeset/10519-pageview-refresh-in-place.md @@ -0,0 +1,32 @@ +--- +'@object-ui/app-shell': patch +--- + +fix(app-shell): a page action refreshes a custom page's data in place instead of remounting the page (objectui#10519) + +`PageView` used to key both of its render branches — the ADR-0047 interface list +and the `SchemaRenderer` page — on a counter it bumped after every successful +page-level action that asks for a refresh (an `api` call, a flow, a server +action, an undo, a screen-flow completion). Each such action therefore remounted +the whole page: scroll position, collapsed sections, tab state and in-progress +edits in every embedded block were lost, and every block refetched from scratch. + +The counter is gone. `PageView` now answers the console action runtime's +post-action refresh by declaring the change on the data-invalidation bus +(`notifyDataChanged` from `@object-ui/react`, with the unknown-scope +`objectName: '*'`, since a page binds no object). The embedded blocks that read +the bus refetch in place, once each: the object grid, chart, detail view, list +view, kanban, metric, calendar, gallery, timeline, gantt, map, tree, pivot and +data table, every `object-form` layout, `object-master-detail-form` (edit-mode +lines), `report` / `spec-report` over a dataset, a `dashboard`'s dataset table +widget and its `optionsFrom` filter options, an `object-view` drawn as a kanban, +calendar, gallery or timeline, `record:line_items` with an authored parent, and +the `element:number` / `element:repeater` / `element:record_picker` readers. An +`object-form` holding unsaved input keeps it and takes the re-read when it is +saved or reverted (objectui#10572), and a dashboard filter keeps its selected +value. A `kind: 'react'` page is no longer remounted either: its own read +re-runs in place when its effect names the `useDataInvalidation` nonce the page +scope injects (objectui#10887), and a read keyed on `useAdapter` alone is not +re-run by a page action. The page node's `context` is `{ params }` alone: +nothing read the `refreshKey` it also carried. No prop, export or schema key +changes. diff --git a/.changeset/10887-react-page-data-invalidation.md b/.changeset/10887-react-page-data-invalidation.md index 25ebd96831..1624351a61 100644 --- a/.changeset/10887-react-page-data-invalidation.md +++ b/.changeset/10887-react-page-data-invalidation.md @@ -7,7 +7,7 @@ feat(components): a `kind: 'react'` page's author scope injects `useDataInvalida A react page reads data through the injected `useAdapter`, in an effect it writes itself. The scope gave that effect no data-invalidation reader to name, so the read the react-pages guide taught, keyed on `[adapter]`, re-ran only -when the page was remounted (as `PageView` does after a page action) or its +when the page was remounted (as `PageView` did after a page action) or its adapter changed. The scope now injects `useDataInvalidation` from `@object-ui/react` beside diff --git a/.changeset/9673-pageview-drop-context-spread.md b/.changeset/9673-pageview-drop-context-spread.md index b706192515..382606917c 100644 --- a/.changeset/9673-pageview-drop-context-spread.md +++ b/.changeset/9673-pageview-drop-context-spread.md @@ -7,7 +7,7 @@ fix(app-shell): `PageView` builds the page node's `context`, it no longer reads `PageView` spread `(page as any).context` into the `context` it hands `SchemaRenderer`. `PageSchema` refuses a page-level `context` key, so on every page that parses the spread added nothing, and the code read as an author channel that no author could use. -The node's `context` is now exactly `{ params, refreshKey }`, built from the route and -the refresh counter, and the `as any` cast at that spot is gone. A stored document -that carries `context` without passing `PageSchema` no longer passes it through. -`context` stays undeclared on `PageSchema` (objectui#9673). +The node's `context` is now built from the route alone, and the `as any` cast at that +spot is gone. A stored document that carries `context` without passing `PageSchema` +no longer passes it through. `context` stays undeclared on `PageSchema` +(objectui#9673). diff --git a/packages/app-shell/src/no-refresh-key-remount.ratchet.test.ts b/packages/app-shell/src/no-refresh-key-remount.ratchet.test.ts index 34d2b4db01..0612822e8b 100644 --- a/packages/app-shell/src/no-refresh-key-remount.ratchet.test.ts +++ b/packages/app-shell/src/no-refresh-key-remount.ratchet.test.ts @@ -19,14 +19,19 @@ * (`notifyDataChanged` from `@object-ui/react`) and let readers refetch via * `useDataInvalidation`. See objectui#2269 / DetailView / RecordDetailView. * - * SCOPE — the RECORD-DETAIL data surfaces #2269 fixed. It is deliberately not - * repo-wide: an explicit user "Refresh this page" affordance - * (`PageView.onRefresh` → `InterfaceListPage key={refreshKey}`) and the Studio - * dev preview harness (`sdui-workbench-preview.tsx`) legitimately remount to - * reset, and are a different concern from "a SAVE silently rebuilt the record - * page under the user". Guarding the fixed surfaces exactly, with no - * allowlist-of-shame, is the honest lock; AGENTS.md Commandment #8 + review - * cover brand-new surfaces. + * SCOPE — the RECORD-DETAIL data surfaces #2269 fixed, plus the custom-page + * host `PageView` (objectui#10519). It is deliberately not repo-wide: the + * Studio dev preview harness (`sdui-workbench-preview.tsx`) legitimately + * remounts to reset, and is a different concern from "a SAVE silently rebuilt + * the page under the user". `PageView.onRefresh` is NOT such an affordance — + * an earlier version of this header called it "an explicit user 'Refresh this + * page' affordance", which was false by a source reading of + * `useConsoleActionRuntime`: `onRefresh` is fed only by the console action + * runtime's post-action refresh (`api`, flow and server-action success, undo, + * screen-flow completion), and `PageView` has no refresh control. objectui#10519 + * routed that refresh through the bus and brought the file into scope. Guarding + * the fixed surfaces exactly, with no allowlist-of-shame, is the honest lock; + * AGENTS.md Commandment #8 + review cover brand-new surfaces. */ import { describe, it, expect } from 'vitest'; @@ -46,14 +51,18 @@ const repoRoot = path.resolve(here, '../../..'); const REFRESH_KEY_REMOUNT = /\bkey=\{\s*[^}]*(?:refresh|reload)[^}]*\}/i; /** - * The record-detail data surfaces #2269 fixed. Path fragments (POSIX) — a - * file is in scope if its repo-relative path contains any of these. + * The record-detail data surfaces #2269 fixed, and the custom-page host + * objectui#10519 fixed. Path fragments (POSIX) — a file is in scope if its + * repo-relative path contains any of these. */ const IN_SCOPE = [ 'packages/plugin-detail/src/', 'packages/app-shell/src/views/RecordDetailView', 'packages/app-shell/src/views/RelatedRecordActionsBridge', 'packages/app-shell/src/console/AppContent', + // objectui#10519: the page host declares a page action's change on the bus; + // its two render branches are keyed on identity, never on a refresh counter. + 'packages/app-shell/src/views/PageView', ]; function collectSourceFiles(): string[] { @@ -96,13 +105,13 @@ function collectSourceFiles(): string[] { } describe('objectui#2269 — no refetch-by-remount ratchet', () => { - it('finds the in-scope record-detail surfaces (guards against a broken scan path)', () => { + it('finds the in-scope record-detail surfaces and the PageView page host (guards against a broken scan path)', () => { // plugin-detail alone has dozens of source files; if this drops the // scope globs have gone stale and the ratchet would silently pass. expect(collectSourceFiles().length).toBeGreaterThan(20); }); - it('has zero `key={…refresh/reload…}` remount sites in the record-detail surfaces', () => { + it('has zero `key={…refresh/reload…}` remount sites in the record-detail surfaces or the PageView page host', () => { const offenders: string[] = []; for (const file of collectSourceFiles()) { const src = readFileSync(file, 'utf8'); diff --git a/packages/app-shell/src/views/PageView.tsx b/packages/app-shell/src/views/PageView.tsx index 1ace6962da..979bb11eb7 100644 --- a/packages/app-shell/src/views/PageView.tsx +++ b/packages/app-shell/src/views/PageView.tsx @@ -9,9 +9,8 @@ * embedding the heavyweight page canvas in the runtime. */ -import { useState } from 'react'; import { useParams, useSearchParams, useNavigate, useLocation } from 'react-router-dom'; -import { SchemaRenderer, useAdapter } from '@object-ui/react'; +import { SchemaRenderer, notifyDataChanged, useAdapter } from '@object-ui/react'; import { Empty, EmptyTitle, EmptyDescription, Spinner } from '@object-ui/components'; import { FileText, Pencil } from 'lucide-react'; import { useObjectTranslation } from '@object-ui/i18n'; @@ -24,6 +23,25 @@ import { ConsoleActionRuntimeProvider } from '../hooks/useConsoleActionRuntime.j import { useCanAuthorMetadata } from '../hooks/useCanAuthorMetadata.js'; import { InterfaceListPage } from './InterfaceListPage.js'; +/** + * After a successful page-level action, declare the change on the + * data-invalidation bus so every embedded block that reads it refetches IN + * PLACE (AGENTS.md #8's corollary: refresh data, don't rebuild UI). A page + * binds no object and the runtime's refresh carries none, so the scope is the + * bus's documented unknown-scope value. + * + * This host used to bump a counter into the `key` of both render branches + * below, which remounted the whole page on every page action: scroll, collapsed + * sections and in-progress edits in every block went with it, and every block + * refetched from scratch (objectui#10519). Both branches are now keyed on + * identity; `no-refresh-key-remount.ratchet` holds this file in scope. + * Module-level so its identity is stable across renders (the runtime names it + * in dependency lists). + */ +function declarePageDataChanged(): void { + notifyDataChanged({ objectName: '*' }); +} + export function PageView() { const { t } = useObjectTranslation(); const { pageName } = useParams<{ pageName: string }>(); @@ -48,9 +66,6 @@ export function PageView() { // container instead of by load order. const { app: activeApp } = useExpressionContext(); const dataSource = useAdapter(); - // Bumped after a successful page action so embedded data (lists, etc.) - // re-fetch. Threaded into the page context AND used to remount the renderer. - const [refreshKey, setRefreshKey] = useState(0); const page = preferLocal(pages as any[], pageName, (activeApp as any)?._packageId); if (!page) { @@ -103,7 +118,7 @@ export function PageView() { setRefreshKey((k) => k + 1)} + onRefresh={declarePageDataChanged} >
@@ -122,10 +137,9 @@ export function PageView() { {(page as any).interfaceConfig?.source ? ( // ADR-0047 interface mode: the page binds a source view into a // curated list surface — rendered directly, not via regions. - + ) : ( )} diff --git a/packages/app-shell/src/views/__tests__/PageView.context-9673.test.tsx b/packages/app-shell/src/views/__tests__/PageView.context-9673.test.tsx index a94afacc17..a4863f761b 100644 --- a/packages/app-shell/src/views/__tests__/PageView.context-9673.test.tsx +++ b/packages/app-shell/src/views/__tests__/PageView.context-9673.test.tsx @@ -13,7 +13,9 @@ * never carries one. PageView used to spread `(page as any).context` into the * node anyway: a no-op on every parsed page, and a channel that read as * author-supplied page context that no author could supply. Ruled "remove" - * (comment 5811058824): the node's `context` is exactly `{ params, refreshKey }`. + * (comment 5811058824): the node's `context` is exactly `{ params }` — the + * `refreshKey` it also carried when this was ruled left with objectui#10519, + * which measured that no block read it. * * The second case is the discriminating one: a document that never passed * `PageSchema` and sneaks `context` in through a cast must not leak it into the @@ -96,7 +98,7 @@ function writeFor(doc: Record, query: string): Record { - it('hands SchemaRenderer a context of exactly { params, refreshKey } for a page that parses', () => { + it('hands SchemaRenderer a context of exactly { params } for a page that parses', () => { // Control: the fixture is a page PageSchema accepts, so this is the case // every real author is in. expect(PageSchema.safeParse(PARSED_PAGE).success).toBe(true); @@ -105,7 +107,7 @@ describe('objectui#9673 — PageView builds the node context, it never reads one // Firing control: absence means the harness stopped reaching SchemaRenderer. expect(schema, 'PageView rendered no schema at all; this probe measured nothing').toBeDefined(); - expect(schema!.context).toEqual({ params: { account: '42', tab: 'notes' }, refreshKey: 0 }); + expect(schema!.context).toEqual({ params: { account: '42', tab: 'notes' } }); }); it('a document that sneaks `context` in past PageSchema does not leak it into the node', () => { @@ -121,6 +123,6 @@ describe('objectui#9673 — PageView builds the node context, it never reads one schema!.context, 'the node context must be built from the route alone; a `context` key on the stored page ' + 'is one PageSchema refuses and must not reach SchemaRenderer (objectui#9673).', - ).toEqual({ params: { account: '42' }, refreshKey: 0 }); + ).toEqual({ params: { account: '42' } }); }); }); diff --git a/packages/app-shell/src/views/__tests__/PageView.refreshInPlace-10519.test.tsx b/packages/app-shell/src/views/__tests__/PageView.refreshInPlace-10519.test.tsx new file mode 100644 index 0000000000..d9cd7e584c --- /dev/null +++ b/packages/app-shell/src/views/__tests__/PageView.refreshInPlace-10519.test.tsx @@ -0,0 +1,589 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#10519 — a successful page-level action refreshes the page's DATA; it + * does not rebuild the page (AGENTS.md #8's corollary: refresh data, don't + * rebuild UI). + * + * ## The defect this pins + * + * `PageView` kept a `refreshKey` counter, bumped it from the console action + * runtime's `onRefresh` — which fires after every successful `api`, flow and + * server action, an undo, and a screen-flow completion — and keyed BOTH render + * branches on it: `` and + * ``. So one page action REMOUNTED the whole + * page: every block's scroll, collapsed sections and in-progress edits went + * with it, and every block refetched from scratch. Nothing read the counter off + * the page context (measured on the card: a node carrying it reached no block). + * + * The host now declares the change on the data-invalidation bus instead — + * `notifyDataChanged({ objectName: '*' })`: a page binds no object and the + * runtime's refresh carries none, so the bus's documented unknown-scope value is + * the page's scope — and the blocks that read the bus refetch in place. + * + * ## The two halves, and which world each one can fail in + * + * Every in-place case asserts BOTH: + * (a) the SAME mounted tree after the action — a block instance id from a + * `useState` initializer, DOM nodes read before the action, a scroll + * offset, a collapsed accordion panel and a half-typed field all survive; + * (b) each bus-reading block re-read exactly once. + * Against the pre-fix source (a) is the red half; (b) stays green there, + * because that world refetched too, through the remount. (b) is the half that + * goes red when the key leaves without the host declaring the change on the + * bus — the regression a naive "remove the key" would ship. + * + * ## What renders for real + * + * `PageView`, the console action runtime, the `page`, `page:accordion` and + * `action:button` renderers from `@object-ui/components`, `object-metric` from + * `@object-ui/plugin-dashboard` and `object-form` from `@object-ui/plugin-form` + * (real bus readers; both are this package's devDependencies); the interface + * branch renders the real `InterfaceListPage` over `plugin-list`'s `ListView`. + * The stand-in registered as `object-gantt` follows the + * `ObjectView.refreshInPlace-10035` precedent (`app-shell` does not depend on + * `plugin-gantt`): it reads the REAL bus hook, counts its own queries, and + * carries an instance id and a scroll box. + * + * The page action is a real `api` action through the runtime's `apiHandler` + * — an authenticated raw-HTTP call, so no dataSource `onMutation` fires — the + * class of write that reached the blocks only through the remount. + * + * ## The round-4 rows (objectui#10887's readers) + * + * The last describe block holds the blocks whose readers landed with + * objectui#10887 members 1 and 2: an `object-view` drawn as a kanban, a + * calendar and a gallery (`ObjectView` fetches those rows itself and names the + * bus nonce in that fetch), and a `dashboard` whose `globalFilters` select reads + * its options through `optionsFrom` (`SelectFilter` names the nonce). Each is + * held in a `regions` page in the stored-page shape (`{ type, properties }`), + * beside the page action and the stand-in, and is read the same two ways: + * (a) the same instance after the action, witnessed by state a remount resets + * (the stand-in's instance id, a card node, the calendar's navigated month, the + * filter's selected value), and (b) exactly one re-read of the object that + * block queries. Before objectui#10887 each of these re-read only through the + * remount, so (b) is the half that goes red on that code path. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeAll, beforeEach, afterEach } from 'vitest'; +import { render, screen, fireEvent, act, cleanup, waitFor } from '@testing-library/react'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; + +const authFetchSpy = vi.fn(); +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u1', name: 'Ada', role: 'user', image: null }, activeOrganization: null }), + useWorkspaceAdminStatus: () => ({ isAdmin: false, isResolved: true }), + createAuthenticatedFetch: () => authFetchSpy, +})); + +vi.mock('@object-ui/i18n', async (importOriginal) => ({ + ...(await importOriginal>()), + // `language` is read by the `page:accordion` label pass; without it the + // renderer throws and the schema error boundary recreates the subtree, + // which would count as a remount here. + useObjectTranslation: () => ({ t: (k: string, o?: any) => o?.defaultValue ?? o?.name ?? k, language: 'en' }), + useObjectLabel: () => ({ + fieldLabel: (_o: any, _n: any, l: any) => l, + fieldOptionLabel: (_o: any, _f: any, _v: any, l: any) => l, + actionParamText: (_o: any, _a: any, _p: any, _attr: any, fallback: any) => fallback, + }), + // The form layouts build their discard-guard strings with this; echo the + // supplied English defaults. + createSafeTranslation: + (defaults: Record) => () => ({ + t: (k: string) => defaults?.[k] ?? k, + }), +})); + +vi.mock('@object-ui/permissions', async (importOriginal) => { + const actual = await importOriginal(); + // STABLE identities: `ListView` names `perms` in its fetch dependency list, + // so a fresh object per call would loop the fetch on its own. + const perms = { + check: () => ({ allowed: true }), + checkField: () => true, + getFieldPermissions: () => [], + getRowFilter: () => undefined, + getObjectApiOperations: () => undefined, + roles: [], + isLoaded: false, + hasCapabilities: () => true, + can: () => true, + cannot: () => false, + }; + const fieldPerms = { canRead: () => true, canWrite: () => true, permissions: [] }; + return { ...actual, usePermissions: () => perms, useFieldPermissions: () => fieldPerms }; +}); + +/** The stored page document the next mount resolves. */ +let storedPage: Record = {}; + +vi.mock('../../providers/MetadataProvider', () => ({ + useMetadata: () => ({ pages: [storedPage], objects: OBJECTS, getTypeStatus: () => 'ready' }), +})); + +vi.mock('../MetadataInspector', () => ({ + MetadataPanel: () => null, + useMetadataInspector: () => ({ showDebug: false }), +})); +vi.mock('../RecordDetailView', () => ({ RecordDetailView: () => null })); + +import { ComponentRegistry } from '@object-ui/core'; +import { AdapterCtx, SchemaRendererProvider, notifyDataChanged, useDataInvalidation } from '@object-ui/react'; +import { registerAllFields } from '@object-ui/fields'; +// Side-effect imports: `page`, `page:accordion` and `action:button`, then the +// two real bus readers the page embeds. +import '@object-ui/components'; +import '@object-ui/plugin-dashboard'; +import '@object-ui/plugin-form'; +// The round-4 rows: `object-view` and the three views it draws them through +// (`object-gallery` is registered by plugin-list), all devDependencies here. +import '@object-ui/plugin-view'; +import '@object-ui/plugin-kanban'; +import '@object-ui/plugin-calendar'; +import '@object-ui/plugin-list'; +import { PageView } from '../PageView'; + +registerAllFields(); + +const OBJECT = 'deal'; + +const FIELDS = { + id: { type: 'text', label: 'Id' }, + name: { type: 'text', label: 'Name' }, + note: { type: 'text', label: 'Note' }, + stage: { type: 'select', label: 'Stage', options: [{ label: 'A', value: 'a' }] }, + amount: { type: 'number', label: 'Amount' }, +}; + +/** One object with one list view (the interface branch's source) and one page action. */ +const OBJECTS = [ + { + name: OBJECT, + label: 'Deal', + fields: FIELDS, + listViews: { + all: { label: 'All', columns: ['name', 'stage'], description: 'Every deal' }, + }, + actions: [{ name: 'create_env', label: 'Create environment', type: 'api', target: '/api/v1/environments' }], + }, + // The round-4 rows' second object: a dashboard filter reads its options here. + { name: 'account', label: 'Account', fields: { id: { type: 'text' }, industry: { type: 'text', label: 'Industry' } } }, +]; + +/** The stand-in's own queries, and the instance ids it mounted with. */ +let ganttQueries = 0; +let ganttInstanceSeq = 0; + +/** + * A self-fetching visualization that refetches on the bus, like `ObjectGantt`, + * with the UI state a remount destroys: an instance id and a scroll box. + */ +function GanttStandIn({ schema }: any) { + const [id] = React.useState(() => ++ganttInstanceSeq); + const nonce = useDataInvalidation(schema?.objectName); + React.useEffect(() => { + ganttQueries++; + }, [nonce]); + return ( +
+
+
+
+
+ ); +} + +function makeDataSource() { + const record = { id: 'r1', name: 'Server v1', note: 'n1', updated_at: '2026-01-01T00:00:00.000Z' }; + return { + find: vi.fn(async () => ({ data: [{ id: 'r1', name: 'Acme', stage: 'a', amount: 7 }], total: 1 })), + findOne: vi.fn(async () => ({ ...record })), + aggregate: vi.fn(async () => [{ amount: 7 }]), + create: vi.fn(async () => ({})), + update: vi.fn(async () => ({})), + delete: vi.fn(async () => ({})), + getObjectSchema: vi.fn(async (name: string) => ({ name, label: 'Deal', fields: FIELDS })), + }; +} +type DS = ReturnType; + +/** The list queries `ListView` issued; its `$top: 0` count probe is excluded. */ +const listQueries = (ds: DS) => ds.find.mock.calls.filter((c: any[]) => c[1]?.$top !== 0).length; + +/** A window long enough for every effect a step schedules to have fired. */ +const settle = (ms = 300) => act(() => new Promise((resolve) => setTimeout(resolve, ms))); + +const ACTION_NODE = { + type: 'action:button', + name: 'create_env', + label: 'Create environment', + actionType: 'api', + target: '/api/v1/environments', +}; + +/** The rendered page: a page action beside three data blocks and a collapsible panel. */ +const PAGE = { + name: 'home', + label: 'Home', + type: 'app', + children: [ + ACTION_NODE, + { type: 'object-metric', id: 'metric', objectName: OBJECT, aggregate: { field: 'amount', function: 'sum' }, label: 'Pipeline' }, + { type: 'object-gantt', id: 'gantt', objectName: OBJECT }, + { + type: 'object-form', + id: 'form', + objectName: OBJECT, + mode: 'edit', + recordId: 'r1', + sections: [{ name: 'main', label: 'Main', fields: ['name', 'note'] }], + }, + { + type: 'page:accordion', + id: 'panels', + items: [{ label: 'Notes', collapsed: false, children: [{ type: 'page:section', id: 'notes', children: [] }] }], + }, + ], +}; + +/** ADR-0047 interface mode: the page binds the `all` view and offers the same action as a toolbar button. */ +const INTERFACE_PAGE = { + name: 'deals', + label: 'Deals', + type: 'list', + interfaceConfig: { source: OBJECT, sourceView: 'all', buttons: ['create_env'] }, +}; + +/** + * Mounts the page under both data-source contexts the console provides: + * `AdapterCtx` is what `PageView` and `InterfaceListPage` read through + * `useAdapter()`, and `SchemaRendererProvider` is what the embedded blocks read. + */ +function mount(page: Record) { + storedPage = page; + const ds = makeDataSource(); + render( + + + + + } /> + + + + , + ); + return ds; +} + +/** Runs the page's `api` action through the console runtime and waits for its refresh. */ +async function runPageAction() { + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: 'Create environment' })); + }); + await waitFor(() => expect(authFetchSpy).toHaveBeenCalled()); + await settle(); +} + +const nameInput = () => document.body.querySelector('input[name="name"]') as HTMLInputElement; +const ganttInstance = () => screen.getByTestId('gantt-stand-in').dataset.instance; +const accordionTrigger = () => screen.getByRole('button', { name: 'Notes' }); + +beforeEach(() => { + cleanup(); + ganttQueries = 0; + ganttInstanceSeq = 0; + ComponentRegistry.register('object-gantt', GanttStandIn as any); + authFetchSpy.mockReset(); + authFetchSpy.mockResolvedValue({ ok: true, json: async () => ({ id: 'env_1' }) }); + // Best-effort metadata probes are not what these cases are about. + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response(JSON.stringify({ data: [] }), { status: 200, headers: { 'content-type': 'application/json' } })), + ); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.clearAllMocks(); +}); + +describe('a page action refreshes the page in place (objectui#10519)', () => { + it('SchemaRenderer branch: the same page keeps its scroll, collapsed panel and half-typed field, and each bus reader re-reads once', async () => { + const ds = mount(PAGE); + await waitFor(() => expect(nameInput()?.value).toBe('Server v1')); + await waitFor(() => expect(ds.aggregate).toHaveBeenCalledTimes(1)); + await settle(); + expect(ganttQueries, 'the stand-in must be drawing this page').toBeGreaterThan(0); + expect(ganttInstanceSeq, 'mount control: exactly one stand-in instance before the action').toBe(1); + + // UI state the action must not reset. + const instance = ganttInstance(); + const scrollBox = screen.getByTestId('gantt-scroll'); + scrollBox.scrollTop = 120; + const metricNode = screen.getByText('Pipeline'); + const input = nameInput(); + fireEvent.change(input, { target: { value: 'User typed' } }); + await settle(150); + fireEvent.click(accordionTrigger()); + await settle(150); + expect(accordionTrigger().getAttribute('aria-expanded'), 'setup: the panel must be collapsed before the action').toBe('false'); + + const aggregatesBefore = ds.aggregate.mock.calls.length; + const ganttBefore = ganttQueries; + + await runPageAction(); + + // (a) the same tree + expect( + ganttInstanceSeq, + '(a) The page action REMOUNTED the page: the `SchemaRenderer` consumer mounted a second\n' + + 'time. The refresh counter is back in the `` key (AGENTS.md #8: refresh\n' + + 'data, don\'t rebuild UI).', + ).toBe(1); + expect(ganttInstance(), '(a) the action REMOUNTED the visualization itself').toBe(instance); + expect(screen.getByTestId('gantt-scroll'), '(a) the scroll box was replaced').toBe(scrollBox); + expect(screen.getByTestId('gantt-scroll').scrollTop, '(a) the scroll position was lost').toBe(120); + expect(screen.getByText('Pipeline'), '(a) the metric was replaced').toBe(metricNode); + expect(nameInput(), '(a) the form was replaced').toBe(input); + expect(nameInput().value, '(a) the in-progress edit was lost').toBe('User typed'); + expect(accordionTrigger().getAttribute('aria-expanded'), '(a) the collapsed panel re-opened').toBe('false'); + + // (b) each bus reader re-read exactly once + expect( + ds.aggregate.mock.calls.length - aggregatesBefore, + '(b) The metric did not re-read after the action, or re-read more than once. The host\n' + + 'must declare the change on the data-invalidation bus (`notifyDataChanged`) now that\n' + + 'the page is no longer remounted to show it the write.', + ).toBe(1); + expect(ganttQueries - ganttBefore, '(b) the stand-in did not re-query exactly once').toBe(1); + }); + + it('interface branch: the same list re-issues its query once', async () => { + const ds = mount(INTERFACE_PAGE); + // A node rendered INSIDE `ListView` (its toolbar's own refresh control) — + // replaced wholesale by a key remount of the branch above it. + await waitFor(() => expect(screen.getByTestId('refresh-button')).toBeTruthy()); + await waitFor(() => expect(listQueries(ds)).toBeGreaterThan(0)); + await settle(); + + const page = screen.getByTestId('interface-list-page'); + const node = screen.getByTestId('refresh-button'); + const before = listQueries(ds); + + await runPageAction(); + + expect( + screen.getByTestId('interface-list-page'), + '(a) The page action REMOUNTED the interface page: the refresh counter is back in the\n' + + '`` key (AGENTS.md #8: refresh data, don\'t rebuild UI).', + ).toBe(page); + expect(screen.getByTestId('refresh-button'), '(a) the action REMOUNTED the list').toBe(node); + expect( + listQueries(ds) - before, + '(b) The list did not re-query after the action, or re-queried more than once. It reads\n' + + 'the data-invalidation bus, so the host must declare the change there.', + ).toBe(1); + }); + + it('lit control: a change declared on the bus reaches the same readers once, without any action', async () => { + const ds = mount(PAGE); + await waitFor(() => expect(ds.aggregate).toHaveBeenCalledTimes(1)); + await settle(); + const aggregatesBefore = ds.aggregate.mock.calls.length; + const ganttBefore = ganttQueries; + + await act(async () => { + notifyDataChanged({ objectName: '*' }); + }); + await settle(); + + expect(ds.aggregate.mock.calls.length - aggregatesBefore, 'control: the metric does not read the bus in this harness').toBe(1); + expect(ganttQueries - ganttBefore, 'control: the stand-in does not read the bus in this harness').toBe(1); + expect(authFetchSpy, 'control: no action ran').not.toHaveBeenCalled(); + }); +}); + +// --------------------------------------------------------------------------- +// Round 4: the blocks whose readers landed with objectui#10887. +// --------------------------------------------------------------------------- + +/** + * Counts reads per object, so each row's re-read is attributed to the object + * that block queries. Every `deal` read answers one more row than the last + * (`Deal 1`, then `Deal 1` and `Deal 2`), dated today so a calendar draws it; + * the second `account` read adds an `energy` option. + */ +function makeCountingDataSource() { + const reads: Record = {}; + const today = new Date().toISOString().slice(0, 10); + return { + reads, + find: vi.fn(async (objectName: string, query?: any) => { + // A `$top: 0` count probe is not a read of the rows. + if (query?.$top === 0) return { data: [], total: 0 }; + const n = (reads[objectName] = (reads[objectName] ?? 0) + 1); + if (objectName === 'account') { + const values = n === 1 ? ['finance', 'retail'] : ['energy', 'finance', 'retail']; + return { data: values.map((industry, i) => ({ id: `a${i}`, industry })) }; + } + const rows = Array.from({ length: n }, (_, i) => ({ id: `d${i + 1}`, name: `Deal ${i + 1}`, stage: 'a', due: today, amount: 7 })); + return { data: rows, total: rows.length }; + }), + findOne: vi.fn(async () => null), + aggregate: vi.fn(async () => []), + create: vi.fn(async () => ({})), + update: vi.fn(async () => ({})), + delete: vi.fn(async () => ({})), + getObjectSchema: vi.fn(async (name: string) => ({ + name, + label: name, + fields: name === 'account' ? OBJECTS[1].fields : { ...FIELDS, due: { type: 'date', label: 'Due' } }, + })), + }; +} +type CountingDS = ReturnType; + +/** A stored page (`regions`, the spec shape) holding the action, the stand-in and one block. */ +function mountRegionPage(block: Record) { + const page = { + name: 'census', + label: 'Census', + type: 'app', + regions: [{ name: 'main', components: [ACTION_NODE, { type: 'object-gantt', id: 'gantt', objectName: OBJECT }, block] }], + }; + storedPage = page; + const ds = makeCountingDataSource(); + render( + + + + + } /> + + + + , + ); + return ds; +} + +/** One page action, then the row's reads of its own object and the stand-in's instance. */ +async function actAndRead(ds: CountingDS, objectName: string) { + const before = ds.reads[objectName] ?? 0; + const instance = ganttInstance(); + await runPageAction(); + expect( + ganttInstance(), + '(a) The page action REMOUNTED the page: the stand-in beside the block has a new\n' + + 'instance. The refresh counter is back in the `` key (AGENTS.md #8).', + ).toBe(instance); + return (ds.reads[objectName] ?? 0) - before; +} + +const objectView = (properties: Record) => ({ type: 'object-view', properties: { objectName: OBJECT, ...properties } }); +const dealCard = () => screen.getByText('Deal 1'); +const calendarMonth = () => (document.body.querySelector('[aria-label^="Current date"] span') as HTMLElement | null)?.textContent; + +describe('the round-4 rows refresh in place after a page action (objectui#10519, readers from objectui#10887)', () => { + beforeAll(() => { + // Radix Select opens on pointer events the DOM environment does not + // implement; the shim `DashboardFilterBar.busReread-10887` uses. + class MockPointerEvent extends Event { + button: number; + ctrlKey: boolean; + pointerType: string; + constructor(type: string, props: PointerEventInit = {}) { + super(type, props); + this.button = props.button ?? 0; + this.ctrlKey = props.ctrlKey ?? false; + this.pointerType = props.pointerType ?? 'mouse'; + } + } + Object.assign(window, { PointerEvent: MockPointerEvent }); + Object.assign(HTMLElement.prototype, { + hasPointerCapture: vi.fn(), + releasePointerCapture: vi.fn(), + scrollIntoView: vi.fn(), + }); + }); + + for (const viewType of ['kanban', 'gallery'] as const) { + it(`object-view drawn as a ${viewType}: the same view re-reads its rows once`, async () => { + const ds = mountRegionPage( + objectView(viewType === 'kanban' ? { defaultViewType: 'kanban', defaultListView: 'board', listViews: { board: { label: 'Board', type: 'kanban', kanban: { groupByField: 'stage' } } } } : { defaultViewType: 'gallery' }), + ); + await waitFor(() => expect(dealCard()).toBeTruthy()); + await settle(); + const card = dealCard(); + + const reReads = await actAndRead(ds, OBJECT); + + expect(dealCard(), `(a) the ${viewType} was rebuilt: its card is a new node`).toBe(card); + expect( + reReads, + `(b) The ${viewType} object-view did not re-read its rows exactly once after the action.\n` + + '`ObjectView` fetches these rows itself; its fetch must name the bus nonce (objectui#10887).', + ).toBe(1); + await waitFor(() => expect(screen.getByText('Deal 2'), 'the re-read rows never reached the view').toBeTruthy()); + }); + } + + it('object-view drawn as a calendar: the same calendar keeps its navigated month and re-reads once', async () => { + const ds = mountRegionPage( + objectView({ defaultViewType: 'calendar', defaultListView: 'cal', listViews: { cal: { label: 'Cal', type: 'calendar', calendar: { startDateField: 'due', titleField: 'name' } } } }), + ); + await waitFor(() => expect(screen.getByRole('button', { name: 'Next period' })).toBeTruthy()); + await settle(); + const month = calendarMonth(); + fireEvent.click(screen.getByRole('button', { name: 'Next period' })); + await settle(150); + const navigated = calendarMonth(); + expect(navigated, 'setup: the calendar must have moved off the current month').not.toBe(month); + + const reReads = await actAndRead(ds, OBJECT); + + // The month lives in `ObjectCalendar`'s own state: a remount of the view + // (or of the page) resets it to the current month. + expect(calendarMonth(), '(a) the calendar was rebuilt: its navigated month was reset').toBe(navigated); + expect(reReads, '(b) The calendar object-view did not re-read its rows exactly once after the action.').toBe(1); + }); + + it('dashboard with an optionsFrom select filter: the same filter keeps its value and re-reads its options once', async () => { + const ds = mountRegionPage({ + type: 'dashboard', + properties: { + name: 'census_dash', + globalFilters: [{ name: 'industry', field: 'industry', label: 'Industry', type: 'select', optionsFrom: { object: 'account', valueField: 'industry', labelField: 'industry' } }], + widgets: [], + }, + }); + const trigger = () => screen.getByTestId('dashboard-filter-industry'); + await waitFor(() => expect(ds.reads.account).toBe(1)); + await settle(); + fireEvent.pointerDown(trigger(), { button: 0 }); + fireEvent.click(await screen.findByRole('option', { name: 'retail' })); + await waitFor(() => expect(trigger().textContent).toBe('retail')); + await settle(150); + const node = trigger(); + + const reReads = await actAndRead(ds, 'account'); + + expect(trigger(), '(a) the dashboard filter was rebuilt').toBe(node); + expect(trigger().textContent, '(a) the selected filter value was lost').toBe('retail'); + expect( + reReads, + '(b) The dashboard filter did not re-read its optionsFrom options exactly once after the action.\n' + + '`SelectFilter` must name the bus nonce for `optionsFrom.object` (objectui#10887).', + ).toBe(1); + }); +});