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
22 changes: 22 additions & 0 deletions .changeset/20936-action-name-refs-related-list.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
"@objectstack/lint": minor
---

fix(lint)!: `action-name-undefined` resolves `record:related_list` action ids against the related object, and refuses an id the list cannot draw (#20936)

Clause-②: no (narrowing)

`action-name-undefined` is the authoring gate for "a surface names an action that renders nothing". It already walked list-view row and bulk menus, the `record:quick_actions` bar, the `record:alert` call-to-action, the `page:header` action ids and app navigation. One page surface that binds actions by id was never read: `record:related_list` → `properties.actions`.

The console now reads that key. It resolves each id against the RELATED (child) object's own actions, never the page's object, and places it by that action's own `locations`: `list_toolbar` draws a header button, `list_item` and `record_related` draw a row-menu item. An id that names no action of the child object, or an action placed at none of those three, draws no button; the list shows a refusal notice naming it instead. The spec types the key as plain strings, so a misspelled id passed spec validation and lint and surfaced only at runtime.

The rule now walks the key, scoped to `record:related_list`, and answers the same two questions the renderer asks:

- each string id must name an action of the related object: one written on that object, or a `stack.actions` entry bound to it by `objectName`. An id defined only on the page's object, or only as a global action, is refused like a typo, and the message names where it is defined. The did-you-mean and the hint's action list are the related object's own;
- the action it names must declare at least one location a related list draws. The location set is read from the spec's `ACTION_LOCATIONS` vocabulary, classified per member, so a location added to the vocabulary has to be classified before this package compiles.

The related object is the component's bound `dataSource.object` when one is set, otherwise `properties.objectName`. A related object this stack does not define is skipped: its actions belong to another package, and the rule does not guess. Inline-object elements are skipped, as on `page:header`, and every id is reported at its authored index. Every other walk of the rule is unchanged: it still asks only whether a name is defined anywhere in the stack.

**What moves for consumers.** A stack whose related list names an id the list cannot draw built clean before and now fails `os validate` / `os lint` / `os build` with `action-name-undefined` (severity `error`). That id never rendered a button, so nothing that worked stops working. The rule still does not run at the runtime publish door for `page` writes. No related list in the platform's own pages or in the example apps authors `actions`, so none of them changes.

<!-- adr-0087: not-required (no-migration-prescription) a refusal at authoring of record:related_list action ids that the console already refuses at runtime with a visible notice: an id that names no action of the related object, or an action declaring none of the locations a related list draws. No authorable key, spelling, export or stored shape moves: RecordRelatedListProps keeps parsing every value, no stored row is read or rewritten, and which action an author meant to name is not something a ledger entry can rewrite. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers this key (not already-registered); and the change is a rule verdict, not a declaration (not runtime-interface-only or type-surface-only). -->
92 changes: 92 additions & 0 deletions packages/lint/src/validate-action-name-refs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,98 @@ describe('validateActionNameRefs — page:header actions', () => {
});
});

// The related list resolves its `actions` ids against the RELATED (child)
// object's own actions — never the page's object — and places each by that
// action's own `locations`; an id that misses either draws no button, only a
// refusal notice. So this walk, unlike the stack-wide ones above, answers from
// the child object, and only when this stack defines it.
describe('validateActionNameRefs — record:related_list actions', () => {
const stackWith = (component: Record<string, unknown>) => ({
objects: [
{
name: 'crm_account',
fields: { name: { type: 'text' } },
actions: [{ name: 'crm_merge_accounts', type: 'script', locations: ['record_header'] }],
},
{
name: 'crm_contact',
fields: { name: { type: 'text' } },
actions: [
{ name: 'crm_log_call', type: 'script', locations: ['record_related'] },
{ name: 'crm_new_contact', type: 'script', locations: ['list_toolbar'] },
{ name: 'crm_email_contact', type: 'script', locations: ['list_item'] },
{ name: 'crm_pin_contact', type: 'script', locations: ['record_header'] },
{ name: 'crm_sync_contact', type: 'script', locations: [] },
{ name: 'crm_score_contact', type: 'script' },
],
},
],
actions: [
{ name: 'crm_tag_contact', objectName: 'crm_contact', type: 'script', locations: ['list_item'] },
{ name: 'crm_export_all', type: 'script', locations: ['list_toolbar'] },
],
pages: [
{
name: 'account_record',
object: 'crm_account',
regions: [{ name: 'main', components: [{ type: 'record:related_list', ...component }] }],
},
],
});
const relatedList = (actions: unknown[], objectName = 'crm_contact') =>
stackWith({ properties: { objectName, relationshipField: 'account_id', actions } });
const at = (i: number) => `pages[0].regions[0].components[0].properties.actions[${i}]`;

it('accepts ids that resolve on the child object at every location a related list draws', () => {
expect(
validateActionNameRefs(
relatedList(['crm_log_call', 'crm_new_contact', 'crm_email_contact', 'crm_tag_contact']),
),
).toEqual([]);
});

it('errors on an id that resolves nowhere, naming the child object', () => {
const findings = validateActionNameRefs(relatedList(['crm_log_call', 'crm_lgo_call']));
expect(findings).toHaveLength(1);
expect(findings[0].rule).toBe(ACTION_NAME_UNDEFINED);
expect(findings[0].severity).toBe('error');
expect(findings[0].path).toBe(at(1));
expect(findings[0].message).toContain('"crm_lgo_call"');
expect(findings[0].hint).toContain('"crm_contact"');
});

// The decision the direction fixes: the list never reads the page's object
// (or a global action), so an id defined only there is as dead as a typo.
it('errors on an id defined only on the page object or as a global action', () => {
const findings = validateActionNameRefs(relatedList(['crm_merge_accounts', 'crm_export_all']));
expect(findings.map((f) => f.path)).toEqual([at(0), at(1)]);
expect(findings.every((f) => f.rule === ACTION_NAME_UNDEFINED && f.severity === 'error')).toBe(true);
expect(findings[0].message).toContain('"crm_contact"');
});

it('errors on a child action placed at no location a related list draws', () => {
const findings = validateActionNameRefs(
relatedList(['crm_pin_contact', 'crm_log_call', 'crm_sync_contact', 'crm_score_contact']),
);
expect(findings.map((f) => f.path)).toEqual([at(0), at(2), at(3)]);
expect(findings.every((f) => f.rule === ACTION_NAME_UNDEFINED && f.severity === 'error')).toBe(true);
});

it('resolves against a bound dataSource object, which the renderer writes over objectName', () => {
const findings = validateActionNameRefs(
stackWith({
dataSource: { object: 'crm_contact' },
properties: { objectName: 'crm_account', relationshipField: 'account_id', actions: ['crm_log_call', 'crm_merge_accounts'] },
}),
);
expect(findings.map((f) => f.path)).toEqual([at(1)]);
});

it('says nothing about a child object this stack does not define', () => {
expect(validateActionNameRefs(relatedList(['invite_user', 'nope'], 'sys_member'))).toEqual([]);
});
});

describe('validateActionNameRefs — navigation action items', () => {
it('errors on an undefined nav actionName', () => {
const findings = validateActionNameRefs({
Expand Down
190 changes: 187 additions & 3 deletions packages/lint/src/validate-action-name-refs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,14 @@
* author reads — so authoring time is where the refusal belongs (#20105).
* Each walk is scoped to its component type, because the same key means
* something else elsewhere: `element:button`'s `action` is an inline
* definition, not a reference, and `actions` is declared separately on
* `record:related_list`.
* definition, not a reference.
* - page components — `record:related_list` → `properties.actions[]` (the
* list's action ids, #20936). objectui resolves each id against the
* RELATED (child) object's own actions — not the page's object — and
* places it by that action's own `locations`; an id that misses either
* test draws no button, only a refusal notice above the list. This walk is
* therefore the one that resolves against an OBJECT rather than the whole
* stack, and the one that checks placement — see the scope note.
* - app navigation — `{ type: 'action', actionDef: { actionName } }`
* - app navigation deep-link auto-run — `{ type: 'object', runAction }`
* (#4848 — the declared form of the `?runAction=<name>` URL contract)
Expand All @@ -51,8 +57,20 @@
* zero-false-positive posture (ADR-0072 D1) for coverage this issue did not ask
* for. An action defined by another installed package is the one legitimate
* miss; it is called out in the hint rather than guessed at.
*
* The `record:related_list` walk is the exception, and it keeps the posture
* rather than trading it. Its renderer asks both questions itself — is the id
* an action of the CHILD object, and does that action declare a location the
* list draws — and refuses the id when either answer is no. A finding that
* repeats that refusal is not a false positive; it is the runtime's own verdict
* moved to authoring time, which is what ADR-0072 D1 asks for ("resolve at
* runtime for the surface being authored"). So the walk answers from the child
* object's actions, and only for a child object this stack DEFINES: one it
* does not define has its actions in another package, and the walk says
* nothing about it rather than guess. Every other walk is unchanged.
*/

import { ACTION_LOCATIONS, type ActionLocation } from '@objectstack/spec/ui';
import { recordsOf, suggestName } from './object-graph.js';
import { walkPageComponents } from './page-walk.js';

Expand Down Expand Up @@ -102,6 +120,82 @@ function collectActionNames(stack: AnyRec): Set<string> {
return names;
}

/**
* What a `record:related_list` draws for an authored action placed at each
* location of the spec's vocabulary (`ACTION_LOCATIONS`), or `null` where it
* draws nothing. Read at objectui's `relatedListActions.ts`
* (`placeAuthoredRelatedListActions`): `list_toolbar` is a header button;
* `list_item` and `record_related` are a row-menu item. The list renders only
* inside a parent record, which is the one scope `record_related` names.
*
* Keyed by `ActionLocation`, so the vocabulary is read from the spec and never
* copied: a location the spec adds fails this package's typecheck until it is
* classified here, instead of leaving a stale pair that silently refuses it.
*/
const RELATED_LIST_DRAWS: Readonly<Record<ActionLocation, string | null>> = {
list_toolbar: 'a header button',
list_item: 'a row-menu item',
record_header: null,
record_more: null,
record_related: 'a row-menu item',
record_section: null,
};

/** The locations a related list draws, in the spec's own order. */
const RELATED_LIST_LOCATIONS: readonly ActionLocation[] = ACTION_LOCATIONS.filter(
(location) => RELATED_LIST_DRAWS[location] !== null,
);

/** `` `list_toolbar` (a header button), … `` — for a hint. */
const RELATED_LIST_PLACEMENTS = RELATED_LIST_LOCATIONS.map(
(location) => `\`${location}\` (${RELATED_LIST_DRAWS[location]})`,
).join(', ');

/**
* The actions a related list resolves an id against, for each object this
* stack DEFINES: the actions written on the object (keyed by the object they
* are written on), then every `stack.actions` entry bound to it by
* `objectName` — the set `defineStack` merges into the object's `actions`, so
* the set the object's metadata serves to the renderer. An object this stack
* does not define is absent from the map: its actions live in another package.
*/
function indexObjectActions(stack: AnyRec): Map<string, Map<string, AnyRec>> {
const index = new Map<string, Map<string, AnyRec>>();
for (const obj of recordsOf(stack.objects)) {
const objectName = strName(obj.name);
if (!objectName) continue;
const byName = index.get(objectName) ?? new Map<string, AnyRec>();
for (const action of recordsOf(obj.actions)) {
const n = strName(action.name);
if (n && !byName.has(n)) byName.set(n, action);
}
index.set(objectName, byName);
}
for (const action of recordsOf(stack.actions)) {
const owner = strName(action.objectName);
const byName = owner ? index.get(owner) : undefined;
const n = strName(action.name);
if (byName && n && !byName.has(n)) byName.set(n, action);
}
return index;
}

/** Where an action name IS defined in the stack: `on object "x"` per owner, or `as a global action`. */
function actionOwners(stack: AnyRec, name: string): string[] {
const owners = new Set<string>();
for (const obj of recordsOf(stack.objects)) {
if (recordsOf(obj.actions).some((a) => a.name === name)) {
owners.add(`on object "${strName(obj.name) ?? '?'}"`);
}
}
for (const action of recordsOf(stack.actions)) {
if (action.name !== name) continue;
const owner = strName(action.objectName);
owners.add(owner ? `on object "${owner}"` : 'as a global action');
}
return [...owners].sort();
}

/**
* Validate every name-bound action reference in a stack. Returns findings
* (empty = clean).
Expand All @@ -111,6 +205,7 @@ export function validateActionNameRefs(stack: AnyRec): ActionNameRefFinding[] {
if (!stack || typeof stack !== 'object') return findings;

const known = collectActionNames(stack);
let objectActions: Map<string, Map<string, AnyRec>> | undefined;

const check = (
name: string,
Expand Down Expand Up @@ -248,9 +343,85 @@ export function validateActionNameRefs(stack: AnyRec): ActionNameRefFinding[] {
}
}

/**
* `record:related_list` → `properties.actions[]`. The renderer resolves each
* id against the RELATED object's own actions and places it by that
* action's own `locations` — naming it here is not a placement — and an id
* that misses either test draws no button, only a refusal notice above the
* list. Only the string elements are ids, each reported at its AUTHORED
* index, as for `page:header`. Silent when `child` is not an object this
* stack defines: its actions live in another package.
*/
const checkRelatedListActions = (
child: string | undefined,
ids: readonly unknown[],
where: string,
path: string,
) => {
if (!child) return;
objectActions ??= indexObjectActions(stack);
const childActions = objectActions.get(child);
if (!childActions) return;
const childNames = [...childActions.keys()].sort();
for (let ri = 0; ri < ids.length; ri++) {
const id = strName(ids[ri]);
if (!id) continue;
const idPath = `${path}.properties.actions[${ri}]`;
const action = childActions.get(id);
if (!action) {
const owners = known.has(id) ? actionOwners(stack, id) : [];
findings.push({
severity: 'error',
rule: ACTION_NAME_UNDEFINED,
where,
path: idPath,
message:
`Related-list actions names action "${id}", which is not an action of the related object ` +
`"${child}"` +
(owners.length > 0
? ` (it is defined in this stack ${owners.join(' and ')}, which this list never reads)`
: ' (no action in this stack defines it)') +
". The list resolves each id against its related object's own actions only — not the " +
"page's object, not a global action — so it draws no button for this one, only a refusal " +
'notice naming it above the list.' +
suggestName(id, childNames),
hint:
`Define "${id}" on "${child}" — in that object's \`actions\`, or in \`stack.actions\` with ` +
`\`objectName: '${child}'\` — with one of ${RELATED_LIST_PLACEMENTS} in its \`locations\`; ` +
`or name one of "${child}"'s own actions; or remove the reference. Ignore this only if ` +
`another installed package binds the action to "${child}".` +
` Actions of "${child}": ${childNames.length > 0 ? childNames.join(', ') : '(none)'}.`,
});
continue;
}
const declared = Array.isArray(action.locations)
? action.locations.filter((l): l is string => typeof l === 'string')
: undefined;
if (declared?.some((l) => (RELATED_LIST_LOCATIONS as readonly string[]).includes(l))) continue;
findings.push({
severity: 'error',
rule: ACTION_NAME_UNDEFINED,
where,
path: idPath,
message:
`Related-list actions names action "${id}", an action of the related object "${child}" ` +
(declared === undefined
? 'that declares no `locations`, so it is placed at none'
: `whose \`locations\` (${declared.length > 0 ? declared.join(', ') : 'empty'}) include none`) +
` of the locations a related list draws (${RELATED_LIST_LOCATIONS.join(', ')}). The list ` +
'places an authored action by its own `locations` — naming it here is not a placement — so ' +
'it draws no button for it, only a refusal notice naming it above the list.',
hint:
`Add one of ${RELATED_LIST_PLACEMENTS} to the \`locations\` of "${id}" on "${child}", ` +
'or remove the reference.',
});
}
};

// ── Page components: record:quick_actions → properties.actionNames,
// record:alert → properties.action.actionName,
// page:header → properties.actions[] ──
// page:header → properties.actions[],
// record:related_list → properties.actions[] (against the child object) ──
const pages = recordsOf(stack.pages);
for (let pi = 0; pi < pages.length; pi++) {
const page = pages[pi];
Expand Down Expand Up @@ -320,6 +491,19 @@ export function validateActionNameRefs(stack: AnyRec): ActionNameRefFinding[] {
);
}
}

// `record:related_list`'s action ids, resolved against the RELATED
// object (see `checkRelatedListActions`). The related object is the
// per-element `dataSource.object` when one is bound (objectui's
// data-source gate writes it over `objectName`), else `objectName`.
if (type === 'record:related_list' && Array.isArray(props.actions)) {
const binding = component.dataSource;
const child =
(binding && typeof binding === 'object' && !Array.isArray(binding)
? strName((binding as AnyRec).object)
: undefined) ?? strName(props.objectName);
checkRelatedListActions(child, props.actions as unknown[], where, path);
}
}
}

Expand Down
Loading