diff --git a/server/src/nest/place-contrib/place-contrib.controller.ts b/server/src/nest/place-contrib/place-contrib.controller.ts index 6d4e86890..8eabf9793 100644 --- a/server/src/nest/place-contrib/place-contrib.controller.ts +++ b/server/src/nest/place-contrib/place-contrib.controller.ts @@ -4,6 +4,7 @@ import { getClientIp } from '../audit/client-ip'; import { AdminGuard } from '../auth-core/admin.guard'; import { CurrentUser } from '../auth-core/current-user.decorator'; import { JwtAuthGuard } from '../auth-core/jwt-auth.guard'; +import { DomainError } from '../common/domain-error'; import { ResponseContract } from '../common/response-contract'; import { PlaceContribDecisionDto, PlaceContribLogQueryDto, PlaceContribWithdrawDto } from './place-contrib.dto'; import { PlaceContribService } from './place-contrib.service'; @@ -49,14 +50,26 @@ export class PlaceContribController { @Body() body: PlaceContribDecisionDto, @Req() req: Request, ): Promise { - // The pipe validated the body against the full union; the DTO only types its shared fields. - const state = await this.contrib.decide(user.id, body as PlaceContribDecisionRequest); - await this.audit.writeAudit({ - userId: user.id, - action: 'admin.places_contribute_decision', - details: decisionDetails(body), - ip: getClientIp(req), - }); + const audit = (details: Record) => + this.audit.writeAudit({ + userId: user.id, + action: 'admin.places_contribute_decision', + details, + ip: getClientIp(req), + }); + let state: PlaceContribState; + try { + // The pipe validated the body against the full union; the DTO only types its shared fields. + state = await this.contrib.decide(user.id, body as PlaceContribDecisionRequest); + } catch (err) { + // Switching off commits before the withdrawal asks the service, so an + // unconfirmed withdrawal still leaves a decision that took effect. + if (body.action === 'off' && err instanceof DomainError && err.getStatus() === 502) { + await audit({ ...decisionDetails(body), withdrawFailed: true }); + } + throw err; + } + await audit(decisionDetails(body)); return state; } diff --git a/server/tests/unit/nest/place-contrib/place-contrib.controller.test.ts b/server/tests/unit/nest/place-contrib/place-contrib.controller.test.ts index 1b10548e7..23d465dc0 100644 --- a/server/tests/unit/nest/place-contrib/place-contrib.controller.test.ts +++ b/server/tests/unit/nest/place-contrib/place-contrib.controller.test.ts @@ -7,6 +7,7 @@ import type { AuditService } from '../../../../src/nest/audit/audit.service'; import { AdminGuard } from '../../../../src/nest/auth-core/admin.guard'; import { JwtAuthGuard } from '../../../../src/nest/auth-core/jwt-auth.guard'; +import { DomainError } from '../../../../src/nest/common/domain-error'; import { RESPONSE_CONTRACT_KEY } from '../../../../src/nest/common/response-contract'; import { PlaceContribController } from '../../../../src/nest/place-contrib/place-contrib.controller'; import { @@ -87,6 +88,20 @@ describe('PlaceContribController', () => { expect(audit.writeAudit).not.toHaveBeenCalled(); }); + it('PCCT-010: switching off whose withdrawal the service did not confirm is still audited, then fails', async () => { + const refused = new DomainError(502, 'The TREK Places API did not confirm the withdrawal'); + contrib.decide.mockRejectedValueOnce(refused); + await expect( + controller.decide(admin, { action: 'off', withdraw: true } as PlaceContribDecisionDto, req), + ).rejects.toBe(refused); + expect(audit.writeAudit).toHaveBeenCalledWith({ + userId: 3, + action: 'admin.places_contribute_decision', + details: { action: 'off', withdraw: true, withdrawFailed: true }, + ip: '203.0.113.9', + }); + }); + it('PCCT-006: the log query goes to the service as parsed', async () => { const query = { page: 2, perPage: 10, result: 'withheld' } as PlaceContribLogQueryDto; expect(await controller.log(query)).toEqual({ entries: [], total: 0, page: 2, perPage: 10 });