Skip to content

Commit 49c6d95

Browse files
committed
fix(plugin-approvals): narrow the delegation fix to the picker filter
Measured: declaring defaultValue 'current_user' changes one engine cell, a system-context insert that carries a userId and omits delegator_id (refused by required before, stored with that userId after). That is an accept-set change the claim's Clause-2 line does not cover, and the picker filter alone makes the form offer only what the guard admits, so the default is dropped here and raised as a decision in the report. What a write may store is now unchanged. Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2cdb9a9 commit 49c6d95

2 files changed

Lines changed: 23 additions & 51 deletions

File tree

‎packages/plugins/plugin-approvals/src/sys-approval-delegation-delegator-picker.test.ts‎

Lines changed: 17 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,43 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

33
/**
4-
* The Delegator field of `sys_approval_delegation` offers exactly what the
4+
* The Delegator picker of `sys_approval_delegation` offers exactly what the
55
* delegation write guard admits.
66
*
77
* `bindDelegationWriteGuard` (lifecycle-hooks.ts) admits one delegator on a
88
* non-system write — the acting user — and refuses every other with 403: a
99
* delegation names its own author as delegator, and acting for an absent
1010
* approver is the approval-level override (the guard's docstring holds the
11-
* ruling). The field used to be a free `sys_user` lookup, so the console's
12-
* create dialog demanded a choice among every user and the server refused all
13-
* but one of them. It now declares that one value, in the object vocabulary
14-
* the create form honours:
15-
*
16-
* - `defaultValue: 'current_user'` — the create form seeds the acting user
17-
* (the token the console resolves client-side on a `sys_user` lookup), and
18-
* an insert that omits the field stores the acting user;
19-
* - `lookupFilters` on `id` = `{current_user_id}` — the picker's candidate
20-
* query, whose placeholder the engine resolves against the caller on every
21-
* read.
11+
* ruling). The field was a free `sys_user` lookup, so the console's create
12+
* dialog offered every user and the server refused all but one of them. It
13+
* now declares `lookupFilters` on `id` = `{current_user_id}`: the picker's
14+
* candidate query, whose placeholder the engine resolves against the caller
15+
* on every read.
2216
*
2317
* Through a real `ObjectQL` over `SqlDriver` (better-sqlite3, in-memory, real
2418
* DDL) with the guard bound, this file asserts:
2519
*
26-
* 1. the declaration, read off the object;
20+
* 1. the declaration, read off the object and off the registered body;
2721
* 2. the picker's candidate query returns the caller and nobody else, for
2822
* each caller;
29-
* 3. every candidate it offers is admitted by the guard, and a create that
30-
* leaves the field empty stores the caller;
23+
* 3. every candidate it offers is admitted by the guard;
3124
* 4. the guard is not widened: a create or an update naming anyone else is
3225
* still refused, `FORBIDDEN` / 403, and nothing is stored;
33-
* 5. `required` stays: a write with no acting user still has no delegator.
26+
* 5. `required` stays: a write with no acting user and no delegator is
27+
* refused.
3428
*/
3529

3630
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
3731
import { ObjectQL } from '@objectstack/objectql';
3832
import { SqlDriver } from '@objectstack/driver-sql';
39-
import { isCurrentUserDefaultToken } from '@objectstack/spec/data';
4033
import { SysApprovalDelegation } from './sys-approval-delegation.object.js';
4134
import { bindDelegationWriteGuard } from './lifecycle-hooks.js';
4235

4336
const OBJECT = 'sys_approval_delegation';
4437
const SYSTEM = { context: { isSystem: true } } as any;
4538
const asUser = (userId: string) => ({ context: { userId, positions: [], permissions: [] } }) as any;
4639

47-
/** The install's people: an admin covering for Priya, and two colleagues. */
40+
/** The install's people: an admin covering for Priya, and a colleague. */
4841
const USERS = [
4942
{ id: 'u_admin', name: 'Dev Admin' },
5043
{ id: 'u_priya', name: 'Priya Raman' },
@@ -69,8 +62,8 @@ const delegatorField = (): Record<string, any> =>
6962
/**
7063
* The picker's candidate `where`, from the declared `lookupFilters`. The
7164
* console lowers each `eq` entry to `{ [field]: value }` and sends it as the
72-
* `$filter` of its `sys_user` query (objectui `lookupFiltersToRecord`); every
73-
* entry here is `eq`, which the declaration test holds.
65+
* `$filter` of its `sys_user` query (objectui `lookupFiltersToRecord`); the
66+
* declaration test holds that every entry here is `eq`.
7467
*/
7568
function pickerWhere(): Record<string, unknown> {
7669
const filters = (delegatorField().lookupFilters ?? []) as LookupFilter[];
@@ -79,7 +72,7 @@ function pickerWhere(): Record<string, unknown> {
7972
return where;
8073
}
8174

82-
describe('sys_approval_delegation — the Delegator field offers only what the write guard admits', () => {
75+
describe('sys_approval_delegation — the Delegator picker offers only what the write guard admits', () => {
8376
let engine: ObjectQL;
8477

8578
beforeAll(async () => {
@@ -102,20 +95,16 @@ describe('sys_approval_delegation — the Delegator field offers only what the w
10295
try { await engine?.destroy(); } catch { /* noop */ }
10396
});
10497

105-
it('declares the acting user as the default and as the picker\'s only candidate, and stays required', () => {
98+
it('declares the acting user as the picker\'s only candidate, and stays a required sys_user lookup', () => {
10699
const field = delegatorField();
107100
expect(field.type).toBe('lookup');
108101
expect(field.reference).toBe('sys_user');
109102
expect(field.required).toBe(true);
110-
// The token the console's create form resolves to the acting user on a
111-
// `sys_user` lookup, and the engine stamps at insert.
112-
expect(isCurrentUserDefaultToken(field.defaultValue)).toBe(true);
113103
expect(field.lookupFilters).toEqual([{ field: 'id', operator: 'eq', value: '{current_user_id}' }]);
114104
});
115105

116-
it('the registered body keeps both declarations', () => {
106+
it('the registered body keeps the declaration', () => {
117107
const registered = (engine.registry.getObject(OBJECT) as any).fields.delegator_id;
118-
expect(isCurrentUserDefaultToken(registered.defaultValue)).toBe(true);
119108
expect(registered.lookupFilters).toEqual(delegatorField().lookupFilters);
120109
});
121110

@@ -135,15 +124,6 @@ describe('sys_approval_delegation — the Delegator field offers only what the w
135124
}
136125
});
137126

138-
it.each([
139-
['omitted', { delegate_id: 'u_tomas' }],
140-
['null', { delegator_id: null, delegate_id: 'u_tomas' }],
141-
])('a create that leaves the delegator %s stores the acting user', async (_label, data) => {
142-
const created = await engine.insert(OBJECT, { ...data }, asUser('u_admin'));
143-
const row = await engine.findOne(OBJECT, { where: { id: created.id } } as any, SYSTEM);
144-
expect(row?.delegator_id).toBe('u_admin');
145-
});
146-
147127
it('the guard is not widened: a create naming anyone else is refused FORBIDDEN / 403 and stores nothing', async () => {
148128
const before = await engine.count(OBJECT, { where: { delegator_id: 'u_priya' } } as any, SYSTEM);
149129
await expect(
@@ -153,7 +133,7 @@ describe('sys_approval_delegation — the Delegator field offers only what the w
153133
});
154134

155135
it('the guard is not widened: an update relabelling the delegator is refused FORBIDDEN / 403', async () => {
156-
const own = await engine.insert(OBJECT, { delegate_id: 'u_tomas' }, asUser('u_admin'));
136+
const own = await engine.insert(OBJECT, { delegator_id: 'u_admin', delegate_id: 'u_tomas' }, asUser('u_admin'));
157137
await expect(
158138
engine.update(OBJECT, { id: own.id, delegator_id: 'u_priya' }, asUser('u_admin')),
159139
).rejects.toMatchObject({ code: 'FORBIDDEN', statusCode: 403 });

‎packages/plugins/plugin-approvals/src/sys-approval-delegation.object.ts‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -89,23 +89,15 @@ export const SysApprovalDelegation = ObjectSchema.create({
8989
// A delegation names its own author as delegator: on every non-system
9090
// write `bindDelegationWriteGuard` (lifecycle-hooks.ts) admits exactly one
9191
// value here, the acting user, and refuses any other with 403 (its
92-
// docstring holds why there is no admin exemption). So the field declares
93-
// that one value in the vocabulary the create form honours, instead of a
94-
// free `sys_user` lookup that offered every user and saw all but one
95-
// refused:
96-
// - `defaultValue: 'current_user'` — the create form opens on the acting
97-
// user, and an insert that omits the field stores the acting user
98-
// (`applyFieldDefaults`, before the guard runs; the guard stamps the
99-
// same value). `required` stays: a write with no acting user still has
100-
// no delegator, and is refused.
101-
// - `lookupFilters` — the picker's candidate query, which the engine
102-
// resolves `{current_user_id}` in for every read, so it offers the
103-
// acting user and nobody else.
104-
// Pinned in `sys-approval-delegation-delegator-picker.test.ts`.
92+
// docstring holds why there is no admin exemption). So the picker offers
93+
// that one value, instead of every user with all but one refused:
94+
// `lookupFilters` is the picker's candidate query, and the engine resolves
95+
// `{current_user_id}` against the caller on every read. Presentation only
96+
// — the guard stays the enforcement, and what a write may store is
97+
// unchanged. Pinned in `sys-approval-delegation-delegator-picker.test.ts`.
10598
delegator_id: Field.lookup('sys_user', {
10699
label: 'Delegator',
107100
required: true,
108-
defaultValue: 'current_user',
109101
lookupFilters: [{ field: 'id', operator: 'eq', value: '{current_user_id}' }],
110102
group: 'Delegation',
111103
description: 'The user going out of office; their individually-routed approver slots are rerouted while active.',

0 commit comments

Comments
 (0)