Skip to content

Commit 898145b

Browse files
fix(service-datasource): fields of an object drafted from a datasource table carry a readable label (#30530)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgPnWQ3Q5ty932xxY9RSUuZ3)_ domain:server#6 · os-bill · session_01Utp6CXCS98uhsFQYk64epE Refs #30469 (item 1) Clause-②: no Metadata-protocol: none Skills-docs: none — `git grep -n -i -E "datasource (table )?(sync|import)|external object|import_object|generateObjectDraft|object draft" -- skills` on `aad627fd` answers 1 line, `skills/objectstack-data/SKILL.md:47` (the catalog entry for the datasources rule), and `skills/objectstack-data/rules/datasources.md` names no introspect, draft, import or field label; no published sentence describes the draft's field shape. ## 维护者速读 - 改了什么:从外部数据源同步(导入)表生成的对象,每个字段现在都带可读的显示名,例如列 `part_no` → `Part No`、`part_name` → `Part Name`;对象自己的名称不变。 - 为什么改:同步后新建记录表单直接显示数据库列名(`part_no`),而列表里读作人话,每个打开表单的成员都会看到原始列名。 - 风险与回滚:只改生成草稿这一处,复用对象名称已在用的同一个转换函数;已导入的对象不回写,重新导入或手改才变。回滚即 revert 本 PR。 - 先看:`packages/services/service-datasource/src/external-datasource-service.ts`(列循环里的 `label: toLabel(fieldName)`、lookup 保留原 label、源码渲染行),再看 `src/__tests__/external-datasource-service.test.ts` 新增的 `generated field labels`。 - 两分钟验证:`pnpm --filter @objectstack/service-datasource exec vitest run src/__tests__/external-datasource-service.test.ts -t "generated field labels"`,预期 3 条通过。 - 需要你做的:无,席位落地。第 2 项(同步对象该住哪个包、是否随导出走)实测结果见下,ADR-0146 没有直接定论,由席位另立决策卡。 ## What changes - `draftObject` gives every generated field a `label`, derived by `toLabel` from the field name it is saved under, the same snake_case → Title Case function that already labels the object (`toLabel(shortName)`). No second derivation. The label follows the field name after `rename` and after the numbering of a taken name (`PartNo` beside `part_no` → `part_no_2` → `Part No 2`), never the raw remote spelling. - A navigation that becomes a `lookup` keeps the label of the scalar column it replaces. - The rendered `*.object.ts` source writes the label on each field line (`part_no: { type: 'text', label: 'Part No' },`), quoted with the file's existing `quoteTs`. - `importObject` reuses the draft, so the persisted object carries the same labels. The object label is unchanged (control pin). - `DraftField` gains `label: string`. ## Tests - New top-level `describe('generated field labels')` in the package's shared harness file `external-datasource-service.test.ts`: a `spare_parts` table (`id`, `part_no`, `part_name`, `PartNo`) yields `Id`, `Part No`, `Part Name`, `Part No 2`; the object label stays `Spare Parts`; a renamed column is labelled from its new name; the import persists the same labels. - Red on the merge base `0c7e95aff`: 3 failed (`expected { type: 'number' } to deeply equal { type: 'number', label: 'Id' }`, `expected undefined to be 'Catalogue Number'`, `expected undefined to be 'Part No'`). - Eight existing pins asserted the old field shape exactly (`toEqual({ type })`, `Object.keys(record) == ['type']`, rendered field lines). They now name the label too; the primary-key pins still fail on any extra key, so the unauthorable `primaryKey` stays out. - Ablation (once, `scripts/ablation-replace.mjs`, anchor hit ×1, blob restored to HEAD, `git diff HEAD` empty): with the field written as `{ type }` again, the 3 new pins go red. - On a booted showcase (scratch dogfood file, not committed): `POST /datasources/showcase_external/external/tables/customers/import` → 201, and `GET /api/v1/meta/object/showcase_customers` serves `name: Name`, `email: Email`, `region: Region`, `lifetime_value: Lifetime Value`. ## Acceptance notes ### Item 2: where a synced object lives, and whether it exports (measured, not built) Reading on a booted showcase at `fc8cea50` (this branch before the main merge; the incoming commits touch `ui/**` and one script only), after importing `customers` from the showcase's code-declared external datasource: - `GET /api/v1/meta/object` lists `showcase_customers` with `_packageId: "sys_metadata"`, no `_packageVersion`. Control: `showcase_task` lists `_packageId: "com.example.showcase"`, `_packageVersion: "0.1.0"`. This matches the `#30328` reading for a datasource added in Setup. - `GET /api/v1/packages/export` (what `os package export` reads) answers `{"success":true,"data":{"packages":[]}}`. The object is not in it. `GET /api/v1/packages/com.example.showcase/export` answers 200 and does not contain the object either. - Why: the import saves through the metadata door with no `packageId` (`plugin.ts` `persistObject`, `writeFace: 'external-import'`), so the row is a package-less `sys_metadata` row. What ADR-0146 says about it: - D11 (Migrate): "A package-less row for an item **no** package ships is environment metadata, not an overlay, and is not touched." A synced object is an item no package ships, so today's placement is that class. Context item 3 states such a row "does not travel": no package export carries it. - D2 / D3 (ADR-0070 D1): "an item the customer adds 'to HotCRM' lives in a writable package", the base's `BASE.custom`. An import over a datasource a package declares takes that package's namespace prefix (`showcase_customers`), which reads as an item added to that package. The ADR does not say whether such an import is a customization of the base. So the ADR does not settle it. For a datasource added in Setup (no package, no namespace), today's package-less placement matches D11 and the object does not export. For a datasource a package declares, D2 can be read as asking for `BASE.custom`. Nothing is moved here. This is recorded as an out-of-scope finding for the seat to take to a decision. That is why this PR says `Refs`, not a closing keyword. ## Verification <details><summary>Package suite and typecheck (final head)</summary> `pnpm --filter @objectstack/service-datasource run typecheck` (tsc --noEmit, lists all 12 `external-*` test files) and `vitest run --maxWorkers=2`: Test Files 71 passed (71), Tests 1172 passed (1172). </details> <details><summary>Derived gates</summary> Head `aad627fdb` (after the `origin/main` merge). `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 61 commands; each was run with its exit code recorded before any pipe, and `--ran` reconciles: 61 derived, 60 run, 1 NOT-MEASURED, 0 UNRUN. - 59 exit 0 on the first run. - `node scripts/check-plugin-teardown-shape.mjs --self-test`: exit 3 on the shallow checkout (it asks for history), exit 0 after `git fetch origin 621a487…` (48 cases pass). - `pnpm check:dual-build-cjs-loads`: exit 3, PREREQUISITE NOT MET (it needs every package's `dist`, and only this card's closure was built). NOT MEASURED here; CI's `Lint & Repo Gates` runs it. - `pnpm lint` (outside the derivation): exit 0. - Per-file `eslint --no-inline-config` on the six changed TypeScript files: exit 0. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CZyxdo9F9Zo2bX81CM)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4ccd790 commit 898145b

7 files changed

Lines changed: 118 additions & 28 deletions
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/service-datasource': patch
3+
---
4+
5+
fix(service-datasource): an object drafted or imported from an external datasource table gives each field a readable label
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
Every field of a generated object draft (`generateObjectDraft`, `importObject`, `os datasource introspect`) now carries a `label`, derived from the field's name by the same snake_case to Title Case step that already labels the object: a table with columns `part_no` and `part_name` yields fields labelled `Part No` and `Part Name`, so the record form of a synced object no longer shows raw column names. The label follows the name the field is saved under (after `rename` and the numbering of a taken name), a lookup keeps the label of the column it replaces, and the rendered `*.object.ts` source carries the same label. The object's own label is unchanged. Objects imported before this release keep their stored fields; re-import or edit the labels to change them.

‎packages/services/service-datasource/src/__tests__/external-datasource-service.test.ts‎

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ describe('generateObjectDraft', () => {
110110
* Where the key went instead is pinned in
111111
* `external-object-draft-primary-key.test.ts`.
112112
*/
113-
expect(fields.order_id).toEqual({ type: 'text' });
113+
expect(fields.order_id).toEqual({ type: 'text', label: 'Order Id' });
114114
expect(fields.amount.type).toBe('number');
115115
expect(fields.ordered_at.type).toBe('datetime');
116116
expect(fields.metadata.type).toBe('json');
@@ -124,7 +124,7 @@ describe('generateObjectDraft', () => {
124124
expect(draft.source).toContain('REVIEW:');
125125
// Same flip, source side: the rendered field line no longer carries the
126126
// key, and the introspected key survives as the comment ruling D requires.
127-
expect(draft.source).toContain("order_id: { type: 'text' },");
127+
expect(draft.source).toContain("order_id: { type: 'text', label: 'Order Id' },");
128128
expect(draft.source).not.toContain('primaryKey: true');
129129
expect(draft.source).toContain('// Remote primary key: order_id');
130130
});
@@ -224,6 +224,80 @@ describe('importObject', () => {
224224
});
225225
});
226226

227+
/**
228+
* Every generated field carries a readable label, derived by the SAME function
229+
* that labels the object (snake_case → Title Case). Without one the record
230+
* form of a synced object shows the raw column name (`part_no`) as the input's
231+
* label, while the object itself reads as a sentence.
232+
*/
233+
describe('generated field labels', () => {
234+
function sparePartsSchema(): IntrospectedSchema {
235+
return {
236+
dialect: 'sqlite',
237+
introspectedAt: '2026-11-05T00:00:00.000Z',
238+
tables: {
239+
spare_parts: {
240+
name: 'spare_parts',
241+
indexes: [],
242+
columns: [
243+
{ name: 'id', type: 'integer', nullable: false, primaryKey: true },
244+
{ name: 'part_no', type: 'text', nullable: false, primaryKey: false },
245+
{ name: 'part_name', type: 'text', nullable: true, primaryKey: false },
246+
{ name: 'PartNo', type: 'text', nullable: true, primaryKey: false },
247+
],
248+
},
249+
},
250+
};
251+
}
252+
function makeSpareParts() {
253+
const persisted: Array<{ name: string; def: Record<string, unknown> }> = [];
254+
const svc = new ExternalDatasourceService({
255+
introspect: async () => sparePartsSchema(),
256+
getDatasource: async () => ({ name: 'parts_db', schemaMode: 'external' }),
257+
getObject: async () => undefined,
258+
listObjects: async () => [],
259+
persistObject: async (name, def) => { persisted.push({ name, def }); },
260+
});
261+
return { svc, persisted };
262+
}
263+
264+
it('labels each field of the draft from its field name, and the object label is unchanged', async () => {
265+
const { svc } = makeSpareParts();
266+
const draft = await svc.generateObjectDraft('parts_db', 'spare_parts');
267+
const fields = draft.definition.fields as Record<string, { type: string; label?: string }>;
268+
expect(fields.id).toEqual({ type: 'number', label: 'Id' });
269+
expect(fields.part_no).toEqual({ type: 'text', label: 'Part No' });
270+
expect(fields.part_name).toEqual({ type: 'text', label: 'Part Name' });
271+
// A column that is not snake_case is labelled from the field name it is
272+
// saved under (`PartNo` → `part_no`, taken → `part_no_2`), never from the
273+
// raw remote spelling.
274+
expect(fields.part_no_2).toEqual({ type: 'text', label: 'Part No 2' });
275+
// Control: the object label is derived exactly as before.
276+
expect(draft.definition.label).toBe('Spare Parts');
277+
// The rendered source carries the same label the definition does.
278+
expect(draft.source).toContain("part_no: { type: 'text', label: 'Part No' },");
279+
});
280+
281+
it('a renamed column is labelled from the name it is renamed to', async () => {
282+
const { svc } = makeSpareParts();
283+
const draft = await svc.generateObjectDraft('parts_db', 'spare_parts', {
284+
includeColumns: ['part_no'],
285+
rename: { part_no: 'catalogue_number' },
286+
});
287+
const fields = draft.definition.fields as Record<string, { label?: string }>;
288+
expect(fields.catalogue_number?.label).toBe('Catalogue Number');
289+
});
290+
291+
it('the persisted object of an import carries the same field labels', async () => {
292+
const { svc, persisted } = makeSpareParts();
293+
await svc.importObject('parts_db', 'spare_parts');
294+
const def = persisted[0]!.def as { label: string; fields: Record<string, { label?: string }> };
295+
expect(def.fields.part_no?.label).toBe('Part No');
296+
expect(def.fields.part_name?.label).toBe('Part Name');
297+
expect(def.label).toBe('Spare Parts');
298+
});
299+
});
300+
227301
/**
228302
* ADR-0028 namespace prefix on the `opts.name` OVERRIDE path (#11061).
229303
*

‎packages/services/service-datasource/src/__tests__/external-object-draft-odata-metadata.test.ts‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,12 @@ describe('an OData entity set drafts with EDM types mapped and snake_case fields
9090

9191
expect(draft.name).toBe('products');
9292
expect(draft.definition.fields).toEqual({
93-
product_id: { type: 'number' },
94-
product_name: { type: 'text' },
95-
supplier_id: { type: 'number' },
96-
category_id: { type: 'number' },
97-
unit_price: { type: 'number' },
98-
discontinued: { type: 'boolean' },
93+
product_id: { type: 'number', label: 'Product Id' },
94+
product_name: { type: 'text', label: 'Product Name' },
95+
supplier_id: { type: 'number', label: 'Supplier Id' },
96+
category_id: { type: 'number', label: 'Category Id' },
97+
unit_price: { type: 'number', label: 'Unit Price' },
98+
discontinued: { type: 'boolean', label: 'Discontinued' },
9999
});
100100
expect(draft.definition.external).toEqual({
101101
remoteName: 'Products',
@@ -169,7 +169,7 @@ describe('navigations become lookups only where a column holds the key of an imp
169169
supplier_id: { type: 'number' },
170170
});
171171
expect(draft.review.map((r) => r.column)).toEqual(['Supplier']);
172-
expect(draft.source).toContain(" category_id: { type: 'lookup', reference: 'categories' },");
172+
expect(draft.source).toContain(" category_id: { type: 'lookup', label: 'Category Id', reference: 'categories' },");
173173
expect(draft.source).toContain("columnMap: { 'ProductID': 'product_id',");
174174
});
175175

@@ -306,12 +306,12 @@ describe('an OData v2 datasource imports the same way: Associations become looku
306306
const { service } = serviceOverV2();
307307
const products = await service.generateObjectDraft('erp', 'Products');
308308
expect(products.definition.fields).toEqual({
309-
product_id: { type: 'number' },
310-
product_name: { type: 'text' },
311-
supplier_id: { type: 'number' },
312-
category_id: { type: 'number' },
313-
unit_price: { type: 'number' },
314-
discontinued: { type: 'boolean' },
309+
product_id: { type: 'number', label: 'Product Id' },
310+
product_name: { type: 'text', label: 'Product Name' },
311+
supplier_id: { type: 'number', label: 'Supplier Id' },
312+
category_id: { type: 'number', label: 'Category Id' },
313+
unit_price: { type: 'number', label: 'Unit Price' },
314+
discontinued: { type: 'boolean', label: 'Discontinued' },
315315
});
316316
const employees = await service.generateObjectDraft('erp', 'Employees');
317317
expect(employees.definition.fields).toMatchObject({ birth_date: { type: 'datetime' } });

‎packages/services/service-datasource/src/__tests__/external-object-draft-os-build.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,8 @@ describe('still-generates — the fix must not be satisfied by emitting a valid
190190
expect(fields.name.type).toBe('text');
191191
expect(fields.signed_up_at.type).toBe('datetime');
192192

193-
expect(draft.source).toContain("id: { type: 'text' }");
194-
expect(draft.source).toContain("signed_up_at: { type: 'datetime' }");
193+
expect(draft.source).toContain("id: { type: 'text', label: 'Id' }");
194+
expect(draft.source).toContain("signed_up_at: { type: 'datetime', label: 'Signed Up At' }");
195195
});
196196

197197
it('keeps the external binding pointed at the REMOTE table, not the renamed object', async () => {

‎packages/services/service-datasource/src/__tests__/external-object-draft-primary-key.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ describe('direction 1 — the unauthorable key is ABSENT from the definition', (
122122
});
123123

124124
for (const [name, record] of fieldRecords(draft)) {
125-
expect(Object.keys(record), `field '${name}'`).toEqual(['type']);
125+
expect(Object.keys(record), `field '${name}'`).toEqual(['type', 'label']);
126126
}
127127
// Spelled twice on purpose: the loop above forbids ANY extra key, this
128128
// line names the one the defect was about, so a regression reports it.
@@ -133,15 +133,15 @@ describe('direction 1 — the unauthorable key is ABSENT from the definition', (
133133
const draft = await svc().generateObjectDraft('warehouse', 'orders');
134134

135135
const fields = draft.definition.fields as Record<string, object>;
136-
expect(Object.keys(fields.order_id)).toEqual(['type']);
136+
expect(Object.keys(fields.order_id)).toEqual(['type', 'label']);
137137
expect(JSON.stringify(draft.definition)).not.toContain('primaryKey');
138138
});
139139

140140
it('renders no `primaryKey:` onto the field LINE either', async () => {
141141
const draft = await svc().generateObjectDraft('warehouse', 'orders', {
142142
primaryKey: ['order_id'],
143143
});
144-
expect(draft.source).toContain("order_id: { type: 'text' },");
144+
expect(draft.source).toContain("order_id: { type: 'text', label: 'Order Id' },");
145145
expect(draft.source).not.toContain('primaryKey: true');
146146
});
147147
});

‎packages/services/service-datasource/src/__tests__/external-object-draft-real-introspection.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,9 @@ describe('generateObjectDraft, driven off a real SqlDriver.introspectSchema()',
105105
// …and it is the introspected key, not "the first column": a table whose
106106
// key is not first is covered below.
107107
expect(draft.definition.fields).toEqual({
108-
id: { type: 'text' },
109-
name: { type: 'text' },
110-
age: { type: 'number' },
108+
id: { type: 'text', label: 'Id' },
109+
name: { type: 'text', label: 'Name' },
110+
age: { type: 'number', label: 'Age' },
111111
});
112112

113113
// The key survives as a comment and NOWHERE else. `fields.<f>.primaryKey`

‎packages/services/service-datasource/src/external-datasource-service.ts‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -411,8 +411,12 @@ function navigationCandidates(table: IntrospectedTable): NavigationCandidate[] {
411411
return out;
412412
}
413413

414-
/** A field of a generated draft: a scalar, or a lookup over a foreign-key column. */
415-
type DraftField = { type: FieldType; reference?: string };
414+
/**
415+
* A field of a generated draft: a scalar, or a lookup over a foreign-key column.
416+
* `label` is derived from the field name by {@link toLabel}, the one derivation
417+
* the object's own label uses, so a synced object's forms read as sentences.
418+
*/
419+
type DraftField = { type: FieldType; label: string; reference?: string };
416420

417421
/**
418422
* The org-wide default every generated federated object declares.
@@ -915,7 +919,9 @@ export class ExternalDatasourceService implements IExternalDatasourceService {
915919
}
916920

917921
const isPk = pkOverride ? pkOverride.has(col.name) : col.primaryKey;
918-
fields[fieldName] = { type: fieldType };
922+
// Labelled from the FIELD name (after rename and numbering), never the
923+
// raw remote spelling: the same derivation as the object's label below.
924+
fields[fieldName] = { type: fieldType, label: toLabel(fieldName) };
919925
if (isPk) primaryKeyFields.push(fieldName);
920926
}
921927

@@ -1067,7 +1073,7 @@ export class ExternalDatasourceService implements IExternalDatasourceService {
10671073
leftOut(`'${target}' is not imported on datasource '${datasource}'; import it before this table to get the lookup`);
10681074
continue;
10691075
}
1070-
fields[field] = { type: 'lookup', reference };
1076+
fields[field] = { type: 'lookup', label: fields[field]!.label, reference };
10711077
}
10721078
}
10731079

@@ -1527,7 +1533,7 @@ function renderObjectSource(
15271533
const note = reviewByField.get(fieldName);
15281534
const comment = note ? ` // REVIEW: ${note}` : '';
15291535
const reference = f.reference ? `, reference: ${quoteTs(f.reference)}` : '';
1530-
return ` ${fieldName}: { type: '${f.type}'${reference} },${comment}`;
1536+
return ` ${fieldName}: { type: '${f.type}', label: ${quoteTs(f.label)}${reference} },${comment}`;
15311537
});
15321538
const unplacedLines = unplaced.map((r) => ` // REVIEW: ${r.column} (${r.remoteType}): ${r.note}`);
15331539

0 commit comments

Comments
 (0)