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
39 changes: 39 additions & 0 deletions .changeset/6730-shared-activity-type-bucket.md
Original file line numberDiff line numberDiff line change
@@ -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.
Original file line numberDiff line numberDiff line change
@@ -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<Record<string, string>> {
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');
});
});
56 changes: 21 additions & 35 deletions packages/app-shell/src/hooks/sharedUserFeeds.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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). */
Expand DownExpand Up@@ -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<string, unknown> => {
if (!row || typeof row !== 'object') return false;
const r = row as Record<string, unknown>;
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);
}

/**
Expand Down
27 changes: 17 additions & 10 deletions packages/app-shell/src/layout/ActivityFeed.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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[];
Expand All@@ -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"). */
Expand All@@ -69,6 +74,7 @@ export function ActivityFeed({ activities = [], className }: ActivityFeedProps)
update: true,
delete: true,
comment: true,
system: true,
});

const togglePreference = (type: ActivityItem['type']) => {
Expand All@@ -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 (
Expand Down
Loading
Loading