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
11 changes: 11 additions & 0 deletions .changeset/29946-record-page-user-field-related-list.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
'@objectstack/rest': patch
---

fix(rest): the one-request record page accepts a related list keyed by a `user` field again

Clause-②: no
Metadata-protocol: none

- **Regression in 18.3.0 (#29946, duly#185).** `POST /data/:object/:id/page` (#27713) checked each related list's field against a private `lookup`/`master_detail` list and read `reference` raw, so a related list on `sys_user` keyed by a `user` field (an owner, an assignee) answered `400 VALIDATION_FAILED … is not a lookup or master-detail field that references 'sys_user'`, and the console's full-page record view showed "could not load this record". The related list's own row query (`{ [field]: id }`) follows that field, and 18.2.0 read each related list through that list read, with no type check.
- The check now asks the spec's one arbiter of a field's target, `referenceTargetOf` (`lookup`, `master_detail`, `user`, `tree`; a `user` field's target is fixed to `sys_user` by its type, whether or not its `reference` spells it), the same judgement the engine's `$expand` and delete walk read. A field that does not reference the page's object is still a located `400 invalid_relationship`: a `user` field on any other object's page, a `lookup` to another object, a `tree` on another object, an unreadable `reference` carrier. The message now reads `… is not a lookup, master-detail, user or tree field that references '<object>'.`
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,15 @@ const CALLERS: Record<string, Classification> = {
"judged as such) and the engine's system delete. It reads those records back only through the harness's " +
'system read, to see whether the delete removed them, and serves no field of the target to any caller',
},
// [#29946] The record page's related-list check.
'packages/rest/src/record-page-routes.ts': {
kind: 'not-a-served-read',
reason:
"compares a related list's field target with the page's own object, only to admit the entry or refuse it " +
'(`invalid_relationship` / 400) before anything is read; it reads no record of the target. The target is the ' +
"ADDRESSED object, whose record is `getData` behind the route's own `get` exposure gate, and the related rows " +
"are the child's `findData` behind the child's `list` exposure. Pinned by `packages/rest/src/record-page-routes.test.ts`",
},
};

const SCAN_TIMEOUT_MS = 60_000;
Expand Down
68 changes: 68 additions & 0 deletions packages/rest/src/record-page-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,74 @@ describe('[#27627] POST /data/:object/:id/page', () => {
expect(findData).not.toHaveBeenCalled();
});

describe('[#29946] a user field relates its child to sys_user, as the row query follows it', () => {
const SYS_USER = { name: 'sys_user', label: 'User', fields: { id: { name: 'id', type: 'text' } } };
const TASK = {
name: 'task',
label: 'Task',
fields: {
id: { name: 'id', type: 'text' },
// A bare `Field.user` as a raw registration spells it: no carrier.
owner: { name: 'owner', type: 'user' },
// ...and as the spec's parse materializes it.
assignee: { name: 'assignee', type: 'user', reference: 'sys_user' },
reviewers: { name: 'reviewers', type: 'user', multiple: true },
parent: { name: 'parent', type: 'tree', reference: 'task' },
account: { name: 'account', type: 'lookup', reference: 'account' },
},
};
const items = { ...ITEMS, sys_user: SYS_USER, task: TASK };

it('a related list on sys_user keyed by a user field answers 200 with its rows (duly#185)', async () => {
const { call, findData } = boot({ items });
const answer = await call(
{ related: [{ object: 'task', field: 'owner', top: 5 }, { object: 'task', field: 'assignee', top: 5 }, { object: 'task', field: 'reviewers', top: 5 }] },
{},
{ object: 'sys_user', id: 'u9' },
);
expect(answer.status).toBe(200);
const data = RecordPageResponseSchema.parse(answer.body.data);
expect(data.related).toEqual([
expect.objectContaining({ object: 'task', field: 'owner', total: 7, records: [{ id: 'task-1' }] }),
expect.objectContaining({ object: 'task', field: 'assignee', total: 7 }),
expect.objectContaining({ object: 'task', field: 'reviewers', total: 7 }),
]);
expect(findData.mock.calls.map(([req]: any) => req.query.where)).toEqual([
{ owner: 'u9' },
{ assignee: 'u9' },
{ reviewers: { $contains: 'u9' } },
]);
});

it('a user field on a page that is not sys_user is still a located 400, naming the kinds it accepts', async () => {
const { call, getData, findData } = boot({ items });
const answer = await call({ related: [{ object: 'task', field: 'owner', top: 1 }] });
expect(answer.status).toBe(400);
expect(answer.body.error.details.fields).toEqual([{
field: 'related.0.field',
code: 'invalid_relationship',
message: "'owner' on 'task' is not a lookup, master-detail, user or tree field that references 'account'.",
}]);
expect(getData).not.toHaveBeenCalled();
expect(findData).not.toHaveBeenCalled();
});

it('a lookup keeps relating only to its own reference, and a tree to its own object', async () => {
const { call } = boot({ items });
expect((await call({ related: [{ object: 'task', field: 'account', top: 1 }] })).status).toBe(200);
expect((await call({ related: [{ object: 'task', field: 'account', top: 1 }] }, {}, { object: 'sys_user', id: 'u9' })).status).toBe(400);
expect((await call({ related: [{ object: 'task', field: 'parent', top: 1 }] }, {}, { object: 'task', id: 't1' })).status).toBe(200);
expect((await call({ related: [{ object: 'task', field: 'parent', top: 1 }] })).status).toBe(400);
});

it('an unreadable reference carrier is not a relationship: the located 400, not a thrown 500', async () => {
const odd = { ...TASK, fields: { ...TASK.fields, bad: { name: 'bad', type: 'lookup', reference: { object: 'account' } } } };
const answer = await boot({ items: { ...items, task: odd } }).call({ related: [{ object: 'task', field: 'bad', top: 1 }] });
expect(answer.status).toBe(400);
expect(answer.body.error.details.fields).toEqual([expect.objectContaining({ code: 'invalid_relationship' })]);
});
});

it('a declared filter the list read refuses is a located 400 before anything is read', async () => {
const { call, getData, findData } = boot();
const answer = await call({ related: [{ object: 'note', field: 'account', top: 1, filter: { title: { $in: 'x' } } }] });
Expand Down
36 changes: 26 additions & 10 deletions packages/rest/src/record-page-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import {
type FindDataRequest,
type RecordPageRequest,
} from '@objectstack/spec/api';
import { isMultiValueField, parseFilterAST } from '@objectstack/spec/data';
import { isMultiValueField, parseFilterAST, referenceTargetOf } from '@objectstack/spec/data';
import { measureServerTiming } from '@objectstack/observability';
import { sendError, sendOk } from '@objectstack/types';
import { recordNotFoundError } from '@objectstack/core';
Expand All @@ -54,9 +54,6 @@ import { apiAccessDenialFromEnable, type RestServer } from './rest-server.js';
/** The closed query-parameter set of `POST {basePath}/data/:object/:id/page`: EMPTY. */
export const RECORD_PAGE_PARAMS: readonly string[] = [];

/** The relationship field types a related list follows back to its parent. */
const RELATIONSHIP_TYPES = new Set(['lookup', 'master_detail']);

/** [#28470] The objects the record's stream parts read. */
const COMMENT_OBJECT = 'sys_comment';
const COMMENT_REACTION_OBJECT = 'sys_comment_reaction';
Expand Down Expand Up @@ -120,10 +117,29 @@ function fieldDefOf(item: any, name: string): any | undefined {
return undefined;
}

/** The target object a relationship field declares (`reference`, the spec's one spelling). */
function referenceOf(def: any): string | undefined {
const ref = def?.reference;
return typeof ref === 'string' && ref ? ref : undefined;
/**
* [#29946] The object a related list's field points back at, by the spec's ONE
* arbiter of a field's target ({@link referenceTargetOf}): the reference value
* types (`lookup`, `master_detail`, `user`, `tree`), with a `user` field's
* target fixed to `sys_user` by its type whatever its carrier spells. It is the
* judgement the engine's own reads ask (`$expand`, the referential delete
* walk), so the related list's row query (`{ [field]: id }`) and this check
* cannot disagree about which fields relate a child to its parent.
*
* 18.3.0 kept a private `lookup`/`master_detail` list here and read `reference`
* raw, so a related list on `sys_user` keyed by a `user` field (an owner, an
* assignee) was a 400 `invalid_relationship` the row query would have answered.
*
* A carrier no reader can read (an object- or array-valued `reference` that
* never went through parse) throws in the arbiter; here it is simply not a
* relationship to this object, the located 400 it was before.
*/
function referenceOf(def: unknown): string | undefined {
try {
return referenceTargetOf(def);
} catch {
return undefined;
}
}

/**
Expand Down Expand Up @@ -201,11 +217,11 @@ export function registerRecordPageEndpoints(this: RecordPageRouteHost, basePath:
const item = childItems[index];
if (!item || apiAccessDenialFromEnable(item.enable, entry.object, 'list')) return;
const def = fieldDefOf(item, entry.field);
if (!def || !RELATIONSHIP_TYPES.has(String(def.type)) || referenceOf(def) !== object) {
if (!def || referenceOf(def) !== object) {
malformed.push({
field: `related.${index}.field`,
code: 'invalid_relationship',
message: `'${entry.field}' on '${entry.object}' is not a lookup or master-detail field that references '${object}'.`,
message: `'${entry.field}' on '${entry.object}' is not a lookup, master-detail, user or tree field that references '${object}'.`,
});
}
});
Expand Down
Loading