diff --git a/.changeset/inbox-actor-name-retired-5203.md b/.changeset/inbox-actor-name-retired-5203.md new file mode 100644 index 000000000..ebb7ec8a9 --- /dev/null +++ b/.changeset/inbox-actor-name-retired-5203.md @@ -0,0 +1,28 @@ +--- +--- + +Internal only — this publishes nothing, declared explicitly with an empty frontmatter +rather than left undeclared. + +Retires `InboxNotification.actor_name` (`packages/app-shell/src/layout/inboxGrouping.ts`), +which was dead at both ends. `mergeInboxRows` +(`packages/app-shell/src/hooks/sharedUserFeeds.ts`) is the single producer of every row +the bell and Home's action centre render and never mapped it; neither consumer read it; +and `sys_inbox_message` declares no actor column for it to have been mapped FROM. It was +the last declared-but-unfilled member of that interface after objectui#5190 removed the +sibling `source_object` / `source_id` pair. + +**No published type surface changes.** `InboxNotification` is not reachable from +`@object-ui/app-shell`'s public entry: neither `src/index.ts` nor `src/layout/index.ts` +re-exports it, the built `dist/index.d.ts` does not name it, and the package `exports` +map offers only `.` and `./styles.css` — no deep subpath an external consumer could +import it through. The type is internal to the package, so removing an optional member +of it is not an externally observable narrowing and nothing user-visible ships. Runtime +behaviour is unchanged in both directions: no code path produced the field and no code +path read it. + +Two pins keep it retired, in opposite directions — a TYPE PIN in +`layout/__tests__/inboxGrouping.test.ts` that fails if the field is re-declared, and a +runtime key-set pin in `hooks/__tests__/sharedInboxFeed.rowShape.test.tsx` that fails if +the producer is ever changed to spread raw `sys_inbox_message` columns through instead +of mapping them field by field. diff --git a/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.rowShape.test.tsx b/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.rowShape.test.tsx new file mode 100644 index 000000000..252ff2dff --- /dev/null +++ b/packages/app-shell/src/hooks/__tests__/sharedInboxFeed.rowShape.test.tsx @@ -0,0 +1,137 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectui#5203 — the produced inbox row carries exactly what it declares. + * + * ## What this is the other half of + * + * `InboxNotification` (`layout/inboxGrouping.ts`) used to declare an + * `actor_name` that was dead at both ends: `mergeInboxRows` below never mapped + * it, neither consumer (`InboxPopover`, `useHomeInbox`) read it, and + * `sys_inbox_message` declares no actor column for it to be mapped FROM. The + * declaration was removed; `inboxGrouping.test.ts` carries the TYPE PIN that + * stops it being re-declared. + * + * A type pin alone cannot see the direction this contract is most likely to rot + * in, because that direction does not go through the type at all: the producer + * builds its row from an explicit field-by-field literal, and the cheap + * "improvement" is to spread the raw `sys_inbox_message` record into it + * (`{ ...m, is_read }`). That compiles, it is invisible to `tsc`, and it + * silently re-admits every raw column the backend happens to grow — including + * an `actor_name` — as an undeclared de-facto field on the UI shape. So this + * suite feeds the producer a raw row LOADED with columns it does not map and + * asserts the produced row's key set, not just the absence of one key. + * + * ## Reverse verification (direction predicted BEFORE running, measured in this PR) + * + * - replace the producer's explicit literal with a `{ ...m }` pass-through ⇒ + * both cases here go RED (extra keys present, `actor_name` among them), + * while the `inboxGrouping.test.ts` type pin stays GREEN — which is why the + * type pin cannot stand in for this one, and both are here; + * - re-declare `actor_name` on `InboxNotification` ⇒ this suite stays GREEN + * (a declaration nobody fills changes no runtime shape) and the type pin + * goes RED. Opposite directions, one per instrument. + */ +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { act, renderHook } from '@testing-library/react'; + +/** One signed-in user — the inbox feed reads nothing without one. */ +vi.mock('@object-ui/auth', () => ({ useAuth: () => ({ user: { id: 'u1' } }) })); + +/** + * A `sys_inbox_message` row as the L5 materialization writes it, PLUS the + * columns the UI shape does not map. `severity` and `delivery_id` are real + * columns on the object today; `actor_name` is deliberately NOT (this is the + * hypothetical future column the retired field was an invitation to add) — the + * point of including it is that the producer's answer must be the same either + * way: an unmapped column does not reach the UI shape. + */ +const RAW_ROW = { + id: 'ibx_1', + user_id: 'u1', + notification_id: 'ntf_1', + delivery_id: 'dlv_1', + topic: 'collab.assignment', + title: 'Assigned to you: Ship it', + body_md: 'Zhang San assigned you a task.', + severity: 'info', + action_url: '/apps/crm/showcase_task/t_42', + created_at: '2026-08-18T09:00:00Z', + actor_name: 'Li Si', +}; + +/** The keys `mergeInboxRows` maps — the whole of `InboxNotification`. */ +const DECLARED_KEYS = [ + 'action_url', + 'body', + 'created_at', + 'id', + 'is_read', + 'notification_id', + 'receipt_id', + 'title', + 'type', +]; + +const fakeAdapter = { + find: (object: string) => { + if (object === 'sys_inbox_message') return Promise.resolve({ data: [RAW_ROW] }); + // Receipts and activity answer emptily — neither is this suite's subject. + return Promise.resolve({ data: [] }); + }, + getClient: () => undefined, +}; +vi.mock('../../providers/AdapterProvider', () => ({ useAdapter: () => fakeAdapter })); + +import { useSharedInboxFeed, __resetSharedUserFeeds } from '../sharedUserFeeds'; + +/** Let the attach-time read settle without moving the clock past a poll. */ +const settle = () => act(async () => { await vi.advanceTimersByTimeAsync(0); }); + +beforeEach(() => { + vi.useFakeTimers(); + // Module-scoped stores outlive any one render tree. + __resetSharedUserFeeds(); + // Approvals degrade to 0 (404) so nothing here depends on the REST feed. + vi.stubGlobal('fetch', vi.fn(() => Promise.resolve(new Response('{}', { status: 404 })))); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.useRealTimers(); +}); + +describe('objectui#5203 — the inbox producer maps fields, it does not pass rows through', () => { + it('produces exactly the declared key set from a raw row carrying more', async () => { + const { result } = renderHook(() => useSharedInboxFeed()); + await settle(); + + expect(result.current.status).toBe('ready'); + expect(result.current.value).toHaveLength(1); + + const row = result.current.value[0]; + expect(Object.keys(row).sort()).toEqual(DECLARED_KEYS); + // The mapping itself still works — otherwise the key-set assertion above + // could be satisfied by a producer that maps nothing. + expect(row.title).toBe('Assigned to you: Ship it'); + expect(row.type).toBe('collab.assignment'); + expect(row.body).toBe('Zhang San assigned you a task.'); + expect(row.action_url).toBe('/apps/crm/showcase_task/t_42'); + }); + + it('does not carry `actor_name` even when the raw row has one', async () => { + // The card's field by name, so a future reader grepping for it lands here + // rather than on one of the live `actor_name` fields elsewhere in this + // package (`sys_activity` -> `ActivityItem`, approval activity rows). + const { result } = renderHook(() => useSharedInboxFeed()); + await settle(); + + // Via `unknown`: `InboxNotification` has no index signature, which is the + // point — the key being probed is not addressable on the declared shape. + const row = result.current.value[0] as unknown as Record; + expect(Object.hasOwn(row, 'actor_name')).toBe(false); + expect(row.actor_name).toBeUndefined(); + }); +}); diff --git a/packages/app-shell/src/layout/__tests__/inboxGrouping.test.ts b/packages/app-shell/src/layout/__tests__/inboxGrouping.test.ts index 7eade9452..13e4fad1b 100644 --- a/packages/app-shell/src/layout/__tests__/inboxGrouping.test.ts +++ b/packages/app-shell/src/layout/__tests__/inboxGrouping.test.ts @@ -79,3 +79,34 @@ describe('groupNotifications', () => { expect(groupNotifications([])).toEqual([]); }); }); + +describe('objectui#5203 — `actor_name` is not part of InboxNotification', () => { + it('TYPE PIN: re-declaring the field fails this file', () => { + const row: InboxNotification = { + id: 'n1', + type: 'collab.assignment', + title: 'Assigned to you', + // @ts-expect-error objectui#5203 removed `actor_name` from the shape — it + // had no producer (`mergeInboxRows` never mapped it), no consumer, and no + // column on `sys_inbox_message` to map FROM. Re-declaring it makes this + // line compile and fails the pin. + actor_name: 'Li Si', + }; + // Same discipline as the objectui#5190 pin in + // `InboxPopover.linklessFallback.test.tsx`: the assertion above is erased at + // runtime, so `tsc -p tsconfig.test.json` (this package's `type-check` + // script, and the CI `Type Check` job) is what actually checks it. The + // runtime line below only keeps the suite honest about having run. + expect(row.id).toBe('n1'); + }); + + it('groups a row that carries the retired key anyway, ignoring it', () => { + // A stray row from some future producer cannot resurrect the field by + // arriving with it: grouping keys off `(type, title)` and the key is simply + // not part of the contract. Cast because the shape no longer declares it. + const stray = { ...n({ id: 's1' }), actor_name: 'Li Si' } as InboxNotification; + const groups = groupNotifications([stray, n({ id: 's2' })]); + expect(groups).toHaveLength(1); + expect(groups[0].items.map((i) => i.id)).toEqual(['s1', 's2']); + }); +}); diff --git a/packages/app-shell/src/layout/inboxGrouping.ts b/packages/app-shell/src/layout/inboxGrouping.ts index ce616699b..3fc312506 100644 --- a/packages/app-shell/src/layout/inboxGrouping.ts +++ b/packages/app-shell/src/layout/inboxGrouping.ts @@ -13,6 +13,28 @@ * @module */ +/** + * One inbox row as the UI knows it: the shape `mergeInboxRows` + * (`hooks/sharedUserFeeds.ts`) produces, and it is the single producer of every + * row both the bell (`InboxPopover`) and Home's action centre render. + * + * Every member below is mapped by that producer and read by at least one of + * those two consumers, and that agreement is what this interface is for. A + * field declared here but filled by nobody is not documentation — it is a + * standing invitation to wire it up, and two rounds of them have now been + * removed: `source_object`/`source_id` (objectui#5190, see `action_url` below) + * and `actor_name` (objectui#5203). + * + * `actor_name` was dead at BOTH ends — `mergeInboxRows` never mapped it, no + * consumer read it, and `sys_inbox_message` declares no actor column for it to + * be mapped FROM. Beware that the same NAME is alive on unrelated shapes in + * this package: the `sys_activity` -> `ActivityItem` map in + * `hooks/sharedUserFeeds.ts` and the approval activity rows in + * `hooks/useRecordApprovals.ts` both carry a real `actor_name`, so a grep for + * the bare name conflates three different fields. Naming an actor on an inbox + * row is a capability expansion (a column on `sys_inbox_message`, then a + * producer that maps it), not a re-declaration here. + */ export interface InboxNotification { id: string; /** FK → sys_notification (L2 event) — keys the read-state receipt (ADR-0030). */ @@ -38,7 +60,6 @@ export interface InboxNotification { * way, by opening the full inbox. */ action_url?: string | null; - actor_name?: string | null; is_read?: boolean; created_at?: string; }