|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * [#29880] `GET /api/v1/approvals/requests` reads `?status=` against the value |
| 5 | + * set `sys_approval_request.status` declares, instead of handing any string to |
| 6 | + * the service. |
| 7 | + * |
| 8 | + * ## The measured defect (c5800818) |
| 9 | + * |
| 10 | + * `?status=bogus` and `?status=Pending` answered **200 `{"data":[]}`**: the |
| 11 | + * service filtered on a status no row can have and found nothing. A caller |
| 12 | + * could not tell "nothing in that state" from "that state does not exist". |
| 13 | + * `?status=pending,returned` (what `@objectstack/client` sends for an array) |
| 14 | + * was forwarded as ONE string and answered the same empty list. |
| 15 | + * |
| 16 | + * ## What each case asserts |
| 17 | + * |
| 18 | + * A refusal asserts the nested ADR-0112 pair the name and arity gates on this |
| 19 | + * handler answer, the located message (the value and every allowed value), and |
| 20 | + * that **the service was never asked**. A preservation case asserts the |
| 21 | + * ARGUMENT the service received, because "still 200" is exactly what the |
| 22 | + * defect looked like. |
| 23 | + */ |
| 24 | + |
| 25 | +import { describe, it, expect, vi } from 'vitest'; |
| 26 | +// `.js` on purpose — NodeNext resolution requires the extension, and this |
| 27 | +// package's TEST_DEBT ceiling has no margin for another TS2835 (#7248). |
| 28 | +import { RestServer } from './rest-server.js'; |
| 29 | +import { APPROVAL_STATUSES } from '@objectstack/spec/contracts'; |
| 30 | + |
| 31 | +const APPROVALS = '/api/v1/approvals/requests'; |
| 32 | + |
| 33 | +function mockServer() { |
| 34 | + return { |
| 35 | + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), |
| 36 | + use: vi.fn(), |
| 37 | + listen: vi.fn().mockResolvedValue(undefined), |
| 38 | + close: vi.fn().mockResolvedValue(undefined), |
| 39 | + }; |
| 40 | +} |
| 41 | + |
| 42 | +function mockRes() { |
| 43 | + const res: any = { |
| 44 | + statusCode: 200, |
| 45 | + json: vi.fn(function (this: any, body: any) { this._body = body; return this; }), |
| 46 | + send: vi.fn(function (this: any) { return this; }), |
| 47 | + setHeader: vi.fn(function (this: any) { return this; }), |
| 48 | + status: vi.fn(function (this: any, code: number) { this.statusCode = code; return this; }), |
| 49 | + }; |
| 50 | + return res; |
| 51 | +} |
| 52 | + |
| 53 | +/** Two rows, so "the unfiltered list came back" is visibly distinguishable. */ |
| 54 | +const ALL_ROWS = [{ id: 'req_1' }, { id: 'req_2' }]; |
| 55 | + |
| 56 | +/** |
| 57 | + * A RestServer with a real approvals service wired, so a request that is NOT |
| 58 | + * refused runs all the way to `listRequests` — which is what lets the |
| 59 | + * preservation cases assert the filter argument rather than the status. |
| 60 | + */ |
| 61 | +function boot() { |
| 62 | + const approvals = { |
| 63 | + listRequests: vi.fn().mockResolvedValue(ALL_ROWS), |
| 64 | + countRequests: vi.fn().mockResolvedValue(2), |
| 65 | + }; |
| 66 | + const protocol: any = { |
| 67 | + getDiscovery: vi.fn().mockResolvedValue({ |
| 68 | + version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' }, |
| 69 | + }), |
| 70 | + getMetaTypes: vi.fn().mockResolvedValue([]), |
| 71 | + getMetaItems: vi.fn().mockResolvedValue({ items: [] }), |
| 72 | + findData: vi.fn().mockResolvedValue({ records: [] }), |
| 73 | + }; |
| 74 | + const rest = new RestServer( |
| 75 | + mockServer() as any, |
| 76 | + protocol as any, |
| 77 | + { api: { requireAuth: false } } as any, |
| 78 | + undefined, // kernelManager |
| 79 | + undefined, // envRegistry |
| 80 | + undefined, // defaultEnvironmentIdProvider |
| 81 | + undefined, // authServiceProvider |
| 82 | + undefined, // objectQLProvider |
| 83 | + undefined, // emailServiceProvider |
| 84 | + undefined, // sharingServiceProvider |
| 85 | + undefined, // reportsServiceProvider |
| 86 | + async () => approvals, // approvalsServiceProvider |
| 87 | + ); |
| 88 | + // `isSystem` clears the capability gates that run BEFORE the query is read, |
| 89 | + // so every request below reaches the recognition rule it is named after. |
| 90 | + (rest as any).resolveExecCtx = async () => ({ isSystem: true, userId: 'u1' }); |
| 91 | + rest.registerRoutes(); |
| 92 | + |
| 93 | + const drive = async (query: Record<string, unknown> = {}) => { |
| 94 | + const found = (rest as any).getRoutes().find( |
| 95 | + (r: any) => r.method === 'GET' && r.path === APPROVALS, |
| 96 | + ); |
| 97 | + if (!found) throw new Error(`route not registered: GET ${APPROVALS}`); |
| 98 | + const res = mockRes(); |
| 99 | + await found.handler( |
| 100 | + { method: 'GET', path: APPROVALS, params: {}, query, headers: {}, body: {} } as any, |
| 101 | + res, |
| 102 | + ); |
| 103 | + return { status: res.statusCode, body: res.json.mock.calls.at(-1)?.[0] }; |
| 104 | + }; |
| 105 | + |
| 106 | + return { approvals, drive }; |
| 107 | +} |
| 108 | + |
| 109 | + |
| 110 | +/** The refusal: the shared envelope, the located sentence, no service call. */ |
| 111 | +async function expectStatusRefusal(status: unknown, ...named: string[]) { |
| 112 | + const { drive, approvals } = boot(); |
| 113 | + const answer = await drive({ status }); |
| 114 | + expect( |
| 115 | + answer.status, |
| 116 | + `?status=${JSON.stringify(status)} must be refused, got ${answer.status} ` |
| 117 | + + `with body ${JSON.stringify(answer.body)}`, |
| 118 | + ).toBe(400); |
| 119 | + expect(answer.body?.error?.code).toBe('VALIDATION_ERROR'); |
| 120 | + expect(answer.body?.data).toBeUndefined(); |
| 121 | + const message = String(answer.body?.error?.message); |
| 122 | + expect(message).toContain('"status"'); |
| 123 | + for (const value of named) expect(message).toContain(`"${value}"`); |
| 124 | + expect(message).toContain(`Allowed values: ${APPROVAL_STATUSES.join(', ')}.`); |
| 125 | + expect( |
| 126 | + approvals.listRequests, |
| 127 | + 'the service must not be asked: an empty answer for a status that does not exist is the defect', |
| 128 | + ).not.toHaveBeenCalled(); |
| 129 | + expect(approvals.countRequests).not.toHaveBeenCalled(); |
| 130 | +} |
| 131 | + |
| 132 | +/** The status argument the service received for one request. */ |
| 133 | +async function statusHandedToService(query: Record<string, unknown>) { |
| 134 | + const { drive, approvals } = boot(); |
| 135 | + const answer = await drive(query); |
| 136 | + expect(answer.status, `body ${JSON.stringify(answer.body)}`).toBe(200); |
| 137 | + expect(approvals.listRequests).toHaveBeenCalledTimes(1); |
| 138 | + return { filter: approvals.listRequests.mock.calls[0][0], answer, approvals }; |
| 139 | +} |
| 140 | + |
| 141 | +describe('#29880 §1 — a status value outside the declared set is refused', () => { |
| 142 | + it('?status=bogus answers 400 naming the value and the allowed values', async () => { |
| 143 | + await expectStatusRefusal('bogus', 'bogus'); |
| 144 | + }); |
| 145 | + |
| 146 | + it('the match is exact: ?status=Pending is refused, not folded to pending', async () => { |
| 147 | + await expectStatusRefusal('Pending', 'Pending'); |
| 148 | + }); |
| 149 | + |
| 150 | + it('one bad entry in a comma list refuses the whole request, and each bad entry is named once', async () => { |
| 151 | + await expectStatusRefusal('pending,waiting,waiting', 'waiting'); |
| 152 | + const { drive } = boot(); |
| 153 | + const { body } = await drive({ status: 'pending,waiting,waiting' }); |
| 154 | + expect(String(body?.error?.message).match(/"waiting"/g)).toHaveLength(1); |
| 155 | + }); |
| 156 | + |
| 157 | + it('an empty value or an empty entry names no status and is refused, not read as "every status"', async () => { |
| 158 | + await expectStatusRefusal('', ''); |
| 159 | + await expectStatusRefusal('pending,', ''); |
| 160 | + }); |
| 161 | + |
| 162 | + it('a value that is not a string (a nested query object) is refused', async () => { |
| 163 | + const { drive, approvals } = boot(); |
| 164 | + const answer = await drive({ status: { $ne: 'pending' } }); |
| 165 | + expect(answer.status).toBe(400); |
| 166 | + expect(answer.body?.error?.code).toBe('VALIDATION_ERROR'); |
| 167 | + expect(approvals.listRequests).not.toHaveBeenCalled(); |
| 168 | + }); |
| 169 | + |
| 170 | + it('the name and arity gates still answer first', async () => { |
| 171 | + const { drive } = boot(); |
| 172 | + const unknownName = await drive({ status: 'bogus', assignedToMe: 'true' }); |
| 173 | + expect(String(unknownName.body?.error?.message)).toContain('"assignedToMe"'); |
| 174 | + const repeated = await drive({ status: ['bogus', 'pending'] }); |
| 175 | + expect(String(repeated.body?.error?.message)).toContain('supplied 2 times'); |
| 176 | + }); |
| 177 | +}); |
| 178 | + |
| 179 | +describe('#29880 §2 — every declared status, and lists of them, still reach the service', () => { |
| 180 | + it.each([...APPROVAL_STATUSES])('?status=%s is handed to the service as that string', async (value) => { |
| 181 | + const { filter } = await statusHandedToService({ status: value }); |
| 182 | + expect(filter.status).toBe(value); |
| 183 | + }); |
| 184 | + |
| 185 | + it('a known status with no rows still answers 200 with an empty list', async () => { |
| 186 | + const { drive, approvals } = boot(); |
| 187 | + approvals.listRequests.mockResolvedValue([]); |
| 188 | + const answer = await drive({ status: 'cancelled' }); |
| 189 | + expect(answer.status).toBe(200); |
| 190 | + expect(answer.body).toEqual({ data: [] }); |
| 191 | + expect(approvals.listRequests.mock.calls[0][0].status).toBe('cancelled'); |
| 192 | + }); |
| 193 | + |
| 194 | + it('a comma list reaches the service as an array (the spelling @objectstack/client sends)', async () => { |
| 195 | + const { filter } = await statusHandedToService({ status: 'pending,returned' }); |
| 196 | + expect(filter.status).toEqual(['pending', 'returned']); |
| 197 | + }); |
| 198 | + |
| 199 | + it('spaces around entries are trimmed and a repeated entry is sent once', async () => { |
| 200 | + const { filter } = await statusHandedToService({ status: ' approved , rejected,approved' }); |
| 201 | + expect(filter.status).toEqual(['approved', 'rejected']); |
| 202 | + }); |
| 203 | + |
| 204 | + it('no ?status= leaves the filter unset, and paging still counts on the same filter', async () => { |
| 205 | + const { filter } = await statusHandedToService({}); |
| 206 | + expect(filter.status).toBeUndefined(); |
| 207 | + const paged = await statusHandedToService({ status: 'pending,returned', limit: '10' }); |
| 208 | + expect(paged.approvals.countRequests).toHaveBeenCalledTimes(1); |
| 209 | + expect(paged.approvals.countRequests.mock.calls[0][0].status).toEqual(['pending', 'returned']); |
| 210 | + expect(paged.answer.body).toEqual({ data: [{ id: 'req_1' }, { id: 'req_2' }], total: 2 }); |
| 211 | + }); |
| 212 | +}); |
![objectstack-fleet[bot]](https://avatars.githubusercontent.com/u/254823548?v=4&size=40){kind=avatar}
0 commit comments