mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
0df82185bd
A saved place always had price, currency, website and phone columns. Saving from a trip, the file import and the copy into a trip all carried them, but neither editor showed them and the update contract did not know them, so the validation pipe stripped them from a PATCH and the MCP tool refused them as unknown keys (#2471). - shared: collectionPlaceUpdateRequestSchema takes price (not negative, nullable), currency (three letters, trimmed and upper-cased), website (http or https, as on save) and phone (trimmed, capped at 60 like the file import). The save schema is unchanged. - server: updatePlace writes the four fields inside the existing transaction. update_collection_place spreads the same shape, so REST and MCP take and refuse the same values; its description now names the price and asks for the currency with it. - client: the desktop detail sheet and the phone sheet edit price with a currency picker, website and phone, and show them in read mode (price only above zero, phone as a call link, website as a link chip through safeHttpUrl). Both shells share one hook, useCollectionPlaceExtras, which sends only the fields the user changed and always pairs a price with its currency, since a list has no base currency. An amount that does not parse marks the field and holds the save. - i18n: collections.price, collections.priceHint and collections.phone in all 23 locales; the website label reuses places.formWebsite. - wiki: Collections and MCP-Addon-Tools name the new fields. Tests: schema spec, COLLECTIONS-SVC-128 to 131, controller pipe parity, update_collection_place with the new fields, COLLECTIONS-E2E-085 to 087, FE-COMP-COLEXTRAS-001 to 012, FE-COMP-COLDETAIL-049 to 054 and FE-MOB-CPLSH-042 to 046.
317 lines
18 KiB
TypeScript
317 lines
18 KiB
TypeScript
import { describe, it, expect, vi, afterEach } from 'vitest';
|
|
import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service';
|
|
import { HttpException } from '@nestjs/common';
|
|
import { CollectionsController } from '../../../src/nest/collections/collections.controller';
|
|
import type { CollectionsService } from '../../../src/nest/collections/collections.service';
|
|
import type { StorageService } from '../../../src/nest/storage/storage.service';
|
|
import { CollectionGpxError } from '../../../src/nest/collections/collection-gpx.helpers';
|
|
import type { User } from '../../../src/types';
|
|
import { collectionPlaceUpdateRequestSchema, type CollectionGpxProblem } from '@trek/shared';
|
|
import { CollectionPlaceUpdateDto } from '../../../src/nest/collections/collections.dto';
|
|
import { ZodValidationPipe } from '../../../src/nest/common/zod-validation.pipe';
|
|
|
|
const storageStub = { put: vi.fn().mockResolvedValue(undefined) } as unknown as StorageService;
|
|
|
|
const user = { id: 1, username: 'owner', role: 'user', email: 'u@example.test' } as User;
|
|
|
|
/** Service mock — access granted + owner by default; each method a spy. */
|
|
function makeService(overrides: Partial<Record<keyof CollectionsService, unknown>> = {}): CollectionsService {
|
|
return {
|
|
listCollections: vi.fn().mockReturnValue({ collections: [], incomingInvites: [] }),
|
|
getCollection: vi.fn().mockReturnValue({ collection: { id: 3 }, places: [] }),
|
|
createCollection: vi.fn().mockReturnValue({ id: 7 }),
|
|
updateCollection: vi.fn().mockReturnValue({ id: 3 }),
|
|
setCollectionCover: vi.fn().mockReturnValue({ id: 3 }),
|
|
deleteCollection: vi.fn(),
|
|
reorderCollections: vi.fn(),
|
|
savePlace: vi.fn().mockReturnValue({ place: { id: 9 } }),
|
|
saveFromTripPlace: vi.fn().mockReturnValue({ place: { id: 9 } }),
|
|
updatePlace: vi.fn().mockReturnValue({ id: 9 }),
|
|
setStatus: vi.fn().mockReturnValue({ id: 9 }),
|
|
deletePlace: vi.fn(),
|
|
deletePlacesMany: vi.fn().mockReturnValue([1, 2]),
|
|
copyToTrip: vi.fn().mockReturnValue({ copied: 1, skipped: [] }),
|
|
findMembership: vi.fn().mockReturnValue({ inLists: [] }),
|
|
assertAccess: vi.fn(),
|
|
isOwner: vi.fn().mockReturnValue(true),
|
|
sendInvite: vi.fn().mockReturnValue({ success: true }),
|
|
acceptInvite: vi.fn().mockReturnValue({ success: true }),
|
|
declineInvite: vi.fn(),
|
|
cancelInvite: vi.fn(),
|
|
leaveCollection: vi.fn(),
|
|
removeMember: vi.fn(),
|
|
availableUsers: vi.fn().mockReturnValue([{ id: 2 }]),
|
|
createLabel: vi.fn().mockReturnValue({ id: 5, collection_id: 3, name: 'Berlin' }),
|
|
updateLabel: vi.fn().mockReturnValue({ id: 5, collection_id: 3, name: 'Hamburg' }),
|
|
deleteLabel: vi.fn(),
|
|
assignLabels: vi.fn().mockReturnValue({ changed: 2 }),
|
|
...overrides,
|
|
} as unknown as CollectionsService;
|
|
}
|
|
|
|
function thrown(fn: () => unknown): { status: number; body: unknown } {
|
|
try {
|
|
fn();
|
|
} catch (err) {
|
|
expect(err).toBeInstanceOf(HttpException);
|
|
const e = err as HttpException;
|
|
return { status: e.getStatus(), body: e.getResponse() };
|
|
}
|
|
throw new Error('expected the handler to throw');
|
|
}
|
|
|
|
async function thrownAsync(fn: () => Promise<unknown>): Promise<{ status: number; body: unknown }> {
|
|
try { await fn(); } catch (err) {
|
|
expect(err).toBeInstanceOf(HttpException);
|
|
const e = err as HttpException;
|
|
return { status: e.getStatus(), body: e.getResponse() };
|
|
}
|
|
throw new Error('expected the handler to throw');
|
|
}
|
|
|
|
describe('CollectionsController', () => {
|
|
afterEach(() => { delete process.env.DEMO_MODE; });
|
|
|
|
it('lists / creates / reads / deletes lists (pass-throughs)', () => {
|
|
const svc = makeService();
|
|
const c = new CollectionsController(svc, new RuntimeEnvService(), storageStub);
|
|
expect(c.list(user)).toEqual({ collections: [], incomingInvites: [] });
|
|
expect(c.create(user, { name: 'Italy' } as never)).toEqual({ id: 7 });
|
|
expect(c.get(user, '3')).toEqual({ collection: { id: 3 }, places: [] });
|
|
expect(c.update(user, '3', { name: 'x' } as never, 'sock')).toEqual({ id: 3 });
|
|
expect(svc.updateCollection).toHaveBeenCalledWith(1, 3, { name: 'x' }, 'sock');
|
|
expect(c.remove(user, '3')).toEqual({ success: true });
|
|
expect(svc.deleteCollection).toHaveBeenCalledWith(1, 3);
|
|
});
|
|
|
|
describe('reorder', () => {
|
|
// The legacy 'orderedIds must be an array of numbers' 400 is gone:
|
|
// collectionReorderRequestSchema types the array, so the pipe rejects a
|
|
// bad payload before the handler runs.
|
|
it('reorders a valid array', () => {
|
|
const svc = makeService();
|
|
expect(new CollectionsController(svc, new RuntimeEnvService(), storageStub).reorder(user, { orderedIds: [3, 1, 2] } as never)).toEqual({ success: true });
|
|
expect(svc.reorderCollections).toHaveBeenCalledWith(1, [3, 1, 2]);
|
|
});
|
|
});
|
|
|
|
describe('places', () => {
|
|
it('savePlace / updatePlace / setStatus / deletePlace forward the socket id', async () => {
|
|
const svc = makeService();
|
|
const c = new CollectionsController(svc, new RuntimeEnvService(), storageStub);
|
|
c.savePlace(user, { collection_id: 3, name: 'A' } as never, 'sid');
|
|
expect(svc.savePlace).toHaveBeenCalledWith(1, { collection_id: 3, name: 'A' }, 'sid');
|
|
c.updatePlace(user, '9', { name: 'B' } as never, 'sid');
|
|
expect(svc.updatePlace).toHaveBeenCalledWith(1, 9, { name: 'B' }, 'sid');
|
|
c.setStatus(user, '9', { status: 'want' } as never, 'sid');
|
|
expect(svc.setStatus).toHaveBeenCalledWith(1, 9, 'want', 'sid');
|
|
expect(await c.deletePlace(user, '9', 'sid')).toEqual({ success: true });
|
|
expect(svc.deletePlace).toHaveBeenCalledWith(1, 9, 'sid');
|
|
c.saveFromTrip(user, { collection_id: 3, source_trip_id: 5, source_place_id: 8 } as never, 'sid');
|
|
expect(svc.saveFromTripPlace).toHaveBeenCalledWith(1, 3, 5, 8, undefined, 'sid');
|
|
expect(c.copyToTrip(user, { trip_id: 5, place_ids: [9] } as never)).toEqual({ copied: 1, skipped: [] });
|
|
});
|
|
|
|
// #2471: the PATCH body stripped price, currency, website and phone before the
|
|
// handler ever saw them. The DTO is the same shape update_collection_place
|
|
// spreads, so REST and MCP take the same four fields and refuse the same values.
|
|
it('updatePlace keeps price, currency, website and phone through the body pipe and forwards them', () => {
|
|
const pipe = new ZodValidationPipe();
|
|
const meta = { type: 'body' as const, metatype: CollectionPlaceUpdateDto };
|
|
const body = pipe.transform({ price: 12.5, currency: ' chf', website: 'https://kunsthaus.example', phone: '+41 44' }, meta);
|
|
expect(body).toEqual({ price: 12.5, currency: 'CHF', website: 'https://kunsthaus.example', phone: '+41 44' });
|
|
|
|
const svc = makeService();
|
|
new CollectionsController(svc, new RuntimeEnvService(), storageStub).updatePlace(user, '9', body as never, 'sid');
|
|
expect(svc.updatePlace).toHaveBeenCalledWith(1, 9, { price: 12.5, currency: 'CHF', website: 'https://kunsthaus.example', phone: '+41 44' }, 'sid');
|
|
|
|
expect(thrown(() => pipe.transform({ price: -1 }, meta)).status).toBe(400);
|
|
expect(thrown(() => pipe.transform({ currency: 'EURO' }, meta)).status).toBe(400);
|
|
expect(Object.keys(collectionPlaceUpdateRequestSchema.shape)).toEqual(
|
|
expect.arrayContaining(['price', 'currency', 'website', 'phone']),
|
|
);
|
|
expect(CollectionPlaceUpdateDto.schema).toBe(collectionPlaceUpdateRequestSchema);
|
|
});
|
|
|
|
// The legacy 'ids must be an array of numbers' 400 is gone:
|
|
// collectionDeleteManyRequestSchema types the array, so the pipe rejects a
|
|
// bad payload before the handler runs.
|
|
it('deleteMany deletes a valid list', async () => {
|
|
const svc = makeService();
|
|
expect(await new CollectionsController(svc, new RuntimeEnvService(), storageStub).deleteMany(user, { ids: [1, 2] } as never, 'sid')).toEqual({ deleted: [1, 2] });
|
|
expect(svc.deletePlacesMany).toHaveBeenCalledWith(1, [1, 2], 'sid');
|
|
});
|
|
});
|
|
|
|
describe('membership lookup', () => {
|
|
it('parses lat/lng when present', () => {
|
|
const svc = makeService();
|
|
new CollectionsController(svc, new RuntimeEnvService(), storageStub).membership(user, 'gpid', undefined, 'Rome', '41.9', '12.5');
|
|
expect(svc.findMembership).toHaveBeenCalledWith(1, { google_place_id: 'gpid', google_ftid: undefined, name: 'Rome', lat: 41.9, lng: 12.5 });
|
|
});
|
|
it('leaves lat/lng undefined when absent', () => {
|
|
const svc = makeService();
|
|
new CollectionsController(svc, new RuntimeEnvService(), storageStub).membership(user);
|
|
expect(svc.findMembership).toHaveBeenCalledWith(1, { google_place_id: undefined, google_ftid: undefined, name: undefined, lat: undefined, lng: undefined });
|
|
});
|
|
});
|
|
|
|
describe('invites (owner-gated)', () => {
|
|
it('403 when a non-owner invites', () => {
|
|
const svc = makeService({ isOwner: vi.fn().mockReturnValue(false) });
|
|
expect(thrown(() => new CollectionsController(svc, new RuntimeEnvService(), storageStub).invite(user, { collection_id: 3, user_id: 2 } as never)))
|
|
.toEqual({ status: 403, body: { error: 'Only the owner can invite' } });
|
|
});
|
|
it('surfaces a sendInvite error with its status', () => {
|
|
const svc = makeService({ sendInvite: vi.fn().mockReturnValue({ error: 'Already a member', status: 409 }) });
|
|
expect(thrown(() => new CollectionsController(svc, new RuntimeEnvService(), storageStub).invite(user, { collection_id: 3, user_id: 2 } as never)))
|
|
.toEqual({ status: 409, body: { error: 'Already a member' } });
|
|
});
|
|
it('invites successfully as owner', () => {
|
|
const svc = makeService();
|
|
expect(new CollectionsController(svc, new RuntimeEnvService(), storageStub).invite(user, { collection_id: 3, user_id: 2 } as never)).toEqual({ success: true });
|
|
});
|
|
it('accept surfaces an error, else succeeds', () => {
|
|
const bad = makeService({ acceptInvite: vi.fn().mockReturnValue({ error: 'Gone', status: 404 }) });
|
|
expect(thrown(() => new CollectionsController(bad, new RuntimeEnvService(), storageStub).acceptInvite(user, { collection_id: 3 } as never, 'sid')))
|
|
.toEqual({ status: 404, body: { error: 'Gone' } });
|
|
const svc = makeService();
|
|
expect(new CollectionsController(svc, new RuntimeEnvService(), storageStub).acceptInvite(user, { collection_id: 3 } as never, 'sid')).toEqual({ success: true });
|
|
expect(svc.acceptInvite).toHaveBeenCalledWith(1, 3, 'sid');
|
|
});
|
|
it('decline + leave forward the socket id', () => {
|
|
const svc = makeService();
|
|
const c = new CollectionsController(svc, new RuntimeEnvService(), storageStub);
|
|
expect(c.declineInvite(user, { collection_id: 3 } as never, 'sid')).toEqual({ success: true });
|
|
expect(svc.declineInvite).toHaveBeenCalledWith(1, 3, 'sid');
|
|
expect(c.leave(user, { collection_id: 3 } as never, 'sid')).toEqual({ success: true });
|
|
expect(svc.leaveCollection).toHaveBeenCalledWith(1, 3, 'sid');
|
|
});
|
|
it('cancel is owner-gated', () => {
|
|
const notOwner = makeService({ isOwner: vi.fn().mockReturnValue(false) });
|
|
expect(thrown(() => new CollectionsController(notOwner, new RuntimeEnvService(), storageStub).cancelInvite(user, { collection_id: 3, user_id: 2 } as never)))
|
|
.toEqual({ status: 403, body: { error: 'Only the owner can cancel invites' } });
|
|
const svc = makeService();
|
|
expect(new CollectionsController(svc, new RuntimeEnvService(), storageStub).cancelInvite(user, { collection_id: 3, user_id: 2 } as never)).toEqual({ success: true });
|
|
});
|
|
});
|
|
|
|
describe('members', () => {
|
|
it('removeMember is owner-gated', () => {
|
|
const notOwner = makeService({ isOwner: vi.fn().mockReturnValue(false) });
|
|
expect(thrown(() => new CollectionsController(notOwner, new RuntimeEnvService(), storageStub).removeMember(user, { collection_id: 3, user_id: 2 } as never)))
|
|
.toEqual({ status: 403, body: { error: 'Only the owner can remove members' } });
|
|
const svc = makeService();
|
|
expect(new CollectionsController(svc, new RuntimeEnvService(), storageStub).removeMember(user, { collection_id: 3, user_id: 2 } as never)).toEqual({ success: true });
|
|
expect(svc.removeMember).toHaveBeenCalledWith(1, 3, 2);
|
|
});
|
|
it('availableUsers is owner-gated', () => {
|
|
const notOwner = makeService({ isOwner: vi.fn().mockReturnValue(false) });
|
|
expect(thrown(() => new CollectionsController(notOwner, new RuntimeEnvService(), storageStub).availableUsers(user, '3')))
|
|
.toEqual({ status: 403, body: { error: 'Only the owner can manage members' } });
|
|
expect(new CollectionsController(makeService(), new RuntimeEnvService(), storageStub).availableUsers(user, '3')).toEqual({ users: [{ id: 2 }] });
|
|
});
|
|
});
|
|
|
|
describe('uploadCover', () => {
|
|
const file = { filename: 'x.jpg' } as Express.Multer.File;
|
|
it('403 in demo mode for a demo user', async () => {
|
|
process.env.DEMO_MODE = 'true';
|
|
const demo = { ...user, email: 'demo@trek.app' } as User;
|
|
expect(await thrownAsync(() => new CollectionsController(makeService(), new RuntimeEnvService(), storageStub).uploadCover(demo, '3', file)))
|
|
.toEqual({ status: 403, body: { error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' } });
|
|
});
|
|
it('400 when no file was uploaded', async () => {
|
|
expect(await thrownAsync(() => new CollectionsController(makeService(), new RuntimeEnvService(), storageStub).uploadCover(user, '3', undefined)))
|
|
.toEqual({ status: 400, body: { error: 'No image uploaded' } });
|
|
});
|
|
it('commits + stores the cover and forwards the socket id', async () => {
|
|
const svc = makeService();
|
|
expect(await new CollectionsController(svc, new RuntimeEnvService(), storageStub).uploadCover(user, '3', file, 'sid')).toEqual({ id: 3 });
|
|
expect(storageStub.put).toHaveBeenCalledWith('covers', 'x.jpg', { tmpPath: undefined });
|
|
expect(svc.setCollectionCover).toHaveBeenCalledWith(1, 3, '/uploads/covers/x.jpg', 'sid');
|
|
});
|
|
});
|
|
|
|
describe('uploadPlaceImage', () => {
|
|
const file = { filename: 'x.jpg' } as Express.Multer.File;
|
|
it('403 in demo mode for a demo user', async () => {
|
|
process.env.DEMO_MODE = 'true';
|
|
const demo = { ...user, email: 'demo@trek.app' } as User;
|
|
expect(await thrownAsync(() => new CollectionsController(makeService(), new RuntimeEnvService(), storageStub).uploadPlaceImage(demo, '9', file)))
|
|
.toEqual({ status: 403, body: { error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' } });
|
|
});
|
|
it('400 when no file was uploaded', async () => {
|
|
expect(await thrownAsync(() => new CollectionsController(makeService(), new RuntimeEnvService(), storageStub).uploadPlaceImage(user, '9', undefined)))
|
|
.toEqual({ status: 400, body: { error: 'No image uploaded' } });
|
|
});
|
|
it('commits + stores the place image via /uploads/places and forwards the socket id', async () => {
|
|
const svc = makeService({ setPlaceImage: vi.fn().mockReturnValue({ id: 9 }) });
|
|
expect(await new CollectionsController(svc, new RuntimeEnvService(), storageStub).uploadPlaceImage(user, '9', file, 'sid')).toEqual({ id: 9 });
|
|
expect(storageStub.put).toHaveBeenCalledWith('places', 'x.jpg', { tmpPath: undefined });
|
|
expect(svc.setPlaceImage).toHaveBeenCalledWith(1, 9, '/uploads/places/x.jpg', 'sid');
|
|
});
|
|
});
|
|
|
|
describe('labels', () => {
|
|
it('create / update / delete / assign / unassign forward user + socket id', () => {
|
|
const svc = makeService();
|
|
const c = new CollectionsController(svc, new RuntimeEnvService(), storageStub);
|
|
|
|
c.createLabel(user, { collection_id: 3, name: 'Berlin', color: '#ff0000' } as never, 'sock');
|
|
expect(svc.createLabel).toHaveBeenCalledWith(1, 3, 'Berlin', '#ff0000', 'sock');
|
|
|
|
c.updateLabel(user, '5', { name: 'Hamburg' } as never, 'sock');
|
|
expect(svc.updateLabel).toHaveBeenCalledWith(1, 5, { name: 'Hamburg' }, 'sock');
|
|
|
|
expect(c.deleteLabel(user, '5', 'sock')).toEqual({ success: true });
|
|
expect(svc.deleteLabel).toHaveBeenCalledWith(1, 5, 'sock');
|
|
|
|
c.assignLabels(user, { label_ids: [5], place_ids: [9, 10] } as never, 'sock');
|
|
expect(svc.assignLabels).toHaveBeenCalledWith(1, [5], [9, 10], false, 'sock');
|
|
|
|
c.unassignLabels(user, { label_ids: [5], place_ids: [9] } as never, 'sock');
|
|
expect(svc.assignLabels).toHaveBeenCalledWith(1, [5], [9], true, 'sock');
|
|
});
|
|
});
|
|
|
|
describe('GPX (#2301)', () => {
|
|
it('exports the list as GPX for the caller, with the id as a number', () => {
|
|
const exported = { name: 'Lisbon', gpx: '<gpx/>', waypoints: 0, omitted: 0 };
|
|
const svc = makeService({ exportCollectionGpx: vi.fn().mockReturnValue(exported) });
|
|
const c = new CollectionsController(svc, new RuntimeEnvService(), storageStub);
|
|
|
|
expect(c.exportCollectionGpx(user, '3')).toBe(exported);
|
|
expect(svc.exportCollectionGpx).toHaveBeenCalledWith(1, 3);
|
|
});
|
|
|
|
it('hands back the file a GPX was read into', () => {
|
|
const read = { file: { name: 'Lisbon' }, skipped: 1, track_points: 4 };
|
|
const svc = makeService({ readCollectionGpx: vi.fn().mockReturnValue(read) });
|
|
const c = new CollectionsController(svc, new RuntimeEnvService(), storageStub);
|
|
|
|
expect(c.readGpx({ gpx: '<gpx/>', file_name: 'lisbon.gpx' })).toBe(read);
|
|
expect(svc.readCollectionGpx).toHaveBeenCalledWith({ gpx: '<gpx/>', file_name: 'lisbon.gpx' });
|
|
});
|
|
|
|
it('turns a refusal into a 400 with a code the client can translate, 413 for size', () => {
|
|
const refuse = (code: CollectionGpxProblem) => makeService({
|
|
readCollectionGpx: vi.fn(() => { throw new CollectionGpxError(code, 'refused'); }),
|
|
});
|
|
|
|
for (const code of ['unreadable', 'not-gpx', 'too-many-places'] as const) {
|
|
const c = new CollectionsController(refuse(code), new RuntimeEnvService(), storageStub);
|
|
expect(thrown(() => c.readGpx({ gpx: 'x' }))).toEqual({ status: 400, body: { error: 'refused', code } });
|
|
}
|
|
const big = new CollectionsController(refuse('too-large'), new RuntimeEnvService(), storageStub);
|
|
expect(thrown(() => big.readGpx({ gpx: 'x' }))).toEqual({ status: 413, body: { error: 'refused', code: 'too-large' } });
|
|
});
|
|
|
|
it('lets any other failure through untouched', () => {
|
|
const svc = makeService({ readCollectionGpx: vi.fn(() => { throw new Error('disk on fire'); }) });
|
|
const c = new CollectionsController(svc, new RuntimeEnvService(), storageStub);
|
|
expect(() => c.readGpx({ gpx: 'x' })).toThrow('disk on fire');
|
|
});
|
|
});
|
|
});
|