diff --git a/.superpowers/sdd/admin-user-management/task-1-report.md b/.superpowers/sdd/admin-user-management/task-1-report.md index 2b8b06c..3220667 100644 --- a/.superpowers/sdd/admin-user-management/task-1-report.md +++ b/.superpowers/sdd/admin-user-management/task-1-report.md @@ -117,3 +117,45 @@ Passed with no whitespace errors. - Shared-team membership now uses every requester `Player` row, including inactive ones, exactly as required by the directory plan. - The user query joins only `status` and `role`, projects safe raw columns, applies shared-team visibility/search in SQL, counts `DISTINCT user.id`, orders by `user.id`, and applies offset/limit before mapping. - Assignment rows are fetched only for the selected page of user IDs and are scoped with the same shared-team subquery for non-admins. No directory query selects or hydrates `User` authentication columns. + +## Fix Round 2 + +### Files changed + +- `myteamwallet_backend/src/users/users.service.spec.ts` — strengthens the inactive-requester regression with a QueryBuilder boundary that rejects `requesterPlayer.active` in the shared-team predicate. + +### RED evidence + +Test file: `myteamwallet_backend/src/users/users.service.spec.ts` + +After installing the boundary guard, the shared-team query was deliberately mutated to add `requesterPlayer.active = :active`. + +```powershell +npm test -- users/users.service.spec.ts --runInBand +``` + +Result: failed as expected, 1/9 tests failed. `treats an inactive requester assignment as a shared team membership` failed with `shared-team membership must not filter inactive requester assignments`. The mutation was then removed; the production query remains user-ID-only. + +### GREEN verification + +```powershell +npm test -- users/users.service.spec.ts --runInBand +``` + +Passed: 1 suite, 9 tests. + +```powershell +.\node_modules\.bin\eslint.cmd src\users\users.service.spec.ts --max-warnings=0 +``` + +Passed with no warnings or errors. + +```powershell +git diff --check +``` + +Passed with no whitespace errors. + +### Implementation notes + +- The test double checks the actual shared-team predicate supplied by the service, rather than returning fixed rows alone. It rejects only for the inactive-requester regression if a predicate references `requesterPlayer.active`, so the test now fails for the realistic authorization regression while preserving the existing output-contract assertions. diff --git a/myteamwallet_backend/src/users/users.service.spec.ts b/myteamwallet_backend/src/users/users.service.spec.ts index 8592345..c5d69f0 100644 --- a/myteamwallet_backend/src/users/users.service.spec.ts +++ b/myteamwallet_backend/src/users/users.service.spec.ts @@ -35,6 +35,7 @@ describe('UsersService directory', () => { let userRows: ReturnType[]; let assignmentRows: ReturnType[]; let total: number; + let rejectActiveSharedTeamPredicate: boolean; const usersRepository = { find: jest.fn(() => { throw new Error('directory queries must use a safe database projection'); @@ -55,6 +56,7 @@ describe('UsersService directory', () => { beforeEach(() => { jest.clearAllMocks(); + rejectActiveSharedTeamPredicate = false; setDirectoryResult( [users[0], users[1], users[2], users[4]], [players[0], players[1], players[2], players[4]], @@ -105,6 +107,7 @@ describe('UsersService directory', () => { }); it('treats an inactive requester assignment as a shared team membership', async () => { + rejectActiveSharedTeamPredicate = true; setDirectoryResult( [users[0], users[1]], [ @@ -353,7 +356,17 @@ describe('UsersService directory', () => { function createSharedTeamsQuery() { const query: any = { select: () => query, - where: () => query, + where: (predicate: string) => { + if ( + rejectActiveSharedTeamPredicate && + /requesterPlayer\.active/i.test(predicate) + ) { + throw new Error( + 'shared-team membership must not filter inactive requester assignments', + ); + } + return query; + }, getQuery: () => 'SELECT requesterPlayer.teamId FROM player requesterPlayer', getParameters: () => ({ requesterId: users[0].id }),