Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .changeset/inbox-actor-name-retired-5203.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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<string, unknown>;
expect(Object.hasOwn(row, 'actor_name')).toBe(false);
expect(row.actor_name).toBeUndefined();
});
});
31 changes: 31 additions & 0 deletions packages/app-shell/src/layout/__tests__/inboxGrouping.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
});
});
23 changes: 22 additions & 1 deletion packages/app-shell/src/layout/inboxGrouping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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). */
Expand All @@ -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;
}
Expand Down
Loading