diff --git a/.changeset/8695-unresolved-lookup-reference-one-answer.md b/.changeset/8695-unresolved-lookup-reference-one-answer.md new file mode 100644 index 0000000000..13ef1ee617 --- /dev/null +++ b/.changeset/8695-unresolved-lookup-reference-one-answer.md @@ -0,0 +1,62 @@ +--- +'@object-ui/fields': minor +'@object-ui/i18n': minor +--- + +A `lookup` reference that resolved to nothing now gets **one** answer instead of +two opposite ones (objectui#8695), carrying objectui#8434's ruling to the second +renderer that had the same defect. + +`LookupCellRenderer` split a single epistemic state — *a reference this screen +did not resolve* — by the **shape of the string**. `isLikelyOpaqueId` sent +opaque-looking ids to a muted `—` and sent everything else to confident bare +text. Measured with `reference_to: 'sys_user'`, `'Ada Lovelace'` rendered +`Ada Lovelace` +— **byte-identical** to what a `text` cell prints for the same string — while +`'01HQZX9K2M4N6P8R'` rendered a muted `—`. So the name-shaped case stated a +confident fact the screen did not have (a dirty row read exactly like a clean +one), and the opaque case destroyed the raw id, which objectui#8434's triage +named as "the only clue for diagnosing existing dirty rows". Two opposite +failures, one state. + +Both now render the affordance objectui#8434 settled on: the raw value kept +**visible**, beside a muted marker glyph and a stated sentence, keyed as +`detail.unresolvedLookupReference` in all ten locale packs. `master_detail` and +`tree` route through the same renderer and get the same answer, as does the +multi-value chip shape — including the `+N` overflow chip's `title`, which now +lists the values it hides instead of a row of dashes. + +**The sentence is epistemic, not ontological, and it is a sibling of the `user` +one rather than the same key.** At least six causes reach this arm and the +renderer distinguishes none of them: never fetched, in flight, the resolver +threw, the resolver answered with no record, it answered with a record no +display field could name, and — for array entries after the first — never asked. +`useLookupName` returns `string | undefined`, dropping the `pending`/`err`/`ok` +discriminator its own cache holds. Several of those also cover a record the +viewer may simply not be allowed to read, and "cannot read" versus "does not +exist" is a boundary this renderer cannot see across. So it states only what is +true of all of them: this screen did not resolve it. The `user` pack value ends +"was not resolved to a user", which is false on a lookup pointing at any other +object, hence a separate key. + +**Why `minor` and not `patch`.** The test this lane applies is whether EXISTING +STORED DATA renders differently, and here some of it is not broken data. +`LookupCellRenderer` auto-resolves only the FIRST primitive of an array +(`primaryPrimitiveId`, and `resolveLabel` returns a name only when +`val === primaryPrimitiveId`) — a documented cheapness policy, not a defect. So +on a multi-value `lookup` holding perfectly clean ids, entries 2..n were never +asked and now wear the unresolved marker where they previously printed a bare +id or a dash. That is a visible change to a working screen over correct data, +which is the line between the two levels; the name-shaped and opaque-shaped +single-value cases would each have been `patch` on their own, since both were +already wrong. `@object-ui/i18n` is `minor` for the ordinary reason: a key is +ADDED — `detail.unresolvedLookupReference`, across all ten packs (the `user` +sentence `detail.unresolvedReference` already shipped with objectui#8434). + +**Nothing else moves.** A reference resolved by an expanded record, by the +author's `options`, or by the fetch-on-demand resolver renders exactly as before, +unmarked; an empty cell keeps `EmptyValue`; the affordance sits inside +`ReferencedRecordLink`, so an unresolved reference is still navigable +(objectui#4336). Display only — no query, sort, export or save path reads a cell +renderer's output. `isLikelyOpaqueId` stays exported (removing it would be a +breaking change) but no longer decides how anything is drawn. diff --git a/packages/fields/src/__tests__/cell-truncation.test.tsx b/packages/fields/src/__tests__/cell-truncation.test.tsx index f420487cf3..4e5b0603d0 100644 --- a/packages/fields/src/__tests__/cell-truncation.test.tsx +++ b/packages/fields/src/__tests__/cell-truncation.test.tsx @@ -54,14 +54,34 @@ describe('cell renderers truncate for real and expose the full text (issue #3466 expectTruncating(screen.getByText(LONG), LONG); }); - it('LookupCellRenderer: primitive non-opaque value', () => { + // ⚠️ UPDATED by objectui#8695, and the update is the finding, not a + // formality: this case used to assert `expectTruncating(...)` on a + // `LookupCellRenderer` primitive, which means #3466 pinned the CONFIDENT + // bare-text rendering of a reference nothing had resolved — the reading was + // deliberate once. objectui#8695 ruled that reading out (it is byte-identical + // to a `text` cell, so the screen states a fact it does not have), so the arm + // now draws the unresolved-reference affordance. #3466's contract is what + // survives and is what is asserted here: a single-line value must not expand + // its column, and its full text must stay reachable. The shape that meets it + // moved — `truncate` sits on the text span inside an `inline-flex` wrapper + // (`overflow: hidden` gives a flex item an automatic minimum size of zero, + // so it shrinks rather than pushing the row wider), and the full value is + // exposed through the wrapper's stated `title` rather than a bare one. + it('LookupCellRenderer: primitive value nothing resolved — objectui#8695 affordance, still truncating', () => { const ds = { find: vi.fn(), findOne: vi.fn() } as any; - render( + const { container } = render( , ); - expectTruncating(screen.getByText(LONG), LONG); + + const mark = container.querySelector('[data-slot="unresolved-reference"]')!; + expect(mark, 'the unresolved arm states itself').not.toBeNull(); + expect(mark).toHaveClass('inline-flex', 'min-w-0', 'max-w-full'); + // The full text stays reachable, inside the sentence that names it. + expect(mark.getAttribute('title')).toContain(LONG); + // …and the value itself still ellipsises instead of growing the column. + expect(screen.getByText(LONG)).toHaveClass('truncate'); }); it('UserCellRenderer: display name beside the avatar', () => { diff --git a/packages/fields/src/__tests__/lookupCell.unresolvedReference-8695.test.tsx b/packages/fields/src/__tests__/lookupCell.unresolvedReference-8695.test.tsx new file mode 100644 index 0000000000..4d0046f0b7 --- /dev/null +++ b/packages/fields/src/__tests__/lookupCell.unresolvedReference-8695.test.tsx @@ -0,0 +1,328 @@ +/** + * 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#8695 — `LookupCellRenderer` answered ONE epistemic state two + * opposite ways, and which one you got was decided by the SHAPE of the string. + * + * ── Re-measured on this base, before anything was changed ───────────────── + * The card's readings were taken at `da5e4f69e`; `packages/fields/src/index.tsx` + * has since been released by objectui#9068. Both readings reproduce verbatim. + * `getCellRenderer('lookup')`, `reference_to: 'sys_user'`, no data source, in + * ONE render: + * + * 'Ada Lovelace' → Ada Lovelace + * 'wo_1' → wo_1 + * '01HQZX9K2M4N6P8R' → — + * + * The first two are BYTE-IDENTICAL to what a `text` cell prints for the same + * string — the screen states a confident fact it does not have, so a dirty row + * reads exactly like a clean one. The third destroys the raw id, which + * objectui#8434's triage named as "the only clue for diagnosing existing dirty + * rows". Two opposite failures, one state, chosen by `isLikelyOpaqueId`. + * + * ── ⭐ Why the pins below assert AGREEMENT ──────────────────────────────── + * A per-shape assertion ("the opaque one now shows its id") would go green on + * a repair that made both shapes wrong in a NEW matching way. The disagreement + * IS the finding, so the load-bearing assertion is that the three shapes get + * the SAME treatment — their markup normalises to one string — with a floor + * underneath it (the affordance is present, the raw value survives, and the + * sentence is epistemic) so that "identically wrong" cannot pass either. + * + * ── ⭐ How many states hide behind "unresolved" — measured, not assumed ──── + * Six causes reach this arm and the renderer distinguishes NONE of them: + * never fetched (no dataSource / no `reference_to`), IN FLIGHT, the resolver + * threw, the resolver answered with no record, it answered with a record no + * display field could name, and — for array entries after the first — never + * asked at all (`primaryPrimitiveId`). `useLookupName` returns + * `string | undefined`, dropping the `pending`/`err`/`ok` discriminator its own + * cache holds. So the card's premise that this renderer "can genuinely + * distinguish 'fetched and absent' from 'never fetched'" is false as the code + * stands, and even surfacing the discriminator could not upgrade the sentence: + * a throw and an empty answer also cover a record the VIEWER may not read, and + * "cannot read" vs "does not exist" is an existence-oracle boundary this lane + * does not cross (objectui#8631). ⇒ the sentence must stay EPISTEMIC, exactly + * as objectui#8434 ruled, and `THE SENTENCE` below pins that. + * + * ── ⛔ What this file must not let pass ─────────────────────────────────── + * The controls are the other half: a genuinely RESOLVED reference must render + * exactly as it does today, unmarked (three ways of resolving: an expanded + * record, the author's `options`, and the fetch-on-demand resolver), and an + * empty cell must keep `EmptyValue`. Without them a reviewer cannot tell this + * repair from "every lookup now looks broken". + */ + +import React from 'react'; +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, cleanup, screen, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { en } from '@object-ui/i18n'; +import { SchemaRendererProvider, RelatedRecordActionsProvider } from '@object-ui/react'; +import { getCellRenderer, resolveCellRendererType } from '../index'; + +afterEach(() => cleanup()); + +/** Resolve + render exactly the way a consumer builds a read-mode cell. */ +function renderCell(type: string, value: unknown, field: Record = {}) { + const Renderer = getCellRenderer(resolveCellRendererType({ type }) || type); + return render( + , + ); +} + +/** The unresolved-reference affordance, counted at its own seam. */ +const marks = (root: HTMLElement) => + root.querySelectorAll('[data-slot="unresolved-reference"]'); + +const textOf = (root: HTMLElement) => (root.textContent ?? '').replace(/\s+/g, ' ').trim(); + +/** + * The three primitive shapes that reach the unresolved arm. All three are the + * SAME state — a reference this screen did not resolve — and the old code gave + * the first two one answer and the third the opposite one. + */ +const SHAPES: ReadonlyArray = [ + ['name-shaped (a person written into a reference column)', 'Ada Lovelace'], + ['a short readable id — the commonest stored form', 'wo_1'], + ['opaque, the shape `isLikelyOpaqueId` used to mute away', '01HQZX9K2M4N6P8R'], +]; + +const REF = { reference_to: 'sys_user' }; + +/** Every family that routes through `LookupCellRenderer` — a bigger surface than `user`. */ +const FAMILIES = ['lookup', 'master_detail', 'tree'] as const; + +describe('objectui#8695 — AGREEMENT: one epistemic state gets ONE answer', () => { + it('all three shapes render the same treatment, in ONE render', () => { + const Renderer = getCellRenderer('lookup'); + const { container } = render( +
+ {SHAPES.map(([, value]) => ( +
+ +
+ ))} +
, + ); + + // Normalise each cell by its OWN raw value: what remains is the treatment. + // Equality across the three is the assertion the card's title asks for. + const normalised = SHAPES.map(([, value]) => { + const cell = container.querySelector(`[data-shape="${value}"]`)!; + return cell.innerHTML.split(value).join('«RAW»'); + }); + + expect( + new Set(normalised).size, + `the shapes still disagree — one state, ${new Set(normalised).size} answers:\n${[...new Set(normalised)].join('\n')}`, + ).toBe(1); + + // …and the floor, so "identically wrong" cannot pass the equality above. + expect(normalised[0], 'the shared answer must BE the stated affordance').toContain( + 'data-slot="unresolved-reference"', + ); + expect(normalised[0], 'and it must be where the raw value is').toContain('«RAW»'); + }); + + it.each(SHAPES)('%s — states unresolved and KEEPS its raw value', (label, value) => { + const { container } = renderCell('lookup', value, REF); + + expect(marks(container), `${label}: exactly one affordance`).toHaveLength(1); + // ⛔ The evidence-destruction half of the defect: the muted `—` replaced + // the id outright, so the only readable trace of a dirty row was gone. + expect( + textOf(container), + `${label}: the raw value is the only clue for diagnosing a dirty row`, + ).toBe(value); + + const stated = marks(container)[0]!.getAttribute('title') ?? ''; + expect(stated.length, `${label}: the affordance must state something`).toBeGreaterThan(0); + expect(stated, `${label}: the sentence must name the value it is about`).toContain(value); + }); + + it.each(FAMILIES)('`%s` routes to the same answer — the surface is all three', (family) => { + const seen = SHAPES.map(([, value]) => { + const { container } = renderCell(family, value, REF); + const html = container.innerHTML.split(value).join('«RAW»'); + cleanup(); + return html; + }); + expect(new Set(seen).size, `${family}: the shapes disagree on this family`).toBe(1); + expect(seen[0], `${family} must state the affordance too`).toContain( + 'data-slot="unresolved-reference"', + ); + }); +}); + +describe('objectui#8695 — ADDITIVE, NOT SUBTRACTIVE: the sentence that graded this card', () => { + // The name-shaped case is the dangerous one: on the unfixed tree the lookup + // cell and the `text` cell produced the SAME string of DOM, so nothing on + // screen separated "we resolved this reference" from "we resolved nothing". + it.each(SHAPES)('%s no longer renders identically to a `text` cell', (label, value) => { + const asLookup = renderCell('lookup', value, REF).container.innerHTML; + cleanup(); + const asText = renderCell('text', value).container.innerHTML; + + expect(asText, 'control: a text cell prints the bare string, unchanged by this card').toBe( + `${value}`, + ); + expect( + asLookup, + `${label}: a lookup cell that is byte-identical to a text cell states nothing`, + ).not.toBe(asText); + }); + + it('the multi-value shape is not left silent while the scalar one speaks', () => { + // One unresolved entry of each shape and one RESOLVED entry, in one cell: + // the resolved chip is this render's own positive control. + const { container } = renderCell( + 'lookup', + ['Ada Lovelace', '01HQZX9K2M4N6P8R', { id: 'a_1', name: 'Globex' }], + { reference_to: 'account' }, + ); + + expect(marks(container), 'both unresolved chips say so').toHaveLength(2); + expect( + [...marks(container)].map((m) => m.textContent), + 'and both keep their raw value inside the chip', + ).toEqual(['Ada Lovelace', '01HQZX9K2M4N6P8R']); + expect(textOf(container), 'the resolved chip is untouched').toContain('Globex'); + }); + + it('the overflow chip lists the values it hides, not a row of dashes', () => { + const { container } = renderCell( + 'lookup', + ['a', 'b', 'c', 'Ada Lovelace', '01HQZX9K2M4N6P8R'], + { reference_to: 'account' }, + ); + const overflow = screen.getByText('+2'); + expect( + overflow.getAttribute('title'), + 'the hidden references stay reachable — `—, —` named nothing', + ).toBe('Ada Lovelace, 01HQZX9K2M4N6P8R'); + expect(container).toBeTruthy(); + }); +}); + +describe('objectui#8695 — THE SENTENCE: epistemic, and about a RECORD', () => { + it.each(SHAPES)('%s — says what it knows, never that the record is absent', (label, value) => { + const { container } = renderCell('lookup', value, REF); + const stated = marks(container)[0]!.getAttribute('title') ?? ''; + + // Six causes reach this arm — including a record the viewer may simply not + // be allowed to read. An ontological claim would be false for most of them. + expect( + stated, + `${label}: this cell cannot know the record is absent — only that it did not resolve it`, + ).not.toMatch(/not found|does not exist|no such|invalid|missing/i); + expect(stated, `${label}: it must say what it DOES know`).toMatch(/unresolved/i); + }); + + // ⭐ The one place objectui#8434's remedy did NOT transplant as-is. Its pack + // value ends "was not resolved to a user", which is false on a `lookup` + // pointing at any other object — and two existing tests pin that wording + // byte-for-byte, so it could not be widened in place either. + it('the lookup sentence is a SIBLING key, and never claims the target is a user', () => { + const { container } = renderCell('lookup', 'Ada Lovelace', { reference_to: 'mtc_work_order' }); + const stated = marks(container)[0]!.getAttribute('title') ?? ''; + + expect(stated, 'a work order is not a user').not.toMatch(/\buser\b/i); + expect( + (en as any).detail.unresolvedLookupReference, + 'the two sentences must stay separate keys', + ).not.toBe((en as any).detail.unresolvedReference); + }); + + it('the provider-less sentence is byte-equal to the `en` pack value', () => { + // The English fallback lives in code (the provider-less path) and the pack + // serves the same sentence; `check:i18n-keys` cannot compare this shape, + // so this pin is the comparison instead. + const { container } = renderCell('lookup', 'Ada Lovelace', REF); + const stated = marks(container)[0]!.getAttribute('title'); + const packed = (en as any).detail.unresolvedLookupReference.replace('{{value}}', 'Ada Lovelace'); + expect(stated, 'the code fallback and the en pack must not drift apart').toBe(packed); + }); +}); + +describe('objectui#8695 — POSITIVE CONTROLS: a resolved reference is untouched', () => { + // Three ways a lookup resolves. All three must render exactly as they do + // today, unmarked — otherwise this repair is indistinguishable from + // "every lookup now looks broken". + it('an EXPANDED record renders its name, unmarked, exactly as before', () => { + const { container } = renderCell('lookup', { id: 'a_1', name: 'Globex' }, { reference_to: 'account' }); + expect(marks(container), 'a resolved reference is not unresolved').toHaveLength(0); + expect(container.innerHTML).toBe( + 'Globex', + ); + }); + + it("the author's `options` resolve the id, unmarked, exactly as before", () => { + const { container } = renderCell('lookup', '01HQZX9K2M4N6P8R', { + ...REF, + options: [{ value: '01HQZX9K2M4N6P8R', label: 'Globex' }], + }); + expect(marks(container), 'an authored option names the reference').toHaveLength(0); + expect(container.innerHTML).toBe( + 'Globex', + ); + }); + + it('the fetch-on-demand resolver names it, and the affordance goes away', async () => { + const findOne = vi.fn(async (_o: string, id: string) => ({ id, name: 'Globex' })); + const Renderer = getCellRenderer('lookup'); + const { container } = render( + + + , + ); + + // ⚠️ The first paint is one of the six causes: IN FLIGHT. The sentence is + // true of it too — this screen has not resolved it — which is why no + // separate "loading" answer is invented here. It is also not new: the same + // paint drew the muted `—` before this card. + expect(marks(container), 'the pending paint states the same true thing').toHaveLength(1); + await waitFor(() => expect(screen.getByText('Globex')).toBeInTheDocument()); + expect(marks(container), 'once resolved, nothing is marked').toHaveLength(0); + }); + + it('an EMPTY cell keeps `EmptyValue` — a different state, a different answer', () => { + const { container } = renderCell('lookup', null, REF); + expect(marks(container), 'nothing to resolve is not a failure to resolve').toHaveLength(0); + expect(container.querySelector('[data-slot="empty-value"]')).not.toBeNull(); + expect(textOf(container)).toBe('—'); + }); + + it('the affordance sits INSIDE the record link, which still addresses the record', () => { + // The raw value IS the foreign key, so an unresolved reference is still + // worth being able to open (objectui#4336). The marker must not cost that. + const Renderer = getCellRenderer('lookup'); + const host = { + resolve: () => ({}), + recordHref: (objectName: string, recordId: string | number) => `/o/${objectName}/r/${recordId}`, + openRecord: vi.fn(), + } as any; + const { container } = render( + + + , + ); + const anchor = container.querySelector('a'); + expect(anchor, 'the reference is still navigable').not.toBeNull(); + expect(anchor).toHaveAttribute('href', '/o/account/r/01HQZX9K2M4N6P8R'); + expect(marks(container), 'and it states that it did not resolve').toHaveLength(1); + }); +}); diff --git a/packages/fields/src/index.tsx b/packages/fields/src/index.tsx index 2ead9e6ce1..aa93359cbf 100644 --- a/packages/fields/src/index.tsx +++ b/packages/fields/src/index.tsx @@ -180,8 +180,18 @@ function resolveLookupRecordName( /** * Heuristic: detect strings that look like opaque foreign-key IDs (e.g. nanoid - * or BSON ObjectId). Used so we don't display random gibberish to users when - * a lookup wasn't expanded. + * or BSON ObjectId). + * + * ⛔ Nothing in this repo calls it any more, and re-introducing a caller that + * decides PRESENTATION from it would re-open objectui#8695. It used to gate + * `LookupCellRenderer`'s muted `—`, i.e. it decided how an unresolved + * reference was drawn from the SHAPE of the string rather than from whether + * anything resolved — so `'Ada Lovelace'` printed as a confident name and an + * opaque id lost its value, two opposite answers to one epistemic state. The + * export is kept because it is part of this package's published surface and + * retiring it is a breaking change no display card is entitled to make; the + * shape question itself is legitimate (a picker filter, an id-vs-name guess), + * it is only an answer to "what did this screen resolve?" that it can never be. */ export function isLikelyOpaqueId(v: unknown): boolean { if (typeof v !== 'string') return false; @@ -2215,7 +2225,10 @@ const MAX_LOOKUP_CELL_CHIPS = 3; * 2. Static `field.options[]` (e.g. when the lookup is a closed enum) → look up label * 3. Fetch-on-demand: when the value is a primitive ID and `field.reference_to` * is known, resolve via dataSource and show the related record's display name. - * Falls back to a muted placeholder while pending and on failure. + * 4. Nothing named it → the unresolved-reference affordance (objectui#8695): + * the raw value, kept visible, beside a stated epistemic marker. This arm + * used to be two — a muted `—` for opaque-LOOKING strings and confident + * bare text for everything else — which answered one state two ways. * * Record → name resolution (1 and 3) goes through the referenced object's * schema when the data source exposes it (`displayField` → nameField/titleFormat @@ -2321,35 +2334,55 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R (field as { options?: Array<{ value: unknown; label: string }> }).options || []; // Resolve a primitive ID to a label. Order: - // options → server-resolved name (via useLookupName) → muted placeholder for opaque IDs → raw value - const resolveLabel = (val: unknown): { text: string; muted: boolean } => { + // options → server-resolved name (via useLookupName) → UNRESOLVED + // + // ⭐ That last arm used to be TWO, and the split between them was the + // defect (objectui#8695). `isLikelyOpaqueId(val)` sent opaque-LOOKING + // strings to a muted `—` and sent everything else to confident bare text — + // so ONE epistemic state, a reference this screen did not resolve, got two + // OPPOSITE answers, chosen by the SHAPE of the string rather than by + // whether anything resolved. Re-measured on this base with + // `reference_to: 'sys_user'`: + // + // 'Ada Lovelace' → Ada Lovelace + // '01HQZX9K2M4N6P8R' → — + // + // The first is BYTE-IDENTICAL to what a `text` cell prints for the same + // string: the screen states a confident fact it does not have, and a dirty + // row reads exactly like a clean one. The second destroys the raw id, which + // objectui#8434's triage named as "the only clue for diagnosing existing + // dirty rows". Opposite failures, one state. + // + // Both are now the SAME answer — the one objectui#8434 settled for `user`: + // additive (a stated marker, never the absence of one), epistemic (this + // screen did not resolve it, never "not found"), raw value kept visible. + // See `UnresolvedLookupReference` for why this renderer is entitled to say + // nothing stronger. + const resolveLabel = (val: unknown): { text: string; unresolved: boolean } => { if (options.length > 0) { const found = options.find((opt) => String(opt.value) === String(val)); - if (found) return { text: found.label, muted: false }; + if (found) return { text: found.label, unresolved: false }; } if (val === primaryPrimitiveId && resolvedName) { - return { text: resolvedName, muted: false }; + return { text: resolvedName, unresolved: false }; } - if (isLikelyOpaqueId(val)) { - // Don't dump a random-looking ID at the user. Show a soft placeholder - // that conveys "this is a reference, name unavailable". - return { text: '—', muted: true }; - } - return { text: String(val), muted: false }; + return { text: String(val), unresolved: true }; }; if (Array.isArray(value)) { - const itemDisplay = (item: unknown): { label: string; muted: boolean } => { + const itemDisplay = (item: unknown): { label: string; unresolved: boolean } => { if (item != null && typeof item === 'object') { return { label: resolveLookupRecordName(item as Record, refSchema, displayField) || String((item as any).id || (item as any)._id || '[Object]'), - muted: false, + unresolved: false, }; } const r = resolveLabel(item); - return { label: r.text, muted: r.muted }; + return { label: r.text, unresolved: r.unresolved }; }; // Cap the chips the same way UserCellRenderer caps its avatars: a @@ -2363,7 +2396,7 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R return (
{visible.map((item, idx) => { - const { label, muted } = itemDisplay(item); + const { label, unresolved } = itemDisplay(item); // Each chip is one referenced record, so each links on its own — // there is no single destination a multi-value cell could point at. return ( @@ -2376,12 +2409,18 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R - {label} + {/* The multi-value shape gets the same ruling as the scalar + one, one input-shape over (objectui#8695): a chip must not + be honest about an unresolved reference on one shape and + silent about it on the other. The chip's muted background + is unchanged — what changes is that the raw value survives + inside it instead of being replaced by `—`. */} + {unresolved ? : label} ); @@ -2409,14 +2448,19 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R ); } - // Primitive value (e.g. raw ID): try options → resolver → opaque-ID placeholder → raw - // The value IS the foreign key, so it addresses the record even when the - // display name could not be resolved (the muted placeholder) — a reference - // that is present but unnamed is still worth being able to open. - const { text, muted } = resolveLabel(value); + // Primitive value (e.g. raw ID): try options → resolver → UNRESOLVED. + // The value IS the foreign key, so it addresses the record even when no + // display name could be resolved — a reference that is present but unnamed + // is still worth being able to open, which is why the affordance stays + // INSIDE the link rather than replacing it. + const { text, unresolved } = resolveLabel(value); return ( - + {unresolved ? ( + + ) : ( + + )} ); } @@ -2492,11 +2536,13 @@ function UnresolvedUserReference({ }): React.ReactElement { const t = useFieldTranslate(); const raw = String(value); - // The key is written as a LITERAL at both sites on purpose: + // The key is written as a LITERAL at every site on purpose: // `check:i18n-keys` judges a literal key against the `en` pack and checks // that the arguments here are exactly the holes that value has, and it // downgrades a key read from a constant to report-only. A shared constant - // would have bought tidiness at the cost of the gate. + // would have bought tidiness at the cost of the gate — which is also why + // `UnresolvedLookupReference` below spells its own key out rather than + // taking one as a prop. const translated = t?.('detail.unresolvedReference', { value: raw }); // Same provider-less rule `useFieldLabel` documents: i18next echoes the key // when nothing resolves it, and the English fallback applies then. That @@ -2506,6 +2552,107 @@ function UnresolvedUserReference({ !translated || translated === 'detail.unresolvedReference' ? `Unresolved reference: ${raw} was not resolved to a user` : translated; + return ; +} + +/** + * A `lookup` / `master_detail` / `tree` reference this screen did NOT resolve + * to a record (objectui#8695), carrying objectui#8434's ruling to the second + * renderer that had the same defect. + * + * ## The state this names, and how many causes hide behind it + * + * `LookupCellRenderer` reaches here when neither the author's `options` nor + * `useLookupName` produced a name. Measured on this base, that ONE seam is fed + * by at least six distinct causes, and the renderer can tell apart NONE of + * them — `useLookupName` returns `string | undefined`, so the + * `pending` / `err` / `ok` discriminator its own cache stores is dropped + * before any caller sees it: + * + * 1. never fetched — no `dataSource`, or no `reference_to` on the field; + * 2. IN FLIGHT — the first paint of every successful resolve passes through + * here (measured: the settled paint replaces it); + * 3. the resolver threw (`state: 'err'`); + * 4. the resolver answered with no record — "fetched and absent"; + * 5. it answered with a record no display field could name; + * 6. not attempted BY POLICY — only the FIRST primitive of an array is + * auto-resolved (`primaryPrimitiveId`), so entries 2..n never ask. + * + * ⇒ the card's premise that this renderer "can genuinely distinguish 'fetched + * and absent' from 'never fetched'" is FALSE as the code stands. And even a + * hook that surfaced the discriminator could not upgrade the sentence: (3) and + * (4) also cover a record the VIEWER may not read, and "cannot read" versus + * "does not exist" is an existence-oracle boundary this lane does not cross + * (objectui#8631). What is true of all six is epistemic, and it is all this + * affordance says: this screen did not resolve it. + * + * ## Why the raw value stays, and the `—` does not + * + * ⛔ This deliberately does NOT keep the muted em-dash this arm used to draw + * for `isLikelyOpaqueId` strings. objectui#8434's triage named that treatment + * by name and ruled against it — the raw string "is the only clue for + * diagnosing existing dirty rows" — and the mother fix's own docblock says it + * again: that treatment buys tidiness by destroying the evidence. The tidiness + * it bought is real and it is the trade-off objectui#8695 flagged against + * itself; it is bought back by TRUNCATION, which hides the id without deleting + * it. The `—` also collided with `EmptyValue`'s glyph, so a cell with no value + * and a cell whose value failed to resolve read identically to a person. + * + * ⚠️ The sentence is a SIBLING key, not the `user` one: that pack value ends + * "was not resolved to a user", which is false on a `lookup` pointing at any + * other object, and it is pinned byte-for-byte by two existing tests. + */ +function UnresolvedLookupReference({ + value, + className, +}: { + value: unknown; + className?: string; +}): React.ReactElement { + const t = useFieldTranslate(); + const raw = String(value); + // Literal key — see `UnresolvedUserReference` above for what reading it + // from a constant would cost at `check:i18n-keys`. + const translated = t?.('detail.unresolvedLookupReference', { value: raw }); + const hint = + !translated || translated === 'detail.unresolvedLookupReference' + ? `Unresolved reference: ${raw} was not resolved to a record on this screen` + : translated; + return ; +} + +/** + * The shipped PRESENTATION of an unresolved reference, shared by the two + * renderers that state one (objectui#8434 for `user`, objectui#8695 for + * `lookup` / `master_detail` / `tree`). + * + * Only the drawing is shared. Each caller keeps its own literal i18n key and + * its own English fallback, because a key reaching this component as a prop + * would be a key `check:i18n-keys` can no longer judge — and because the two + * sentences are genuinely different claims: one is about a person, the other + * about a record of whatever object the lookup points at. + * + * ⚠️ No `pointer-events-none` here, unlike `EmptyValue`: that utility stops the + * span being a hit target, so a `title` on it never renders a tooltip + * (objectui#8506). The stated sentence has to be reachable by hovering. + * + * ⚠️ `truncate` on the inner span rather than the outer one, and the outer is + * `inline-flex`: `overflow: hidden` gives a flex item an automatic minimum + * size of zero, so the text shrinks and ellipsises instead of forcing the row + * wider. The full value stays reachable through the `title` sentence, which + * names it — that is how this shape meets objectui#3466's truncation contract + * (a single-line value must never expand its column and must expose its full + * text) with an icon in front of the text. + */ +function UnresolvedReferenceMark({ + raw, + hint, + className, +}: { + raw: string; + hint: string; + className?: string; +}): React.ReactElement { return ( ` floor. * - Footer: `created_by` / `updated_by` are always user references on * ObjectStack; when the fetched schema omits the audit system fields the - * footer must still render them through the reference renderer (which shows - * a resolved name or a muted placeholder) — never the raw opaque id. + * footer must still render them through the REFERENCE RENDERER rather than + * degrading to a `text` cell that prints `String(value)`. + * + * ## The footer assertions were re-derived at objectui#8695 (PR objectui#9078) + * + * They used to read `expect(queryByText(OPAQUE_ID)).toBeNull()` — "the raw id + * must not appear". ⛔ That was never objectui#2688's ask, and it is now false. + * + * objectui#2688's own expected-correct column is `创建人 Dev Admin · 47分钟前` + * — the RESOLVED NAME — and its card records that the id it complains about + * DOES exist in `sys_user` with `name = Dev Admin`. In the scenario the card + * describes, the id disappears because it RESOLVES, not because anything hides + * it. The located defect the card names is the degradation itself: + * `objectSchema.fields.created_by` absent ⇒ `type:'text'` ⇒ `String(value)`. + * So "the id is absent" was only ever a PROXY for "this went through the + * reference renderer", read off the placeholder that renderer happened to draw + * when nothing resolved. + * + * The proxy was weak even then. Measured on this fixture: the old placeholder + * was a muted `—`, which is byte-identical to `EmptyValue`'s glyph, and a + * footer rendered with NO `created_by` at all omits the actor entirely — both + * satisfy `queryByText(OPAQUE_ID) === null`. The assertion could not tell the + * reference renderer from a blank cell. + * + * objectui#8434 then ruled on that placeholder directly: the affordance for an + * unresolved reference must be ADDITIVE (a stated marker, not an absence), + * EPISTEMIC ("this screen did not resolve it", never "not found"), and it must + * keep the raw value VISIBLE because it "is the only clue for diagnosing + * existing dirty rows". objectui#8695 carried that ruling to the second + * renderer with the same defect — the one this footer routes through. + * + * ⇒ The assertions below now name what objectui#2688 actually asked for, and + * name it directly instead of through a placeholder: the footer must render + * the audit actor through the reference renderer's unresolved affordance, not + * as a bare text cell. That is STRICTLY STRONGER than the assertion it + * replaces, which passed for an empty cell too. */ import { describe, it, expect } from 'vitest'; @@ -69,7 +103,15 @@ describe('DetailView header title — record-key probe before the Record # floor describe('RecordMetaFooter — audit fields default to a sys_user reference (#2688)', () => { const OPAQUE_ID = 'g3WkZnvugj4DnYw8u5Mo6ig3ljDhiFGO'; - it('never prints the raw created_by id when the schema omits the audit field', () => { + /** + * What a `text` cell would have printed: the bare string as the span's whole + * content, with no marker element around it. This is the degradation + * objectui#2688 located, and it is what these assertions refuse. + */ + const renderedAsBareTextCell = (el: HTMLElement | null): boolean => + el !== null && el.closest('[data-slot="unresolved-reference"]') === null; + + it('routes created_by through the reference renderer when the schema omits the audit field', () => { render( , ); expect(screen.getByTestId('record-meta-footer')).toBeInTheDocument(); - // Reference renderer shows a resolved name or a muted placeholder — the - // opaque id itself must not leak into the footer text. - expect(screen.queryByText(OPAQUE_ID)).toBeNull(); + + // Nothing in this fixture can resolve the reference (no dataSource, no + // options), so the renderer reaches its unresolved arm — and that arm is + // the observable proof the value did NOT degrade to a `text` cell. + const mark = document.querySelector('[data-slot="unresolved-reference"]'); + expect(mark).not.toBeNull(); + + // objectui#8434: additive and epistemic. The sentence must be reachable + // (it rides on `title`) and must state non-resolution, not absence. + expect(mark?.getAttribute('title')).toContain(OPAQUE_ID); + expect(mark?.getAttribute('title')).toMatch(/not resolved/i); + + // objectui#8434: the raw value STAYS — it is the only clue for diagnosing + // an existing dirty row — and it stays INSIDE the affordance, which is + // exactly the distinction the retired `queryByText(...).toBeNull()` could + // not draw. + expect(renderedAsBareTextCell(screen.queryByText(OPAQUE_ID))).toBe(false); }); it('still honours an explicit audit-field definition from the schema', () => { @@ -93,6 +149,7 @@ describe('RecordMetaFooter — audit fields default to a sys_user reference (#26 objectName="production_plan" />, ); - expect(screen.queryByText(OPAQUE_ID)).toBeNull(); + expect(document.querySelector('[data-slot="unresolved-reference"]')).not.toBeNull(); + expect(renderedAsBareTextCell(screen.queryByText(OPAQUE_ID))).toBe(false); }); });