Skip to content

Commit e391f87

Browse files
fix(plugin-detail): name the actor of an activity row that carries only actor_id (objectui#12067) (#12070)
Fixes #12067 Clause-②: no Implemented by the os-dev dispatched from seat `domain:ui#3`, session `https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8`. ## What changed - **`record:history`** (`record-history.tsx`): its `sys_activity` self-fetch now passes `$expand: ['actor_id']`, and the entry's user comes from `activityActorName(row)` instead of `actor_name` alone. - **`recordActivityFeed.ts`**: a new module-level reader, `activityActorName`, applies the card's rule. It returns the `actor_name` snapshot when that is non-blank. Otherwise it returns the expanded `actor_id` record's `name` (`sys_user`'s declared name field). Otherwise it returns `null`, so the caller keeps its existing fallback. `activityRowToFeedItem` now uses it in place of `row.actor_name ?? systemActorLabel`. - The two other `sys_activity` reads whose rows go through `activityRowToFeedItem` pass the same expand: the `record:activity` block's self-fetch (`record-activity.tsx`) and the console record page's merged feed (`RecordDetailView.tsx`, app-shell). - Docs: one paragraph in `content/docs/plugins/plugin-detail.mdx` (AGENTS.md #2). - Changeset `.changeset/12067-history-actor-id.md`: `patch` for `@object-ui/plugin-detail` and `@object-ui/app-shell`. No export, prop, type or pack key changes. `activityActorName` is not re-exported from the package index. In the built `packages/plugin-detail/dist/index.d.ts` it has 0 hits, against 3 hits for the control `activityRowToFeedItem`. `SysActivityRow` is unchanged too: the reader reads `actor_id` through its existing index signature. ## Measurements (the dispatch's three mechanism hypotheses) 1. **How History reads `sys_activity`, and whether the read can expand `actor_id`: yes, in the same request.** - The renderer calls `dataSource.find('sys_activity', { $filter: { object_name, record_id }, $orderby: { timestamp: 'desc' }, $top })`. `$expand` is a declared `QueryParams` member (`@object-ui/types` `data.ts`). - `ObjectStackAdapter.find` sends any `$expand` to `rawFindWithPopulate`: one `GET /api/v1/data/sys_activity?populate=actor_id&...`. Its filter conversion is the same as the SDK route's: `translateFilterToAST` calls `convertFiltersToAST`. - On the server (objectstack `6befe19c`, spec 17.7.0), `sys_activity.actor_id` is `Field.lookup('sys_user')`. It is the same at `@objectstack/plugin-audit@17.0.0`, so every 17.x server declares the field and the protocol's expand gate (`assertExpandTargetsExist`) admits it. - `expandRelatedRecords` loads the ids of the whole page with ONE `find(sys_user, id in [...])` through the engine's own read path (CRUD gate, RLS, FLS). When that read is refused it keeps the bare id, and the parent read still succeeds. - This is the route objectui#11701's Audit Log page took for the same question: `$expand=user_id`, then read `name`. 2. **The `sys_user` read door: ordinary viewers are not refused, so the stop clause does not fire.** - `member_default` grants `sys_user` `allowRead: true`, row-scoped by `sys_user_self` (`id == current_user.id`) and `sys_user_org_members` (`id in current_user.org_user_ids`). The read-only set carries the same two `select` policies. - So a viewer can read the name of every member of the same organization. An actor outside that scope, or a deleted user, keeps the bare id, and the entry shows the existing fallback. A raw id is never shown as a name. 3. **`recordActivityFeed.ts`: the same gap.** - The same id-only rows reach `activityRowToFeedItem` through two reads: the `record:activity` self-fetch, and `RecordDetailView`'s merged feed on the default console record page. - `actor: row.actor_name ?? systemActorLabel` showed those rows as made by "System", which credits the change to no person. Per the claim, the same rule now applies there. - The rule takes effect only where the read expands `actor_id`, so both of those reads now do. ## Surface beyond the claim's file list (stated, not silent) The claim names `record-history.tsx`, `recordActivityFeed.ts` (conditional), their tests and the changeset. This PR also touches: - `record-activity.tsx` and `RecordDetailView.tsx`: one `$expand: ['actor_id']` line each. Without them, the rule the claim admits into `recordActivityFeed.ts` would read an expanded record that never arrives. Leaving `RecordDetailView` out would also make the console record page and the `record:activity` block disagree about the same row, a disagreement that file's own comment calls a bug. - `RecordDetailView.activityActorId-12067.test.tsx` (that read's pin) and the docs paragraph. The seat may amend the claim's file surface. ## Tests (all at `6b1a3ff64`) New pins: - `record-history.actorId-12067.test.tsx`, the claim's four: - a row with only `actor_id` shows the user's name; - a row with both shows `actor_name`; - a row with neither shows the existing fallback, and so does a bare id the viewer may not read; - a page of 6 id-only rows makes exactly one `find`, on `sys_activity` with `$expand: ['actor_id']`, and no `sys_user` read. - `recordActivityFeed.actorId-12067.test.tsx`: the same three rules on the mapper, plus the `record:activity` self-fetch (one read, expanded, both names shown, no "System"). - `RecordDetailView.activityActorId-12067.test.tsx`: the console page's `sys_activity` read carries the expand, and the feed shows the user, not "System". Runs: - `pnpm exec vitest run packages/plugin-detail/ --maxWorkers=2`: `Test Files 250 passed | 1 skipped (251)`, `Tests 2433 passed | 8 skipped (2441)`. - app-shell, **narrowed and declared**: the 111 app-shell test files that name `RecordDetailView` (`git grep -l RecordDetailView` over app-shell tests), run in two halves: `56 passed (56)` / `495 passed`, and `55 passed (55)` / `394 passed`. The full app-shell suite (1245 files) does not fit the foreground cap, so it is CI's. - `apps/console/src/__tests__/record-block-record-reach.test.tsx`: `13 passed (13)`. - `pnpm --filter @object-ui/plugin-detail type-check` and `pnpm --filter @object-ui/app-shell type-check`: both exit 0, after building each dependency closure. `tsc -p tsconfig.test.json --listFilesOnly` lists each new test file. - `pnpm exec eslint` on the 7 touched TS/TSX files: 0 errors and 140 warnings (`no-explicit-any` and the like; the repo has no warnings ratchet). This is a narrowed run, and three facts back it: - population: the repo's own `eslint.config.js`; - count: 7 files, read from `--format json`; - invariance: no `parserOptions.project` or `projectService`, and no custom rule in `eslint-rules/` reads the filesystem, so this diff cannot move a verdict on an untouched file. - `check:control-bytes`, `check:test-path-roots`, `check:changeset-claims`, `check:pending-changeset-literals`, `check:new-line-citations` (0 new), `check:vi-mock-specifiers`, `check:vi-mock-inherit`, `check:vi-mock-override-shape`, `check:doc-types` and `check:doc-fences`: all exit 0. So do `check-changeset-presence.mjs` (7 source files of 2 released packages, 1 changeset), `check-changeset-no-major.mjs`, `check-changeset-fixed.mjs` and `check-changeset-overwrite.mjs`. `check-governed-queue-guard.mjs --test` reports NOT GOVERNED for all 9 paths. - NOT MEASURED: - `check:doc-snippets` and `check:doc-examples`. Reason: prerequisite. The gate printed "THE GATE COULD NOT RUN" because the packages its examples import are not built. The docs diff adds 0 fenced blocks. - The console eager-closure budget. Reason: it needs a console build, which is CI's Bundle Analysis job. ## Ablations (predictions written before each run) Each run used `ablation-replace.mjs` in wrap mode. Every anchor hit 1 time and went to 0 on disk. Every restore matched the HEAD blob, and `git diff HEAD` was empty afterwards. | Mutation | Predicted | Observed | |---|---|---| | A1: drop `$expand: ['actor_id']` from `record-history.tsx` | history: 2 red (id-only, N rows), 2 green | `Tests 2 failed \| 2 passed (4)` | | A2: `user_name: activityActorName(r)` back to `r.actor_name ?? null` | the same 2 red / 2 green | `Tests 2 failed \| 2 passed (4)` | | A3: mapper back to `row.actor_name ?? systemActorLabel` | feed: 3 red, "both" green; app-shell pin red | `Tests 4 failed \| 1 passed (5)` across both files | | A4: drop the expand from `record-activity.tsx` | feed: self-fetch red, 3 mapper pins green | `Tests 1 failed \| 3 passed (4)` | | A5: drop the expand from `RecordDetailView.tsx` | app-shell pin red | `Tests 1 failed (1)` | ## Acceptance notes - **The `$expand` route skips the adapter's missing-resource memo.** On a deployment with no `sys_activity` (no audit plugin), each mount now repeats a 404 read instead of remembering the first one. What the user sees is unchanged: a 404 is not a refusal, so the history and the feed stay empty. Observation only, nothing filed; carrier: none. - **Avatar.** `actor_avatar_url` is still the only avatar source. An id-only row shows initials from the resolved name. This is outside the card's acceptance. - **Writer half.** The writer half is objectstack-ai/objectstack#22510, and this PR does not touch it. Once it lands, new rows carry `actor_name`, which still wins. The expand then names only the older rows, at the cost of one batched server-side `sys_user` read per page. --- _Generated by [Claude Code](https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent acc4328 commit e391f87

9 files changed

Lines changed: 454 additions & 2 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@object-ui/plugin-detail': patch
3+
'@object-ui/app-shell': patch
4+
---
5+
6+
A record's History tab and activity feed name the user behind an activity row that carries `actor_id` and no `actor_name` (objectui#12067). On `@objectstack/*` 17.7.0 the audit writer fills only `actor_id`, so `record:history` showed every such entry as "Unknown user", and the `record:activity` block and the console record page's activity feed showed the change as made by "System". Their `sys_activity` reads now expand `actor_id`, which brings each user's record back with the page in the same request, and the entry shows that user's `name`. `actor_name`, when present, still wins: it is the name recorded when the action happened. A row with neither, or whose user the viewer may not read, keeps the existing fallback, and a raw user id is never shown as a name.

‎content/docs/plugins/plugin-detail.mdx‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,14 @@ lost data — the row is on the timeline; what it lacks is the icon and colour a
260260
mapped type gets. Give it one by adding the type to `ACTIVITY_TYPE_TO_FEED_TYPE`
261261
in `@object-ui/plugin-detail`.
262262

263+
Each item names who acted. The row's `actor_name`, the name recorded when the
264+
action happened, wins when it is present. A row without one is named by its
265+
`actor_id`, a lookup to `sys_user`: the block's read expands it, so each user's
266+
record comes back with the page in the same request and the item shows that
267+
user's `name`. A row with neither, or whose user the viewer may not read, shows
268+
"System". `record:history` names the entries of its own `sys_activity` read the
269+
same way, and falls back to "Unknown user".
270+
263271
The rows that really are dropped are the four the map lists as non-activity —
264272
`commented` / `mentioned` / `login` / `logout` — and they go silently on
265273
purpose: each is a decision already taken, and a warning about a decision
Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* objectui#12067 — the console record page's activity feed names the user
11+
* behind a `sys_activity` row that carries `actor_id` and no `actor_name`.
12+
*
13+
* On `@objectstack/*` 17.7.0 the audit writer fills `actor_id` (a lookup to
14+
* `sys_user`) and never `actor_name` (objectstack#22510). The page builds its
15+
* feed items with `activityRowToFeedItem`, which read `actor_name` alone, so
16+
* every such row read "System". The constructor now names the expanded
17+
* `actor_id`, and this page's `sys_activity` read asks for that expansion.
18+
*
19+
* The data source answers the way objectql does: a read that passes
20+
* `$expand: ['actor_id']` gets the user's record in place of the id, and a
21+
* read without it gets the bare id, so the pin goes red if the page stops
22+
* asking.
23+
*/
24+
25+
import * as React from 'react';
26+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
27+
import { render, screen, waitFor, cleanup } from '@testing-library/react';
28+
import { MemoryRouter } from 'react-router-dom';
29+
import { MetadataCtx } from '@object-ui/react';
30+
31+
vi.mock('@object-ui/auth', async (importOriginal) => ({
32+
...(await importOriginal<Record<string, unknown>>()),
33+
useAuth: () => ({ user: { id: 'u-me', name: 'Viewer', image: null }, activeOrganization: null }),
34+
createAuthenticatedFetch: () => vi.fn(),
35+
}));
36+
37+
vi.mock('@object-ui/collaboration', async (importOriginal) => ({
38+
...(await importOriginal<Record<string, unknown>>()),
39+
useRecordPresence: () => [],
40+
PresenceAvatars: () => null,
41+
}));
42+
43+
vi.mock('sonner', () => ({
44+
toast: Object.assign(vi.fn(), {
45+
success: vi.fn(),
46+
error: vi.fn(),
47+
info: vi.fn(),
48+
warning: vi.fn(),
49+
loading: vi.fn(),
50+
dismiss: vi.fn(),
51+
}),
52+
}));
53+
54+
// Orthogonal chrome, stubbed so the only asynchrony in this file is the feed.
55+
vi.mock('./ActionConfirmDialog', () => ({ ActionConfirmDialog: () => null }));
56+
vi.mock('./ActionParamDialog', () => ({ ActionParamDialog: () => null }));
57+
vi.mock('./ActionResultDialog', () => ({ ActionResultDialog: () => null }));
58+
vi.mock('./FlowRunner', () => ({ FlowRunner: () => null }));
59+
vi.mock('./MetadataInspector', () => ({
60+
MetadataPanel: () => null,
61+
useMetadataInspector: () => ({ showDebug: false, toggle: () => {} }),
62+
}));
63+
64+
import { RecordDetailView } from './RecordDetailView';
65+
66+
const OBJECT_NAME = 'crm_lead';
67+
const RECORD_ID = 'rec-1';
68+
const ADA = { id: 'u-ada', name: 'Ada Lovelace' };
69+
70+
const ACTIVITY_ROWS = [
71+
{
72+
id: 'a-1',
73+
type: 'updated',
74+
summary: 'Stage changed',
75+
timestamp: '2026-01-02T00:00:00.000Z',
76+
actor_id: 'u-ada',
77+
},
78+
];
79+
80+
function makeDataSource() {
81+
return {
82+
find: vi.fn((objectName: string, params: { $expand?: string[] } = {}) =>
83+
Promise.resolve({
84+
data:
85+
objectName === 'sys_activity'
86+
? ACTIVITY_ROWS.map((r) =>
87+
params.$expand?.includes('actor_id') ? { ...r, actor_id: ADA } : { ...r },
88+
)
89+
: [],
90+
}),
91+
),
92+
create: vi.fn(async (_o: string, row: any) => row),
93+
findOne: vi.fn(async (_o: string, recordId: string) => ({
94+
id: recordId,
95+
name: `Record ${recordId}`,
96+
})),
97+
update: vi.fn(async () => ({})),
98+
delete: vi.fn(async () => ({})),
99+
} as any;
100+
}
101+
102+
const OBJECTS = [
103+
{
104+
name: OBJECT_NAME,
105+
label: 'Lead',
106+
managedBy: 'platform',
107+
fields: {
108+
id: { type: 'text', label: 'Id' },
109+
name: { type: 'text', label: 'Name' },
110+
},
111+
},
112+
];
113+
114+
function makeMetadata() {
115+
return {
116+
objects: OBJECTS,
117+
pages: [],
118+
loading: false,
119+
error: null,
120+
refresh: async () => {},
121+
invalidate: () => {},
122+
ensureType: async () => [],
123+
getItem: async () => null,
124+
getItemsByType: () => [],
125+
} as any;
126+
}
127+
128+
beforeEach(() => {
129+
cleanup();
130+
vi.spyOn(console, 'warn').mockImplementation(() => {});
131+
// Unrelated chrome (approvals, favourites…) reaches for the platform API.
132+
vi.stubGlobal(
133+
'fetch',
134+
vi.fn(async () =>
135+
new Response(JSON.stringify({ data: [] }), {
136+
status: 200,
137+
headers: { 'content-type': 'application/json' },
138+
}),
139+
),
140+
);
141+
});
142+
143+
afterEach(() => {
144+
vi.unstubAllGlobals();
145+
vi.restoreAllMocks();
146+
});
147+
148+
describe('the record page feed names the actor of an id-only activity row (objectui#12067)', () => {
149+
it('reads `sys_activity` with `actor_id` expanded and shows the user, not "System"', async () => {
150+
const dataSource = makeDataSource();
151+
render(
152+
<MemoryRouter initialEntries={[`/app/demo/${OBJECT_NAME}/${RECORD_ID}`]}>
153+
<MetadataCtx.Provider value={makeMetadata()}>
154+
<RecordDetailView
155+
dataSource={dataSource}
156+
objects={OBJECTS}
157+
onEdit={() => {}}
158+
objectNameOverride={OBJECT_NAME}
159+
recordIdOverride={RECORD_ID}
160+
embedded
161+
/>
162+
</MetadataCtx.Provider>
163+
</MemoryRouter>,
164+
);
165+
166+
expect(await screen.findByText('Stage changed')).toBeTruthy();
167+
expect(screen.getByText('Ada Lovelace')).toBeTruthy();
168+
expect(screen.queryByText('System')).toBeNull();
169+
170+
await waitFor(() => {
171+
const reads = dataSource.find.mock.calls.filter((c: unknown[]) => c[0] === 'sys_activity');
172+
expect(reads).toHaveLength(1);
173+
expect(reads[0][1].$expand).toEqual(['actor_id']);
174+
});
175+
});
176+
});

‎packages/app-shell/src/views/RecordDetailView.tsx‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1970,6 +1970,11 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
19701970
$filter: { object_name: objectName, record_id: pureRecordId },
19711971
$orderby: { timestamp: 'asc' },
19721972
$top: 200,
1973+
// The actor's record in place of its id, so a row with no `actor_name`
1974+
// names its user instead of reading "System" (objectui#12067). The
1975+
// shared constructor reads it; the `record:activity` block's own read
1976+
// passes the same expand.
1977+
$expand: ['actor_id'],
19731978
})
19741979
.then((res: any) => {
19751980
recordRefusal('activity', false);
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* `record:history` names the user behind a row that carries `actor_id` and no
11+
* `actor_name` (objectui#12067).
12+
*
13+
* On `@objectstack/*` 17.7.0 the audit writer fills `sys_activity.actor_id` (a
14+
* lookup to `sys_user`) and never `actor_name` (objectstack#22510 fixes the
15+
* writer from then on). The self-fetch mapped the entry's user from
16+
* `actor_name` alone, so every such row read "Unknown user".
17+
*
18+
* The data source below answers the way objectql does: rows store the bare
19+
* user id, and a read that passes `$expand: ['actor_id']` gets each user's
20+
* record in place of the id, or keeps the bare id for a user the viewer may
21+
* not read. A read without the expand gets bare ids only, so the name pins go
22+
* red when the renderer stops asking for it.
23+
*/
24+
25+
import * as React from 'react';
26+
import { describe, it, expect, vi, beforeEach } from 'vitest';
27+
import { render, screen, waitFor, cleanup } from '@testing-library/react';
28+
import { RecordContextProvider } from '@object-ui/react';
29+
import { RecordHistoryRenderer } from '../record-history';
30+
31+
const UNKNOWN_USER = 'Unknown user';
32+
33+
/** The `sys_user` rows this viewer may read. */
34+
const USERS: Record<string, { id: string; name: string }> = {
35+
'u-ada': { id: 'u-ada', name: 'Ada Lovelace' },
36+
'u-grace': { id: 'u-grace', name: 'Grace Hopper' },
37+
};
38+
39+
type Row = Record<string, unknown>;
40+
41+
const row = (id: string, extra: Row): Row => ({
42+
id,
43+
type: 'updated',
44+
summary: `summary of ${id}`,
45+
timestamp: '2026-01-02T00:00:00.000Z',
46+
...extra,
47+
});
48+
49+
/** One `find()` that expands `actor_id` the way the engine does, or does not. */
50+
function makeDataSource(rows: Row[]) {
51+
return {
52+
find: vi.fn(async (_object: string, params: { $expand?: string[] } = {}) => {
53+
const expand = params.$expand?.includes('actor_id') ?? false;
54+
return {
55+
data: rows.map((r) => {
56+
const id = r.actor_id;
57+
if (!expand || typeof id !== 'string') return { ...r };
58+
return { ...r, actor_id: USERS[id] ?? id };
59+
}),
60+
};
61+
}),
62+
} as any;
63+
}
64+
65+
function mount(rows: Row[], schema: Record<string, unknown> = {}) {
66+
const dataSource = makeDataSource(rows);
67+
render(
68+
<RecordContextProvider
69+
objectName="crm_lead"
70+
recordId="rec-1"
71+
data={{ id: 'rec-1', name: 'Lead 1' }}
72+
dataSource={dataSource}
73+
>
74+
<RecordHistoryRenderer schema={schema as any} />
75+
</RecordContextProvider>,
76+
);
77+
return dataSource;
78+
}
79+
80+
beforeEach(() => {
81+
cleanup();
82+
});
83+
84+
describe('record:history names the actor of a row with only `actor_id` (objectui#12067)', () => {
85+
it("a row with only `actor_id` renders that user's name", async () => {
86+
mount([row('h-1', { actor_id: 'u-ada' })]);
87+
expect(await screen.findByText('Ada Lovelace')).toBeTruthy();
88+
expect(screen.queryByText(UNKNOWN_USER)).toBeNull();
89+
});
90+
91+
it('a row with both renders `actor_name`, the snapshot taken at the time of the action', async () => {
92+
mount([row('h-1', { actor_id: 'u-ada', actor_name: 'Ada (at the time)' })]);
93+
expect(await screen.findByText('Ada (at the time)')).toBeTruthy();
94+
expect(screen.queryByText('Ada Lovelace')).toBeNull();
95+
});
96+
97+
it('a row with neither renders the existing fallback, and so does an id the viewer may not read', async () => {
98+
mount(
99+
[
100+
row('h-1', {}),
101+
// The engine keeps the bare id when the user cannot be read.
102+
row('h-2', { actor_id: 'u-not-readable' }),
103+
],
104+
{ unknownUserText: 'Someone' },
105+
);
106+
await screen.findByText('summary of h-1');
107+
expect(screen.getAllByText('Someone')).toHaveLength(2);
108+
// A raw id is never shown as the actor's name.
109+
expect(screen.queryByText('u-not-readable')).toBeNull();
110+
});
111+
112+
it('a page of N id-only rows issues one read, not one per row', async () => {
113+
const rows = Array.from({ length: 6 }, (_, i) =>
114+
row(`h-${i}`, { actor_id: i % 2 === 0 ? 'u-ada' : 'u-grace' }),
115+
);
116+
const dataSource = mount(rows);
117+
118+
expect(await screen.findAllByText('Ada Lovelace')).toHaveLength(3);
119+
expect(screen.getAllByText('Grace Hopper')).toHaveLength(3);
120+
await waitFor(() => expect(dataSource.find).toHaveBeenCalled());
121+
// The names arrive on the page's own read: one `sys_activity` find that
122+
// expands `actor_id`, and no `sys_user` read at all.
123+
const objects = dataSource.find.mock.calls.map((c: unknown[]) => c[0]);
124+
expect(objects).toEqual(['sys_activity']);
125+
expect(dataSource.find.mock.calls[0][1].$expand).toEqual(['actor_id']);
126+
});
127+
});

0 commit comments

Comments
 (0)