|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | +// |
| 3 | +// [#13259] `hidden` is a floor on BOTH passes of `getUiView`'s list branch. |
| 4 | +// |
| 5 | +// `FieldSchema` declares `hidden` as "Hidden from default UI" |
| 6 | +// (`packages/spec/src/data/field.zod.ts`). `getUiView` IS the default UI — it |
| 7 | +// is the producer behind `GET /api/v1/ui/view/:object/:type` — so that |
| 8 | +// sentence is a floor here or it is a floor nowhere. |
| 9 | +// |
| 10 | +// It was not one. The list branch picks columns in two passes: |
| 11 | +// |
| 12 | +// let columns = fieldKeys.filter(k => priorityFields.includes(k)); |
| 13 | +// if (columns.length < 5) { /* fill pass, WITH !fields[k].hidden */ } |
| 14 | +// |
| 15 | +// and `!fields[k].hidden` sat on the fill pass alone. A field declared |
| 16 | +// `hidden: true` was therefore dropped for eight of nine spellings and served |
| 17 | +// — with its label — for the ninth: whenever the author happened to name it |
| 18 | +// one of `name`, `title`, `label`, `subject`, `email`, `status`, `type`, |
| 19 | +// `category`, `created_at`. Those are the ordinary names, not exotic ones. |
| 20 | +// Meanwhile the `form` branch of the same function filtered every hidden field |
| 21 | +// uniformly, so two branches of one producer disagreed about what `hidden` |
| 22 | +// means. |
| 23 | +// |
| 24 | +// ## Why this file drives more than one field |
| 25 | +// |
| 26 | +// ⛔ An earlier measurement (PR #13244) drove ONE hidden field, which happened |
| 27 | +// not to be a priority name, saw it dropped, and reported *"hidden is dropped |
| 28 | +// by declaration"*. That reading was true of the field it drove and false of |
| 29 | +// the class — a **false clearance**: a result that reads as general because |
| 30 | +// its single case fell on the safe side. |
| 31 | +// |
| 32 | +// So every case here carries an arm that would have come out the other way: |
| 33 | +// |
| 34 | +// 1. a hidden field that IS a priority name (`status`) — the defect; |
| 35 | +// 2. a hidden field that is NOT a priority name (`beta_secret`) |
| 36 | +// — already correct before the fix, so it guards the fill pass against a |
| 37 | +// repair that over-reaches in the other direction; |
| 38 | +// 3. a NON-hidden priority field (`name`) |
| 39 | +// — without it, "nothing is emitted" would satisfy arms 1 and 2 |
| 40 | +// vacuously. This is the control. |
| 41 | +// |
| 42 | +// ⚠️ The nine-name sweep below then closes the gap between "true of `status`" |
| 43 | +// and "true of the class": it drives EVERY priority name hidden at once. |
| 44 | +// |
| 45 | +// ⚠️ The sibling harness `packages/rest/src/ui-view-route-tenancy.measurement.test.ts` |
| 46 | +// (#13214 / PR #13258) drives the same defect through the REST route and pins |
| 47 | +// the pre-fix answer as a measurement. It belongs to that card and is |
| 48 | +// deliberately not edited here; this file is the pin next to the code. |
| 49 | + |
| 50 | +import { describe, it, expect } from 'vitest'; |
| 51 | +import { GetUiViewResponseSchema } from '@objectstack/spec/api'; |
| 52 | +import { ObjectStackProtocolImplementation } from './protocol.js'; |
| 53 | + |
| 54 | +/** |
| 55 | + * The producer's own priority list, restated. It is a local `const` inside |
| 56 | + * `getUiView` and cannot be imported, so this copy is a duplicate by |
| 57 | + * necessity — which is why the assertions below never rely on it alone: each |
| 58 | + * case also asserts the name-agnostic invariant *no emitted column is declared |
| 59 | + * hidden*, computed from the fixture. A tenth priority name added without the |
| 60 | + * filter fails that invariant even though this list would not know about it. |
| 61 | + */ |
| 62 | +const PRIORITY_NAMES = [ |
| 63 | + 'name', 'title', 'label', 'subject', 'email', 'status', 'type', 'category', 'created_at', |
| 64 | +] as const; |
| 65 | + |
| 66 | +/** |
| 67 | + * Three arms in one object, per the header: |
| 68 | + * - `status` — hidden AND a priority name (arm 1, the defect) |
| 69 | + * - `beta_secret` — hidden, NOT a priority name (arm 2, already correct) |
| 70 | + * - `name` — a priority name, NOT hidden (arm 3, the control) |
| 71 | + * `plain_note` keeps the fill pass exercised, and `created_at` keeps the |
| 72 | + * `sort` branch on the same path it takes in production. |
| 73 | + */ |
| 74 | +const MIXED = { |
| 75 | + name: 'account', |
| 76 | + label: 'Account', |
| 77 | + fields: { |
| 78 | + id: { name: 'id', type: 'text' }, |
| 79 | + name: { name: 'name', type: 'text', label: 'Account Name', required: true }, |
| 80 | + status: { name: 'status', type: 'text', label: 'Beta Status', hidden: true }, |
| 81 | + beta_secret: { name: 'beta_secret', type: 'text', label: 'Beta Secret', hidden: true }, |
| 82 | + plain_note: { name: 'plain_note', type: 'text', label: 'Plain Note' }, |
| 83 | + created_at: { name: 'created_at', type: 'datetime', label: 'Created' }, |
| 84 | + }, |
| 85 | +} as const; |
| 86 | + |
| 87 | +function protocolFor(schema: unknown) { |
| 88 | + const engine = { registry: { getObject: () => schema } }; |
| 89 | + return new ObjectStackProtocolImplementation(engine as any); |
| 90 | +} |
| 91 | + |
| 92 | +const columnsOf = (body: any): string[] => (body.list.columns as any[]).map((c) => c.field); |
| 93 | +const labelsOf = (body: any): string[] => (body.list.columns as any[]).map((c) => c.label); |
| 94 | +const formFieldsOf = (body: any): string[] => |
| 95 | + (body.form.sections[0].fields as any[]).map((f) => f.field); |
| 96 | + |
| 97 | +/** Every key the fixture declares `hidden: true` on — the fixture's own answer. */ |
| 98 | +const hiddenKeysOf = (schema: any): string[] => |
| 99 | + Object.keys(schema.fields).filter((k) => schema.fields[k].hidden === true); |
| 100 | + |
| 101 | +describe('[#13259] getUiView list branch honours `hidden` on the priority pass', () => { |
| 102 | + it('drops a hidden PRIORITY-named field, drops a hidden non-priority field, and still serves a visible priority field', async () => { |
| 103 | + const body: any = await protocolFor(MIXED).getUiView({ object: 'account', type: 'list' }); |
| 104 | + const columns = columnsOf(body); |
| 105 | + |
| 106 | + // Arm 1 — the defect. `status` is hidden AND a priority name. Before |
| 107 | + // the fix this came back as `{ field: 'status', label: 'Beta Status', |
| 108 | + // sortable: true }`. |
| 109 | + expect(columns).not.toContain('status'); |
| 110 | + |
| 111 | + // Arm 2 — hidden, not a priority name. Correct before the fix too; it |
| 112 | + // is here so a repair that broke the fill pass would not read as green. |
| 113 | + expect(columns).not.toContain('beta_secret'); |
| 114 | + |
| 115 | + // Arm 3 — the control. Without this the two assertions above are |
| 116 | + // satisfied by a producer that emits nothing at all. |
| 117 | + expect(columns).toContain('name'); |
| 118 | + expect(columns).toContain('plain_note'); |
| 119 | + expect(columns.length).toBeGreaterThan(0); |
| 120 | + |
| 121 | + // The name-agnostic form of the same statement: whatever the priority |
| 122 | + // list happens to contain, no emitted column may be declared hidden. |
| 123 | + expect(columns.filter((c) => (MIXED.fields as any)[c]?.hidden === true)).toEqual([]); |
| 124 | + }); |
| 125 | + |
| 126 | + it('does not leak the LABEL of a hidden field either', async () => { |
| 127 | + // The card's finding was not "a field name appears" — the emitted |
| 128 | + // column carried `label: 'Beta Status'`, an authored human string. |
| 129 | + const body: any = await protocolFor(MIXED).getUiView({ object: 'account', type: 'list' }); |
| 130 | + expect(labelsOf(body)).not.toContain('Beta Status'); |
| 131 | + expect(labelsOf(body)).not.toContain('Beta Secret'); |
| 132 | + // Control: the visible field's label is still served. |
| 133 | + expect(labelsOf(body)).toContain('Account Name'); |
| 134 | + }); |
| 135 | + |
| 136 | + it('does not offer a hidden field as searchable', async () => { |
| 137 | + // `searchableFields` is `columns.slice(0, 3)`, so a hidden priority |
| 138 | + // name reaching `columns` also reached the search affordance. Derived, |
| 139 | + // but worth pinning: it is a second user-visible consequence of the |
| 140 | + // same line, and a future rewrite could re-derive it independently. |
| 141 | + const body: any = await protocolFor(MIXED).getUiView({ object: 'account', type: 'list' }); |
| 142 | + const searchable: string[] = body.list.searchableFields; |
| 143 | + expect(searchable).not.toContain('status'); |
| 144 | + expect(searchable).not.toContain('beta_secret'); |
| 145 | + expect(searchable.length).toBeGreaterThan(0); |
| 146 | + }); |
| 147 | + |
| 148 | + // ⚠️ The class, not the field. Every one of the nine priority names is |
| 149 | + // declared hidden at once, plus a single visible non-priority field so the |
| 150 | + // expected answer is a specific non-empty set rather than "empty". |
| 151 | + it('holds for ALL NINE priority names, not just the one the card drove', async () => { |
| 152 | + const allHidden: any = { |
| 153 | + name: 'sweep', |
| 154 | + label: 'Sweep', |
| 155 | + fields: { |
| 156 | + id: { name: 'id', type: 'text' }, |
| 157 | + visible_note: { name: 'visible_note', type: 'text', label: 'Visible Note' }, |
| 158 | + ...Object.fromEntries( |
| 159 | + PRIORITY_NAMES.map((n) => [n, { name: n, type: 'text', label: `L ${n}`, hidden: true }]), |
| 160 | + ), |
| 161 | + }, |
| 162 | + }; |
| 163 | + |
| 164 | + const body: any = await protocolFor(allHidden).getUiView({ object: 'sweep', type: 'list' }); |
| 165 | + const columns = columnsOf(body); |
| 166 | + |
| 167 | + // Exactly the one visible field — every priority name is withheld, and |
| 168 | + // the answer is not vacuously empty. |
| 169 | + expect(columns).toEqual(['visible_note']); |
| 170 | + for (const n of PRIORITY_NAMES) expect(columns).not.toContain(n); |
| 171 | + expect(columns.filter((c) => allHidden.fields[c]?.hidden === true)).toEqual([]); |
| 172 | + }); |
| 173 | + |
| 174 | + // The other half of the finding: two branches of ONE producer disagreed. |
| 175 | + // Asserting they now agree is not the same as asserting the list branch |
| 176 | + // changed, so both are driven from the same fixture and compared. |
| 177 | + it('the list and form branches now agree about what `hidden` withholds', async () => { |
| 178 | + const p = protocolFor(MIXED); |
| 179 | + const list: any = await p.getUiView({ object: 'account', type: 'list' }); |
| 180 | + const form: any = await p.getUiView({ object: 'account', type: 'form' }); |
| 181 | + |
| 182 | + const hidden = hiddenKeysOf(MIXED); |
| 183 | + expect(hidden).toEqual(['status', 'beta_secret']); // the fixture says what it says |
| 184 | + |
| 185 | + for (const k of hidden) { |
| 186 | + expect(columnsOf(list)).not.toContain(k); |
| 187 | + expect(formFieldsOf(form)).not.toContain(k); |
| 188 | + } |
| 189 | + }); |
| 190 | + |
| 191 | + // ⛔ The form branch is NOT what this card changes, so its exact output is |
| 192 | + // pinned rather than merely asserted to be "still filtering". If the repair |
| 193 | + // had over-reached into the form branch, this is what would say so. |
| 194 | + it('the form branch is unchanged — exact field list pinned', async () => { |
| 195 | + const form: any = await protocolFor(MIXED).getUiView({ object: 'account', type: 'form' }); |
| 196 | + // `id`, `created_at` and `updated_at` are excluded by the form branch's |
| 197 | + // own rule; `status` and `beta_secret` by `hidden`. Order is the |
| 198 | + // schema's declaration order. |
| 199 | + expect(formFieldsOf(form)).toEqual(['name', 'plain_note']); |
| 200 | + }); |
| 201 | + |
| 202 | + // The narrowed body must still satisfy the response contract it declares — |
| 203 | + // a fix that emitted a well-shaped-but-invalid payload would otherwise go |
| 204 | + // out unchecked (`rest-server.ts` does a bare `res.json(view)`). |
| 205 | + it('the narrowed list body still parses GREEN against GetUiViewResponseSchema', async () => { |
| 206 | + const body = await protocolFor(MIXED).getUiView({ object: 'account', type: 'list' }); |
| 207 | + const parsed = GetUiViewResponseSchema.safeParse(body); |
| 208 | + const explain = parsed.success |
| 209 | + ? 'GREEN' |
| 210 | + : parsed.error.issues.map((i: any) => `[${i.code}] path=${JSON.stringify(i.path)} ${i.message}`).join('\n'); |
| 211 | + expect(explain).toBe('GREEN'); |
| 212 | + }); |
| 213 | +}); |
0 commit comments