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
16 changes: 16 additions & 0 deletions .changeset/24939-bu-parent-delete-restrict.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
'@objectstack/platform-objects': minor
---

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

Clause-②: no (narrowing)

<!-- 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). -->

**BREAKING** (the delete of a business unit with children is refused), shipped as `minor` under the launch-window convention for accept-set narrowings.

- **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.
- **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.
- **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.
- **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.
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,7 @@ export const enObjects: NonNullable<TranslationData['objects']> = {
},
parent_business_unit_id: {
label: "Parent Business Unit",
help: "Self-reference for the org tree. Null = root of tenant."
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."
},
timezone: {
label: "Timezone",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,7 @@ export const esESObjects: NonNullable<TranslationData['objects']> = {
},
parent_business_unit_id: {
label: "Unidad de negocio principal",
help: "Autorreferencia para el árbol organizativo. Null = raíz del tenant."
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."
},
timezone: {
label: "Zona horaria",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,7 @@ export const jaJPObjects: NonNullable<TranslationData['objects']> = {
},
parent_business_unit_id: {
label: "親ビジネスユニット",
help: "組織ツリーの自己参照。null = テナントのルート。"
help: "組織ツリーの自己参照。null = テナントのルート。子ユニットが残っているユニットは、子ユニットを別の親に付け替えるか削除するまで削除できません。"
},
timezone: {
label: "タイムゾーン",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,7 @@ export const zhCNObjects: NonNullable<TranslationData['objects']> = {
},
parent_business_unit_id: {
label: "上级业务单元",
help: "组织树的自关联字段。Null 表示租户根节点。"
help: "组织树的自关联字段。Null 表示租户根节点。仍有下级单元的业务单元无法删除,须先将下级单元改挂到其他上级或删除。"
},
timezone: {
label: "时区",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,19 @@ export const SysBusinessUnit = ObjectSchema.create({
),

// ── Hierarchy ────────────────────────────────────────────────
// `restrict`, not the lookup default `set_null`: deleting a unit that still
// has child units is refused (`409 DELETE_RESTRICTED`, naming this object and
// the child count) until they are re-parented or deleted. Under `set_null`
// the delete answered 200 and silently made each child a root of the
// tenant, so a `unit_and_subordinates` grant anchored above the deleted
// unit stopped reaching the former subtree. A delete that removes the
// children first (leaf to root) still completes.
parent_business_unit_id: Field.lookup('sys_business_unit', {
label: 'Parent Business Unit',
required: false,
description: 'Self-reference for the org tree. Null = root of tenant.',
deleteBehavior: 'restrict',
description:
'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.',
group: 'Hierarchy',
}),

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#24939] Deleting a business unit that still has child units is REFUSED.
*
* `sys_business_unit.parent_business_unit_id` used to declare no
* `deleteBehavior`, so it took the lookup default `set_null`: deleting a unit
* answered 200 and silently made each child a root of the tenant. The tree is
* what a `unit_and_subordinates` sharing rule walks
* (`BusinessUnitGraphService.descendants`), so a grant anchored above the
* deleted unit stopped reaching the former subtree, with no word to the
* administrator. The field now declares `restrict`.
*
* Measured on a REAL engine (ObjectQL + SqlDriver over in-memory SQLite)
* carrying the SHIPPED `SysBusinessUnit` from `@objectstack/platform-objects`,
* because the refusal is the engine's referential pass reading the built
* declaration — a fake engine would pin only itself. It lives here rather than
* in `platform-objects`, which has no engine or driver among its dependencies;
* plugin-sharing has both, and owns the subtree walk the refusal protects.
*
* Every platform deleter of this object is pinned to still complete on a tree:
* - by id (`DELETE /api/v1/data/sys_business_unit/:id`): a leaf deletes;
* - in bulk (`deleteManyData` and `batchData`'s delete case, a per-id loop in
* the caller's order, its `atomic` arm one transaction): leaf to root
* completes, root first refuses the root alone;
* - an organization's delete: `organization_id` is a plain lookup, so the
* units stay and keep their tree.
*/

import { describe, it, expect, afterEach } from 'vitest';
import { ObjectQL } from '@objectstack/objectql';
import { SqlDriver } from '@objectstack/driver-sql';
import { SysBusinessUnit, SysOrganization } from '@objectstack/platform-objects/identity';

const SYS = { isSystem: true, userId: 'usr_admin' } as any;
const ORG = 'org_qa';

const openEngines: ObjectQL[] = [];
afterEach(async () => {
while (openEngines.length) {
try { await openEngines.pop()?.destroy(); } catch { /* noop */ }
}
});

/** A real engine carrying the shipped business-unit (and organization) objects. */
async function boot(): Promise<ObjectQL> {
const driver = new SqlDriver({
client: 'better-sqlite3',
connection: { filename: ':memory:' },
useNullAsDefault: true,
});
await driver.connect();
const engine = new ObjectQL();
openEngines.push(engine);
engine.registerDriver(driver as never, true);
await engine.init();
for (const def of [SysOrganization, SysBusinessUnit]) {
engine.registry.registerObject(def as never, 'platform-objects', 'sys');
}
await engine.syncSchemas();
// The organization every unit belongs to: with `sys_organization` registered,
// a system write on a tenant-scoped object must carry one.
await engine.insert('sys_organization', { id: ORG, name: 'QA Org', slug: 'qa-org' }, { context: SYS });
return engine;
}

/** The card's tree, one level deeper: QA EMEA → QA EMEA Office → QA EMEA Desk. */
async function seedTree(engine: ObjectQL): Promise<void> {
for (const [id, name, parent] of [
['bu_emea', 'QA EMEA', null],
['bu_office', 'QA EMEA Office', 'bu_emea'],
['bu_desk', 'QA EMEA Desk', 'bu_office'],
] as const) {
await engine.insert(
'sys_business_unit',
{ id, name, kind: 'department', parent_business_unit_id: parent, organization_id: ORG },
{ context: SYS },
);
}
}

/** `id → parent id` for every unit still present. */
async function tree(engine: ObjectQL): Promise<Record<string, string | null>> {
const rows = (await engine.find('sys_business_unit', { where: {}, context: SYS })) as Array<Record<string, any>>;
return Object.fromEntries(rows.map((r) => [r.id, r.parent_business_unit_id ?? null]));
}

const del = (engine: ObjectQL, id: string, context: any = SYS) =>
engine.delete('sys_business_unit', { where: { id }, context });

describe('#24939 a business unit with child units refuses its delete (real engine, shipped object)', () => {
it('declares restrict on the parent link', () => {
expect((SysBusinessUnit.fields as any).parent_business_unit_id.deleteBehavior).toBe('restrict');
});

it('refuses a unit with children with 409 DELETE_RESTRICTED, and its children keep their parent', async () => {
const engine = await boot();
await seedTree(engine);

const err: any = await del(engine, 'bu_emea').catch((e) => e);
expect(err).toBeInstanceOf(Error);
expect(err.code).toBe('DELETE_RESTRICTED');
expect(err.status).toBe(409);
// What tells the administrator to re-parent first: the blocking object
// and how many of its rows hold the link.
expect(err.dependentObject).toBe('sys_business_unit');
expect(err.dependentCount).toBe(1);

expect(await tree(engine)).toEqual({ bu_emea: null, bu_office: 'bu_emea', bu_desk: 'bu_office' });
});

it('deletes a leaf unit', async () => {
const engine = await boot();
await seedTree(engine);

await expect(del(engine, 'bu_desk')).resolves.toBe(true);
expect(await tree(engine)).toEqual({ bu_emea: null, bu_office: 'bu_emea' });
});

it('a unit whose children were re-parented deletes, and the children keep their new parent', async () => {
const engine = await boot();
await seedTree(engine);
await engine.update(
'sys_business_unit',
{ id: 'bu_desk', parent_business_unit_id: 'bu_emea' },
{ where: { id: 'bu_desk' }, context: SYS } as any,
);

await expect(del(engine, 'bu_office')).resolves.toBe(true);
expect(await tree(engine)).toEqual({ bu_emea: null, bu_desk: 'bu_emea' });
});

it('bulk, leaf to root (the per-id loop of deleteManyData / batchData) completes on the tree', async () => {
const engine = await boot();
await seedTree(engine);

for (const id of ['bu_desk', 'bu_office', 'bu_emea']) await expect(del(engine, id)).resolves.toBe(true);
expect(await tree(engine)).toEqual({});
});

it('bulk, leaf to root, in one transaction (the atomic arm) completes on the tree', async () => {
const engine = await boot();
await seedTree(engine);

await engine.transaction(async (trxCtx: any) => {
for (const id of ['bu_desk', 'bu_office', 'bu_emea']) await del(engine, id, trxCtx);
}, SYS);
expect(await tree(engine)).toEqual({});
});

it('bulk, root first: the root alone is refused, and its subtree is left whole', async () => {
const engine = await boot();
await seedTree(engine);

const first: any = await del(engine, 'bu_emea').catch((e) => e);
expect(first.code).toBe('DELETE_RESTRICTED');
expect(first.status).toBe(409);
expect(await tree(engine)).toEqual({ bu_emea: null, bu_office: 'bu_emea', bu_desk: 'bu_office' });
});

it("an organization's delete completes on a tree: its units stay, and keep their parents", async () => {
const engine = await boot();
await seedTree(engine);

await expect(engine.delete('sys_organization', { where: { id: ORG }, context: SYS })).resolves.toBe(true);
expect(await tree(engine)).toEqual({ bu_emea: null, bu_office: 'bu_emea', bu_desk: 'bu_office' });
});
});
Loading