From fd835e3cac5a67be5c4f69c36356611e1c7bc331 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 07:29:01 +0000 Subject: [PATCH 1/3] Give an applied duty its owner's business unit, and say when there is none Apply catalog read only the position-assignment anchor, and no demo assignment is anchored, so on 18.2 every applied demo duty had no unit (13 of 13). Apply runs as system, so the hand-made duty stamp from #178 never covers it. The handler now resolves the unit in the seat's R-A order: the owner's primary business unit, then a position anchor only when there is no primary and the anchored rows name one unit, otherwise none. Each created entry carries business_unit, the result counts unanchored duties and names their owners, and a closed outcome picks the console toast through outcomeMessages (en and zh-CN). Sync is unchanged. Fixes #179 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01JQwxbYga1GGouLwJs9WbD3 --- src/actions/catalog.actions.ts | 12 ++ src/actions/catalog.handlers.ts | 157 ++++++++++++++++------ src/objects/duty.object.ts | 6 +- src/translations/authored-text.ts | 12 +- src/translations/zh-CN.ts | 14 ++ test/catalog-apply-unit.test.ts | 207 +++++++++++++++++++++++++++++ test/catalog-engine-facade.test.ts | 52 ++++++-- test/catalog-instantiate.test.ts | 124 ++++++++++++++--- test/duty-provenance.test.ts | 5 +- 9 files changed, 514 insertions(+), 75 deletions(-) create mode 100644 test/catalog-apply-unit.test.ts diff --git a/src/actions/catalog.actions.ts b/src/actions/catalog.actions.ts index 9d868c4..a2dc734 100644 --- a/src/actions/catalog.actions.ts +++ b/src/actions/catalog.actions.ts @@ -80,6 +80,18 @@ export const CatalogApplyAction = defineAction({ // could reach the route could mint a duty for any `sys_user` id they typed, // in bulk. Granted by the `duly_admin` permission set (src/security/). requiredPermissions: ['duly.catalog.apply'], + // The toast after a run, picked by the handler's `outcome` (#179). The + // handler returns facts — counts and a closed outcome — and the copy lives + // here, so it is translated like every other label. The one the reader must + // not miss is `created_unanchored`: a duty with no business unit drops out + // of every per-unit reading, so the toast says how many and why, and the + // result's `unanchored_owners` says who. Inherited by the twin below. + outcomeMessages: { + created: 'Created ${result.created} duties. ${result.skipped} already existed and were skipped.', + created_unanchored: + 'Created ${result.created} duties. ${result.unanchored} have no business unit, because their owner belongs to none. ${result.skipped} already existed and were skipped.', + nothing_created: 'Nothing new: all ${result.skipped} duties already existed.', + }, params: [ { name: 'position_code', diff --git a/src/actions/catalog.handlers.ts b/src/actions/catalog.handlers.ts index c5ee88b..d1a6038 100644 --- a/src/actions/catalog.handlers.ts +++ b/src/actions/catalog.handlers.ts @@ -120,8 +120,9 @@ export function resolveDutyTimezone(): string { // envelope became `{ where: { where: { … } } }`: no row has a field called // `where`, so the read came back EMPTY WITH NO ERROR. `duly_catalog_apply` // reported a successful run of zero, `duly_catalog_sync` scanned nothing and -// called every duty unchanged, and `resolveBusinessUnit` anchored no duty at -// all — silently, because "no position row" is a legitimate day-one state. +// called every duty unchanged, and the business-unit read (now +// `resolveBusinessUnits`) anchored no duty at all — silently, because "no +// position row" is a legitimate day-one state. // // Filed upstream as objectstack-ai/objectstack#14175; the platform's answer // (#19223, in 17.5) was to withdraw the bare filter rather than declare it, so @@ -157,15 +158,36 @@ export interface CatalogApplyEntry { outcome: 'created' | 'skipped'; /** Id of the duty created by THIS run. Absent for a skip. */ duty?: string; + /** + * The unit the created duty was anchored to, or `null` when its owner + * belongs to none (#179). Absent for a skip: a skipped pair wrote nothing. + */ + business_unit?: string | null; } +/** + * How the run went, as one closed fact the console picks its toast by + * (`outcomeMessages` on the two apply actions, `catalog.actions.ts`). + * + * - `created`: every duty this run created has a business unit. + * - `created_unanchored`: at least one has none, because its owner belongs to + * no unit. The toast says how many; `unanchored_owners` says who. + * - `nothing_created`: every pair already had its duty. + */ +export type CatalogApplyOutcome = 'created' | 'created_unanchored' | 'nothing_created'; + export interface CatalogApplyResult { action: typeof CATALOG_APPLY_ACTION; + outcome: CatalogApplyOutcome; position_code: string; catalog_items: number; users: number; created: number; skipped: number; + /** Duties created by this run with no business unit. */ + unanchored: number; + /** The owners those duties belong to, each once: the people with no unit. */ + unanchored_owners: string[]; entries: CatalogApplyEntry[]; } @@ -266,46 +288,96 @@ export function pairKey(catalogItem: unknown, owner: unknown): string { // ── Business unit ─────────────────────────────────────────────────────────── /** - * The duty's rollup anchor: the business unit of the person's position - * assignment. + * The duty's rollup anchor: the unit the owner's work reports to (#179). * - * `sys_user_position.business_unit_id` is a real, nullable lookup to - * `sys_business_unit` — "[ADR-0090 Addendum] Assignment-level BU anchor: where - * this position assignment applies … Null = unanchored". The object lives in - * `@objectstack/plugin-security`, not `@objectstack/platform-objects`. + * Resolved in the order the seat ruled on #179 (R-A), first answer wins: * - * Returns `undefined` when the person has no position row or no anchored one, - * and the duty is then created WITHOUT a business unit. That is deliberate: - * `position_code` is free text so a customer can load their catalog before - * positions are modelled, which means "no `sys_user_position` row" is a normal - * day-one state and must not fail the apply. + * 1. **The owner's primary business unit** — + * `sys_user.primary_business_unit_id`, plugin-sharing's projection of the + * person's `is_primary` `sys_business_unit_member` row ("the canonical one + * for reporting"; `is_primary` defaults to `true`). It is the rung every + * other derived unit in Duly reads: a hand-made duty + * (`duly_duty_unit_stamp`, #178), a hand-made task (#176) and the + * assignment fan-out. So a person's applied duties and their own duties + * land in the same unit. + * 2. **A position-assignment anchor** — `sys_user_position.business_unit_id` + * ("where this position assignment applies … Null = unanchored", + * ADR-0090 Addendum) — only when there is no primary unit, and only when + * the anchored rows name ONE unit. Those rows are the platform's + * capability positions (`duly_member`, `duly_manager`, …), not the + * catalog's free-text `position_code`, and nothing joins the two, so two + * different anchors are ambiguous and this does not pick one. Before #179 + * this was the only rung and it took the first anchored row of any + * position; measured on 18.2, that put a person's catalog duties in a + * unit their primary membership and their hand-made duties disagree with. + * 3. **None.** The duty is created without a unit — `position_code` is free + * text and a person may be in no unit yet, so this must not fail the + * apply — and the run SAYS so: the entry carries `business_unit: null`, + * the result counts it in `unanchored` and names the owner in + * `unanchored_owners`, and the outcome is `created_unanchored`, which is + * the toast the console shows. * - * (`sys_user.primary_business_unit_id` is a second, user-level anchor that - * exists on the platform. It is NOT read here — the issue names the - * assignment-level anchor, and adding a fallback rung is a product decision, - * reported rather than taken.) + * Membership is not a rung of its own: the primary unit already is its + * projection, and the one case it would add — a person whose memberships are + * all ticked non-primary — is ambiguous by the admin's own choice. * - * ⚠️ That tolerance is why this read's query shape matters more than the other - * three. The other reads fail into a visibly empty report — zero items, zero - * scanned — but this one fails into a state the handler is WRITTEN to accept: - * an envelope-shaped filter returned nothing, "nothing" reads as "not yet - * modelled", and every duty was created unanchored with no error anywhere. The - * rollups that the business unit exists to feed were simply empty. See the + * Why the handler resolves it rather than the duty stamp hook: an action's + * engine runs as system (`buildActionExecutionContext` sets `isSystem`), and + * `duly_duty_unit_stamp` deliberately leaves every system write alone. Same + * first rung, two writers: the hook for a caller's insert, this for Apply. + * + * Two reads for the whole run, not two per person: `sys_user` and + * `sys_user_position` with `$in` on the selected people. + * + * ⚠️ The tolerance in rung 3 is why these reads' query shape matters more + * than the others. They fail into a state the handler is WRITTEN to accept: + * an envelope-shaped filter that returned nothing reads as "nobody has a + * unit", and every duty would be created unanchored with no error anywhere — + * which is exactly what an unreported null looked like before #179. See the * facade-shape note above; the end-to-end coverage is in * `test/catalog-engine-facade.test.ts`. */ -async function resolveBusinessUnit( +export async function resolveBusinessUnits( engine: ActionEngineFacade, - userId: string, -): Promise { - const rows = await engine.find('sys_user_position', { where: { user_id: userId } }); - for (const row of rows) { - // A person can hold several positions; take the first anchored one. + userIds: readonly string[], +): Promise> { + const resolved = new Map(); + for (const id of userIds) resolved.set(id, null); + if (userIds.length === 0) return resolved; + + const users = await engine.find('sys_user', { where: { id: { $in: [...userIds] } } }); + for (const user of users) { + const id = idOf(user?.id); + const primary = idOf(user?.primary_business_unit_id); + if (resolved.has(id) && primary) resolved.set(id, primary); + } + + const anchors = new Map>(); + const positions = await engine.find('sys_user_position', { where: { user_id: { $in: [...userIds] } } }); + for (const row of positions) { + const userId = idOf(row?.user_id); // Unanchored rows (`null`) are legacy/tenant-wide and carry no depth. - const anchor = row?.business_unit_id; - if (typeof anchor === 'string' && anchor) return anchor; + const anchor = idOf(row?.business_unit_id); + if (!userId || !anchor) continue; + const set = anchors.get(userId) ?? new Set(); + set.add(anchor); + anchors.set(userId, set); + } + for (const [userId, set] of anchors) { + if (resolved.get(userId) !== null) continue; + if (set.size === 1) resolved.set(userId, [...set][0]!); } - return undefined; + return resolved; +} + +/** A lookup arrives as an id or, expanded, as a record carrying one. */ +function idOf(value: unknown): string { + if (typeof value === 'string') return value; + if (value !== null && typeof value === 'object') { + const id = (value as { id?: unknown }).id; + if (typeof id === 'string') return id; + } + return ''; } // ── duly_catalog_apply ────────────────────────────────────────────────────── @@ -337,12 +409,11 @@ export const applyCatalogHandler: ActionHandler = async (ctx if (typeof item === 'string' && item) taken.add(pairKey(item, duty?.owner)); } - // Resolved once per person, not once per (item, person): a 26-item catalog - // for 3 people is 3 lookups here, not 78. - const businessUnits = new Map(); - for (const userId of users) { - businessUnits.set(userId, await resolveBusinessUnit(engine, userId)); - } + // Resolved once for the run, not once per (item, person): a 26-item catalog + // for 3 people is 2 reads here, not 78. + const businessUnits = await resolveBusinessUnits(engine, users); + const unanchoredOwners: string[] = []; + let unanchored = 0; const timezone = resolveDutyTimezone(); const entries: CatalogApplyEntry[] = []; @@ -360,7 +431,7 @@ export const applyCatalogHandler: ActionHandler = async (ctx continue; } - const businessUnit = businessUnits.get(owner); + const businessUnit = businessUnits.get(owner) ?? null; const duty: Record = { // ── from the catalog item ── name: item.name, @@ -389,23 +460,31 @@ export const applyCatalogHandler: ActionHandler = async (ctx // Same-run guard: two identical items in one catalog cannot both land on // the same person just because the pre-probe ran before either existed. taken.add(pairKey(itemId, owner)); + if (!businessUnit) { + unanchored += 1; + if (!unanchoredOwners.includes(owner)) unanchoredOwners.push(owner); + } entries.push({ catalog_item: itemId, catalog_item_name: itemName, owner, outcome: 'created', duty: inserted?.id, + business_unit: businessUnit, }); } } const result: CatalogApplyResult = { action: CATALOG_APPLY_ACTION, + outcome: created === 0 ? 'nothing_created' : unanchored > 0 ? 'created_unanchored' : 'created', position_code: positionCode, catalog_items: activeItems.length, users: users.length, created, skipped, + unanchored, + unanchored_owners: unanchoredOwners, entries, }; return result; diff --git a/src/objects/duty.object.ts b/src/objects/duty.object.ts index 26a5ef2..5f591db 100644 --- a/src/objects/duty.object.ts +++ b/src/objects/duty.object.ts @@ -105,8 +105,10 @@ export const Duty = ObjectSchema.create({ }), // On a hand-made duty, `duly_duty_unit_stamp` (src/hooks/duty.hook.ts) - // writes the owner's primary business unit at create. Apply catalog - // resolves it from the person's position anchor; the seed names it. + // writes the owner's primary business unit at create. Apply catalog runs + // as system, so the stamp skips it; its handler resolves the same rung + // first, then a lone position anchor, and reports a duty left without a + // unit (`resolveBusinessUnits`, #179). The seed names it. business_unit: Field.lookup('sys_business_unit', { label: 'Business unit', group: 'basics', diff --git a/src/translations/authored-text.ts b/src/translations/authored-text.ts index 5bd2aec..014193c 100644 --- a/src/translations/authored-text.ts +++ b/src/translations/authored-text.ts @@ -556,6 +556,14 @@ const VERDICTS: Readonly> = { // sits at `.source` and keeps the action's own `successMessage` key. 'action.successMessage.source': translate((c) => actionKey(c, 'successMessage')), 'action.successMessage.dialect': machine('template dialect'), + // Success copy per handler outcome (#179). The bundle address is the + // action's own entry, `outcomeMessages.`, the key the platform's + // translation-reference check names; the outcome is the last path segment. + 'action.outcomeMessages{}': translate((c) => { + const outcome = c.path[c.path.length - 1]; + const base = actionKey(c, 'outcomeMessages'); + return outcome && base ? [...base, outcome] : undefined; + }), 'action.params[].label': translate((c) => actionParamKey(c, 'label')), 'action.params[].helpText': translate((c) => actionParamKey(c, 'helpText')), 'action.params[].placeholder': translate((c) => actionParamKey(c, 'placeholder')), @@ -779,7 +787,9 @@ function actionParamKey(ctx: KeyContext, leaf: string): readonly string[] | unde * segment, which is what makes an unknown map show up as an unclassified path * rather than being silently folded away. */ -const RECORD_MAPS: ReadonlySet = new Set(['object.fields']); +// `action.outcomeMessages` is keyed by the outcome names a handler returns +// (#179), which are data, not structure. +const RECORD_MAPS: ReadonlySet = new Set(['object.fields', 'action.outcomeMessages']); /** * Containers that RE-ENTER the vocabulary they sit inside, so the subtree diff --git a/src/translations/zh-CN.ts b/src/translations/zh-CN.ts index 47099ac..fb6e89c 100644 --- a/src/translations/zh-CN.ts +++ b/src/translations/zh-CN.ts @@ -516,6 +516,13 @@ export const dulyChinese = defineTranslationBundle({ duly_catalog_apply_to_people: { label: '应用到人员', description: '为选中的每个人创建这个岗位应尽的职责。可以放心重复执行——已经从职责库条目获得过职责的人会被跳过，不会重复创建。', + // 操作完成后的提示，按处理器返回的 outcome 选取（#179）。 + outcomeMessages: { + created: '已创建 ${result.created} 条职责。${result.skipped} 条已存在，已跳过。', + created_unanchored: + '已创建 ${result.created} 条职责。其中 ${result.unanchored} 条没有业务单元，因为负责人不属于任何业务单元。${result.skipped} 条已存在，已跳过。', + nothing_created: '没有新职责：全部 ${result.skipped} 条都已存在。', + }, params: { position_code: { label: '岗位', @@ -824,6 +831,13 @@ export const dulyChinese = defineTranslationBundle({ duly_catalog_apply: { label: '应用岗位职责库', description: '为选中的每个人创建这个岗位应尽的职责。可以放心重复执行——已经从职责库条目获得过职责的人会被跳过，不会重复创建。', + // 操作完成后的提示，按处理器返回的 outcome 选取（#179）。 + outcomeMessages: { + created: '已创建 ${result.created} 条职责。${result.skipped} 条已存在，已跳过。', + created_unanchored: + '已创建 ${result.created} 条职责。其中 ${result.unanchored} 条没有业务单元，因为负责人不属于任何业务单元。${result.skipped} 条已存在，已跳过。', + nothing_created: '没有新职责：全部 ${result.skipped} 条都已存在。', + }, params: { position_code: { label: '岗位', diff --git a/test/catalog-apply-unit.test.ts b/test/catalog-apply-unit.test.ts new file mode 100644 index 0000000..ac43fb8 --- /dev/null +++ b/test/catalog-apply-unit.test.ts @@ -0,0 +1,207 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; +import { AppPlugin, HttpDispatcher, ObjectKernel, createStandaloneStack } from '@objectstack/runtime'; +import type { HttpProtocolContext } from '@objectstack/runtime'; +import { PlatformObjectsPlugin } from '@objectstack/platform-objects/plugin'; +import { SecurityPlugin } from '@objectstack/plugin-security'; +import { SharingServicePlugin } from '@objectstack/plugin-sharing'; +import { createIdentityObjectsPlugin } from '@objectstack/plugin-auth'; + +import { dulyActions } from '../src/actions/index.js'; +import { registerDulyActionHandlers } from '../src/actions/register-handlers.js'; +import { + CATALOG_APPLY_ACTION, + CATALOG_APPLY_TO_PEOPLE_ACTION, +} from '../src/actions/catalog.handlers.js'; +import type { CatalogApplyOutcome, CatalogApplyResult } from '../src/actions/catalog.handlers.js'; +import { ADMIN, PEOPLE, POSITIONS } from '../src/data/demo-org.js'; +import { dulyChinese } from '../src/translations/zh-CN.js'; + +/** + * #179 — Apply catalog gives every applied DEMO duty its owner's unit. + * + * Measured on 18.2.0 before the fix (project file + * `duly-179-apply-unit/step0.md`): Apply to demo people created 13 duties and + * all 13 had no business unit. The handler read only the position-assignment + * anchor, and no demo position assignment is anchored. The demo seed writes + * its own duties' units directly, which hid the gap. + * + * The demo relies on the fallback rather than anchoring its positions: each + * of the twelve has a primary membership row (`org.seed.ts`), which + * plugin-sharing projects onto `sys_user.primary_business_unit_id`, the first + * rung Apply now reads. The account you sign in as has no unit BY DESIGN (a + * membership row would make the projection hook update the live login + * account; `test/business-unit-membership.test.ts` pins its absence), so its + * applied duties stay unanchored — and the run has to say so. + * + * Booted with the demo seed through the real action route, so the units come + * from the same membership rows and the same projection a `pnpm demo` has. + */ + +const DEMO_SEED_ENV_VAR = 'DULY_DEMO_SEED'; +const SYSTEM = { isSystem: true } as const; + +let kernel: any; +let data: any; +let dispatcher: HttpDispatcher; + +const all = async (object: string, where?: Record): Promise => + (await data.find(object, where ? { where } : {}, { context: SYSTEM })) ?? []; + +const settle = async (label: string, check: () => Promise, ms = 120_000): Promise => { + const deadline = Date.now() + ms; + let last: string | null = 'never evaluated'; + for (;;) { + last = await check(); + if (last === null) return; + if (Date.now() > deadline) throw new Error(`${label} did not settle in ${ms}ms — ${last}`); + await new Promise((resolve) => setTimeout(resolve, 250)); + } +}; + +beforeAll(async () => { + // Read at module-evaluation time, so set before the config is imported. + vi.stubEnv(DEMO_SEED_ENV_VAR, '1'); + const stack = (await import('../objectstack.config.js')).default as unknown as Record; + + const { plugins } = await createStandaloneStack({ + databaseDriver: 'sqlite', + databaseUrl: ':memory:', + skipSeedData: true, + // Must not resolve to a real path, or a local `pnpm build` makes this suite + // report on the last BUILD instead of on `src/`. + artifactPath: 'dist/objectstack.this-suite-must-not-load-an-artifact.json', + }); + kernel = new ObjectKernel(); + for (const plugin of plugins) await kernel.use(plugin); + await kernel.use(createIdentityObjectsPlugin()); + await kernel.use(new PlatformObjectsPlugin()); + // The projection hook that derives `primary_business_unit_id` from the + // membership rows, and `sys_user_position`'s owner. + await kernel.use(new SharingServicePlugin()); + await kernel.use(new SecurityPlugin()); + await kernel.use(new AppPlugin(stack as any, undefined, { skipSeedData: false })); + await kernel.bootstrap(); + data = kernel.getService('data'); + registerDulyActionHandlers(data); + dispatcher = new HttpDispatcher(kernel); + + // The inline seed races a budget rather than being awaited by bootstrap: + // wait for every person's projection, not for a fixed delay. + await settle('the primary-unit projection', async () => { + const users = await all('sys_user'); + const projected = PEOPLE.filter((person) => + users.some((user) => user.name === person.name && user.primary_business_unit_id)); + return projected.length === PEOPLE.length ? null : `${projected.length} of ${PEOPLE.length} projected`; + }); + await settle('the catalog seed', async () => { + const items = await all('duly_catalog_item', { position_code: POSITIONS.compliance }); + return items.length > 0 ? null : 'no catalog items yet'; + }); +}, 240_000); + +afterAll(async () => { + await kernel?.shutdown?.(); + vi.unstubAllEnvs(); +}); + +const ADMIN_CALLER: HttpProtocolContext = { + request: {}, + executionContext: { userId: 'admin_179', systemPermissions: ['duly.catalog.apply'] }, +}; + +describe('Apply catalog on the demo organisation (#179)', () => { + let result: CatalogApplyResult; + let userIdOf: Map; + let unitIdOf: Map; + + beforeAll(async () => { + const users = await all('sys_user'); + userIdOf = new Map(users.map((user) => [String(user.name), String(user.id)])); + unitIdOf = new Map((await all('sys_business_unit')).map((unit) => [String(unit.name), String(unit.id)])); + + // Everyone at once, the admin included, on a position whose items the + // seed has not already given to most of them. + const people = [...PEOPLE.map((person) => person.name), ADMIN]; + const response = (await dispatcher.handleActions( + `/${CATALOG_APPLY_ACTION}`, + 'POST', + { params: { position_code: POSITIONS.compliance, users: people.map((name) => userIdOf.get(name)) } }, + ADMIN_CALLER, + )).response; + expect(response?.status, JSON.stringify(response?.body)).toBe(200); + result = response?.body?.data as CatalogApplyResult; + }); + + it('creates duties for every demo person', () => { + const owners = new Set(result.entries.filter((entry) => entry.outcome === 'created').map((entry) => entry.owner)); + for (const person of PEOPLE) expect(owners.has(userIdOf.get(person.name)!), person.name).toBe(true); + }); + + it('anchors every one of the twelve people\'s applied duties in their seeded unit', async () => { + let anchored = 0; + for (const person of PEOPLE) { + const owner = userIdOf.get(person.name)!; + const unit = unitIdOf.get(person.unit); + expect(unit, `${person.unit} exists`).toBeTruthy(); + const created = result.entries.filter((entry) => entry.outcome === 'created' && entry.owner === owner); + for (const entry of created) { + expect(entry.business_unit, `${person.name} · ${entry.catalog_item_name} (result)`).toBe(unit); + const [duty] = await all('duly_duty', { id: entry.duty }); + expect(duty?.business_unit, `${person.name} · ${entry.catalog_item_name} (stored)`).toBe(unit); + anchored += 1; + } + } + // Every created duty that is not the admin's is anchored. + const admin = userIdOf.get(ADMIN)!; + expect(anchored).toBe(result.entries.filter((e) => e.outcome === 'created' && e.owner !== admin).length); + expect(anchored).toBeGreaterThan(0); + }); + + it('leaves the admin\'s duties without a unit, and says so', async () => { + const admin = userIdOf.get(ADMIN)!; + const created = result.entries.filter((entry) => entry.outcome === 'created' && entry.owner === admin); + expect(created.length).toBeGreaterThan(0); + for (const entry of created) { + expect(entry.business_unit).toBeNull(); + const [duty] = await all('duly_duty', { id: entry.duty }); + expect(duty?.business_unit ?? null).toBeNull(); + } + expect(result.outcome).toBe('created_unanchored'); + expect(result.unanchored).toBe(created.length); + expect(result.unanchored_owners).toEqual([admin]); + }); +}); + +describe('the toast the console shows for each outcome', () => { + const OUTCOMES: CatalogApplyOutcome[] = ['created', 'created_unanchored', 'nothing_created']; + const RESULT_KEYS = new Set(['created', 'skipped', 'unanchored']); + + for (const name of [CATALOG_APPLY_ACTION, CATALOG_APPLY_TO_PEOPLE_ACTION]) { + it(`${name} declares one message per outcome, each reading only real result keys`, () => { + const action = dulyActions.find((candidate) => candidate.name === name) as { outcomeMessages?: Record }; + expect(Object.keys(action.outcomeMessages ?? {}).sort()).toEqual([...OUTCOMES].sort()); + for (const message of Object.values(action.outcomeMessages ?? {})) { + for (const [, key] of message.matchAll(/\$\{result\.(\w+)\}/g)) { + expect(RESULT_KEYS.has(key as keyof CatalogApplyResult), `${name}: \${result.${key}}`).toBe(true); + } + } + expect(action.outcomeMessages?.created_unanchored).toContain('${result.unanchored}'); + }); + } + + it('zh-CN carries the same outcomes and placeholders on both addresses', () => { + const bundle = (dulyChinese as any)['zh-CN']; + const zh = [ + bundle.globalActions?.[CATALOG_APPLY_ACTION]?.outcomeMessages, + bundle.objects?.duly_catalog_item?._actions?.[CATALOG_APPLY_TO_PEOPLE_ACTION]?.outcomeMessages, + ]; + const en = (dulyActions.find((candidate) => candidate.name === CATALOG_APPLY_ACTION) as any).outcomeMessages; + const placeholders = (text: string) => [...text.matchAll(/\$\{result\.\w+\}/g)].map((m) => m[0]).sort(); + for (const messages of zh) { + expect(Object.keys(messages ?? {}).sort()).toEqual([...OUTCOMES].sort()); + for (const outcome of OUTCOMES) expect(placeholders(messages[outcome])).toEqual(placeholders(en[outcome])); + } + }); +}); diff --git a/test/catalog-engine-facade.test.ts b/test/catalog-engine-facade.test.ts index c96d053..64c677c 100644 --- a/test/catalog-engine-facade.test.ts +++ b/test/catalog-engine-facade.test.ts @@ -103,7 +103,7 @@ beforeAll(async () => { data = kernel.getService('data'); await ensureUsers(data, [ 'u_anchored', 'u_denied', 'u_fa1', 'u_fa2', 'u_fi1', 'u_fn1', 'u_fn2', - 'u_fr1', 'u_fs1', 'u_fs2', 'u_ft1', 'u_unanchored', + 'u_fr1', 'u_fs1', 'u_fs2', 'u_ft1', 'u_unanchored', 'u_primary', ]); // Through the REAL registration function, so a handler dropped from @@ -203,33 +203,59 @@ describe('duly_catalog_apply through the runtime\'s own facade (#79)', () => { } }); - it('anchors the duty to the business unit of the owner\'s position', async () => { - // The read that fails SILENTLY. `resolveBusinessUnit` tolerates "no - // `sys_user_position` row" as a legitimate day-one state, so its broken - // read produced no error and no report — just duties with nothing to roll - // up to. Both people are applied in ONE run: the anchored one proves the - // read returns, the unanchored one proves the tolerated state is still - // tolerated and did not turn into a refusal. + it('anchors the duty to the owner\'s unit: primary first, then a lone anchor, else reported (#179)', async () => { + // The reads that fail SILENTLY. The handler tolerates "this person has no + // unit" as a legitimate day-one state, so a broken read produces no error + // — just duties with nothing to roll up to. Three people in ONE run, one + // per rung of the ladder: the primary unit (written the way a real org + // writes it, through a primary membership row that plugin-sharing + // projects onto `sys_user`), a position anchor for someone with no + // primary, and nobody's unit at all, which must still be tolerated and + // must now be reported. const position = 'facade_anchor'; await insertItem({ position_code: position }); - const unit = await data.insert('sys_business_unit', { name: 'Plant A' }, { context: SYSTEM }); + const home = await data.insert('sys_business_unit', { name: 'Plant Home' }, { context: SYSTEM }); + const away = await data.insert('sys_business_unit', { name: 'Plant A' }, { context: SYSTEM }); + await data.insert( + 'sys_business_unit_member', + { user_id: 'u_primary', business_unit_id: home.id, is_primary: true }, + { context: SYSTEM }, + ); + // An anchor elsewhere for the primary-unit person: the primary wins. + await data.insert( + 'sys_user_position', + { user_id: 'u_primary', position: 'duly_member', business_unit_id: away.id }, + { context: SYSTEM }, + ); await data.insert( 'sys_user_position', - { user_id: 'u_anchored', position, business_unit_id: unit.id }, + { user_id: 'u_anchored', position, business_unit_id: away.id }, { context: SYSTEM }, ); + const [primaryUser] = await data.find('sys_user', { where: { id: 'u_primary' } }, { context: SYSTEM }); + expect(primaryUser?.primary_business_unit_id, 'the platform projected the membership').toBe(home.id); const result = await run(`/${CATALOG_APPLY_ACTION}`, { position_code: position, - users: ['u_anchored', 'u_unanchored'], + users: ['u_primary', 'u_anchored', 'u_unanchored'], }); - expect(result.created).toBe(2); + expect(result.created).toBe(3); + + const [primary] = await dutiesOf('u_primary'); + expect(primary?.business_unit).toBe(home.id); const [anchored] = await dutiesOf('u_anchored'); - expect(anchored?.business_unit).toBe(unit.id); + expect(anchored?.business_unit).toBe(away.id); const [unanchored] = await dutiesOf('u_unanchored'); expect(unanchored?.business_unit ?? null).toBeNull(); + + // …and the run says so, in the body the console's toast reads. + expect(result.outcome).toBe('created_unanchored'); + expect(result.unanchored).toBe(1); + expect(result.unanchored_owners).toEqual(['u_unanchored']); + const units = Object.fromEntries(result.entries.map((entry) => [entry.owner, entry.business_unit])); + expect(units).toEqual({ u_primary: home.id, u_anchored: away.id, u_unanchored: null }); }); it('is idempotent on (catalog_item, owner) across two dispatched runs', async () => { diff --git a/test/catalog-instantiate.test.ts b/test/catalog-instantiate.test.ts index 5d87cf9..6c8eb74 100644 --- a/test/catalog-instantiate.test.ts +++ b/test/catalog-instantiate.test.ts @@ -252,38 +252,126 @@ describe('duly_catalog_apply', () => { expect(items).not.toContain('item_other'); }); - it('anchors business_unit on sys_user_position.business_unit_id', async () => { + // ── business_unit: the R-A ladder (#179) ──────────────────────────── + // Primary unit first; an anchor only when there is no primary AND the + // anchored rows name one unit; otherwise no unit, and the run says so. + + const applyTo = async (users: string[]) => + (await applyCatalogHandler( + contextFor(engine, { position_code: 'plant_compliance_officer', users }), + )) as CatalogApplyResult; + const unitsOf = (owner: string) => + [...new Set(engine.rows('duly_duty').filter((d) => d.owner === owner).map((d) => d.business_unit))]; + + it('anchors business_unit on the owner\'s primary business unit', async () => { + engine.seed('sys_user', [{ id: 'user_a', primary_business_unit_id: 'bu_home' }]); + + const result = await applyTo(['user_a']); + + expect(unitsOf('user_a')).toEqual(['bu_home']); + expect(result.outcome).toBe('created'); + expect(result.unanchored).toBe(0); + expect(result.unanchored_owners).toEqual([]); + for (const entry of result.entries) expect(entry.business_unit).toBe('bu_home'); + }); + + it('prefers the primary unit over a position anchor that names another unit', async () => { + // Measured on 18.2 (#179 Step 0): the anchor sits on a CAPABILITY position + // (`duly_member`), not the catalog's free-text position, so it is not + // evidence about where this catalog's duties belong. + engine.seed('sys_user', [{ id: 'user_a', primary_business_unit_id: 'bu_home' }]); engine.seed('sys_user_position', [ - { id: 'up_1', user_id: 'user_a', position: 'plant_compliance_officer', business_unit_id: 'bu_north' }, + { id: 'up_1', user_id: 'user_a', position: 'duly_member', business_unit_id: 'bu_away' }, ]); - await applyCatalogHandler( - contextFor(engine, { position_code: 'plant_compliance_officer', users: ['user_a'] }), - ); + await applyTo(['user_a']); - for (const duty of engine.rows('duly_duty')) expect(duty.business_unit).toBe('bu_north'); + expect(unitsOf('user_a')).toEqual(['bu_home']); }); - it('does NOT require a sys_user_position row — day one, before positions are modelled', async () => { - const result = (await applyCatalogHandler( - contextFor(engine, { position_code: 'plant_compliance_officer', users: ['user_a'] }), - )) as CatalogApplyResult; + it('falls back to the position anchor when there is no primary unit', async () => { + engine.seed('sys_user', [{ id: 'user_a', primary_business_unit_id: null }]); + engine.seed('sys_user_position', [ + { id: 'up_1', user_id: 'user_a', position: 'duly_member', business_unit_id: 'bu_north' }, + // A second row anchored to the SAME unit is not ambiguous. + { id: 'up_2', user_id: 'user_a', position: 'duly_manager', business_unit_id: 'bu_north' }, + // An unanchored row carries no depth and is not a second answer. + { id: 'up_3', user_id: 'user_a', position: 'duly_admin', business_unit_id: null }, + ]); - expect(result.created).toBe(26); - // Absent, not null: an unanchored duty must not claim a business unit. - for (const duty of engine.rows('duly_duty')) expect(duty.business_unit).toBeUndefined(); + const result = await applyTo(['user_a']); + + expect(unitsOf('user_a')).toEqual(['bu_north']); + expect(result.outcome).toBe('created'); }); - it('ignores an unanchored position row rather than writing a null business unit', async () => { + it('does not guess between two anchors that name different units', async () => { + engine.seed('sys_user', [{ id: 'user_a' }]); engine.seed('sys_user_position', [ - { id: 'up_1', user_id: 'user_a', position: 'plant_compliance_officer', business_unit_id: null }, + { id: 'up_1', user_id: 'user_a', position: 'duly_member', business_unit_id: 'bu_north' }, + { id: 'up_2', user_id: 'user_a', position: 'duly_manager', business_unit_id: 'bu_south' }, ]); - await applyCatalogHandler( - contextFor(engine, { position_code: 'plant_compliance_officer', users: ['user_a'] }), - ); + const result = await applyTo(['user_a']); + + // Absent, not null: an unanchored duty must not claim a business unit. + for (const duty of engine.rows('duly_duty')) expect(duty.business_unit).toBeUndefined(); + expect(result.outcome).toBe('created_unanchored'); + expect(result.unanchored_owners).toEqual(['user_a']); + }); + + it('does NOT require a unit at all — day one, before the org is modelled — and reports it', async () => { + const result = await applyTo(['user_a']); + expect(result.created).toBe(26); for (const duty of engine.rows('duly_duty')) expect(duty.business_unit).toBeUndefined(); + expect(result.outcome).toBe('created_unanchored'); + expect(result.unanchored).toBe(26); + expect(result.unanchored_owners).toEqual(['user_a']); + for (const entry of result.entries) expect(entry.business_unit).toBeNull(); + }); + + it('reports exactly the owners with no unit, once each, in a mixed run', async () => { + engine.seed('sys_user', [ + { id: 'user_a', primary_business_unit_id: 'bu_home' }, + { id: 'user_b' }, + // An expanded lookup reads the same as an id. + { id: 'user_c', primary_business_unit_id: { id: 'bu_east', name: 'East' } }, + ]); + + const result = await applyTo(THREE_USERS); + + expect(unitsOf('user_a')).toEqual(['bu_home']); + expect(unitsOf('user_b')).toEqual([undefined]); + expect(unitsOf('user_c')).toEqual(['bu_east']); + expect(result.created).toBe(78); + expect(result.unanchored).toBe(26); + expect(result.unanchored_owners).toEqual(['user_b']); + expect(result.outcome).toBe('created_unanchored'); + }); + + it('says nothing_created on a re-run, and leaves a skip without a business_unit claim', async () => { + await applyTo(['user_a']); + const again = await applyTo(['user_a']); + + expect(again.outcome).toBe('nothing_created'); + expect(again.created).toBe(0); + expect(again.unanchored).toBe(0); + for (const entry of again.entries) expect(entry).not.toHaveProperty('business_unit'); + }); + + it('reads the units in two queries for the whole run, not one per person', async () => { + const reads: string[] = []; + const find = engine.find.bind(engine); + engine.find = async (object, query) => { + reads.push(object); + return find(object, query); + }; + + await applyTo(THREE_USERS); + + expect(reads.filter((object) => object === 'sys_user')).toHaveLength(1); + expect(reads.filter((object) => object === 'sys_user_position')).toHaveLength(1); }); it('refuses a blank position_code or an empty user list instead of reporting a no-op run', async () => { diff --git a/test/duty-provenance.test.ts b/test/duty-provenance.test.ts index 1ad9818..3b56880 100644 --- a/test/duty-provenance.test.ts +++ b/test/duty-provenance.test.ts @@ -327,14 +327,15 @@ describe('the writers that put a duty on somebody else keep writing all three', expect(response?.body?.data?.created).toBe(1); const [duty] = await data.find('duly_duty', { where: { catalog_item: item.id, owner: 'user_sami' } }, { context: SYSTEM }); - expect([duty.source, duty.owner, duty.business_unit]).toEqual(['catalog', 'user_sami', UNIT.quality]); + // The primary unit wins over the position anchor (#179, ruling R-A). + expect([duty.source, duty.owner, duty.business_unit]).toEqual(['catalog', 'user_sami', UNIT.ops]); // Sync replays cadence onto it and leaves its provenance alone. await data.update('duly_catalog_item', { id: item.id, due_offset_days: 9 }, { context: SYSTEM }); const synced = await action(IDA, CATALOG_SYNC_ACTION, { position_code: position }); expect(synced?.status, JSON.stringify(synced?.body)).toBe(200); const after = await stored(String(duty.id)); - expect([after.source, after.owner, after.business_unit, after.due_offset_days]).toEqual(['catalog', 'user_sami', UNIT.quality, 9]); + expect([after.source, after.owner, after.business_unit, after.due_offset_days]).toEqual(['catalog', 'user_sami', UNIT.ops, 9]); }); it('a member still cannot call Apply catalog', async () => { From 9e85c151805879070ba7f33169233c280cbf50c1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 07:32:19 +0000 Subject: [PATCH 2/3] Word the unanchored toast for both causes: no primary unit, or no single anchor Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01JQwxbYga1GGouLwJs9WbD3 --- src/actions/catalog.actions.ts | 2 +- src/actions/catalog.handlers.ts | 10 ++++++---- src/translations/zh-CN.ts | 4 ++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/actions/catalog.actions.ts b/src/actions/catalog.actions.ts index a2dc734..1aa086f 100644 --- a/src/actions/catalog.actions.ts +++ b/src/actions/catalog.actions.ts @@ -89,7 +89,7 @@ export const CatalogApplyAction = defineAction({ outcomeMessages: { created: 'Created ${result.created} duties. ${result.skipped} already existed and were skipped.', created_unanchored: - 'Created ${result.created} duties. ${result.unanchored} have no business unit, because their owner belongs to none. ${result.skipped} already existed and were skipped.', + 'Created ${result.created} duties. ${result.unanchored} have no business unit, because none could be determined for their owner. ${result.skipped} already existed and were skipped.', nothing_created: 'Nothing new: all ${result.skipped} duties already existed.', }, params: [ diff --git a/src/actions/catalog.handlers.ts b/src/actions/catalog.handlers.ts index d1a6038..786197b 100644 --- a/src/actions/catalog.handlers.ts +++ b/src/actions/catalog.handlers.ts @@ -159,8 +159,9 @@ export interface CatalogApplyEntry { /** Id of the duty created by THIS run. Absent for a skip. */ duty?: string; /** - * The unit the created duty was anchored to, or `null` when its owner - * belongs to none (#179). Absent for a skip: a skipped pair wrote nothing. + * The unit the created duty was anchored to, or `null` when none could be + * determined for its owner (#179). Absent for a skip: a skipped pair wrote + * nothing. */ business_unit?: string | null; } @@ -170,8 +171,9 @@ export interface CatalogApplyEntry { * (`outcomeMessages` on the two apply actions, `catalog.actions.ts`). * * - `created`: every duty this run created has a business unit. - * - `created_unanchored`: at least one has none, because its owner belongs to - * no unit. The toast says how many; `unanchored_owners` says who. + * - `created_unanchored`: at least one has none, because no unit could be + * determined for its owner (no primary unit, and no single anchor). The + * toast says how many; `unanchored_owners` says who. * - `nothing_created`: every pair already had its duty. */ export type CatalogApplyOutcome = 'created' | 'created_unanchored' | 'nothing_created'; diff --git a/src/translations/zh-CN.ts b/src/translations/zh-CN.ts index fb6e89c..e727bbd 100644 --- a/src/translations/zh-CN.ts +++ b/src/translations/zh-CN.ts @@ -520,7 +520,7 @@ export const dulyChinese = defineTranslationBundle({ outcomeMessages: { created: '已创建 ${result.created} 条职责。${result.skipped} 条已存在，已跳过。', created_unanchored: - '已创建 ${result.created} 条职责。其中 ${result.unanchored} 条没有业务单元，因为负责人不属于任何业务单元。${result.skipped} 条已存在，已跳过。', + '已创建 ${result.created} 条职责。其中 ${result.unanchored} 条没有业务单元，因为无法确定负责人所属的业务单元。${result.skipped} 条已存在，已跳过。', nothing_created: '没有新职责：全部 ${result.skipped} 条都已存在。', }, params: { @@ -835,7 +835,7 @@ export const dulyChinese = defineTranslationBundle({ outcomeMessages: { created: '已创建 ${result.created} 条职责。${result.skipped} 条已存在，已跳过。', created_unanchored: - '已创建 ${result.created} 条职责。其中 ${result.unanchored} 条没有业务单元，因为负责人不属于任何业务单元。${result.skipped} 条已存在，已跳过。', + '已创建 ${result.created} 条职责。其中 ${result.unanchored} 条没有业务单元，因为无法确定负责人所属的业务单元。${result.skipped} 条已存在，已跳过。', nothing_created: '没有新职责：全部 ${result.skipped} 条都已存在。', }, params: { From 5c298ecc00cdfddf94990861061e4a0716007bfd Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 07:42:11 +0000 Subject: [PATCH 3/3] Describe the business unit as the owner's primary unit, with Apply's anchor fallback Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01JQwxbYga1GGouLwJs9WbD3 --- src/hooks/duty.hook.ts | 10 ++++++---- src/objects/duty.object.ts | 3 ++- src/translations/zh-CN.ts | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/hooks/duty.hook.ts b/src/hooks/duty.hook.ts index df9f696..ba1d638 100644 --- a/src/hooks/duty.hook.ts +++ b/src/hooks/duty.hook.ts @@ -149,7 +149,7 @@ export const DutyTimezoneGuard: Hook = { /** * `duly_duty.business_unit` for a duty somebody creates by hand (#178). * - * The field's description has always said "Set from the owner's position on + * The field's description used to say "Set from the owner's position on * create", and on a hand-made duty nothing set it: measured on 18.2.0, a * plain create through the form, data POST or MCP stored `business_unit: * null`. Since #178 a member or manager can no longer name a unit at all (the @@ -169,9 +169,11 @@ export const DutyTimezoneGuard: Hook = { * * ── What it never touches ──────────────────────────────────────────────── * - A SYSTEM write: the seed names each duty's unit itself, and - * `duly_catalog_apply` resolves the unit from the person's position - * anchor. Both say what they mean; a second rung here would be a product - * decision taken behind their backs (that is card #179's to make). + * `duly_catalog_apply` (whose engine runs as system) resolves its own: + * the owner's primary unit first — this hook's rung — then a lone + * position anchor, and it reports a duty left without one + * (`resolveBusinessUnits` in `src/actions/catalog.handlers.ts`, #179). + * Both say what they mean, so this hook leaves them alone. * - A write that NAMES a unit: only a `duly_admin` can, the Import and a * correction by hand among them, and their value stands. * - An update. The anchor is written once, at create. diff --git a/src/objects/duty.object.ts b/src/objects/duty.object.ts index 5f591db..bc6b4e5 100644 --- a/src/objects/duty.object.ts +++ b/src/objects/duty.object.ts @@ -112,7 +112,8 @@ export const Duty = ObjectSchema.create({ business_unit: Field.lookup('sys_business_unit', { label: 'Business unit', group: 'basics', - description: 'Rollup anchor. Set from the owner\'s position on create.', + description: + 'Rollup anchor. Set on create from the owner\'s primary business unit. Apply catalog falls back to the owner\'s position anchor when they have no primary unit and their anchored positions name one unit; otherwise it stays empty.', }), // ── Where the duty came from ────────────────────────────────────────── diff --git a/src/translations/zh-CN.ts b/src/translations/zh-CN.ts index e727bbd..e62bbce 100644 --- a/src/translations/zh-CN.ts +++ b/src/translations/zh-CN.ts @@ -163,7 +163,7 @@ export const dulyChinese = defineTranslationBundle({ }, business_unit: { label: '部门', - help: '汇总口径的锚点。创建时取自负责人的岗位。', + help: '汇总口径的锚点。创建时取自负责人的主业务单元。负责人没有主业务单元时，应用岗位职责库会改用其岗位锚定的业务单元（仅当这些岗位都指向同一个业务单元时）；否则留空。', }, source: { label: '来源',