diff --git a/.changeset/member-pins-slice-7.md b/.changeset/member-pins-slice-7.md new file mode 100644 index 0000000000..05ea66bdb9 --- /dev/null +++ b/.changeset/member-pins-slice-7.md @@ -0,0 +1,9 @@ +--- +--- + +Test-only change (objectui#8071 slice 7): converts the last four one-key +`MEMBER_PIN_EXEMPTIONS` blocks — `element:button.action`, +`element:number.filter`, `page:accordion.items` and `page:tabs.items` — into +real per-block member pins, deletes those four exemptions and lowers +`MEMBER_PIN_EXEMPTION_CEILING` 41 -> 37 in the same commit. Adds four member-shape +test files under `packages/components`; no published behaviour changes. diff --git a/apps/console/src/__tests__/registry-inputs-spec-parity.test.ts b/apps/console/src/__tests__/registry-inputs-spec-parity.test.ts index c5e76b6b9d..c5497f50c8 100644 --- a/apps/console/src/__tests__/registry-inputs-spec-parity.test.ts +++ b/apps/console/src/__tests__/registry-inputs-spec-parity.test.ts @@ -2149,10 +2149,18 @@ interface MemberPin { * was built for. */ const MEMBER_PINS: Record = { + 'element:button.action': { + file: 'packages/components/src/__tests__/elementButtonActionMembers-8071.test.tsx', + pins: 'The inline ActionDef\'s member set as a WHITELIST, driven through the real renderer and asserted on what the ActionRunner is handed: the forwarded keys are pinned as a SORTED SET, so a member silently added to or dropped from `ElementButtonRenderer`\'s explicit forward list is red in either direction — the defect shape objectstack#6837 (`bodyExtra`) and objectstack#6938 (`bodyShape`) were both filed for, an authored key that validates, publishes and then evaporates one hop before the runner. Three off-list keys are authored alongside and asserted ABSENT: `bodyShape` (deliberately not inline vocabulary — `action-bodyShape-forward.test.tsx` argues the boundary in prose, this pins it as behaviour) plus `icon` and `variant`, the block\'s own sibling props misplaced one level in. `actionType` OUTRANKS `type` with the losing spelling\'s handler asserted un-run, an ARRAY `params` is re-routed to `actionParams` while an OBJECT `params` stays `params` (objectstack#5777 direction A) with each arm controlling the other, the navigation members reach the navigation handler, and an omitted `action` dispatches nothing at all — the non-vacuity control for every row. LIMIT: `confirmText` is on the whitelist but unexercised, because the runner awaits confirmation before the dispatch this file observes. New file (objectui#8071 slice 7).', + }, 'element:button.label': { file: 'apps/console/src/__tests__/component-input-union-specimens.test.ts', pins: 'The `object` arm is the inline translation map `{ en, "zh-CN" }` and nothing else — driven through the real `manifestFromConfigs` + `validateTree` pair the JSX-page compiler and the save gate use, each positive paired with a value matching NEITHER arm that must still be reported (objectui#4970).', }, + 'element:number.filter': { + file: 'packages/components/src/renderers/basic/__tests__/elementNumberFilterMembers-8071.test.tsx', + pins: 'The TWO wire spellings one authored predicate takes, chosen by an adapter capability the author cannot see, plus the re-query rule — asserted through the real renderer on a stubbed adapter. `aggregate()` receives it FLAT under its own name and beside the members that make the call an aggregate (`field`, `function`, `groupBy: \'_all\'`), asserted as the whole options bag rather than the one key; the `find()` fallback receives it WRAPPED as `$filter`; and an unfiltered metric sends `undefined` rather than an empty envelope some adapters read as "match nothing". Collapsing the two spellings into one drops the predicate silently and the metric paints a confidently wrong number over every row, with no diagnostic and no empty state. The third half is that the key is read BY VALUE, not by identity (`filterKey` is a `JSON.stringify` memo): a deep-equal filter rebuilt by a re-rendering parent must NOT re-probe, while a changed comparand MUST and carries the new predicate — each arm the other\'s control, so a dependency array "simplified" to the raw object (a per-render fetch storm) is red. New file (objectui#8071 slice 7).', + }, 'element:record_picker.dataSource': { file: 'packages/components/src/__tests__/record-picker-element-data-source.test.tsx', pins: 'The per-element binding\'s own members, read through the REAL renderer and asserted at the query it fires: `view` resolves a saved view whose `filter`/`sort`/row cap become the picker\'s baseline, an authored binding key WINS over the same key from the view, the binding\'s own `filter` arrives AND-combined with the view\'s rather than replacing it, and an unresolvable `view` reports instead of falling back to every record of the object — the quieter failure mode objectstack#6953 fixed, one class below the throw objectstack#5576 fixed on `list-view`. A `view`-less binding leaves every one of the four flat keys (`object`/`filter`/`sort`/`limit`) behaving exactly as it did before a `dataSource` was authored at all, which is the control that keeps the override rows from reading as a coincidence. `columns` is deliberately absent from this pin: unlike `record:related_list.dataSource` (objectui#8071 slice 2), this renderer never reads `composed.columns` at all — a select dropdown has no column list to narrow. Pre-existing file, promoted to a pin here after being read end to end (objectui#8071).', @@ -2257,6 +2265,10 @@ const MEMBER_PINS: Record = { file: 'packages/plugin-kanban/src/__tests__/ObjectKanban.structuredMembersReachTheirSinks-8313.test.tsx', pins: 'ONE nested position and no more: `schema.grouping?.fields?.[0]?.field` is the FALLBACK source of `swimlaneField`, and that is the entire member contract this board carries for the key. Three rows make it a reading rather than a claim — the swimlane layout appears keyed by `fields[0].field` where without the key there is none; an explicit `swimlaneField` WINS over it; and a second `fields` entry changes nothing, which is what pins the read at `[0]` rather than at "the fields list". The declared description says the rest is inert precisely so the declaration does not recommend a write the renderer cannot honour — this file is what keeps that sentence true. The spec row is `z.unknown()`, so the read site is the whole member contract (objectui#8313).', }, + 'page:accordion.items': { + file: 'packages/components/src/__tests__/pageAccordionItemMembers-8071.test.tsx', + pins: 'Which member of one panel definition becomes which part of the rendered accordion — the four `PageAccordionItem` members (`label`, `icon`, `collapsed`, `children`) asserted as a SET through the real renderer, which nothing did before: `label` becomes the trigger\'s accessible name and `children` the panel BODY, asserted against each other so a renderer painting the wrong one cannot pass. `collapsed` is the reading with real semantics to get wrong and it is strictly `=== false`: `collapsed: false` OPENS a panel while `collapsed: true` AND an omitted `collapsed` both leave it shut — the omitted-key arm is the control an "obvious" edit to `!it.collapsed` breaks, and a no-opener fixture keeps the two shut rows from passing on a renderer that opens nothing. The single/multiple split is pinned on the SAME items so only `allowMultiple` varies: single mode takes `defaultOpen[0]` and drops later openers, multiple mode opens them all without opening panels that never asked. `icon` is covered narrowly on purpose — `page-accordion-icon.test.tsx` (objectui#4721) is its pin and is not re-litigated. New file (objectui#8071 slice 7).', + }, 'page:card.title': { file: 'apps/console/src/__tests__/component-input-union-specimens.test.ts', pins: 'The `object` arm is the inline translation map, with a non-matching control that must still be reported (objectui#3832).', @@ -2273,6 +2285,10 @@ const MEMBER_PINS: Record = { file: 'apps/console/src/__tests__/component-input-union-specimens.test.ts', pins: 'The `object` arm is the inline translation map, with a non-matching control (objectui#3832).', }, + 'page:tabs.items': { + file: 'packages/components/src/__tests__/pageTabsItemMembers-8071.test.tsx', + pins: 'The SIX members of one tab definition (`label`, `value`, `icon`, `count`, `visibleWhen`, `children`) mapped to the six different parts of the strip they each become, driven through the real renderer — the mapping as a set, which none of the five pre-existing `page:tabs` suites asserts (each pins one member, so none would fail if `count` and `value` swapped roles). `label` becomes the trigger name and `children` the PANEL, with the inactive tab\'s body asserted absent since Radix renders only the active one. `value` is pinned BEHAVIOURALLY through `defaultTab` — the host key app-shell restores `?tab=` with — so selecting a tab by its authored value proves the member is the tab\'s identity without reaching into Radix\'s generated ids; both fallback arms follow, an absent `value` and an EMPTY-STRING one each becoming the positional `tab-INDEX`, which is what separates the real read from a bare `typeof value === \'string\'`. `count` is gated on `Number(count) > 0`, NOT on presence: an authored `count: 0` paints no badge, with a positive count on the same strip as its control. `icon` and `count` are asserted per-item against a neighbour declaring neither, so neither can be strip-wide. `visibleWhen` is covered narrowly — `page-tabs-visibility.test.tsx` is its pin. New file (objectui#8071 slice 7).', + }, 'record:activity.types': { file: 'packages/plugin-detail/src/renderers/__tests__/recordActivityFeed.test.ts', pins: 'The allow-list\'s MEMBERS, asserted on `applyFeedConfig` — the call `record-activity.tsx` makes on every render — rather than through a DOM: each entry must name a `FeedItemType` the spec declares (the vocabulary is read from `@objectstack/spec`, never re-typed), a recognised list narrows the timeline to exactly those kinds, and the three ways a list can fail to name anything are kept APART from "no filter at all" (objectui#5841): `types: []`, an all-unrecognised list, and a non-array `types: \'comment\'` each render an EMPTY timeline, while an omitted `types` renders every kind. A mixed list keeps the recognised members and drops the rest. Both diagnostic channels are asserted in both directions with controls that a well-formed filter stays silent — including the objectui#5877 channel for a declared kind no producer emits, whose populations are derived from the producers rather than hand-listed. Pre-existing file, promoted to a pin here after being read end to end (objectui#8071 slice 6).', @@ -2447,11 +2463,11 @@ const NO_READ_SITE_TO_PIN = * 51st entry, because the count may only go down. */ const MEMBER_PIN_EXEMPTIONS: Record = { - // element:button - 'element:button.action': AWAITING_A_PIN, + // element:button — objectui#8071 slice 7 pinned `action`, the block's one + // remaining key; fully pinned, header kept as a landmark for a future grep. - // element:number - 'element:number.filter': AWAITING_A_PIN, + // element:number — objectui#8071 slice 7 pinned `filter`, the block's one + // remaining key; fully pinned. // element:record_picker — objectui#8071 slice 3 pinned all four remaining // keys (`dataSource`, `label`, `placeholder`, `sort`); the block is now @@ -2500,11 +2516,11 @@ const MEMBER_PIN_EXEMPTIONS: Record = { 'object-metric.filter': AWAITING_A_PIN, 'object-metric.trend': AWAITING_A_PIN, - // page:accordion - 'page:accordion.items': AWAITING_A_PIN, + // page:accordion — objectui#8071 slice 7 pinned `items`, the block's one + // remaining key; fully pinned. - // page:tabs - 'page:tabs.items': AWAITING_A_PIN, + // page:tabs — objectui#8071 slice 7 pinned `items`, the block's one + // remaining key; fully pinned. // record:activity — objectui#8071 slice 6 pinned `types`, the block's one // remaining key; fully pinned, header kept as a landmark for a future grep. @@ -2740,11 +2756,66 @@ const NEWLY_JUDGED_UNPINNED_MEMBERS = [ * record:activity". That is a contract defect rather than a member shape, so it * is filed (objectui#8934) rather than frozen into the pin. * + * ## 41 -> 37, the seventh slice, and the LAST four one-key blocks closed + * + * objectui#8071's seventh slice applied slice 6's selection rule to what it left + * behind: every block whose remainder was exactly one key. Four qualified — + * `element:button.action`, `element:number.filter`, `page:accordion.items` and + * `page:tabs.items` — so the ceiling follows to 37 in the same commit, and all + * four blocks now carry zero exemptions. + * + * The batch is coherent rather than opportunistic on the same terms slice 6 set: + * one package (`packages/components`) registers all four, the two `element:*` + * keys sharing `renderers/basic/elements.tsx` and the two `page:*` keys sharing + * `renderers/layout/containers.tsx`, and the rule is stateable in a sentence. + * + * ⚠️ TWO one-key blocks were deliberately NOT taken, and neither is a leftover + * this slice could have absorbed: + * + * - `record:related_list.actions` is the `NO_READ_SITE_TO_PIN` sentinel, and + * the reading was RE-MEASURED here rather than inherited: on this tree + * `renderers/record-related-list.tsx` still contains ZERO case-sensitive + * occurrences of `actions` (control: `import` reads 9 in the same file), and + * the only near-matches are `useRelatedRecordActions` / `relatedActions` — + * the host bridge the constant already names. Nothing to pin, still. + * - `object-kanban`'s `columns` / `dataSource` are the + * `NEWLY_JUDGED_UNPINNED_MEMBERS` pair, held by an unruled contract question + * (objectui#8913) about whether `columns` survives as judged structure at + * all. Pinning a shape a pending ruling may delete would be worse than + * leaving it exempt, so `NEWLY_JUDGED_UNPINNED_MEMBERS` is UNCHANGED by this + * slice — no block it names was touched. + * + * ⇒ After this slice every remaining exemption sits on one of four + * MULTI-key blocks (`object-grid` 15, `object-form` 7, + * `object-master-detail-form` 6, `object-metric` 6) plus the two held keys. The + * "close a block outright by taking its last key" shape is now EXHAUSTED, and + * the next slice is the first that has to take a partial block or close a + * multi-key one whole — stated here so the next reader does not spend the + * search rediscovering it. + * + * Three of the four pins are new files; none of the four keys had a + * pre-existing file covering its member SET, and each candidate was read end to + * end before being rejected rather than dismissed on its greps. + * `element-button-action.test.tsx` drives only `type` and `to` on one + * navigation action; `action-bodyExtra-forward.test.tsx` pins one member across + * four renderers; `page-accordion-icon.test.tsx` pins `icon` alone; the five + * `page:tabs` suites pin one member each; and + * `element-number.contractEnvelope-6726.test.tsx` drives the same `find()` + * fallback branch but never authors a `filter`, so the locator itself would + * refuse it. ⚠️ `action-bodyShape-forward.test.tsx` is the one that had to be + * refused rather than merely passed over: it NAMES `element:button` and the key + * `action`, so it would satisfy the locator, while what it actually says is + * that `element:button` is deliberately out of its scope. Crediting it would + * have been a pin that asserts the opposite of its claim — the false-negative + * shape objectui#8068 exists to end. Its boundary is instead pinned as + * behaviour in the new file, which authors `bodyShape` and asserts it is + * dropped. + * * ⇒ The rule for every future slice of objectui#8071: delete the entry, register * the pin, and set this constant to the new count. Not to the new count plus * room. */ -const MEMBER_PIN_EXEMPTION_CEILING = 41; +const MEMBER_PIN_EXEMPTION_CEILING = 37; /** * Every test file a member pin can live in, as LAZY `?raw` loaders. diff --git a/packages/components/src/__tests__/elementButtonActionMembers-8071.test.tsx b/packages/components/src/__tests__/elementButtonActionMembers-8071.test.tsx new file mode 100644 index 0000000000..3d79c32bc6 --- /dev/null +++ b/packages/components/src/__tests__/elementButtonActionMembers-8071.test.tsx @@ -0,0 +1,247 @@ +/** + * 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. + */ + +/** + * `element:button.action` — the MEMBER SHAPE of the inline ActionDef + * (objectui#8071). + * + * The member pin for this key. `ElementButtonRenderer` + * (`renderers/basic/elements.tsx`) does NOT spread the authored `action` object + * into the runner. It forwards an EXPLICIT WHITELIST of members, key by key, + * and the registration publishes the key as `Inline ActionDef executed on click + * (url / navigation / api / script / modal / flow); omitted -> renders inert`. + * The whitelist IS the member shape: a key on it is honoured, a key off it is + * dropped one hop before the runner and the author gets no signal at all. + * + * That drop is the exact defect shape objectstack#6837 (`bodyExtra`) and + * objectstack#6938 (`bodyShape`) were filed for — an authored key that + * validates, publishes, and then evaporates at this seam. + * + * ## Why a new file rather than promoting one of the two that already exist + * + * Both were read end to end before this pin was written: + * + * - `element-button-action.test.tsx` — the closest in SUBJECT (it is about + * this key), but it drives exactly two members, `type` and `to`, on a + * single navigation action, plus the inert path. It would not fail if any + * other member left the whitelist. + * - `action-bodyExtra-forward.test.tsx` — drives `element:button`'s inline + * action for real and asserts `bodyExtra` survives the hop, plus the + * `bodyExtra`-beats-`params` merge order on the wire. Its subject is ONE + * member across four renderers, not this renderer's member set. + * + * A third file, `action-bodyShape-forward.test.tsx`, names `element:button` + * only to say it is deliberately NOT covered there — its forward list mirrors + * spec's `InlineActionSchema` pick list, which does not include `bodyShape`, + * and forwarding it from here would be the forbidden renderer-first direction. + * ⛔ That file must never be credited as this key's pin: it would satisfy the + * locator (it names the block and the key) while asserting the opposite. The + * negative row below is this file taking that reading over as a POSITIVE + * statement instead — `bodyShape` is authored here and its absence downstream + * is asserted, so the boundary is pinned rather than merely described. + * + * ## Limit, recorded rather than papered over + * + * `confirmText` is on the whitelist and is not exercised here. The runner + * gates on it (`if (action.confirmText)`) and awaits a confirmation handler + * before it ever reaches the action handler this file observes, so authoring it + * would suppress the very dispatch being measured. It is pinned by the runner's + * own confirmation suites, not by a member pin. + */ + +import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +import type { ActionContext, ActionDef, ActionResult } from '@object-ui/core'; +import { ActionProvider } from '@object-ui/react'; +import '../renderers/basic/elements'; + +function ElementButton({ schema }: { schema: any }) { + const C = ComponentRegistry.get('element:button'); + if (!C) throw new Error('element:button not registered'); + // eslint-disable-next-line react-hooks/static-components -- ComponentRegistry.get returns a registered component (stable), not one created during render + return ; +} + +const button = (action: unknown, label = 'Go') => ({ + type: 'element:button', + properties: { label, action }, +}); + +describe('element:button action — member shape (objectui#8071)', () => { + // Typed with the signature `ActionProvider`'s `handlers` prop declares, not + // `ReturnType` (objectui#4040). + let api: Mock<(action: ActionDef, ctx: ActionContext) => Promise>; + /** The members the runner was handed, snapshotted AT dispatch. */ + let received: Record; + + beforeEach(() => { + received = {}; + api = vi.fn(async (action: ActionDef) => { + // Snapshot immediately: the runner may mutate the ActionDef it holds + // (it merges collected params back onto `action.params`), so a read + // taken after the await would not be a reading of the FORWARD. + received = { ...(action as unknown as Record) }; + return { success: true }; + }); + }); + + /** The forwarded members that actually carry a value. */ + const definedKeys = () => + Object.entries(received) + .filter(([, v]) => v !== undefined) + .map(([k]) => k) + .sort(); + + async function click(schema: any) { + render( + + + , + ); + fireEvent.click(screen.getByRole('button', { name: /Go/i })); + await waitFor(() => expect(api).toHaveBeenCalledOnce()); + } + + it('forwards the whitelisted members and DROPS every key off the list', async () => { + await click( + button({ + // …every whitelisted member that is inert for an `api` action… + type: 'api', + name: 'close_order', + label: 'Close order', + description: 'Closes the order', + target: '/api/v1/order/close', + endpoint: '/api/v1/order/close', + method: 'PATCH', + bodyExtra: { status: 'closed' }, + successMessage: 'Closed', + errorMessage: 'Failed', + refreshAfter: true, + params: { note: 'from the author' }, + // …and three keys the forward list does not carry. `bodyShape` is the + // measured one (objectstack#6938): spec's InlineActionSchema pick list + // does not include it, so it must not arrive. `icon` and `variant` are + // element:button's OWN sibling props — authored one level in by + // mistake, they must not become action members either. + bodyShape: { wrap: 'data' }, + icon: 'trash', + variant: 'danger', + }), + ); + + // The whitelist as a SET — arrival and drop in one assertion, so a member + // quietly added to or removed from the forward list is red either way. + expect(definedKeys()).toEqual([ + 'bodyExtra', + 'description', + 'endpoint', + 'errorMessage', + 'label', + 'method', + 'name', + 'params', + 'refreshAfter', + 'successMessage', + 'target', + 'type', + ]); + // Named individually as well, because the sorted-list form above reads as + // a blob at review time and these three are the point of the row. + expect(received.bodyShape).toBeUndefined(); + expect(received.icon).toBeUndefined(); + expect(received.variant).toBeUndefined(); + // …and the members that DID arrive kept their values, not just their names. + expect(received.bodyExtra).toEqual({ status: 'closed' }); + expect(received.target).toBe('/api/v1/order/close'); + }); + + it('`actionType` OUTRANKS `type` when both are authored', async () => { + // `type: action.actionType || action.type`. Authored with two different + // action kinds so a reversed read routes the click to the other handler + // instead of tying. + const onNavigate = vi.fn(); + render( + + + , + ); + fireEvent.click(screen.getByRole('button', { name: /Go/i })); + + await waitFor(() => expect(api).toHaveBeenCalledOnce()); + expect(received.type).toBe('api'); + // The control that makes the line above a precedence reading rather than a + // coincidence: the losing spelling's handler never ran. + expect(onNavigate).not.toHaveBeenCalled(); + }); + + it('an ARRAY `params` is re-routed to `actionParams` — the collection DEFINITION', async () => { + // The one member whose NAME changes across the hop. Spec spells the + // parameter-definition array `params`; the runner disambiguates it from the + // static values object as `actionParams` (objectstack#5777 direction A). + const defs = [{ name: 'reason', type: 'string' }]; + await click(button({ type: 'api', target: '/api/v1/order/close', params: defs })); + + expect(received.actionParams).toEqual(defs); + // …and it does NOT also arrive under its authored name, which is what would + // let a downstream consumer read the definition array as a values map. + expect(received.params).toBeUndefined(); + }); + + it('an OBJECT `params` stays `params` — the static values map', async () => { + // The other arm of the same branch, same shape, so the row above is a + // statement about ARRAY-ness and not about `params` generally. + await click( + button({ type: 'api', target: '/api/v1/order/close', params: { reason: 'duplicate' } }), + ); + + expect(received.params).toEqual({ reason: 'duplicate' }); + expect(received.actionParams).toBeUndefined(); + }); + + it('the navigation members reach the navigation handler', async () => { + // `to` and `opensInNewTab` are on the whitelist but inert for an `api` + // action, so they are pinned on the action kind that actually consumes + // them. + const onNavigate = vi.fn(); + render( + + + , + ); + fireEvent.click(screen.getByRole('button', { name: /Go/i })); + + await waitFor(() => + expect(onNavigate).toHaveBeenCalledWith('/apps/crm/contacts', expect.anything()), + ); + }); + + it('an omitted `action` leaves the button inert — no dispatch, no crash', async () => { + // The registration's own words ("omitted -> renders inert"), and the + // non-vacuity control for every row above: the dispatches they observe + // happen because an action was authored, not because clicking always fires. + render( + + + , + ); + fireEvent.click(screen.getByRole('button', { name: /Go/i })); + + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(api).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/components/src/__tests__/pageAccordionItemMembers-8071.test.tsx b/packages/components/src/__tests__/pageAccordionItemMembers-8071.test.tsx new file mode 100644 index 0000000000..88aff1b064 --- /dev/null +++ b/packages/components/src/__tests__/pageAccordionItemMembers-8071.test.tsx @@ -0,0 +1,162 @@ +/** + * 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. + */ + +/** + * `page:accordion.items` — the MEMBER SHAPE of one panel definition + * (objectui#8071). + * + * The member pin for this key: which member of an authored item becomes which + * part of the rendered accordion. `PageAccordionRenderer` + * (`renderers/layout/containers.tsx`) reads exactly four members off each + * element of `items` — `label`, `icon`, `collapsed`, `children` — and the + * registration publishes that set verbatim + * (`[{ label, icon?, collapsed?, children }] — collapsed: false opens a panel + * by default`). Nothing asserted the mapping AS A SET before this file. + * + * What already existed, read end to end before this pin was written rather + * than trusted on its greps: `page-accordion-icon.test.tsx` (objectui#4721) + * pins ONE member, `icon`, and pins it well — per-item forwarding, a positive + * control where only some panels declare one, and the no-icon case. It asserts + * nothing about `label`, `collapsed` or `children`, and would not fail if those + * three swapped roles. So `icon` is covered narrowly below rather than + * re-litigated, and the members this file exists for are the other three. + * + * The `collapsed` member is the one with real semantics to get wrong, and the + * renderer's reading is STRICTLY `=== false`: + * + * const defaultOpen = itemsWithValue.filter((it) => it.collapsed === false) + * + * so `collapsed: false` OPENS a panel, while `collapsed: true` AND an omitted + * `collapsed` both leave it shut. That is the inverse of how the member name + * reads at a glance, which is exactly why it is pinned: an "obvious" edit to + * `!it.collapsed` would open every panel that never mentioned the key. + * + * The single/multiple split is asserted too, because `allowMultiple` changes + * what the SAME `collapsed` members mean: single mode takes `defaultOpen[0]` + * (the FIRST opener wins, later ones are dropped), multiple mode takes them + * all. A pin that only ever rendered one open panel could not tell the two + * readings apart. + * + * Radix unmounts a closed panel's content, so "open" is asserted as the panel + * BODY being in the document — the same observation `page-tabs-*` suites make + * of an inactive tab, and one that cannot be satisfied by a header alone. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import { SchemaRenderer } from '@object-ui/react'; +// Registers the renderers at module scope, NOT inside a `beforeAll` — there the +// cold transform is billed to `hookTimeout`. See +// object-ui/no-dynamic-import-in-test-hook (objectui#3010/#3021). +import '../renderers'; + +vi.mock('../lib/lazy-icon', () => ({ + LazyIcon: ({ name, className }: { name?: string; className?: string }) => ( + + ), +})); + +afterEach(() => cleanup()); + +const textChild = (content: string) => [{ type: 'element:text', properties: { content } }]; + +const renderAccordion = (items: any[], rest: Record = {}) => + render(); + +describe('page:accordion items — member shape (objectui#8071)', () => { + it('`label` becomes the panel trigger and `children` becomes that panel BODY', () => { + // The two structural members, asserted against each other so a renderer + // that painted the wrong one cannot pass: the label text is a trigger, the + // child text is the panel content, and `collapsed: false` is what puts the + // body in the document at all. + const { getByRole, getByText } = renderAccordion([ + { label: 'Details', collapsed: false, children: textChild('DETAILS BODY') }, + { label: 'Notes', children: textChild('NOTES BODY') }, + ]); + + // `label` -> the trigger's accessible name. + expect(getByRole('button', { name: /Details/i })).toBeTruthy(); + expect(getByRole('button', { name: /Notes/i })).toBeTruthy(); + // `children` -> the open panel's body. NOT the label, and not a sibling of it. + expect(getByText('DETAILS BODY')).toBeTruthy(); + }); + + it('`collapsed: false` opens a panel; `collapsed: true` and an OMITTED collapsed both stay shut', () => { + // The `=== false` reading, pinned against the two ways a panel stays + // closed. The omitted-key arm is the half an `!it.collapsed` edit breaks. + const { queryByText } = renderAccordion([ + { label: 'Open me', collapsed: false, children: textChild('OPEN BODY') }, + { label: 'Explicitly shut', collapsed: true, children: textChild('SHUT BODY') }, + { label: 'Says nothing', children: textChild('SILENT BODY') }, + ]); + + expect(queryByText('OPEN BODY')).not.toBeNull(); + expect(queryByText('SHUT BODY')).toBeNull(); + // The control that separates "reads `=== false`" from "reads truthiness": + // an item with no `collapsed` at all must behave like `collapsed: true`. + expect(queryByText('SILENT BODY')).toBeNull(); + }); + + it('every panel stays shut when NO item declares `collapsed: false` (the default)', () => { + // Non-vacuity for the assertion above: without this, a renderer that opened + // nothing ever would satisfy both "shut" rows for the wrong reason. + const { queryByText, getByRole } = renderAccordion([ + { label: 'Alpha', children: textChild('ALPHA BODY') }, + { label: 'Beta', children: textChild('BETA BODY') }, + ]); + + expect(getByRole('button', { name: /Alpha/i })).toBeTruthy(); + expect(queryByText('ALPHA BODY')).toBeNull(); + expect(queryByText('BETA BODY')).toBeNull(); + }); + + it('single mode opens only the FIRST `collapsed: false` panel', () => { + // `defaultValue={defaultOpen[0]}` — later openers are dropped, because a + // Radix `type="single"` accordion holds one value. + const { queryByText } = renderAccordion([ + { label: 'First', collapsed: false, children: textChild('FIRST BODY') }, + { label: 'Second', collapsed: false, children: textChild('SECOND BODY') }, + ]); + + expect(queryByText('FIRST BODY')).not.toBeNull(); + expect(queryByText('SECOND BODY')).toBeNull(); + }); + + it('`allowMultiple` opens EVERY `collapsed: false` panel — same items, different reading', () => { + // The same two items as the row above, so the only variable is + // `allowMultiple`. This is what makes the previous row a statement about + // single mode rather than about `collapsed`. + const { queryByText } = renderAccordion( + [ + { label: 'First', collapsed: false, children: textChild('FIRST BODY') }, + { label: 'Second', collapsed: false, children: textChild('SECOND BODY') }, + { label: 'Third', children: textChild('THIRD BODY') }, + ], + { allowMultiple: true }, + ); + + expect(queryByText('FIRST BODY')).not.toBeNull(); + expect(queryByText('SECOND BODY')).not.toBeNull(); + // …and `allowMultiple` does not open panels that never asked to be open. + expect(queryByText('THIRD BODY')).toBeNull(); + }); + + it('`icon` rides on the same item as its label, per panel', () => { + // Narrow on purpose — `page-accordion-icon.test.tsx` (objectui#4721) is the + // pin for this member and is not duplicated here. This row exists so the + // member SET this file states is complete: four members, four assertions. + const { getAllByTestId } = renderAccordion([ + { label: 'Details', icon: 'user', children: textChild('DETAILS BODY') }, + { label: 'Notes', children: textChild('NOTES BODY') }, + ]); + + const icons = getAllByTestId('accordion-item-icon'); + expect(icons).toHaveLength(1); + expect(icons[0].getAttribute('data-name')).toBe('user'); + }); +}); diff --git a/packages/components/src/__tests__/pageTabsItemMembers-8071.test.tsx b/packages/components/src/__tests__/pageTabsItemMembers-8071.test.tsx new file mode 100644 index 0000000000..91fa3124b8 --- /dev/null +++ b/packages/components/src/__tests__/pageTabsItemMembers-8071.test.tsx @@ -0,0 +1,199 @@ +/** + * 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. + */ + +/** + * `page:tabs.items` — the MEMBER SHAPE of one tab definition (objectui#8071). + * + * The member pin for this key. `PageTabsRenderer` + * (`renderers/layout/containers.tsx`) reads SIX members off each element of + * `items` — `label`, `value`, `icon`, `count`, `visibleWhen`, `children` — and + * the registration publishes that set verbatim (`[{ label, value?, icon?, + * count?, visibleWhen?, children }]`). Each becomes a DIFFERENT part of the + * rendered strip, and this file pins the mapping as a set: label to the trigger + * name, icon to the trigger's leading glyph, count to the trailing badge, value + * to the tab's IDENTITY, children to the panel, `visibleWhen` to the tab's + * existence. + * + * ## Why a new file rather than promoting one of the five that already exist + * + * All five were read end to end before this pin was written. Each is a good pin + * on ONE member and asserts nothing about the mapping as a whole: + * + * - `page-tabs-visibility.test.tsx` — `visibleWhen`, and thoroughly: whole-tab + * removal, live re-evaluation against page variables, the + * `{ dialect, source }` envelope, the active-tab fallback, and the + * deprecated `visibility` alias staying unread. It does exercise `label` + * and `children` as real members, which is why the two are not re-litigated + * in depth here. + * - `page-tabs-count-badge-i18n.test.tsx` — the badge's ACCESSIBLE NAME and + * its plural key selection, not which member feeds it. + * - `pageTabsUrlSync.test.ts` — `value` as the `?tab=` URL token, one layer up + * in `app-shell`, off this renderer entirely. + * - `page-tabs-always-show-strip.test.tsx` — the strip-visibility rule, a + * BLOCK-level key (`alwaysShowStrip`), not an item member. + * - `page-tabs-builtin-label-i18n-4645.test.tsx` — `label` localization of + * well-known English tokens. + * + * So no single existing file would fail if `count` and `value` swapped roles, + * which is the shape a member pin has to catch. + * + * ## The two readings with real semantics to get wrong + * + * `count` is NOT rendered whenever it is present. The renderer gates on + * `!== undefined && !== null && !== '' && Number(count) > 0`, so an authored + * `count: 0` paints NO badge — a zero-count tab reads as a plain tab, not as a + * tab wearing a `0`. And `value` is `typeof value === 'string' && value !== ''` + * or else `tab-INDEX`: an authored value is the stable identity, an absent one + * silently becomes positional. Both arms are pinned, because both are the kind + * of thing a tidy-up edit rewrites without noticing. + * + * `value` is asserted BEHAVIOURALLY, through `defaultTab` — the host key + * app-shell restores `?tab=` with, which is honoured "only when it names an + * actual tab". Selecting a tab by its authored value proves the member IS the + * identity, without reaching into Radix's generated element ids. + * + * Labels here are deliberately NOT well-known English tokens (`Details`, + * `Related`, …): those route through `translateLabel`'s pack lookup, which is + * `page-tabs-builtin-label-i18n-4645.test.tsx`'s subject. Plain names keep this + * file's rows about member ROUTING rather than about i18n. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import { SchemaRenderer } from '@object-ui/react'; +// Registers the renderers at module scope, NOT inside a `beforeAll` — there the +// cold transform is billed to `hookTimeout`. See +// object-ui/no-dynamic-import-in-test-hook (objectui#3010/#3021). +import '../renderers'; + +vi.mock('../lib/lazy-icon', () => ({ + LazyIcon: ({ name, className }: { name?: string; className?: string }) => ( + + ), +})); + +afterEach(() => cleanup()); + +const textChild = (content: string) => [{ type: 'element:text', properties: { content } }]; + +const renderTabs = (items: any[], rest: Record = {}) => + render(); + +describe('page:tabs items — member shape (objectui#8071)', () => { + it('`label` becomes the tab trigger and `children` becomes that tab PANEL', () => { + // The two structural members against each other. Radix renders only the + // ACTIVE panel, so the second tab's body being absent is part of the claim: + // `children` is panel content, not strip content. + const { getByRole, getByText, queryByText } = renderTabs([ + { label: 'Alpha', value: 'alpha', children: textChild('ALPHA BODY') }, + { label: 'Beta', value: 'beta', children: textChild('BETA BODY') }, + ]); + + expect(getByRole('tab', { name: /Alpha/i })).toBeTruthy(); + expect(getByRole('tab', { name: /Beta/i })).toBeTruthy(); + expect(getByText('ALPHA BODY')).toBeTruthy(); + expect(queryByText('BETA BODY')).toBeNull(); + }); + + it('`value` is the tab IDENTITY — `defaultTab` selects a tab by the value its item authored', () => { + // Behavioural pin on the member: the host restores a tab by NAME, so an + // authored `value` has to be what names it. + const { getByText, queryByText } = renderTabs( + [ + { label: 'Alpha', value: 'alpha', children: textChild('ALPHA BODY') }, + { label: 'Beta', value: 'beta', children: textChild('BETA BODY') }, + ], + { defaultTab: 'beta' }, + ); + + // The SECOND tab is active because its item said `value: 'beta'` — not + // because of its position. + expect(getByText('BETA BODY')).toBeTruthy(); + expect(queryByText('ALPHA BODY')).toBeNull(); + }); + + it('an item with NO `value` falls back to its positional `tab-INDEX` identity', () => { + // The other arm of the same read. Same shape as the row above, with the + // `value` members deleted — so a `defaultTab` naming the positional token + // is what selects the second tab. + const { getByText, queryByText } = renderTabs( + [ + { label: 'Alpha', children: textChild('ALPHA BODY') }, + { label: 'Beta', children: textChild('BETA BODY') }, + ], + { defaultTab: 'tab-1' }, + ); + + expect(getByText('BETA BODY')).toBeTruthy(); + expect(queryByText('ALPHA BODY')).toBeNull(); + }); + + it('an EMPTY-STRING `value` is not an identity — it falls back positionally too', () => { + // `typeof value === 'string' && value !== ''`. Without this row a read of + // just `typeof value === 'string'` would pass every assertion above. + const { getByText, queryByText } = renderTabs( + [ + { label: 'Alpha', value: '', children: textChild('ALPHA BODY') }, + { label: 'Beta', value: '', children: textChild('BETA BODY') }, + ], + { defaultTab: 'tab-1' }, + ); + + expect(getByText('BETA BODY')).toBeTruthy(); + expect(queryByText('ALPHA BODY')).toBeNull(); + }); + + it('`count` becomes the trailing badge, and `icon` the leading glyph, on their OWN item', () => { + // Per-item routing for the two decorative members, with a neighbour that + // declares neither — so neither can be strip-wide. + const { getAllByTestId, getByRole } = renderTabs([ + { label: 'Alpha', value: 'alpha', icon: 'user', count: 7, children: textChild('ALPHA BODY') }, + { label: 'Beta', value: 'beta', children: textChild('BETA BODY') }, + ]); + + const icons = getAllByTestId('tab-item-icon'); + expect(icons).toHaveLength(1); + expect(icons[0].getAttribute('data-name')).toBe('user'); + + // The badge digits live inside the trigger that owns the count… + expect(getByRole('tab', { name: /Alpha/i }).textContent).toContain('7'); + // …and not inside its neighbour. + expect(getByRole('tab', { name: /Beta/i }).textContent).not.toContain('7'); + }); + + it('`count: 0` paints NO badge — presence is not the gate, a positive value is', () => { + // `Number(item.count) > 0`. A zero-count tab is a plain tab; the badge is an + // affordance for "there is something in here", not a readout of the member. + const { getByRole } = renderTabs([ + { label: 'Alpha', value: 'alpha', count: 0, children: textChild('ALPHA BODY') }, + { label: 'Beta', value: 'beta', count: 4, children: textChild('BETA BODY') }, + ]); + + expect(getByRole('tab', { name: /Alpha/i }).textContent).not.toContain('0'); + // The positive control on the same strip, same run: the gate really does + // let a real count through, so the row above is a reading and not a + // renderer that never badges anything. + expect(getByRole('tab', { name: /Beta/i }).textContent).toContain('4'); + }); + + it('`visibleWhen` removes the whole tab — trigger AND panel', () => { + // Narrow on purpose: `page-tabs-visibility.test.tsx` is the pin for this + // member's depth (live re-evaluation, the Expression envelope, the + // active-tab fallback, the unread `visibility` alias). This row is here so + // the member SET this file states is complete — six members, six routings. + const { queryByRole, queryByText, getByRole } = renderTabs([ + { label: 'Alpha', value: 'alpha', children: textChild('ALPHA BODY') }, + { label: 'Beta', value: 'beta', children: textChild('BETA BODY') }, + { label: 'Gamma', value: 'gamma', visibleWhen: '1 == 2', children: textChild('GAMMA BODY') }, + ]); + + expect(getByRole('tab', { name: /Alpha/i })).toBeTruthy(); + expect(queryByRole('tab', { name: /Gamma/i })).toBeNull(); + expect(queryByText('GAMMA BODY')).toBeNull(); + }); +}); diff --git a/packages/components/src/renderers/basic/__tests__/elementNumberFilterMembers-8071.test.tsx b/packages/components/src/renderers/basic/__tests__/elementNumberFilterMembers-8071.test.tsx new file mode 100644 index 0000000000..f338a07833 --- /dev/null +++ b/packages/components/src/renderers/basic/__tests__/elementNumberFilterMembers-8071.test.tsx @@ -0,0 +1,167 @@ +/** + * 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. + */ + +/** + * `element:number.filter` — the MEMBER SHAPE this metric reads (objectui#8071). + * + * The member pin for this key. Unlike the other keys this card has pinned, + * `filter` carries no NAMED member set of its own: it is a query predicate + * `ElementNumberRenderer` (`renderers/basic/elements.tsx`) never inspects. So + * the member shape here is the SPELLING it arrives under downstream, and the + * renderer uses TWO different ones on its two paths: + * + * - `adapter.aggregate(object, { field, function, groupBy: '_all', filter })` + * — flat, under its own name, beside the three keys that make the call an + * aggregate at all. + * - `adapter.find(object, { $filter: filter })` — the fallback when the + * adapter carries no `aggregate()`, WRAPPED under `$filter`. + * + * One authored key, two wire spellings, chosen by a capability check the author + * cannot see. That is precisely the kind of thing that gets "tidied" into one + * spelling, and either direction silently drops the predicate: a metric that + * should read "open opportunities" then counts EVERY row and paints a + * confidently wrong number. No diagnostic, no empty state — the failure mode + * this whole direction (objectui#8068) exists to make loud. + * + * The third member semantic is the re-query rule. The effect keys on + * `JSON.stringify(props.filter)`, not on the object identity: + * + * const filterKey = React.useMemo( + * () => (props.filter ? JSON.stringify(props.filter) : ''), [props.filter]); + * + * so the predicate is read BY VALUE. A parent that rebuilds an equal filter + * literal every render must NOT re-probe the server, and a parent that changes + * one comparand MUST. Both arms are asserted, because a dependency array + * "simplified" to `props.filter` passes the second and turns the first into a + * per-render fetch storm. + * + * ## Nothing pre-existing covered this + * + * Measured before writing: exactly two test files name `element:number` and + * touch this area, and neither can be credited. + * `element-number.contractEnvelope-6726.test.tsx` (read end to end) drives the + * SAME `find()` fallback branch but authors no `filter` at all — the word does + * not occur in it, so the locator itself would refuse it ("pin file never names + * the key"). `data-objectstack`'s `aggregate-filter-lowering.test.ts` mentions + * `element:number` once, in a comment, and tests the adapter's own lowering + * below this seam rather than what the renderer hands it. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, waitFor, cleanup } from '@testing-library/react'; +import { AdapterCtx, SchemaRenderer } from '@object-ui/react'; +// Registers every `element:*` renderer at module scope, not in a hook +// (object-ui/no-dynamic-import-in-test-hook, objectui#3010). +import '../../../renderers'; + +afterEach(cleanup); + +const OPEN_ONLY = { stage: { $ne: 'closed' } }; + +const metric = (filter?: unknown) => ({ + type: 'element:number', + id: 'metric', + // Element config lives in the `properties` bag (`readProps`), not on the + // node — the same door an authored page writes through. + properties: { object: 'opportunity', field: 'amount', aggregate: 'sum', ...(filter ? { filter } : {}) }, +}); + +/** An adapter that CAN aggregate — the primary path. */ +const aggregating = () => ({ + aggregate: vi.fn(async () => [{ amount_sum: 42 }]), + find: vi.fn(async () => ({ data: [] })), +}); + +/** An adapter that cannot — `find()` is the only way to a number. */ +const findingOnly = () => ({ find: vi.fn(async () => ({ data: [{ id: 'r1' }] })) }); + +function mount(adapter: unknown, filter?: unknown) { + return render( + + + , + ); +} + +describe('element:number filter — member shape (objectui#8071)', () => { + it('reaches `aggregate()` FLAT, under its own name, beside the aggregate members', async () => { + const adapter = aggregating(); + mount(adapter, OPEN_ONLY); + + await waitFor(() => expect(adapter.aggregate).toHaveBeenCalledOnce()); + // The whole options bag, not just the filter — `groupBy: '_all'` and + // `function` are what make this an aggregate rather than a query, and a + // filter that arrived without them would be counting something else. + expect(adapter.aggregate).toHaveBeenCalledWith('opportunity', { + field: 'amount', + function: 'sum', + groupBy: '_all', + filter: OPEN_ONLY, + }); + // Verbatim: the renderer does not normalise, wrap or re-key the predicate + // on this path. + expect((adapter.aggregate as any).mock.calls[0][1].filter).toEqual(OPEN_ONLY); + }); + + it('reaches the `find()` fallback WRAPPED as `$filter` — the other spelling', async () => { + // Same authored key, same value, different adapter capability. This is the + // row that makes "two spellings" a reading rather than an assumption. + const adapter = findingOnly(); + mount(adapter, OPEN_ONLY); + + await waitFor(() => expect(adapter.find).toHaveBeenCalledOnce()); + expect(adapter.find).toHaveBeenCalledWith('opportunity', { $filter: OPEN_ONLY }); + }); + + it('sends NO options at all to `find()` when no filter is authored', async () => { + // `props.filter ? { $filter: props.filter } : undefined`. The control for + // the row above: an unfiltered metric must not ship an empty envelope, + // which some adapters read as "match nothing". + const adapter = findingOnly(); + mount(adapter); + + await waitFor(() => expect(adapter.find).toHaveBeenCalledOnce()); + expect(adapter.find).toHaveBeenCalledWith('opportunity', undefined); + }); + + it('is read BY VALUE — an equal filter rebuilt with a new identity does NOT re-probe', async () => { + // The re-query rule. A parent re-rendering with a fresh object literal is + // the normal case, not an exotic one, and a dependency on the reference + // would turn every such render into a server round trip. + const adapter = aggregating(); + const view = mount(adapter, { stage: { $ne: 'closed' } }); + await waitFor(() => expect(adapter.aggregate).toHaveBeenCalledOnce()); + + view.rerender( + + {/* Deep-equal, freshly allocated — a different object, the same predicate. */} + + , + ); + + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(adapter.aggregate).toHaveBeenCalledOnce(); + }); + + it('…and a filter whose VALUE changes does re-probe, with the new predicate', async () => { + // The other arm, and the non-vacuity control for the row above: without it + // a renderer that never re-probed at all would pass that assertion. + const adapter = aggregating(); + const view = mount(adapter, { stage: { $ne: 'closed' } }); + await waitFor(() => expect(adapter.aggregate).toHaveBeenCalledOnce()); + + view.rerender( + + + , + ); + + await waitFor(() => expect(adapter.aggregate).toHaveBeenCalledTimes(2)); + expect((adapter.aggregate as any).mock.calls[1][1].filter).toEqual({ stage: { $ne: 'won' } }); + }); +});