From a347fec94861bdd8505b9a1c76cb72cd348d73d6 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <50353452+hotlong@users.noreply.github.com> Date: Tue, 8 Sep 2026 06:58:42 +0800 Subject: [PATCH] fix(plugin-kanban): the card title-dedupe skip set reads the shared name-field resolver, not a key nothing emits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A kanban card printed its record title twice — once as the card heading, once as the first row of the card body. `ObjectKanban` resolves each heading through ADR-0079's `getRecordDisplayName`, then builds `titleFieldsToSkip` so the title field's raw value is not rendered again as a card field. That skip set read `objectDef.NAME_FIELD_KEY`, a key NOTHING produces: `@objectstack/spec@17`'s object schema declares `nameField` (canonical) and `displayNameField` (its deprecated alias), and `NAME_FIELD_KEY` occurs 0 times in the framework tree and 0 times in the published package — this repo reads it only as the last rung of `declaredNameField` in `record-title.ts`, and never emits it. So the read was always `undefined` and the skip set collapsed to its five hard-coded literals (`name` / `full_name` / `title` / `subject` / `display_name`). Any object whose name field is spelled otherwise duplicated its title, which made the defect universal on AI-built apps — whose objects name fields `visit_title`, `owner_name`, `_name` — and invisible on hand-built objects whose name field is literally `name`. The skip set now reads `@object-ui/core`'s `resolveNameField`, the name-space twin of the resolver that produced the heading, so the two agree on WHICH field titles the object. Deliberately ONE rung, unlike the same dedupe in `record-details.tsx` (objectui#8175), which also carries `deriveTitleField`: that ladder filters a SYNTHESIZED field list, whereas this one filters an AUTHOR-DECLARED `cardFields`, where dropping a field the author asked for is a worse failure than a repeated title. The fourth test is the guard — an object whose declared (`code`) and derived (`owner_name`) pointers disagree — and it goes RED if the second rung is ever added. Closes #8400 Co-Authored-By: Claude Fable 5.1 --- .changeset/8400-kanban-name-field-skip-set.md | 34 +++ packages/plugin-kanban/src/ObjectKanban.tsx | 35 ++- ...bjectKanban.nameFieldSkipSet-8400.test.tsx | 222 ++++++++++++++++++ 3 files changed, 290 insertions(+), 1 deletion(-) create mode 100644 .changeset/8400-kanban-name-field-skip-set.md create mode 100644 packages/plugin-kanban/src/__tests__/ObjectKanban.nameFieldSkipSet-8400.test.tsx diff --git a/.changeset/8400-kanban-name-field-skip-set.md b/.changeset/8400-kanban-name-field-skip-set.md new file mode 100644 index 0000000000..efcc8025b0 --- /dev/null +++ b/.changeset/8400-kanban-name-field-skip-set.md @@ -0,0 +1,34 @@ +--- +'@object-ui/plugin-kanban': patch +--- + +Kanban cards: key the title-dedupe skip set off the shared name-field resolver + +A kanban card printed its record title twice — once as the card heading, once as +the first row of the card body. `ObjectKanban` resolves each heading through +ADR-0079's `getRecordDisplayName`, then builds a skip set so the title field's +raw value is not rendered again as a card field. That skip set read +`objectDef.NAME_FIELD_KEY`, a key nothing produces: `@objectstack/spec@17`'s +object schema declares `nameField` (canonical) and `displayNameField` (its +deprecated alias), and `NAME_FIELD_KEY` occurs nowhere in the framework tree — +this repo reads it only as the last rung of the compatibility ladder inside +`record-title.ts`, and never emits it. + +So the read was always `undefined` and the skip set collapsed to its five +hard-coded literals (`name` / `full_name` / `title` / `subject` / +`display_name`). Any object whose name field is spelled otherwise duplicated its +title, which made the defect universal on AI-built apps — their objects name +fields `visit_title`, `owner_name`, `_name` — and invisible on +hand-built objects whose name field is literally `name`. + +The skip set now reads `@object-ui/core`'s `resolveNameField`, the name-space +twin of the resolver that produced the heading, so the two agree on which field +titles the object: declared `nameField` / `displayNameField` / `NAME_FIELD_KEY`, +otherwise the type-aware derivation. + +Deliberately one rung, unlike the same dedupe in `record-details.tsx`, which +also carries `deriveTitleField`: that ladder filters a synthesized field list, +whereas this one filters an author-declared `cardFields`, where dropping a field +the author asked for is a worse failure than a repeated title. A regression test +pins both directions, including an object whose declared and derived pointers +disagree. diff --git a/packages/plugin-kanban/src/ObjectKanban.tsx b/packages/plugin-kanban/src/ObjectKanban.tsx index 48bd449dfe..b4a1413acb 100644 --- a/packages/plugin-kanban/src/ObjectKanban.tsx +++ b/packages/plugin-kanban/src/ObjectKanban.tsx @@ -25,6 +25,7 @@ import { extractRecords, buildExpandFields, getRecordDisplayName, + resolveNameField, } from '@object-ui/core'; import { getBadgeColorClasses, getBadgeHexAppearance, getCellRenderer, resolveCellRendererType } from '@object-ui/fields'; import { usePermissions } from '@object-ui/permissions'; @@ -415,7 +416,39 @@ export const ObjectKanban: React.FC = ({ // resolver removes that footgun. // `nameFieldKey` is retained: it still feeds the description-field skip set // below so the title field's raw value isn't repeated in the card body. - const nameFieldKey: string | undefined = objectDef?.NAME_FIELD_KEY; + // + // ⭐ It reads the SHARED name-space resolver, not a key of its own + // (objectui#8400). This line used to be `objectDef?.NAME_FIELD_KEY`, and + // that key is produced by NOTHING: `@objectstack/spec@17`'s object schema + // declares `nameField` (canonical, ADR-0079) and `displayNameField` (its + // deprecated alias), and `NAME_FIELD_KEY` appears nowhere in the framework + // tree — this repo reads it only as the last rung of `declaredNameField`, + // for objects old enough to have been written against it. So the read was + // always `undefined`, the skip set collapsed to the five literals below, + // and every object whose name field is spelled anything else printed its + // title twice: once as the card heading, once as the first body row. That + // spelling is the NORM for AI-built apps (`visit_title`, `owner_name`, + // `_name`), which is why the duplicate was invisible on hand-built + // objects whose name field is literally `name`. + // + // `resolveNameField` is the name-space twin of the `getRecordDisplayName` + // call that resolves the heading a few lines below, so the two now agree + // about WHICH field titles this object — declared `nameField` / + // `displayNameField` / `NAME_FIELD_KEY`, else the type-aware derivation. + // + // ⚠️ Deliberately ONE rung, unlike the same dedupe in `record-details.tsx` + // (objectui#8175), which lists `resolveNameField()` AND `deriveTitleField()` + // so a declared-but-blank pointer still dedupes against the derivation the + // header fell through to. The surfaces differ in what the skip set is + // allowed to hide: that one filters a SYNTHESIZED field list, this one + // filters an AUTHOR-DECLARED `cardFields`. Carrying the derivation + // alongside a declared pointer would drop a field the author explicitly + // asked for whenever the two disagree (`nameField: 'code'` titles the card + // while the derivation answers `owner_name`) — on a four-field card that is + // a worse failure than a repeated title. Pinned by the over-skip guard in + // `__tests__/ObjectKanban.nameFieldSkipSet-8400.test.tsx`, which goes RED + // if the second rung is ever added here. + const nameFieldKey: string | undefined = resolveNameField(objectDef); return rawData.map(item => { let resolvedTitle: any = undefined; diff --git a/packages/plugin-kanban/src/__tests__/ObjectKanban.nameFieldSkipSet-8400.test.tsx b/packages/plugin-kanban/src/__tests__/ObjectKanban.nameFieldSkipSet-8400.test.tsx new file mode 100644 index 0000000000..88c39044a7 --- /dev/null +++ b/packages/plugin-kanban/src/__tests__/ObjectKanban.nameFieldSkipSet-8400.test.tsx @@ -0,0 +1,222 @@ +/** + * 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#8400 — the kanban card's title-dedupe skip set has to be keyed off + * the SAME name-field ladder the card title itself is resolved with. + * + * ## The defect these tests pin + * + * `ObjectKanban` resolves each card's title through `getRecordDisplayName` + * (ADR-0079), and then builds `titleFieldsToSkip` so the title's raw value is + * not printed a second time as a card-body field. That skip set read + * `objectDef?.NAME_FIELD_KEY` — a key NOTHING produces: + * + * - `@objectstack/spec@17`'s object schema declares `nameField` (canonical) + * and `displayNameField` (deprecated alias). `NAME_FIELD_KEY` appears + * nowhere in the framework tree or in the published package. + * - This repo's own `record-title.ts` already treats it as a legacy alias, + * read as the LAST rung of `declaredNameField`, never emitted. + * + * So the read was always `undefined`, the skip set collapsed to its five + * hard-coded literals (`name` / `full_name` / `title` / `subject` / + * `display_name`), and any object whose name field is spelled otherwise printed + * its title twice — once as the card heading, once as the first body row. + * + * That spelling is the norm for AI-built apps, whose objects name their fields + * `visit_title` / `owner_name` / `_name`, which is why the defect was + * invisible on hand-built objects whose name field is literally `name`. + * + * ## Why the ASSERTIONS are counts, not `toContain` + * + * The bug is a DUPLICATE, so presence proves nothing and absence would be the + * wrong fix. Each test counts occurrences in the card's rendered text: exactly + * one for the title, and at least one for every other declared card field. The + * second half is what stops the suite from being satisfied by a board that + * dropped the body entirely. + * + * ## The third test is the over-skip guard — read it before widening the ladder + * + * `record-details.tsx` solves the same duplicate for the detail grid + * (objectui#8175) and UNROLLS the ladder into two candidates, + * `resolveNameField()` **and** `deriveTitleField()`, because its dedupe has to + * follow a value-keyed header that falls through to the derivation when the + * declared pointer is blank on a record. + * + * That second rung is deliberately NOT copied here, and the third test is what + * says so: it uses an object whose declared pointer (`code`) and derived + * pointer (`owner_name`) DISAGREE, and asserts `owner_name` still renders. The + * two surfaces differ in what the skip set is allowed to hide — the detail + * grid walks a synthesized field list, whereas this set filters an + * AUTHOR-DECLARED `cardFields`. On a four-field card, silently dropping a field + * the author explicitly asked for is a worse failure than repeating a title, so + * the ladder here stops at the one rung that answers "which field titles this + * object". + */ +import { describe, it, expect, vi } from 'vitest'; +import { render, waitFor } from '@testing-library/react'; +import React from 'react'; +import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react'; +// Registers `object-kanban`. +import '../index'; +// Cards 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 under full parallelism (the objectui#3010 rule) — +// same specifier as `index.tsx`'s factory, so ESM's module cache makes that +// factory resolve immediately. +import '../KanbanImpl'; + +/** Occurrences of `needle` in `haystack`. Plain scan — no regex escaping. */ +function countOccurrences(haystack: string, needle: string): number { + let n = 0; + let from = 0; + for (;;) { + const at = haystack.indexOf(needle, from); + if (at === -1) return n; + n += 1; + from = at + needle.length; + } +} + +function makeAdapter(objectDef: Record, record: Record) { + return { + find: vi.fn().mockResolvedValue({ data: [record] }), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + getObjectSchema: vi.fn().mockResolvedValue(objectDef), + }; +} + +async function renderBoard( + objectDef: Record, + record: Record, + cardFields: string[], + awaitText: string, +) { + const { container } = render( + + + , + ); + // Waiting on text that is on the card is what makes the counts below mean + // "the card rendered and this is how often the value appears" rather than + // "nothing has rendered yet" — the failure mode that makes a count vacuous. + await waitFor(() => expect(container.textContent).toContain(awaitText)); + return container; +} + +/** The AI-built shape from the report: an `_title` name field. */ +const VISIT_DEF = { + name: 'visit', + nameField: 'visit_title', + fields: { + visit_title: { type: 'text', label: 'Visit' }, + pet_name: { type: 'text', label: 'Pet' }, + visit_at: { type: 'text', label: 'When' }, + status: { type: 'text', label: 'Status' }, + }, +}; + +const VISIT_ROW = { + id: 'v1', + visit_title: 'Dental cleaning assessment', + pet_name: 'Cola', + visit_at: '2026-09-07 09:30', + status: 'scheduled', +}; + +const TITLE = VISIT_ROW.visit_title; + +describe('kanban cards do not print the record title twice (objectui#8400)', () => { + it('a DECLARED `nameField` listed in `cardFields` renders once, as the heading only', async () => { + // THE repro. `visit_title` is the object's declared name field AND an + // author-listed card field. Before the fix the skip set was keyed off + // `objectDef.NAME_FIELD_KEY` (always undefined), so `visit_title` was not + // skipped and its value rendered as both heading and first body row. + const container = await renderBoard( + VISIT_DEF, + VISIT_ROW, + ['visit_title', 'pet_name', 'visit_at'], + TITLE, + ); + expect(countOccurrences(container.textContent ?? '', TITLE)).toBe(1); + }); + + it('the other declared card fields still render (positive control)', async () => { + // Without this, the assertion above is satisfied by a board that skipped + // every card field, or rendered no body at all. + const container = await renderBoard( + VISIT_DEF, + VISIT_ROW, + ['visit_title', 'pet_name', 'visit_at'], + TITLE, + ); + const text = container.textContent ?? ''; + expect(text).toContain(VISIT_ROW.pet_name); + expect(text).toContain(VISIT_ROW.visit_at); + }); + + it('a DERIVED name field (no `nameField` declared) is deduped too, and does not get worse', async () => { + // No declared pointer at all, so `getRecordDisplayName` reaches its + // type-aware derivation (step 4) and titles the card with `visit_title` + // because of the `_title` affix. `resolveNameField` falls through to the + // SAME derivation, so the skip set follows the heading here as well. + const { name, fields } = VISIT_DEF; + const derivedDef = { name, fields }; + const container = await renderBoard( + derivedDef, + VISIT_ROW, + ['visit_title', 'pet_name'], + TITLE, + ); + const text = container.textContent ?? ''; + expect(countOccurrences(text, TITLE)).toBe(1); + // Same positive control: the derivation must dedupe the title, not the card. + expect(text).toContain(VISIT_ROW.pet_name); + }); + + it('does NOT hide a card field that merely LOOKS name-ish while another field is declared', async () => { + // Over-skip guard (see the file docblock). Declared pointer and derived + // pointer disagree: `nameField: 'code'` titles the card, while + // `deriveTitleField` would answer `owner_name` on the `_name` affix. The + // author listed `owner_name`, the heading never showed it, so it must + // render. This goes RED if the skip set is ever widened to also carry the + // derivation alongside a declared pointer. + const def = { + name: 'contract', + nameField: 'code', + fields: { + code: { type: 'text', label: 'Code' }, + owner_name: { type: 'text', label: 'Owner' }, + status: { type: 'text', label: 'Status' }, + }, + }; + const row = { + id: 'c1', + code: 'CT-4471-KLM', + owner_name: 'Zhou Mingxuan', + status: 'scheduled', + }; + const container = await renderBoard(def, row, ['code', 'owner_name'], row.code); + const text = container.textContent ?? ''; + expect(countOccurrences(text, row.code)).toBe(1); + expect(text).toContain(row.owner_name); + }); +});