Skip to content

Commit 4b8f597

Browse files
fix(platform-objects)!: a business unit with child units refuses its delete instead of silently re-rooting them (#24984)
Fixes #24939 Clause-②: no (narrowing) Dispatched by the `domain:engine` seat 1 (claim 6300791873); built by dev session `session_01VBFXArkqADbAp54xq6RAZE`. The narrowing face owes a `CONTRACT_REVIEW_TIER` review of record before the queue. ## What changes - `packages/platform-objects/src/identity/sys-business-unit.object.ts`: `parent_business_unit_id` declares `deleteBehavior: 'restrict'`. Nothing else in the object moves. - Its help text becomes true: "Self-reference for the org tree. Null = root of tenant. A unit that still has child units cannot be deleted until they are re-parented or deleted." A comment beside the field says why `restrict` and not the default `set_null`. - The `en` bundle is regenerated (`node scripts/check-i18n-bundles.mjs --write`); the hand-written `zh-CN` / `ja-JP` / `es-ES` help strings are translated to match (values only, no key moves). - Changeset: `minor` for `@objectstack/platform-objects`, **BREAKING** with FROM / TO / Fix, the `Clause-②` line and the `adr-0087: not-required (no-migration-prescription)` marker (`check-adr-0087-registration` green). ## Before / after, measured on the wire Fresh showcase backend (`pnpm dev -- --fresh`) at this branch's head, as the seeded admin, the card's steps. The before column is the card's own measurement on 18.0.0 and, on `main` `f32d795ce5`, a real-engine probe with the shipped object (delete answered `true`, the child's parent became `null`): | request | before (main, `set_null`) | after (this PR) | |:--|:--|:--| | `DELETE /api/v1/data/sys_business_unit/bu_qa_emea` (has 1 child) | `200`; the child's parent becomes `null` (card's measurement, and re-measured on a real engine below) | `409`, `code: DELETE_RESTRICTED`, `dependentObject: sys_business_unit`, `dependentCount: 1`, `error`: "This Business Unit is still referenced by 1 Business Unit record(s) through “Parent Business Unit”. Delete or reassign them first." The child keeps `parent_business_unit_id = bu_qa_emea`. | | `DELETE …/bu_field_ops` (seeded, 2 children) | `200`, both children re-rooted (inferred from the engine probe, not re-measured on the wire) | `409 DELETE_RESTRICTED`, `dependentCount: 2` | | `POST …/deleteMany` `{"ids":["bu_qa_emea","bu_qa_office"]}` (root first) | `200`, both deleted (inferred, not re-measured) | `200` with `success: false`: row 0 `DELETE_RESTRICTED` (409), row 1 `NOT_ATTEMPTED` (the batch stops, as `deleteMany` always does without `continueOnError`); nothing deleted | | `POST …/deleteMany` `{"ids":["bu_qa_office","bu_qa_emea"],"options":{"atomic":true}}` (leaf to root) | `200` (inferred) | `200`, `succeeded: 2` | | `DELETE …/bu_west_coast` (leaf) | `200` (inferred) | `200` | The refusal names the blocking object and the child count, so the administrator knows to re-parent first. How the console displays it is objectui's (read only, not changed here). ## Every deleter of `sys_business_unit`, read before the edit - **Data API by id** (`DELETE /data/:object/:id`) → `engine.delete` by id → `cascadeDeleteRelations` → refused while children exist. - **Bulk** (`POST /data/:object/deleteMany`, `batchData`'s delete case): a per-id loop in the caller's order (`runDeleteManyLoop`), `atomic` = one transaction. Leaf to root completes; root first refuses the root (measured above). A `multi: true` engine delete addressed by id was also refused on a real engine. - **One cascade:** nothing cascades into `sys_business_unit` (its `organization_id` is a plain lookup, `set_null`), so no cascade set ever holds units. Were one to, the engine's #22305 rule already judges `restrict` only against rows outside the cascade set. - **An organization's delete:** `organization_id` is `set_null`, so the units stay and keep their tree (pinned below). No organization teardown deletes units. The v18 ownership ceremony (`os migrate organization-ownership`) gives `sys_business_unit` fate `attribution`, never a delete. - **Sample-data purge** (`cloud-connection` `marketplace-install-local-purge.ts`): deletes through the engine in the REVERSE of the loader's insert order. The showcase seed lists the tree parent first, so the purge deletes leaf to root and completes. See open note 2. - **Seeds / resets / `os` commands / plugin-sharing:** the seed loader never deletes; no `os` command deletes units; plugin-sharing binds `afterDelete` recomputes only, which a refused delete never reaches. - **Tests and fixtures:** the objectql suites that delete a `sys_business_unit` (`engine-delete-restricted-locale`, `engine-cascade-federated-tenant-anchor`) register their own fixture objects; `bu-tree-recompute.test.ts` uses a fake engine. No existing pin moves. The stop condition was not met: no platform path deletes a parent before its children in a way `restrict` breaks. ## Pins `packages/plugins/plugin-sharing/src/business-unit-parent-delete-restrict.test.ts` (new): a REAL engine (ObjectQL + SqlDriver over in-memory SQLite) carrying the SHIPPED `SysBusinessUnit` and `SysOrganization`. It lives in plugin-sharing because `platform-objects` has no engine or driver among its dependencies; plugin-sharing already depends on both and owns the subtree walk the refusal protects. No recompute code is touched. 1. the field declares `restrict`; 2. a unit with children: `code` `DELETE_RESTRICTED`, `status` 409, `dependentObject`, `dependentCount` 1; the tree is unchanged; 3. a leaf deletes; 4. a unit whose child was re-parented deletes, and the child keeps its new parent; 5. bulk leaf to root completes; 6. the same in one transaction (the atomic arm) completes; 7. bulk root first: the root alone is refused, its subtree whole; 8. an organization's delete completes on a tree, the units and their parents intact. **Ablation** (`scripts/ablation-replace.mjs` + `scripts/ablation-dist-preflight.mjs`, platform-objects rebuilt on each leg because plugin-sharing reads its `dist/`): removing `deleteBehavior: 'restrict'` (anchor 1 → 0, dist marker absent from all 66 built files) turns pins 1, 2 and 7 red (3 failed / 5 passed). Restored: blob `c68f61b6c63a` equals `HEAD`, whole-tree `git status` clean, dist marker present again, 8/8 green. ## Verification record (head `cbcab4e10`, origin/main `3a7a60af8` merged) - `pnpm --filter @objectstack/platform-objects typecheck` exit 0; `test` exit 0 (82 files, 1232 tests). - `pnpm --filter @objectstack/plugin-sharing typecheck` exit 0 (test-typecheck included); `test` exit 0 (53 files, 1199 tests). - `pnpm --filter @objectstack/objectql test` exit 0 (450 files, 8490 tests). - `pnpm --filter @objectstack/core test:repo` exit 0 (6 files, 82 tests). - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (no paths): 68 lines, each run with its exit code recorded before any pipe; 67 exit 0 first time, `pnpm check:dual-build-cjs-loads` exit 3 (PREREQUISITE NOT MET, no `dist/` for 10 packages) → full `turbo run build` (77/77) → re-run exit 0. `--ran`: 68 derived, 68 run, 0 NOT-MEASURED, 0 UNRUN, exit 0. - `node scripts/check-adr-0087-registration.mjs` exit 0. - `node scripts/check-i18n-bundles.mjs --write` exit 0 after building its closure (first attempt exit 3, PREREQUISITE NOT MET, built and re-run). ## Acceptance notes 1. **Bulk order is the caller's.** A bulk delete that lists a parent before its child now refuses the parent (and, without `continueOnError`, stops the batch). The console's bulk delete sends ids in the grid's order, which is objectui's; leaf to root completes. 2. **Sample-data purge on a seed authored child before parent.** The purge deletes in reverse record order within one self-referencing dataset; a seed listing a child unit before its parent would have its parent refused (counted in the purge's `errors`, which already names "a restrict" as a cause). Every shipped seed of this object (`examples/app-showcase`) lists parents first. 3. **Docs.** `content/docs/permissions/administrator-guide.mdx` Step 1 says nothing false (it does not describe deleting a unit); a sentence on retiring a unit leaf-first would help. A `docs/qa/platform-checklist/**` item for the refusal would be `domain:devx`'s. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01VBFXArkqADbAp54xq6RAZE --- _Generated by [Claude Code](https://claude.ai/code/session_01VBFXArkqADbAp54xq6RAZE)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f90ea67 commit 4b8f597

7 files changed

Lines changed: 198 additions & 5 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
'@objectstack/platform-objects': minor
3+
---
4+
5+
fix(platform-objects)!: deleting a business unit that still has child units is refused, instead of silently turning each child into a root of the tenant
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) One shipped object field, `sys_business_unit.parent_business_unit_id`, gains `deleteBehavior: 'restrict'`, a key every lookup was always free to set, and its help text says what that means. No spec key, authorable spelling, export, type or stored shape is added, removed or renamed, and no stored row or column is read or rewritten, so `objectstack migrate meta` has nothing to rewrite. What narrows is the accept set of one delete: a business unit that still has child units is refused where it was accepted. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers this field (not already-registered or registered); and the change is metadata on a shipped object, not an interface, a type or an export (not runtime-interface-only, type-surface-only or runtime-export-retired). -->
10+
11+
**BREAKING** (the delete of a business unit with children is refused), shipped as `minor` under the launch-window convention for accept-set narrowings.
12+
13+
- **FROM:** `parent_business_unit_id` declared no `deleteBehavior`, so it took the lookup default `set_null`. Deleting a business unit that had child units answered `200`, and each child's `parent_business_unit_id` became `null`: a root of the tenant, without a word. A `unit_and_subordinates` sharing rule anchored above the deleted unit stopped reaching the former children.
14+
- **TO:** the same delete answers `409 DELETE_RESTRICTED`, naming the blocking object (`dependentObject: 'sys_business_unit'`) and the number of child units (`dependentCount`), with the sentence "This Business Unit is still referenced by N Business Unit record(s) through “Parent Business Unit”. Delete or reassign them first." The unit and its children are left exactly as they were. The field's help text now says a unit with child units cannot be deleted until they are re-parented or deleted.
15+
- **Unchanged:** a unit with no child units deletes as before. A bulk delete (`deleteMany`, `batch`) that lists the children before their parent (leaf to root) completes, atomic or not. Deleting an organization leaves its units and their tree in place, as before.
16+
- **Fix:** before deleting a business unit, re-parent its child units (edit each child's Parent Business Unit) or delete them first. A bulk delete of a whole subtree lists the units leaf to root; listed root first, the root is refused.

‎packages/platform-objects/src/apps/translations/en.objects.generated.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -930,7 +930,7 @@ export const enObjects: NonNullable<TranslationData['objects']> = {
930930
},
931931
parent_business_unit_id: {
932932
label: "Parent Business Unit",
933-
help: "Self-reference for the org tree. Null = root of tenant."
933+
help: "Self-reference for the org tree. Null = root of tenant. A unit that still has child units cannot be deleted until they are re-parented or deleted."
934934
},
935935
timezone: {
936936
label: "Timezone",

‎packages/platform-objects/src/apps/translations/es-ES.objects.generated.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -930,7 +930,7 @@ export const esESObjects: NonNullable<TranslationData['objects']> = {
930930
},
931931
parent_business_unit_id: {
932932
label: "Unidad de negocio principal",
933-
help: "Autorreferencia para el árbol organizativo. Null = raíz del tenant."
933+
help: "Autorreferencia para el árbol organizativo. Null = raíz del tenant. Una unidad que aún tiene unidades hijas no se puede eliminar hasta que se reasignen a otro padre o se eliminen."
934934
},
935935
timezone: {
936936
label: "Zona horaria",

‎packages/platform-objects/src/apps/translations/ja-JP.objects.generated.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -930,7 +930,7 @@ export const jaJPObjects: NonNullable<TranslationData['objects']> = {
930930
},
931931
parent_business_unit_id: {
932932
label: "親ビジネスユニット",
933-
help: "組織ツリーの自己参照。null = テナントのルート。"
933+
help: "組織ツリーの自己参照。null = テナントのルート。子ユニットが残っているユニットは、子ユニットを別の親に付け替えるか削除するまで削除できません。"
934934
},
935935
timezone: {
936936
label: "タイムゾーン",

‎packages/platform-objects/src/apps/translations/zh-CN.objects.generated.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -930,7 +930,7 @@ export const zhCNObjects: NonNullable<TranslationData['objects']> = {
930930
},
931931
parent_business_unit_id: {
932932
label: "上级业务单元",
933-
help: "组织树的自关联字段。Null 表示租户根节点。"
933+
help: "组织树的自关联字段。Null 表示租户根节点。仍有下级单元的业务单元无法删除,须先将下级单元改挂到其他上级或删除。"
934934
},
935935
timezone: {
936936
label: "时区",

‎packages/platform-objects/src/identity/sys-business-unit.object.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,19 @@ export const SysBusinessUnit = ObjectSchema.create({
128128
),
129129

130130
// ── Hierarchy ────────────────────────────────────────────────
131+
// `restrict`, not the lookup default `set_null`: deleting a unit that still
132+
// has child units is refused (`409 DELETE_RESTRICTED`, naming this object and
133+
// the child count) until they are re-parented or deleted. Under `set_null`
134+
// the delete answered 200 and silently made each child a root of the
135+
// tenant, so a `unit_and_subordinates` grant anchored above the deleted
136+
// unit stopped reaching the former subtree. A delete that removes the
137+
// children first (leaf to root) still completes.
131138
parent_business_unit_id: Field.lookup('sys_business_unit', {
132139
label: 'Parent Business Unit',
133140
required: false,
134-
description: 'Self-reference for the org tree. Null = root of tenant.',
141+
deleteBehavior: 'restrict',
142+
description:
143+
'Self-reference for the org tree. Null = root of tenant. A unit that still has child units cannot be deleted until they are re-parented or deleted.',
135144
group: 'Hierarchy',
136145
}),
137146

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#24939] Deleting a business unit that still has child units is REFUSED.
5+
*
6+
* `sys_business_unit.parent_business_unit_id` used to declare no
7+
* `deleteBehavior`, so it took the lookup default `set_null`: deleting a unit
8+
* answered 200 and silently made each child a root of the tenant. The tree is
9+
* what a `unit_and_subordinates` sharing rule walks
10+
* (`BusinessUnitGraphService.descendants`), so a grant anchored above the
11+
* deleted unit stopped reaching the former subtree, with no word to the
12+
* administrator. The field now declares `restrict`.
13+
*
14+
* Measured on a REAL engine (ObjectQL + SqlDriver over in-memory SQLite)
15+
* carrying the SHIPPED `SysBusinessUnit` from `@objectstack/platform-objects`,
16+
* because the refusal is the engine's referential pass reading the built
17+
* declaration — a fake engine would pin only itself. It lives here rather than
18+
* in `platform-objects`, which has no engine or driver among its dependencies;
19+
* plugin-sharing has both, and owns the subtree walk the refusal protects.
20+
*
21+
* Every platform deleter of this object is pinned to still complete on a tree:
22+
* - by id (`DELETE /api/v1/data/sys_business_unit/:id`): a leaf deletes;
23+
* - in bulk (`deleteManyData` and `batchData`'s delete case, a per-id loop in
24+
* the caller's order, its `atomic` arm one transaction): leaf to root
25+
* completes, root first refuses the root alone;
26+
* - an organization's delete: `organization_id` is a plain lookup, so the
27+
* units stay and keep their tree.
28+
*/
29+
30+
import { describe, it, expect, afterEach } from 'vitest';
31+
import { ObjectQL } from '@objectstack/objectql';
32+
import { SqlDriver } from '@objectstack/driver-sql';
33+
import { SysBusinessUnit, SysOrganization } from '@objectstack/platform-objects/identity';
34+
35+
const SYS = { isSystem: true, userId: 'usr_admin' } as any;
36+
const ORG = 'org_qa';
37+
38+
const openEngines: ObjectQL[] = [];
39+
afterEach(async () => {
40+
while (openEngines.length) {
41+
try { await openEngines.pop()?.destroy(); } catch { /* noop */ }
42+
}
43+
});
44+
45+
/** A real engine carrying the shipped business-unit (and organization) objects. */
46+
async function boot(): Promise<ObjectQL> {
47+
const driver = new SqlDriver({
48+
client: 'better-sqlite3',
49+
connection: { filename: ':memory:' },
50+
useNullAsDefault: true,
51+
});
52+
await driver.connect();
53+
const engine = new ObjectQL();
54+
openEngines.push(engine);
55+
engine.registerDriver(driver as never, true);
56+
await engine.init();
57+
for (const def of [SysOrganization, SysBusinessUnit]) {
58+
engine.registry.registerObject(def as never, 'platform-objects', 'sys');
59+
}
60+
await engine.syncSchemas();
61+
// The organization every unit belongs to: with `sys_organization` registered,
62+
// a system write on a tenant-scoped object must carry one.
63+
await engine.insert('sys_organization', { id: ORG, name: 'QA Org', slug: 'qa-org' }, { context: SYS });
64+
return engine;
65+
}
66+
67+
/** The card's tree, one level deeper: QA EMEA → QA EMEA Office → QA EMEA Desk. */
68+
async function seedTree(engine: ObjectQL): Promise<void> {
69+
for (const [id, name, parent] of [
70+
['bu_emea', 'QA EMEA', null],
71+
['bu_office', 'QA EMEA Office', 'bu_emea'],
72+
['bu_desk', 'QA EMEA Desk', 'bu_office'],
73+
] as const) {
74+
await engine.insert(
75+
'sys_business_unit',
76+
{ id, name, kind: 'department', parent_business_unit_id: parent, organization_id: ORG },
77+
{ context: SYS },
78+
);
79+
}
80+
}
81+
82+
/** `id → parent id` for every unit still present. */
83+
async function tree(engine: ObjectQL): Promise<Record<string, string | null>> {
84+
const rows = (await engine.find('sys_business_unit', { where: {}, context: SYS })) as Array<Record<string, any>>;
85+
return Object.fromEntries(rows.map((r) => [r.id, r.parent_business_unit_id ?? null]));
86+
}
87+
88+
const del = (engine: ObjectQL, id: string, context: any = SYS) =>
89+
engine.delete('sys_business_unit', { where: { id }, context });
90+
91+
describe('#24939 a business unit with child units refuses its delete (real engine, shipped object)', () => {
92+
it('declares restrict on the parent link', () => {
93+
expect((SysBusinessUnit.fields as any).parent_business_unit_id.deleteBehavior).toBe('restrict');
94+
});
95+
96+
it('refuses a unit with children with 409 DELETE_RESTRICTED, and its children keep their parent', async () => {
97+
const engine = await boot();
98+
await seedTree(engine);
99+
100+
const err: any = await del(engine, 'bu_emea').catch((e) => e);
101+
expect(err).toBeInstanceOf(Error);
102+
expect(err.code).toBe('DELETE_RESTRICTED');
103+
expect(err.status).toBe(409);
104+
// What tells the administrator to re-parent first: the blocking object
105+
// and how many of its rows hold the link.
106+
expect(err.dependentObject).toBe('sys_business_unit');
107+
expect(err.dependentCount).toBe(1);
108+
109+
expect(await tree(engine)).toEqual({ bu_emea: null, bu_office: 'bu_emea', bu_desk: 'bu_office' });
110+
});
111+
112+
it('deletes a leaf unit', async () => {
113+
const engine = await boot();
114+
await seedTree(engine);
115+
116+
await expect(del(engine, 'bu_desk')).resolves.toBe(true);
117+
expect(await tree(engine)).toEqual({ bu_emea: null, bu_office: 'bu_emea' });
118+
});
119+
120+
it('a unit whose children were re-parented deletes, and the children keep their new parent', async () => {
121+
const engine = await boot();
122+
await seedTree(engine);
123+
await engine.update(
124+
'sys_business_unit',
125+
{ id: 'bu_desk', parent_business_unit_id: 'bu_emea' },
126+
{ where: { id: 'bu_desk' }, context: SYS } as any,
127+
);
128+
129+
await expect(del(engine, 'bu_office')).resolves.toBe(true);
130+
expect(await tree(engine)).toEqual({ bu_emea: null, bu_desk: 'bu_emea' });
131+
});
132+
133+
it('bulk, leaf to root (the per-id loop of deleteManyData / batchData) completes on the tree', async () => {
134+
const engine = await boot();
135+
await seedTree(engine);
136+
137+
for (const id of ['bu_desk', 'bu_office', 'bu_emea']) await expect(del(engine, id)).resolves.toBe(true);
138+
expect(await tree(engine)).toEqual({});
139+
});
140+
141+
it('bulk, leaf to root, in one transaction (the atomic arm) completes on the tree', async () => {
142+
const engine = await boot();
143+
await seedTree(engine);
144+
145+
await engine.transaction(async (trxCtx: any) => {
146+
for (const id of ['bu_desk', 'bu_office', 'bu_emea']) await del(engine, id, trxCtx);
147+
}, SYS);
148+
expect(await tree(engine)).toEqual({});
149+
});
150+
151+
it('bulk, root first: the root alone is refused, and its subtree is left whole', async () => {
152+
const engine = await boot();
153+
await seedTree(engine);
154+
155+
const first: any = await del(engine, 'bu_emea').catch((e) => e);
156+
expect(first.code).toBe('DELETE_RESTRICTED');
157+
expect(first.status).toBe(409);
158+
expect(await tree(engine)).toEqual({ bu_emea: null, bu_office: 'bu_emea', bu_desk: 'bu_office' });
159+
});
160+
161+
it("an organization's delete completes on a tree: its units stay, and keep their parents", async () => {
162+
const engine = await boot();
163+
await seedTree(engine);
164+
165+
await expect(engine.delete('sys_organization', { where: { id: ORG }, context: SYS })).resolves.toBe(true);
166+
expect(await tree(engine)).toEqual({ bu_emea: null, bu_office: 'bu_emea', bu_desk: 'bu_office' });
167+
});
168+
});

0 commit comments

Comments
 (0)