diff --git a/.changeset/22785-import-user-field-target.md b/.changeset/22785-import-user-field-target.md new file mode 100644 index 00000000000..01cc172462b --- /dev/null +++ b/.changeset/22785-import-user-field-target.md @@ -0,0 +1,34 @@ +--- +'@objectstack/core': minor +--- + +fix(core)!: an import reads WHICH fields are references through the spec's arbiter, so a `user` field written without `reference` resolves against `sys_user` (#22785) + +Clause-②: yes (narrowing) + + + +**BREAKING** (an accept-set narrowing and a change to published output), shipped as `minor` under the launch-window convention for breaking changes. Two populations are affected, both only for a `user` field written without `reference`: + +- an import row whose cell holds the id of a user the caller cannot see (a user outside the caller's organizations): stored before, `reference_not_found` now, as on a `Field.user` field; +- a downstream reader of that column in `GET /api/v1/data/:object/export`: the cell held the stored user id and now holds the user's name, as a `Field.user` column already does. + +A `user` field's target is fixed by its type: the spec's arbiter `referenceTargetOf` gives `{ type: 'user' }` the target `sys_user` whether or not `reference` is written (`Field.user()` writes it; hand-written JSON, a form or an AI author may not), and the data door's `$expand` already reads it that way. `buildFieldMetaMap`, the field map behind the import, export and import-template doors, counted a field as a reference only when it carried a `reference` string, so those doors disagreed with the arbiter for such a field. It now asks the arbiter both whether a field is a reference and what its target is. + +**Door by door, before and after.** Measured on a real boot, as an administrator and as a member, on a fixture object holding a `user` field written without `reference` beside a `Field.user` one; the connector pull through its executor over a stub protocol. + +- **Import: `POST /api/v1/data/:object/import`, `POST /api/v1/data/:object/import/jobs`, and a mapping's `connectorSource` pull (same runner).** + - FROM: a cell holding a user's email or name was written as the field's id, so the row failed `reference_not_found` ("no sys_user record has id …") and nothing was stored. + - TO: the cell is matched against `sys_user` exactly as on a `Field.user` field, so an email or a name stores the user's id. A pasted id of a user the caller can see still stores. A value matching no user still fails `reference_not_found`, now in the import's own words ("no record matches …"), and a name matching several users fails `reference_ambiguous`. + - `sys_user` is asked its declared exposure first, as every lookup target is: where its `enable` declaration refuses `list`, the cell is refused. +- **The one population that narrows, to `Field.user`'s answer.** A pasted id of a user the caller's row scope hides (a user outside the caller's organizations) used to store, since the write's own reference check runs elevated. It now fails `reference_not_found`, as it already did on a `Field.user` field. The data door's `POST /api/v1/data/:object` is unchanged and still accepts that id. +- **Export: `GET /api/v1/data/:object/export`.** FROM: the column carried the stored user id (JSON and CSV). TO: it is expanded like a `Field.user` column and renders the user's name, so an exported file imports back through the name. +- **Import template: `GET /api/v1/data/:object/export?template=true`.** FROM: the column's instructions named no target ("The name of a record, or its id."). TO: they name `sys_user` by its label ("The name of a User record, or its id."). +- **Unchanged:** + - `Field.user` and every field declaring `reference`. + - A `lookup`, `master_detail` or `tree` field without `reference`: the arbiter names no target for it. + - A non-reference type declaring `reference`: still no target. + - A present `reference` that is not a string: still no target, and no throw. + - plugin-auth's identity import, which reads `sys_user`'s own map. That map has no reference-typed field without `reference`, so it is identical. + +**Fix, if a pipeline relied on the old answer:** none is needed for one that wrote visible user ids. A pipeline that imported ids of users outside the caller's organizations runs as a caller who can see them, as a `Field.user` field already required. A reader that needs the stored user id takes it from the record read (`GET /api/v1/data/:object`, without `expand`), not from the export. diff --git a/packages/core/src/utils/import-field-meta.ts b/packages/core/src/utils/import-field-meta.ts index fd25270d28b..a1dcc2309a2 100644 --- a/packages/core/src/utils/import-field-meta.ts +++ b/packages/core/src/utils/import-field-meta.ts @@ -18,11 +18,12 @@ export interface ExportFieldMeta { label?: string; options?: Array<{ label?: string; value?: unknown; color?: string }>; /** - * Target object of a field that declares a `reference` string, read through - * the spec's one arbiter `referenceTargetOf` (so a non-reference type names - * none). The import's reference resolution matches cells against it and asks - * it its exposure first (#22739). Only a declared `reference` makes a field - * one: a `user` field written without it keeps no target here, as before. + * Target object of a reference field: WHICH fields are references, and their + * target, are both the spec's one arbiter `referenceTargetOf`'s answer — so a + * non-reference type names none, and a `user` field written without + * `reference` names `sys_user`, as `$expand` reads it (#22785). The import's + * reference resolution matches cells against it and asks it its exposure + * first (#22739). */ reference?: string; /** Field on the referenced record to show as its label. */ @@ -87,7 +88,9 @@ export function buildFieldMetaMap(schema: unknown): Map type: typeof f.type === 'string' ? f.type : undefined, label: typeof f.label === 'string' ? f.label : undefined, options: Array.isArray(f.options) ? f.options : undefined, - reference: typeof f.reference === 'string' ? referenceTargetOf(f) : undefined, + // A present non-string carrier stays out of the arbiter, which throws on + // it: such a field names no target here, as before. + reference: f.reference == null || typeof f.reference === 'string' ? referenceTargetOf(f) : undefined, displayField: typeof f.displayField === 'string' ? f.displayField : undefined, multiple: f.multiple === true, }); diff --git a/packages/core/src/utils/import-runner-reference-exposure.test.ts b/packages/core/src/utils/import-runner-reference-exposure.test.ts index d9d39fc6363..d98b38285ed 100644 --- a/packages/core/src/utils/import-runner-reference-exposure.test.ts +++ b/packages/core/src/utils/import-runner-reference-exposure.test.ts @@ -91,32 +91,51 @@ describe('[#22739] import reference resolution asks the target its declared expo }); /** - * CONTROL — which fields the import treats as references is unchanged: a field - * declaring a `reference` string. A `user` field written without one is NOT - * resolved (its raw cell reaches the write, as on `main`), although the spec's - * arbiter gives it `sys_user`; adopting that target widens the accept set and - * is not this change. A non-reference type declaring `reference` names no target. + * [#22785] WHICH fields are references is the spec arbiter's answer too: a + * `user` field written without `reference` targets `sys_user`, as `$expand` + * reads it, so its cell is resolved like a `Field.user` one. The widened + * target is still asked its exposure first. A non-reference type declaring + * `reference`, and a non-string carrier, name no target (and do not throw). + * The real import and export doors: `import-user-field-target.dogfood.test.ts`. */ -describe('[#22739] the reference field set stays the declared-reference set', () => { - it('a user field without reference passes its cell through unresolved; one declaring it is resolved', async () => { - const meta = buildFieldMetaMap({ fields: { - owner: { type: 'user', label: 'Owner' }, - assignee: { type: 'user', label: 'Assignee', reference: 'sys_user' }, - amount: { type: 'number', label: 'Amount', reference: 'rx_target' }, - } }); - expect([meta.get('owner')!.reference, meta.get('assignee')!.reference, meta.get('amount')!.reference]).toEqual([undefined, 'sys_user', undefined]); - - const findData = vi.fn(async (args: FindArgs) => (args.object === 'sys_user' && Object.values(args.query!.where!)[0] === 'Ann' ? [{ id: 'u_ann' }] : [])); - const getMetaItem = vi.fn(async ({ name }: { type: string; name: string }) => ({ type: 'object', name, item: { name } })); +describe('[#22785] a user field without reference resolves through the arbiter target', () => { + const EMAIL = 'ann@iuf.test'; + const fields = { + owner: { type: 'user', label: 'Owner' }, + assignee: { type: 'user', label: 'Assignee', reference: 'sys_user' }, + amount: { type: 'number', label: 'Amount', reference: 'rx_target' }, + broken: { type: 'user', label: 'Broken', reference: { object: 'sys_user' } }, + }; + const userProtocol = (rows: Array>, enable?: unknown) => { + const findData = vi.fn(async (args: FindArgs) => (args.object === 'sys_user' && (args.query!.where as Record).email === EMAIL ? [{ id: 'u_ann' }] : [])); + const getMetaItem = vi.fn(async ({ name }: { type: string; name: string }) => ({ type: 'object', name, item: { name, ...(enable ? { enable } : {}) } })); const createData = vi.fn(async (a: { data: Record }) => ({ id: 'new', ...a.data })); const p: ImportProtocolLike = { findData, getMetaItem, createData, updateData: vi.fn() }; - const summary = await runImport({ - p, objectName: 'rx_source', metaMap: meta, writeMode: 'insert', matchFields: [], dryRun: false, - runAutomations: false, trimWhitespace: true, createMissingOptions: false, skipBlankMatchKey: false, - rows: [{ owner: 'Ann', assignee: 'Ann' }], + const summary = runImport({ + p, objectName: 'rx_source', metaMap: buildFieldMetaMap({ fields }), writeMode: 'insert', matchFields: [], dryRun: false, + runAutomations: false, trimWhitespace: true, createMissingOptions: false, skipBlankMatchKey: false, rows, + }); + return { summary, findData, getMetaItem, createData }; + }; + const OK = { ok: true, code: undefined, field: undefined }; + + it('names sys_user for both user fields, and no target for a non-reference type or a non-string carrier', () => { + const meta = buildFieldMetaMap({ fields }); + expect(['owner', 'assignee', 'amount', 'broken'].map((f) => meta.get(f)!.reference)).toEqual(['sys_user', 'sys_user', undefined, undefined]); + }); + + for (const field of ['owner', 'assignee'] as const) { + it(`${field === 'owner' ? 'the field without reference' : 'CONTROL Field.user'}: an email stores the user id, sys_user asked once`, async () => { + const { summary, getMetaItem, createData } = userProtocol([{ [field]: EMAIL }]); + expect((await summary).results.map(verdict)).toEqual([OK]); + expect(createData.mock.calls.map(([a]) => a.data)).toEqual([{ [field]: 'u_ann' }]); + expect(getMetaItem.mock.calls.map(([r]) => r.name)).toEqual(['sys_user']); }); - expect(summary.results.map(verdict)).toEqual([{ ok: true, code: undefined, field: undefined }]); - expect(createData.mock.calls[0]![0].data).toMatchObject({ owner: 'Ann', assignee: 'u_ann' }); - expect(getMetaItem.mock.calls.map(([r]) => r.name)).toEqual(['sys_user']); + } + + it('CONTROL — a sys_user the decision does not serve refuses both cells and is never read', async () => { + const { summary, findData } = userProtocol([{ owner: EMAIL }, { assignee: EMAIL }], { apiEnabled: false }); + expect((await summary).results.map(verdict)).toEqual([{ ...NOT_FOUND, field: 'owner' }, { ...NOT_FOUND, field: 'assignee' }]); + expect(findData).not.toHaveBeenCalled(); }); }); diff --git a/packages/qa/dogfood/test/import-user-field-target.dogfood.test.ts b/packages/qa/dogfood/test/import-user-field-target.dogfood.test.ts new file mode 100644 index 00000000000..23156966133 --- /dev/null +++ b/packages/qa/dogfood/test/import-user-field-target.dogfood.test.ts @@ -0,0 +1,115 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#22785] A `user` field written without `reference` is a reference to +// `sys_user` at the import and export doors, as the spec's arbiter +// (`referenceTargetOf`) and the data door's `$expand` read it — on a real boot. +// The `Field.user` field (which writes `reference: 'sys_user'`) is the control: +// its answers do not move. The refusal of an unserved target is pinned in +// core's `import-runner-reference-exposure.test.ts`. +// +// `@objectstack/core` (the field map and the runner) resolves through its +// BUILT output here, so a verdict on a change to it is a verdict on its last +// build. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; + +const SYS = { context: { isSystem: true } } as const; +const ADMIN = 'admin@objectos.ai'; + +const Task = ObjectSchema.create({ + name: 'iuf_task', + sharingModel: 'public_read_write', + label: 'Task', + pluralLabel: 'Tasks', + fields: { + name: Field.text({ label: 'Name', required: true }), + owner: { type: 'user' as const, label: 'Owner' }, + assignee: Field.user({ label: 'Assignee' }), + }, +}); + +const stackDef = defineStack({ + manifest: { + id: 'com.dogfood.import-user-field-target', + namespace: 'iuf', + version: '0.0.0', + type: 'app', + name: 'Import User Field Target Fixture', + description: 'A user field written without reference beside a Field.user one, at the import and export doors.', + }, + objects: [Task], +}); + +/** Read, create and export on the fixture object, so the export door answers. */ +const exporter = PermissionSetSchema.parse({ + name: 'iuf_fixture_exporter', + label: 'Import user field fixture exporter', + objects: { iuf_task: { allowRead: true, allowCreate: true, allowExport: true } }, +}); + +type Row = { ok: boolean; code?: string; field?: string; id?: string }; + +describe('[#22785] a user field without reference resolves to sys_user at the import and export doors', () => { + let stack: VerifyStack; + let token = ''; + let adminId = ''; + let ql: any; + + beforeAll(async () => { + stack = await bootStack(stackDef as never, { + security: new SecurityPlugin({ defaultPermissionSets: [...securityDefaultPermissionSets, exporter], fallbackPermissionSet: exporter.name }), + }); + token = await stack.signIn(); + ql = await stack.kernel.getServiceAsync('objectql'); + adminId = String((await ql.find('sys_user', { where: { email: ADMIN }, ...SYS }))[0].id); + }, 180_000); + + afterAll(async () => { + await stack?.stop?.(); + }); + + const doors = { + sync: async (rows: object[]): Promise => { + const res = await stack.apiAs(token, 'POST', '/data/iuf_task/import', { format: 'json', rows, runAutomations: false }); + expect(res.status, await res.clone().text()).toBe(200); + return ((await res.json()) as { results: Row[] }).results; + }, + jobs: async (rows: object[]): Promise => { + const res = await stack.apiAs(token, 'POST', '/data/iuf_task/import/jobs', { format: 'json', rows, runAutomations: false }); + expect(res.status, await res.clone().text()).toBe(201); + const { jobId } = (await res.json()) as { jobId: string }; + for (let i = 0; i < 200; i++) { + const body: any = await (await stack.apiAs(token, 'GET', `/data/import/jobs/${jobId}/results`)).json(); + if (body.status === 'succeeded') return body.results; + expect(['pending', 'running'], JSON.stringify(body)).toContain(body.status); + await new Promise((r) => setTimeout(r, 25)); + } + throw new Error(`import job ${jobId} did not finish`); + }, + }; + + for (const door of ['sync', 'jobs'] as const) { + for (const field of ['owner', 'assignee'] as const) { + it(`${door}: an email in ${field === 'owner' ? 'the field without reference' : 'CONTROL Field.user'} stores the user id`, async () => { + const [row] = await doors[door]([{ name: `${door}-${field}`, [field]: ADMIN }]); + expect(row!.ok, JSON.stringify(row)).toBe(true); + expect((await ql.find('iuf_task', { where: { id: row!.id }, ...SYS }))[0]?.[field]).toBe(adminId); + }); + } + } + + it('export: the field without reference renders the user as Field.user does, not the stored id', async () => { + await ql.insert('iuf_task', { name: 'export-row', owner: adminId, assignee: adminId }, SYS); + const filter = encodeURIComponent(JSON.stringify({ name: 'export-row' })); + const res = await stack.apiAs(token, 'GET', `/data/iuf_task/export?format=json&filter=${filter}`); + expect(res.status, await res.clone().text()).toBe(200); + const [row] = (await res.json()) as Array>; + expect(row!.assignee, 'CONTROL Field.user renders a name').not.toBe(adminId); + expect(row!.owner).toBe(row!.assignee); + }); +}); diff --git a/packages/rest/src/export-format.test.ts b/packages/rest/src/export-format.test.ts index a0544530be5..826dad0d3ff 100644 --- a/packages/rest/src/export-format.test.ts +++ b/packages/rest/src/export-format.test.ts @@ -18,6 +18,7 @@ import { exportContentDisposition, formatCellValue, buildFieldMetaMap, + referenceFieldNames, type ExportFieldMeta, } from './export-format'; @@ -296,3 +297,19 @@ describe('buildFieldMetaMap — presentation keys only (#6536)', () => { }); }); }); + +/** + * [#22785] The export expands a `user` field written without `reference` like a + * `Field.user` one: the map names its target through the spec's arbiter, so its + * cell renders the user's name instead of the stored id. + */ +describe('referenceFieldNames — the fields the export expands (#22785)', () => { + it('a user field without reference is expanded, as Field.user is; a non-reference type is not', () => { + const map = buildFieldMetaMap({ fields: { + owner: { type: 'user', label: 'Owner' }, + assignee: { type: 'user', label: 'Assignee', reference: 'sys_user' }, + amount: { type: 'number', label: 'Amount', reference: 'contracts' }, + } }); + expect(referenceFieldNames(map)).toEqual(['owner', 'assignee']); + }); +}); diff --git a/packages/rest/src/import-template.test.ts b/packages/rest/src/import-template.test.ts index 2fe16a314e7..94402090f50 100644 --- a/packages/rest/src/import-template.test.ts +++ b/packages/rest/src/import-template.test.ts @@ -440,6 +440,12 @@ describe('describeTemplateColumns', () => { } }); + it('[#22785] a user field without reference names sys_user by its label, as Field.user does', () => { + const schema = { fields: { owner: { name: 'owner', type: 'user', label: 'Owner' } } }; + const [owner] = describeTemplateColumns(schema, ['owner'], { locale: 'en', referenceLabels: new Map([['sys_user', 'User']]) }); + expect(owner.howToFill).toContain('The name of a User record'); + }); + it('a name in ?fields= that is no field of the object is described as such', () => { const [unknown] = describeTemplateColumns(KITCHEN_SINK, ['nope'], { locale: 'en' }); expect(unknown).toMatchObject({ header: 'nope', type: '', howToFill: 'Not a field of this object.' });