From 6af8577f687e9a2418e6293a3cfea8d979c825b2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 15:13:39 +0000 Subject: [PATCH 1/2] fix(related-list): compile the badge's parent scope by relationship arity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The related-list tab badge compiled the parent-relationship condition with a second, independent compiler that always sent bare equality. The ROW query has compiled that condition to match the relationship field's ARITY since objectui#7299, so on a `multiple: true` relationship the two sides asked the driver two different questions: the rows rendered and the badge did not, because the driver refuses equality on an array-valued column and the count store swallows the refusal without a setCount. Patching the badge's copy to match would have left two compilers in place to drift again. Instead the condition has ONE implementation — `@object-ui/core`'s `composeParentScopeFilter`, with the `isMultiValueRelationship` verdict behind it — and both the row query and the badge probe call it. The arity rule itself is still `@objectstack/spec/data`'s `isMultiValueField`, the same predicate the driver executing the query decides on. `RelatedCountStore.fetch` gains an optional trailing `fields` argument, and the `page:tabs` probe effect resolves the child object's schema from the same DataSource the row side reads it from before probing. A caller that cannot see metadata keeps the historical equality wire byte for byte. The store's badge/row parity claim is no longer asserted-and-unenforced: it is held up by a page-level pin that renders a real detail page over a backend which refuses equality the way driver-sql does, and reads the badge digits against the rendered row count at a positive count. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ --- .changeset/related-badge-arity-8882.md | 26 + ...DetailView.relatedBadgeArity-8882.test.tsx | 451 ++++++++++++++++++ .../src/hooks/related-count-store.ts | 47 +- .../src/renderers/layout/containers.tsx | 88 +++- packages/core/src/index.ts | 4 + packages/core/src/utils/parent-scope.ts | 123 +++++ packages/plugin-detail/src/RelatedList.tsx | 79 ++- 7 files changed, 736 insertions(+), 82 deletions(-) create mode 100644 .changeset/related-badge-arity-8882.md create mode 100644 packages/app-shell/src/views/RecordDetailView.relatedBadgeArity-8882.test.tsx create mode 100644 packages/core/src/utils/parent-scope.ts diff --git a/.changeset/related-badge-arity-8882.md b/.changeset/related-badge-arity-8882.md new file mode 100644 index 0000000000..a96ac37ed0 --- /dev/null +++ b/.changeset/related-badge-arity-8882.md @@ -0,0 +1,26 @@ +--- +'@object-ui/core': minor +'@object-ui/components': minor +'@object-ui/plugin-detail': minor +--- + +fix(related-list): the tab badge compiles its parent scope by relationship ARITY, like the rows + +A related list on a `multiple: true` relationship rendered its rows above a tab +with no count at all. The row query has compiled the parent-relationship +condition to match the field's arity since objectui#7299 (`$contains` for a +multi-value relationship, `=` for a single-value one), but the badge's count +probe carried a second compiler that always sent bare equality — which the +driver refuses on an array-valued column, and the count store swallows the +refusal without caching anything. + +Rather than teaching the second compiler the same rule, there is now one: +`@object-ui/core` exports `composeParentScopeFilter` (and the +`isMultiValueRelationship` verdict behind it), and both the row query and the +badge probe call it. The arity verdict remains `@objectstack/spec/data`'s own +`isMultiValueField`, so the renderer and the driver that executes the query +still decide on the same rule. + +`RelatedCountStore.fetch` takes the child object's field defs as a new optional +last argument; callers that cannot see metadata keep the previous equality +wire, byte for byte. Single-value related lists are unchanged on both sides. diff --git a/packages/app-shell/src/views/RecordDetailView.relatedBadgeArity-8882.test.tsx b/packages/app-shell/src/views/RecordDetailView.relatedBadgeArity-8882.test.tsx new file mode 100644 index 0000000000..c5895f5999 --- /dev/null +++ b/packages/app-shell/src/views/RecordDetailView.relatedBadgeArity-8882.test.tsx @@ -0,0 +1,451 @@ +/** + * 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#8882 — the related-list tab BADGE compiles the parent-relationship + * condition to match the relationship field's ARITY, exactly as the ROWS do. + * + * `RelatedList` has compiled that condition by arity since objectui#7299: a + * `multiple: true` relationship stores an ARRAY of parent ids, so the question + * is MEMBERSHIP (`$contains`) and not equality (equality asks whether the whole + * stored array IS one id). The badge probe went on sending bare equality, so on + * a multi-value related list the two sides asked two different questions of the + * same driver — the rows rendered and the badge did not, because the store's + * `catch` swallows the driver's refusal without a `setCount` and the tab then + * has no count to draw at all. + * + * ## Why this file renders the whole page + * + * Same reason as its neighbour `RecordDetailView.relatedListFilter-4664`: the + * subject is what `dataSource.find` is CALLED WITH on BOTH reads, plus what + * ends up on screen. Badge/row parity is a property OF THE PAGE; asserting it + * anywhere narrower is asserting that two implementations agree today. + * + * ## Why the fake backend REFUSES equality instead of answering it + * + * The defect is invisible to a permissive backend. A fake that answers bare + * equality against an array-valued column with "no rows" — or worse, with the + * rows — turns a driver-level refusal into a plausible number, and every + * assertion here would then be satisfied by the broken filter. The real driver + * does not do that: `driver-sql` answers the equality form on a multi-value + * column with `400 INVALID_FILTER`, which is the behaviour `RelatedList` cites + * as its reason for compiling by arity. So the evaluator below THROWS on that + * exact combination, and `FIXTURE` asserts that it does — in both directions. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import { MetadataCtx } from '@object-ui/react'; +import { RelatedCountStore } from '@object-ui/components'; + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u1', name: 'Ada', image: null }, activeOrganization: null }), + createAuthenticatedFetch: () => vi.fn(), +})); + +vi.mock('@object-ui/collaboration', async (importOriginal) => ({ + ...(await importOriginal>()), + useRecordPresence: () => [], + PresenceAvatars: () => null, +})); + +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(), + }), +})); + +// Orthogonal chrome — stubbed so the only asynchrony in this file is the +// related list's own fetch and the tab strip's count probe. +vi.mock('./ActionConfirmDialog', () => ({ ActionConfirmDialog: () => null })); +vi.mock('./ActionParamDialog', () => ({ ActionParamDialog: () => null })); +vi.mock('./ActionResultDialog', () => ({ ActionResultDialog: () => null })); +vi.mock('./FlowRunner', () => ({ FlowRunner: () => null })); +vi.mock('./MetadataInspector', () => ({ + MetadataPanel: () => null, + useMetadataInspector: () => ({ showDebug: false, toggle: () => {} }), +})); + +import { RecordDetailView } from './RecordDetailView'; + +const PARENT = 'task_version'; +const CHILD = 'check_item'; +const RECORD_ID = 'tv-1'; +const OTHER_ID = 'tv-2'; + +/** The multi-value relationship field — an ARRAY of parent ids per child row. */ +const MULTI_REF = 'task_versions'; +/** The single-value control's field — one parent id per child row. */ +const SINGLE_REF = 'task_version'; + +const parentObject = { + name: PARENT, + label: 'Task Version', + managedBy: 'platform', + fields: { + id: { type: 'text', label: 'Id' }, + name: { type: 'text', label: 'Name' }, + }, +}; + +/** + * The child object, in the two arities this card is about. Only the ONE field + * def differs between them — `multiple: true` — so every difference the page + * shows is attributable to the arity and to nothing else. + */ +const multiChild = { + name: CHILD, + label: 'Check Item', + managedBy: 'platform', + fields: { + id: { type: 'text', label: 'Id' }, + name: { type: 'text', label: 'Name' }, + [MULTI_REF]: { + type: 'lookup', + reference: PARENT, + multiple: true, + label: 'Task Versions', + relatedListColumns: ['name'], + }, + }, +}; + +const singleChild = { + name: CHILD, + label: 'Check Item', + managedBy: 'platform', + fields: { + id: { type: 'text', label: 'Id' }, + name: { type: 'text', label: 'Name' }, + [SINGLE_REF]: { + type: 'lookup', + reference: PARENT, + label: 'Task Version', + relatedListColumns: ['name'], + }, + }, +}; + +/** Rows for the multi-value world — the relationship column stores an array. */ +const MULTI_ROWS = [ + { id: 'ci-a', name: 'Multi Item A', [MULTI_REF]: [RECORD_ID] }, + { id: 'ci-b', name: 'Multi Item B', [MULTI_REF]: [OTHER_ID, RECORD_ID] }, + { id: 'ci-c', name: 'Other Parent Only', [MULTI_REF]: [OTHER_ID] }, +]; + +/** Rows for the single-value control — the relationship column stores an id. */ +const SINGLE_ROWS = [ + { id: 'ci-a', name: 'Single Item A', [SINGLE_REF]: RECORD_ID }, + { id: 'ci-b', name: 'Single Item B', [SINGLE_REF]: RECORD_ID }, + { id: 'ci-c', name: 'Other Parent Only', [SINGLE_REF]: OTHER_ID }, +]; + +// --- the filter evaluator --------------------------------------------------- + +/** What `driver-sql` answers the equality form with on a multi-value column. */ +class InvalidFilterError extends Error { + code = 'INVALID_FILTER'; + status = 400; +} + +/** + * Compare one field against one scalar, the way a real driver does. + * + * An array-valued column under a bare scalar equality is REFUSED, not answered: + * that is the whole mechanism of this card. Returning `false` here would make + * the defect look like an empty related list; returning the members would make + * it look like it works. Neither is what the driver does. + */ +function scalarEquals(stored: unknown, value: unknown, field: string): boolean { + if (Array.isArray(stored)) { + throw new InvalidFilterError( + `[fixture] 400 INVALID_FILTER: '${field}' stores multiple values; ` + + `equality cannot be evaluated against an array (use $contains)`, + ); + } + return stored === value; +} + +/** Membership against a multi-value column; a scalar column holds one member. */ +function containsValue(stored: unknown, value: unknown): boolean { + return Array.isArray(stored) ? stored.includes(value) : stored === value; +} + +/** + * Evaluate one filter node against a row. Handles exactly the two shapes this + * repo's single filter sink puts on the wire — the MongoDB-style object and the + * ObjectQL AST array — and THROWS on anything else, for the reason its + * neighbour file gives: a permissive evaluator answers "all rows" for a shape + * it does not understand, and every assertion here would be satisfied by it. + */ +function matchesFilter(row: Record, node: unknown): boolean { + if (node === undefined || node === null) { + throw new Error('[fixture] a child query reached the backend with no $filter at all'); + } + if (Array.isArray(node)) { + const [head, ...rest] = node as any[]; + if (head === 'and') return rest.every((n) => matchesFilter(row, n)); + if (head === 'or') return rest.some((n) => matchesFilter(row, n)); + if (node.length !== 3) { + throw new Error(`[fixture] unsupported AST node: ${JSON.stringify(node)}`); + } + const [field, op, value] = node as [string, string, any]; + switch (op) { + case '=': + return scalarEquals(row[field], value, field); + case '!=': + return !scalarEquals(row[field], value, field); + case 'contains': + return containsValue(row[field], value); + default: + throw new Error(`[fixture] unsupported AST operator '${op}' in ${JSON.stringify(node)}`); + } + } + if (typeof node === 'object') { + return Object.entries(node as Record).every(([field, cond]) => { + if (cond !== null && typeof cond === 'object' && !Array.isArray(cond)) { + return Object.entries(cond as Record).every(([op, value]) => { + if (op === '$eq') return scalarEquals(row[field], value, field); + if (op === '$ne') return !scalarEquals(row[field], value, field); + if (op === '$contains') return containsValue(row[field], value); + throw new Error(`[fixture] unsupported operator '${op}' on '${field}'`); + }); + } + return scalarEquals(row[field], cond, field); + }); + } + throw new Error(`[fixture] unsupported filter: ${JSON.stringify(node)}`); +} + +function makeDataSource(rows: Record[], objects: any[]) { + return { + find: vi.fn(async (objectName: string, params: any) => { + if (objectName !== CHILD) return { data: [], total: 0 }; + // Not a rejected promise built by hand: the evaluator throws from inside + // this async function, which is the same refusal an adapter surfaces. + const matched = rows.filter((r) => matchesFilter(r, params?.$filter)); + const skip = typeof params?.$skip === 'number' ? params.$skip : 0; + const data = + typeof params?.$top === 'number' ? matched.slice(skip, skip + params.$top) : matched; + return { data, total: matched.length }; + }), + getObjectSchema: vi.fn(async (objectName: string) => + objects.find((o) => o.name === objectName), + ), + create: vi.fn(async (_o: string, row: any) => row), + findOne: vi.fn(async (_o: string, recordId: string) => ({ + id: recordId, + name: `Version ${recordId}`, + })), + update: vi.fn(async () => ({})), + delete: vi.fn(async () => ({})), + } as any; +} + +function renderPage(objects: any[], dataSource: any) { + const metadata = { + objects, + pages: [], + loading: false, + error: null, + refresh: async () => {}, + invalidate: () => {}, + ensureType: async () => [], + getItem: async () => null, + getItemsByType: () => [], + } as any; + return render( + + + {}} + objectNameOverride={PARENT} + recordIdOverride={RECORD_ID} + embedded + /> + + , + ); +} + +/** + * The child object is queried TWICE and the two reads are the two halves of + * this card: the ROW query (windowed, `$top` = page size, no `$count`) and the + * tab strip's BADGE probe (`$top: 1`, `$count: true`). + */ +const isRowQuery = (p: any) => !p?.$count && typeof p?.$top === 'number'; +const isBadgeProbe = (p: any) => p?.$count === true; + +async function renderAndCollect(child: any, rows: Record[]) { + const objects = [parentObject, child]; + const ds = makeDataSource(rows, objects); + renderPage(objects, ds); + const childCalls = () => ds.find.mock.calls.filter((c: any[]) => c[0] === CHILD); + // Fail loudly if either read never happened, rather than returning "no + // filter" — which every counter-probe here would read as a pass. + await waitFor(() => { + expect(childCalls().some((c: any[]) => isRowQuery(c[1]))).toBe(true); + expect(childCalls().some((c: any[]) => isBadgeProbe(c[1]))).toBe(true); + }); + const rowQueries = () => childCalls().filter((c: any[]) => isRowQuery(c[1])).map((c) => c[1]); + return { + ds, + rowQueries, + // The SETTLED row query. `RelatedList` deliberately does not gate its fetch + // on "schema has loaded" (objectui#7299): on a multi-value relationship its + // FIRST attempt is the historical equality query, which the driver refuses, + // and it refetches with the membership spelling once the arity is known. + // Parity is a property of where the two reads LAND, so this is the one the + // badge is compared against — and `rowQueries()` keeps the earlier attempt + // visible rather than hiding the difference. + rowQuery: rowQueries()[rowQueries().length - 1] as Record, + badgeProbe: childCalls().find((c: any[]) => isBadgeProbe(c[1]))![1] as Record, + }; +} + +/** The digits rendered inside the Related tab's count badge, or `null`. */ +async function relatedTabBadge(): Promise { + const tab = await screen.findByRole('tab', { name: /Related/i }); + // The badge is the only span in the trigger carrying an accessible name + // (the label span has none) — see `page:tabs` in components/layout. + return tab.querySelector('span[aria-label]')?.textContent?.trim() ?? null; +} + +/** How many child rows the related list actually drew. */ +function renderedRowNames(rows: Record[]): string[] { + return rows.map((r) => r.name as string).filter((n) => screen.queryByText(n) !== null); +} + +beforeEach(() => { + cleanup(); + // The count store is module-scoped and shared by every consumer in the + // process, so a warm entry from a previous case would badge this one. + RelatedCountStore._reset(); + vi.stubGlobal( + 'fetch', + vi.fn(async () => + new Response(JSON.stringify({ data: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ), + ); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe('related-list tab badge — parent scope compiled by ARITY (objectui#8882)', () => { + it('FIXTURE — the backend refuses equality on the array column and answers membership', () => { + // Forward: equality against the stored ARRAY is a refusal, not an answer. + expect(() => matchesFilter(MULTI_ROWS[0], { [MULTI_REF]: RECORD_ID })).toThrow( + /INVALID_FILTER/, + ); + // Reverse: membership answers it, and answers it SELECTIVELY — two of the + // three rows carry this parent, so an evaluator that admitted everything + // (the one bug that would make this whole file lie) reads 3 here. + expect(MULTI_ROWS.filter((r) => matchesFilter(r, { [MULTI_REF]: { $contains: RECORD_ID } })) + .map((r) => r.id)).toEqual(['ci-a', 'ci-b']); + // And the single-value control's column is answered by plain equality, so + // the control below is not passing for want of a working evaluator. + expect(SINGLE_ROWS.filter((r) => matchesFilter(r, { [SINGLE_REF]: RECORD_ID })) + .map((r) => r.id)).toEqual(['ci-a', 'ci-b']); + }); + + it('SUBJECT — the badge probe sends the SAME parent condition as the rows', async () => { + const { rowQuery, badgeProbe, rowQueries } = await renderAndCollect(multiChild, MULTI_ROWS); + // The row side settles on the membership question, because the field + // declares `multiple: true` (objectui#7299). + await waitFor(() => { + expect(rowQueries()[rowQueries().length - 1].$filter).toEqual({ + [MULTI_REF]: { $contains: RECORD_ID }, + }); + }); + // Byte-equal, not merely equivalent: ONE composition of the parent + // relationship, used by both reads. Two compilers that happen to agree are + // what produced this card. + expect(badgeProbe.$filter).toEqual(rowQueries()[rowQueries().length - 1].$filter); + expect(badgeProbe.$filter).toEqual({ [MULTI_REF]: { $contains: RECORD_ID } }); + // Nothing above is read off the first attempt: `rowQuery` is the settled + // one, and it is the same value. + expect(rowQuery.$filter).toEqual(badgeProbe.$filter); + }); + + it('MEASURED DIFFERENCE — the badge waits for the arity, the rows attempt first', async () => { + // Not a defect and not symmetry for its own sake: the two sides make a + // DIFFERENT trade with the same seam, and the difference is worth pinning + // because it is the one thing a reader would otherwise call a bug. + // + // `RelatedList` refuses to gate rows on a loaded schema — an adapter + // without `getObjectSchema` would then render every related list empty — + // so it attempts equality, is refused, and refetches. The BADGE cannot copy + // that: the store caches the first answer it gets, so a lenient backend + // that answered the wrong question with a number would have that number + // cached and never re-probed. It therefore resolves the arity FIRST and + // probes once. + const { rowQueries, badgeProbe, ds } = await renderAndCollect(multiChild, MULTI_ROWS); + await waitFor(() => { + expect(rowQueries().length).toBeGreaterThan(1); + }); + // The rows' first attempt is the historical equality wire… + expect(rowQueries()[0].$filter).toEqual({ [MULTI_REF]: RECORD_ID }); + // …and the badge made exactly one probe, already correct. + const badgeProbes = ds.find.mock.calls + .filter((c: any[]) => c[0] === CHILD && isBadgeProbe(c[1])); + expect(badgeProbes.length).toBe(1); + expect(badgeProbe.$filter).toEqual({ [MULTI_REF]: { $contains: RECORD_ID } }); + }); + + it('SUBJECT — badge/row parity on a multi-value relationship, at a POSITIVE count', async () => { + await renderAndCollect(multiChild, MULTI_ROWS); + // The rows the list drew — read off the screen, not off the fixture. + await waitFor(() => { + expect(renderedRowNames(MULTI_ROWS)).toEqual(['Multi Item A', 'Multi Item B']); + }); + const drawn = renderedRowNames(MULTI_ROWS).length; + // TWO ZEROS ARE EQUAL — so parity is only evidence at a positive count. + // Without this the assertion below is satisfied by a page that shows no + // rows and badges none, which is a different bug wearing this one's face. + expect(drawn).toBeGreaterThan(0); + await waitFor(async () => { + expect(await relatedTabBadge()).toBe(String(drawn)); + }); + expect(await relatedTabBadge()).not.toBe('0'); + expect(await relatedTabBadge()).not.toBeNull(); + }); + + it('LIVE CONTROL — a single-value relationship is correct today and stays correct', async () => { + const { rowQuery, badgeProbe } = await renderAndCollect(singleChild, SINGLE_ROWS); + // Byte for byte the plain MongoDB-style equality object both reads have + // always sent — not a freshly lowered AST that means the same thing. + expect(rowQuery.$filter).toEqual({ [SINGLE_REF]: RECORD_ID }); + expect(badgeProbe.$filter).toEqual({ [SINGLE_REF]: RECORD_ID }); + await waitFor(() => { + expect(renderedRowNames(SINGLE_ROWS)).toEqual(['Single Item A', 'Single Item B']); + }); + const drawn = renderedRowNames(SINGLE_ROWS).length; + expect(drawn).toBeGreaterThan(0); + await waitFor(async () => { + expect(await relatedTabBadge()).toBe(String(drawn)); + }); + // The other parent's row is excluded on both sides, arity or no arity. + expect(screen.queryByText('Other Parent Only')).toBeNull(); + }); +}); diff --git a/packages/components/src/hooks/related-count-store.ts b/packages/components/src/hooks/related-count-store.ts index a18a618184..d7594e9fcd 100644 --- a/packages/components/src/hooks/related-count-store.ts +++ b/packages/components/src/hooks/related-count-store.ts @@ -27,7 +27,7 @@ import { useSyncExternalStore } from 'react'; import { subscribeDataChanges } from '@object-ui/react'; -import { mergeFilterNodes } from '@object-ui/core'; +import { composeParentScopeFilter, mergeFilterNodes, type FieldContainerLike } from '@object-ui/core'; type Listener = () => void; @@ -131,6 +131,7 @@ async function fetchCount( relField: string | undefined, parentId: string | undefined, filter?: CountScopeFilter, + fields?: FieldContainerLike, ): Promise { const k = key(objectName, relField, parentId, filter); const cached = counts.get(k); @@ -144,10 +145,15 @@ async function fetchCount( // `limit` which most adapters silently ignored, so the probe ended // up fetching the entire target table and returning its global // count — completely wrong for parent-scoped badges. - const parentScope: Record = {}; + // The parent-relationship condition, compiled to match the relationship + // field's ARITY by the ONE compiler of it — `@object-ui/core`'s + // `composeParentScopeFilter`, the very call `RelatedList` makes for the + // ROWS. Without `fields` the seam answers equality, which is the historical + // wire and the only answer available to a caller that cannot see metadata. + let parentScope: Record = {}; if (relField) { if (!parentId) return 0; - parentScope[relField] = parentId; + parentScope = composeParentScopeFilter(relField, parentId, fields); } // objectui#4664 — the parent relationship AND the list's own declared // scope, composed exactly as `RelatedList` composes them for the ROWS @@ -156,16 +162,25 @@ async function fetchCount( // implementations agreeing by luck: the badge cannot count a set the list // does not show, because both sides send the same `$filter`. // - // ⚠️ ONE known exception, and it is a gap rather than a design: the parent - // condition `RelatedList` sends is compiled to match the relationship - // field's ARITY since objectui#7299 (`{ [relField]: { $contains: parentId } }` - // when the child object declares it `multiple: true`), and this probe still - // sends bare equality. It has no field metadata in hand to decide with — - // four scalars and a filter is the whole input — so closing it is a design - // change in this package, tracked as objectui#8882. Until then a related - // list on a multi-value relationship renders its ROWS and gets no badge: - // the `catch` below swallows the driver's refusal without a `setCount`, so - // the store holds no entry and the tab renders no count at all. + // ⭐ BOTH halves of that `$filter` are now shared, which is what lets the + // claim above be stated without an exception. The declared scope has gone + // through `mergeFilterNodes` since objectui#4664; the PARENT condition went + // through a second compiler until objectui#8882 — `RelatedList` matched the + // relationship field's ARITY (objectui#7299) while this probe sent bare + // equality, so a multi-value related list rendered its ROWS and got no + // badge: the `catch` below swallows the driver's refusal without a + // `setCount`, leaving the store with no entry and the tab with no count. + // Both sides now call `composeParentScopeFilter`. + // + // ⛔ The claim is not self-enforcing, and an unenforced claim is how the + // exception above survived being false. What holds it up is a PIN that goes + // RED, on the page, with both reads on one wire: + // `app-shell/src/views/RecordDetailView.relatedBadgeArity-8882.test.tsx` + // renders a real detail page over a backend that REFUSES equality on a + // multi-value column the way `driver-sql` does, and asserts the badge digits + // equal the rendered row count at a POSITIVE count. Its sibling + // `RecordDetailView.relatedListFilter-4664.test.tsx` holds the declared-scope + // half. Delete either and this comment is a claim again. // // The parent condition is never negotiable — a declared filter may only // NARROW this parent's children — and `mergeFilterNodes` guarantees that @@ -294,6 +309,12 @@ export function useRelatedCountVersion(): number { /** * Imperative store API for non-React callers (mutation handlers, tests). * Prefer `useRelatedCount` in components. + * + * `fetch`'s last parameter is the CHILD object's field defs. It is optional + * because a caller that cannot see metadata must still be able to probe — the + * seam then compiles the historical equality wire — but a caller that CAN see + * them owes them: that is the difference between a badge and no badge on a + * multi-value relationship (objectui#8882). */ export const RelatedCountStore = { get: getCount, diff --git a/packages/components/src/renderers/layout/containers.tsx b/packages/components/src/renderers/layout/containers.tsx index 0ba09c7583..02fdc8a408 100644 --- a/packages/components/src/renderers/layout/containers.tsx +++ b/packages/components/src/renderers/layout/containers.tsx @@ -637,31 +637,71 @@ const PageTabsRenderer: React.FC = ({ schema, className, ...props }) => { if (!ds || typeof ds.find !== 'function') return; if (probeTargets.size === 0) return; let cancelled = false; - for (const probes of probeTargets.values()) { - for (const probe of probes) { - // RelatedCountStore.fetch is internally deduplicated, so concurrent - // mounts of multiple tab strips don't generate redundant requests. - // The attachments probe overrides the store-built single-key filter - // with the two-key `(parent_object, parent_id)` scope; the synthetic - // relationshipField keeps the cache key unique, and the store's - // `sys_attachment` invalidation (data-change bus) still hits it. - const finder = probe.attachments - ? (object: string, query: any) => - ds.find(object, { - ...query, - $filter: { parent_object: recordObject, parent_id: parentId }, - }) - : (object: string, query: any) => ds.find(object, query); - void RelatedCountStore.fetch( - finder, - probe.objectName, - probe.relationshipField, - parentId, - probe.filter, - ).catch(() => 0); - if (cancelled) return; + void (async () => { + // objectui#8882 — the badge asks the SAME question of the parent + // relationship that the rows do, and that question's spelling depends on + // the relationship field's ARITY. The store compiles it through + // `composeParentScopeFilter`, the one compiler `RelatedList` uses for the + // ROWS, but that seam can only answer from METADATA — so this call site + // owes it the child object's field defs. It is the same `DataSource` the + // row side reads them from, one layer up. + // + // Resolved BEFORE any probe rather than gating on a loaded schema: an + // adapter without `getObjectSchema`, or one whose fetch rejects, still + // probes — the seam then compiles the historical equality wire, which is + // byte for byte what this effect sent before this card. What is NOT done + // is probing first and correcting later: the store caches the first + // answer it gets, and a lenient backend that answers the wrong question + // with a number would have that number cached and never re-probed. + const fieldsFor = new Map(); + if (typeof ds.getObjectSchema === 'function') { + const names = new Set(); + for (const probes of probeTargets.values()) { + for (const probe of probes) { + // The attachments probe's `relationshipField` is a synthetic cache + // discriminator, not a field on `sys_attachment`, and its wrapper + // below replaces `$filter` outright — there is no arity to read. + if (!probe.attachments) names.add(probe.objectName); + } + } + await Promise.all( + Array.from(names).map(async (name) => { + try { + fieldsFor.set(name, (await ds.getObjectSchema(name))?.fields); + } catch { + // Equality it is — the wire this effect has always sent. + } + }), + ); } - } + if (cancelled) return; + for (const probes of probeTargets.values()) { + for (const probe of probes) { + // RelatedCountStore.fetch is internally deduplicated, so concurrent + // mounts of multiple tab strips don't generate redundant requests. + // The attachments probe overrides the store-built single-key filter + // with the two-key `(parent_object, parent_id)` scope; the synthetic + // relationshipField keeps the cache key unique, and the store's + // `sys_attachment` invalidation (data-change bus) still hits it. + const finder = probe.attachments + ? (object: string, query: any) => + ds.find(object, { + ...query, + $filter: { parent_object: recordObject, parent_id: parentId }, + }) + : (object: string, query: any) => ds.find(object, query); + void RelatedCountStore.fetch( + finder, + probe.objectName, + probe.relationshipField, + parentId, + probe.filter, + probe.attachments ? undefined : (fieldsFor.get(probe.objectName) as any), + ).catch(() => 0); + if (cancelled) return; + } + } + })(); return () => { cancelled = true; }; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 4962c5c6d6..012e314fe4 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -125,6 +125,10 @@ export * from './utils/reference-keys.js'; // `toPredicateRecord` for why an unnormalized one gives the same predicate // different verdicts on different surfaces. export * from './utils/predicate-record.js'; +// The parent-relationship condition a detail-page related list is scoped by. +// One implementation, imported by BOTH the row query and the tab-badge count +// probe — objectui#8882 is what two of them cost. +export * from './utils/parent-scope.js'; // The other half of a view's field appetite: the fields its PREDICATES read, // which the column-derived `$select` never asked the server for. export * from './utils/predicate-fields.js'; diff --git a/packages/core/src/utils/parent-scope.ts b/packages/core/src/utils/parent-scope.ts new file mode 100644 index 0000000000..5ecc618c42 --- /dev/null +++ b/packages/core/src/utils/parent-scope.ts @@ -0,0 +1,123 @@ +/** + * 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. + * + * The parent-relationship condition a detail-page related list is scoped by — + * compiled ONCE, here, for every surface that asks the question. + * + * ## Why this is a seam and not a helper + * + * A related list asks its backend "which children belong to this parent?" from + * two places: the ROW query (`RelatedList`) and the tab-badge count probe + * (`RelatedCountStore`). Those two used to compile the condition separately, + * and objectui#8882 is what that costs: objectui#7299 taught the ROW side to + * compile by the relationship field's ARITY, the BADGE side kept sending bare + * equality, and on a multi-value relationship the page then rendered rows above + * a tab with no count at all — two implementations of one question, drifted. + * + * Patching the second copy to match the first would leave two copies. So the + * condition has one implementation, and both callers import it. + * + * ## The rule itself is not invented here + * + * The arity verdict is `@objectstack/spec/data`'s own `isMultiValueField`, the + * same predicate the driver that executes the query decides on. That matters + * more here than anywhere: this function chooses `$contains` vs `=`, the driver + * chooses whether to accept it, and two readers of one question disagreeing is + * the entire defect class. The spec's rule is BROADER than an eyeballed + * `multiple === true` in both directions — `multiselect` / `checkboxes` / + * `tags` persist an array with no flag at all, and `multiple: true` is INERT on + * a type outside the spec's multi-capable set (`master_detail`, say) — so a + * local approximation is wrong in both directions, not merely incomplete. + * + * ⛔ Do not add a local arity rule at any call site, however small, and ⛔ do + * not widen this function to accept an arity the caller computed: the parameter + * it takes is METADATA, and the verdict is drawn from it here. + */ + +import { isMultiValueField, type ValueShapeFieldDef } from '@objectstack/spec/data'; +import type { FieldContainerLike } from './predicate-record.js'; + +/** + * Look one field def up in either container shape the metadata API serves. + * + * The pair is the one {@link FieldContainerLike} names: the Record keyed by + * field name, and the array of defs carrying their own `name`. A reader that + * knows only one of them silently answers "no such field" for the other — and + * "no such field" here means "single-valued", which is this card's own bug + * spelled as a default. + */ +export function parentRelationshipFieldDef( + fields: FieldContainerLike, + fieldName: string | undefined, +): ValueShapeFieldDef | undefined { + if (!fieldName || !fields || typeof fields !== 'object') return undefined; + const def = Array.isArray(fields) + ? fields.find((f) => (f as { name?: unknown } | null)?.name === fieldName) + : (fields as Record)[fieldName]; + if (!def || typeof def !== 'object') return undefined; + // `type` is the one member the spec's predicate reads besides `multiple`; a + // def without it answers `false` through both of the predicate's set lookups, + // which is the right answer for a field whose type nobody declared. + return def as ValueShapeFieldDef; +} + +/** + * Does this relationship field store MANY parent ids rather than one? + * + * The seam's verdict, exposed on its own because a caller sometimes needs the + * ANSWER without the condition — the raw-URL related-list path, whose + * `filter[]=` grammar has no membership operator, has to know + * the arity in order to REFUSE. Deriving it a second time at that call site is + * how the two compilers this module exists to merge came about, so it is + * derived once here and read from both shapes below. + */ +export function isMultiValueRelationship( + fields: FieldContainerLike, + fieldName: string | undefined, +): boolean { + const def = parentRelationshipFieldDef(fields, fieldName); + return def !== undefined && isMultiValueField(def); +} + +/** + * The parent-relationship condition, compiled to match the field's ARITY. + * + * - single-valued → `{ [relationshipField]: parentId }` — equality, byte for + * byte what both surfaces have always sent; + * - `multiple: true` (per the spec predicate) → + * `{ [relationshipField]: { $contains: parentId } }` — MEMBERSHIP, because + * the stored value is an ARRAY of ids and equality asks whether that whole + * array IS one id. `$contains` is the spelling the drivers execute for it, + * the one `driver-sql` names in the `400 INVALID_FILTER` it answers the + * equality form with. + * + * The author never writes either: they named a relationship, and its storage + * form is the renderer's business. + * + * The return value is the plain MongoDB-style object both call sites have + * always put on the wire, NOT a lowered ObjectQL AST. That is deliberate: with + * nothing else declared the query must stay byte-identical to what it was, and + * a freshly lowered AST would mean the same thing, be invisible on screen, and + * break every caller pinning the wire. Composition with a list's own declared + * scope stays the caller's step, through `mergeFilterNodes`. + * + * @param relationshipField The child field pointing back at the parent. + * @param parentId The parent record's primary key value. + * @param fields The CHILD object's field defs (`objectSchema.fields`), + * in either served shape. Without them the condition + * compiles to equality — the historical wire, and the + * only answer available to a caller that cannot see + * the metadata. A caller that CAN see it must pass it. + */ +export function composeParentScopeFilter( + relationshipField: string, + parentId: string | number, + fields?: FieldContainerLike, +): Record { + const multi = isMultiValueRelationship(fields, relationshipField); + return { [relationshipField]: multi ? { $contains: parentId } : parentId }; +} diff --git a/packages/plugin-detail/src/RelatedList.tsx b/packages/plugin-detail/src/RelatedList.tsx index 9c985aa192..497c46e56d 100644 --- a/packages/plugin-detail/src/RelatedList.tsx +++ b/packages/plugin-detail/src/RelatedList.tsx @@ -41,7 +41,6 @@ import { import type { LucideIcon } from 'lucide-react'; import type { DataSource, FieldMetadata } from '@object-ui/types'; import type { ViewFilterRule } from '@objectstack/spec/ui'; -import { isMultiValueField, type ValueShapeFieldDef } from '@objectstack/spec/data'; import { getCellRenderer, resolveCellRendererType, RecordPickerDialog, deriveLookupColumns } from '@object-ui/fields'; import { columnIdentity, @@ -53,6 +52,8 @@ import { isExpandableFieldType, isPlatformSortableField, isUnmaterializedFieldType, + composeParentScopeFilter, + isMultiValueRelationship, mergeFilterNodes, readObjectSortability, toFilterNode, @@ -232,8 +233,10 @@ export interface RelatedListProps { * asks whether the whole array IS one id. * * The verdict is `@objectstack/spec/data`'s own `isMultiValueField`, not a - * local rule — see {@link parentRelationshipFieldDef} for why that matters - * here of all places. + * local rule, and it is reached through `@object-ui/core`'s + * {@link composeParentScopeFilter} — the ONE compiler of this condition, + * shared with the tab-badge count probe that used to carry a second one + * (objectui#8882). */ parentId?: string | number; /** Lucide icon name (kebab-case) to render next to the section title. */ @@ -351,44 +354,25 @@ export const RelatedToolbarButton: React.FC<{ ); }; -/** - * Pull one field's definition out of an object schema, in either served shape. +/* + * The two-shape field lookup and the ARITY VERDICT that used to live here are + * now `@object-ui/core`'s `parent-scope` seam (`composeParentScopeFilter` / + * `isMultiValueRelationship`), imported above. * - * The ARITY VERDICT itself is NOT computed here — it is - * `@objectstack/spec/data`'s `isMultiValueField`, imported above. This function - * exists only to find the def to hand it, which is the part the spec cannot do: - * the spec takes a `ValueShapeFieldDef`, and the metadata API serves a - * CONTAINER of them in two shapes — the Record keyed by field name, and the - * array of defs carrying their own `name` (the pair `FieldContainerLike` in - * `@object-ui/core` names). A reader that knows only one of them silently - * answers "no such field" for the other, which is this card's own bug spelled - * as a default. + * They moved because this component was never the only reader of the question. + * The related-list tab BADGE compiles the same parent-relationship condition, + * it kept sending bare equality after objectui#7299 taught this file to compile + * by arity, and objectui#8882 is the result: a multi-value related list that + * renders its rows above a tab with no count at all. * - * ⛔ Do not reintroduce a local arity rule here, however small. This component - * decides `$contains` vs `=` on the answer, and the driver that refuses the - * query decides on the spec's — two readers of one question, disagreeing, is - * exactly the defect objectui#7299 is about, and putting it one layer up would - * be a worse version of it. The spec's rule is BROADER than an eyeballed - * `multiple === true` in both directions: `multiselect` / `checkboxes` / `tags` - * persist an array with no flag at all, and `multiple: true` is INERT on a type - * outside `MULTI_CAPABLE_TYPES` (`master_detail`, say). Both are pinned. + * ⛔ Do not reintroduce a local arity rule here, however small — the warning + * that stood at this spot still stands, and now names one more reader. This + * component decides `$contains` vs `=`, the driver that refuses the query + * decides on `@objectstack/spec/data`'s `isMultiValueField`, and the badge + * decides too; readers of one question disagreeing is the whole defect class. + * Moving the decision to a shared seam is NOT "putting a local rule one layer + * up" — the rule is still the spec's, and there is now exactly one caller of it. */ -function parentRelationshipFieldDef( - objectSchema: unknown, - fieldName: string | undefined, -): ValueShapeFieldDef | undefined { - if (!fieldName || !objectSchema || typeof objectSchema !== 'object') return undefined; - const fields = (objectSchema as { fields?: unknown }).fields; - if (!fields || typeof fields !== 'object') return undefined; - const def = Array.isArray(fields) - ? fields.find((f) => (f as { name?: unknown } | null)?.name === fieldName) - : (fields as Record)[fieldName]; - if (!def || typeof def !== 'object') return undefined; - // `type` is the one member the spec's predicate reads besides `multiple`; a - // def without it answers `false` through both of the predicate's set lookups, - // which is the right answer for a field whose type nobody declared. - return def as ValueShapeFieldDef; -} export const RelatedList: React.FC = ({ title, @@ -535,10 +519,13 @@ export const RelatedList: React.FC = ({ // `getObjectSchema`, or one whose schema fetch rejects, would then never fetch // rows at all — trading this card's loud 400 on one relationship shape for a // silent empty list on EVERY related list in the app. - const referenceFieldIsMultiValue = React.useMemo(() => { - const def = parentRelationshipFieldDef(objectSchema, referenceField); - return def !== undefined && isMultiValueField(def); - }, [objectSchema, referenceField]); + // The seam's verdict, not a second reading of the metadata: the query below + // and this flag must never be able to disagree about the arity, which is the + // defect objectui#8882 records when two call sites each decide for themselves. + const referenceFieldIsMultiValue = React.useMemo( + () => isMultiValueRelationship(objectSchema?.fields, referenceField), + [objectSchema, referenceField], + ); // Add-picker target schema, fetched lazily on first open. It drives the // picker's display column (`add.picker.labelField` → displayField), the @@ -619,9 +606,11 @@ export const RelatedList: React.FC = ({ // `400 INVALID_FILTER` it answers the equality form with. Single-valued // keeps `=`, unchanged. The author never writes either: they named a // relationship, and its storage form is this component's business. - const parentScope = { - [referenceField!]: referenceFieldIsMultiValue ? { $contains: parentId } : parentId, - } as Record; + const parentScope = composeParentScopeFilter( + referenceField!, + parentId!, + objectSchema?.fields, + ) as Record; // Parent relationship AND the list's own scope (objectstack#7118). The // parent condition is never negotiable — an "additional" criterion may only // narrow this parent's children — and with nothing authored the query is From 0d921222fd137391da3bc376a59cbd547d1b7445 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 15:56:11 +0000 Subject: [PATCH 2/2] test(app-shell): type the row-query accessor in the 8882 pin `tsc` refused the implicit `any` on the map callback (TS7006), which the package's `type-check` reaches because its test config compiles the test files. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ --- .../src/views/RecordDetailView.relatedBadgeArity-8882.test.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/app-shell/src/views/RecordDetailView.relatedBadgeArity-8882.test.tsx b/packages/app-shell/src/views/RecordDetailView.relatedBadgeArity-8882.test.tsx index c5895f5999..27c5a50655 100644 --- a/packages/app-shell/src/views/RecordDetailView.relatedBadgeArity-8882.test.tsx +++ b/packages/app-shell/src/views/RecordDetailView.relatedBadgeArity-8882.test.tsx @@ -302,7 +302,8 @@ async function renderAndCollect(child: any, rows: Record[]) { expect(childCalls().some((c: any[]) => isRowQuery(c[1]))).toBe(true); expect(childCalls().some((c: any[]) => isBadgeProbe(c[1]))).toBe(true); }); - const rowQueries = () => childCalls().filter((c: any[]) => isRowQuery(c[1])).map((c) => c[1]); + const rowQueries = (): Record[] => + childCalls().filter((c: any[]) => isRowQuery(c[1])).map((c: any[]) => c[1]); return { ds, rowQueries,