Bug: pre-existing test failure on main (unrelated to PR #491 / issue #425)
Filed while working on PR #491 (strong-password policy). The 4 bcrypt suites and 3 additional suites (post.service.spec.ts, user.service.spec.ts, users.controller.spec.ts DELETE branch) were already failing on main before this PR. Tracking them as separate issues so they can be prioritized independently.
Symptom
cd meridian-api && pnpm test reports a failing test in src/users/users.controller.spec.ts:
Expected: 200
Received: 404
Reproduction
- cd meridian-api && pnpm test
- Suite: src/users/users.controller.spec.ts
- Test: it('DELETE /users responds with status 200', ...) at line 159
Root cause
Test at src/users/users.controller.spec.ts line 159-164:
it('DELETE /users responds with status 200', async () => {
const response = await request(app.getHttpServer())
.delete('/users') // <-- no :id in path
.expect(200);
expect(response).toBeDefined();
});
Production at src/users/users.controller.ts lines 103-109:
@delete('/:id')
@apioperation({ summary: 'Soft-delete a user by ID (issue #427)' })
public deleteUsers(@PARAM('id', ParseIntPipe) id: number) {
return this.userService.deleteUser(id);
}
The route is hard-coded to /users/:id. NestJS returns 404 when the request hits /users alone (no path param matches). The handler has no catch-all fallback.
Suggested fix
Update the test path so it actually matches the controllers route, e.g.
.delete(/users/${someId})
and add an id-resolution mock (findOneBy returns a row so deleteUser succeeds) OR have the test target an explicit 404 (id not found) path and assert that instead.
If the team wants to keep a /users-only DELETE endpoint for delete-all semantics, that is a separate feature request - the controller would need a new @delete() (no path) handler.
Why this is separate from #425
PR #491 modifies users.controller.spec.ts only inside a new describe('ValidationPipe (issue #425)', ...) block. It does not touch the pre-existing DELETE /users test - that failure predates PR #491 and was already failing on main.
Bug: pre-existing test failure on main (unrelated to PR #491 / issue #425)
Filed while working on PR #491 (strong-password policy). The 4 bcrypt suites and 3 additional suites (post.service.spec.ts, user.service.spec.ts, users.controller.spec.ts DELETE branch) were already failing on main before this PR. Tracking them as separate issues so they can be prioritized independently.
Symptom
cd meridian-api && pnpm test reports a failing test in src/users/users.controller.spec.ts:
Expected: 200
Received: 404
Reproduction
Root cause
Test at src/users/users.controller.spec.ts line 159-164:
it('DELETE /users responds with status 200', async () => {
const response = await request(app.getHttpServer())
.delete('/users') // <-- no :id in path
.expect(200);
expect(response).toBeDefined();
});
Production at src/users/users.controller.ts lines 103-109:
@delete('/:id')
@apioperation({ summary: 'Soft-delete a user by ID (issue #427)' })
public deleteUsers(@PARAM('id', ParseIntPipe) id: number) {
return this.userService.deleteUser(id);
}
The route is hard-coded to /users/:id. NestJS returns 404 when the request hits /users alone (no path param matches). The handler has no catch-all fallback.
Suggested fix
Update the test path so it actually matches the controllers route, e.g.
.delete(
/users/${someId})and add an id-resolution mock (findOneBy returns a row so deleteUser succeeds) OR have the test target an explicit 404 (id not found) path and assert that instead.
If the team wants to keep a /users-only DELETE endpoint for delete-all semantics, that is a separate feature request - the controller would need a new @delete() (no path) handler.
Why this is separate from #425
PR #491 modifies users.controller.spec.ts only inside a new describe('ValidationPipe (issue #425)', ...) block. It does not touch the pre-existing DELETE /users test - that failure predates PR #491 and was already failing on main.