diff --git a/.changeset/24939-bu-parent-delete-restrict.md b/.changeset/24939-bu-parent-delete-restrict.md new file mode 100644 index 00000000000..0a40b5e0482 --- /dev/null +++ b/.changeset/24939-bu-parent-delete-restrict.md @@ -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) + + + +**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. diff --git a/packages/platform-objects/src/apps/translations/en.objects.generated.ts b/packages/platform-objects/src/apps/translations/en.objects.generated.ts index 0876b874cf8..6d2afeae3a6 100644 --- a/packages/platform-objects/src/apps/translations/en.objects.generated.ts +++ b/packages/platform-objects/src/apps/translations/en.objects.generated.ts @@ -930,7 +930,7 @@ export const enObjects: NonNullable = { }, 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", diff --git a/packages/platform-objects/src/apps/translations/es-ES.objects.generated.ts b/packages/platform-objects/src/apps/translations/es-ES.objects.generated.ts index 34db3f2150a..edbcf7f5d33 100644 --- a/packages/platform-objects/src/apps/translations/es-ES.objects.generated.ts +++ b/packages/platform-objects/src/apps/translations/es-ES.objects.generated.ts @@ -930,7 +930,7 @@ export const esESObjects: NonNullable = { }, 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", diff --git a/packages/platform-objects/src/apps/translations/ja-JP.objects.generated.ts b/packages/platform-objects/src/apps/translations/ja-JP.objects.generated.ts index 0b7797b6a4a..4df7c58f277 100644 --- a/packages/platform-objects/src/apps/translations/ja-JP.objects.generated.ts +++ b/packages/platform-objects/src/apps/translations/ja-JP.objects.generated.ts @@ -930,7 +930,7 @@ export const jaJPObjects: NonNullable = { }, parent_business_unit_id: { label: "親ビジネスユニット", - help: "組織ツリーの自己参照。null = テナントのルート。" + help: "組織ツリーの自己参照。null = テナントのルート。子ユニットが残っているユニットは、子ユニットを別の親に付け替えるか削除するまで削除できません。" }, timezone: { label: "タイムゾーン", diff --git a/packages/platform-objects/src/apps/translations/zh-CN.objects.generated.ts b/packages/platform-objects/src/apps/translations/zh-CN.objects.generated.ts index 65c34bca849..0cf8748071c 100644 --- a/packages/platform-objects/src/apps/translations/zh-CN.objects.generated.ts +++ b/packages/platform-objects/src/apps/translations/zh-CN.objects.generated.ts @@ -930,7 +930,7 @@ export const zhCNObjects: NonNullable = { }, parent_business_unit_id: { label: "上级业务单元", - help: "组织树的自关联字段。Null 表示租户根节点。" + help: "组织树的自关联字段。Null 表示租户根节点。仍有下级单元的业务单元无法删除，须先将下级单元改挂到其他上级或删除。" }, timezone: { label: "时区", diff --git a/packages/platform-objects/src/identity/sys-business-unit.object.ts b/packages/platform-objects/src/identity/sys-business-unit.object.ts index 57e4f4e62ed..c68f61b6c63 100644 --- a/packages/platform-objects/src/identity/sys-business-unit.object.ts +++ b/packages/platform-objects/src/identity/sys-business-unit.object.ts @@ -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', }), diff --git a/packages/plugins/plugin-sharing/src/business-unit-parent-delete-restrict.test.ts b/packages/plugins/plugin-sharing/src/business-unit-parent-delete-restrict.test.ts new file mode 100644 index 00000000000..4146339089b --- /dev/null +++ b/packages/plugins/plugin-sharing/src/business-unit-parent-delete-restrict.test.ts @@ -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 { + 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 { + 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> { + const rows = (await engine.find('sys_business_unit', { where: {}, context: SYS })) as Array>; + 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' }); + }); +});