From c353302e3f61da7140bd2986ac980f06d984a331 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 10 Oct 2026 16:32:26 +0200 Subject: [PATCH] refactor(server): packing templates, road trip, day and booking-import services refuse with DomainError --- .../booking-import/booking-import.service.ts | 5 +- server/src/nest/days/days.service.ts | 9 +- .../admin-packing-templates.controller.ts | 42 ++----- server/src/nest/packing/packing.mcp.ts | 13 +- server/src/nest/packing/packing.service.ts | 25 ++-- server/src/nest/roadtrip/charging.service.ts | 5 +- .../nest/roadtrip/day-boundaries.service.ts | 5 +- .../src/nest/roadtrip/google-route.service.ts | 17 +-- .../nest/roadtrip/roadtrip-plan.service.ts | 9 +- .../roadtrip/roadtrip-preferences.service.ts | 5 +- server/src/nest/roadtrip/roadtrip.service.ts | 5 +- ...admin-packing-templates.controller.test.ts | 21 +--- server/tests/unit/nest/google-route.test.ts | 2 +- .../tests/unit/nest/packing.service.test.ts | 111 +++++++++--------- 14 files changed, 127 insertions(+), 147 deletions(-) diff --git a/server/src/nest/booking-import/booking-import.service.ts b/server/src/nest/booking-import/booking-import.service.ts index 9856e4f5d..670cdcc4e 100644 --- a/server/src/nest/booking-import/booking-import.service.ts +++ b/server/src/nest/booking-import/booking-import.service.ts @@ -1,4 +1,5 @@ -import { Injectable, HttpException } from '@nestjs/common'; +import { DomainError } from '../common/domain-error'; +import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import { RealtimeService } from '../realtime/realtime.service'; import { PermissionsService } from '../permissions/permissions.service'; @@ -166,7 +167,7 @@ export class BookingImportService { const kitineraryAvailable = this.extractor.isAvailable(); const aiAvailable = await this.llmParse.isAvailable(userId); if (!kitineraryAvailable && !aiAvailable) { - throw new HttpException({ error: 'KItinerary extractor is not available on this server' }, 503); + throw new DomainError(503, 'KItinerary extractor is not available on this server'); } const allItems: ParsedBookingItem[] = []; diff --git a/server/src/nest/days/days.service.ts b/server/src/nest/days/days.service.ts index 3a4b4769c..e12d1ba8c 100644 --- a/server/src/nest/days/days.service.ts +++ b/server/src/nest/days/days.service.ts @@ -1,4 +1,5 @@ -import { HttpException, Injectable } from '@nestjs/common'; +import { DomainError } from '../common/domain-error'; +import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import { addIsoDays, MAX_TRIP_DAYS, planDatedAppend } from '@trek/shared'; import type { RoadtripDayBoundary, TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; @@ -303,7 +304,7 @@ export class DaysService { // (it stored those through affinity and answered 201) — the same class // of defect `PlacesService.create` was fixed for (A-M1). const tripIdNum = toRowId(tripId); - if (tripIdNum === null) throw new HttpException({ error: 'Trip not found' }, 404); + if (tripIdNum === null) throw new DomainError(404, 'Trip not found'); // DY5: the next day number and the row that takes it in one transaction, so // two days added at once cannot both read the same MAX. `|| 0` keeps a stored // 0 becoming 1, as the legacy formula did. @@ -606,7 +607,7 @@ export class DaysService { // through affinity and answered 201 for — rather than the renumber- // then-skip-the-invariant split H2 found for hex ids. const tripIdNum = toRowId(tripId); - if (tripIdNum === null) throw new HttpException({ error: 'Trip not found' }, 404); + if (tripIdNum === null) throw new DomainError(404, 'Trip not found'); const rows: DayOrderRow[] = await this.daysRepo.listOrderedForReorder(tripIdNum); const n = rows.length; const pos = Math.min(Math.max(position ?? n + 1, 1), n + 1); @@ -673,7 +674,7 @@ export class DaysService { async appendDated(tripId: string | number, viewerId: number, notes?: string): Promise { // The same parse-once gate `create`/`insert` use (class docstring). const trip = toRowId(tripId); - if (trip === null) throw new HttpException({ error: 'Trip not found' }, 404); + if (trip === null) throw new DomainError(404, 'Trip not found'); const appended = await this.uow.transactional(async () => { const range = await this.tripsRepo.findDatesById(trip); const rows: DayOrderRow[] = await this.daysRepo.listOrderedForReorder(trip); diff --git a/server/src/nest/packing/admin-packing-templates.controller.ts b/server/src/nest/packing/admin-packing-templates.controller.ts index eb7f6e389..e294db0c3 100644 --- a/server/src/nest/packing/admin-packing-templates.controller.ts +++ b/server/src/nest/packing/admin-packing-templates.controller.ts @@ -1,4 +1,4 @@ -import { Body, Controller, Delete, Get, HttpCode, HttpException, Param, Post, Put, Req, UseGuards } from '@nestjs/common'; +import { Body, Controller, Delete, Get, HttpCode, Param, Post, Put, Req, UseGuards } from '@nestjs/common'; import type { Request } from 'express'; import { PackingService } from './packing.service'; import { AdminTemplateNameDto } from './packing.dto'; @@ -10,26 +10,6 @@ import { AuditService } from '../audit/audit.service'; import type { User } from '../../types'; /** Throw the legacy {error,status} envelope when a service call reports failure. */ -function ok(result: T): Exclude { - if (result && typeof result === 'object' && 'error' in (result as Record)) { - const r = result as unknown as { error: string; status?: number }; - throw new HttpException({ error: r.error }, r.status ?? 400); - } - return result as Exclude; -} - -/** - * /api/admin/packing-templates — the admin CRUD for packing templates. - * - * These twelve routes sat on AdminController and reached PackingService through - * eleven pass-through methods on AdminService that did nothing but forward and cast. - * PackingService already owns all three template tables, so the routes belong here: - * the pass-through block is gone, and there is one owner instead of a controller in - * one domain calling a service in another through a third. - * - * The path, the admin gate, the {error,status} envelope, the create-201-vs-rest-200 - * split and the audit-log writes are unchanged — this is a move, not a redesign. - */ @Controller('api/admin/packing-templates') @UseGuards(JwtAuthGuard, AdminGuard) export class AdminPackingTemplatesController { @@ -45,13 +25,13 @@ export class AdminPackingTemplatesController { @Get(':id') async get(@Param('id') id: string) { - return ok(await this.packing.getPackingTemplate(id)); + return await this.packing.getPackingTemplate(id); } @Post() @HttpCode(201) async create(@CurrentUser() user: User, @Body() body: AdminTemplateNameDto, @Req() req: Request) { - const result = ok(await this.packing.createPackingTemplate(body.name, user.id)); + const result = await this.packing.createPackingTemplate(body.name, user.id); await this.audit.writeAudit({ userId: user.id, action: 'admin.packing_template_create', @@ -64,12 +44,12 @@ export class AdminPackingTemplatesController { @Put(':id') async update(@Param('id') id: string, @Body() body: AdminTemplateNameDto) { - return ok(await this.packing.updatePackingTemplate(id, body)); + return await this.packing.updatePackingTemplate(id, body); } @Delete(':id') async remove(@CurrentUser() user: User, @Param('id') id: string, @Req() req: Request) { - const result = ok(await this.packing.deletePackingTemplate(id)); + const result = await this.packing.deletePackingTemplate(id); await this.audit.writeAudit({ userId: user.id, action: 'admin.packing_template_delete', @@ -83,34 +63,34 @@ export class AdminPackingTemplatesController { @Post(':id/categories') @HttpCode(201) async createCategory(@Param('id') id: string, @Body() body: AdminTemplateNameDto) { - return ok(await this.packing.createTemplateCategory(id, body.name)); + return await this.packing.createTemplateCategory(id, body.name); } @Put(':templateId/categories/:catId') async updateCategory(@Param('templateId') templateId: string, @Param('catId') catId: string, @Body() body: AdminTemplateNameDto) { - return ok(await this.packing.updateTemplateCategory(templateId, catId, body)); + return await this.packing.updateTemplateCategory(templateId, catId, body); } @Delete(':templateId/categories/:catId') async deleteCategory(@Param('templateId') templateId: string, @Param('catId') catId: string) { - ok(await this.packing.deleteTemplateCategory(templateId, catId)); + await this.packing.deleteTemplateCategory(templateId, catId); return { success: true }; } @Post(':templateId/categories/:catId/items') @HttpCode(201) async createItem(@Param('templateId') templateId: string, @Param('catId') catId: string, @Body() body: AdminTemplateNameDto) { - return ok(await this.packing.createTemplateItem(templateId, catId, body.name)); + return await this.packing.createTemplateItem(templateId, catId, body.name); } @Put(':templateId/items/:itemId') async updateItem(@Param('templateId') templateId: string, @Param('itemId') itemId: string, @Body() body: AdminTemplateNameDto) { - return ok(await this.packing.updateTemplateItem(templateId, itemId, body)); + return await this.packing.updateTemplateItem(templateId, itemId, body); } @Delete(':templateId/items/:itemId') async deleteItem(@Param('templateId') templateId: string, @Param('itemId') itemId: string) { - ok(await this.packing.deleteTemplateItem(templateId, itemId)); + await this.packing.deleteTemplateItem(templateId, itemId); return { success: true }; } } diff --git a/server/src/nest/packing/packing.mcp.ts b/server/src/nest/packing/packing.mcp.ts index e2f70d440..0dda8157f 100644 --- a/server/src/nest/packing/packing.mcp.ts +++ b/server/src/nest/packing/packing.mcp.ts @@ -62,7 +62,7 @@ export class PackingMcp { description: 'Add an item to the packing checklist for a trip. It lands on the common list everyone shares unless visibility says otherwise; use set_packing_item_sharing to move an existing item between those tiers.', inputSchema: { tripId: idSchema, - name: z.string().min(1).max(200), + name: packingCreateItemRequestSchema.shape.name.max(200), category: z.string().max(100).optional().describe('Packing category (e.g. Clothes, Electronics)'), bag_id: packingCreateItemRequestSchema.shape.bag_id.describe('Bag to pack the item into (ids come from list_packing_bags)'), quantity: packingCreateItemRequestSchema.shape.quantity.describe('How many to pack, clamped to 1-999'), @@ -138,7 +138,7 @@ export class PackingMcp { inputSchema: { tripId: idSchema, itemId: idSchema, - name: z.string().min(1).max(200).optional(), + name: packingCreateItemRequestSchema.shape.name.max(200).optional(), category: z.string().max(100).optional(), bag_id: packingUpdateItemRequestSchema.shape.bag_id.describe('Bag to pack the item into (ids come from list_packing_bags); null takes it out of its bag'), quantity: packingUpdateItemRequestSchema.shape.quantity.describe('How many to pack, clamped to 1-999'), @@ -234,8 +234,8 @@ export class PackingMcp { description: 'Create a new packing bag (e.g. "Carry-on", "Checked bag").', inputSchema: { tripId: idSchema, - name: z.string().min(1).max(100), - color: z.string().optional(), + name: packingCreateBagRequestSchema.shape.name.max(100), + color: packingCreateBagRequestSchema.shape.color, weight_limit_grams: packingCreateBagRequestSchema.shape.weight_limit_grams.describe('Allowance in grams the bag is measured against (the fill bar)'), }, annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, @@ -258,8 +258,8 @@ export class PackingMcp { inputSchema: { tripId: idSchema, bagId: idSchema, - name: z.string().optional(), - color: z.string().optional(), + name: packingUpdateBagRequestSchema.shape.name, + color: packingUpdateBagRequestSchema.shape.color, weight_limit_grams: packingUpdateBagRequestSchema.shape.weight_limit_grams.describe('Allowance in grams the bag is measured against (the fill bar); null lifts the limit'), user_id: packingUpdateBagRequestSchema.shape.user_id.describe('Trip member the bag belongs to; null leaves it unassigned, and an id outside the trip roster unassigns it too'), }, @@ -441,7 +441,6 @@ export class PackingMcp { // Templates are global; the REST route restricts management to admins. Match it. if (!(await this.guards.isAdminUser(ctx.userId))) return adminRequired(); const result = await this.packing.deletePackingTemplate(String(templateId)); - if ('error' in result) return errorResult(result.error); return ok({ success: true, name: result.name }); } diff --git a/server/src/nest/packing/packing.service.ts b/server/src/nest/packing/packing.service.ts index 8b62d047a..e82f41694 100644 --- a/server/src/nest/packing/packing.service.ts +++ b/server/src/nest/packing/packing.service.ts @@ -1,3 +1,4 @@ +import { DomainError } from '../common/domain-error'; import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import { resolvePackedState, type TrekWsPayload, type TrekWsTripEventName } from '@trek/shared'; @@ -806,14 +807,14 @@ export class PackingService { async getPackingTemplate(id: string) { const template = await this.templatesRepo.findById(id); - if (!template) return { error: 'Template not found', status: 404 }; + if (!template) throw new DomainError(404, 'Template not found'); const categories = await this.templateCategoriesRepo.listForTemplate(id); const items = await this.templateItemsRepo.listForTemplate(id); return { template, categories, items }; } async createPackingTemplate(name: string, createdBy: number) { - if (!name?.trim()) return { error: 'Name is required', status: 400 }; + if (!name?.trim()) throw new DomainError(400, 'Name is required'); const newId = await this.templatesRepo.insertTemplate(name.trim(), createdBy); const template = await this.templatesRepo.findById(newId); return { template }; @@ -821,14 +822,14 @@ export class PackingService { async updatePackingTemplate(id: string, data: { name?: string }) { const template = await this.templatesRepo.findById(id); - if (!template) return { error: 'Template not found', status: 404 }; + if (!template) throw new DomainError(404, 'Template not found'); if (data.name?.trim()) await this.templatesRepo.updateName(id, data.name.trim()); return { template: await this.templatesRepo.findById(id) }; } async deletePackingTemplate(id: string) { const template = await this.templatesRepo.findById(id); - if (!template) return { error: 'Template not found', status: 404 }; + if (!template) throw new DomainError(404, 'Template not found'); await this.templatesRepo.delete(id); return { name: template.name }; } @@ -836,9 +837,9 @@ export class PackingService { // Template categories async createTemplateCategory(templateId: string, name: string) { - if (!name?.trim()) return { error: 'Category name is required', status: 400 }; + if (!name?.trim()) throw new DomainError(400, 'Category name is required'); const template = await this.templatesRepo.findById(templateId); - if (!template) return { error: 'Template not found', status: 404 }; + if (!template) throw new DomainError(404, 'Template not found'); const newId = await this.uow.transactional(async () => { const maxOrder = await this.templateCategoriesRepo.maxSortOrder(templateId); return await this.templateCategoriesRepo.insertCategory(templateId, name.trim(), (maxOrder ?? -1) + 1); @@ -848,7 +849,7 @@ export class PackingService { async updateTemplateCategory(templateId: string, catId: string, data: { name?: string }) { const cat = await this.templateCategoriesRepo.findInTemplate(catId, templateId); - if (!cat) return { error: 'Category not found', status: 404 }; + if (!cat) throw new DomainError(404, 'Category not found'); if (data.name?.trim()) await this.templateCategoriesRepo.updateName(catId, data.name.trim()); return { category: await this.templateCategoriesRepo.findById(catId) }; @@ -856,7 +857,7 @@ export class PackingService { async deleteTemplateCategory(templateId: string, catId: string) { const cat = await this.templateCategoriesRepo.findInTemplate(catId, templateId); - if (!cat) return { error: 'Category not found', status: 404 }; + if (!cat) throw new DomainError(404, 'Category not found'); await this.templateCategoriesRepo.delete(catId); return {}; } @@ -864,9 +865,9 @@ export class PackingService { // Template items async createTemplateItem(templateId: string, catId: string, name: string) { - if (!name?.trim()) return { error: 'Item name is required', status: 400 }; + if (!name?.trim()) throw new DomainError(400, 'Item name is required'); const cat = await this.templateCategoriesRepo.findInTemplate(catId, templateId); - if (!cat) return { error: 'Category not found', status: 404 }; + if (!cat) throw new DomainError(404, 'Category not found'); const newId = await this.uow.transactional(async () => { const maxOrder = await this.templateItemsRepo.maxSortOrder(catId); return await this.templateItemsRepo.insertTemplateItem({ category_id: catId, name: name.trim(), sort_order: (maxOrder ?? -1) + 1 }); @@ -876,7 +877,7 @@ export class PackingService { async updateTemplateItem(templateId: string, itemId: string, data: { name?: string }) { const item = await this.templateItemsRepo.findScoped(itemId, templateId); - if (!item) return { error: 'Item not found', status: 404 }; + if (!item) throw new DomainError(404, 'Item not found'); if (data.name?.trim()) await this.templateItemsRepo.updateName(itemId, data.name.trim()); return { item: await this.templateItemsRepo.findById(itemId) }; @@ -884,7 +885,7 @@ export class PackingService { async deleteTemplateItem(templateId: string, itemId: string) { const item = await this.templateItemsRepo.findScoped(itemId, templateId); - if (!item) return { error: 'Item not found', status: 404 }; + if (!item) throw new DomainError(404, 'Item not found'); await this.templateItemsRepo.delete(itemId); return {}; } diff --git a/server/src/nest/roadtrip/charging.service.ts b/server/src/nest/roadtrip/charging.service.ts index 1c19974f9..bc3d1fe54 100644 --- a/server/src/nest/roadtrip/charging.service.ts +++ b/server/src/nest/roadtrip/charging.service.ts @@ -1,4 +1,5 @@ -import { HttpException, Injectable } from '@nestjs/common'; +import { DomainError } from '../common/domain-error'; +import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import type { ChargingInfo } from '@trek/shared'; import { z } from 'zod'; @@ -24,7 +25,7 @@ export class ChargingService { /** CH1 — `PlacesRepository.findChargingProbe` (trip-scoped). */ async read(tripId: number, placeId: number) { const place = await this.placesRepo.findChargingProbe(placeId, tripId); - if (!place) throw new HttpException({ error: 'Place not found' }, 404); + if (!place) throw new DomainError(404, 'Place not found'); if (place.stop_type !== 'charging' || place.lat == null || place.lng == null) return empty('unknown'); return this.lookup(place.lat, place.lng, place.name); } diff --git a/server/src/nest/roadtrip/day-boundaries.service.ts b/server/src/nest/roadtrip/day-boundaries.service.ts index da789dc2d..f9c71662e 100644 --- a/server/src/nest/roadtrip/day-boundaries.service.ts +++ b/server/src/nest/roadtrip/day-boundaries.service.ts @@ -1,5 +1,6 @@ +import { DomainError } from '../common/domain-error'; import { InjectRepository } from '@mikro-orm/nestjs'; -import { HttpException, Injectable } from '@nestjs/common'; +import { Injectable } from '@nestjs/common'; import type { RoadtripDayBoundary } from '@trek/shared'; import { DayAssignments } from '../../db/entities/DayAssignments.entity'; import type { DayAssignmentsRepository } from '../../db/repositories/DayAssignments.repository'; @@ -30,7 +31,7 @@ export class DayBoundariesService { // `DayAssignmentsRepository.findInTrip` (AS12) is the trip-scoped read. const belongs = async (id: number) => !!(await this.dayAssignmentsRepo.findInTrip(id, tripIdNum)); if (!(await belongs(boundary.from_assignment_id)) || (boundary.to_assignment_id !== null && !(await belongs(boundary.to_assignment_id)))) { - throw new HttpException({ error: 'Stop not found' }, 404); + throw new DomainError(404, 'Stop not found'); } // RB2 → RB3, un-transacted (R7 — pin, don't fix): the ownership check above and // the upsert below are two statements, not one. diff --git a/server/src/nest/roadtrip/google-route.service.ts b/server/src/nest/roadtrip/google-route.service.ts index ff17cf6e5..a13ab6087 100644 --- a/server/src/nest/roadtrip/google-route.service.ts +++ b/server/src/nest/roadtrip/google-route.service.ts @@ -1,4 +1,5 @@ -import { HttpException, Injectable } from '@nestjs/common'; +import { DomainError } from '../common/domain-error'; +import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import type { GoogleRouteImport, GoogleRoutePreview } from '@trek/shared'; import { MapsService, GOOGLE_SHORT_HOSTS, isGoogleMapsHost } from '../maps/maps.service'; @@ -25,18 +26,18 @@ export class GoogleRouteService { let url = new URL(raw); if (url.protocol !== 'https:' || url.username || url.password || url.port || !isGoogleMapsHost(url.hostname) && !GOOGLE_SHORT_HOSTS.includes(url.hostname)) - throw new HttpException({ error: 'Use a Google Maps directions link.' }, 400); + throw new DomainError(400, 'Use a Google Maps directions link.'); if (GOOGLE_SHORT_HOSTS.includes(url.hostname)) { const reply = await safeFetchFollow(url.href, { signal: AbortSignal.timeout(10000) }); url = new URL(reply.url); await reply.body?.cancel(); - if (!isGoogleMapsHost(url.hostname)) throw new HttpException({ error: 'Use a Google Maps directions link.' }, 400); + if (!isGoogleMapsHost(url.hostname)) throw new DomainError(400, 'Use a Google Maps directions link.'); } if (url.protocol !== 'https:' || !isDirectionsUrl(url.href)) - throw new HttpException({ error: 'Use a Google Maps directions link.' }, 400); + throw new DomainError(400, 'Use a Google Maps directions link.'); const waypoints = parseDirectionsUrl(url.href, MAX_DIR_WAYPOINTS + 1); if (waypoints.length < 2 || waypoints.length > MAX_DIR_WAYPOINTS) - throw new HttpException({ error: 'The link must contain between 2 and 30 readable stops.' }, 400); + throw new DomainError(400, 'The link must contain between 2 and 30 readable stops.'); const stops: GoogleRoutePreview['stops'] = []; for (const waypoint of waypoints) { let name = (waypoint.name || `${waypoint.lat}, ${waypoint.lng}`).slice(0, 200); @@ -63,16 +64,16 @@ export class GoogleRouteService { async import(tripId: number, userId: number, input: GoogleRouteImport, socketId?: string) { // GR0 — `TripsRepository.findAccessible` (keeps the row: `access.user_id` feeds the permission check below). const access = await this.tripsRepo.findAccessible(tripId, userId); - if (!access) throw new HttpException({ error: 'Trip not found' }, 404); + if (!access) throw new DomainError(404, 'Trip not found'); // GR1 — `UsersRepository.getRole`. const role = (await this.usersRepo.getRole(userId)) ?? 'user'; // `every` cannot await the permission check, so the same all-of test runs as // an explicit loop — same actions, same order, same short-circuit. for (const action of ['place_edit', 'day_edit']) { if (!(await this.permissions.checkPermission(action, role, access.user_id, userId, access.user_id !== userId))) - throw new HttpException({ error: 'Permission denied' }, 403); + throw new DomainError(403, 'Permission denied'); } - if (!(await this.assignments.dayExists(String(input.dayId), String(tripId)))) throw new HttpException({ error: 'Day not found' }, 404); + if (!(await this.assignments.dayExists(String(input.dayId), String(tripId)))) throw new DomainError(404, 'Day not found'); // `map` cannot await the now-async assignment write, so the same per-stop // sequence runs as an explicit loop inside the transaction. const imported = await this.uow.transactional(async () => { diff --git a/server/src/nest/roadtrip/roadtrip-plan.service.ts b/server/src/nest/roadtrip/roadtrip-plan.service.ts index e660e73d9..03daf50dd 100644 --- a/server/src/nest/roadtrip/roadtrip-plan.service.ts +++ b/server/src/nest/roadtrip/roadtrip-plan.service.ts @@ -1,10 +1,11 @@ +import { DomainError } from '../common/domain-error'; import { InjectRepository } from '@mikro-orm/nestjs'; import { SettingsService } from '../settings/settings.service'; import { DayBoundariesService } from './day-boundaries.service'; import { RoadtripPreferencesService } from './roadtrip-preferences.service'; import { RoadtripRouterService } from './roadtrip-router.service'; import { RoadtripService } from './roadtrip.service'; -import { Injectable, HttpException } from '@nestjs/common'; +import { Injectable } from '@nestjs/common'; import { type RoadtripPreferences } from '@trek/shared'; import { Days } from '../../db/entities/Days.entity'; import type { DaysRepository } from '../../db/repositories/Days.repository'; @@ -69,7 +70,7 @@ export class RoadtripPlanService { ) {} async context(tripId: number, userId: number) { - if (!(await this.tripsRepo.findAccessible(tripId, userId))) throw new HttpException({ error: 'Trip not found' }, 404); + if (!(await this.tripsRepo.findAccessible(tripId, userId))) throw new DomainError(404, 'Trip not found'); // RPL1 — `DaysRepository.listPlanDays`. const days = await this.daysRepo.listPlanDays(tripId); // RPL2 — `DayAssignmentsRepository.listRoadtripVisits`. @@ -139,7 +140,7 @@ export class RoadtripPlanService { preferences.roadtrip_day_end_mode, ); if (preferences.roadtrip_day_start && preferences.roadtrip_day_end && !window) - throw new HttpException({ error: 'Day end must be later than day start.' }, 400); + throw new DomainError(400, 'Day end must be later than day start.'); const seams = context.carriers.map((booking) => carrierSeam(booking)).filter((seam): seam is CarrierSeam => seam !== null); const dayNumberOf = (dayId: number): number => context.days.find((d) => d.id === dayId)?.day_number ?? 0; const stored: PlanDay[] = context.days.map((day) => { @@ -196,7 +197,7 @@ export class RoadtripPlanService { const avoid = parseAvoid(preferences.roadtrip_avoid); const connectDays = preferences.roadtrip_connect_days || window !== null; if (context.visits.length > 150) - throw new HttpException({ error: 'This trip exceeds the 150-visit calculation limit.' }, 400); + throw new DomainError(400, 'This trip exceeds the 150-visit calculation limit.'); const asked = new Set(); const distanceUnit: DistanceUnit = (await this.settings.getUserSettings(userId)).distance_unit === 'imperial' ? 'imperial' : 'metric'; diff --git a/server/src/nest/roadtrip/roadtrip-preferences.service.ts b/server/src/nest/roadtrip/roadtrip-preferences.service.ts index eb07dc024..4801002ca 100644 --- a/server/src/nest/roadtrip/roadtrip-preferences.service.ts +++ b/server/src/nest/roadtrip/roadtrip-preferences.service.ts @@ -1,7 +1,8 @@ +import { DomainError } from '../common/domain-error'; import { InjectRepository } from '@mikro-orm/nestjs'; import { RealtimeService } from '../realtime/realtime.service'; import { UnitOfWork } from '../database/unit-of-work'; -import { HttpException, Injectable } from '@nestjs/common'; +import { Injectable } from '@nestjs/common'; import { ROADTRIP_PREFERENCE_KEYS, roadtripPreferencesSchema, @@ -42,7 +43,7 @@ export class RoadtripPreferencesService { const saved = await this.uow.transactional(async () => { const next = { ...(await this.read(tripId)), ...validated }; if (next.roadtrip_day_start && next.roadtrip_day_end && next.roadtrip_day_end <= next.roadtrip_day_start) { - throw new HttpException({ error: 'Day end must be later than day start.' }, 400); + throw new DomainError(400, 'Day end must be later than day start.'); } // RPF3 — `RoadtripPreferencesRepository.upsertValue`. for (const [key, value] of Object.entries(validated)) { diff --git a/server/src/nest/roadtrip/roadtrip.service.ts b/server/src/nest/roadtrip/roadtrip.service.ts index 116820306..1d9dc554a 100644 --- a/server/src/nest/roadtrip/roadtrip.service.ts +++ b/server/src/nest/roadtrip/roadtrip.service.ts @@ -1,4 +1,5 @@ -import { HttpException, Injectable } from '@nestjs/common'; +import { DomainError } from '../common/domain-error'; +import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import type { RoadtripDayTrack, RoadtripVia, TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; @@ -75,7 +76,7 @@ export class RoadtripService { */ private requireDayId(dayId: string | number): number { const parsed = toRowId(dayId); - if (parsed === null) throw new HttpException({ error: 'Day not found' }, 404); + if (parsed === null) throw new DomainError(404, 'Day not found'); return parsed; } diff --git a/server/tests/unit/nest/admin-packing-templates.controller.test.ts b/server/tests/unit/nest/admin-packing-templates.controller.test.ts index 535d87f63..471d212d0 100644 --- a/server/tests/unit/nest/admin-packing-templates.controller.test.ts +++ b/server/tests/unit/nest/admin-packing-templates.controller.test.ts @@ -4,6 +4,7 @@ * envelope, same create-201 split, same audit actions — these cases came over from * admin.controller.test.ts with the routes. */ +import { DomainError } from '../../../src/nest/common/domain-error'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { HttpException } from '@nestjs/common'; import { AdminPackingTemplatesController } from '../../../src/nest/packing/admin-packing-templates.controller'; @@ -35,16 +36,6 @@ function controller(over: Partial = {}) { return { c: new AdminPackingTemplatesController(packing, { writeAudit } as unknown as AuditService), packing }; } -/** - * An override whose return value the real method signature cannot produce. Every error - * branch in PackingService sets a status, so an error envelope without one is not part - * of the service's return type, yet the controller still has a fallback for it and - * PACKTPL-003 is the case that covers that fallback. - */ -function offContract(name: K, impl: () => unknown) { - return { [name]: vi.fn(impl) } as unknown as Partial; -} - const thrown = async (run: () => unknown) => { try { await run(); @@ -63,14 +54,14 @@ describe('AdminPackingTemplatesController', () => { expect(await controller().c.list()).toEqual({ templates: [{ id: 1 }] }); }); - it('PACKTPL-002 a service {error,status} becomes that HTTP status, not a 200 body', async () => { - const { c } = controller({ getPackingTemplate: vi.fn(async () => ({ error: 'not found', status: 404 })) }); + it('PACKTPL-002 a service refusal becomes that HTTP status, not a 200 body', async () => { + const { c } = controller({ getPackingTemplate: vi.fn(async () => { throw new DomainError(404, 'not found'); }) }); expect(await thrown(() => c.get('9'))).toEqual({ status: 404, body: { error: 'not found' } }); }); - it('PACKTPL-003 an error without a status defaults to 400', async () => { - const { c } = controller(offContract('createTemplateCategory', async () => ({ error: 'name required' }))); - expect(await thrown(() => c.createCategory('1', { name: '' }))).toEqual({ status: 400, body: { error: 'name required' } }); + it('PACKTPL-003 a 400 refusal keeps its status and text', async () => { + const { c } = controller({ createTemplateCategory: vi.fn(async () => { throw new DomainError(400, 'Category name is required'); }) }); + expect(await thrown(() => c.createCategory('1', { name: '' }))).toEqual({ status: 400, body: { error: 'Category name is required' } }); }); it('PACKTPL-004 create audits with the new template id', async () => { diff --git a/server/tests/unit/nest/google-route.test.ts b/server/tests/unit/nest/google-route.test.ts index 2d71aebd0..5c0efec5c 100644 --- a/server/tests/unit/nest/google-route.test.ts +++ b/server/tests/unit/nest/google-route.test.ts @@ -59,7 +59,7 @@ describe('Google route import', () => { it('rejects unrelated URLs and oversized routes without fetching or truncating', async () => { const { service, maps } = await setup(); for (const url of ['https://example.com/maps/dir/A/B', 'http://google.com/maps/dir/A/B', 'https://user:pass@google.com/maps/dir/A/B', 'https://google.com/search?q=A']) { - await expect(service.preview(url)).rejects.toThrow('Http Exception'); + await expect(service.preview(url)).rejects.toThrow('Use a Google Maps directions link.'); } await expect(service.preview(`https://google.com/maps/dir/${Array.from({ length: 31 }, (_, i) => `A${i}`).join('/')}`)).rejects.toThrow(); expect(maps.geocodeQuery).not.toHaveBeenCalled(); diff --git a/server/tests/unit/nest/packing.service.test.ts b/server/tests/unit/nest/packing.service.test.ts index a1b9074e6..d127ae556 100644 --- a/server/tests/unit/nest/packing.service.test.ts +++ b/server/tests/unit/nest/packing.service.test.ts @@ -14,6 +14,7 @@ * 2026-08 admin fold, since this service already owned all three template * tables. */ +import { asLegacyResult } from '../../helpers/domain-error'; import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; // ── DB setup ────────────────────────────────────────────────────────────────── @@ -1049,44 +1050,44 @@ describe('PackingService — the surface the deleted bridge exposed', () => { describe('Packing templates', () => { it('ADMIN-SVC-031 — createPackingTemplate returns template', async () => { const { user: admin } = createAdmin(testDb); - const result = await svc.createPackingTemplate('Beach Trip', admin.id) as any; + const result = await asLegacyResult(svc.createPackingTemplate('Beach Trip', admin.id)) as any; expect(result.template.name).toBe('Beach Trip'); }); it('ADMIN-SVC-032 — createPackingTemplate returns 400 for empty name', async () => { const { user: admin } = createAdmin(testDb); - const result = await svc.createPackingTemplate('', admin.id) as any; + const result = await asLegacyResult(svc.createPackingTemplate('', admin.id)) as any; expect(result.status).toBe(400); }); it('ADMIN-SVC-033 — listPackingTemplates returns array', async () => { const { user: admin } = createAdmin(testDb); - await svc.createPackingTemplate('Template A', admin.id); + await asLegacyResult(svc.createPackingTemplate('Template A', admin.id)); const templates = await svc.listPackingTemplates() as any[]; expect(templates.length).toBeGreaterThanOrEqual(1); }); it('ADMIN-SVC-034 — updatePackingTemplate updates name', async () => { const { user: admin } = createAdmin(testDb); - const created = await svc.createPackingTemplate('Old Name', admin.id) as any; - const result = await svc.updatePackingTemplate(String(created.template.id), { name: 'New Name' }) as any; + const created = await asLegacyResult(svc.createPackingTemplate('Old Name', admin.id)) as any; + const result = await asLegacyResult(svc.updatePackingTemplate(String(created.template.id), { name: 'New Name' })) as any; expect(result.template.name).toBe('New Name'); }); it('ADMIN-SVC-035 — updatePackingTemplate returns 404 for non-existent', async () => { - const result = await svc.updatePackingTemplate('99999', { name: 'Ghost' }) as any; + const result = await asLegacyResult(svc.updatePackingTemplate('99999', { name: 'Ghost' })) as any; expect(result.status).toBe(404); }); it('ADMIN-SVC-036 — deletePackingTemplate removes template', async () => { const { user: admin } = createAdmin(testDb); - const created = await svc.createPackingTemplate('To Delete', admin.id) as any; - const result = await svc.deletePackingTemplate(String(created.template.id)) as any; + const created = await asLegacyResult(svc.createPackingTemplate('To Delete', admin.id)) as any; + const result = await asLegacyResult(svc.deletePackingTemplate(String(created.template.id))) as any; expect(result.name).toBe('To Delete'); }); it('ADMIN-SVC-037 — deletePackingTemplate returns 404 for non-existent', async () => { - const result = await svc.deletePackingTemplate('99999') as any; + const result = await asLegacyResult(svc.deletePackingTemplate('99999')) as any; expect(result.status).toBe(404); }); }); @@ -1094,50 +1095,50 @@ describe('Packing templates', () => { describe('Template categories', () => { it('ADMIN-SVC-038 — createTemplateCategory creates a category', async () => { const { user: admin } = createAdmin(testDb); - const tpl = await svc.createPackingTemplate('Tpl', admin.id) as any; - const result = await svc.createTemplateCategory(String(tpl.template.id), 'Clothing') as any; + const tpl = await asLegacyResult(svc.createPackingTemplate('Tpl', admin.id)) as any; + const result = await asLegacyResult(svc.createTemplateCategory(String(tpl.template.id), 'Clothing')) as any; expect(result.category.name).toBe('Clothing'); }); it('ADMIN-SVC-039 — createTemplateCategory returns 400 for empty name', async () => { const { user: admin } = createAdmin(testDb); - const tpl = await svc.createPackingTemplate('Tpl', admin.id) as any; - const result = await svc.createTemplateCategory(String(tpl.template.id), '') as any; + const tpl = await asLegacyResult(svc.createPackingTemplate('Tpl', admin.id)) as any; + const result = await asLegacyResult(svc.createTemplateCategory(String(tpl.template.id), '')) as any; expect(result.status).toBe(400); }); it('ADMIN-SVC-040 — createTemplateCategory returns 404 for missing template', async () => { - const result = await svc.createTemplateCategory('99999', 'Clothing') as any; + const result = await asLegacyResult(svc.createTemplateCategory('99999', 'Clothing')) as any; expect(result.status).toBe(404); }); it('ADMIN-SVC-041 — updateTemplateCategory updates name', async () => { const { user: admin } = createAdmin(testDb); - const tpl = await svc.createPackingTemplate('Tpl', admin.id) as any; - const cat = await svc.createTemplateCategory(String(tpl.template.id), 'Old') as any; - const result = await svc.updateTemplateCategory(String(tpl.template.id), String(cat.category.id), { name: 'New' }) as any; + const tpl = await asLegacyResult(svc.createPackingTemplate('Tpl', admin.id)) as any; + const cat = await asLegacyResult(svc.createTemplateCategory(String(tpl.template.id), 'Old')) as any; + const result = await asLegacyResult(svc.updateTemplateCategory(String(tpl.template.id), String(cat.category.id), { name: 'New' })) as any; expect(result.category.name).toBe('New'); }); it('ADMIN-SVC-042 — updateTemplateCategory returns 404 for missing category', async () => { const { user: admin } = createAdmin(testDb); - const tpl = await svc.createPackingTemplate('Tpl', admin.id) as any; - const result = await svc.updateTemplateCategory(String(tpl.template.id), '99999', { name: 'X' }) as any; + const tpl = await asLegacyResult(svc.createPackingTemplate('Tpl', admin.id)) as any; + const result = await asLegacyResult(svc.updateTemplateCategory(String(tpl.template.id), '99999', { name: 'X' })) as any; expect(result.status).toBe(404); }); it('ADMIN-SVC-043 — deleteTemplateCategory removes category', async () => { const { user: admin } = createAdmin(testDb); - const tpl = await svc.createPackingTemplate('Tpl', admin.id) as any; - const cat = await svc.createTemplateCategory(String(tpl.template.id), 'Remove Me') as any; - const result = await svc.deleteTemplateCategory(String(tpl.template.id), String(cat.category.id)) as any; + const tpl = await asLegacyResult(svc.createPackingTemplate('Tpl', admin.id)) as any; + const cat = await asLegacyResult(svc.createTemplateCategory(String(tpl.template.id), 'Remove Me')) as any; + const result = await asLegacyResult(svc.deleteTemplateCategory(String(tpl.template.id), String(cat.category.id))) as any; expect(result.error).toBeUndefined(); }); it('ADMIN-SVC-044 — deleteTemplateCategory returns 404 for missing', async () => { const { user: admin } = createAdmin(testDb); - const tpl = await svc.createPackingTemplate('Tpl', admin.id) as any; - const result = await svc.deleteTemplateCategory(String(tpl.template.id), '99999') as any; + const tpl = await asLegacyResult(svc.createPackingTemplate('Tpl', admin.id)) as any; + const result = await asLegacyResult(svc.deleteTemplateCategory(String(tpl.template.id), '99999')) as any; expect(result.status).toBe(404); }); }); @@ -1145,11 +1146,11 @@ describe('Template categories', () => { describe('getPackingTemplate', () => { it('ADMIN-SVC-056 — returns template with categories and items when template exists', async () => { const { user: admin } = createAdmin(testDb); - const tpl = await svc.createPackingTemplate('Full Template', admin.id) as any; - const cat = await svc.createTemplateCategory(String(tpl.template.id), 'Clothing') as any; - await svc.createTemplateItem(String(tpl.template.id), String(cat.category.id), 'T-Shirt'); + const tpl = await asLegacyResult(svc.createPackingTemplate('Full Template', admin.id)) as any; + const cat = await asLegacyResult(svc.createTemplateCategory(String(tpl.template.id), 'Clothing')) as any; + await asLegacyResult(svc.createTemplateItem(String(tpl.template.id), String(cat.category.id), 'T-Shirt')); - const result = await svc.getPackingTemplate(String(tpl.template.id)) as any; + const result = await asLegacyResult(svc.getPackingTemplate(String(tpl.template.id))) as any; expect(result.template).toBeDefined(); expect(result.template.name).toBe('Full Template'); expect(Array.isArray(result.categories)).toBe(true); @@ -1160,7 +1161,7 @@ describe('getPackingTemplate', () => { }); it('ADMIN-SVC-057 — returns 404 for non-existent template', async () => { - const result = await svc.getPackingTemplate('99999') as any; + const result = await asLegacyResult(svc.getPackingTemplate('99999')) as any; expect(result.status).toBe(404); expect(result.error).toBeDefined(); }); @@ -1169,54 +1170,54 @@ describe('getPackingTemplate', () => { describe('Template items', () => { it('ADMIN-SVC-058 — createTemplateItem returns item with name', async () => { const { user: admin } = createAdmin(testDb); - const tpl = await svc.createPackingTemplate('Tpl', admin.id) as any; - const cat = await svc.createTemplateCategory(String(tpl.template.id), 'Gear') as any; - const result = await svc.createTemplateItem(String(tpl.template.id), String(cat.category.id), 'Backpack') as any; + const tpl = await asLegacyResult(svc.createPackingTemplate('Tpl', admin.id)) as any; + const cat = await asLegacyResult(svc.createTemplateCategory(String(tpl.template.id), 'Gear')) as any; + const result = await asLegacyResult(svc.createTemplateItem(String(tpl.template.id), String(cat.category.id), 'Backpack')) as any; expect(result.item).toBeDefined(); expect(result.item.name).toBe('Backpack'); }); it('ADMIN-SVC-059 — createTemplateItem returns 400 for empty name', async () => { const { user: admin } = createAdmin(testDb); - const tpl = await svc.createPackingTemplate('Tpl', admin.id) as any; - const cat = await svc.createTemplateCategory(String(tpl.template.id), 'Gear') as any; - const result = await svc.createTemplateItem(String(tpl.template.id), String(cat.category.id), '') as any; + const tpl = await asLegacyResult(svc.createPackingTemplate('Tpl', admin.id)) as any; + const cat = await asLegacyResult(svc.createTemplateCategory(String(tpl.template.id), 'Gear')) as any; + const result = await asLegacyResult(svc.createTemplateItem(String(tpl.template.id), String(cat.category.id), '')) as any; expect(result.status).toBe(400); }); it('ADMIN-SVC-060 — createTemplateItem returns 404 for non-existent category', async () => { const { user: admin } = createAdmin(testDb); - const tpl = await svc.createPackingTemplate('Tpl', admin.id) as any; - const result = await svc.createTemplateItem(String(tpl.template.id), '99999', 'Item') as any; + const tpl = await asLegacyResult(svc.createPackingTemplate('Tpl', admin.id)) as any; + const result = await asLegacyResult(svc.createTemplateItem(String(tpl.template.id), '99999', 'Item')) as any; expect(result.status).toBe(404); }); it('ADMIN-SVC-061 — updateTemplateItem updates name', async () => { const { user: admin } = createAdmin(testDb); - const tpl = await svc.createPackingTemplate('Tpl', admin.id) as any; - const cat = await svc.createTemplateCategory(String(tpl.template.id), 'Gear') as any; - const item = await svc.createTemplateItem(String(tpl.template.id), String(cat.category.id), 'Old Item') as any; - const result = await svc.updateTemplateItem(String(tpl.template.id), String(item.item.id), { name: 'New Item' }) as any; + const tpl = await asLegacyResult(svc.createPackingTemplate('Tpl', admin.id)) as any; + const cat = await asLegacyResult(svc.createTemplateCategory(String(tpl.template.id), 'Gear')) as any; + const item = await asLegacyResult(svc.createTemplateItem(String(tpl.template.id), String(cat.category.id), 'Old Item')) as any; + const result = await asLegacyResult(svc.updateTemplateItem(String(tpl.template.id), String(item.item.id), { name: 'New Item' })) as any; expect(result.item.name).toBe('New Item'); }); it('ADMIN-SVC-062 — updateTemplateItem returns 404 for non-existent item', async () => { - const result = await svc.updateTemplateItem('1', '99999', { name: 'Ghost' }) as any; + const result = await asLegacyResult(svc.updateTemplateItem('1', '99999', { name: 'Ghost' })) as any; expect(result.status).toBe(404); }); it('ADMIN-SVC-063 — deleteTemplateItem removes item', async () => { const { user: admin } = createAdmin(testDb); - const tpl = await svc.createPackingTemplate('Tpl', admin.id) as any; - const cat = await svc.createTemplateCategory(String(tpl.template.id), 'Gear') as any; - const item = await svc.createTemplateItem(String(tpl.template.id), String(cat.category.id), 'To Delete') as any; - const result = await svc.deleteTemplateItem(String(tpl.template.id), String(item.item.id)) as any; + const tpl = await asLegacyResult(svc.createPackingTemplate('Tpl', admin.id)) as any; + const cat = await asLegacyResult(svc.createTemplateCategory(String(tpl.template.id), 'Gear')) as any; + const item = await asLegacyResult(svc.createTemplateItem(String(tpl.template.id), String(cat.category.id), 'To Delete')) as any; + const result = await asLegacyResult(svc.deleteTemplateItem(String(tpl.template.id), String(item.item.id))) as any; expect(result.error).toBeUndefined(); expect(await findRow(await orm(), PackingTemplateItems, { id: item.item.id })).toBeNull(); }); it('ADMIN-SVC-064 — deleteTemplateItem returns 404 for non-existent item', async () => { - const result = await svc.deleteTemplateItem('1', '99999') as any; + const result = await asLegacyResult(svc.deleteTemplateItem('1', '99999')) as any; expect(result.status).toBe(404); }); }); @@ -1226,23 +1227,23 @@ describe('Template items', () => { describe('Template item scoping (post-fold quirk fix)', () => { it('ADMIN-SVC-076 — update/deleteTemplateItem honour :templateId instead of ignoring it', async () => { const { user: admin } = createAdmin(testDb); - const tplA = await svc.createPackingTemplate('A', admin.id) as any; - const tplB = await svc.createPackingTemplate('B', admin.id) as any; - const catA = await svc.createTemplateCategory(String(tplA.template.id), 'Gear') as any; - const item = await svc.createTemplateItem(String(tplA.template.id), String(catA.category.id), 'Tent') as any; + const tplA = await asLegacyResult(svc.createPackingTemplate('A', admin.id)) as any; + const tplB = await asLegacyResult(svc.createPackingTemplate('B', admin.id)) as any; + const catA = await asLegacyResult(svc.createTemplateCategory(String(tplA.template.id), 'Gear')) as any; + const item = await asLegacyResult(svc.createTemplateItem(String(tplA.template.id), String(catA.category.id), 'Tent')) as any; // Template B does not own the item — both routes must 404 rather than act. - expect(await svc.updateTemplateItem(String(tplB.template.id), String(item.item.id), { name: 'Hijacked' }) as any) + expect(await asLegacyResult(svc.updateTemplateItem(String(tplB.template.id), String(item.item.id), { name: 'Hijacked' })) as any) .toMatchObject({ status: 404 }); - expect(await svc.deleteTemplateItem(String(tplB.template.id), String(item.item.id)) as any) + expect(await asLegacyResult(svc.deleteTemplateItem(String(tplB.template.id), String(item.item.id))) as any) .toMatchObject({ status: 404 }); expect((await findRow(await orm(), PackingTemplateItems, { id: item.item.id }))!.name) .toBe('Tent'); // The owning template still works. - expect((await svc.updateTemplateItem(String(tplA.template.id), String(item.item.id), { name: 'Tarp' }) as any).item.name) + expect((await asLegacyResult(svc.updateTemplateItem(String(tplA.template.id), String(item.item.id), { name: 'Tarp' })) as any).item.name) .toBe('Tarp'); - expect(await svc.deleteTemplateItem(String(tplA.template.id), String(item.item.id)) as any).toEqual({}); + expect(await asLegacyResult(svc.deleteTemplateItem(String(tplA.template.id), String(item.item.id))) as any).toEqual({}); }); });