diff --git a/.changeset/9925-row-cap-non-positive.md b/.changeset/9925-row-cap-non-positive.md new file mode 100644 index 0000000000..924a0f60e2 --- /dev/null +++ b/.changeset/9925-row-cap-non-positive.md @@ -0,0 +1,39 @@ +--- +'@object-ui/plugin-kanban': patch +'@object-ui/plugin-timeline': patch +'@object-ui/plugin-detail': patch +--- + +Refuse a row cap the contract already refuses before it reaches `$top`, at the +three read points that still forwarded one — `object-kanban`, `object-timeline` +and each `record:reference_rail` entry (objectui#9925). + +Each of these blocks spelled its row cap as a bare `?? DEFAULT`. `??` rejects +only `null` and `undefined`, so a value the contract refuses was not nullish and +survived as a real fetch window: it reached the adapter as `$top: 0`, the block +asked the server for nothing, and the empty board / empty rail / empty card +named no cause. A negative went out the same way, and a non-integer became a +fractional window. + +All three now go through one resolver per site, mirroring the shape objectui#9853 +landed on `ObjectGrid` and objectui#9897 repeated on `ListView` — one resolver at +every entry is what keeps the answer single. A refused value is dropped, the +site's own default is used, and one `console.warn` names the block, the object +and the value. The warning is conditional and deduped: an absent `limit` and a +usable one both stay silent, and one declaration warns once rather than once per +render. + +This closes BOTH entrances into these blocks, which is why the repair is at the +read point. Each block reads one key, and two authoring shapes fill it: a +`dataSource` binding lowers a named view's `pagination.pageSize` into it, and a +block with no binding at all carries the authored `limit` straight through. A +repair at the lowering layer would close only the first. + +Refusing these is not a renderer choosing a meaning. `@objectstack/spec` declares +every one of these members a positive integer — `object-kanban`'s `limit`, the +element data source `limit` a binding lowers into `object-timeline`'s, and the +rail entry's own `limit` on `ReferenceRailEntrySchema`. + +⛔ No fallback literal changed. Each site keeps the default it already +documented; the rail's was spelled inline twice and is now one named constant so +its two read points cannot drift. diff --git a/packages/plugin-detail/src/renderers/__tests__/record-reference-rail.rowLimitNonPositive-9925.test.tsx b/packages/plugin-detail/src/renderers/__tests__/record-reference-rail.rowLimitNonPositive-9925.test.tsx new file mode 100644 index 0000000000..aac0c7ca00 --- /dev/null +++ b/packages/plugin-detail/src/renderers/__tests__/record-reference-rail.rowLimitNonPositive-9925.test.tsx @@ -0,0 +1,280 @@ +/** + * 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#9925 — `record:reference_rail` spent each entry's preview-row cap + * with a bare `??`, and `??` rejects only `null`/`undefined`, so a value the + * contract refuses survived as a real fetch window and reached the adapter as + * `$top`. + * + * ## Why this site is in the card at all + * + * The card names three blocks and derives its population by CONCEPT, not by one + * identifier. Inside this package the concept turns up at five read points, and + * four of them already refuse a non-positive value before it reaches `$top` + * (`record:related_list` tests the number is positive; `record:history` and + * `record:activity` floor theirs). This entry was the one that did not, so it + * is the one this file pins. + * + * ## Why "refuse it" is not this file inventing a meaning + * + * `@objectstack/spec` already answers what `limit: 0` means: the rail entry's + * own member is declared a POSITIVE INTEGER on `ReferenceRailEntrySchema` + * (`z.number().int().positive().optional()`, described there as the `$top` of + * the one query the entry issues). So `0` is not a spelling whose meaning a + * consumer may choose; it is a value the contract refuses. + * + * ## Why the silence matters more here than at the two sibling sites + * + * This rail degrades silently by design — a failed or empty entry renders "—" + * rather than blanking the rail — so an entry asked for nothing draws a card + * that looks merely empty. There is no on-screen channel to say it on, which is + * why the diagnostic assertions below are not decoration. + * + * ## What the assertions are, and what each control buys + * + * The subject is the RELATION, never a literal: a refused value does not reach + * `$top` and the site's own default does. Each refusal is paired with a control + * that must NOT fire, so a rail that ignores the member entirely cannot pass + * for a measurement and an always-on diagnostic cannot pass for a diagnosis. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; +import { MemoryRouter } from 'react-router-dom'; +import { RecordContextProvider } from '@object-ui/react'; + +import { + RecordReferenceRailRenderer, + DEFAULT_REFERENCE_RAIL_LIMIT, +} from '../record-reference-rail'; + +/** + * The rail gates its queries on an IntersectionObserver. Report intersecting + * immediately so the fetch effect runs deterministically under jsdom. + */ +class ImmediateIO { + constructor(private cb: (records: { isIntersecting: boolean }[]) => void) {} + observe() { this.cb([{ isIntersecting: true }]); } + disconnect() {} + unobserve() {} +} + +/** The three values `??` and the resolver DISAGREE about. */ +const REFUSED = [0, -5, 2.5]; + +const makeDataSource = () => ({ + find: vi.fn(async () => ({ data: [{ id: 'c1', name: 'Ada' }], total: 1 })), +}); + +const railWith = (limit: unknown) => ({ + hideEmpty: false, + entries: [ + { + objectName: 'contact', + relationshipField: 'account_id', + title: 'Contacts', + ...(limit === undefined ? {} : { limit }), + }, + ], +}); + +const renderRail = (schema: Record, dataSource: any) => + render( + + + + + , + ); + +const tops = async (ds: ReturnType) => { + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + return ds.find.mock.calls.map((c: any[]) => (c[1] as any)?.$top); +}; + +let warnings: string[] = []; +beforeEach(() => { + warnings = []; + vi.stubGlobal('IntersectionObserver', ImmediateIO as unknown as typeof IntersectionObserver); + vi.spyOn(console, 'warn').mockImplementation((...args: unknown[]) => { + warnings.push(args.map(String).join(' ')); + }); +}); +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +const rowCapWarnings = () => warnings.filter((w) => w.includes('RecordReferenceRail row cap')); + +describe('record:reference_rail — a row cap the contract refuses never reaches the wire (objectui#9925)', () => { + it.each(REFUSED)('never sends `$top: %s`', async (bad) => { + const ds = makeDataSource(); + renderRail(railWith(bad), ds); + + const sent = await tops(ds); + expect(sent.length).toBeGreaterThan(0); + expect(sent, `the authored ${bad} reached the wire`).not.toContain(bad); + for (const top of sent) { + expect(typeof top).toBe('number'); + expect(Number.isInteger(top)).toBe(true); + expect(top).toBeGreaterThan(0); + } + }); + + it.each(REFUSED)('falls back to this entry’s OWN default instead of %s', async (bad) => { + const ds = makeDataSource(); + renderRail(railWith(bad), ds); + + const sent = await tops(ds); + // The relation, not the literal: whatever this site documents as its + // default is what a refused declaration falls back to. + for (const top of sent) expect(top).toBe(DEFAULT_REFERENCE_RAIL_LIMIT); + }); + + it('CONTROL — a legitimate authored limit still reaches `$top` unchanged', async () => { + const ds = makeDataSource(); + renderRail(railWith(7), ds); + + const sent = await tops(ds); + // Without this row, "never 0" is satisfied by a rail that ignores the + // member entirely and always sends its own default. + expect(sent).toContain(7); + expect(sent).not.toContain(DEFAULT_REFERENCE_RAIL_LIMIT); + }); + + it('CONTROL — declaring no limit at all still sends the default', async () => { + const ds = makeDataSource(); + renderRail(railWith(undefined), ds); + + const sent = await tops(ds); + expect(sent).toContain(DEFAULT_REFERENCE_RAIL_LIMIT); + }); + + it('CONTROL — the entry’s parent scope still arrives when its limit is refused', async () => { + // ⛔ No capability removed: refusing one member must not cost the query the + // entry exists to issue. + const ds = makeDataSource(); + renderRail(railWith(0), ds); + + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + const [object, params] = ds.find.mock.calls[0] as unknown as [string, any]; + expect(object).toBe('contact'); + expect(params.$filter).toEqual({ account_id: 'A1' }); + expect(params.$count).toBe(true); + }); + + it('refuses ONE entry without disarming its neighbour', async () => { + // A rail is a list, and the resolver runs per entry: a refused entry must + // not take the legitimate one's window with it, in either direction. + const ds = makeDataSource(); + renderRail( + { + hideEmpty: false, + entries: [ + { objectName: 'contact', relationshipField: 'account_id', limit: 0 }, + { objectName: 'task', relationshipField: 'account_id', limit: 7 }, + ], + }, + ds, + ); + + await waitFor(() => expect(ds.find).toHaveBeenCalledTimes(2)); + const byObject = new Map( + ds.find.mock.calls.map((c: any[]) => [c[0] as string, (c[1] as any)?.$top]), + ); + expect(byObject.get('contact')).toBe(DEFAULT_REFERENCE_RAIL_LIMIT); + expect(byObject.get('task')).toBe(7); + }); + + // ── THE DIAGNOSTIC ─────────────────────────────────────────────────────── + it.each(REFUSED)('says so, naming the block, the entry’s object and the value %s', async (bad) => { + const ds = makeDataSource(); + renderRail(railWith(bad), ds); + + await waitFor(() => expect(rowCapWarnings().length).toBeGreaterThan(0)); + const message = rowCapWarnings()[0]; + // This rail is silent on screen by construction, so substituting a number + // the author never wrote would otherwise be entirely unobservable. + expect(message).toContain('record:reference_rail'); + expect(message).toContain('contact'); + expect(message).toContain('limit'); + expect(message).toContain(String(bad)); + }); + + it('fires exactly ONCE for one declaration, across re-renders', async () => { + const ds = makeDataSource(); + const { rerender } = renderRail(railWith(0), ds); + + await waitFor(() => expect(rowCapWarnings().length).toBeGreaterThan(0)); + // A fresh schema OBJECT carrying the same declaration. The effect is keyed + // on the declaration's CONTENT and deduped per (object, value), so this + // must say nothing more. + rerender( + + + + + , + ); + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + expect(rowCapWarnings()).toHaveLength(1); + }); + + it('stays silent about an entry it has already named when a NEIGHBOUR is added', async () => { + // The dedupe is not the effect's dependency key: adding a second entry + // genuinely changes the declaration, so the effect re-runs and walks the + // list again. Without a per-(object, value) memory the untouched first + // entry would be named a second time — one authoring change, two warnings + // about a value that did not move. + const ds = makeDataSource(); + const { rerender } = renderRail(railWith(0), ds); + await waitFor(() => expect(rowCapWarnings().length).toBe(1)); + + rerender( + + + + + , + ); + await waitFor(() => expect(ds.find).toHaveBeenCalledTimes(3)); + expect( + rowCapWarnings(), + 'the untouched first entry was named again when its neighbour arrived', + ).toHaveLength(1); + }); + + it('CONTROL — a legitimate limit produces no such diagnostic', async () => { + const ds = makeDataSource(); + renderRail(railWith(7), ds); + + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + // An always-on marker states nothing. + expect(rowCapWarnings()).toHaveLength(0); + }); + + it('CONTROL — declaring no limit at all produces no diagnostic', async () => { + const ds = makeDataSource(); + renderRail(railWith(undefined), ds); + + await waitFor(() => expect(ds.find).toHaveBeenCalled()); + expect(rowCapWarnings()).toHaveLength(0); + }); +}); diff --git a/packages/plugin-detail/src/renderers/record-reference-rail.tsx b/packages/plugin-detail/src/renderers/record-reference-rail.tsx index 6d3e1bab08..e843786886 100644 --- a/packages/plugin-detail/src/renderers/record-reference-rail.tsx +++ b/packages/plugin-detail/src/renderers/record-reference-rail.tsx @@ -144,6 +144,79 @@ interface EntryState { error?: string; } +/** + * Preview rows fetched for one entry when the author declared no `limit`. + * + * Named rather than spelled inline because it was spelled TWICE — once in the + * `$top` the entry's query carries and once in the fetch signature that decides + * whether to re-issue it — and objectui#9925 gave both a resolver, which needs + * one fallback to agree on. + */ +export const DEFAULT_REFERENCE_RAIL_LIMIT = 3; + +/** + * What the contract admits as a preview-row cap for a rail entry. + * + * `@objectstack/spec` has already answered what `limit: 0` means: this entry's + * own member is declared a POSITIVE INTEGER on `ReferenceRailEntrySchema` + * (`z.number().int().positive().optional()`, described there as "Preview rows + * per card, and the `$top` of the one query this entry issues"). So `0` is not + * a spelling whose meaning this renderer may choose; it is a value the contract + * refuses. + */ +function isUsableRowLimit(value: unknown): value is number { + return typeof value === 'number' && Number.isInteger(value) && value > 0; +} + +/** + * The ONE resolver for a rail entry's row cap, for the reason objectui#9853 + * gave when it landed the same shape on `ObjectGrid` and objectui#9897 repeated + * on `ListView`: one resolver at every entry is what keeps the answer single. + * + * Before objectui#9925 both read points were a bare `entry.limit ?? 3`, and + * `??` rejects only `null` and `undefined` — so an authored `limit: 0` was not + * nullish and survived as a real window, reaching the wire as `$top: 0`. This + * rail degrades silently by design (a failed entry renders "—"), so an entry + * asked for nothing rendered an empty card with a zero badge and named no + * cause. A negative goes out the same way. + * + * ⚠️ The refusal is FAIL-SOFT on purpose, as it is at the two sibling sites + * this card repairs: throwing would take out the whole rail over one entry's + * declaration, which is a worse outcome than the defect. The value is dropped, + * this rail's own default is used, and `describeRefusedRowLimit` states it + * through the developer channel — the only channel available, because the + * suppression this rail already does is silent on screen by construction. + * ⛔ Not a silent clamp, and ⛔ not a clamp to 1. + */ +function resolveRowLimit(authored: unknown, fallback: number): number { + return isUsableRowLimit(authored) ? authored : fallback; +} + +/** + * The diagnostic half. `null` means "nothing to say" — an absent `limit` is not + * a mistake, and a usable one is not either, so the message is CONDITIONAL and + * the silence controls in the pin are what keep it from being an always-on + * marker that states nothing. + * + * ⛔ NOT a second guard: the predicate lives once, in `isUsableRowLimit`, and + * this reads it. Two predicates would be free to drift, and the drift would be + * invisible — a value refused by one and admitted by the other. + */ +function describeRefusedRowLimit(authored: unknown, objectName: unknown): string | null { + if (authored === undefined || authored === null) return null; + if (isUsableRowLimit(authored)) return null; + const where = + typeof objectName === 'string' && objectName + ? `record:reference_rail entry for ${objectName}` + : 'record:reference_rail entry'; + return ( + `[ObjectUI] RecordReferenceRail row cap: ${where} declared limit: ${String(authored)}, ` + + 'which is not a positive integer. A row cap must be a positive integer ' + + '(the spec refuses zero and negative values), so it was ignored and this ' + + `entry fell back to its default preview-row cap (${DEFAULT_REFERENCE_RAIL_LIMIT}).` + ); +} + const humanize = (s: string) => s .replace(/[_-]+/g, ' ') @@ -258,7 +331,39 @@ export const RecordReferenceRailRenderer: React.FC>(new Set()); - const entriesSig = JSON.stringify(entries.map((e) => `${e.objectName}:${e.relationshipField}:${e.limit ?? 3}`)); + // [objectui#9925] One warning per (object, refused value) per mounted rail — + // the same dedupe shape as the link suppression above, and for the same + // reason: an entry that asked for nothing is silent on screen by + // construction, so the developer channel is the only place it can be said. + // Fired from an effect, never from render, and keyed on the DECLARATION so a + // re-render with the same authored value says nothing a second time. + const warnedRefusedLimits = React.useRef>(new Set()); + + // [objectui#9925] Through the resolver, so this signature names the window + // that actually leaves — two entries whose refused `limit`s differ (`0` and + // `-5`) issue the SAME query and must not read as two different fetches. + const entriesSig = JSON.stringify( + entries.map( + (e) => + `${e.objectName}:${e.relationshipField}:${resolveRowLimit(e.limit, DEFAULT_REFERENCE_RAIL_LIMIT)}`, + ), + ); + // [objectui#9925] The DECLARATION, kept apart from the signature above: the + // diagnostic has to re-fire when the authored value changes even though the + // resolved window does not, which is exactly the pair the resolver collapses. + const authoredLimitSig = JSON.stringify(entries.map((e) => [e.objectName, e.limit ?? null])); + React.useEffect(() => { + for (const entry of entries) { + const message = describeRefusedRowLimit(entry.limit, entry.objectName); + if (!message) continue; + const key = `${entry.objectName}:${String(entry.limit)}`; + if (warnedRefusedLimits.current.has(key)) continue; + warnedRefusedLimits.current.add(key); + console.warn(message); + } + // eslint-disable-next-line react-hooks/exhaustive-deps -- `entries` is tracked by CONTENT (`authoredLimitSig`), the way the fetch effect below tracks it by `entriesSig`; an inline array on a schema node is a new object every render. + }, [authoredLimitSig]); + React.useEffect(() => { if (!railVisible) return; if (!dataSource?.find || !parentId || entries.length === 0) return; @@ -290,7 +395,7 @@ export const RecordReferenceRailRenderer: React.FC = { */ export const DEFAULT_KANBAN_LIMIT = 100; +/** + * What the contract admits as a row cap for this board. + * + * `@objectstack/spec` has already answered what `limit: 0` means. The + * `object-kanban` props declare the member a POSITIVE INTEGER + * (`z.number().int().positive().optional()`, described there as the row cap + * "lowered to the query's top-level `$top`"), and the element data source + * `limit` that a `dataSource` binding lowers into this SAME key is declared + * positive as well. So `0` is not a spelling whose meaning this renderer may + * choose; it is a value the contract refuses, and a renderer that forwards it + * to the wire is the only party not saying so. + */ +function isUsableRowLimit(value: unknown): value is number { + return typeof value === 'number' && Number.isInteger(value) && value > 0; +} + +/** + * The ONE resolver for this board's row cap, for the reason objectui#9853 gave + * when it landed the same shape on `ObjectGrid` and objectui#9897 repeated on + * `ListView`: one resolver at every entry is what keeps the answer single. + * + * Before objectui#9925 this read was a bare `schema.limit ?? DEFAULT_KANBAN_LIMIT`, + * and `??` rejects only `null` and `undefined` — so an authored `limit: 0` was + * not nullish and survived as a real window. It reached the wire as `$top: 0`, + * the board asked the server for nothing, and the empty board named no cause. + * A negative goes out the same way. Both ENTRANCES converge on this key: a + * `dataSource` binding lowers a view's `pagination.pageSize` into `schema.limit` + * before this component sees it, and a board with no binding at all reads the + * authored `limit` from the same place — so resolving HERE covers both, which a + * repair at the lowering layer could not. + * + * ⚠️ The refusal is FAIL-SOFT on purpose. Throwing would take out the whole + * board over one declaration, which is a worse outcome than the defect. The + * value is dropped, this board's own default is used, and + * `describeRefusedRowLimit` states it once through the channel this component + * already uses for "you declared it, the renderer dropped it". ⛔ Not a silent + * clamp: without the loud half this is a substitution the author cannot see, + * and ⛔ not a clamp to 1 either — the author's number is not repaired, it is + * refused, and the board falls back to the window it documents. + */ +function resolveRowLimit(authored: unknown, fallback: number): number { + return isUsableRowLimit(authored) ? authored : fallback; +} + +/** + * The diagnostic half. `null` means "nothing to say" — an absent `limit` is not + * a mistake, and a usable one is not either, so the message is CONDITIONAL and + * the silence controls in the pin are what keep it from being an always-on + * marker that states nothing. + * + * ⛔ NOT a second guard: the predicate lives once, in `isUsableRowLimit`, and + * this reads it. Two predicates would be free to drift, and the drift would be + * invisible — a value refused by one and admitted by the other. + */ +function describeRefusedRowLimit(authored: unknown, objectName: unknown): string | null { + if (authored === undefined || authored === null) return null; + if (isUsableRowLimit(authored)) return null; + const where = + typeof objectName === 'string' && objectName + ? `object-kanban on ${objectName}` + : 'object-kanban'; + return ( + `[ObjectUI] ObjectKanban row cap: ${where} declared limit: ${String(authored)}, ` + + 'which is not a positive integer. A row cap must be a positive integer ' + + '(the spec refuses zero and negative values), so it was ignored and this ' + + `board fell back to its default row cap (${DEFAULT_KANBAN_LIMIT}).` + ); +} + /** * Safe wrapper for useObjectTranslation that falls back to the English defaults * above when no `I18nProvider` is mounted (standalone board, tests). @@ -475,6 +544,16 @@ export const ObjectKanban: React.FC = ({ } }, [externalLoading, hasExternalData]); + // [objectui#9925] The loud half of the row-cap refusal, on the channel this + // component already uses for "you declared it, the renderer dropped it". + // Keyed on the DECLARATION, so it is one warning per declaration rather than + // one per render — and it fires from an effect, never from render, so a + // re-render with the same authored value says nothing a second time. + useEffect(() => { + const message = describeRefusedRowLimit(schema.limit, schema.objectName); + if (message) console.warn(message); + }, [schema.limit, schema.objectName]); + useEffect(() => { // Skip internal fetch when data is managed by a parent component if (hasExternalData) return; @@ -578,15 +657,23 @@ export const ObjectKanban: React.FC = ({ // the saturation reading below (objectui#8307) compares the row // count against `query.$top` — the very number this request // carried. One spelling of the window, read back from the request - // itself: a second `schema.limit ?? DEFAULT_KANBAN_LIMIT` kept in a + // itself: a second `resolveRowLimit(schema.limit, …)` kept in a // local for the comparison could drift from the one on the wire, // and a marker computed against a window the server was never asked - // for is exactly the silent wrongness objectui#8307 is about. - // Keeping it inline here also keeps the spelling objectui#7322 - // pins off disk (`object-kanban-group-by-limit-7322.test.ts`). + // for is exactly the silent wrongness objectui#8307 is about. That + // reasoning is why objectui#9925's refusal was put INSIDE this + // named object rather than beside it: the resolver runs once, and + // the saturation reading keeps reading the number that left. + // Keeping it inline here also keeps this read where objectui#7322 + // pins it off disk (`object-kanban-group-by-limit-7322.test.ts`): + // that pin reads the `$top` expression out of THIS named object, so + // the refusal landing inside it moved the SPELLING and not the + // read. objectui#9925 re-pointed the pin's `READ_TEXT` entry to the + // expression below; the pinned fact — `schema.limit` lowered into + // the query's top-level `$top` — is the same one it always held. const query = { $filter: schema.filter, - $top: schema.limit ?? DEFAULT_KANBAN_LIMIT, + $top: resolveRowLimit(schema.limit, DEFAULT_KANBAN_LIMIT), ...(expand.length > 0 ? { $expand: expand } : {}), }; const results = await dataSource.find(schema.objectName, query); diff --git a/packages/plugin-kanban/src/__tests__/rowLimitNonPositive-9925.test.tsx b/packages/plugin-kanban/src/__tests__/rowLimitNonPositive-9925.test.tsx new file mode 100644 index 0000000000..5ac0198fd2 --- /dev/null +++ b/packages/plugin-kanban/src/__tests__/rowLimitNonPositive-9925.test.tsx @@ -0,0 +1,262 @@ +/** + * 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#9925 — `object-kanban` spent its row cap with a bare `??`, and `??` + * rejects only `null`/`undefined`, so a value the contract refuses survived as + * a real fetch window and reached the adapter as `$top`. + * + * ## Why "refuse it" is not this file inventing a meaning + * + * `@objectstack/spec` already answers what `limit: 0` means. The + * `object-kanban` props declare the member a POSITIVE INTEGER + * (`z.number().int().positive().optional()`), and the element data source + * `limit` a `dataSource` binding lowers into this same key is declared positive + * as well. So `0` is not a spelling whose meaning a consumer may choose; it is + * a value the contract refuses. + * + * ## BOTH entrances, because there are two and only one of them has a gate + * + * The board reads ONE key, `schema.limit`, and two different authoring shapes + * fill it: a `dataSource` binding lowers a named view's `pagination.pageSize` + * into it before this component sees it, and a board with NO binding at all + * carries the authored `limit` straight through. A repair at the lowering layer + * closes only the first. Every refusal row below is therefore run twice, once + * per entrance, and the two are asserted separately rather than in one loop + * whose failure would not say which entrance broke. + * + * ## What the assertions are, and what each control buys + * + * The subject is the RELATION, never a literal: a refused value does not reach + * `$top` and the site's own default does. Each refusal is paired with a control + * that must NOT fire, so a board that simply ignores the member and always + * sends its default cannot pass for a measurement, and an always-on diagnostic + * cannot pass for a diagnosis. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; +import React from 'react'; +import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; +import { DEFAULT_KANBAN_LIMIT } from '../ObjectKanban'; +// Registers `object-kanban` (and the ElementDataSourceGate wiring that carries +// the bound entrance). +import '../index'; +// The lane titles render inside `KanbanRenderer`'s `React.lazy` boundary; +// importing the chunk at module scope bills the cold transform to the import +// phase instead of racing a `waitFor` budget (the objectui#3010 rule). +import '../KanbanImpl'; + +const LANES = [ + { id: 'open', title: 'Open' }, + { id: 'won', title: 'Won' }, +]; + +/** The three values `??` and the resolver DISAGREE about. */ +const REFUSED = [0, -5, 2.5]; + +/** + * A view whose `pagination.pageSize` is what the binding lowers into + * `schema.limit`. Built per test so each row can name its own page size. + */ +const viewWithPageSize = (pageSize: unknown) => ({ + name: 'hot', + label: 'Hot accounts', + columns: ['name', 'rating'], + filter: [['rating', '=', 'hot']], + pagination: { pageSize }, +}); + +function makeAdapter(listViews: Record = {}) { + return { + find: vi.fn().mockResolvedValue({ data: [{ id: '1', name: 'Acme', status: 'open' }] }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ + name: 'account', + fields: { name: { type: 'text' }, status: { type: 'text' }, rating: { type: 'text' } }, + listViews, + }), + }; +} + +const renderBlock = (schema: Record, adapter: ReturnType) => + render( + + + , + ); + +/** Every `$top` that left this board, in call order. */ +const tops = async (adapter: ReturnType) => { + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + return adapter.find.mock.calls.map((c: any[]) => c[1]?.$top); +}; + +/** ENTRANCE 1 — authored `limit`, no `dataSource` binding anywhere. */ +const authoredBoard = (limit: unknown) => ({ + type: 'object-kanban', + objectName: 'account', + groupBy: 'status', + columns: LANES, + ...(limit === undefined ? {} : { limit }), +}); + +/** ENTRANCE 2 — no authored `limit`; a bound view's page size fills the key. */ +const boundBoard = () => ({ + type: 'object-kanban', + groupBy: 'status', + columns: LANES, + dataSource: { object: 'account', view: 'hot' }, +}); + +let warnings: string[] = []; +beforeEach(() => { + warnings = []; + vi.spyOn(console, 'warn').mockImplementation((...args: unknown[]) => { + warnings.push(args.map(String).join(' ')); + }); +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +const rowCapWarnings = () => warnings.filter((w) => w.includes('ObjectKanban row cap')); + +describe('object-kanban — a row cap the contract refuses never reaches the wire (objectui#9925)', () => { + // ── ENTRANCE 1: AUTHORED, NO BINDING ─────────────────────────────────── + describe('authored `limit`, with no `dataSource` binding at all', () => { + it.each(REFUSED)('never sends `$top: %s`', async (bad) => { + const adapter = makeAdapter(); + renderBlock(authoredBoard(bad), adapter); + + const sent = await tops(adapter); + expect(sent.length).toBeGreaterThan(0); + expect(sent, `the authored ${bad} reached the wire`).not.toContain(bad); + for (const top of sent) { + expect(typeof top).toBe('number'); + expect(Number.isInteger(top)).toBe(true); + expect(top).toBeGreaterThan(0); + } + }); + + it.each(REFUSED)('falls back to this board’s OWN default instead of %s', async (bad) => { + const adapter = makeAdapter(); + renderBlock(authoredBoard(bad), adapter); + + const sent = await tops(adapter); + // The relation, not the literal: whatever this site documents as its + // default is what a refused declaration falls back to. + for (const top of sent) expect(top).toBe(DEFAULT_KANBAN_LIMIT); + }); + + it('CONTROL — a legitimate authored limit still reaches `$top` unchanged', async () => { + const adapter = makeAdapter(); + renderBlock(authoredBoard(7), adapter); + + const sent = await tops(adapter); + // Without this row, "never 0" is satisfied by a board that ignores the + // member entirely and always sends its own default. + expect(sent).toContain(7); + expect(sent).not.toContain(DEFAULT_KANBAN_LIMIT); + }); + + it('CONTROL — declaring no limit at all still sends the default', async () => { + const adapter = makeAdapter(); + renderBlock(authoredBoard(undefined), adapter); + + const sent = await tops(adapter); + expect(sent).toContain(DEFAULT_KANBAN_LIMIT); + }); + }); + + // ── ENTRANCE 2: A BOUND VIEW'S PAGE SIZE ─────────────────────────────── + describe('a bound view’s `pagination.pageSize`, lowered into the same key', () => { + it.each(REFUSED)('never sends `$top: %s`', async (bad) => { + const adapter = makeAdapter({ hot: viewWithPageSize(bad) }); + renderBlock(boundBoard(), adapter); + + const sent = await tops(adapter); + expect(sent.length).toBeGreaterThan(0); + expect(sent, `the view's ${bad} reached the wire`).not.toContain(bad); + for (const top of sent) expect(top).toBe(DEFAULT_KANBAN_LIMIT); + }); + + it('CONTROL — a legitimate page size still becomes the board’s window', async () => { + const adapter = makeAdapter({ hot: viewWithPageSize(7) }); + renderBlock(boundBoard(), adapter); + + const sent = await tops(adapter); + expect(sent).toContain(7); + }); + + it('CONTROL — the view’s filter still arrives when its page size is refused', async () => { + // ⛔ No capability removed: refusing one member must not cost the rest of + // the binding. + const adapter = makeAdapter({ hot: viewWithPageSize(0) }); + renderBlock(boundBoard(), adapter); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + const [object, params] = adapter.find.mock.calls[0] as [string, any]; + expect(object).toBe('account'); + expect(params.$filter).toEqual([['rating', '=', 'hot']]); + }); + }); + + // ── THE DIAGNOSTIC ───────────────────────────────────────────────────── + describe('the diagnostic half', () => { + it.each(REFUSED)('names this block, the object and the value %s', async (bad) => { + const adapter = makeAdapter(); + renderBlock(authoredBoard(bad), adapter); + + await waitFor(() => expect(rowCapWarnings().length).toBeGreaterThan(0)); + const message = rowCapWarnings()[0]; + // Substituting a number the author never wrote is the quieter half of the + // same defect; this is what makes it a diagnosis. + expect(message).toContain('object-kanban'); + expect(message).toContain('account'); + expect(message).toContain('limit'); + expect(message).toContain(String(bad)); + }); + + it('fires exactly ONCE for one declaration, across re-renders', async () => { + const adapter = makeAdapter(); + const { rerender } = renderBlock(authoredBoard(0), adapter); + + await waitFor(() => expect(rowCapWarnings().length).toBeGreaterThan(0)); + // A fresh schema OBJECT carrying the same declaration. The effect is + // keyed on the declared primitives, so this must say nothing more. + rerender( + + + , + ); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + expect(rowCapWarnings()).toHaveLength(1); + }); + + it('CONTROL — a legitimate limit produces no such diagnostic', async () => { + const adapter = makeAdapter(); + renderBlock(authoredBoard(7), adapter); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + // An always-on marker states nothing. + expect(rowCapWarnings()).toHaveLength(0); + }); + + it('CONTROL — declaring no limit at all produces no diagnostic', async () => { + const adapter = makeAdapter(); + renderBlock(authoredBoard(undefined), adapter); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + expect(rowCapWarnings()).toHaveLength(0); + }); + }); +}); diff --git a/packages/plugin-timeline/src/ObjectTimeline.tsx b/packages/plugin-timeline/src/ObjectTimeline.tsx index 3a64820ec3..c53f8dc891 100644 --- a/packages/plugin-timeline/src/ObjectTimeline.tsx +++ b/packages/plugin-timeline/src/ObjectTimeline.tsx @@ -28,6 +28,73 @@ import { useTimelineTranslation } from './useTimelineTranslation'; */ export const DEFAULT_TIMELINE_LIMIT = 100; +/** + * What the contract admits as a row cap for this rail. + * + * `@objectstack/spec` has already answered what `limit: 0` means: the element + * data source `limit` that a `dataSource` binding lowers into this key is + * declared a POSITIVE INTEGER (`z.number().int().positive().optional()`), and + * so is the `pagination.pageSize` of a named view that fills it. So `0` is not + * a spelling whose meaning this renderer may choose; it is a value the contract + * refuses, and a renderer that forwards it to the wire is the only party not + * saying so. + */ +function isUsableRowLimit(value: unknown): value is number { + return typeof value === 'number' && Number.isInteger(value) && value > 0; +} + +/** + * The ONE resolver for this rail's row cap, for the reason objectui#9853 gave + * when it landed the same shape on `ObjectGrid` and objectui#9897 repeated on + * `ListView`: one resolver at every entry is what keeps the answer single. + * + * Before objectui#9925 this read was a bare `schema.limit ?? DEFAULT_TIMELINE_LIMIT`, + * and `??` rejects only `null` and `undefined` — so an authored `limit: 0` was + * not nullish and survived as a real window. It reached the wire as `$top: 0`, + * the rail asked the server for nothing, and the empty rail named no cause. A + * negative goes out the same way. Both ENTRANCES converge on this key: a + * `dataSource` binding lowers a view's `pagination.pageSize` into `schema.limit` + * before this component sees it, and a rail with no binding at all reads the + * authored `limit` from the same place — so resolving HERE covers both, which a + * repair at the lowering layer could not. + * + * ⚠️ The refusal is FAIL-SOFT on purpose. Throwing would take out the whole + * rail over one declaration, which is a worse outcome than the defect. The + * value is dropped, this rail's own default is used, and + * `describeRefusedRowLimit` states it once through the channel this component + * already uses for "you declared it, the renderer dropped it" (the same + * `console.warn` the timeline-config parse above writes to). ⛔ Not a silent + * clamp, and ⛔ not a clamp to 1: the author's number is refused, not repaired. + */ +function resolveRowLimit(authored: unknown, fallback: number): number { + return isUsableRowLimit(authored) ? authored : fallback; +} + +/** + * The diagnostic half. `null` means "nothing to say" — an absent `limit` is not + * a mistake, and a usable one is not either, so the message is CONDITIONAL and + * the silence controls in the pin are what keep it from being an always-on + * marker that states nothing. + * + * ⛔ NOT a second guard: the predicate lives once, in `isUsableRowLimit`, and + * this reads it. Two predicates would be free to drift, and the drift would be + * invisible — a value refused by one and admitted by the other. + */ +function describeRefusedRowLimit(authored: unknown, objectName: unknown): string | null { + if (authored === undefined || authored === null) return null; + if (isUsableRowLimit(authored)) return null; + const where = + typeof objectName === 'string' && objectName + ? `object-timeline on ${objectName}` + : 'object-timeline'; + return ( + `[ObjectUI] ObjectTimeline row cap: ${where} declared limit: ${String(authored)}, ` + + 'which is not a positive integer. A row cap must be a positive integer ' + + '(the spec refuses zero and negative values), so it was ignored and this ' + + `timeline fell back to its default row cap (${DEFAULT_TIMELINE_LIMIT}).` + ); +} + /** * The variants an OBJECT-BOUND timeline can render. * @@ -201,6 +268,18 @@ export const ObjectTimeline: React.FC = ({ } }, [schema]); + // [objectui#9925] The loud half of the row-cap refusal, on the same channel + // as the parse warning just above. Keyed on the DECLARATION, so it is one + // warning per declaration rather than one per render — and it fires from an + // effect, never from render, so a re-render with the same authored value says + // nothing a second time. (`TimelineExtensionSchema` above declares no + // `limit`, and it is a non-strict object, so an authored `limit: 0` passed + // that parse in silence; this is the only place it is stated.) + useEffect(() => { + const message = describeRefusedRowLimit(schema.limit, schema.objectName); + if (message) console.warn(message); + }, [schema.limit, schema.objectName]); + const boundData = useDataScope(schema.bind); /** @@ -325,7 +404,7 @@ export const ObjectTimeline: React.FC = ({ const results = await dataSource.find(schema.objectName, { $filter: schema.filter, $orderby: convertSortToQueryParams(schema.sort), - $top: schema.limit ?? DEFAULT_TIMELINE_LIMIT, + $top: resolveRowLimit(schema.limit, DEFAULT_TIMELINE_LIMIT), ...(expand.length > 0 ? { $expand: expand } : {}), }); const data = extractRecords(results); diff --git a/packages/plugin-timeline/src/__tests__/rowLimitNonPositive-9925.test.tsx b/packages/plugin-timeline/src/__tests__/rowLimitNonPositive-9925.test.tsx new file mode 100644 index 0000000000..ef15b0f13b --- /dev/null +++ b/packages/plugin-timeline/src/__tests__/rowLimitNonPositive-9925.test.tsx @@ -0,0 +1,260 @@ +/** + * 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#9925 — `object-timeline` spent its row cap with a bare `??`, and + * `??` rejects only `null`/`undefined`, so a value the contract refuses + * survived as a real fetch window and reached the adapter as `$top`. + * + * ## Why "refuse it" is not this file inventing a meaning + * + * The element data source `limit` a `dataSource` binding lowers into + * `schema.limit` is declared a POSITIVE INTEGER by `@objectstack/spec` + * (`z.number().int().positive().optional()`), and so is the + * `pagination.pageSize` of a named view that fills it. So `0` is not a spelling + * whose meaning a consumer may choose; it is a value the contract refuses. + * + * ⚠️ This renderer's own `TimelineExtensionSchema.safeParse` is NOT that + * refusal and cannot be read as one: it declares no `limit` member and is a + * non-strict object, so an authored `limit: 0` passed it in silence. The + * control at the foot of this file pins that separation — the parse warning + * and the row-cap warning are different messages on the same channel. + * + * ## BOTH entrances, because there are two and only one of them has a gate + * + * The rail reads ONE key, `schema.limit`, and two authoring shapes fill it: a + * `dataSource` binding lowers a named view's `pagination.pageSize` into it, and + * a rail with NO binding at all carries the authored `limit` straight through. + * A repair at the lowering layer closes only the first, so every refusal row is + * run once per entrance and the two are asserted separately. + * + * ## What the assertions are, and what each control buys + * + * The subject is the RELATION, never a literal: a refused value does not reach + * `$top` and the site's own default does. Each refusal is paired with a control + * that must NOT fire, so a rail that simply ignores the member cannot pass for + * a measurement and an always-on diagnostic cannot pass for a diagnosis. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; +import React from 'react'; +import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; +import { DEFAULT_TIMELINE_LIMIT } from '../ObjectTimeline'; +// Registers `object-timeline` (and the ElementDataSourceGate wiring that +// carries the bound entrance). +import '../index'; + +const TIMELINE = { startDateField: 'start_date', endDateField: 'end_date', titleField: 'name' }; + +/** The three values `??` and the resolver DISAGREE about. */ +const REFUSED = [0, -5, 2.5]; + +const viewWithPageSize = (pageSize: unknown) => ({ + name: 'hot', + label: 'Hot campaigns', + columns: ['name', 'stage'], + filter: [['stage', '=', 'live']], + pagination: { pageSize }, +}); + +function makeAdapter(listViews: Record = {}) { + return { + find: vi.fn().mockResolvedValue({ data: [] }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue({ + name: 'campaign', + fields: { + name: { type: 'text' }, + stage: { type: 'text' }, + start_date: { type: 'datetime' }, + end_date: { type: 'datetime' }, + }, + listViews, + }), + }; +} + +const renderBlock = (schema: Record, adapter: ReturnType) => + render( + + + , + ); + +const tops = async (adapter: ReturnType) => { + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + return adapter.find.mock.calls.map((c: any[]) => c[1]?.$top); +}; + +/** ENTRANCE 1 — authored `limit`, no `dataSource` binding anywhere. */ +const authoredRail = (limit: unknown) => ({ + type: 'object-timeline', + objectName: 'campaign', + timeline: TIMELINE, + ...(limit === undefined ? {} : { limit }), +}); + +/** ENTRANCE 2 — no authored `limit`; a bound view's page size fills the key. */ +const boundRail = () => ({ + type: 'object-timeline', + timeline: TIMELINE, + dataSource: { object: 'campaign', view: 'hot' }, +}); + +let warnings: string[] = []; +beforeEach(() => { + warnings = []; + vi.spyOn(console, 'warn').mockImplementation((...args: unknown[]) => { + warnings.push(args.map(String).join(' ')); + }); +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +const rowCapWarnings = () => warnings.filter((w) => w.includes('ObjectTimeline row cap')); + +describe('object-timeline — a row cap the contract refuses never reaches the wire (objectui#9925)', () => { + // ── ENTRANCE 1: AUTHORED, NO BINDING ─────────────────────────────────── + describe('authored `limit`, with no `dataSource` binding at all', () => { + it.each(REFUSED)('never sends `$top: %s`', async (bad) => { + const adapter = makeAdapter(); + renderBlock(authoredRail(bad), adapter); + + const sent = await tops(adapter); + expect(sent.length).toBeGreaterThan(0); + expect(sent, `the authored ${bad} reached the wire`).not.toContain(bad); + for (const top of sent) { + expect(typeof top).toBe('number'); + expect(Number.isInteger(top)).toBe(true); + expect(top).toBeGreaterThan(0); + } + }); + + it.each(REFUSED)('falls back to this rail’s OWN default instead of %s', async (bad) => { + const adapter = makeAdapter(); + renderBlock(authoredRail(bad), adapter); + + const sent = await tops(adapter); + for (const top of sent) expect(top).toBe(DEFAULT_TIMELINE_LIMIT); + }); + + it('CONTROL — a legitimate authored limit still reaches `$top` unchanged', async () => { + const adapter = makeAdapter(); + renderBlock(authoredRail(7), adapter); + + const sent = await tops(adapter); + expect(sent).toContain(7); + expect(sent).not.toContain(DEFAULT_TIMELINE_LIMIT); + }); + + it('CONTROL — declaring no limit at all still sends the default', async () => { + const adapter = makeAdapter(); + renderBlock(authoredRail(undefined), adapter); + + const sent = await tops(adapter); + expect(sent).toContain(DEFAULT_TIMELINE_LIMIT); + }); + }); + + // ── ENTRANCE 2: A BOUND VIEW'S PAGE SIZE ─────────────────────────────── + describe('a bound view’s `pagination.pageSize`, lowered into the same key', () => { + it.each(REFUSED)('never sends `$top: %s`', async (bad) => { + const adapter = makeAdapter({ hot: viewWithPageSize(bad) }); + renderBlock(boundRail(), adapter); + + const sent = await tops(adapter); + expect(sent.length).toBeGreaterThan(0); + expect(sent, `the view's ${bad} reached the wire`).not.toContain(bad); + for (const top of sent) expect(top).toBe(DEFAULT_TIMELINE_LIMIT); + }); + + it('CONTROL — a legitimate page size still becomes the rail’s window', async () => { + const adapter = makeAdapter({ hot: viewWithPageSize(7) }); + renderBlock(boundRail(), adapter); + + const sent = await tops(adapter); + expect(sent).toContain(7); + }); + + it('CONTROL — the view’s filter still arrives when its page size is refused', async () => { + // ⛔ No capability removed: refusing one member must not cost the rest of + // the binding. + const adapter = makeAdapter({ hot: viewWithPageSize(0) }); + renderBlock(boundRail(), adapter); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + const [object, params] = adapter.find.mock.calls[0] as [string, any]; + expect(object).toBe('campaign'); + expect(params.$filter).toEqual([['stage', '=', 'live']]); + }); + }); + + // ── THE DIAGNOSTIC ───────────────────────────────────────────────────── + describe('the diagnostic half', () => { + it.each(REFUSED)('names this block, the object and the value %s', async (bad) => { + const adapter = makeAdapter(); + renderBlock(authoredRail(bad), adapter); + + await waitFor(() => expect(rowCapWarnings().length).toBeGreaterThan(0)); + const message = rowCapWarnings()[0]; + expect(message).toContain('object-timeline'); + expect(message).toContain('campaign'); + expect(message).toContain('limit'); + expect(message).toContain(String(bad)); + }); + + it('fires exactly ONCE for one declaration, across re-renders', async () => { + const adapter = makeAdapter(); + const { rerender } = renderBlock(authoredRail(0), adapter); + + await waitFor(() => expect(rowCapWarnings().length).toBeGreaterThan(0)); + rerender( + + + , + ); + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + expect(rowCapWarnings()).toHaveLength(1); + }); + + it('CONTROL — a legitimate limit produces no such diagnostic', async () => { + const adapter = makeAdapter(); + renderBlock(authoredRail(7), adapter); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + expect(rowCapWarnings()).toHaveLength(0); + }); + + it('CONTROL — declaring no limit at all produces no diagnostic', async () => { + const adapter = makeAdapter(); + renderBlock(authoredRail(undefined), adapter); + + await waitFor(() => expect(adapter.find).toHaveBeenCalled()); + expect(rowCapWarnings()).toHaveLength(0); + }); + + it('is a SEPARATE message from this renderer’s own config parse warning', async () => { + // The parse above the resolver declares no `limit` and is non-strict, so + // it says nothing about this value. Without this row, "a warning fired" + // could be satisfied by the pre-existing parse warning. + const adapter = makeAdapter(); + renderBlock(authoredRail(0), adapter); + + await waitFor(() => expect(rowCapWarnings().length).toBeGreaterThan(0)); + expect( + warnings.filter((w) => w.includes('Invalid timeline configuration')), + 'the config parse refused this value, so the row-cap warning proves nothing', + ).toHaveLength(0); + }); + }); +}); diff --git a/packages/types/src/__tests__/object-kanban-group-by-limit-7322.test.ts b/packages/types/src/__tests__/object-kanban-group-by-limit-7322.test.ts index f5f8308325..13f3fa5567 100644 --- a/packages/types/src/__tests__/object-kanban-group-by-limit-7322.test.ts +++ b/packages/types/src/__tests__/object-kanban-group-by-limit-7322.test.ts @@ -21,13 +21,15 @@ * ## The defect * * `packages/plugin-kanban/src/ObjectKanban.tsx` — the component the - * `object-kanban` registration renders — reads `schema.groupBy` at thirteen + * `object-kanban` registration renders — read `schema.groupBy` at thirteen * sites (lane materialisation, card moves, their effect deps) and * `schema.limit` at two (`$top: schema.limit ?? DEFAULT_KANBAN_LIMIT` and the - * effect deps). `groupField` has ZERO read sites anywhere under - * `packages/plugin-kanban/`. Yet the declaration in `../objectql.ts` REQUIRED - * `groupField` and declared neither `groupBy` nor `limit`, and the zod mirror in - * `../zod/objectql.zod.ts` restated it. Measured on `53ded82b` from source: + * effect deps) WHEN THIS CARD MEASURED IT — the `limit` half has since moved, + * see the objectui#9925 note closing this section. `groupField` has ZERO read + * sites anywhere under `packages/plugin-kanban/`. Yet the declaration in + * `../objectql.ts` REQUIRED `groupField` and declared neither `groupBy` nor + * `limit`, and the zod mirror in `../zod/objectql.zod.ts` restated it. + * Measured on `53ded82b` from source: * the documented, tested, working shape — `{ type: 'object-kanban', * objectName, groupBy, limit }` — FAILED `ObjectKanbanSchema.safeParse` and * `safeValidateSchema` on the missing `groupField`, while a `groupField`-only @@ -35,6 +37,18 @@ * `limit` only ever reached the renderer through `BaseSchema`'s * `[key: string]: any` and `.passthrough()` — admitted, never examined. * + * ⚠️ THE `limit` READING ABOVE IS THE ONE objectui#7322 MEASURED, NOT TODAY'S + * (objectui#9925). That card made the board REFUSE a non-positive row cap + * before it reaches the wire, and both halves of the reading moved with it: + * the spelling is now `$top: resolveRowLimit(schema.limit, + * DEFAULT_KANBAN_LIMIT)`, and the `schema.limit` CODE read sites are FOUR, not + * two — that `$top`, its effect's deps, the refusal diagnostic and that + * diagnostic's own effect deps. The older reading is left standing rather than + * rewritten away because it is the measurement that motivated the declaration; + * what is LIVE is `READ_TEXT` below, which a test re-derives off disk every + * run, while this paragraph is prose nothing re-checks. The thirteen + * `groupBy` sites are untouched by that card. + * * ## What this file pins, and the shapes it borrows * * The declare half is `chat-message-avatar-keys-7295.test.ts` / @@ -82,6 +96,18 @@ const RETIRED = 'groupField'; /** * Exact source text of the reads, as they stand today. Line numbers drift and * live in the docblocks' prose only; the READ is the fact. + * + * The `limit` text was RE-POINTED by objectui#9925, which is what that rule + * prescribes rather than an exception to it. The pinned FACT is that the + * renderer lowers `schema.limit` into the query's top-level `$top`; that fact + * survived intact and only its spelling moved. `??` rejects `null` and + * `undefined` and nothing else, so an authored `limit: 0` was not nullish, rode + * through as a real window and reached the wire as `$top: 0` — on a key + * `@objectstack/spec` declares a POSITIVE integer. `resolveRowLimit` refuses + * that value instead of forwarding it. Same file, same reader, same named + * `query` object, same key, and still ONE `toContain`: the pin keeps the + * strength it had, and the new string is narrow enough that the retired `??` + * spelling could not satisfy it either. */ const READ_TEXT: Record = { groupBy: [ @@ -89,7 +115,7 @@ const READ_TEXT: Record = { 'if (schema.groupBy && objectDef?.fields?.[schema.groupBy]?.options) {', 'const groupBy = schema.groupBy;', ], - limit: ['$top: schema.limit ?? DEFAULT_KANBAN_LIMIT'], + limit: ['$top: resolveRowLimit(schema.limit, DEFAULT_KANBAN_LIMIT)'], }; /** The default the `limit` docblock names. */ const DEFAULT_LIMIT_TEXT = 'export const DEFAULT_KANBAN_LIMIT = 100;'; diff --git a/packages/types/src/objectql.ts b/packages/types/src/objectql.ts index b0951b67e7..7d0a01db9c 100644 --- a/packages/types/src/objectql.ts +++ b/packages/types/src/objectql.ts @@ -3713,12 +3713,13 @@ export interface ObjectKanbanSchema extends BaseSchema { }>; /** * Row cap — the most records the board fetches, sent as a real `$top` on - * the query (`packages/plugin-kanban/src/ObjectKanban.tsx:264`, - * `$top: schema.limit ?? DEFAULT_KANBAN_LIMIT`; objectui#4025). The board - * renders every fetched record into a lane and offers no pagination, so - * this is the author's window on the object rather than a page size. A - * bound `dataSource` (its own `limit`, or the named view's - * `pagination.pageSize`) sets it too. Undeclared until objectui#7322. + * the query (`packages/plugin-kanban/src/ObjectKanban.tsx`, + * `$top: resolveRowLimit(schema.limit, DEFAULT_KANBAN_LIMIT)`; objectui#4025, + * re-spelled by objectui#9925, which refuses a non-positive cap rather than + * forwarding it). The board renders every fetched record into a lane and + * offers no pagination, so this is the author's window on the object rather + * than a page size. A bound `dataSource` (its own `limit`, or the named + * view's `pagination.pageSize`) sets it too. Undeclared until objectui#7322. * * @default 100 — `DEFAULT_KANBAN_LIMIT` */ diff --git a/packages/types/src/zod/objectql.zod.ts b/packages/types/src/zod/objectql.zod.ts index f579544b47..a8e768826e 100644 --- a/packages/types/src/zod/objectql.zod.ts +++ b/packages/types/src/zod/objectql.zod.ts @@ -2093,13 +2093,14 @@ function requireKanbanRecordSource( } // objectui#7322 — `groupBy` and `limit` are the keys `ObjectKanban.tsx` reads -// (thirteen `schema.groupBy` sites; `$top: schema.limit ?? DEFAULT_KANBAN_LIMIT` -// at `:264`); until this card neither was declared and both rode `BaseSchema`'s -// `.passthrough()` unexamined, while the REQUIRED `groupField` had zero read -// sites. `groupField` is now a `retirementTombstone()` — still a member, so -// the parity ratchet's key sets stay equal and an authored value is refused -// BY NAME rather than stripped — and it is node-local: the VIEW-LEVEL alias -// `KanbanConfig.groupField` above is live and untouched. +// (thirteen `schema.groupBy` sites; the row cap lowered into the query as +// `$top: resolveRowLimit(schema.limit, DEFAULT_KANBAN_LIMIT)`, re-spelled by +// objectui#9925); until this card neither was declared and both rode +// `BaseSchema`'s `.passthrough()` unexamined, while the REQUIRED `groupField` +// had zero read sites. `groupField` is now a `retirementTombstone()` — still +// a member, so the parity ratchet's key sets stay equal and an authored value +// is refused BY NAME rather than stripped — and it is node-local: the +// VIEW-LEVEL alias `KanbanConfig.groupField` above is live and untouched. export const ObjectKanbanSchema = BaseSchema.extend({ type: z.literal('object-kanban'), objectName: z.string().optional().describe('ObjectQL object name — the LAST rung of the board ladder, after the pre-fetched data prop, bind and the inline row array on data; one of bind, data, objectName must be present (objectui#7780)'),