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
12 changes: 12 additions & 0 deletions src/actions/catalog.actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 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: [
{
name: 'position_code',
Expand Down
159 changes: 120 additions & 39 deletions src/actions/catalog.handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -157,15 +158,38 @@ 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 none could be
* determined for its owner (#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 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';

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[];
}

Expand Down Expand Up @@ -266,46 +290,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<string | undefined> {
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<Map<string, string | null>> {
const resolved = new Map<string, string | null>();
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<string, Set<string>>();
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<string>();
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 ──────────────────────────────────────────────────────
Expand Down Expand Up @@ -337,12 +411,11 @@ export const applyCatalogHandler: ActionHandler<CatalogApplyParams> = 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<string, string | undefined>();
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[] = [];
Expand All @@ -360,7 +433,7 @@ export const applyCatalogHandler: ActionHandler<CatalogApplyParams> = async (ctx
continue;
}

const businessUnit = businessUnits.get(owner);
const businessUnit = businessUnits.get(owner) ?? null;
const duty: Record<string, unknown> = {
// ── from the catalog item ──
name: item.name,
Expand Down Expand Up @@ -389,23 +462,31 @@ export const applyCatalogHandler: ActionHandler<CatalogApplyParams> = 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;
Expand Down
10 changes: 6 additions & 4 deletions src/hooks/duty.hook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
9 changes: 6 additions & 3 deletions src/objects/duty.object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,15 @@ 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',
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 ──────────────────────────────────────────
Expand Down
12 changes: 11 additions & 1 deletion src/translations/authored-text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,14 @@ const VERDICTS: Readonly<Record<string, Verdict>> = {
// 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.<outcome>`, 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')),
Expand Down Expand Up @@ -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<string> = 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<string> = new Set(['object.fields', 'action.outcomeMessages']);

/**
* Containers that RE-ENTER the vocabulary they sit inside, so the subtree
Expand Down
16 changes: 15 additions & 1 deletion src/translations/zh-CN.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export const dulyChinese = defineTranslationBundle({
},
business_unit: {
label: '部门',
help: '汇总口径的锚点。创建时取自负责人的岗位。',
help: '汇总口径的锚点。创建时取自负责人的主业务单元。负责人没有主业务单元时,应用岗位职责库会改用其岗位锚定的业务单元(仅当这些岗位都指向同一个业务单元时);否则留空。',
},
source: {
label: '来源',
Expand Down Expand Up @@ -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: '岗位',
Expand Down Expand Up @@ -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: '岗位',
Expand Down
Loading
Loading