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
8 changes: 8 additions & 0 deletions .changeset/10186-related-list-fls-select.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,11 @@ row field through something other than a `visible` / `disabled` predicate or
URL, only gets that field if the projection already carries it (a column, `id`,
or another predicate's operand). `ListView` and `ObjectGrid` already behave this
way.

⇒ Narrowed in this same release by objectui#10277: the harvest now also names
the row key a `defaultFromRow` parameter seeds from and every bare-identifier
`{field}` token in an action's `target`, through the same declared-field and FLS
gates, so on all three surfaces those two no longer rely on the projection
already carrying the field. (Noted here by the objectui#10277 seat, because this
body publishes verbatim into the CHANGELOG and the paragraph above would
otherwise describe a gap the same release closes.)
34 changes: 34 additions & 0 deletions .changeset/10277-harvest-defaultfromrow-target-tokens.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
'@object-ui/core': patch
---

fix(core): a list's `$select` now carries the field a row action's `defaultFromRow` param seeds from and the `{field}` tokens of its `target`

`listViewPredicates` is the harvest `ListView`, `ObjectGrid` and `RelatedList`'s
authored path use to decide which fields a projected row must carry. It read an
action's `visible` / `disabled` predicates and its `recordIdField`, but not two
other row keys an action reads. On a backend that honours `$select`, a field
named only there was absent from the row, and nothing said so:

- a param with `defaultFromRow: true` bound to a field no column shows opened
the console's param dialog blank, because the param is seeded only when the
row has the key;
- a `{field}` token in an `api` action's `target` was filled with an empty
string by the console's `api` handler.

For each action on the view's row, bulk and object action lists, the harvest now
also names:

- the row key a `defaultFromRow` param seeds from: its `field` when it declares
one, its `name` otherwise. That is the precedence the param seeding reads, so
a `teamId` param bound to `field: 'team_id'` asks for `team_id`;
- every `{field}` token in `target` whose content is a bare identifier, the
grammar the console's `api` handler fills from the row. A dotted path, an
expression and the runner's `${param.X}` / `${ctx.X}` tokens are not filled
from the row that way, and are not harvested.

A param key that is not a bare identifier is dropped, as a `recordIdField` is.
Every harvested name still passes each consumer's existing gates before it
reaches `$select`: the check against the object's declared fields and the
platform columns every record carries, and field-level security on a declared
field once the permission answer has loaded.
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ interface Entry {
const INVENTORY: Record<string, Entry> = {
// ── the column-identity family: EMPTY (was 24 in PR1; 22 after re-triage) ──

// ── two layers, not two spellings (5) ────────────────────────────────────
// ── two layers, not two spellings (6) ────────────────────────────────────
'app-shell/src/utils/resolveActionParams.ts': {
count: 2,
verdict: 'two-layer',
Expand All @@ -143,6 +143,11 @@ const INVENTORY: Record<string, Entry> = {
verdict: 'two-layer',
why: 'Action param name, same layering as resolveActionParams.',
},
'core/src/utils/predicate-fields.ts': {
count: 1,
verdict: 'two-layer',
why: 'The ROW-key half of resolveActionParams\' pair, mirrored on purpose (objectui#10277): `defaultFromRowKey` names the row key a `defaultFromRow` param seeds from, so the `$select` harvest can ask for it. `field ?? name` because row data is keyed by object field, the precedence `rowValueKey` reads. Deliberately NOT `columnIdentity()`: that reader also accepts `fieldName` and treats an empty string as absent, so it would harvest keys the runtime never looks up.',
},

// ── the form cluster — settled the other way (#3090) (3) ─────────────────
'plugin-form/src/ObjectForm.tsx': {
Expand Down Expand Up @@ -260,8 +265,9 @@ describe('column identity dual-read ratchet (#3104)', () => {
// cannot hide a real one.
expect(sum((e) => e.verdict === 'column-identity')).toBe(0);
// 12 at #3104 PR2; 11 since objectui#3174 routed `resolveActionParams`'
// two orders through one named reader each.
expect(sum(() => true)).toBe(11);
// two orders through one named reader each; 12 again since objectui#10277
// mirrored that file's row-key reader into the `$select` harvest.
expect(sum(() => true)).toBe(12);
});

it('records a precedence for every read left in the family', () => {
Expand Down
118 changes: 118 additions & 0 deletions packages/core/src/utils/__tests__/predicate-fields.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,124 @@ describe('listViewPredicates', () => {
});
});

// objectui#10277 — two more row keys an action READS. A `defaultFromRow` param
// is seeded only when the row owns the key, and a `{field}` token in an `api`
// target is filled from the row; on a projected row without the key the first
// came up blank and the second wrote an empty path segment, without a word.
describe('listViewPredicates — `defaultFromRow` params and `target` tokens (objectui#10277)', () => {
/** The named producer's shape: objectstack `sys_team_member.remove_team_member`. */
const REMOVE_TEAM_MEMBER = {
name: 'remove_team_member',
type: 'api',
locations: ['list_item'],
target: '/api/v1/auth/organization/remove-team-member',
params: [
{ name: 'teamId', field: 'team_id', required: true, defaultFromRow: true },
{ name: 'userId', field: 'user_id', required: true, defaultFromRow: true },
],
};
const refs = (view: Parameters<typeof listViewPredicates>[0]) =>
collectPredicateFieldRefs(listViewPredicates(view));

it('harvests the FIELD a `defaultFromRow` param seeds from, not its payload name', () => {
// The runtime reads the row at `field ?? name`; `teamId` is the request-body
// key and is a column nowhere.
expect(refs({ objectActions: [REMOVE_TEAM_MEMBER] })).toEqual(['team_id', 'user_id']);
});

it('falls back to the param `name` when it declares no `field`', () => {
expect(
refs({ rowActionDefs: [{ name: 'a', params: [{ name: 'region', defaultFromRow: true }] }] }),
).toEqual(['region']);
});

it('harvests only the params that seed from the row', () => {
// `seeded` is the control in the same fixture: it proves the harvest ran,
// so the two absent names are a reading rather than a harvest that saw nothing.
expect(
refs({
rowActionDefs: [{
name: 'a',
params: [
{ name: 'reason', field: 'reason' },
{ name: 'note', field: 'note', defaultFromRow: false },
{ name: 'seeded', field: 'seeded', defaultFromRow: true },
],
}],
}),
).toEqual(['seeded']);
});

it('drops a `defaultFromRow` key that is not a bare identifier', () => {
// The same gate `recordIdField` takes: without it the harvester would read a
// PREFIX of the malformed name (`not`) and project a plausible wrong field.
// `ok_field` is the same-fixture control.
expect(
refs({
objectActions: [{
name: 'a',
params: [
{ name: 'not a field', defaultFromRow: true },
{ name: 'ok_field', defaultFromRow: true },
],
}],
}),
).toEqual(['ok_field']);
});

it('harvests every `{field}` token of an action `target`', () => {
expect(
refs({ rowActionDefs: [{ name: 'a', type: 'api', target: '/api/v1/teams/{team_id}/members/{user_id}' }] }),
).toEqual(['team_id', 'user_id']);
});

it('skips a `target` token that is not a bare identifier — the handler does not fill it from the row', () => {
// A dotted path, an expression and the runner's own `${param.X}` /
// `${ctx.X}` scopes are outside the `{field}` grammar; a leading digit is
// not an identifier. Only `{ok}` is read off the row.
expect(
refs({
rowActionDefs: [{
name: 'a',
target: '/x/{owner.name}/{a + b}/${param.token}/${ctx.recordId}/{1st}/{ok}',
}],
}),
).toEqual(['ok']);
});

it('reaches params and tokens on all three action lists, beside the action’s other keys', () => {
expect(
refs({
rowActionDefs: [{
name: 'a',
visible: 'record.active',
recordIdField: 'token',
params: [{ name: 'p', field: 'seed_a', defaultFromRow: true }],
target: '/a/{tok_a}',
}],
bulkActionDefs: [{ name: 'b', params: [{ field: 'seed_b', defaultFromRow: true }], target: '/b/{tok_b}' }],
objectActions: [{ name: 'c', params: [{ field: 'seed_c', defaultFromRow: true }], target: '/c/{tok_c}' }],
}),
).toEqual(['active', 'token', 'seed_a', 'tok_a', 'seed_b', 'tok_b', 'seed_c', 'tok_c']);
});

it('reads nothing out of a runtime value-bag `params` or a malformed entry', () => {
// A non-array `params` is the host's internal value bag (`_rowRecord`,
// collected values), never authored metadata. Action `d` is the
// same-fixture control: the harvest reached this list.
expect(
refs({
rowActionDefs: [
{ name: 'a', params: { _rowRecord: { team_id: 't' } } },
{ name: 'b', params: [null, 42, 'team_id', { field: 42, defaultFromRow: true }] },
{ name: 'c', target: 42 },
{ name: 'd', params: [{ field: 'valid_seed', defaultFromRow: true }] },
],
}),
).toEqual(['valid_seed']);
});
});

describe('isProjectableField', () => {
const declared = { title: { type: 'text' }, owner: { type: 'user' } };

Expand Down
90 changes: 75 additions & 15 deletions packages/core/src/utils/predicate-fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@
* predicate.)
*
* Since objectstack#8018 the same reader also carries the non-predicate row keys
* a view's actions read — today just `recordIdField`. The name kept the
* "predicate" spelling because the mechanism is identical (harvest a name, gate
* it against the declared fields, add it to `$select`); see
* {@link listViewPredicates} for what is in the set and why.
* a view's actions read: `recordIdField`, and since objectui#10277 the field a
* `defaultFromRow` param seeds from and the `{field}` tokens of an action's
* `target`. The name kept the "predicate" spelling because the mechanism is
* identical (harvest a name, gate it against the declared fields, add it to
* `$select`); see {@link listViewPredicates} for what is in the set and why.
*/

/**
Expand Down Expand Up @@ -96,6 +97,32 @@ const RECORD_REF = /\b(?:record|data)\.([A-Za-z_][A-Za-z0-9_]*)/g;
*/
const BARE_IDENTIFIER = /^[A-Za-z_][A-Za-z0-9_]*$/;

/**
* A `{field}` token in an action's `target` — the grammar the console's `api`
* handler (`useConsoleActionRuntime` in `@object-ui/app-shell`) substitutes from
* the row record, spelled exactly as that handler spells it, so the harvest
* reads the same tokens it fills. The capture is a bare identifier by
* construction: a dotted path (`{owner.name}`), an expression (`{a + b}`) and
* the runner's own `${param.X}` / `${ctx.X}` scopes all fall outside it and
* match nothing here — this grammar does not read them off the row, so they owe
* the projection nothing.
*/
const TARGET_ROW_TOKEN = /\{([a-z_][a-z0-9_]*)\}/gi;

/**
* The row key a `defaultFromRow` param seeds its value from — `field` wins,
* because row data is keyed by object field; `name` is the fallback. The same
* precedence `resolveActionParam` (`@object-ui/app-shell`) reads, which is the
* point: the projection must carry the key the runtime looks up, not the
* param's payload name. `remove_team_member` seeds `teamId` from `team_id`.
*/
function defaultFromRowKey(param: unknown): unknown {
if (!param || typeof param !== 'object') return undefined;
const p = param as Record<string, unknown>;
if (!p.defaultFromRow) return undefined;
return p.field ?? p.name;
}

/** Pull the CEL source out of any of the shapes a predicate is authored in. */
function predicateSource(pred: unknown): string | null {
if (typeof pred === 'string') return pred.trim() || null;
Expand Down Expand Up @@ -145,24 +172,42 @@ export function collectPredicateFieldRefs(predicates: readonly unknown[]): strin
* kebab (custom defs AND the built-in Edit/Delete overrides), the selection bar,
* and the object's declared actions.
*
* Most entries are predicates. Two are not, and both are spelled as a synthetic
* Most entries are predicates. Four are not, and all are spelled as a synthetic
* `record.<name>` so the one harvester handles them: conditional formatting's
* native `{ field, operator, value }` shape, and an action's `recordIdField`
* (objectstack#8018). The `recordIdField` case is the same *class* of bug the
* predicate harvest exists to close, one surface over — the action runtime reads
* native `{ field, operator, value }` shape, an action's `recordIdField`
* (objectstack#8018), the field each `defaultFromRow` param seeds from, and each
* `{field}` token of the action's `target` (both objectui#10277). The
* `recordIdField` case is the same *class* of bug the predicate harvest exists
* to close, one surface over — the action runtime reads
* `rowRecord[action.recordIdField]` to seed `recordIdParam`, so a key outside the
* listView columns arrived absent and the injection was silently skipped, which
* turns a record-scoped mutation into one that names no record while still
* reporting success. The default (`id`) is already projected unconditionally, so
* only an explicit declaration adds anything here.
*
* A `recordIdField` that is not a bare identifier is dropped here, and one naming
* a field the object does not declare is dropped by the caller's
* `isProjectableField` guard. Both drops are the safe direction: such a name is
* not a column anywhere, and an unknown key in `$select` is not ignored by every
* backend. The identifier gate is not decoration — without it the harvester's
* regex would read a PREFIX of a malformed name (`record.not a field` → `not`)
* and contribute a plausible wrong field instead of nothing.
* The last two are the same class again, and they were silent in the same way.
* `resolveActionParam` seeds a `defaultFromRow` param only when the row HAS the
* key (an own-property check), so an absent key opened the param dialog with the
* field blank; and the console's `api` handler fills a `{field}` token from the
* row, so an absent key put an empty path segment into the URL. The key a param
* seeds from is `field ?? name` — the runtime's precedence, not the param's
* payload name — and the token grammar is the handler's own. Neither is
* narrowed by the action's `type`, for the asymmetry stated below: an extra
* column costs bytes.
*
* A `recordIdField` or param key that is not a bare identifier is dropped here
* (a `target` token is one by construction of its grammar), and a name the
* object does not declare is dropped by the caller's `isProjectableField`
* guard. Both drops are the safe direction: such a name is not a column
* anywhere, and an unknown key in `$select` is not ignored by every backend.
* The identifier gate is not decoration — without it the harvester's regex would
* read a PREFIX of a malformed name (`record.not a field` → `not`) and
* contribute a plausible wrong field instead of nothing.
*
* ⛔ Harvesting is not a read grant. Every name here is a CANDIDATE: each
* consumer still passes it through its own field-level read filter before it
* reaches `$select`, so a field the principal may not read is not requested
* because an action names it.
*
* `objectActions` is deliberately the object's WHOLE action set rather than
* only the ones this view promotes. Narrowing it would mean re-running the
Expand Down Expand Up @@ -200,6 +245,21 @@ export function listViewPredicates(view: {
if (typeof d.recordIdField === 'string' && BARE_IDENTIFIER.test(d.recordIdField)) {
preds.push(`record.${d.recordIdField}`);
}
// A `defaultFromRow` param's seed field and the `{field}` tokens of the
// action's `target` (objectui#10277) — two more row keys the runtime
// READS, by the same rule and the same identifier gate.
if (Array.isArray(d.params)) {
for (const param of d.params) {
const key = defaultFromRowKey(param);
if (typeof key === 'string' && BARE_IDENTIFIER.test(key)) preds.push(`record.${key}`);
}
}
if (typeof d.target === 'string') {
TARGET_ROW_TOKEN.lastIndex = 0;
for (let m = TARGET_ROW_TOKEN.exec(d.target); m; m = TARGET_ROW_TOKEN.exec(d.target)) {
preds.push(`record.${m[1]}`);
}
}
}
}
for (const override of Object.values(view.userActions ?? {})) {
Expand Down
Loading
Loading