From 5b2a23bf5e4557508c83a9e85a4c02882bf4a372 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 02:21:02 +0000 Subject: [PATCH 1/2] perf(console): one field-access request per record open, and the bell polls only as a push channel's 60 s fallback The record field projection is asked by every scope on the record page in the same millisecond; concurrent asks now share one request, and a 404 or 501 "no answer" is remembered until a write to the object. The inbox and approvals feeds poll at 60 s while no push channel is open, and a host can report a channel through a package-internal seam that stops the poll. Claude-Session: https://claude.ai/code/session_01Utp6CXJLrFrzeHHa8StCja Co-authored-by: Claude --- .../sharedInboxFeed.cadence-7249.test.tsx | 8 +- .../sharedUserFeeds.hiddenStop-12439.test.tsx | 9 +- ...sharedUserFeeds.pushChannel-27626.test.tsx | 207 ++++++++++++++++++ .../app-shell/src/hooks/sharedUserFeeds.ts | 125 ++++++++++- ...ordFieldAccess.inflightShare-27626.test.ts | 179 +++++++++++++++ ui/packages/data-objectstack/src/index.ts | 53 ++++- 6 files changed, 560 insertions(+), 21 deletions(-) create mode 100644 ui/packages/app-shell/src/hooks/__tests__/sharedUserFeeds.pushChannel-27626.test.tsx create mode 100644 ui/packages/data-objectstack/src/getRecordFieldAccess.inflightShare-27626.test.ts diff --git a/ui/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.cadence-7249.test.tsx b/ui/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.cadence-7249.test.tsx index c8e7e44c899..c87eafa7b64 100644 --- a/ui/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.cadence-7249.test.tsx +++ b/ui/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.cadence-7249.test.tsx @@ -142,8 +142,12 @@ vi.mock('../../providers/AdapterProvider', () => ({ useAdapter: () => fakeAdapte import { useSharedInboxFeed, __resetSharedUserFeeds, __markFirstPainted } from '../sharedUserFeeds'; import { useHomeInbox } from '../useHomeInbox'; -/** The declared foreground cadence (`INBOX_POLL_MS`), restated as the pin's unit. */ -const INBOX_POLL_MS = 10_000; +/** + * The declared foreground cadence (`INBOX_POLL_MS`), restated as the pin's + * unit. 60 s since objectstack#27626 made polling the fallback of a push + * channel; the 10 s measured in the header above is the cadence of its day. + */ +const INBOX_POLL_MS = 60_000; /** The declared backgrounded cadence (`HIDDEN_POLL_MS`). */ const HIDDEN_POLL_MS = 60_000; /** The measurement window both counting cases use. */ diff --git a/ui/packages/app-shell/src/hooks/__tests__/sharedUserFeeds.hiddenStop-12439.test.tsx b/ui/packages/app-shell/src/hooks/__tests__/sharedUserFeeds.hiddenStop-12439.test.tsx index c430e1c6920..f6fac7536f3 100644 --- a/ui/packages/app-shell/src/hooks/__tests__/sharedUserFeeds.hiddenStop-12439.test.tsx +++ b/ui/packages/app-shell/src/hooks/__tests__/sharedUserFeeds.hiddenStop-12439.test.tsx @@ -61,9 +61,12 @@ import { } from '../sharedUserFeeds'; import { HIDDEN_POLL_STOP_MS } from '../hiddenTabPollStop'; -/** The declared cadences, restated as the pins' units. */ -const INBOX_POLL_MS = 10_000; -const APPROVALS_POLL_MS = 30_000; +/** + * The declared cadences, restated as the pins' units — both the 60 s fallback + * of a push channel since objectstack#27626 (they were 10 s and 30 s). + */ +const INBOX_POLL_MS = 60_000; +const APPROVALS_POLL_MS = 60_000; const HIDDEN_POLL_MS = 60_000; const approvalsReads = () => diff --git a/ui/packages/app-shell/src/hooks/__tests__/sharedUserFeeds.pushChannel-27626.test.tsx b/ui/packages/app-shell/src/hooks/__tests__/sharedUserFeeds.pushChannel-27626.test.tsx new file mode 100644 index 00000000000..bb1a063b758 --- /dev/null +++ b/ui/packages/app-shell/src/hooks/__tests__/sharedUserFeeds.pushChannel-27626.test.tsx @@ -0,0 +1,207 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectstack#27626 — the inbox and approvals feeds poll only as the fallback + * of a push channel, and then at 60 s or slower. + * + * The bell polled `sys_inbox_message` every 10 s and `approvals/requests` every + * 30 s on every console page; one measuring tab had sent more than 110 000 + * requests in a working day. The platform serves no realtime channel today + * (discovery's `websockets` capability is off), so the fallback is what runs, + * and the seam a host would report a channel through is pinned beside it: + * + * - no channel: each feed reads at mount, then once per 60 s — never sooner; + * - a channel open: no poll at all, however long the page stays up, and a + * pushed change reads exactly the feed it names, once; + * - the channel closing: one read per feed at once, then the 60 s fallback; + * - returning to the tab while the channel is open reads nothing. + * + * Counted as requests both feeds actually send, under fake timers (the + * `sharedUserFeeds.hiddenStop-12439` technique). + */ +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { act, renderHook } from '@testing-library/react'; + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u1', email: 'u1@example.com' } }), +})); + +const inboxFinds: string[] = []; +const fakeAdapter = { + find: (object: string) => { + if (object === 'sys_inbox_message') inboxFinds.push(object); + return Promise.resolve({ data: [] }); + }, + getObjectSchema(this: { getClient(): any }, name: string) { + return this.getClient() + .meta.getItem('object', name) + .then((res: any) => (res && typeof res === 'object' && 'item' in res ? res.item : res)); + }, + getClient: () => undefined, +}; +vi.mock('../../providers/AdapterProvider', () => ({ useAdapter: () => fakeAdapter })); + +import { + useSharedInboxFeed, + useSharedPendingApprovalsCount, + setUserFeedPushChannelOpen, + notifyUserFeedPushed, + __resetSharedUserFeeds, + __markFirstPainted, +} from '../sharedUserFeeds'; + +/** The fallback cadence the card asks for: 60 s or slower. */ +const FALLBACK_MS = 60_000; + +const approvalsReads = () => + (globalThis.fetch as unknown as { mock: { calls: unknown[][] } }).mock.calls.filter((c) => + String(c[0]).includes('/api/v1/approvals/requests'), + ).length; +const reads = () => ({ approvals: approvalsReads(), inbox: inboxFinds.length }); + +const settle = () => act(async () => { await vi.advanceTimersByTimeAsync(0); }); +const advance = (ms: number) => act(async () => { await vi.advanceTimersByTimeAsync(ms); }); + +function setHidden(hidden: boolean): void { + Object.defineProperty(document, 'hidden', { configurable: true, get: () => hidden }); + Object.defineProperty(document, 'visibilityState', { + configurable: true, + get: () => (hidden ? 'hidden' : 'visible'), + }); +} + +function mountBothFeeds() { + return renderHook(() => { + useSharedInboxFeed(); + useSharedPendingApprovalsCount(); + }); +} + +async function setChannel(open: boolean): Promise { + await act(async () => { + setUserFeedPushChannelOpen(open); + await vi.advanceTimersByTimeAsync(0); + }); +} + +beforeEach(() => { + vi.useFakeTimers(); + inboxFinds.length = 0; + setHidden(false); + __resetSharedUserFeeds(); + __markFirstPainted(); + vi.stubGlobal( + 'fetch', + vi.fn(() => Promise.resolve(new Response('{"data":[],"total":0}', { status: 200 }))), + ); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.useRealTimers(); + setHidden(false); + __resetSharedUserFeeds(); +}); + +describe('objectstack#27626 — the bell polls only as the fallback of a push channel', () => { + it('with no channel, each feed reads at mount and then once per 60 s, never sooner', async () => { + mountBothFeeds(); + await settle(); + expect(reads()).toEqual({ approvals: 1, inbox: 1 }); + + await advance(FALLBACK_MS - 1); + expect(reads()).toEqual({ approvals: 1, inbox: 1 }); + + await advance(1); + expect(reads()).toEqual({ approvals: 2, inbox: 2 }); + + await advance(9 * FALLBACK_MS); + expect(reads()).toEqual({ approvals: 11, inbox: 11 }); + }); + + it('with a channel open, nothing polls, and a pushed change reads only its own feed', async () => { + mountBothFeeds(); + await settle(); + await setChannel(true); + expect(reads()).toEqual({ approvals: 1, inbox: 1 }); + + await advance(10 * FALLBACK_MS); + expect(reads()).toEqual({ approvals: 1, inbox: 1 }); + expect(vi.getTimerCount()).toBe(0); + + await act(async () => { + notifyUserFeedPushed('inbox'); + await vi.advanceTimersByTimeAsync(0); + }); + expect(reads()).toEqual({ approvals: 1, inbox: 2 }); + + await act(async () => { + notifyUserFeedPushed('approvals'); + await vi.advanceTimersByTimeAsync(0); + }); + expect(reads()).toEqual({ approvals: 2, inbox: 2 }); + + // A pushed read arms no poll behind it. + await advance(10 * FALLBACK_MS); + expect(reads()).toEqual({ approvals: 2, inbox: 2 }); + }); + + it('a channel already open when the bell mounts reads once and then waits for pushes', async () => { + await setChannel(true); + mountBothFeeds(); + await settle(); + expect(reads()).toEqual({ approvals: 1, inbox: 1 }); + + await advance(10 * FALLBACK_MS); + expect(reads()).toEqual({ approvals: 1, inbox: 1 }); + }); + + it('the channel closing reads each feed once at once, then resumes the 60 s fallback', async () => { + mountBothFeeds(); + await settle(); + await setChannel(true); + await advance(5 * FALLBACK_MS); + expect(reads()).toEqual({ approvals: 1, inbox: 1 }); + + await setChannel(false); + expect(reads()).toEqual({ approvals: 2, inbox: 2 }); + + await advance(FALLBACK_MS - 1); + expect(reads()).toEqual({ approvals: 2, inbox: 2 }); + await advance(1); + expect(reads()).toEqual({ approvals: 3, inbox: 3 }); + }); + + it('returning to the tab while the channel is open reads nothing', async () => { + mountBothFeeds(); + await settle(); + await setChannel(true); + + setHidden(true); + await act(async () => { + document.dispatchEvent(new Event('visibilitychange')); + await vi.advanceTimersByTimeAsync(0); + }); + setHidden(false); + await act(async () => { + document.dispatchEvent(new Event('visibilitychange')); + await vi.advanceTimersByTimeAsync(0); + }); + + expect(reads()).toEqual({ approvals: 1, inbox: 1 }); + }); + + it('a pushed change for a feed nobody shows reads nothing', async () => { + const { unmount } = mountBothFeeds(); + await settle(); + unmount(); + await act(async () => { + notifyUserFeedPushed('inbox'); + await vi.advanceTimersByTimeAsync(0); + }); + expect(reads()).toEqual({ approvals: 1, inbox: 1 }); + }); +}); diff --git a/ui/packages/app-shell/src/hooks/sharedUserFeeds.ts b/ui/packages/app-shell/src/hooks/sharedUserFeeds.ts index 7459d047cbf..fa340bcd75a 100644 --- a/ui/packages/app-shell/src/hooks/sharedUserFeeds.ts +++ b/ui/packages/app-shell/src/hooks/sharedUserFeeds.ts @@ -73,6 +73,18 @@ * the badge draws nothing, exactly as it does while a read is in flight, and * never a `0` it did not read. * + * ## A push channel replaces the poll (objectstack#27626) + * + * The inbox and approvals feeds poll only while no push channel is open, and + * then at 60 s or slower. A host that holds a channel announcing these changes + * reports it with {@link setUserFeedPushChannelOpen} and relays each change + * with {@link notifyUserFeedPushed}: while it is open neither feed arms a poll + * timer, a pushed change refetches its feed once, and the channel closing + * refetches once and resumes the fallback poll. Today no host opens one — the + * platform serves no realtime channel (discovery's `websockets` capability is + * off and the realtime service names no route) — so the 60 s fallback is what + * runs. The activity feed is not polled and has no push half. + * * @module */ import { useEffect, useMemo, useRef, useSyncExternalStore } from 'react'; @@ -97,10 +109,18 @@ import type { ActivityItem } from '../layout/ActivityFeed.js'; import { activityRowToActivityItem } from '../layout/activityItemType.js'; import type { InboxNotification } from '../layout/inboxGrouping.js'; -/** Approvals poll cadence — the bell's original 30s (M11.C15). */ -const APPROVALS_POLL_MS = 30_000; -/** Inbox poll cadence — the bell's original 10s (ADR-0030 L5, #4110). */ -const INBOX_POLL_MS = 10_000; +/** + * Approvals poll cadence. Polling is the FALLBACK of a push channel + * (objectstack#27626, see "A push channel replaces the poll" above), so it is + * held at 60 s or slower; it was the bell's 30 s (M11.C15). + */ +const APPROVALS_POLL_MS = 60_000; +/** + * Inbox poll cadence, the same fallback rule (objectstack#27626); it was the + * bell's 10 s (ADR-0030 L5, #4110). A message can therefore reach the bell up + * to a minute after it is written while no push channel is open. + */ +const INBOX_POLL_MS = 60_000; /** * Cadence while the tab is backgrounded. The bell's own poller carried this * (60s hidden against 10s foregrounded) plus an immediate refetch when the @@ -228,6 +248,14 @@ type FeedRunner = (ctx: { markRefused: () => void; }) => Promise; +/** + * Whether a host reported an open push channel for the inbox and approvals + * feeds (objectstack#27626, {@link setUserFeedPushChannelOpen}), and the + * pushable feeds' reactions to it opening or closing. + */ +let pushChannelOpen = false; +const pushChannelListeners = new Set<() => void>(); + /** * One feed's shared state. Consumers `attach` (from an effect) and read via * `useSyncExternalStore`; the first one in starts the fetch and the poll, the @@ -256,11 +284,14 @@ class SharedFeed { /** Current inter-poll delay: `pollMs`, doubled per failure, capped. */ private backoffMs: number; private visibilityBound = false; + private pushBound = false; constructor( private readonly empty: T, /** Re-fetch cadence while at least one consumer is mounted; 0 = fetch once. */ private readonly pollMs: number, + /** Whether an open push channel replaces the poll (objectstack#27626). */ + private readonly pushable = false, ) { this.snapshot = { value: empty, status: 'idle' }; this.idleSnapshot = this.snapshot; @@ -309,6 +340,7 @@ class SharedFeed { this.consumers += 1; if (this.consumers === 1) { this.bindVisibility(); + this.bindPush(); this.schedule(); } void this.refresh(); @@ -317,10 +349,25 @@ class SharedFeed { if (this.consumers === 0) { this.stopPolling(); this.unbindVisibility(); + this.unbindPush(); } }; } + /** + * A change the push channel announced for this feed: read it now + * (objectstack#27626). Nothing is read for a feed nobody is showing. + */ + pushedChange(): void { + if (!this.pushable || this.consumers === 0) return; + void this.refresh(true); + } + + /** Whether an open push channel stands in for this feed's poll right now. */ + private pushCovers(): boolean { + return this.pushable && pushChannelOpen; + } + /** * `force` is the poll tick: it bypasses the freshness window (which exists * to collapse mounts, not to defeat the cadence). Concurrent callers are @@ -444,6 +491,9 @@ class SharedFeed { return; } if (this.pollMs <= 0) return; + // An open push channel replaces the poll (objectstack#27626); a retired + // feed's probes above are not polls and keep their own cadence. + if (this.pushCovers()) return; // Backgrounded tabs poll at the slower cadence — but never FASTER than the // current backoff, so a failing feed stays backed off either way. const hidden = typeof document !== 'undefined' && document.hidden; @@ -475,6 +525,9 @@ class SharedFeed { private readonly onVisibilityChange = (): void => { if (typeof document === 'undefined' || document.hidden) return; if (this.consumers === 0) return; + // An open push channel kept the feed current while the tab was away + // (objectstack#27626): nothing to catch up. + if (this.pushCovers() && !this.unavailable) return; // Returning to a tab is exactly when a bell should be current, so a retired // feed re-probes here too — but under the same cadence gate as the timer, // or a user switching tabs would spend the whole budget in three seconds @@ -498,6 +551,32 @@ class SharedFeed { this.visibilityBound = false; } + /** + * The push channel opened or closed (objectstack#27626). Opening stops the + * poll. Closing reads once — what the channel would have announced since is + * unknown — and that read's `finally` re-arms the fallback poll. A retired + * feed's probe timer is left alone either way. + */ + private readonly onPushChannelChange = (): void => { + if (this.consumers === 0 || this.unavailable) return; + this.stopPolling(); + if (this.pushCovers()) return; + this.backoffMs = this.pollMs; + void this.refresh(true); + }; + + private bindPush(): void { + if (!this.pushable || this.pushBound) return; + pushChannelListeners.add(this.onPushChannelChange); + this.pushBound = true; + } + + private unbindPush(): void { + if (!this.pushBound) return; + pushChannelListeners.delete(this.onPushChannelChange); + this.pushBound = false; + } + private stopPolling(): void { if (this.timer) { clearTimeout(this.timer); @@ -523,6 +602,7 @@ class SharedFeed { reset(): void { this.stopPolling(); this.unbindVisibility(); + this.unbindPush(); this.snapshot = { value: this.empty, status: 'idle' }; this.key = null; this.runner = null; @@ -728,7 +808,7 @@ const NO_APPROVALS: PendingApprovalPage = { requests: [], total: 0 }; * the bell cannot disagree about how many are waiting (objectui#13027 moved * the count from the list's length to the page's `total`, still one read). */ -const approvalsFeed = new SharedFeed(NO_APPROVALS, APPROVALS_POLL_MS); +const approvalsFeed = new SharedFeed(NO_APPROVALS, APPROVALS_POLL_MS, true); function str(v: unknown): string | undefined { return typeof v === 'string' && v.trim() ? v : undefined; @@ -1274,7 +1354,7 @@ export function useHumanActivityFeed(limit: number): ActivityFeedReading { // ── Inbox messages ─────────────────────────────────────────────────────────── -const inboxFeed = new SharedFeed(NO_MESSAGES, INBOX_POLL_MS); +const inboxFeed = new SharedFeed(NO_MESSAGES, INBOX_POLL_MS, true); /** * Receipt states that count as READ (ADR-0030). `delivered` is not one of them @@ -1360,8 +1440,8 @@ function listedNotificationIds(rows: readonly unknown[]): string[] { * The signed-in user's 20 most recent in-app inbox messages, joined with their * read-state receipts (ADR-0030 L5, the `mine` materialization). * - * Two scoped reads, joined client-side, polled at 10s while the tab is - * foregrounded — the bell's cadence, now the store's: + * Two scoped reads, joined client-side, polled at `INBOX_POLL_MS` (the 60 s + * fallback of a push channel, objectstack#27626) — the store's cadence: * - `sys_inbox_message` filtered by `user_id`, newest first, `$top: 20`. * - `sys_notification_receipt` filtered by `user_id` + `channel:'inbox'`, * narrowed to the notification ids that message read just listed @@ -1452,6 +1532,34 @@ export function useSharedInboxFeed(): SharedFeedSnapshot { }); } +// ── Push channel seam (objectstack#27626) ──────────────────────────────────── + +/** + * Report whether a push channel that announces changes to the signed-in user's + * inbox and pending approvals is open. While it is, neither feed polls; when it + * closes, each reads once and resumes its 60 s fallback poll. The host that + * holds the channel calls this on its open and close, and + * {@link notifyUserFeedPushed} on each change it carries. + * + * No host calls it today: the platform serves no realtime channel, so the + * fallback poll is what runs. Package-internal: not exported from + * `./index.ts` or the package entry. + */ +export function setUserFeedPushChannelOpen(open: boolean): void { + if (pushChannelOpen === open) return; + pushChannelOpen = open; + for (const listener of [...pushChannelListeners]) listener(); +} + +/** + * The push channel announced a change to one feed: that feed reads once, now, + * whether or not its poll is running. Package-internal, like + * {@link setUserFeedPushChannelOpen}. + */ +export function notifyUserFeedPushed(feed: 'inbox' | 'approvals'): void { + (feed === 'inbox' ? inboxFeed : approvalsFeed).pushedChange(); +} + /** * Test seam: drop every shared feed's cached value, key and in-flight state so * cases do not inherit each other's reads. Not part of the public surface. @@ -1461,6 +1569,7 @@ export function __resetSharedUserFeeds(): void { // not a feed's state. A case that needs to watch for it again calls // `__resetFirstPaint`. __resetHiddenTabClock(); + pushChannelOpen = false; approvalsFeed.reset(); activityFeed.reset(); inboxFeed.reset(); diff --git a/ui/packages/data-objectstack/src/getRecordFieldAccess.inflightShare-27626.test.ts b/ui/packages/data-objectstack/src/getRecordFieldAccess.inflightShare-27626.test.ts new file mode 100644 index 00000000000..bbeb7f79ebf --- /dev/null +++ b/ui/packages/data-objectstack/src/getRecordFieldAccess.inflightShare-27626.test.ts @@ -0,0 +1,179 @@ +/** + * 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. + */ + +/** + * objectstack#27626 — opening one record asked `GET /data/OBJ/ID/field-access` + * four times in the same millisecond: `record:details` and `record:highlights` + * each mount a `RecordFieldAccessScope`, and the drawer's form and the page + * host ask too. Every ask reaches `getRecordFieldAccess`, so the adapter is + * where the four become one. + * + * Measured through the REAL adapter and the REAL client over a transport that + * holds every data request open until the test answers it, so "at once" means + * the first request is genuinely still pending when the next call is made. + * + * Pinned: + * - concurrent asks for one record share one request and one answer; + * - a settled projection is not remembered: the scopes re-ask after a save, + * and that ask must reach the server (this is not a response cache); + * - "no answer" is remembered — a record the read answers 404 for stays + * unanswered until a write to its object, and a server without the route + * (501) is not asked again for any record; + * - a write to the object drops both the shared ask and the remembered 404, + * so an ask made after the save is never answered from before it. + */ +import { describe, it, expect, vi } from 'vitest'; +import { ObjectStackAdapter } from './index'; + +const BASE = 'http://test.local'; + +interface HeldRequest { + method: string; + path: string; + respond: (status: number, body: unknown) => void; +} + +function json(status: number, body: unknown): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'Content-Type': 'application/json' }, + }); +} + +/** A connected adapter whose data requests wait until the test answers them. */ +async function heldAdapter() { + const held: HeldRequest[] = []; + const fetch = vi.fn((input: RequestInfo | URL, init?: RequestInit) => { + const url = + typeof input === 'string' ? input : input instanceof URL ? input.href : input.url; + const method = ( + init?.method ?? (typeof input === 'object' && 'method' in input ? input.method : 'GET') + ).toUpperCase(); + if (url.endsWith('/api/v1/discovery')) { + return Promise.resolve(json(200, { success: true, data: { capabilities: {}, routes: {} } })); + } + return new Promise((resolve) => { + held.push({ + method, + path: url.replace(BASE, ''), + respond: (status, body) => resolve(json(status, body)), + }); + }); + }); + const ds = new ObjectStackAdapter({ baseUrl: BASE, fetch, autoReconnect: false }); + await ds.connect(); + return { ds, held }; +} + +/** Let every queued continuation run, so a call that WOULD fetch has fetched. */ +async function drain() { + for (let i = 0; i < 5; i++) await new Promise((r) => setTimeout(r, 0)); +} + +const accessOf = (path: string) => /\/field-access$/.test(path); +const ACCESS = { object: 'fa_contact', recordId: 'c1', readable: ['name'], editable: ['name'] }; +const NOT_FOUND = { success: false, error: { code: 'RECORD_NOT_FOUND', message: 'Record c1 not found' } }; + +describe('[#27626] getRecordFieldAccess shares one request per record', () => { + it('four concurrent asks for one record send ONE request and all get its answer', async () => { + const { ds, held } = await heldAdapter(); + const asks = [1, 2, 3, 4].map(() => ds.getRecordFieldAccess('fa_contact', 'c1')); + await drain(); + const wire = held.filter((r) => accessOf(r.path)); + expect(wire).toHaveLength(1); + expect(wire[0].path).toBe('/api/v1/data/fa_contact/c1/field-access'); + wire[0].respond(200, { success: true, data: ACCESS }); + await expect(Promise.all(asks)).resolves.toEqual([ACCESS, ACCESS, ACCESS, ACCESS]); + }); + + it('another record is another question', async () => { + const { ds, held } = await heldAdapter(); + void ds.getRecordFieldAccess('fa_contact', 'c1'); + void ds.getRecordFieldAccess('fa_contact', 'c2'); + void ds.getRecordFieldAccess('fa_account', 'c1'); + await drain(); + expect(held.filter((r) => accessOf(r.path)).map((r) => r.path)).toEqual([ + '/api/v1/data/fa_contact/c1/field-access', + '/api/v1/data/fa_contact/c2/field-access', + '/api/v1/data/fa_account/c1/field-access', + ]); + }); + + it('a settled projection is not remembered — the next ask reaches the server', async () => { + const { ds, held } = await heldAdapter(); + const first = ds.getRecordFieldAccess('fa_contact', 'c1'); + await drain(); + held.filter((r) => accessOf(r.path))[0].respond(200, { success: true, data: ACCESS }); + await first; + void ds.getRecordFieldAccess('fa_contact', 'c1'); + await drain(); + expect(held.filter((r) => accessOf(r.path))).toHaveLength(2); + }); + + it('a 404 is remembered as no answer for that record, and only that record', async () => { + const { ds, held } = await heldAdapter(); + const first = ds.getRecordFieldAccess('fa_contact', 'c1'); + await drain(); + held.filter((r) => accessOf(r.path))[0].respond(404, NOT_FOUND); + await expect(first).resolves.toBeNull(); + await expect(ds.getRecordFieldAccess('fa_contact', 'c1')).resolves.toBeNull(); + await expect(ds.getRecordFieldAccess('fa_contact', 'c1')).resolves.toBeNull(); + await drain(); + expect(held.filter((r) => accessOf(r.path))).toHaveLength(1); + // Control: a different record of the same object is still asked. + void ds.getRecordFieldAccess('fa_contact', 'c2'); + await drain(); + expect(held.filter((r) => accessOf(r.path))).toHaveLength(2); + }); + + it('a server without the route (501) is not asked again for any record', async () => { + const { ds, held } = await heldAdapter(); + const first = ds.getRecordFieldAccess('fa_contact', 'c1'); + await drain(); + held.filter((r) => accessOf(r.path))[0].respond(501, { + success: false, + error: { code: 'NOT_IMPLEMENTED', message: 'no record-scoped projection' }, + }); + await expect(first).resolves.toBeNull(); + await expect(ds.getRecordFieldAccess('fa_contact', 'c2')).resolves.toBeNull(); + await expect(ds.getRecordFieldAccess('fa_account', 'a9')).resolves.toBeNull(); + await drain(); + expect(held.filter((r) => accessOf(r.path))).toHaveLength(1); + }); + + it('a write to the object drops the remembered 404 and the shared ask', async () => { + const { ds, held } = await heldAdapter(); + const first = ds.getRecordFieldAccess('fa_contact', 'c1'); + await drain(); + held.filter((r) => accessOf(r.path))[0].respond(404, NOT_FOUND); + await first; + + // An ask on the wire when the write lands was asked before it. + void ds.getRecordFieldAccess('fa_contact', 'c2'); + await drain(); + expect(held.filter((r) => accessOf(r.path))).toHaveLength(2); + + const save = ds.update('fa_contact', 'c1', { name: 'Ada' }); + await drain(); + held.find((r) => r.method === 'PATCH' || r.method === 'PUT')!.respond(200, { + success: true, + data: { id: 'c1', name: 'Ada' }, + }); + await save; + + void ds.getRecordFieldAccess('fa_contact', 'c1'); + void ds.getRecordFieldAccess('fa_contact', 'c2'); + await drain(); + expect(held.filter((r) => accessOf(r.path)).map((r) => r.path)).toEqual([ + '/api/v1/data/fa_contact/c1/field-access', + '/api/v1/data/fa_contact/c2/field-access', + '/api/v1/data/fa_contact/c1/field-access', + '/api/v1/data/fa_contact/c2/field-access', + ]); + }); +}); diff --git a/ui/packages/data-objectstack/src/index.ts b/ui/packages/data-objectstack/src/index.ts index a258fe17a1e..3cee2ae00e1 100644 --- a/ui/packages/data-objectstack/src/index.ts +++ b/ui/packages/data-objectstack/src/index.ts @@ -3296,6 +3296,17 @@ export class ObjectStackAdapter implements DataSource { // write to a resource drops that resource's entries (see `emitMutation`), // so a read asked after a save is never answered by one sent before it. private inflightFindOnes = new Map>(); + // objectstack#27626 — `getRecordFieldAccess` asks, keyed by resource + id. + // One record open asked the same projection four times in the same + // millisecond (`record:details`, `record:highlights`, the form and the + // host), so concurrent asks share one request. A settled projection is not + // kept — the scopes re-ask after a save and must reach the server — but "no + // answer" is: a record the read answers 404 for stays unanswered until a + // write to its resource (both maps are dropped in `dropInflightReads`), and + // a server without the route (501) is not asked again for any record. + private inflightFieldAccess = new Map>(); + private unansweredFieldAccess = new Set(); + private fieldAccessUnsupported = false; // Resources that have responded 404 at least once (collection not installed // on this backend). Subsequent find() calls short-circuit to an empty result // so optional collections like sys_presence don't hammer the server with @@ -3918,6 +3929,14 @@ export class ObjectStackAdapter implements DataSource { for (const key of Array.from(this.inflightFindOnes.keys())) { if (key.startsWith(prefix)) this.inflightFindOnes.delete(key); } + // objectstack#27626 — the record field projection's shared asks and its + // remembered 404s, by the same rule. + for (const key of Array.from(this.inflightFieldAccess.keys())) { + if (key.startsWith(prefix)) this.inflightFieldAccess.delete(key); + } + for (const key of Array.from(this.unansweredFieldAccess)) { + if (key.startsWith(prefix)) this.unansweredFieldAccess.delete(key); + } } /** Hand one event to every mutation subscriber, each isolated. */ @@ -7262,16 +7281,34 @@ export class ObjectStackAdapter implements DataSource { * `null` when there is no answer to give: a record the caller cannot see * (the route answers the record read's own 404), or a server with no * record-scoped projection (501). Any other failure is thrown, as a read's. + * + * Concurrent asks for one record share one request, and both kinds of "no + * answer" are remembered (objectstack#27626, see `inflightFieldAccess`). */ async getRecordFieldAccess(resource: string, id: string): Promise { - await this.connect(); - try { - const access = await this.client.data.fieldAccess(resource, String(id)); - return access ?? null; - } catch (error: unknown) { - if (is404Error(error) || isNotImplementedError(error)) return null; - throw error; - } + const key = `${resource}::${String(id)}`; + if (this.fieldAccessUnsupported || this.unansweredFieldAccess.has(key)) return null; + const ask: Promise = this.shareInFlight(this.inflightFieldAccess, key, async () => { + await this.connect(); + try { + const access = await this.client.data.fieldAccess(resource, String(id)); + return access ?? null; + } catch (error: unknown) { + if (isNotImplementedError(error)) { + this.fieldAccessUnsupported = true; + return null; + } + if (is404Error(error)) { + // Not remembered when a write to the resource landed while this ask + // was on the wire: the write dropped the entry, and the record may + // have moved into the caller's sight since. + if (this.inflightFieldAccess.get(key) === ask) this.unansweredFieldAccess.add(key); + return null; + } + throw error; + } + }); + return ask; } /** From 03b2def18b3c6a7a69ac4806248690ec65264084 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 03:13:06 +0000 Subject: [PATCH 2/2] perf(console): opening a record keeps the list under it, and the drawer reads the registry beside the scope The record drawer made the route read the whole object registry instead of the app's object scope, and the route slot waited for it: the first open swapped the list for the route skeleton and remounted it (re-reading its rows and its count), and every later open and close handed the list a different document for its object, which rebuilt its columns and re-ran the list query. The drawer now reads the registry beside the route's scope without holding the route, and the registry joins the scope once it lands with the scope's own documents kept. The list page holds columns and searchableFields by value as it holds the view-type blocks. The record form overlay still reads the whole registry. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01Utp6CXJLrFrzeHHa8StCja --- .changeset/27626-record-open-requests.md | 15 + .../app-shell/src/console/AppContent.tsx | 38 ++- .../AppContent.objectScope-12811.test.tsx | 7 +- ...tent.recordDrawerKeepsScope-27626.test.tsx | 307 ++++++++++++++++++ .../__tests__/appObjectScope-12811.test.tsx | 16 +- .../app-shell/src/console/appObjectScope.ts | 24 +- .../sharedInboxFeed.transient404.test.tsx | 18 +- ...View.objectDocSwapNoRefetch-27626.test.tsx | 216 ++++++++++++ .../src/views/objectViewListRenderer.tsx | 7 +- 9 files changed, 623 insertions(+), 25 deletions(-) create mode 100644 .changeset/27626-record-open-requests.md create mode 100644 ui/packages/app-shell/src/console/__tests__/AppContent.recordDrawerKeepsScope-27626.test.tsx create mode 100644 ui/packages/app-shell/src/views/ObjectView.objectDocSwapNoRefetch-27626.test.tsx diff --git a/.changeset/27626-record-open-requests.md b/.changeset/27626-record-open-requests.md new file mode 100644 index 00000000000..97b8ea6f350 --- /dev/null +++ b/.changeset/27626-record-open-requests.md @@ -0,0 +1,15 @@ +--- +'@object-ui/data-objectstack': patch +'@object-ui/app-shell': patch +--- + +perf(console): opening a record no longer re-reads the list under it, asks its field access once, and the bell polls once a minute + +Clause-②: no +Metadata-protocol: none + +Measured in the console on app-crm, opening one record from a list and closing it again: + +- **The list under the drawer is not read again.** The first record opened replaced the list with the route skeleton (the drawer made the route read the whole object registry instead of the app's object scope, and the route waited for it), so the list mounted again and re-read its rows; every later open and close handed the list a different document for its object, which rebuilt its columns and re-ran the list query. The drawer now reads the whole registry beside the route's scope, without holding the route, and the registry joins the scope once it lands with the scope's own documents kept as they are. The list page also holds `columns` and `searchableFields` by value, as it holds the view-type blocks. The record form overlay (`?form=`) still reads the whole registry. +- **One field-access request per record.** The record's sections, highlights, form and page each asked `GET /data/:object/:id/field-access` at once; `ObjectStackAdapter.getRecordFieldAccess` now shares one request between concurrent asks for the same record, remembers a 404 for that record until a write to its object, and stops asking a server that answers 501. A settled answer is not cached: the next ask after a save reaches the server. +- **The bell polls once a minute.** The inbox and approvals feeds polled every 10 s and 30 s on every console page. With no realtime channel in the platform, they now poll every 60 s in the foreground, still stop in a hidden tab, and stop polling altogether while a host reports an open push channel (a pushed change reads the feed it names). A new inbox message can show up to about a minute later. diff --git a/ui/packages/app-shell/src/console/AppContent.tsx b/ui/packages/app-shell/src/console/AppContent.tsx index d7094fbe6ab..cf41e0a1cee 100644 --- a/ui/packages/app-shell/src/console/AppContent.tsx +++ b/ui/packages/app-shell/src/console/AppContent.tsx @@ -38,7 +38,7 @@ import { useLauncherApps } from '../hooks/useLauncherApps.js'; import { useSignedInUserLocale } from '../hooks/useUserLocale.js'; import { resolveRecordFormTarget, resolveRecordFormModalLayout, resolveFormViewChrome, selectRecordFormView, resolveNavigateCreateUrl, resolveNavigateEditUrl, resolvePostCreateTarget, recordFormCreateValues, readRecordFormCreateValues, RECORD_FORM_CREATE_VALUES_STATE } from '../utils/recordFormNavigation.js'; import { deriveRecordSurface, deriveRecordFlowSurface } from '@object-ui/plugin-view'; -import { RECORD_FORM_PARAM, RECORD_FORM_OBJECT_PARAM, RECORD_FORM_LINK_PARAM } from '../urlParams.js'; +import { RECORD_DRAWER_PARAM, RECORD_FORM_PARAM, RECORD_FORM_OBJECT_PARAM, RECORD_FORM_LINK_PARAM } from '../urlParams.js'; import { appAddressSegment, appRouteSegment, canonicalAppPathname, matchAppBySegment } from '../utils/appRoute.js'; import { resolveHref, type NavTemplateContext } from '@object-ui/layout'; import type { NavigationItem } from '@object-ui/types'; @@ -462,8 +462,23 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = // Until the app list has loaded, a scope candidate does not know its package // yet, and reads nothing (the splash is up). const readsWholeRegistry = !scopeCandidate || (!metadataLoading && !objectScope); + // objectstack#27626 — a record drawer (`?recordId=`) over a scoped route + // reads the whole registry for its related lists, BESIDE the route's scope: + // the route slot never waits on it, and once it has landed it joins the + // scope, whose own documents keep their identity. The list under the drawer + // reads the same object definition before, during and after it, so opening + // or closing a record re-reads none of its rows (the skeleton this used to + // show remounted the list on every first open). + const drawerReadsRegistry = !!objectScope && searchParamsHasRecordDrawer(location.search); + const drawerRegistrySettled = useLazyMetadataSettled(drawerReadsRegistry ? 'object' : '', ensureType, getTypeStatus); + const scopeObjects: any[] | undefined = objectScope?.objects; + const registryObjects: any[] | undefined = drawerReadsRegistry && drawerRegistrySettled ? metadata.objects : undefined; + const scopeWithRegistry = useMemo( + () => (scopeObjects && registryObjects ? joinRegistryToScope(scopeObjects, registryObjects) : undefined), + [scopeObjects, registryObjects], + ); const allObjects: any[] = objectScope - ? objectScope.objects + ? scopeWithRegistry ?? objectScope.objects : readsWholeRegistry ? metadata.objects : NO_OBJECTS; const routeReadsKey = [ ...routeReads.filter((type) => type !== 'object' || readsWholeRegistry), @@ -1941,6 +1956,25 @@ function searchParamsHasRecordForm(search: string): boolean { return sp.has(RECORD_FORM_PARAM) || sp.has(RECORD_FORM_OBJECT_PARAM); } +/** + * objectstack#27626 — whether the record drawer is open in the URL. Over a + * scoped route it reads the whole registry beside the scope. + */ +function searchParamsHasRecordDrawer(search: string): boolean { + return new URLSearchParams(search).has(RECORD_DRAWER_PARAM); +} + +/** + * objectstack#27626 — the scope's objects, each the same document, then the + * registry's objects the scope does not hold. The scope itself while the + * registry adds nothing. + */ +function joinRegistryToScope(scope: any[], registry: any[]): any[] { + const held = new Set(scope.map((o: any) => o?.name)); + const extra = registry.filter((o: any) => !held.has(o?.name)); + return extra.length > 0 ? [...scope, ...extra] : scope; +} + const NO_OBJECTS: any[] = []; /** diff --git a/ui/packages/app-shell/src/console/__tests__/AppContent.objectScope-12811.test.tsx b/ui/packages/app-shell/src/console/__tests__/AppContent.objectScope-12811.test.tsx index 8777695934c..aff3802a469 100644 --- a/ui/packages/app-shell/src/console/__tests__/AppContent.objectScope-12811.test.tsx +++ b/ui/packages/app-shell/src/console/__tests__/AppContent.objectScope-12811.test.tsx @@ -22,7 +22,9 @@ * and so does the root of an app whose package holds no objects of its own (a * platform app such as Setup; its object routes read by name, objectui#13046); * a record overlay (`?recordId=`) reads the whole registry, because a record's - * related lists come from every object that points at it. + * related lists come from every object that points at it (since + * objectstack#27626 the drawer reads it beside the list's scope, and the list + * does not wait for it: `AppContent.recordDrawerKeepsScope-27626.test.tsx`). */ import '@testing-library/jest-dom/vitest'; @@ -253,10 +255,9 @@ describe('one console open asks for the app\'s objects, not the whole registry ( it('CONTROL: a record overlay reads the whole registry (a record\'s related lists come from every object)', async () => { boot('/apps/crm/crm_account?recordId=r1'); - await waitFor(() => expect(screen.getByTestId('object-view')).toBeInTheDocument()); + await waitFor(() => expect(screen.getByTestId('object-view')).toHaveTextContent('crm_case')); expect(objectCalls()).toContain('object'); - expect(screen.getByTestId('object-view')).toHaveTextContent('crm_case'); }); }); diff --git a/ui/packages/app-shell/src/console/__tests__/AppContent.recordDrawerKeepsScope-27626.test.tsx b/ui/packages/app-shell/src/console/__tests__/AppContent.recordDrawerKeepsScope-27626.test.tsx new file mode 100644 index 00000000000..c7547c7e14e --- /dev/null +++ b/ui/packages/app-shell/src/console/__tests__/AppContent.recordDrawerKeepsScope-27626.test.tsx @@ -0,0 +1,307 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack#27626 — opening a record from a scoped list keeps the list. + * + * Measured in the console (app-crm): the first record opened from a list + * replaced the list with the route skeleton, then mounted it again, and the + * remounted list read its rows again. The drawer (`?recordId=`) made the route + * read the whole registry INSTEAD of the app's object scope (a record's related + * lists come from every object that points at it), and the route slot waited + * for that registry. Every later open and close also handed the list a + * different document for its object (the registry's, then the scope's). + * + * Pinned through the REAL `MetadataProvider` and the real `AppContent`, with + * the `AppContent.objectScope-12811` harness and a whole-registry read the test + * holds open: + * - the route keeps its scope under the drawer and never waits for the + * registry: no skeleton, the route element is never remounted; + * - the drawer still reads the whole registry, and it joins the scope once it + * lands (the record's related lists see every object); + * - the list's object keeps ONE document before, during and after the + * drawer, cold and warm; + * - a deep link to a drawer draws the list on its scope before the registry + * has landed. + * CONTROL: the record form overlay (`?form=`) still reads the whole registry + * instead of the scope. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import React, { useEffect } from 'react'; +import { act, cleanup, render, screen, waitFor } from '@testing-library/react'; +import { MemoryRouter, Routes, Route, useNavigate, type NavigateFunction } from 'react-router-dom'; +import { AuthProvider, ActiveOrganizationStorage } from '@object-ui/auth'; +import type { AuthClient, AuthOrganization } from '@object-ui/auth'; + +vi.mock('@object-ui/plugin-designer', async (importOriginal) => ({ + ...(await importOriginal()), + CreateAppPage: () => null, + EditAppPage: () => null, + DashboardDesignPage: () => null, +})); +// The chrome: what the sidebar's capability gate asks, and the list it is handed. +vi.mock('../../layout/ConsoleLayout', async () => { + const { useNavMemberGates } = await import('../../hooks/useNavMemberGates'); + return { + ConsoleLayout: ({ children, objects }: { children?: React.ReactNode; objects: Array<{ name: string }> }) => { + const gates = useNavMemberGates(); + return ( +
o.name).join(',')}> + {String(gates.checkCapability('object', 'cloud_only'))} + {children} +
+ ); + }, + }; +}); +vi.mock('../../chrome/CommandPalette', () => ({ CommandPalette: () => null })); +vi.mock('../../chrome/KeyboardShortcutsDialog', () => ({ KeyboardShortcutsDialog: () => null })); +vi.mock('../../chrome/OnboardingWalkthrough', () => ({ OnboardingWalkthrough: () => null })); +const routeElement = vi.hoisted(() => ({ mounts: 0, docs: [] as unknown[] })); +vi.mock('../../views/ObjectView', async () => { + const { useEffect: useMountEffect } = await import('react'); + return { + ObjectView: ({ objects }: { objects: Array<{ name: string }> }) => { + useMountEffect(() => { + routeElement.mounts++; + }, []); + const doc = objects.find((o) => o.name === 'crm_account'); + if (doc && routeElement.docs[routeElement.docs.length - 1] !== doc) routeElement.docs.push(doc); + return
{objects.map((o) => o.name).join(',')}
; + }, + }; +}); +vi.mock('@object-ui/i18n', async (importOriginal) => ({ + ...(await importOriginal>()), + useObjectTranslation: () => ({ + t: (key: string, options?: Record) => String(options?.defaultValue ?? key), + }), + useObjectLabel: () => ({ objectLabel: ({ label }: { label?: string }) => label, objectPluralLabel: () => '' }), +})); + +const dataSourceStub = { + onConnectionStateChange: () => () => {}, + getConnectionState: () => 'connected', +}; +vi.mock('../../providers/AdapterProvider', async (importOriginal) => ({ + ...(await importOriginal>()), + useAdapter: () => dataSourceStub, +})); + +const actionRunnerStub = { registerHandler: vi.fn(), getContext: () => ({}) }; +vi.mock('@object-ui/react', async (importOriginal) => ({ + ...(await importOriginal>()), + useActionRunner: () => ({ execute: vi.fn(), runner: actionRunnerStub }), + useGlobalUndo: () => {}, + useMutationInvalidationBridge: () => {}, +})); + +import { AppContent } from '../AppContent'; +import { MetadataProvider } from '../../providers/MetadataProvider'; +import { useActionModal } from '../../hooks/useActionModal'; + +const CRM = 'app.crm'; +const OBJECTS = [ + { name: 'crm_account', _packageId: CRM, fields: { owner: { type: 'lookup', reference: 'sys_user' }, parent: { type: 'lookup', reference: 'crm_account' } } }, + { name: 'crm_contact', _packageId: CRM, fields: { account: { type: 'lookup', reference: 'crm_account' } } }, + { name: 'crm_case', _packageId: 'app.service', fields: { account: { type: 'lookup', reference: 'crm_account' } } }, + { name: 'sys_user', _packageId: 'plugin.auth', fields: {} }, +]; +const NAVIGATION = [ + { id: 'n1', type: 'object', label: 'Accounts', objectName: 'crm_account' }, + { id: 'n2', type: 'object', label: 'Cases', objectName: 'crm_case' }, + // An entry for an object this runtime may not register (a cloud-only one). + { id: 'n3', type: 'object', label: 'Cloud', objectName: 'cloud_only', requiresObject: 'cloud_only' }, +]; + +interface Server { + calls: string[]; + apps: unknown[]; + holdByName: boolean; + release: () => void; + /** Hold the whole-registry read (`meta/object` with no package) until `releaseRegistry`. */ + holdRegistry: boolean; + releaseRegistry: () => void; +} +const server: Server = { + calls: [], apps: [], holdByName: false, release: () => {}, holdRegistry: false, releaseRegistry: () => {}, +}; + +function makeAdapter() { + const held: Array<() => void> = []; + server.release = () => held.splice(0).forEach((go) => go()); + const heldRegistry: Array<() => void> = []; + server.releaseRegistry = () => heldRegistry.splice(0).forEach((go) => go()); + return { + clearCache: vi.fn(), + invalidateCache: vi.fn(), + // objectui#13044 — the provider's by-name OBJECT read goes through the + // adapter's `getObjectSchema`. This double answers it from the same + // by-name door, unwrapped the way the adapter unwraps it. + getObjectSchema(this: { getClient(): any }, name: string) { + return this.getClient() + .meta.getItem('object', name) + .then((res: any) => (res && typeof res === 'object' && 'item' in res ? res.item : res)); + }, + getClient: () => ({ + meta: { + getItems: (type: string, options?: { packageId?: string }) => { + server.calls.push(options?.packageId ? `${type}?package=${options.packageId}` : type); + const all = type === 'object' ? OBJECTS : type === 'app' ? server.apps : []; + const items = options?.packageId ? all.filter((o: any) => o._packageId === options.packageId) : all; + const answer = { type, items: structuredClone(items) }; + if (type === 'object' && !options?.packageId && server.holdRegistry) { + return new Promise((resolve) => heldRegistry.push(() => resolve(answer))); + } + return Promise.resolve(answer); + }, + getItem: (type: string, name: string) => { + server.calls.push(`${type}/${name}`); + const item = (type === 'object' ? OBJECTS : []).find((o) => o.name === name); + const answer = () => + item ? Promise.resolve({ type, name, item: structuredClone(item) }) : Promise.reject(Object.assign(new Error('not found'), { httpStatus: 404, code: 'RESOURCE_NOT_FOUND' })); + if (!server.holdByName) return answer(); + return new Promise((resolve, reject) => held.push(() => answer().then(resolve, reject))); + }, + }, + }), + } as unknown as Parameters[0]['adapter']; +} + +const USER = 'u_12811'; +const ORG: AuthOrganization = { id: 'org_a', name: 'Org A', slug: 'a' }; + +function authClient(): AuthClient { + return { + getSession: vi.fn(async () => ({ + // An admin by position: the workspace-admin verdict needs no member row. + user: { id: USER, name: 'Ada', email: 'ada@test.com', role: 'admin', positions: ['admin'] }, + session: { token: 'tok-12811', activeOrganizationId: ORG.id }, + })), + signOut: vi.fn(async () => undefined), + getConfig: vi.fn(async () => ({ features: {} })), + listOrganizations: vi.fn(async () => [ORG]), + getActiveOrganization: vi.fn(async () => ORG), + setActiveOrganization: vi.fn(async () => ORG), + getActiveMember: vi.fn(async () => ({ id: 'm1', organizationId: ORG.id, userId: USER, role: 'owner' })), + } as unknown as AuthClient; +} + +/** `ConsoleShell` mounts `useActionModal` at the console root. */ +function ActionModalHost() { + const { modalElement } = useActionModal(dataSourceStub); + return <>{modalElement}; +} + +const nav: { current: NavigateFunction | null } = { current: null }; +function NavProbe() { + const navigate = useNavigate(); + useEffect(() => { nav.current = navigate; }); + return null; +} + +function boot(url: string) { + render( + + + + + + + } /> + + + + , + ); +} + +const objectCalls = () => server.calls.filter((c) => c === 'object' || c.startsWith('object?') || c.startsWith('object/')); +const splashUp = () => document.querySelector('[data-boot-splash]') !== null; + +beforeEach(() => { + server.calls = []; + server.holdByName = false; + server.holdRegistry = false; + routeElement.mounts = 0; + routeElement.docs = []; + server.apps = [{ name: 'crm', label: 'CRM', _packageId: CRM, navigation: NAVIGATION }]; + localStorage.clear(); + sessionStorage.clear(); + ActiveOrganizationStorage.clear(); +}); + +afterEach(() => { + cleanup(); + ActiveOrganizationStorage.clear(); +}); + +const skeletonUp = () => screen.queryByTestId('route-metadata-skeleton') !== null; +const registryReads = () => server.calls.filter((c) => c === 'object').length; +const go = (to: string) => act(async () => { nav.current!(to); }); +const SCOPE = 'crm_account,crm_contact,sys_user'; + +describe('objectstack#27626 — a record drawer reads the registry beside the scope, never instead of it', () => { + it('cold: the list keeps its scope, is never remounted, and the registry joins it once it lands', async () => { + boot('/apps/crm/crm_account'); + await waitFor(() => expect(screen.getByTestId('object-view')).toHaveTextContent(SCOPE)); + expect(registryReads()).toBe(0); + expect(routeElement.mounts).toBe(1); + + server.holdRegistry = true; + await go('/apps/crm/crm_account?recordId=r1'); + await waitFor(() => expect(registryReads()).toBe(1)); + // The registry is out: the route draws on its scope meanwhile. + expect(skeletonUp()).toBe(false); + expect(screen.getByTestId('object-view')).toHaveTextContent(SCOPE); + + act(() => server.releaseRegistry()); + await waitFor(() => expect(screen.getByTestId('object-view')).toHaveTextContent('crm_case')); + expect(screen.getByTestId('object-view')).toHaveTextContent(`${SCOPE},crm_case`); + + await go('/apps/crm/crm_account'); + await waitFor(() => expect(screen.getByTestId('object-view')).not.toHaveTextContent('crm_case')); + + expect(routeElement.mounts).toBe(1); + expect(routeElement.docs).toHaveLength(1); + }); + + it('warm: a later open joins the registry at once, asks it no more, and keeps the same document', async () => { + boot('/apps/crm/crm_account'); + await waitFor(() => expect(screen.getByTestId('object-view')).toHaveTextContent(SCOPE)); + await go('/apps/crm/crm_account?recordId=r1'); + await waitFor(() => expect(screen.getByTestId('object-view')).toHaveTextContent('crm_case')); + await go('/apps/crm/crm_account'); + await waitFor(() => expect(screen.getByTestId('object-view')).not.toHaveTextContent('crm_case')); + + await go('/apps/crm/crm_account?recordId=r2'); + expect(screen.getByTestId('object-view')).toHaveTextContent('crm_case'); + expect(skeletonUp()).toBe(false); + await go('/apps/crm/crm_account'); + + expect(registryReads()).toBe(1); + expect(routeElement.mounts).toBe(1); + expect(routeElement.docs).toHaveLength(1); + }); + + it('a deep link to a drawer draws the list on its scope before the registry has landed', async () => { + server.holdRegistry = true; + boot('/apps/crm/crm_account?recordId=r1'); + await waitFor(() => expect(screen.getByTestId('object-view')).toHaveTextContent(SCOPE)); + expect(registryReads()).toBe(1); + expect(screen.getByTestId('object-view')).not.toHaveTextContent('crm_case'); + + act(() => server.releaseRegistry()); + await waitFor(() => expect(screen.getByTestId('object-view')).toHaveTextContent(`${SCOPE},crm_case`)); + expect(routeElement.mounts).toBe(1); + expect(routeElement.docs).toHaveLength(1); + }); + + it('CONTROL: the record form overlay still reads the whole registry instead of the scope', async () => { + boot('/apps/crm/crm_account?form=new'); + await waitFor(() => expect(registryReads()).toBe(1)); + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(objectCalls().some((c) => c.startsWith('object?'))).toBe(false); + }); +}); diff --git a/ui/packages/app-shell/src/console/__tests__/appObjectScope-12811.test.tsx b/ui/packages/app-shell/src/console/__tests__/appObjectScope-12811.test.tsx index 707c68c3099..45d0dc924f0 100644 --- a/ui/packages/app-shell/src/console/__tests__/appObjectScope-12811.test.tsx +++ b/ui/packages/app-shell/src/console/__tests__/appObjectScope-12811.test.tsx @@ -11,8 +11,10 @@ * asks the scope as soon as the app list names the app: the package list, * then by name the route object's lookup targets outside the package. * - * CONTROLs: a route outside an app, a record overlay and an app item with no - * package id ask nothing here (`AppContent` reads the whole registry there). + * CONTROLs: a route outside an app, a record form overlay and an app item + * with no package id ask nothing here (`AppContent` reads the whole registry + * there). A record drawer (`?recordId=`) asks the scope of the list under it, + * as `AppContent` does (objectstack#27626). */ import { describe, it, expect, vi, afterEach } from 'vitest'; @@ -88,9 +90,15 @@ describe('the console root starts the app object scope (objectui#12811)', () => expect(objectCalls(calls)).toEqual([`object?package=${CRM}`, 'object/sys_user']); }); - it('CONTROL: outside an app, on a record overlay, or for an app without a package id, it asks nothing', async () => { + it('objectstack#27626 — under a record drawer it asks the scope of the list the drawer opens over', async () => { + const calls = mount('/apps/crm/crm_account?recordId=r1', APPS); + await waitFor(() => expect(calls).toContain('object/sys_user')); + expect(objectCalls(calls)).toEqual([`object?package=${CRM}`, 'object/sys_user']); + }); + + it('CONTROL: outside an app, on a record form overlay, or for an app without a package id, it asks nothing', async () => { const outside = mount('/home', APPS); - const overlay = mount('/apps/crm/crm_account?recordId=r1', APPS); + const overlay = mount('/apps/crm/crm_account?form=new', APPS); const noPackage = mount('/apps/crm/crm_account', [{ name: 'crm', navigation: [] }]); await waitFor(() => expect(overlay).toContain('view')); await waitFor(() => expect(noPackage).toContain('view')); diff --git a/ui/packages/app-shell/src/console/appObjectScope.ts b/ui/packages/app-shell/src/console/appObjectScope.ts index ec27bc75010..dfbdcdf4ad7 100644 --- a/ui/packages/app-shell/src/console/appObjectScope.ts +++ b/ui/packages/app-shell/src/console/appObjectScope.ts @@ -43,7 +43,13 @@ * overlay the URL did not carry — `modal`, `split`, `popover` — until * objectstack#26677 retired those modes: every record overlay a list opens is * now the drawer, or the split pane a reader prefers, and both are carried by - * `?recordId=`.) An app item + * `?recordId=`.) The record drawer reads it BESIDE the route's scope, never + * instead of it (objectstack#27626): the route keeps its scope and is not + * held for the registry, which `AppContent` joins to the scope once it has + * landed. Dropping the scope for the registry had swapped the list for the + * route skeleton on every first open (remounting it, and re-reading its + * rows), and handed it new object documents on every open and close. The + * record form overlay still reads the whole registry instead. An app item * without a package id has no package to scope by. * * Two readers ask the same question with the same answer: `AppContent`, which @@ -57,7 +63,7 @@ import { isRecordPageOf, useMetadata, type MetadataContextValue } from '@object- import { defaultRecordPageObject } from '@objectstack/spec/ui'; import { APP_ROUTE_METADATA_READS } from './appRouteMetadataReads.js'; import { matchAppBySegment } from '../utils/appRoute.js'; -import { COMMAND_PALETTE_PARAM, RECORD_DRAWER_PARAM, RECORD_FORM_OBJECT_PARAM, RECORD_FORM_PARAM } from '../urlParams.js'; +import { COMMAND_PALETTE_PARAM, RECORD_FORM_OBJECT_PARAM, RECORD_FORM_PARAM } from '../urlParams.js'; type ReadObjectScope = NonNullable; export type AppObjectScope = ReturnType; @@ -268,10 +274,14 @@ function declaredRecordPage(pages: readonly unknown[], objectName: string, pageN return isRecord(page) && page.kind !== 'slotted' ? page : null; } -/** A record overlay the URL carries: the detail drawer, or the record form. */ -function hasRecordOverlay(search: string): boolean { +/** + * The record form overlay the URL carries. The record drawer (`?recordId=`) + * is not one here (objectstack#27626): the route under it keeps its scope, + * and `AppContent` reads the whole registry for the drawer beside it. + */ +function hasRecordFormOverlay(search: string): boolean { const sp = new URLSearchParams(search); - return sp.has(RECORD_DRAWER_PARAM) || sp.has(RECORD_FORM_PARAM) || sp.has(RECORD_FORM_OBJECT_PARAM); + return sp.has(RECORD_FORM_PARAM) || sp.has(RECORD_FORM_OBJECT_PARAM); } /** The objects an object's lookup and master-detail fields point at. */ @@ -318,7 +328,7 @@ function appRequiredObjectNames(app: Record | undefined): strin } export interface AppObjectScopeRead { - /** The route reads a scope when its app names a package: an app route on the scope list, no record overlay. */ + /** The route reads a scope when its app names a package: an app route on the scope list, no record form overlay. */ candidate: boolean; /** The app's object scope, or `null` when this route reads the whole registry. */ scope: AppObjectScope | null; @@ -361,7 +371,7 @@ export function readAppObjectScope( readPages?: () => PagesRead, ): AppObjectScopeRead { const route = matchObjectScopeRoute(subPath, hostPaths); - const candidate = !!route && !!readObjectScope && !hasRecordOverlay(search); + const candidate = !!route && !!readObjectScope && !hasRecordFormOverlay(search); const packageId = typeof app?._packageId === 'string' && app._packageId ? app._packageId : undefined; if (candidate && readObjectScope && route && route.recordId !== undefined && route.objectName) { // objectui#13046 — a record route: the app list, then the page list, then diff --git a/ui/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.transient404.test.tsx b/ui/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.transient404.test.tsx index 4fb4cc2996b..a5aa679c62d 100644 --- a/ui/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.transient404.test.tsx +++ b/ui/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.transient404.test.tsx @@ -235,9 +235,9 @@ describe('#4289 — a transient missing-resource answer does not cost the sessio it('resumes its normal cadence once revived, rather than staying on probes', async () => { // Recovery is not a one-shot read: a feed that answers is a live feed - // again, polling at the bell's 10s (ADR-0030 L5) like one that never - // failed. Pinned because "it came back once" and "it is working" are - // different claims, and only the second one is a bell. + // again, polling at the bell's cadence like one that never failed. + // Pinned because "it came back once" and "it is working" are different + // claims, and only the second one is a bell. let reads = 0; inboxBehaviour = () => { reads += 1; @@ -249,11 +249,13 @@ describe('#4289 — a transient missing-resource answer does not cost the sessio await advance(UNAVAILABLE_PROBE_MS); const atRevival = inboxReads(); - // Half the probe cadence at the foreground poll rate is many ticks; the - // exact number is the cadence's business, that there ARE more is this - // case's. - await advance(UNAVAILABLE_PROBE_MS / 2); - expect(inboxReads()).toBeGreaterThan(atRevival + 1); + // A feed still on probes reads at most `UNAVAILABLE_PROBE_LIMIT` more + // times, ever; a live one keeps reading. objectstack#27626 slowed the + // foreground poll to the probe cadence (60 s), so the two are told apart + // by the budget, not by the rate: the exact number is the cadence's + // business, that there are MORE than the budget is this case's. + await advance((UNAVAILABLE_PROBE_LIMIT + 3) * UNAVAILABLE_PROBE_MS); + expect(inboxReads()).toBeGreaterThan(atRevival + UNAVAILABLE_PROBE_LIMIT + 1); }); }); diff --git a/ui/packages/app-shell/src/views/ObjectView.objectDocSwapNoRefetch-27626.test.tsx b/ui/packages/app-shell/src/views/ObjectView.objectDocSwapNoRefetch-27626.test.tsx new file mode 100644 index 00000000000..11cd574dfbe --- /dev/null +++ b/ui/packages/app-shell/src/views/ObjectView.objectDocSwapNoRefetch-27626.test.tsx @@ -0,0 +1,216 @@ +/** + * 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. + */ + +/** + * objectstack#27626 — opening a record re-read the list under it. + * + * Measured in the console (app-crm, a contact list, the record drawer): the + * list query went out again on opening a record and again on closing it. + * Opening a record took the console from the app's object scope to the whole + * registry and closing it took it back, and the two hand this page DIFFERENT + * object documents of equal content. The page rebuilt `schema.columns` (and + * `searchableFields`) from the new document, `ListView` names both BY IDENTITY + * in its fetch effect (objectui#4567 put the stabilisation at the producer), + * and every swap re-ran the effect into an identical `dataSource.find`. + * + * The console no longer swaps the documents under an open drawer + * (`AppContent.recordDrawerKeepsScope-27626.test.tsx`); this page holds the + * two by value all the same, as it holds the view-type blocks (objectui#10046), + * so a host that does swap them costs no read. + * + * Everything renders for real (`ObjectView`, `plugin-view`, `ListView`), with + * the `ObjectView.hostRerenderRefetch-10046` harness and mocks. The CONTROL is + * a real column change, which must still refetch. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, cleanup, act } from '@testing-library/react'; +import { MemoryRouter, Routes, Route, useNavigate } from 'react-router-dom'; + +vi.mock('@object-ui/permissions', async (importOriginal) => { + const actual = await importOriginal(); + // ⚠️ STABLE identities, on purpose. `ListView` names `perms` in the same + // fetch dependency list, so a mock returning a FRESH object per call loops + // the fetch on its own (measured: thousands of queries in one test) and + // measures the mock instead of the host. + 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 }; +}); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u1', name: 'Ada' }, activeOrganization: null }), + useWorkspaceAdminStatus: () => ({ isAdmin: false, isResolved: true }), + createAuthenticatedFetch: () => vi.fn(), +})); + +vi.mock('@object-ui/collaboration', async (importOriginal) => ({ + ...(await importOriginal>()), + useRealtimeSubscription: () => ({ lastMessage: null }), + useConflictResolution: () => ({ hasConflicts: false, resolveAllConflicts: () => {} }), +})); + +vi.mock('sonner', () => ({ + toast: Object.assign(vi.fn(), { + success: vi.fn(), error: vi.fn(), info: vi.fn(), + warning: vi.fn(), loading: vi.fn(), dismiss: vi.fn(), + }), +})); + +vi.mock('./MetadataInspector', () => ({ + MetadataPanel: () => null, + useMetadataInspector: () => ({ showDebug: false, toggle: () => {} }), +})); +vi.mock('./RecordDetailView', () => ({ RecordDetailView: () => null })); + +import { ObjectView } from './ObjectView'; +import { ExpressionProvider } from '../providers/ExpressionProvider'; + +const OBJECT_NAME = 'duly_contact'; + +/** A FRESH document every call: equal content, new identity all the way down. */ +function objectDocuments(columns: string[] = ['name', 'stage']) { + return [ + { + name: OBJECT_NAME, + label: 'Contact', + fields: { + id: { type: 'text', label: 'Id' }, + name: { type: 'text', label: 'Name' }, + stage: { type: 'select', label: 'Stage', options: [{ label: 'A', value: 'a' }] }, + }, + listViews: { + all: { label: 'All', type: 'grid', columns: [...columns], searchableFields: ['name'] }, + }, + }, + ]; +} + +/** The list queries `ListView` issued; the record-count probe (`$top: 0`) is excluded. */ +let listQueries = 0; +function makeDataSource() { + return { + find: vi.fn(async (_object: string, params: any) => { + if (params?.$top !== 0) listQueries++; + return { data: [], total: 0 }; + }), + findOne: vi.fn(async () => null), + create: vi.fn(async () => ({})), + update: vi.fn(async () => ({})), + delete: vi.fn(async () => ({})), + } as any; +} + +const settle = () => act(() => new Promise((resolve) => setTimeout(resolve, 400))); + +interface Host { + /** Navigate and hand the page new documents in the same step, as the console did. */ + goWith: (path: string, objects: any[]) => Promise; + setObjects: (objects: any[]) => Promise; +} + +async function mountHost(): Promise { + const dataSource = makeDataSource(); + let navigate: (to: string) => void = () => {}; + let swap: (objects: any[]) => void = () => {}; + function NavGrab() { + navigate = useNavigate(); + return null; + } + function Harness() { + const [objects, setObjects] = React.useState(() => objectDocuments()); + swap = setObjects; + return ( + + + + + {}} />} + /> + + + + ); + } + render(); + await settle(); + return { + goWith: async (path, objects) => { + await act(async () => { + navigate(path); + swap(objects); + }); + await settle(); + }, + setObjects: async (objects) => { + await act(async () => swap(objects)); + await settle(); + }, + }; +} + +beforeEach(() => { + cleanup(); + listQueries = 0; + vi.stubGlobal( + 'fetch', + vi.fn(async () => + new Response(JSON.stringify({ data: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ), + ); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.clearAllMocks(); +}); + +const VIEW = `/apps/demo/${OBJECT_NAME}/view/all`; + +describe('objectstack#27626 — new object documents of equal content re-read no list', () => { + it('opening and closing a record while the documents swap each time re-issues no list query', async () => { + const host = await mountHost(); + expect(listQueries).toBe(1); + await host.goWith(`${VIEW}?recordId=r1`, objectDocuments()); + await host.goWith(VIEW, objectDocuments()); + await host.goWith(`${VIEW}?recordId=r2`, objectDocuments()); + await host.goWith(VIEW, objectDocuments()); + expect(listQueries).toBe(1); + }); + + it('a content-equal swap with no navigation re-issues no list query', async () => { + const host = await mountHost(); + await host.setObjects(objectDocuments()); + await host.setObjects(objectDocuments()); + expect(listQueries).toBe(1); + }); + + it('CONTROL: a real column change still re-reads the list', async () => { + const host = await mountHost(); + await host.setObjects(objectDocuments(['name'])); + expect(listQueries).toBeGreaterThanOrEqual(2); + }); +}); diff --git a/ui/packages/app-shell/src/views/objectViewListRenderer.tsx b/ui/packages/app-shell/src/views/objectViewListRenderer.tsx index 4d7f0cb648f..b5313c22da7 100644 --- a/ui/packages/app-shell/src/views/objectViewListRenderer.tsx +++ b/ui/packages/app-shell/src/views/objectViewListRenderer.tsx @@ -412,6 +412,10 @@ export function useObjectViewListRenderer({ const fullSchema: ListViewSchema = { ...listSchema, + // objectstack#27626 — `columns` is named by identity in `ListView`'s + // fetch effect too, and opening or closing a record hands this page + // new object documents of equal content: hold it like the blocks. + columns: holdBlock('columns', listSchema.columns), // objectui#10035 — the refresh signal, in place of the remount the // key used to force. The spread above carries only the plugin // ObjectView's counter; this page's own counter (actions, import, @@ -643,7 +647,8 @@ export function useObjectViewListRenderer({ navigation: viewDef.navigation ?? listSchema.navigation, selection: viewDef.selection ?? listSchema.selection, pagination: viewDef.pagination ?? listSchema.pagination, - searchableFields: viewDef.searchableFields ?? listSchema.searchableFields, + // objectstack#27626 — named by identity in the fetch effect too. + searchableFields: holdBlock('searchableFields', viewDef.searchableFields ?? listSchema.searchableFields), resizable: viewDef.resizable ?? listSchema.resizable, rowActions: viewDef.rowActions ?? listSchema.rowActions, /**