diff --git a/.changeset/12106-milestone-kind-reader.md b/.changeset/12106-milestone-kind-reader.md new file mode 100644 index 0000000000..a95bcf5f94 --- /dev/null +++ b/.changeset/12106-milestone-kind-reader.md @@ -0,0 +1,11 @@ +--- +'@object-ui/plugin-detail': patch +--- + +A record activity feed shows an object's `activityMilestones` rows of type `completed`, such as "Case resolved", instead of hiding them as completed tasks (objectui#12106). + +A milestone keeps the `type` its declaration names, so "Case resolved" declared with `type: 'completed'` arrived as a `completed` row. The feed mapped every `completed` row to a `task` item, and `showCompleted` is off by default, so the milestone was hidden on every record page that did not set it, including the default page. + +plugin-audit now stamps `metadata.kind: 'milestone'` on a row a milestone fired (objectstack#22771). The feed reads that marker. A `completed` row that carries it renders as a `system` item, the presentation every other author-typed milestone already gets, and shows whatever `showCompleted` says. A task's own "Task completed" milestone shows on that task's page for the same reason. + +A `completed` row without the marker is unchanged: it is a `task` item, hidden unless `showCompleted` is on. That covers rows from a runtime that does not stamp the marker yet, and rows an app inserts itself. `filterMode: 'tasks_only'` and `types: ['task']` still select tasks only, so they do not list a milestone. The `record:activity` block's own read and the console record page's feed both read rows through `activityRowToFeedItem`, so both change together. No export, prop or type changes. diff --git a/content/docs/plugins/plugin-detail.mdx b/content/docs/plugins/plugin-detail.mdx index 1c3f1af242..1d2175bd65 100644 --- a/content/docs/plugins/plugin-detail.mdx +++ b/content/docs/plugins/plugin-detail.mdx @@ -251,6 +251,17 @@ meeting is `completed` → `task`, hidden unless `showCompleted`; a not-yet-held meeting is `scheduled` → `event`, shown, because an upcoming meeting is the part of a timeline you still act on. +A milestone is the exception to the `completed` rule. When an object's +`activityMilestones` entry fires, plugin-audit writes the row with the type the +declaration names and stamps `metadata.kind: 'milestone'` on it. A milestone is +not a completed activity, so a `completed` row with that marker renders as a +`system` item and shows whatever `showCompleted` says. That covers "Case +resolved" on a case, and a task's own "Task completed" on that task's page. A +`completed` row without the marker is still a `task` item, hidden by default. +That includes every row a runtime from before the marker serves, a row an app +inserts itself, and a milestone row served to a reader who may not read a field +the milestone watches (the server leaves the marker off for that reader). + A row whose `type` is in neither list is **not dropped**: it renders through the generic `system` presentation and logs one `console.warn` naming the type. `sys_activity.type` is author-extensible and is not validated on write, so a @@ -285,7 +296,7 @@ itself, so everything above describes that page too. | `filterMode` | Which slice the filter dropdown opens on (`all` / `comments_only` / `changes_only` / `tasks_only`). The user can still change it. An unrecognised value falls back to `all`. | — | | `showFilterToggle` | Shows/hides that dropdown. | — | | `limit` | Items per page; also caps the scoped `sys_activity` read. "Load more" grows the window by this much and re-reads. Must be a positive integer **number**: any other value (a string such as `"5"`, a boolean, a fraction, zero or a negative) is ignored, the block uses its default of 20, and one `console.warn` names the value. The same rule applies to `feed.limit` on `record:chatter` / `record:discussion` and to `limit` on `record:history` (default 50). | — | -| `showCompleted` | Include completed activities (`sys_activity.type = 'completed'`, which surfaces as a `task` item). Off by default, per spec. | — | +| `showCompleted` | Include completed activities (`sys_activity.type = 'completed'`, which surfaces as a `task` item). Off by default, per spec. A milestone row (`metadata.kind: 'milestone'`) is not a completed activity: it surfaces as a `system` item and shows either way. | — | | `unifiedTimeline` | Default `true` mixes field changes and comments in one timeline (Airtable style). `false` un-mixes them: the panel becomes a discussion stream and field changes stay in `record:history`. | — | | `showCommentInput` | Shows the composer. | A host discussion context to persist the comment. Without one the feed is read-only and no composer is rendered. | | `enableMentions` | Offers @-mention autocomplete in the composer. `false` withholds the suggestions. | The host context's user list. | @@ -307,9 +318,9 @@ itself, so everything above describes that page too. | kind | produced by | |---|---| | `field_change` | `sys_activity` types `created` / `updated` / `deleted` / `assigned` / `shared` | -| `task` | `sys_activity` type `completed` | +| `task` | `sys_activity` type `completed`, except a milestone row | | `event` | `sys_activity` type `scheduled` | -| `system` | `sys_activity` type `system`, **and** any author-extended type the map has never seen (it renders through the generic presentation rather than vanishing) | +| `system` | `sys_activity` type `system`, **and** any author-extended type the map has never seen (it renders through the generic presentation rather than vanishing), **and** a `completed` row a milestone fired (`metadata.kind: 'milestone'`) | | `comment` | `sys_comment` rows, merged in by the console's record page and handed to the block through the discussion context | The other eight — `email`, `call`, `note`, `file`, `record_create`, @@ -400,7 +411,7 @@ decision as a defect is how a warning channel gets ignored: filterMode?: string, // Default filter mode showFilterToggle?: boolean, // Show filter mode buttons limit?: number, // Items per page - showCompleted?: boolean, // Show completed tasks + showCompleted?: boolean, // Include completed activities unifiedTimeline?: boolean, // Unified timeline view showCommentInput?: boolean, // Show comment input enableMentions?: boolean, // Enable @mentions diff --git a/packages/plugin-detail/src/index.tsx b/packages/plugin-detail/src/index.tsx index ca1a806cb5..f4796f1c8b 100644 --- a/packages/plugin-detail/src/index.tsx +++ b/packages/plugin-detail/src/index.tsx @@ -785,7 +785,7 @@ ComponentRegistry.register('activity', RecordActivityRenderer, { }, { name: 'showFilterToggle', type: 'boolean', description: 'Expose the activity-type filter dropdown in the panel header' }, { name: 'limit', type: 'number', description: 'Items per page. Also caps the scoped sys_activity read; "Load more" grows the window by this much.' }, - { name: 'showCompleted', type: 'boolean', description: 'Include completed activities (sys_activity type "completed", which surfaces as a task item). Off by default.' }, + { name: 'showCompleted', type: 'boolean', description: 'Include completed activities (sys_activity type "completed", which surfaces as a task item). Off by default. A milestone row (metadata.kind "milestone") is not a completed activity and shows either way.' }, { name: 'unifiedTimeline', type: 'boolean', description: 'Mix field changes and comments in one timeline (Airtable style). Off keeps the panel a discussion stream — field changes stay in record:history.' }, { name: 'showCommentInput', type: 'boolean', description: 'Show the composer. Requires a host discussion context to persist the comment (record detail pages provide one); without it the feed is read-only.' }, { name: 'enableMentions', type: 'boolean', description: 'Offer @-mention autocomplete in the composer, from the host discussion context\'s user list. Off withholds the suggestions.' }, diff --git a/packages/plugin-detail/src/renderers/__tests__/recordActivityFeed.milestoneKind-12106.test.tsx b/packages/plugin-detail/src/renderers/__tests__/recordActivityFeed.milestoneKind-12106.test.tsx new file mode 100644 index 0000000000..35a8ffc09e --- /dev/null +++ b/packages/plugin-detail/src/renderers/__tests__/recordActivityFeed.milestoneKind-12106.test.tsx @@ -0,0 +1,191 @@ +/** + * 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. + */ + +/** + * A `completed` row that an object's `activityMilestones` fired is not a + * completed task (objectui#12106). + * + * plugin-audit forwards a milestone's declared `type` into `sys_activity.type` + * verbatim, so "Case resolved" declared with `type: 'completed'` arrives as a + * `completed` row. The mapper sent every such row to `task`, and + * `showCompleted`'s spec default (`false`) hid it from every record page that + * did not author the flag, including the synthesized default page. + * + * Since objectstack#22771 the writer stamps `metadata.kind: 'milestone'` on a + * fired milestone row (ADR-0052 §5), and the redaction serves it. Triage ruled + * route B: this reader reads that marker. The unlock added one condition: a + * row WITHOUT the marker behaves exactly as today, because a runtime from + * before the writer change (the v17 install face) serves no marker at all. The + * CONTROL pins below hold that face. + * + * The served `metadata` is a JSON STRING: the column is a `textarea` declared + * "JSON-serialized additional context", and the writer's own test reads the + * served row with `JSON.parse(r.metadata)`. The fixtures below spell it the + * same way. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import { FeedItemType as SpecFeedItemType } from '@objectstack/spec/data'; +import { RecordContextProvider } from '@object-ui/react'; +import type { DataSource, FeedItem } from '@object-ui/types'; +import { + MILESTONE_ACTIVITY_FEED_TYPE, + PRODUCED_FEED_TYPES, + activityRowToFeedItem, + applyFeedConfig, + type SysActivityRow, +} from '../recordActivityFeed'; +import { RecordActivityRenderer } from '../record-activity'; + +/** The served `metadata` of a fired milestone: the change, plus the kind. */ +const milestoneMetadata = (from: string, to: string): string => + JSON.stringify({ old: { status: from }, new: { status: to }, kind: 'milestone' }); + +/** "Case resolved", declared on `crm_case` with `type: 'completed'`. */ +const CASE_RESOLVED: SysActivityRow = { + id: 'act-case-resolved', + type: 'completed', + summary: 'Case resolved: Printer jam on floor 3', + timestamp: '2026-10-02T09:00:00.000Z', + actor_name: 'Grace', + object_name: 'crm_case', + record_id: 'case-1', + metadata: milestoneMetadata('in_progress', 'resolved'), +}; + +/** + * A completed activity that is not a milestone, as a runtime serves it with no + * marker: the CRUD mirror's change without a `kind`. A v17 runtime serves a + * fired milestone in exactly this shape too. + */ +const COMPLETED_UNMARKED: SysActivityRow = { + id: 'act-completed-unmarked', + type: 'completed', + summary: 'Follow-up call logged', + timestamp: '2026-10-01T09:00:00.000Z', + actor_name: 'Ada', + object_name: 'crm_case', + record_id: 'case-1', + metadata: JSON.stringify({ old: { status: 'open' }, new: { status: 'held' } }), +}; + +/** The showcase task's own milestone, on that task's own record page. */ +const TASK_OWN_COMPLETION: SysActivityRow = { + id: 'act-task-completed', + type: 'completed', + summary: '✅ Task completed: Write the launch notes', + timestamp: '2026-10-03T09:00:00.000Z', + actor_name: 'Ada', + object_name: 'showcase_task', + record_id: 'task-1', + metadata: milestoneMetadata('in_progress', 'done'), +}; + +const typeOf = (row: SysActivityRow): FeedItem['type'] | undefined => + activityRowToFeedItem(row, 'System')?.type; + +const mapAll = (rows: SysActivityRow[]): FeedItem[] => + rows.map((row) => activityRowToFeedItem(row, 'System')).filter((i): i is FeedItem => i !== null); + +const ids = (items: readonly FeedItem[]) => items.map((i) => String(i.id)).sort(); + +describe('the mapper reads the milestone marker (objectui#12106)', () => { + it('maps a marked `completed` row to the milestone feed type, not `task`', () => { + expect(typeOf(CASE_RESOLVED)).toBe(MILESTONE_ACTIVITY_FEED_TYPE); + expect(MILESTONE_ACTIVITY_FEED_TYPE).toBe('system'); + }); + + it('the milestone feed type is one the spec declares, and the produced set names it', () => { + expect(SpecFeedItemType.options).toContain(MILESTONE_ACTIVITY_FEED_TYPE); + expect(PRODUCED_FEED_TYPES.has(MILESTONE_ACTIVITY_FEED_TYPE)).toBe(true); + }); + + it('changes only the type: the rest of the item is built as before', () => { + const marked = activityRowToFeedItem(CASE_RESOLVED, 'System'); + const unmarked = activityRowToFeedItem({ ...CASE_RESOLVED, metadata: undefined }, 'System'); + expect(unmarked?.type).toBe('task'); + expect(marked).toEqual({ ...unmarked, type: MILESTONE_ACTIVITY_FEED_TYPE }); + }); + + it.each([ + ['no `metadata` at all (a row an app inserted)', undefined], + ['the change without a `kind` (the v17 face)', JSON.stringify({ old: { status: 'open' }, new: { status: 'resolved' } })], + ['a `kind` an app wrote that is not a milestone', JSON.stringify({ kind: 'email' })], + ['a string that is not JSON', 'kind=milestone'], + ['a JSON value that is not an object', JSON.stringify(['milestone'])], + ['an already-parsed object, which no served row carries', { kind: 'milestone' }], + ])('CONTROL: a `completed` row with %s still maps to `task`', (_label, metadata) => { + expect(typeOf({ ...CASE_RESOLVED, metadata })).toBe('task'); + }); + + it('CONTROL: the marker moves no row that would not be hidden as completed', () => { + const marked = { metadata: milestoneMetadata('open', 'qualified') }; + expect(typeOf({ ...CASE_RESOLVED, ...marked, type: 'updated' })).toBe('field_change'); + expect(typeOf({ ...CASE_RESOLVED, ...marked, type: 'scheduled' })).toBe('event'); + expect(typeOf({ ...CASE_RESOLVED, ...marked, type: 'system' })).toBe('system'); + }); +}); + +describe('`showCompleted` keeps its meaning for unmarked rows only (objectui#12106)', () => { + const feed = mapAll([CASE_RESOLVED, COMPLETED_UNMARKED]); + + it('a milestone row is shown under `showCompleted`\'s default', () => { + // `record:discussion` is the block the synthesized default page composes. + const applied = applyFeedConfig(feed, {}, 50, 'record:discussion'); + expect(ids(applied.items)).toContain(String(CASE_RESOLVED.id)); + }); + + it('CONTROL: a `completed` row with no marker is hidden by default, as today', () => { + for (const block of ['record:discussion', 'record:activity']) { + const applied = applyFeedConfig(feed, {}, 50, block); + expect(ids(applied.items), block).toEqual([String(CASE_RESOLVED.id)]); + } + }); + + it('CONTROL: `showCompleted: true` shows both', () => { + const applied = applyFeedConfig(feed, { showCompleted: true }, 50, 'record:discussion'); + expect(ids(applied.items)).toEqual(ids(feed)); + }); + + it('a task\'s own completion milestone, on that task\'s own page, is shown', () => { + const applied = applyFeedConfig(mapAll([TASK_OWN_COMPLETION]), {}, 50, 'record:discussion'); + expect(ids(applied.items)).toEqual([String(TASK_OWN_COMPLETION.id)]); + }); + + it('`types: [\'task\']` still selects tasks only: a milestone is not one', () => { + const applied = applyFeedConfig(feed, { types: ['task'], showCompleted: true }, 50, 'record:activity'); + expect(ids(applied.items)).toEqual([String(COMPLETED_UNMARKED.id)]); + }); +}); + +describe('the `record:activity` self-fetch shows the milestone (objectui#12106)', () => { + beforeEach(() => { + cleanup(); + }); + + it('renders "Case resolved" and hides the unmarked completion by default', async () => { + // A partial double: the self-fetch calls `find` and nothing else. + const find = vi.fn(async () => ({ data: [CASE_RESOLVED, COMPLETED_UNMARKED], total: 2 })); + const dataSource = { find } as unknown as DataSource; + render( + + + , + ); + await waitFor(() => expect(find).toHaveBeenCalled()); + expect(await screen.findByText(String(CASE_RESOLVED.summary))).toBeTruthy(); + expect(screen.queryByText(String(COMPLETED_UNMARKED.summary))).toBeNull(); + }); +}); diff --git a/packages/plugin-detail/src/renderers/recordActivityFeed.ts b/packages/plugin-detail/src/renderers/recordActivityFeed.ts index e649c13c6d..25b2756a83 100644 --- a/packages/plugin-detail/src/renderers/recordActivityFeed.ts +++ b/packages/plugin-detail/src/renderers/recordActivityFeed.ts @@ -79,7 +79,10 @@ export const DEFAULT_ACTIVITY_LIMIT = 20; * ADR-0052 §5b.2 `activityMilestones[].type` is applied verbatim by * plugin-audit's `audit-writers.ts` (`if (milestone.type) activityType = * milestone.type`). That is how `completed` is produced, and it is a general - * door, not a special case. + * door, not a special case. A row that door wrote also carries + * `metadata.kind: 'milestone'`, and the mapper overrides this table's + * `completed` entry for it: see {@link MILESTONE_ACTIVITY_FEED_TYPE} + * (objectui#12106). * * So `scheduled` is a value that is written, stored and queryable while being * undeclared upstream. Dropping it was not a decision this map made; it was the @@ -165,6 +168,48 @@ export const ACTIVITY_TYPE_TO_FEED_TYPE: Readonly = new Set([ ...Object.values(ACTIVITY_TYPE_TO_FEED_TYPE).filter((v): v is FeedItemType => Boolean(v)), UNMAPPED_ACTIVITY_FEED_TYPE, + MILESTONE_ACTIVITY_FEED_TYPE, ...FEED_TYPES_PRODUCED_OFF_MAP, ]); @@ -261,9 +307,47 @@ export const PRODUCED_FEED_TYPES: ReadonlySet = new Set(['task']); +/** + * Whether a `sys_activity` row carries the milestone marker, + * `metadata.kind === 'milestone'` (objectui#12106). + * + * `metadata` is a `textarea` column, declared as "JSON-serialized additional + * context". plugin-audit writes it with `safeStringify`, the read-side + * redaction re-serialises it, and the writer's own test reads the served row + * with `JSON.parse(r.metadata)`. So the marker is read from that JSON string + * and from nothing else. ⛔ Not from an already-parsed object: no served row + * carries one, and reading both would be a second dialect (AGENTS.md #0.1). + * + * Every other value is "no marker": an absent or non-string `metadata`, a + * string that is not a JSON object, or a `kind` that is not `'milestone'` (an + * app may write its own kind, such as `'email'`). Such a row keeps today's + * reading. That includes every row a runtime from before objectstack#22771 + * serves, and every row the redaction served without the kind because the + * reader is not served a field the milestone watches. + */ +function isMilestoneActivityRow(row: SysActivityRow): boolean { + const raw = row?.metadata; + if (typeof raw !== 'string') return false; + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return false; + } + return ( + typeof parsed === 'object' + && parsed !== null + && !Array.isArray(parsed) + && (parsed as { kind?: unknown }).kind === ACTIVITY_MILESTONE_KIND + ); +} + /** A row as `sys_activity` returns it. Loose on purpose: the block reads a * system table it does not own, and an unknown column is not an error. */ export interface SysActivityRow { @@ -788,6 +872,12 @@ export function activityActorName(row: SysActivityRow): string | null { * {@link UNMAPPED_ACTIVITY_FEED_TYPE} and says so once. It used to return * `null` here, which made every extended value invisible — the same outcome * objectui#5840 was filed for, reached by a different route. + * + * One override sits on top of the first outcome (objectui#12106): a row whose + * type maps to a feed type `showCompleted` hides, but which carries the + * milestone marker, renders as {@link MILESTONE_ACTIVITY_FEED_TYPE}. A + * milestone is not a completed activity. A row without the marker is mapped + * exactly as before. */ export function activityRowToFeedItem( row: SysActivityRow, @@ -796,8 +886,13 @@ export function activityRowToFeedItem( const rawType = String(row?.type); const known = Object.prototype.hasOwnProperty.call(ACTIVITY_TYPE_TO_FEED_TYPE, rawType); if (!known) warnUnknownActivityType(rawType); - const feedType = known ? ACTIVITY_TYPE_TO_FEED_TYPE[rawType] : UNMAPPED_ACTIVITY_FEED_TYPE; - if (!feedType) return null; + const mapped = known ? ACTIVITY_TYPE_TO_FEED_TYPE[rawType] : UNMAPPED_ACTIVITY_FEED_TYPE; + if (!mapped) return null; + // The marker is read only for a row that would otherwise be hidden as + // completed, so every other row is mapped without parsing its `metadata`. + const feedType = COMPLETED_FEED_TYPES.has(mapped) && isMilestoneActivityRow(row) + ? MILESTONE_ACTIVITY_FEED_TYPE + : mapped; return { id: row.id as string | number, type: feedType,