feat(server): adopt zod DTOs for the packing body contracts

This commit is contained in:
jubnl
2026-07-26 16:55:53 +02:00
parent f1960dc364
commit 5da2abf4f9
6 changed files with 173 additions and 99 deletions
+36
View File
@@ -118,6 +118,7 @@ vi.mock('../../src/services/permissions', () => ({ checkPermission }));
import { PackingModule } from '../../src/nest/packing/packing.module';
import { DatabaseModule } from '../../src/nest/database/database.module';
import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter';
import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe';
function insertItem(tripId: number, name: string, extra: Partial<{ sort_order: number; category: string; is_private: number; owner_id: number }> = {}): number {
const res = db
@@ -136,6 +137,9 @@ describe('Packing e2e (real auth guard + real SQL over temp SQLite)', () => {
const nest = moduleRef.createNestApplication();
nest.use(cookieParser());
nest.useGlobalFilters(new TrekExceptionFilter());
// Mirror the production APP_PIPE (app.module.ts): the DTO-typed bodies
// validate by metatype, exactly as they do under buildApp().
nest.useGlobalPipes(new ZodValidationPipe());
await nest.init();
return nest;
}
@@ -275,6 +279,38 @@ describe('Packing e2e (real auth guard + real SQL over temp SQLite)', () => {
expect(res.body).toEqual({ error: 'items must be a non-empty array' });
});
it('400 from the Zod pipe on create without a name', async () => {
const res = await request(server).post(`/api/trips/${tripId}/packing`).set('Cookie', sessionCookie(1)).send({});
expect(res.status).toBe(400);
expect(res.body.error).toContain('name');
});
it('400 from the Zod pipe on reorder without orderedIds', async () => {
const res = await request(server).put(`/api/trips/${tripId}/packing/reorder`).set('Cookie', sessionCookie(1)).send({});
expect(res.status).toBe(400);
expect(res.body.error).toContain('orderedIds');
});
it('400 from the Zod pipe on sharing with an invalid visibility', async () => {
const id = insertItem(tripId, 'Tent');
const res = await request(server).put(`/api/trips/${tripId}/packing/${id}/sharing`).set('Cookie', sessionCookie(1)).send({ visibility: 'secret' });
expect(res.status).toBe(400);
expect(res.body.error).toContain('visibility');
});
it('accepts the legacy numeric checked form through the pipe', async () => {
const id = insertItem(tripId, 'Toggle me');
const res = await request(server).put(`/api/trips/${tripId}/packing/${id}`).set('Cookie', sessionCookie(1)).send({ checked: 1 });
expect(res.status).toBe(200);
expect(res.body.item.checked).toBe(1);
});
it('whitespace-only bag name still gets the bespoke 400', async () => {
const res = await request(server).post(`/api/trips/${tripId}/packing/bags`).set('Cookie', sessionCookie(1)).send({ name: ' ' });
expect(res.status).toBe(400);
expect(res.body).toEqual({ error: 'Name is required' });
});
it('bags round-trip: 201 create (default color), 200 update (COALESCE), 200 delete, then 404', async () => {
const created = await request(server).post(`/api/trips/${tripId}/packing/bags`).set('Cookie', sessionCookie(1)).send({ name: 'Duffel' });
expect(created.status).toBe(201);
@@ -57,12 +57,10 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
});
});
it('400 when name missing', () => {
const svc = makeService();
expect(thrown(() => new PackingController(svc).create(user, '5', {}))).toEqual({
status: 400, body: { error: 'Item name is required' },
});
});
// The missing-name 400 moved from a bespoke controller check into the
// ZodValidationPipe (packingCreateItemRequestSchema) — direct method calls
// bypass parameter pipes, so that path is covered by the e2e suite and
// the schema spec in @trek/shared.
it('creates an item (owned by the creator) and broadcasts a Common item to the room', () => {
// Common item (is_private falsy) → viewersOf null → whole-room broadcast.
@@ -93,23 +91,20 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
});
describe('POST /import', () => {
it('400 when items is not a non-empty array (empty array)', () => {
it('400 when items is an empty array (bespoke check, schema permits [])', () => {
const svc = makeService();
expect(thrown(() => new PackingController(svc).importItems(user, '5', []))).toEqual({
expect(thrown(() => new PackingController(svc).importItems(user, '5', { items: [] }))).toEqual({
status: 400, body: { error: 'items must be a non-empty array' },
});
});
it('400 when items is not an array at all (non-array branch)', () => {
const svc = makeService();
expect(thrown(() => new PackingController(svc).importItems(user, '5', 'nope'))).toEqual({
status: 400, body: { error: 'items must be a non-empty array' },
});
});
// The non-array 400 moved into the ZodValidationPipe (packingImportRequestSchema
// requires an array) — direct method calls bypass parameter pipes, so that
// path is covered by the e2e suite and the schema spec in @trek/shared.
it('403 without packing_edit permission', () => {
const svc = makeService({ canEdit: vi.fn().mockReturnValue(false) });
expect(thrown(() => new PackingController(svc).importItems(user, '5', [{ name: 'a' }]))).toEqual({
expect(thrown(() => new PackingController(svc).importItems(user, '5', { items: [{ name: 'a' }] }))).toEqual({
status: 403, body: { error: 'No permission' },
});
});
@@ -118,7 +113,7 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
const bulkImport = vi.fn().mockReturnValue([{ id: 1 }, { id: 2 }]);
const broadcastItem = vi.fn();
const svc = makeService({ bulkImport, broadcastItem } as Partial<PackingService>);
const res = new PackingController(svc).importItems(user, '5', [{ name: 'a' }, { name: 'b' }], 'sock');
const res = new PackingController(svc).importItems(user, '5', { items: [{ name: 'a' }, { name: 'b' }] }, 'sock');
expect(res).toEqual({ items: [{ id: 1 }, { id: 2 }], count: 2 });
expect(bulkImport).toHaveBeenCalledWith('5', [{ name: 'a' }, { name: 'b' }], user.id);
expect(broadcastItem).toHaveBeenCalledTimes(2);
@@ -138,8 +133,9 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
const broadcast = vi.fn();
const svc = makeService({ updateItem, broadcast } as Partial<PackingService>);
new PackingController(svc).update(user, '5', '9', { name: 'X', checked: true }, 'sock');
// acting user id is forwarded so privatizing an unowned item can stamp the owner (#858)
expect(updateItem).toHaveBeenCalledWith('5', '9', expect.objectContaining({ name: 'X', checked: true }), ['name', 'checked'], undefined, user.id);
// acting user id is forwarded so privatizing an unowned item can stamp the
// owner (#858); checked is normalized to the 0/1 the SQL binds.
expect(updateItem).toHaveBeenCalledWith('5', '9', expect.objectContaining({ name: 'X', checked: 1 }), ['name', 'checked'], undefined, user.id);
// A public item (is_private undefined, was public) broadcasts to the whole room.
expect(broadcast).toHaveBeenCalledWith('5', 'packing:updated', { item: { id: 9, name: 'X' } }, 'sock');
});
@@ -196,13 +192,13 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
it('reorders the items and reports success', () => {
const reorderItems = vi.fn();
const svc = makeService({ reorderItems } as Partial<PackingService>);
expect(new PackingController(svc).reorder(user, '5', [3, 1, 2])).toEqual({ success: true });
expect(new PackingController(svc).reorder(user, '5', { orderedIds: [3, 1, 2] })).toEqual({ success: true });
expect(reorderItems).toHaveBeenCalledWith('5', [3, 1, 2]);
});
it('403 without packing_edit permission', () => {
const svc = makeService({ canEdit: vi.fn().mockReturnValue(false) });
expect(thrown(() => new PackingController(svc).reorder(user, '5', [1]))).toEqual({
expect(thrown(() => new PackingController(svc).reorder(user, '5', { orderedIds: [1] }))).toEqual({
status: 403, body: { error: 'No permission' },
});
});
@@ -236,8 +232,11 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
});
describe('sharing, contributors, clone (#858 three-tier)', () => {
it('PUT /:id/sharing 400 invalid, 404 missing, 403 non-owner, else drops + re-emits', () => {
expect(thrown(() => new PackingController(makeService()).setSharing(user, '5', '9', { visibility: 'nope' as never }))).toEqual({ status: 400, body: { error: 'Invalid visibility' } });
// The invalid-visibility 400 moved into the ZodValidationPipe
// (packingSetSharingRequestSchema requires the enum) — direct method calls
// bypass parameter pipes, so that path is covered by the e2e suite and the
// schema spec in @trek/shared.
it('PUT /:id/sharing 404 missing, 403 non-owner, else drops + re-emits', () => {
expect(thrown(() => new PackingController(makeService({ setItemSharing: vi.fn().mockReturnValue(null) } as Partial<PackingService>)).setSharing(user, '5', '9', { visibility: 'personal' }))).toEqual({ status: 404, body: { error: 'Item not found' } });
expect(thrown(() => new PackingController(makeService({ setItemSharing: vi.fn().mockReturnValue({ forbidden: true }) } as Partial<PackingService>)).setSharing(user, '5', '9', { visibility: 'personal' }))).toEqual({ status: 403, body: { error: 'Only the owner can change sharing' } });
@@ -292,19 +291,16 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
expect(new PackingController(svc).listBags(user, '5')).toEqual({ bags: [{ id: 3, name: 'Carry-on' }] });
});
it('400 on bag create with blank name', () => {
it('400 on bag create with blank name (bespoke check — the schema cannot see whitespace)', () => {
const svc = makeService();
expect(thrown(() => new PackingController(svc).createBag(user, '5', { name: ' ' }))).toEqual({
status: 400, body: { error: 'Name is required' },
});
});
it('400 on bag create with no name at all (optional-chain short-circuit)', () => {
const svc = makeService();
expect(thrown(() => new PackingController(svc).createBag(user, '5', {}))).toEqual({
status: 400, body: { error: 'Name is required' },
});
});
// The missing-name 400 moved into the ZodValidationPipe
// (packingCreateBagRequestSchema requires a non-empty name) — direct method
// calls bypass parameter pipes, so that path is covered by the e2e suite.
it('creates a bag and broadcasts', () => {
const createBag = vi.fn().mockReturnValue({ id: 3, name: 'Carry-on' });
@@ -349,27 +345,24 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
it('404 on set-members when the bag is missing', () => {
const svc = makeService({ setBagMembers: vi.fn().mockReturnValue(null) } as Partial<PackingService>);
expect(thrown(() => new PackingController(svc).setBagMembers(user, '5', '3', [1, 2]))).toEqual({
expect(thrown(() => new PackingController(svc).setBagMembers(user, '5', '3', { user_ids: [1, 2] }))).toEqual({
status: 404, body: { error: 'Bag not found' },
});
});
it('sets bag members and broadcasts (array branch)', () => {
it('sets bag members and broadcasts', () => {
const setBagMembers = vi.fn().mockReturnValue([{ user_id: 1 }, { user_id: 2 }]);
const broadcast = vi.fn();
const svc = makeService({ setBagMembers, broadcast } as Partial<PackingService>);
const res = new PackingController(svc).setBagMembers(user, '5', '3', [1, 2], 'sock');
const res = new PackingController(svc).setBagMembers(user, '5', '3', { user_ids: [1, 2] }, 'sock');
expect(res).toEqual({ members: [{ user_id: 1 }, { user_id: 2 }] });
expect(setBagMembers).toHaveBeenCalledWith('5', '3', [1, 2]);
expect(broadcast).toHaveBeenCalledWith('5', 'packing:bag-members-updated', { bagId: 3, members: [{ user_id: 1 }, { user_id: 2 }] }, 'sock');
});
it('coerces non-array members to an empty list (ternary else branch)', () => {
const setBagMembers = vi.fn().mockReturnValue([]);
const svc = makeService({ setBagMembers } as Partial<PackingService>);
new PackingController(svc).setBagMembers(user, '5', '3', 'not-an-array');
expect(setBagMembers).toHaveBeenCalledWith('5', '3', []);
});
// The non-array coercion is gone: packingBagMembersRequestSchema requires
// user_ids to be an array, so the pipe 400s a malformed body before the
// handler runs (covered by the schema spec in @trek/shared).
});
describe('templates', () => {
@@ -418,28 +411,24 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
expect(applyTemplate).toHaveBeenCalledWith('5', 't1', 'common', user.id);
});
it('400 when an admin saves a template with no name (whitespace)', () => {
it('400 when an admin saves a template with no name (whitespace — the schema cannot see it)', () => {
const saveAsTemplate = vi.fn();
const svc = makeService({ saveAsTemplate } as Partial<PackingService>);
expect(thrown(() => new PackingController(svc).saveAsTemplate(admin, '5', ' '))).toEqual({
expect(thrown(() => new PackingController(svc).saveAsTemplate(admin, '5', { name: ' ' }))).toEqual({
status: 400, body: { error: 'Template name is required' },
});
expect(saveAsTemplate).not.toHaveBeenCalled();
});
it('400 when an admin saves a template with no name at all (optional-chain)', () => {
const saveAsTemplate = vi.fn();
const svc = makeService({ saveAsTemplate } as Partial<PackingService>);
expect(thrown(() => new PackingController(svc).saveAsTemplate(admin, '5'))).toEqual({
status: 400, body: { error: 'Template name is required' },
});
expect(saveAsTemplate).not.toHaveBeenCalled();
});
// The missing-name 400 moved into the ZodValidationPipe
// (packingSaveTemplateRequestSchema requires a non-empty name) — direct
// method calls bypass parameter pipes, so that path is covered by the e2e
// suite and the schema spec in @trek/shared.
it('403 when a non-admin tries to save a template', () => {
const saveAsTemplate = vi.fn();
const svc = makeService({ saveAsTemplate } as Partial<PackingService>);
expect(thrown(() => new PackingController(svc).saveAsTemplate(user, '5', 'My template'))).toEqual({
expect(thrown(() => new PackingController(svc).saveAsTemplate(user, '5', { name: 'My template' }))).toEqual({
status: 403, body: { error: 'Admin access required' },
});
expect(saveAsTemplate).not.toHaveBeenCalled();
@@ -447,7 +436,7 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
it('400 when an admin saves a template with no items', () => {
const svc = makeService({ saveAsTemplate: vi.fn().mockReturnValue(null) } as Partial<PackingService>);
expect(thrown(() => new PackingController(svc).saveAsTemplate(admin, '5', 'My template'))).toEqual({
expect(thrown(() => new PackingController(svc).saveAsTemplate(admin, '5', { name: 'My template' }))).toEqual({
status: 400, body: { error: 'No items to save' },
});
});
@@ -455,7 +444,7 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
it('saves a template for an admin', () => {
const saveAsTemplate = vi.fn().mockReturnValue({ id: 7, name: 'My template' });
const svc = makeService({ saveAsTemplate } as Partial<PackingService>);
expect(new PackingController(svc).saveAsTemplate(admin, '5', 'My template')).toEqual({
expect(new PackingController(svc).saveAsTemplate(admin, '5', { name: 'My template' })).toEqual({
template: { id: 7, name: 'My template' },
});
expect(saveAsTemplate).toHaveBeenCalledWith('5', admin.id, 'My template');
@@ -477,7 +466,7 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
const broadcast = vi.fn();
const notifyTagged = vi.fn();
const svc = makeService({ updateCategoryAssignees, broadcast, notifyTagged } as Partial<PackingService>);
new PackingController(svc).updateCategoryAssignees(user, '5', 'Toys%20%26%20Games', [2]);
new PackingController(svc).updateCategoryAssignees(user, '5', 'Toys%20%26%20Games', { user_ids: [2] });
expect(updateCategoryAssignees).toHaveBeenCalledWith('5', 'Toys & Games', [2]);
});
@@ -486,7 +475,7 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
const broadcast = vi.fn();
const notifyTagged = vi.fn();
const svc = makeService({ updateCategoryAssignees, broadcast, notifyTagged } as Partial<PackingService>);
const res = new PackingController(svc).updateCategoryAssignees(user, '5', 'Clothes', [2], 'sock');
const res = new PackingController(svc).updateCategoryAssignees(user, '5', 'Clothes', { user_ids: [2] }, 'sock');
expect(res).toEqual({ assignees: [{ user_id: 2 }] });
expect(broadcast).toHaveBeenCalledWith('5', 'packing:assignees', { category: 'Clothes', assignees: [{ user_id: 2 }] }, 'sock');
expect(notifyTagged).toHaveBeenCalledWith('5', user, 'Clothes', [2]);