Skip to content

Commit bcd6f68

Browse files
os-zhuangclaude
andcommitted
perf(approvals): accept multiple approverIds in one listRequests call
The Console topbar badge resolved "my pending approvals" by looping one GET /approvals/requests per identity (user id, email, each role:<r>), firing N near-simultaneous requests every poll — a dominant source of duplicate control-plane traffic. Widen the listRequests filter (spec contract + service + client) and the REST handler to accept `approverId` as a single value, a comma-separated list, or a repeated query param, matching a request when ANY identity is a pending approver. The client now sends one request for all identities. Backward compatible: a single approverId still works. Adds a test covering list matching + empty-id handling. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 05dacf2 commit bcd6f68

5 files changed

Lines changed: 54 additions & 7 deletions

File tree

packages/client/src/index.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2519,7 +2519,7 @@ export class ObjectStackClient {
25192519
object?: string;
25202520
recordId?: string;
25212521
status?: ApprovalStatus | ApprovalStatus[];
2522-
approverId?: string;
2522+
approverId?: string | string[];
25232523
submitterId?: string;
25242524
}): Promise<ApprovalRequestRow[]> => {
25252525
const route = this.getRoute('approvals');
@@ -2529,7 +2529,9 @@ export class ObjectStackClient {
25292529
if (filter?.status) {
25302530
params.set('status', Array.isArray(filter.status) ? filter.status.join(',') : filter.status);
25312531
}
2532-
if (filter?.approverId) params.set('approverId', filter.approverId);
2532+
if (filter?.approverId) {
2533+
params.set('approverId', Array.isArray(filter.approverId) ? filter.approverId.join(',') : filter.approverId);
2534+
}
25332535
if (filter?.submitterId) params.set('submitterId', filter.submitterId);
25342536
const qs = params.toString();
25352537
const res = await this.fetch(`${this.baseUrl}${route}/requests${qs ? `?${qs}` : ''}`);

packages/plugins/plugin-approvals/src/approval-service.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,22 @@ describe('ApprovalService (node era)', () => {
264264
expect(none).toHaveLength(0);
265265
});
266266

267+
it('listRequests: approverId accepts a list and matches ANY identity', async () => {
268+
await svc.openNodeRequest(openInput(['u9']), CTX);
269+
// None of these identities individually except the last is the approver.
270+
const hit = await svc.listRequests(
271+
{ status: 'pending', approverId: ['someone-else', 'user@example.com', 'u9'] },
272+
SYS,
273+
);
274+
expect(hit).toHaveLength(1);
275+
// A list with no matching identity returns nothing.
276+
const miss = await svc.listRequests({ approverId: ['a', 'b', 'role:viewer'] }, SYS);
277+
expect(miss).toHaveLength(0);
278+
// Empty / whitespace-only ids are ignored, not treated as a match-all.
279+
const ignored = await svc.listRequests({ approverId: ['', ' '] }, SYS);
280+
expect(ignored).toHaveLength(1);
281+
});
282+
267283
it('listActions: returns the audit trail for a request', async () => {
268284
const req = await svc.openNodeRequest(openInput(['u9']), CTX);
269285
await svc.decideNode(req.id, { decision: 'approve', actorId: 'u9' }, SYS);

packages/plugins/plugin-approvals/src/approval-service.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ export class ApprovalService implements IApprovalService {
470470
object?: string;
471471
recordId?: string;
472472
status?: ApprovalStatus | ApprovalStatus[];
473-
approverId?: string;
473+
approverId?: string | string[];
474474
submitterId?: string;
475475
} | undefined,
476476
context: SharingExecutionContext,
@@ -498,8 +498,20 @@ export class ApprovalService implements IApprovalService {
498498
let list = Array.isArray(rows) ? rows.map(rowFromRequest) : [];
499499
if (statusFilter) list = list.filter(r => statusFilter!.includes(r.status));
500500
if (filter?.approverId) {
501-
const target = filter.approverId;
502-
list = list.filter(r => (r.pending_approvers ?? []).includes(target));
501+
// Accept one identity or a list: a request matches when ANY of the
502+
// caller's identities (user id / email / role:<r>) is a pending
503+
// approver. This lets the Console badge fetch "my pending approvals"
504+
// in a single request instead of one-per-identity (previously the
505+
// client looped, firing N near-simultaneous calls per poll).
506+
const targets = (Array.isArray(filter.approverId) ? filter.approverId : [filter.approverId])
507+
.map(t => String(t).trim())
508+
.filter(Boolean);
509+
if (targets.length) {
510+
list = list.filter(r => {
511+
const pending = r.pending_approvers ?? [];
512+
return targets.some(t => pending.includes(t));
513+
});
514+
}
503515
}
504516
return list;
505517
}

packages/rest/src/rest-server.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3990,11 +3990,21 @@ export class RestServer {
39903990
return;
39913991
}
39923992
const q = req.query ?? {};
3993+
// `approverId` accepts a single id, a comma-separated
3994+
// list, or the param repeated (→ array). Normalise all
3995+
// three to a string[] so the Console can resolve "my
3996+
// pending approvals" across every identity (user id /
3997+
// email / role:<r>) in ONE request rather than looping.
3998+
const rawApprover = q.approverId ?? q.approver_id;
3999+
const approverIds = (Array.isArray(rawApprover) ? rawApprover : (rawApprover != null ? [rawApprover] : []))
4000+
.flatMap((s: any) => String(s).split(','))
4001+
.map((s: string) => s.trim())
4002+
.filter(Boolean);
39934003
const rows = await svc.listRequests({
39944004
object: q.object,
39954005
recordId: q.recordId ?? q.record_id,
39964006
status: q.status,
3997-
approverId: q.approverId ?? q.approver_id,
4007+
approverId: approverIds.length ? approverIds : undefined,
39984008
submitterId: q.submitterId ?? q.submitter_id,
39994009
}, context ?? {});
40004010
res.json({ data: rows });

packages/spec/src/contracts/approval-service.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,14 @@ export interface IApprovalService {
8888
object?: string;
8989
recordId?: string;
9090
status?: ApprovalStatus | ApprovalStatus[];
91-
approverId?: string;
91+
/**
92+
* Match requests where ANY of these identities is a pending approver.
93+
* Accepts a single id or a list (a user typically has several
94+
* identities: their user id, email, and `role:<r>` entries). Passing
95+
* the list lets a caller resolve "my pending approvals" in ONE request
96+
* instead of one request per identity.
97+
*/
98+
approverId?: string | string[];
9299
submitterId?: string;
93100
} | undefined,
94101
context: SharingExecutionContext,

0 commit comments

Comments
 (0)