fix(plugin-detail): name the actor of an activity row that carries only actor_id (objectui#12067)

.changeset/12067-history-actor-id.md

@@ -0,0 +1,6 @@
+---
+'@object-ui/plugin-detail': patch
+'@object-ui/app-shell': patch
+---
+
+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

@@ -260,6 +260,14 @@ lost data — the row is on the timeline; what it lacks is the icon and colour a
 mapped type gets. Give it one by adding the type to `ACTIVITY_TYPE_TO_FEED_TYPE`
 in `@object-ui/plugin-detail`.
 
+Each item names who acted. The row's `actor_name`, the name recorded when the
+action happened, wins when it is present. A row without one is named by its
+`actor_id`, a lookup to `sys_user`: the block's read expands it, so each user's
+record comes back with the page in the same request and the item shows that
+user's `name`. A row with neither, or whose user the viewer may not read, shows
+"System". `record:history` names the entries of its own `sys_activity` read the
+same way, and falls back to "Unknown user".
+
 The rows that really are dropped are the four the map lists as non-activity —
 `commented` / `mentioned` / `login` / `logout` — and they go silently on
 purpose: each is a decision already taken, and a warning about a decision

packages/app-shell/src/views/RecordDetailView.activityActorId-12067.test.tsx

@@ -0,0 +1,176 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * objectui#12067 — the console record page's activity feed names the user
+ * behind a `sys_activity` row that carries `actor_id` and no `actor_name`.
+ *
+ * On `@objectstack/*` 17.7.0 the audit writer fills `actor_id` (a lookup to
+ * `sys_user`) and never `actor_name` (objectstack#22510). The page builds its
+ * feed items with `activityRowToFeedItem`, which read `actor_name` alone, so
+ * every such row read "System". The constructor now names the expanded
+ * `actor_id`, and this page's `sys_activity` read asks for that expansion.
+ *
+ * The data source answers the way objectql does: a read that passes
+ * `$expand: ['actor_id']` gets the user's record in place of the id, and a
+ * read without it gets the bare id, so the pin goes red if the page stops
+ * asking.
+ */
+
+import * as React from 'react';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { render, screen, waitFor, cleanup } from '@testing-library/react';
+import { MemoryRouter } from 'react-router-dom';
+import { MetadataCtx } from '@object-ui/react';
+
+vi.mock('@object-ui/auth', async (importOriginal) => ({
+  ...(await importOriginal<Record<string, unknown>>()),
+  useAuth: () => ({ user: { id: 'u-me', name: 'Viewer', image: null }, activeOrganization: null }),
+  createAuthenticatedFetch: () => vi.fn(),
+}));
+
+vi.mock('@object-ui/collaboration', async (importOriginal) => ({
+  ...(await importOriginal<Record<string, unknown>>()),
+  useRecordPresence: () => [],
+  PresenceAvatars: () => null,
+}));
+
+vi.mock('sonner', () => ({
+  toast: Object.assign(vi.fn(), {
+    success: vi.fn(),
+    error: vi.fn(),
+    info: vi.fn(),
+    warning: vi.fn(),
+    loading: vi.fn(),
+    dismiss: vi.fn(),
+  }),
+}));
+
+// Orthogonal chrome, stubbed so the only asynchrony in this file is the feed.
+vi.mock('./ActionConfirmDialog', () => ({ ActionConfirmDialog: () => null }));
+vi.mock('./ActionParamDialog', () => ({ ActionParamDialog: () => null }));
+vi.mock('./ActionResultDialog', () => ({ ActionResultDialog: () => null }));
+vi.mock('./FlowRunner', () => ({ FlowRunner: () => null }));
+vi.mock('./MetadataInspector', () => ({
+  MetadataPanel: () => null,
+  useMetadataInspector: () => ({ showDebug: false, toggle: () => {} }),
+}));
+
+import { RecordDetailView } from './RecordDetailView';
+
+const OBJECT_NAME = 'crm_lead';
+const RECORD_ID = 'rec-1';
+const ADA = { id: 'u-ada', name: 'Ada Lovelace' };
+
+const ACTIVITY_ROWS = [
+  {
+    id: 'a-1',
+    type: 'updated',
+    summary: 'Stage changed',
+    timestamp: '2026-01-02T00:00:00.000Z',
+    actor_id: 'u-ada',
+  },
+];
+
+function makeDataSource() {
+  return {
+    find: vi.fn((objectName: string, params: { $expand?: string[] } = {}) =>
+      Promise.resolve({
+        data:
+          objectName === 'sys_activity'
+            ? ACTIVITY_ROWS.map((r) =>
+                params.$expand?.includes('actor_id') ? { ...r, actor_id: ADA } : { ...r },
+              )
+            : [],
+      }),
+    ),
+    create: vi.fn(async (_o: string, row: any) => row),
+    findOne: vi.fn(async (_o: string, recordId: string) => ({
+      id: recordId,
+      name: `Record ${recordId}`,
+    })),
+    update: vi.fn(async () => ({})),
+    delete: vi.fn(async () => ({})),
+  } as any;
+}
+
+const OBJECTS = [
+  {
+    name: OBJECT_NAME,
+    label: 'Lead',
+    managedBy: 'platform',
+    fields: {
+      id: { type: 'text', label: 'Id' },
+      name: { type: 'text', label: 'Name' },
+    },
+  },
+];
+
+function makeMetadata() {
+  return {
+    objects: OBJECTS,
+    pages: [],
+    loading: false,
+    error: null,
+    refresh: async () => {},
+    invalidate: () => {},
+    ensureType: async () => [],
+    getItem: async () => null,
+    getItemsByType: () => [],
+  } as any;
+}
+
+beforeEach(() => {
+  cleanup();
+  vi.spyOn(console, 'warn').mockImplementation(() => {});
+  // Unrelated chrome (approvals, favourites…) reaches for the platform API.
+  vi.stubGlobal(
+    'fetch',
+    vi.fn(async () =>
+      new Response(JSON.stringify({ data: [] }), {
+        status: 200,
+        headers: { 'content-type': 'application/json' },
+      }),
+    ),
+  );
+});
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+  vi.restoreAllMocks();
+});
+
+describe('the record page feed names the actor of an id-only activity row (objectui#12067)', () => {
+  it('reads `sys_activity` with `actor_id` expanded and shows the user, not "System"', async () => {
+    const dataSource = makeDataSource();
+    render(
+      <MemoryRouter initialEntries={[`/app/demo/${OBJECT_NAME}/${RECORD_ID}`]}>
+        <MetadataCtx.Provider value={makeMetadata()}>
+          <RecordDetailView
+            dataSource={dataSource}
+            objects={OBJECTS}
+            onEdit={() => {}}
+            objectNameOverride={OBJECT_NAME}
+            recordIdOverride={RECORD_ID}
+            embedded
+          />
+        </MetadataCtx.Provider>
+      </MemoryRouter>,
+    );
+
+    expect(await screen.findByText('Stage changed')).toBeTruthy();
+    expect(screen.getByText('Ada Lovelace')).toBeTruthy();
+    expect(screen.queryByText('System')).toBeNull();
+
+    await waitFor(() => {
+      const reads = dataSource.find.mock.calls.filter((c: unknown[]) => c[0] === 'sys_activity');
+      expect(reads).toHaveLength(1);
+      expect(reads[0][1].$expand).toEqual(['actor_id']);
+    });
+  });
+});

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

@@ -1970,6 +1970,11 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
       $filter: { object_name: objectName, record_id: pureRecordId },
       $orderby: { timestamp: 'asc' },
       $top: 200,
+      // The actor's record in place of its id, so a row with no `actor_name`
+      // names its user instead of reading "System" (objectui#12067). The
+      // shared constructor reads it; the `record:activity` block's own read
+      // passes the same expand.
+      $expand: ['actor_id'],
     })
       .then((res: any) => {
         recordRefusal('activity', false);

packages/plugin-detail/src/renderers/__tests__/record-history.actorId-12067.test.tsx

@@ -0,0 +1,127 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * `record:history` names the user behind a row that carries `actor_id` and no
+ * `actor_name` (objectui#12067).
+ *
+ * On `@objectstack/*` 17.7.0 the audit writer fills `sys_activity.actor_id` (a
+ * lookup to `sys_user`) and never `actor_name` (objectstack#22510 fixes the
+ * writer from then on). The self-fetch mapped the entry's user from
+ * `actor_name` alone, so every such row read "Unknown user".
+ *
+ * The data source below answers the way objectql does: rows store the bare
+ * user id, and a read that passes `$expand: ['actor_id']` gets each user's
+ * record in place of the id, or keeps the bare id for a user the viewer may
+ * not read. A read without the expand gets bare ids only, so the name pins go
+ * red when the renderer stops asking for it.
+ */
+
+import * as React from 'react';
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { render, screen, waitFor, cleanup } from '@testing-library/react';
+import { RecordContextProvider } from '@object-ui/react';
+import { RecordHistoryRenderer } from '../record-history';
+
+const UNKNOWN_USER = 'Unknown user';
+
+/** The `sys_user` rows this viewer may read. */
+const USERS: Record<string, { id: string; name: string }> = {
+  'u-ada': { id: 'u-ada', name: 'Ada Lovelace' },
+  'u-grace': { id: 'u-grace', name: 'Grace Hopper' },
+};
+
+type Row = Record<string, unknown>;
+
+const row = (id: string, extra: Row): Row => ({
+  id,
+  type: 'updated',
+  summary: `summary of ${id}`,
+  timestamp: '2026-01-02T00:00:00.000Z',
+  ...extra,
+});
+
+/** One `find()` that expands `actor_id` the way the engine does, or does not. */
+function makeDataSource(rows: Row[]) {
+  return {
+    find: vi.fn(async (_object: string, params: { $expand?: string[] } = {}) => {
+      const expand = params.$expand?.includes('actor_id') ?? false;
+      return {
+        data: rows.map((r) => {
+          const id = r.actor_id;
+          if (!expand || typeof id !== 'string') return { ...r };
+          return { ...r, actor_id: USERS[id] ?? id };
+        }),
+      };
+    }),
+  } as any;
+}
+
+function mount(rows: Row[], schema: Record<string, unknown> = {}) {
+  const dataSource = makeDataSource(rows);
+  render(
+    <RecordContextProvider
+      objectName="crm_lead"
+      recordId="rec-1"
+      data={{ id: 'rec-1', name: 'Lead 1' }}
+      dataSource={dataSource}
+    >
+      <RecordHistoryRenderer schema={schema as any} />
+    </RecordContextProvider>,
+  );
+  return dataSource;
+}
+
+beforeEach(() => {
+  cleanup();
+});
+
+describe('record:history names the actor of a row with only `actor_id` (objectui#12067)', () => {
+  it("a row with only `actor_id` renders that user's name", async () => {
+    mount([row('h-1', { actor_id: 'u-ada' })]);
+    expect(await screen.findByText('Ada Lovelace')).toBeTruthy();
+    expect(screen.queryByText(UNKNOWN_USER)).toBeNull();
+  });
+
+  it('a row with both renders `actor_name`, the snapshot taken at the time of the action', async () => {
+    mount([row('h-1', { actor_id: 'u-ada', actor_name: 'Ada (at the time)' })]);
+    expect(await screen.findByText('Ada (at the time)')).toBeTruthy();
+    expect(screen.queryByText('Ada Lovelace')).toBeNull();
+  });
+
+  it('a row with neither renders the existing fallback, and so does an id the viewer may not read', async () => {
+    mount(
+      [
+        row('h-1', {}),
+        // The engine keeps the bare id when the user cannot be read.
+        row('h-2', { actor_id: 'u-not-readable' }),
+      ],
+      { unknownUserText: 'Someone' },
+    );
+    await screen.findByText('summary of h-1');
+    expect(screen.getAllByText('Someone')).toHaveLength(2);
+    // A raw id is never shown as the actor's name.
+    expect(screen.queryByText('u-not-readable')).toBeNull();
+  });
+
+  it('a page of N id-only rows issues one read, not one per row', async () => {
+    const rows = Array.from({ length: 6 }, (_, i) =>
+      row(`h-${i}`, { actor_id: i % 2 === 0 ? 'u-ada' : 'u-grace' }),
+    );
+    const dataSource = mount(rows);
+
+    expect(await screen.findAllByText('Ada Lovelace')).toHaveLength(3);
+    expect(screen.getAllByText('Grace Hopper')).toHaveLength(3);
+    await waitFor(() => expect(dataSource.find).toHaveBeenCalled());
+    // The names arrive on the page's own read: one `sys_activity` find that
+    // expands `actor_id`, and no `sys_user` read at all.
+    const objects = dataSource.find.mock.calls.map((c: unknown[]) => c[0]);
+    expect(objects).toEqual(['sys_activity']);
+    expect(dataSource.find.mock.calls[0][1].$expand).toEqual(['actor_id']);
+  });
+});