diff --git a/src/apps/duly.app.ts b/src/apps/duly.app.ts index c4ff419..19ee05d 100644 --- a/src/apps/duly.app.ts +++ b/src/apps/duly.app.ts @@ -69,11 +69,24 @@ export const DulyApp = App.create({ // entry that declares the dependency. It is also the right runtime // behaviour — the entry hides instead of 404-ing where `sys_user` is // not registered. + // + // `viewName` is load-bearing in the same way, and leaving it off was a + // real defect (#118). A nav entry with no `viewName` lands on the + // object's DEFAULT view, and `sys_user`'s default is `me` — "My + // Profile", filtered `id == {current_user_id}` with `pageSize: 1`. So + // the manager who followed the dashboard here to look at OTHER PEOPLE + // saw exactly one row: themselves. `all_users` is the platform's own + // unfiltered lens (`@objectstack/platform-objects`, + // `sys_user.listViews.all_users`) — named here rather than redeclared, + // because the views of a runtime-provided object are not ours to + // author, and a local copy would drift from the one the platform + // maintains. { id: 'nav_people', type: 'object', objectName: 'sys_user', requiresObject: 'sys_user', + viewName: 'all_users', label: 'People', icon: 'users-round', }, diff --git a/src/views/task.view.ts b/src/views/task.view.ts index 8203c49..08fc062 100644 --- a/src/views/task.view.ts +++ b/src/views/task.view.ts @@ -175,10 +175,35 @@ export const TaskViews = defineView({ listViews: { /** - * The frontline screen (deck p16). Column order is the deck's, read left - * to right the way the work is: what state it is in, what it is, who put - * it there, when it is owed, what the last word on it was, and whether - * anything is attached. + * The frontline screen (deck p16). Column order reads left to right the + * way the work is: WHAT IT IS first, then what state it is in, who put it + * there, when it is owed, and what the last word on it was. + * + * ── Why `subject` leads, and why that is not taste (#118) ─────────── + * The first column is not merely the leftmost one: two renderers read it + * as the row's IDENTITY, so putting `status` there made the state of a + * thing stand in for the thing. Both consequences were measured on the + * demo, not reasoned about: + * + * desktop grid the Console makes the FIRST column the record link, so + * the only clickable thing on the row was the `Open` + * pill, and the task name sat beside it as inert text — + * while every user goes for the name. + * 390px cards the card renderer takes the first column as the card + * TITLE and renders the RAW stored value, so every card + * was headed `open` / `in_progress` with the task itself + * demoted to a field. + * + * The two lenses below (`late`, `stalled`) and `board` already lead with + * `subject`; this one was the exception, and now is not. + * + * ── And `attachments` is gone from THIS lens only ───────────────── + * It was a dash on every row of the busiest screen in the product, which + * is width spent on the absence of a thing. Nothing is hidden and nothing + * is un-uploadable: the field is untouched, the shared `columns` array + * above still carries it on the other grids, and the upload #108 built + * lives in the record page's "Progress and attachments" group — which is + * where a person who has a file to attach is going anyway. * * ── `inlineEdit` is what makes the phrase one tap ──────────────────── * Without it the row is read-only and reporting progress costs a record @@ -204,12 +229,11 @@ export const TaskViews = defineView({ type: 'grid', data, columns: [ - { field: 'status' }, { field: 'subject' }, + { field: 'status' }, { field: 'source' }, { field: 'due_date' }, { field: 'progress' }, - { field: 'attachments' }, ], inlineEdit: true, filter: [ diff --git a/test/member-page.test.ts b/test/member-page.test.ts index 364a54a..29dabaa 100644 --- a/test/member-page.test.ts +++ b/test/member-page.test.ts @@ -109,6 +109,15 @@ describe('duly_member — wiring', () => { expect(people!.type).toBe('object'); expect(people!.objectName).toBe('sys_user'); expect(people!.requiresObject).toBe('sys_user'); + // #118 — and it must name a view. With no `viewName` the Console lands on + // the object's DEFAULT view, which for `sys_user` is `me` ("My Profile"), + // filtered `id == {current_user_id}`: the manager who came here to look at + // other people got exactly one row, themselves. `all_users` is the + // platform's own unfiltered lens. + expect( + people!.viewName, + 'People with no viewName lands on sys_user\'s default view — `me`, one row (#118)', + ).toBe('all_users'); }); it('every component carries a stable id', () => { diff --git a/test/metadata-bindings.test.ts b/test/metadata-bindings.test.ts index 7081e7b..5094f90 100644 --- a/test/metadata-bindings.test.ts +++ b/test/metadata-bindings.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; +import { SysUser } from '@objectstack/platform-objects/identity'; import { isPlatformProvidedObjectName } from '@objectstack/spec/system'; import { SystemFieldName } from '@objectstack/spec/system'; @@ -438,6 +439,24 @@ interface Stack { * `dulyDatasets`. Optional so the self-test fixtures below can omit it. */ readonly dashboards?: readonly unknown[]; + /** + * List-view names declared by the PLATFORM objects this app binds nav to — + * `sys_user` → { me, all_users, unverified, … }. Supplied by the real stack + * from `@objectstack/platform-objects`, which ships the definitions on disk; + * omitted by the synthetic fixtures below, which have no platform package to + * read and so keep exercising the boundary branch. + * + * It exists because #118 authored this app's first cross-boundary nav + * reference, and the boundary was the WRONG answer for it. A platform + * object's FIELDS genuinely cannot be judged from `@objectstack/spec` (it + * exports the name registry, not field lists) — but its VIEWS can: the + * package that declares them is a devDependency and its `listViews` is a + * plain object. Recording `viewName` as an unjudgeable boundary would have + * left exactly the defect #118 fixed — a nav entry silently falling back to + * the object's default view while keeping its authored label — unguarded on + * the one surface where it had just been measured. + */ + readonly platformViews?: ReadonlyMap>; } /** @@ -733,7 +752,26 @@ export const metadataBindingFindings = (stack: Stack): WalkResult => { } if (bound.kind === 'platform') { - result.boundaries.push({ where: `${where} · viewName`, reference: viewName, at: objectName }); + // Resolvable after all, when the platform package is on disk: see + // `Stack.platformViews`. With no map supplied the reference stays a + // declared boundary, which is what the synthetic fixtures assert. + const platformKnown = stack.platformViews?.get(objectName); + if (!platformKnown) { + result.boundaries.push({ where: `${where} · viewName`, reference: viewName, at: objectName }); + continue; + } + if (platformKnown.has(viewName)) { + result.resolved.push(`${where} · viewName → ${objectName}.${viewName} (platform)`); + continue; + } + result.findings.push({ + where: `${where} · viewName`, + reference: viewName, + reason: + `the platform object ${objectName} declares no list view named "${viewName}" — the shell ` + + `SILENTLY falls back to its default view and keeps this entry's authored label. Declared: ` + + `${[...platformKnown].sort().join(', ') || '(none)'}`, + }); continue; } const known = viewsByObject.get(objectName) ?? new Set(); @@ -789,12 +827,22 @@ function filterFieldKeys(filter: unknown, out: string[] = []): string[] { // ─── The check, over this app's real metadata ──────────────────────────── +/** + * Read off `@objectstack/platform-objects` rather than hand-listed, so the + * day the platform renames or drops a view this walk says so instead of + * pinning a name that stopped existing. + */ +const platformViews = new Map>([ + [String(SysUser.name), new Set(Object.keys(SysUser.listViews ?? {}))], +]); + const stack: Stack = { views: dulyViews as unknown[], datasets: dulyDatasets as unknown[], apps: dulyApps as unknown[], objects: dulyObjects as unknown as DeclaredObject[], dashboards: dulyDashboards as unknown[], + platformViews, }; const result = metadataBindingFindings(stack); @@ -851,6 +899,14 @@ describe('metadata bindings — every reference resolves (stopgap for objectstac // `@objectstack/spec` exports the platform object NAME registry but no // field lists, so a path like `owner.some_typo` cannot be judged. Rather // than let that be a silent hole, fail the day one is authored. + // + // #118 authored the first one — `nav_people` naming `sys_user.all_users` + // — and answering it by widening this pin would have been the wrong + // move: a platform object's VIEWS are on disk even though its fields are + // not, so the walk now RESOLVES that reference (`Stack.platformViews`) + // and this stays an exact zero. What still lands here is the case that + // genuinely cannot be judged: a field path hopping into a platform + // object. expect( result.boundaries.map((b) => `${b.where}: "${b.reference}" stops at ${b.at}`), 'a reference reaches into a platform object, whose fields are not on disk — this guard cannot check it', @@ -1179,6 +1235,47 @@ describe('metadata bindings — the guard can fail (self-test on synthetic metad expect(r.findings[0]!.reason).toContain('no default `list` view'); }); + // ── Nav `viewName` on a PLATFORM object (#118) ───────────────────────── + // + // Three cases, because the branch has three outcomes and the middle one is + // the whole point: an unresolvable platform view name is a real finding, + // not a boundary. All three drive the same `metadataBindingFindings` the + // shipped metadata goes through. + const platformNav = (viewName?: string): Partial => ({ + apps: [app([{ id: 'people', type: 'object', objectName: 'sys_user', ...(viewName ? { viewName } : {}) }])], + }); + + it('resolves a nav `viewName` on a platform object against the platform package', () => { + const r = run({ + ...platformNav('all_users'), + platformViews: new Map([['sys_user', new Set(['me', 'all_users'])]]), + }); + expect(messages(r), 'a view the platform really declares is not a finding').toEqual([]); + expect(r.boundaries, 'and it is no longer an unjudgeable boundary either').toEqual([]); + expect(r.resolved.some((x) => x.includes('sys_user.all_users (platform)'))).toBe(true); + }); + + it('DOES fire on a nav `viewName` the platform object does not declare', () => { + // The defect #118 fixed, in its typo form: the shell falls back to the + // object's default view and keeps the authored label, so the screen looks + // right and lists the wrong rows. + const r = run({ + ...platformNav('all_userz'), + platformViews: new Map([['sys_user', new Set(['me', 'all_users'])]]), + }); + expect(r.findings.map((f) => f.reference)).toEqual(['all_userz']); + expect(r.findings[0]!.reason).toContain('SILENTLY falls back'); + expect(r.findings[0]!.reason, 'and it names what IS declared').toContain('all_users'); + }); + + it('records a boundary when no platform view map is supplied — the pre-#118 behaviour', () => { + // Unchanged for any platform object this repo has not taught the walk + // about: unjudgeable is still declared rather than assumed fine. + const r = run(platformNav('all_users')); + expect(messages(r)).toEqual([]); + expect(r.boundaries.map((b) => `${b.reference}@${b.at}`)).toEqual(['all_users@sys_user']); + }); + // ── 5. Dashboard nav ─────────────────────────────────────────────────── it('fires on a nav `dashboardName` that names no dashboard', () => { const r = run({ diff --git a/test/views.test.ts b/test/views.test.ts index 1e4e28c..054d35c 100644 --- a/test/views.test.ts +++ b/test/views.test.ts @@ -180,16 +180,47 @@ describe('the lenses say what the product means', () => { }); /** - * #108 — the frontline screen the deck's p16 draws. + * #108 / #118 — the frontline screen the deck's p16 draws. * * The column SET and its ORDER are both the card's, so this is not a * restatement of the file: a reorder here is a product change and should - * have to be argued for. The two new columns are the ones the whole card is - * about — a list without them is the list we already had. + * have to be argued for. + * + * #118 argued for one: `subject` leads and `attachments` is gone. `status` + * led until then, and the FIRST column is not merely the leftmost — the + * desktop grid makes it the record link and the 390px card renderer makes + * it the card title, so the row's link was the `Open` pill and every card + * was headed with a raw `open` / `in_progress`. */ it('my_week carries the deck\'s columns, in the deck\'s order', () => { const fields = ((byName('my_week').view.columns as Rec[]) ?? []).map((c) => String(c.field)); - expect(fields).toEqual(['status', 'subject', 'source', 'due_date', 'progress', 'attachments']); + expect(fields).toEqual(['subject', 'status', 'source', 'due_date', 'progress']); + }); + + /** + * The half of #118 that outlives the exact column list above: whatever the + * set becomes, the first column is the row's IDENTITY on both renderers, so + * it must be the field a person reads the row by. Asserted across every + * grid lens rather than on `my_week` alone — the defect was one lens + * disagreeing with the other four, and a per-view pin would not have said + * so. + */ + it('every task grid leads with the column a person reads the row by', () => { + // Walked rather than listed by name, so a grid lens added later is covered + // the day it lands — and it reaches the container default (`list`), which + // `byName` cannot address. + const grids = allViews.filter((v) => v.object === 'duly_task' && v.view.type === 'grid'); + expect(grids.length, 'no duly_task grid was found — this check would pass vacuously') + .toBeGreaterThanOrEqual(5); + for (const grid of grids) { + const first = String(((grid.view.columns as Rec[]) ?? [])[0]?.field); + expect( + first, + `${grid.where} leads with \`${first}\` — the first column is the record link on ` + + 'desktop and the card TITLE at 390px, so a status or a date there makes the row ' + + 'link a pill and titles every card with a raw stored value (#118)', + ).toBe('subject'); + } }); /**