diff --git a/.changeset/6730-shared-activity-type-bucket.md b/.changeset/6730-shared-activity-type-bucket.md new file mode 100644 index 0000000000..4f8227d02d --- /dev/null +++ b/.changeset/6730-shared-activity-type-bucket.md @@ -0,0 +1,39 @@ +--- +'@object-ui/app-shell': minor +'@object-ui/i18n': patch +--- + +The shell's `sys_activity.type` reading stops calling every unrecognised type an +update (objectui#6730). + +`mapActivityRows` in `hooks/sharedUserFeeds.ts` — the feed behind the AppHeader +bell's Activity tab, Home's activity card and the exported `ActivityFeed` panel +— carried the third hand-written reading of that column in this repo, and it +bucketed every value outside `created` / `deleted` / `commented` / `mentioned` +as `update`. That is not a missing decision; it is a wrong one stated out loud: +a `scheduled` meeting, a `login`, a nightly `system` rollup and an author's +`contract_countersigned` all rendered as "somebody updated this record". + +- New `layout/activityItemType.ts` holds the whole reading — the table, the + generic bucket, the `"NOW()"` timestamp fallback and the row constructor that + applies all three — DOM-free, so what a row becomes is assertable directly. +- `ActivityItem['type']` gains a fifth kind, `system`: the generic bucket, with + its own icon, label and notification toggle. Following + `UNMAPPED_ACTIVITY_FEED_TYPE`'s precedent, an unrecognised value renders + through it and is named once on `console.warn` rather than being dropped — + `sys_activity.type` is author-extensible (objectstack#11507 direction 4), so + an unmapped value is real activity nobody has ruled on, not a mistake. +- The built-ins that had no honest presentation among the four existing kinds — + `system`, `completed`, `scheduled`, `login`, `logout` — now land in that + bucket instead of claiming `update`. `assigned` and `shared` stay `update`: + both write to the record. + +⛔ The two readings of this column are deliberately NOT converged. +`activityRowToFeedItem` builds a `FeedItem`, and the vocabularies cross: +`FeedItem` collapses create/update/delete into one `field_change` and drops +`commented` / `mentioned` outright, so routing this surface through it would +cost the bell every comment row and every create/delete distinction. What is +shared is a pin, not an import — the new suite reads plugin-detail's real table +(a devDependency; no runtime edge) and fails when the declared vocabulary grows +an entry this side has not read, or when the two readings stop disagreeing in +the three measured ways. diff --git a/packages/app-shell/src/hooks/__tests__/sharedUserFeeds.activityType-6730.test.tsx b/packages/app-shell/src/hooks/__tests__/sharedUserFeeds.activityType-6730.test.tsx new file mode 100644 index 0000000000..e2711d9db4 --- /dev/null +++ b/packages/app-shell/src/hooks/__tests__/sharedUserFeeds.activityType-6730.test.tsx @@ -0,0 +1,149 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectui#6730 — what the bell's Activity tab and Home's card actually + * receive, end to end through the shared feed. + * + * The pure pins live in `layout/__tests__/activityItemType-6730.test.ts`; this + * suite is the other half, and it exists because the defect was never in the + * table — it was in the reading `mapActivityRows` did INLINE around it. A pin + * on a table that the producer does not call is the objectui#5896 failure mode + * (the constructor drifting while the tables agreed), so this file asserts the + * items the hook hands its consumers, not the map. + * + * ## Reverse verification (direction predicted BEFORE running) + * + * - restore the old `: 'update'` catch-all in `mapActivityRows` ⇒ the + * `scheduled` and author-extended cases here go RED, and so does the + * warn-once case (nothing warns) — the pure suite goes red too; + * - keep the catch-all but leave the table in place ⇒ the pure suite stays + * GREEN (the table is fine, nobody reads it) and only this file goes red. + * That asymmetry is why both files are here. + */ +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { act, renderHook } from '@testing-library/react'; + +vi.mock('@object-ui/auth', () => ({ useAuth: () => ({ user: { id: 'u1' } }) })); + +/** + * One row per branch of the reading, plus the two the card names by hand. + * `summary` is non-blank everywhere: a blank one is a DIFFERENT rejection and + * is pinned in the pure suite. + */ +const ACTIVITY_ROWS = [ + { id: 'r1', type: 'created', summary: 'created the lead', object_name: 'crm_lead', + actor_name: 'Li Si', timestamp: '2026-08-20T10:00:00Z' }, + { id: 'r2', type: 'updated', summary: 'changed the stage', object_name: 'crm_lead', + actor_name: 'Li Si', timestamp: '2026-08-20T10:01:00Z' }, + { id: 'r3', type: 'deleted', summary: 'deleted the note', object_name: 'crm_lead', + actor_name: 'Li Si', timestamp: '2026-08-20T10:02:00Z' }, + { id: 'r4', type: 'mentioned', summary: 'mentioned you', object_name: 'crm_lead', + actor_name: 'Li Si', timestamp: '2026-08-20T10:03:00Z' }, + // The value objectui#5878 gave the console record page and never gave this + // surface. HotCRM's `schedule_meeting` action writes it. + { id: 'r5', type: 'scheduled', summary: 'scheduled a meeting', object_name: 'crm_lead', + actor_name: 'Li Si', timestamp: '2026-08-20T10:04:00Z' }, + // An author-extended value under the objectstack#11507 direction-4 ruling. + { id: 'r6', type: 'contract_countersigned', summary: 'countersigned', object_name: 'crm_contract', + actor_name: 'Li Si', timestamp: '2026-08-20T10:05:00Z' }, + // The `"NOW()"` sentinel: plugin-audit writes the unevaluated default + // through on some paths, and `new Date('NOW()')` is `Invalid Date`. + { id: 'r7', type: 'system', summary: 'ran the nightly rollup', object_name: 'crm_lead', + actor_name: 'System', timestamp: 'NOW()', created_at: '2026-08-19T23:00:00Z' }, +]; + +const fakeAdapter = { + find: (object: string) => + object === 'sys_activity' + ? Promise.resolve({ data: ACTIVITY_ROWS }) + : Promise.resolve({ data: [] }), + getClient: () => undefined, +}; +vi.mock('../../providers/AdapterProvider', () => ({ useAdapter: () => fakeAdapter })); + +import { useSharedActivityFeed, __resetSharedUserFeeds } from '../sharedUserFeeds'; +import { resetUnmappedActivityTypeWarnings } from '../../layout/activityItemType'; + +const settle = () => act(async () => { await vi.advanceTimersByTimeAsync(0); }); + +/** Every `console.warn` this suite provokes, as text — a typed array rather + * than a spy handle so the assertions read as the messages they are. */ +const warnings: string[] = []; + +beforeEach(() => { + vi.useFakeTimers(); + __resetSharedUserFeeds(); + resetUnmappedActivityTypeWarnings(); + warnings.length = 0; + vi.spyOn(console, 'warn').mockImplementation((...args: unknown[]) => { + warnings.push(args.map((a) => String(a)).join(' ')); + }); + vi.stubGlobal('fetch', vi.fn(() => Promise.resolve(new Response('{}', { status: 404 })))); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + vi.useRealTimers(); +}); + +/** id -> type, so a failure names the row rather than an array index. */ +async function typesByRow(): Promise> { + const { result } = renderHook(() => useSharedActivityFeed()); + await settle(); + return Object.fromEntries(result.current.map((a) => [a.id, a.type])); +} + +describe('objectui#6730 — the shared activity feed no longer calls everything an update', () => { + it('gives each row the presentation its type earns', async () => { + expect(await typesByRow()).toEqual({ + r1: 'create', + r2: 'update', + r3: 'delete', + r4: 'comment', + // Both of these were `update` before this PR — the silent widening the + // card is filed for. A scheduled meeting is not a record update, and + // neither is an author's countersignature. + r5: 'system', + r6: 'system', + r7: 'system', + }); + }); + + it('keeps the unrecognised row rather than dropping it, and says so once', async () => { + const { result } = renderHook(() => useSharedActivityFeed()); + await settle(); + + // A bucket, not a drop: every row that named an action and said something + // still reaches the surface. (objectui#5840's failure mode was the drop.) + expect(result.current.map((a) => a.id)).toEqual(ACTIVITY_ROWS.map((r) => r.id)); + + const unmapped = result.current.find((a) => a.id === 'r6'); + expect(unmapped?.description).toBe('countersigned'); + expect(unmapped?.objectName).toBe('crm_contract'); + + // One diagnostic, for the one value nobody has ruled on — not for + // `scheduled` or `system`, which the table maps on purpose. + const named = warnings.filter((m) => m.includes('sys_activity row with type')); + expect(named).toHaveLength(1); + expect(named[0]).toContain('contract_countersigned'); + expect(named[0]).not.toContain('"scheduled"'); + }); + + it('resolves the `"NOW()"` sentinel to `created_at` on this surface', async () => { + const { result } = renderHook(() => useSharedActivityFeed()); + await settle(); + + const nowRow = result.current.find((a) => a.id === 'r7'); + // The fold is behaviour-preserving: the same answer the inline copy gave, + // now produced by the one reading this package owns. Pinned against + // plugin-detail's folded copy value-for-value in the pure suite. + expect(nowRow?.timestamp).toBe('2026-08-19T23:00:00Z'); + expect(nowRow?.timestamp).not.toBe('NOW()'); + + // Unchanged rows keep their own timestamp — the fallback is a fallback. + expect(result.current.find((a) => a.id === 'r1')?.timestamp).toBe('2026-08-20T10:00:00Z'); + }); +}); diff --git a/packages/app-shell/src/hooks/sharedUserFeeds.ts b/packages/app-shell/src/hooks/sharedUserFeeds.ts index b98ea22867..687fd01255 100644 --- a/packages/app-shell/src/hooks/sharedUserFeeds.ts +++ b/packages/app-shell/src/hooks/sharedUserFeeds.ts @@ -53,6 +53,7 @@ import { errorCodeIs } from '@object-ui/types'; import { useAdapter } from '../providers/AdapterProvider.js'; import { bearerAuthHeaders } from '../utils/authToken.js'; import type { ActivityItem } from '../layout/ActivityFeed.js'; +import { activityRowToActivityItem } from '../layout/activityItemType.js'; import type { InboxNotification } from '../layout/inboxGrouping.js'; /** Approvals poll cadence — the bell's original 30s (M11.C15). */ @@ -566,45 +567,30 @@ function adapterKey(adapter: unknown): string | null { } /** - * Raw `sys_activity` rows carry plugin-audit's column names - * (`summary` / `actor_name` / `object_name` / `timestamp`); casting them - * straight through leaves every `ActivityItem` field undefined, which is what - * once rendered the Activity tab as blank rows showing only a relative time. + * Raw `sys_activity` rows -> `ActivityItem`s: rows that name an action and say + * something. Home narrows it further (human actors only) at its own call site. * - * This is the shared superset: rows that name an action and say something. - * Home narrows it further (human actors only) at its own call site. + * The reading itself moved to `layout/activityItemType.ts` (objectui#6730). + * What lived here was the THIRD hand-written reading of `sys_activity.type` in + * this repo — objectui#5878 shared the table between the `record:activity` + * block and `RecordDetailView`, objectui#5896 shared the constructor around it, + * and this copy survived both — plus its own copy of the `"NOW()"` timestamp + * quirk whose two others #5896 folded into one. + * + * ⛔ It did NOT become a call to `activityRowToFeedItem`, and the module it + * moved to explains at length why not: the target types CROSS. `FeedItem` + * collapses create/update/delete into one `field_change`, and drops + * `commented` / `mentioned` outright — so routing this surface through the + * shared constructor would cost the bell every comment row and every + * distinction between a create and a delete. What is shared instead is a PIN, + * not an import: `activityItemType-6730.test.ts` reads plugin-detail's real + * table (devDependency, no runtime edge) and fails when the column's declared + * vocabulary grows an entry this side has not read. */ function mapActivityRows(rows: unknown[]): ActivityItem[] { return rows - .filter((row): row is Record => { - if (!row || typeof row !== 'object') return false; - const r = row as Record; - return typeof r.type === 'string' && String(r.summary ?? '').trim().length > 0; - }) - .map((r) => { - let when = r.timestamp as string | undefined; - if (!when || when === 'NOW()' || Number.isNaN(Date.parse(when))) { - when = r.created_at as string | undefined; - } - const raw = String(r.type); - const type: ActivityItem['type'] = - raw === 'commented' || raw === 'mentioned' - ? 'comment' - : raw === 'deleted' - ? 'delete' - : raw === 'created' - ? 'create' - : 'update'; - return { - id: String(r.id), - type, - objectName: String(r.object_name ?? ''), - recordId: r.record_id != null ? String(r.record_id) : undefined, - user: String(r.actor_name ?? ''), - description: String(r.summary ?? ''), - timestamp: when ?? '', - }; - }); + .map((row) => activityRowToActivityItem(row)) + .filter((item): item is ActivityItem => item !== null); } /** diff --git a/packages/app-shell/src/layout/ActivityFeed.tsx b/packages/app-shell/src/layout/ActivityFeed.tsx index 3efa1727d7..2cad4117d7 100644 --- a/packages/app-shell/src/layout/ActivityFeed.tsx +++ b/packages/app-shell/src/layout/ActivityFeed.tsx @@ -17,18 +17,18 @@ import { SheetTitle, SheetTrigger, } from '@object-ui/components'; -import { Activity, Plus, Pencil, Trash2, MessageSquare, Filter } from 'lucide-react'; +import { Activity, Plus, Pencil, Trash2, MessageSquare, Filter, Info } from 'lucide-react'; import { useObjectTranslation } from '@object-ui/i18n'; +import type { ActivityItem } from './activityItemType.js'; -export interface ActivityItem { - id: string; - type: 'create' | 'update' | 'delete' | 'comment'; - objectName: string; - recordId?: string; - user: string; - description: string; - timestamp: string; -} +/** + * The item shape and its kind union live in `activityItemType.ts` with the + * `sys_activity` reading that produces them (objectui#6730) — that module is + * DOM-free, so what a row BECOMES can be asserted without mounting this Sheet. + * Re-exported here so every existing `from './ActivityFeed.js'` import (and the + * package barrel's `ActivityItem`) keeps resolving unchanged. + */ +export type { ActivityItem, ActivityItemType } from './activityItemType.js'; export interface ActivityFeedProps { activities?: ActivityItem[]; @@ -43,6 +43,11 @@ const typeConfig: Record< update: { icon: Pencil, color: 'text-blue-500' }, delete: { icon: Trash2, color: 'text-red-500' }, comment: { icon: MessageSquare, color: 'text-amber-500' }, + // The generic bucket (objectui#6730): built-ins these four kinds have no + // honest presentation for (`system` / `completed` / `scheduled` / `login` / + // `logout`) plus every author-extended value. Neutral on purpose — the point + // of the bucket is that it does not claim the row was an update. + system: { icon: Info, color: 'text-muted-foreground' }, }; /** Format an ISO timestamp as a localized relative string (e.g. "2m ago"). */ @@ -69,6 +74,7 @@ export function ActivityFeed({ activities = [], className }: ActivityFeedProps) update: true, delete: true, comment: true, + system: true, }); const togglePreference = (type: ActivityItem['type']) => { @@ -83,6 +89,7 @@ export function ActivityFeed({ activities = [], className }: ActivityFeedProps) update: t('layout.activityFeed.typeUpdate'), delete: t('layout.activityFeed.typeDelete'), comment: t('layout.activityFeed.typeComment'), + system: t('layout.activityFeed.typeSystem'), }; return ( diff --git a/packages/app-shell/src/layout/__tests__/activityItemType-6730.test.ts b/packages/app-shell/src/layout/__tests__/activityItemType-6730.test.ts new file mode 100644 index 0000000000..045493aad1 --- /dev/null +++ b/packages/app-shell/src/layout/__tests__/activityItemType-6730.test.ts @@ -0,0 +1,267 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectui#6730 — the shell's `sys_activity.type` reading, pinned; and the + * reason it is NOT the same reading `record:activity` uses. + * + * ## What this suite is for + * + * The card is a DRIFT card, not a bug report: before this PR nothing on either + * side of the repo failed when the two hand-written readings of one column + * disagreed, and they already had (`scheduled` reached the console record page + * after objectui#5878 and never reached the bell). A second table is only safe + * if something red-flags the disagreement, so this file is the instrument that + * makes two vocabularies a DECISION rather than an accident: + * + * 1. the shell's table is TOTAL over the column's declared vocabulary, read + * from plugin-detail's real `ACTIVITY_TYPE_TO_FEED_TYPE` rather than from a + * hand-copied list — a new built-in upstream turns this red; + * 2. the two readings DISAGREE in three specific, measured ways, so a future + * "just call `activityRowToFeedItem`" cleanup turns this red and reads why; + * 3. the unrecognised case is explicit and is not `update`; + * 4. the third copy of the `"NOW()"` timestamp quirk agrees with the folded + * copy objectui#5896 produced, value for value. + * + * ## Resolution note (why no build is needed to run this) + * + * `vitest.config.mts` aliases `@object-ui/plugin-detail` to + * `packages/plugin-detail/src`, so both imports below read SOURCE, not `dist`. + * `activityTimestamp` is not on plugin-detail's barrel (objectui#5896 published + * the whole `FeedItem` reading on purpose, not its pieces) so it is reached by + * the same relative-src shape another cross-package pin in this package already + * uses (`anonSeedScope-5746.enumeration.test.tsx` -> `auth/src`). A test-only + * edge: this is a devDependency, and no runtime import of plugin-detail exists + * in the shell's header chrome — which is the whole point of pinning instead of + * importing. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { ACTIVITY_TYPE_TO_FEED_TYPE } from '@object-ui/plugin-detail'; +import type { SysActivityRow } from '@object-ui/plugin-detail'; +import { activityTimestamp } from '../../../../plugin-detail/src/renderers/recordActivityFeed'; +import { + ACTIVITY_TYPE_TO_ACTIVITY_ITEM_TYPE, + UNMAPPED_ACTIVITY_ITEM_TYPE, + activityItemTypeOf, + activityRowTimestamp, + activityRowToActivityItem, + resetUnmappedActivityTypeWarnings, +} from '../activityItemType'; +import type { ActivityItemType } from '../activityItemType'; + +/** + * The `unit` project runs with `isolate: false`, so this module's warn-once + * bucket is shared with every other file in the worker. Clearing it per test is + * what makes "warns once" assertable at all. + */ +beforeEach(() => { + resetUnmappedActivityTypeWarnings(); +}); +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('the mapping is pinned for every type the column can carry', () => { + /** + * The whole table, spelled out. Not a loop over the source object — that + * would pass whatever the source says. Changing any line here is meant to be + * a decision somebody wrote down. + */ + it('maps every declared value explicitly', () => { + expect(ACTIVITY_TYPE_TO_ACTIVITY_ITEM_TYPE).toEqual({ + // The record's own stored state changed, split by which way. This is + // plugin-detail's `field_change` group, refined. + created: 'create', + updated: 'update', + deleted: 'delete', + assigned: 'update', + shared: 'update', + // Somebody said something. plugin-detail drops these on purpose. + commented: 'comment', + mentioned: 'comment', + // No honest create/update/delete/comment presentation exists for these. + // Every one of them claimed `update` before objectui#6730. + system: 'system', + completed: 'system', + scheduled: 'system', + login: 'system', + logout: 'system', + }); + }); + + /** + * The superset pin (`map ⊇ built-ins`), measured against the other reading's + * real table rather than a copied list — the same shape objectui#5969 landed + * on for that side. It is deliberately ONE-directional: `sys_activity.type` + * is author-extensible (objectstack#11507 direction 4, ruled 2026-08-24), so + * an equality pin would be false by construction. + */ + it('has an entry for every value the platform declares', () => { + const builtIns = Object.keys(ACTIVITY_TYPE_TO_FEED_TYPE); + // The control: the upstream table is non-empty and reachable, so an empty + // `missing` below is an answer rather than an artefact of a failed import. + expect(builtIns.length).toBeGreaterThan(8); + expect(builtIns).toContain('scheduled'); + + const missing = builtIns.filter( + (t) => !Object.prototype.hasOwnProperty.call(ACTIVITY_TYPE_TO_ACTIVITY_ITEM_TYPE, t), + ); + expect(missing).toEqual([]); + }); + + it('every entry resolves through the reading, and `scheduled` is no longer an update', () => { + for (const [raw, expected] of Object.entries(ACTIVITY_TYPE_TO_ACTIVITY_ITEM_TYPE)) { + expect(activityItemTypeOf(raw)).toBe(expected); + } + // The value objectui#5878 fixed on the console record page and left broken + // here — the card's second finding, pinned by name. + expect(activityItemTypeOf('scheduled')).not.toBe('update'); + }); +}); + +describe('the unrecognised case is explicit, and it is a bucket rather than a claim', () => { + it('renders an author-extended value through the generic bucket', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + expect(activityItemTypeOf('contract_countersigned')).toBe(UNMAPPED_ACTIVITY_ITEM_TYPE); + // The regression this card is about: it used to be a specific, wrong claim. + expect(activityItemTypeOf('contract_countersigned')).not.toBe('update'); + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0]?.[0])).toContain('contract_countersigned'); + }); + + it('is a bucket, NOT a drop — the row still reaches the surface', () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + const item = activityRowToActivityItem({ + id: 'a1', + type: 'contract_countersigned', + summary: 'Countersigned by Legal', + object_name: 'crm_contract', + actor_name: 'Zhang San', + timestamp: '2026-08-20T10:00:00Z', + }); + expect(item).not.toBeNull(); + expect(item?.type).toBe(UNMAPPED_ACTIVITY_ITEM_TYPE); + expect(item?.description).toBe('Countersigned by Legal'); + }); + + it('warns ONCE per distinct value, and never for a value the table maps', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + for (let i = 0; i < 5; i++) activityItemTypeOf('teleported'); + expect(warn).toHaveBeenCalledTimes(1); + + activityItemTypeOf('beamed_up'); + expect(warn).toHaveBeenCalledTimes(2); + + // `system` produces the SAME value as the fallback and is still a decision, + // so it must stay silent. This is why the reading asks `hasOwnProperty` + // instead of comparing its result to the bucket. + activityItemTypeOf('system'); + activityItemTypeOf('scheduled'); + expect(warn).toHaveBeenCalledTimes(2); + }); +}); + +describe('⛔ the two readings CROSS — this is why they are not converged', () => { + /** + * Each assertion below is one half of the answer to the card's first + * question. If a future change makes any of them false, converging the two + * surfaces may be back on the table — but it is a decision, taken here. + */ + it('`FeedItem` is coarser: create/update/delete all collapse to one feed type', () => { + const collapsed = ['created', 'updated', 'deleted', 'assigned', 'shared']; + const feedTypes = new Set(collapsed.map((t) => ACTIVITY_TYPE_TO_FEED_TYPE[t])); + expect(feedTypes).toEqual(new Set(['field_change'])); + + // …while this vocabulary splits the same five three ways. Routing through + // `activityRowToFeedItem` would make every create and every delete arrive + // as an update, because `field_change` cannot be decomposed. + const itemTypes = new Set(collapsed.map((t) => ACTIVITY_TYPE_TO_ACTIVITY_ITEM_TYPE[t])); + expect(itemTypes).toEqual(new Set(['create', 'update', 'delete'])); + }); + + it('`FeedItem` drops what this vocabulary names: comments', () => { + // A deliberate `undefined` on the other side — that content lives in + // `sys_comment`. `activityRowToFeedItem` returns null for these rows. + expect(ACTIVITY_TYPE_TO_FEED_TYPE.commented).toBeUndefined(); + expect(ACTIVITY_TYPE_TO_FEED_TYPE.mentioned).toBeUndefined(); + // Here they are one of the four presentation kinds. Converging would cost + // the bell's Activity tab every comment row. + expect(activityItemTypeOf('commented')).toBe('comment'); + expect(activityItemTypeOf('mentioned')).toBe('comment'); + }); + + it('`FeedItem` is finer where this vocabulary is coarse', () => { + const finer = ['system', 'completed', 'scheduled']; + expect(new Set(finer.map((t) => ACTIVITY_TYPE_TO_FEED_TYPE[t])).size).toBe(3); + expect(new Set(finer.map((t) => ACTIVITY_TYPE_TO_ACTIVITY_ITEM_TYPE[t])).size).toBe(1); + }); +}); + +describe('the `"NOW()"` quirk agrees with the copy objectui#5896 folded', () => { + /** + * The whole input table of the quirk, asserted against BOTH implementations. + * Two copies of a five-line predicate are only safe while something says they + * still agree; there is no package that owns "how to read a `sys_activity` + * column" for both a widget plugin and the shell's chrome, so this pin is the + * instrument until there is one. + */ + const CASES: readonly SysActivityRow[] = [ + { timestamp: '2026-08-20T10:00:00Z', created_at: '2026-01-01T00:00:00Z' }, + { timestamp: 'NOW()', created_at: '2026-01-01T00:00:00Z' }, + { timestamp: 'NOW()', created_at: null }, + { timestamp: '', created_at: '2026-01-01T00:00:00Z' }, + { timestamp: null, created_at: '2026-01-01T00:00:00Z' }, + { timestamp: undefined, created_at: '2026-01-01T00:00:00Z' }, + { timestamp: 'not a date', created_at: '2026-01-01T00:00:00Z' }, + { timestamp: undefined, created_at: undefined }, + { timestamp: 'NOW()', created_at: undefined }, + ]; + + it('produces the same string as `activityTimestamp` for every input', () => { + for (const row of CASES) { + expect(activityRowTimestamp(row)).toBe(activityTimestamp(row)); + } + }); + + it('resolves the sentinel to `created_at`, and a real timestamp to itself', () => { + // Stated independently so this suite still says what the behaviour IS if + // both copies ever drift together. + expect(activityRowTimestamp({ timestamp: 'NOW()', created_at: '2026-01-01T00:00:00Z' })) + .toBe('2026-01-01T00:00:00Z'); + expect(activityRowTimestamp({ timestamp: '2026-08-20T10:00:00Z', created_at: 'x' })) + .toBe('2026-08-20T10:00:00Z'); + expect(activityRowTimestamp({ timestamp: 'NOW()' })).toBe(''); + }); +}); + +describe('the row constructor keeps what `mapActivityRows` used to do inline', () => { + it('maps plugin-audit column names onto the item shape', () => { + expect( + activityRowToActivityItem({ + id: 7, + type: 'created', + summary: 'Created the lead', + object_name: 'crm_lead', + record_id: 42, + actor_name: 'Li Si', + timestamp: '2026-08-20T10:00:00Z', + }), + ).toEqual({ + id: '7', + type: 'create', + objectName: 'crm_lead', + recordId: '42', + user: 'Li Si', + description: 'Created the lead', + timestamp: '2026-08-20T10:00:00Z', + }); + }); + + it('rejects rows that name no action or say nothing', () => { + expect(activityRowToActivityItem(null)).toBeNull(); + expect(activityRowToActivityItem('nope')).toBeNull(); + expect(activityRowToActivityItem({ id: '1', summary: 'no type' })).toBeNull(); + expect(activityRowToActivityItem({ id: '1', type: 'created' })).toBeNull(); + expect(activityRowToActivityItem({ id: '1', type: 'created', summary: ' ' })).toBeNull(); + }); +}); diff --git a/packages/app-shell/src/layout/activityItemType.ts b/packages/app-shell/src/layout/activityItemType.ts new file mode 100644 index 0000000000..3eb9d3dd29 --- /dev/null +++ b/packages/app-shell/src/layout/activityItemType.ts @@ -0,0 +1,272 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * The app-shell reading of a `sys_activity` row — the pure half (objectui#6730). + * + * `sys_activity` is read by two packages with two DIFFERENT target types: + * + * - `@object-ui/plugin-detail` builds a `FeedItem` (the closed 13-value + * `FeedItemType` spec enum) for the `record:activity` block and, through + * `activityRowToFeedItem`, for `RecordDetailView`'s merged feed; + * - this package builds an {@link ActivityItem} for the AppHeader bell's + * Activity tab, Home's activity card and the exported `ActivityFeed` panel. + * + * Everything here is DOM-free on purpose, exactly as `recordActivityFeed.ts` + * is on the other side: the reading is the part worth asserting directly, and + * a test that has to mount a Sheet to find out what a `scheduled` row becomes + * is a test nobody writes. + * + * ## ⛔ These two vocabularies CROSS. Do not "converge" them (objectui#6730) + * + * The obvious cleanup — have this surface call `activityRowToFeedItem` and be + * done — is wrong, and it is wrong in a way that costs rows. Measured against + * `ACTIVITY_TYPE_TO_FEED_TYPE` as it stands: + * + * 1. **`FeedItem` is COARSER where `ActivityItem` is fine.** `created`, + * `updated`, `deleted`, `assigned` and `shared` all map to the single + * `field_change` feed type. This vocabulary splits that group three ways — + * `create` / `update` / `delete` — with three icons, three labels and three + * independent notification toggles. `field_change` cannot be decomposed + * back into them, so `sys_activity.type -> FeedItemType -> ActivityItem` + * is lossy: every create and every delete would arrive as an update. + * 2. **`FeedItem` DROPS what `ActivityItem` names.** `commented` and + * `mentioned` map to `undefined` there — a deliberate exclusion, because + * that content lives in `sys_comment` with reactions and threading + * attached. Here they are the `comment` kind, one of four. Routing through + * the shared constructor returns `null` for them, i.e. the bell's Activity + * tab would silently lose every comment row. + * 3. **`FeedItem` is FINER where this vocabulary is coarse.** `system`, + * `task` and `event` are three feed types; here they land in one bucket. + * + * So neither type is a projection of the other: each refines the other + * somewhere and coarsens it somewhere else. A shared reading would have to be a + * third table keyed on `sys_activity.type` with two value columns, which is not + * a convergence — it is the same two decisions written next to each other, plus + * a cross-package runtime dependency from the shell's header chrome onto a + * record-detail widget plugin. + * + * What IS shared is pinned by tests rather than by imports, which is the whole + * point: `activityItemType-6730.test.ts` reads plugin-detail's real + * `ACTIVITY_TYPE_TO_FEED_TYPE` (a devDependency — no runtime edge) and asserts + * that every value the column is DECLARED to carry has an entry here too, and + * that the two readings still disagree in the three ways above. A new built-in + * upstream turns that pin red; a future "convergence" turns it red as well, + * and the message says why. + * + * ## The unrecognised case is explicit, and it is not `update` + * + * Before objectui#6730 every value outside the four named ones fell through to + * `update`. That is not a missing decision, it is a WRONG one stated out loud: + * a `scheduled` meeting and an author's `contract_countersigned` both rendered + * as "somebody updated this record". `sys_activity.type` is author-extensible + * (objectstack#11507 direction 4, ruled 2026-08-24 — the column's fields are + * `readonly: true` so objectql never validates them on write, and ADR-0052 + * §5b.2 forwards `activityMilestones[].type` into it verbatim), so unrecognised + * values are not mistakes to be papered over; they are real activity nobody has + * ruled on yet. + * + * The in-repo precedent for that is `UNMAPPED_ACTIVITY_FEED_TYPE`: a generic + * bucket plus one diagnostic per distinct value. This module follows it. + */ + +/** + * What the shell's activity surfaces can present a row AS. + * + * Four presentation kinds plus a generic bucket. Adding a member is a real + * cost — `ActivityFeed` keeps three `Record` tables + * (icon, label, notification toggle), so `tsc` refuses a member without a + * presentation, which is the property that makes the bucket safe to add. + * + * ⚠️ `system` shares a SPELLING with `UNMAPPED_ACTIVITY_FEED_TYPE` and nothing + * else. This union is not, and must not become, a projection of the spec's + * `FeedItemType` — that would bind an app-shell internal type to a published + * enum, which is a contract decision this module has no standing to make. + */ +export type ActivityItemType = 'create' | 'update' | 'delete' | 'comment' | 'system'; + +/** + * One activity row as the shell's surfaces consume it. + * + * Declared here rather than beside the `ActivityFeed` component so the reading + * above stays importable without pulling React in; `ActivityFeed` re-exports + * it, so every existing `import type { ActivityItem } from './ActivityFeed.js'` + * keeps resolving. + */ +export interface ActivityItem { + id: string; + type: ActivityItemType; + objectName: string; + recordId?: string; + user: string; + description: string; + timestamp: string; +} + +/** + * `sys_activity.type` -> {@link ActivityItemType}. + * + * The rule, so that a new entry is a reading rather than a guess: + * + * - `create` / `update` / `delete` mean **the record's own stored state + * changed**, split by which way. That is exactly plugin-detail's + * `field_change` group, refined — `assigned` and `shared` are updates + * because both write to the record (owner, sharing rules), which is also + * why they sit in that group on the other side. + * - `comment` means **somebody said something**. `mentioned` is a comment that + * named you; the shell has no separate presentation for that and does not + * need one. + * - `system` is the generic bucket: a value this four-kind vocabulary has no + * honest kind for. `completed`, `scheduled`, `login` and `logout` are here + * on purpose — plugin-detail gives them `task` / `event` / dropped, and none + * of those is a create, an update, a delete or a comment. Before #6730 all + * four claimed `update`. + * + * The table is TOTAL over the built-in vocabulary (`map ⊇ built-ins`), pinned + * against plugin-detail's table by `activityItemType-6730.test.ts`. It is + * deliberately NOT set-equal to it: the column is author-extensible, so an + * equality pin would be false by construction — the same reading objectui#5840 + * and objectui#5969 landed on for the other copy. + */ +export const ACTIVITY_TYPE_TO_ACTIVITY_ITEM_TYPE: Readonly> = { + created: 'create', + updated: 'update', + deleted: 'delete', + assigned: 'update', + shared: 'update', + commented: 'comment', + mentioned: 'comment', + system: 'system', + completed: 'system', + scheduled: 'system', + login: 'system', + logout: 'system', +}; + +/** + * The presentation a value outside {@link ACTIVITY_TYPE_TO_ACTIVITY_ITEM_TYPE} + * renders through. + * + * ⚠️ A FLOOR under the table, never a substitute for it — the same caveat + * `UNMAPPED_ACTIVITY_FEED_TYPE` carries. What keeps a catch-all honest is that + * it cannot swallow a value somebody has ruled on: the superset pin forces + * every built-in to keep its own entry above, so this can only ever receive + * values nobody has mapped, and {@link activityRowToActivityItem} names each + * one once so somebody can. + * + * Note it is a bucket, not a drop. Dropping the row is the objectui#5840 + * failure mode — stored, queryable, invisible — reached by a different route. + */ +export const UNMAPPED_ACTIVITY_ITEM_TYPE: ActivityItemType = 'system'; + +/** `sys_activity.type` values already named as unmapped. Module scope so one + * unknown type warns ONCE, not once per row: a 20-row page of the same + * extended type is one missing decision, not twenty. */ +const warnedUnmappedActivityTypes = new Set(); + +/** Test seam: forget which unmapped types have already been named. */ +export function resetUnmappedActivityTypeWarnings(): void { + warnedUnmappedActivityTypes.clear(); +} + +/** + * Say out loud that a row reached the shell's feed through the generic bucket. + * + * Deliberately NOT fired for a value the table maps to `system` on purpose + * (`system`, `completed`, `scheduled`, `login`, `logout`): those are decisions, + * and warning about a decision teaches authors to ignore the channel. It fires + * only for a value outside the table entirely — which is why the lookup below + * asks `hasOwnProperty` rather than comparing the result to the bucket. + */ +function warnUnmappedActivityType(type: string): void { + if (warnedUnmappedActivityTypes.has(type)) return; + warnedUnmappedActivityTypes.add(type); + console.warn( + `[app-shell] rendered a sys_activity row with type "${type}" through the generic ` + + `"${UNMAPPED_ACTIVITY_ITEM_TYPE}" presentation: no activity item type is mapped ` + + 'for it. `sys_activity.type` is author-extensible (objectstack#11507, ruled ' + + '2026-08-24) and is not validated on write, so a producer can store a value the ' + + 'platform never declared — the row is shown rather than dropped, and it no longer ' + + 'claims to be an update. Map it in ACTIVITY_TYPE_TO_ACTIVITY_ITEM_TYPE ' + + '(@object-ui/app-shell, layout/activityItemType.ts) to give it its own ' + + 'presentation.', + ); +} + +/** + * Read `sys_activity.type` as an {@link ActivityItemType}. + * + * Three outcomes and only two spellings, which is why the diagnostic hangs off + * the lookup and not off the result: a MAPPED `system` and an UNMAPPED value + * both produce `'system'`, and only the second one is missing a decision. + */ +export function activityItemTypeOf(rawType: string): ActivityItemType { + const mapped = Object.prototype.hasOwnProperty.call(ACTIVITY_TYPE_TO_ACTIVITY_ITEM_TYPE, rawType) + ? ACTIVITY_TYPE_TO_ACTIVITY_ITEM_TYPE[rawType] + : undefined; + if (mapped) return mapped; + warnUnmappedActivityType(rawType); + return UNMAPPED_ACTIVITY_ITEM_TYPE; +} + +/** + * `timestamp`, falling back to `created_at` when the column holds the literal + * `"NOW()"` — plugin-audit writes the unevaluated default through on some + * paths, and `new Date('NOW()')` is `Invalid Date`, which renders as a blank + * relative time. + * + * This is the THIRD copy of that quirk in the repo; objectui#5896 folded the + * other two into `activityTimestamp` (@object-ui/plugin-detail). It stays a + * copy rather than an import because the quirk is the only part of the reading + * that is target-type-independent, and importing it would put a runtime edge + * from the shell's header chrome onto a record-detail widget plugin for one + * five-line predicate — plugin-detail is a PEER dependency of this package, so + * that edge is a real install-time requirement, not a free one. There is no + * package that owns "how to read a `sys_activity` column" today; until there + * is, the honest instrument is a pin, and + * `activityItemType-6730.test.ts` asserts this function agrees with + * `activityTimestamp` value-for-value over the quirk's whole input table. + */ +export function activityRowTimestamp(row: { + timestamp?: unknown; + created_at?: unknown; +}): string { + const when = row.timestamp; + if (!when || when === 'NOW()' || Number.isNaN(Date.parse(String(when)))) { + return String(row.created_at ?? ''); + } + return String(when); +} + +/** + * One raw `sys_activity` row -> one {@link ActivityItem}, or `null` when the + * row is not something these surfaces can show. + * + * `null` covers exactly what `mapActivityRows` used to drop with a `.filter()` + * ahead of its `.map()`: a row that names no action (`type` is not a string) or + * says nothing (`summary` blank). Raw rows carry plugin-audit's column names + * (`summary` / `actor_name` / `object_name` / `timestamp`); casting one straight + * through leaves every field `undefined`, which is what once rendered the + * Activity tab as blank rows showing only a relative time. + * + * Exported as the whole reading — table, floor, timestamp and the constructor + * that applies all three — for the reason objectui#5896 gave on the other side: + * publishing the lookup table alone left the mirror one level up, and the + * constructions drifted where the tables did not. + */ +export function activityRowToActivityItem(row: unknown): ActivityItem | null { + if (!row || typeof row !== 'object') return null; + const r = row as Record; + if (typeof r.type !== 'string') return null; + const description = String(r.summary ?? '').trim(); + if (description.length === 0) return null; + return { + id: String(r.id), + type: activityItemTypeOf(r.type), + objectName: String(r.object_name ?? ''), + recordId: r.record_id != null ? String(r.record_id) : undefined, + user: String(r.actor_name ?? ''), + description: String(r.summary ?? ''), + timestamp: activityRowTimestamp(r), + }; +} diff --git a/packages/i18n/src/locales/ar.ts b/packages/i18n/src/locales/ar.ts index 0e2750b86c..00a0ff4ab8 100644 --- a/packages/i18n/src/locales/ar.ts +++ b/packages/i18n/src/locales/ar.ts @@ -2349,6 +2349,7 @@ const ar = { typeUpdate: "تحديث", typeDelete: "حذف", typeComment: "تعليق", + typeSystem: "النظام", relativeJustNow: "الآن", relativeSecondsAgo: "منذ {{count}} ث", relativeMinutesAgo: "منذ {{count}} د", diff --git a/packages/i18n/src/locales/de.ts b/packages/i18n/src/locales/de.ts index 8384cadc1b..2b84ba6637 100644 --- a/packages/i18n/src/locales/de.ts +++ b/packages/i18n/src/locales/de.ts @@ -2342,6 +2342,7 @@ const de = { typeUpdate: "Aktualisieren", typeDelete: "Löschen", typeComment: "Kommentar", + typeSystem: "System", relativeJustNow: "gerade eben", relativeSecondsAgo: "vor {{count}}s", relativeMinutesAgo: "vor {{count}}m", diff --git a/packages/i18n/src/locales/en.ts b/packages/i18n/src/locales/en.ts index 09c23dbb77..384a39e038 100644 --- a/packages/i18n/src/locales/en.ts +++ b/packages/i18n/src/locales/en.ts @@ -2589,6 +2589,7 @@ const en = { typeUpdate: 'Update', typeDelete: 'Delete', typeComment: 'Comment', + typeSystem: 'System', relativeJustNow: 'just now', relativeSecondsAgo: '{{count}}s ago', relativeMinutesAgo: '{{count}}m ago', diff --git a/packages/i18n/src/locales/es.ts b/packages/i18n/src/locales/es.ts index 37fbba5ad2..46bcc40dd0 100644 --- a/packages/i18n/src/locales/es.ts +++ b/packages/i18n/src/locales/es.ts @@ -2346,6 +2346,7 @@ const es = { typeUpdate: "Actualizar", typeDelete: "Eliminar", typeComment: "Comentario", + typeSystem: "Sistema", relativeJustNow: "ahora mismo", relativeSecondsAgo: "hace {{count}}s", relativeMinutesAgo: "hace {{count}}m", diff --git a/packages/i18n/src/locales/fr.ts b/packages/i18n/src/locales/fr.ts index a6293c053d..e57f51eead 100644 --- a/packages/i18n/src/locales/fr.ts +++ b/packages/i18n/src/locales/fr.ts @@ -2344,6 +2344,7 @@ const fr = { typeUpdate: "Mettre à jour", typeDelete: "Supprimer", typeComment: "Commentaire", + typeSystem: "Système", relativeJustNow: "à l'instant", relativeSecondsAgo: "il y a {{count}}s", relativeMinutesAgo: "il y a {{count}}m", diff --git a/packages/i18n/src/locales/ja.ts b/packages/i18n/src/locales/ja.ts index 0f9c178871..ba5edc3e58 100644 --- a/packages/i18n/src/locales/ja.ts +++ b/packages/i18n/src/locales/ja.ts @@ -2344,6 +2344,7 @@ const ja = { typeUpdate: "更新", typeDelete: "削除", typeComment: "コメント", + typeSystem: "システム", relativeJustNow: "たった今", relativeSecondsAgo: "{{count}}秒前", relativeMinutesAgo: "{{count}}分前", diff --git a/packages/i18n/src/locales/ko.ts b/packages/i18n/src/locales/ko.ts index 283a888b6a..addd5edfc7 100644 --- a/packages/i18n/src/locales/ko.ts +++ b/packages/i18n/src/locales/ko.ts @@ -2341,6 +2341,7 @@ const ko = { typeUpdate: "업데이트", typeDelete: "삭제", typeComment: "댓글", + typeSystem: "시스템", relativeJustNow: "방금 전", relativeSecondsAgo: "{{count}}초 전", relativeMinutesAgo: "{{count}}분 전", diff --git a/packages/i18n/src/locales/pt.ts b/packages/i18n/src/locales/pt.ts index 40ab35546e..b63521268d 100644 --- a/packages/i18n/src/locales/pt.ts +++ b/packages/i18n/src/locales/pt.ts @@ -2341,6 +2341,7 @@ const pt = { typeUpdate: "Atualizar", typeDelete: "Excluir", typeComment: "Comentário", + typeSystem: "Sistema", relativeJustNow: "agora mesmo", relativeSecondsAgo: "há {{count}}s", relativeMinutesAgo: "há {{count}}m", diff --git a/packages/i18n/src/locales/ru.ts b/packages/i18n/src/locales/ru.ts index 6259301df0..c3d215e3ee 100644 --- a/packages/i18n/src/locales/ru.ts +++ b/packages/i18n/src/locales/ru.ts @@ -2355,6 +2355,7 @@ const ru = { typeUpdate: "Обновление", typeDelete: "Удаление", typeComment: "Комментарий", + typeSystem: "Система", relativeJustNow: "только что", relativeSecondsAgo: "{{count}} с назад", relativeMinutesAgo: "{{count}} м назад", diff --git a/packages/i18n/src/locales/zh.ts b/packages/i18n/src/locales/zh.ts index fecf32f15a..09af775936 100644 --- a/packages/i18n/src/locales/zh.ts +++ b/packages/i18n/src/locales/zh.ts @@ -2432,6 +2432,7 @@ const zh = { typeUpdate: '更新', typeDelete: '删除', typeComment: '评论', + typeSystem: '系统', relativeJustNow: '刚刚', relativeSecondsAgo: '{{count}} 秒前', relativeMinutesAgo: '{{count}} 分钟前',