From f7edd849abd15353d001d8ce5952cfd9b0378c70 Mon Sep 17 00:00:00 2001 From: Maurice Date: Wed, 7 Oct 2026 20:54:10 +0200 Subject: [PATCH] fix(tours): one tour type catalogue, and no column nothing writes tour_types now holds every key the shared contract accepts, all but hike disabled, and create and edit turn a disabled type away with a 400 instead of failing on the foreign key. tours.tour_group_id is dropped: nothing ever wrote or read it. difficulty and wanderer_ref stay for the Wanderer import. --- .../Tours/TourDetailDialog.test.tsx | 1 - .../components/Tours/ToursSidebar.test.tsx | 1 - .../Tours/planner/TourPlannerPanels.test.tsx | 1 - .../Tours/planner/useTourPlanner.test.tsx | 1 - .../src/components/Tours/toursI18n.test.tsx | 2 +- .../shared/CustomSelect.consumers.test.tsx | 2 +- client/src/hooks/useTourPlaceIds.test.ts | 1 - client/src/hooks/useTripWebSocket.test.ts | 1 - .../trip/places/MToursSelectionList.test.tsx | 1 - .../screens/trip/sheets/MTripSheets.test.tsx | 1 - .../src/pages/TripPlannerPage.wiring.test.tsx | 5 -- .../pages/tripPlanner/useTripPlanner.test.tsx | 11 ++-- .../unit/mobile/trip/MPlacesBrowser.test.tsx | 2 +- .../unit/mobile/trip/MTripSheets.test.tsx | 2 +- server/src/db/entities/Tours.entity.ts | 2 - ...2300_every_tour_type_the_contract_names.ts | 33 ++++++++++ .../db/repositories/TourTypes.repository.ts | 14 ++-- .../src/db/repositories/Tours.repository.ts | 3 - server/src/nest/tours/tours.module.ts | 3 +- server/src/nest/tours/tours.service.ts | 18 ++++- server/tests/helpers/tours-repos.ts | 6 ++ .../db/repositories/Tours.repository.test.ts | 10 +-- .../unit/db/tour-types-migration.test.ts | 65 +++++++++++++++++++ .../tests/unit/nest/tours.gpx.atomic.test.ts | 4 +- ...s.permissions.boundaries.mock-only.test.ts | 4 +- .../nest/tours.service.permissions.test.ts | 4 +- server/tests/unit/nest/tours.service.test.ts | 29 ++++++++- server/tests/unit/nest/trips.service.test.ts | 4 +- shared/src/tours/tours.schema.ts | 1 - 29 files changed, 182 insertions(+), 50 deletions(-) create mode 100644 server/src/db/migrations/Migration20200101042300_every_tour_type_the_contract_names.ts create mode 100644 server/tests/unit/db/tour-types-migration.test.ts diff --git a/client/src/components/Tours/TourDetailDialog.test.tsx b/client/src/components/Tours/TourDetailDialog.test.tsx index e259a4014..c41025c53 100644 --- a/client/src/components/Tours/TourDetailDialog.test.tsx +++ b/client/src/components/Tours/TourDetailDialog.test.tsx @@ -35,7 +35,6 @@ const tour: TourListItem = { difficulty: null, wanderer_ref: null, match_confidence: null, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, diff --git a/client/src/components/Tours/ToursSidebar.test.tsx b/client/src/components/Tours/ToursSidebar.test.tsx index cad5e72f9..8667b9418 100644 --- a/client/src/components/Tours/ToursSidebar.test.tsx +++ b/client/src/components/Tours/ToursSidebar.test.tsx @@ -27,7 +27,6 @@ const tour: TourListItem = { difficulty: null, wanderer_ref: null, match_confidence: null, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, diff --git a/client/src/components/Tours/planner/TourPlannerPanels.test.tsx b/client/src/components/Tours/planner/TourPlannerPanels.test.tsx index 81bb601e6..8cab05d07 100644 --- a/client/src/components/Tours/planner/TourPlannerPanels.test.tsx +++ b/client/src/components/Tours/planner/TourPlannerPanels.test.tsx @@ -25,7 +25,6 @@ const tour: TourListItem = { difficulty: null, wanderer_ref: null, match_confidence: 1, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, diff --git a/client/src/components/Tours/planner/useTourPlanner.test.tsx b/client/src/components/Tours/planner/useTourPlanner.test.tsx index 04aedac09..c649983d0 100644 --- a/client/src/components/Tours/planner/useTourPlanner.test.tsx +++ b/client/src/components/Tours/planner/useTourPlanner.test.tsx @@ -40,7 +40,6 @@ const savedTour = { difficulty: null, wanderer_ref: null, match_confidence: 1, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, diff --git a/client/src/components/Tours/toursI18n.test.tsx b/client/src/components/Tours/toursI18n.test.tsx index ebf8e9050..6e2de52ba 100644 --- a/client/src/components/Tours/toursI18n.test.tsx +++ b/client/src/components/Tours/toursI18n.test.tsx @@ -21,7 +21,7 @@ vi.mock('better-sqlite3', () => { throw new Error('SQLite imports are forbidden const tour: TourListItem = { place_id: 42, name: 'Ridge walk', tour_type: 'hike', distance: 4, elevation_gain: 100, elevation_loss: 80, duration: 60, difficulty: null, - wanderer_ref: null, match_confidence: 1, tour_group_id: null, + wanderer_ref: null, match_confidence: 1, max_hiking_difficulty: 2, planned: false, caution: false, } diff --git a/client/src/components/shared/CustomSelect.consumers.test.tsx b/client/src/components/shared/CustomSelect.consumers.test.tsx index 6c6e4d7ac..2a3e0fee6 100644 --- a/client/src/components/shared/CustomSelect.consumers.test.tsx +++ b/client/src/components/shared/CustomSelect.consumers.test.tsx @@ -22,7 +22,7 @@ vi.mock('../../store/tripStore', () => ({ const tour: TourListItem = { place_id: 42, name: 'Ridge walk', tour_type: 'hike', distance: 4, elevation_gain: 100, elevation_loss: 80, duration: 60, difficulty: null, - wanderer_ref: null, match_confidence: 1, tour_group_id: null, + wanderer_ref: null, match_confidence: 1, max_hiking_difficulty: 2, planned: false, caution: false, } const days = [{ id: 7, trip_id: 1, day_number: 1, title: 'Summit day', date: '2026-05-15' } as Day] diff --git a/client/src/hooks/useTourPlaceIds.test.ts b/client/src/hooks/useTourPlaceIds.test.ts index da8f7040e..a27601825 100644 --- a/client/src/hooks/useTourPlaceIds.test.ts +++ b/client/src/hooks/useTourPlaceIds.test.ts @@ -19,7 +19,6 @@ const tour: TourListItem = { difficulty: null, wanderer_ref: null, match_confidence: null, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, diff --git a/client/src/hooks/useTripWebSocket.test.ts b/client/src/hooks/useTripWebSocket.test.ts index 2247cd002..dc1a3270d 100644 --- a/client/src/hooks/useTripWebSocket.test.ts +++ b/client/src/hooks/useTripWebSocket.test.ts @@ -46,7 +46,6 @@ const baseTour: TourListItem = { difficulty: null, wanderer_ref: null, match_confidence: null, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, diff --git a/client/src/mobile/screens/trip/places/MToursSelectionList.test.tsx b/client/src/mobile/screens/trip/places/MToursSelectionList.test.tsx index baa2b169d..805bf60c4 100644 --- a/client/src/mobile/screens/trip/places/MToursSelectionList.test.tsx +++ b/client/src/mobile/screens/trip/places/MToursSelectionList.test.tsx @@ -19,7 +19,6 @@ const tour: TourListItem = { difficulty: null, wanderer_ref: null, match_confidence: null, - tour_group_id: null, has_waypoints: true, max_hiking_difficulty: 2, planned: false, diff --git a/client/src/mobile/screens/trip/sheets/MTripSheets.test.tsx b/client/src/mobile/screens/trip/sheets/MTripSheets.test.tsx index 869bafa32..da688d706 100644 --- a/client/src/mobile/screens/trip/sheets/MTripSheets.test.tsx +++ b/client/src/mobile/screens/trip/sheets/MTripSheets.test.tsx @@ -28,7 +28,6 @@ const tour: TourListItem = { difficulty: null, wanderer_ref: null, match_confidence: null, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, diff --git a/client/src/pages/TripPlannerPage.wiring.test.tsx b/client/src/pages/TripPlannerPage.wiring.test.tsx index 7410f93af..1ae2aebf1 100644 --- a/client/src/pages/TripPlannerPage.wiring.test.tsx +++ b/client/src/pages/TripPlannerPage.wiring.test.tsx @@ -546,7 +546,6 @@ describe('TripPlannerPage — shell', () => { difficulty: null, wanderer_ref: null, match_confidence: 1, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, @@ -600,7 +599,6 @@ describe('TripPlannerPage — shell', () => { difficulty: null, wanderer_ref: null, match_confidence: 1, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, @@ -735,7 +733,6 @@ describe('TripPlannerPage — shell', () => { difficulty: null, wanderer_ref: null, match_confidence: null, - tour_group_id: null, max_hiking_difficulty: 2, planned: true, caution: false, @@ -762,7 +759,6 @@ describe('TripPlannerPage — shell', () => { difficulty: null, wanderer_ref: null, match_confidence: null, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, @@ -866,7 +862,6 @@ describe('TripPlannerPage — shell', () => { difficulty: null, wanderer_ref: null, match_confidence: 1, - tour_group_id: null, planned: false, caution: false, has_waypoints: false, diff --git a/client/src/pages/tripPlanner/useTripPlanner.test.tsx b/client/src/pages/tripPlanner/useTripPlanner.test.tsx index 13ba9abe8..01b36e34f 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.test.tsx +++ b/client/src/pages/tripPlanner/useTripPlanner.test.tsx @@ -667,7 +667,7 @@ describe('useTripPlanner — map derivations', () => { const tour: TourListItem = { place_id: 7, name: 'Ridge walk', tour_type: 'hike', distance: 4, elevation_gain: 100, elevation_loss: 80, duration: null, difficulty: null, wanderer_ref: null, match_confidence: 1, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, + max_hiking_difficulty: 2, planned: false, caution: false, } useAddonStore.setState({ addons: [{ id: 'tours', enabled: true } as never], loaded: true }) vi.mocked(addonsApi.enabled).mockResolvedValue({ addons: [{ id: 'tours' }] }) @@ -1439,7 +1439,6 @@ describe('useTripPlanner — place CRUD', () => { difficulty: null, wanderer_ref: null, match_confidence: 1, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, @@ -1638,7 +1637,7 @@ describe('useTripPlanner — day plan CRUD', () => { const tour: TourListItem = { place_id: 1, name: 'Ridge walk', tour_type: 'hike', distance: 4, elevation_gain: 100, elevation_loss: 80, duration: null, difficulty: null, wanderer_ref: null, match_confidence: 1, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, + max_hiking_difficulty: 2, planned: false, caution: false, } const tourPlace = buildPlace({ id: 1, name: 'Ridge walk', tour_place_id: 1 }) const ordinaryPlace = buildPlace({ id: 2, name: 'Ordinary place' }) @@ -1669,7 +1668,7 @@ describe('useTripPlanner — day plan CRUD', () => { const unplanned: TourListItem = { place_id: 1, name: 'Ridge walk', tour_type: 'hike', distance: 4, elevation_gain: 100, elevation_loss: 80, duration: null, difficulty: null, wanderer_ref: null, match_confidence: 1, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, + max_hiking_difficulty: 2, planned: false, caution: false, } const planned = { ...unplanned, planned: true } useAddonStore.setState({ addons: [{ id: 'tours', enabled: true } as never], loaded: true }) @@ -1707,7 +1706,7 @@ describe('useTripPlanner — day plan CRUD', () => { const unplanned: TourListItem = { place_id: 1, name: 'Ridge walk', tour_type: 'hike', distance: 4, elevation_gain: 100, elevation_loss: 80, duration: null, difficulty: null, wanderer_ref: null, match_confidence: 1, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, + max_hiking_difficulty: 2, planned: false, caution: false, } useAddonStore.setState({ addons: [{ id: 'tours', enabled: true } as never], loaded: true }) const listTours = vi.spyOn(toursApi, 'list').mockResolvedValue({ tours: [unplanned] }) @@ -1729,7 +1728,7 @@ describe('useTripPlanner — day plan CRUD', () => { const unplanned: TourListItem = { place_id: 1, name: 'Ridge walk', tour_type: 'hike', distance: 4, elevation_gain: 100, elevation_loss: 80, duration: null, difficulty: null, wanderer_ref: null, match_confidence: 1, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, + max_hiking_difficulty: 2, planned: false, caution: false, } const planned: TourListItem = { ...unplanned, planned: true } useAddonStore.setState({ addons: [{ id: 'tours', enabled: true } as never], loaded: true }) diff --git a/client/tests/unit/mobile/trip/MPlacesBrowser.test.tsx b/client/tests/unit/mobile/trip/MPlacesBrowser.test.tsx index 151f71f3d..1077bba56 100644 --- a/client/tests/unit/mobile/trip/MPlacesBrowser.test.tsx +++ b/client/tests/unit/mobile/trip/MPlacesBrowser.test.tsx @@ -366,7 +366,7 @@ describe('MPlacesBrowser', () => { const ridge: TourListItem = { place_id: TOUR.id, name: 'Ridge walk', tour_type: 'hike', distance: 4, elevation_gain: 100, elevation_loss: 80, duration: null, difficulty: null, wanderer_ref: null, match_confidence: 1, - tour_group_id: null, max_hiking_difficulty: 2, planned: false, caution: false, + max_hiking_difficulty: 2, planned: false, caution: false, } seedStore(useAddonStore, { addons: [{ id: 'collections', enabled: true }, { id: 'tours', enabled: true }] }) const list = vi.spyOn(toursApi, 'list').mockResolvedValue({ tours: [ridge] }) diff --git a/client/tests/unit/mobile/trip/MTripSheets.test.tsx b/client/tests/unit/mobile/trip/MTripSheets.test.tsx index f68a09de9..66f69451f 100644 --- a/client/tests/unit/mobile/trip/MTripSheets.test.tsx +++ b/client/tests/unit/mobile/trip/MTripSheets.test.tsx @@ -440,7 +440,7 @@ describe('MTripSheets', () => { const mobileTour = { place_id: 42, name: 'Mobile ridge', tour_type: 'hike', distance: 1, elevation_gain: 10, elevation_loss: 0, duration: null, difficulty: null, - wanderer_ref: null, match_confidence: 1, tour_group_id: null, + wanderer_ref: null, match_confidence: 1, max_hiking_difficulty: 2, planned: false, caution: false, } as TourListItem const readOnlyFile = { diff --git a/server/src/db/entities/Tours.entity.ts b/server/src/db/entities/Tours.entity.ts index beedbaad7..80f39f6e6 100644 --- a/server/src/db/entities/Tours.entity.ts +++ b/server/src/db/entities/Tours.entity.ts @@ -17,7 +17,6 @@ export class Tours { difficulty?: string | null; wanderer_ref?: string | null; match_confidence?: number | null; - tour_group_id?: number | null; created_at?: string | null; max_hiking_difficulty: number & Opt = 2; tourTypeRef!: Ref; @@ -38,7 +37,6 @@ export const ToursSchema = defineEntity({ difficulty: p.text().nullable(), wanderer_ref: p.text().nullable(), match_confidence: p.double().nullable(), - tour_group_id: p.integer().nullable(), created_at: p.text().nullable().defaultRaw(`CURRENT_TIMESTAMP`), max_hiking_difficulty: p.integer().default(2), tourTypeRef: () => p.manyToOne(TourTypes).ref().joinColumn('tour_type').hidden().index('idx_tours_tour_type'), diff --git a/server/src/db/migrations/Migration20200101042300_every_tour_type_the_contract_names.ts b/server/src/db/migrations/Migration20200101042300_every_tour_type_the_contract_names.ts new file mode 100644 index 000000000..7b59fddae --- /dev/null +++ b/server/src/db/migrations/Migration20200101042300_every_tour_type_the_contract_names.ts @@ -0,0 +1,33 @@ +import { Migration } from '@mikro-orm/migrations'; +import { columnNames } from '../migration-utils'; + +/** + * `tour_types` holds every key the shared `tourTypeKeySchema` accepts. Only + * `hike` was seeded, so a tour of any other type the contract allows failed + * on the foreign key with a 500 instead of a clean refusal. The six others + * are written disabled: the planner only offers `hike` today, and the + * service turns a disabled type away with a 400. + * + * `tours.tour_group_id` goes. Nothing ever wrote or read it, and a column + * nothing writes lets every copy and export carry a meaning nobody defined. + * `difficulty` and `wanderer_ref` stay: the Wanderer import fills them. + */ +export class Migration20200101042300_every_tour_type_the_contract_names extends Migration { + override name = 'Migration20200101042300_every_tour_type_the_contract_names'; + + override async up(): Promise { + this.addSql(` + INSERT OR IGNORE INTO tour_types (key, label_key, icon, color, routing_profile, is_sport, enabled, sort_order) VALUES + ('bike', 'tourTypes.bike', 'Bike', '#2563eb', 'bicycle', 1, 0, 1), + ('run', 'tourTypes.run', 'Footprints', '#dc2626', 'pedestrian', 1, 0, 2), + ('ski', 'tourTypes.ski', 'MountainSnow', '#0891b2', NULL, 1, 0, 3), + ('kayak', 'tourTypes.kayak', 'Sailboat', '#0d9488', NULL, 1, 0, 4), + ('walk', 'tourTypes.walk', 'Landmark', '#9333ea', 'pedestrian', 0, 0, 5), + ('food', 'tourTypes.food', 'UtensilsCrossed', '#ea580c', 'pedestrian', 0, 0, 6) + `); + + if ((await columnNames(this, 'tours')).has('tour_group_id')) { + this.addSql('ALTER TABLE tours DROP COLUMN tour_group_id'); + } + } +} diff --git a/server/src/db/repositories/TourTypes.repository.ts b/server/src/db/repositories/TourTypes.repository.ts index 49c5c9314..92fdb7982 100644 --- a/server/src/db/repositories/TourTypes.repository.ts +++ b/server/src/db/repositories/TourTypes.repository.ts @@ -2,8 +2,14 @@ import type { TourTypes } from '../entities/TourTypes.entity'; import { TrekRepository } from './_shared/trek-repository'; /** - * `tour_types` — the controlled vocabulary `tours.tour_type` points at. Only - * `hike` exists, written by the migration that creates the table; nothing - * reads the catalogue yet, so this repository has no methods of its own. + * `tour_types` — the controlled vocabulary `tours.tour_type` points at, one + * row per key of the shared `tourTypeKeySchema`. The migrations write it; + * `enabled` says which types a tour may be created with. */ -export class TourTypesRepository extends TrekRepository {} +export class TourTypesRepository extends TrekRepository { + /** Whether `key` is a known tour type a tour may be created with. */ + async isEnabled(key: string): Promise { + const row = await this.findOne({ key }, { fields: ['enabled'] }); + return row?.enabled === 1; + } +} diff --git a/server/src/db/repositories/Tours.repository.ts b/server/src/db/repositories/Tours.repository.ts index 7e3b34acf..8e04686a5 100644 --- a/server/src/db/repositories/Tours.repository.ts +++ b/server/src/db/repositories/Tours.repository.ts @@ -19,7 +19,6 @@ export interface TourListRow { difficulty: string | null; wanderer_ref: string | null; match_confidence: number | null; - tour_group_id: number | null; max_hiking_difficulty: number; planned: number; has_waypoints: number; @@ -36,7 +35,6 @@ export interface TourRow { difficulty: string | null; wanderer_ref: string | null; match_confidence: number | null; - tour_group_id: number | null; created_at: string | null; max_hiking_difficulty: number; } @@ -95,7 +93,6 @@ export class ToursRepository extends TrekRepository { 't.difficulty as difficulty', 't.wanderer_ref as wanderer_ref', 't.match_confidence as match_confidence', - 't.tour_group_id as tour_group_id', 't.max_hiking_difficulty as max_hiking_difficulty', eb.exists(eb.selectFrom('day_assignments as da').select('da.id').whereRef('da.place_id', '=', 'p.id')).as('planned'), eb.exists(eb.selectFrom('tour_waypoints as tw').select('tw.id').whereRef('tw.place_id', '=', 'p.id')).as('has_waypoints'), diff --git a/server/src/nest/tours/tours.module.ts b/server/src/nest/tours/tours.module.ts index 16bf958c9..1b6de1636 100644 --- a/server/src/nest/tours/tours.module.ts +++ b/server/src/nest/tours/tours.module.ts @@ -2,6 +2,7 @@ import { MikroOrmModule } from '@mikro-orm/nestjs'; import { Module } from '@nestjs/common'; import { Places } from '../../db/entities/Places.entity'; import { Tours } from '../../db/entities/Tours.entity'; +import { TourTypes } from '../../db/entities/TourTypes.entity'; import { TourWaypoints } from '../../db/entities/TourWaypoints.entity'; import { ToursController } from './tours.controller'; import { ToursImportController } from './tours-import.controller'; @@ -24,7 +25,7 @@ import { AddonsModule } from '../addons/addons.module'; PermissionsModule, AuthModule, AddonsModule, - MikroOrmModule.forFeature([Tours, TourWaypoints, Places]), + MikroOrmModule.forFeature([Tours, TourTypes, TourWaypoints, Places]), ], controllers: [ToursController, ToursImportController], providers: [ToursService], diff --git a/server/src/nest/tours/tours.service.ts b/server/src/nest/tours/tours.service.ts index b064fd64d..5dcc10309 100644 --- a/server/src/nest/tours/tours.service.ts +++ b/server/src/nest/tours/tours.service.ts @@ -1,11 +1,13 @@ import { InjectRepository } from '@mikro-orm/nestjs'; -import { Injectable, Logger, NotFoundException } from '@nestjs/common'; +import { BadRequestException, Injectable, Logger, NotFoundException } from '@nestjs/common'; import type { TourCreateRequest, TourCreateResponse, TourDetailResponse, TourListItem, TourWaypoint } from '@trek/shared'; import { Places } from '../../db/entities/Places.entity'; import { Tours } from '../../db/entities/Tours.entity'; +import { TourTypes } from '../../db/entities/TourTypes.entity'; import { TourWaypoints } from '../../db/entities/TourWaypoints.entity'; import type { PlacesRepository } from '../../db/repositories/Places.repository'; import type { TourListRow, ToursRepository, TourUpdate } from '../../db/repositories/Tours.repository'; +import type { TourTypesRepository } from '../../db/repositories/TourTypes.repository'; import type { TourWaypointsRepository } from '../../db/repositories/TourWaypoints.repository'; import { toRowId } from '../common/row-id'; import { UnitOfWork } from '../database/unit-of-work'; @@ -33,6 +35,7 @@ export class ToursService { private readonly uow: UnitOfWork, private readonly places: PlacesService, @InjectRepository(Tours) private readonly toursRepo: ToursRepository, + @InjectRepository(TourTypes) private readonly tourTypesRepo: TourTypesRepository, @InjectRepository(TourWaypoints) private readonly waypointsRepo: TourWaypointsRepository, @InjectRepository(Places) private readonly placesRepo: PlacesRepository, ) {} @@ -49,7 +52,6 @@ export class ToursService { difficulty: r.difficulty, wanderer_ref: r.wanderer_ref, match_confidence: r.match_confidence, - tour_group_id: r.tour_group_id, max_hiking_difficulty: r.max_hiking_difficulty ?? 2, planned: Boolean(r.planned), caution: r.match_confidence !== null && r.match_confidence < LOW_CONFIDENCE_THRESHOLD, @@ -71,6 +73,15 @@ export class ToursService { }; } + /** + * `tour_types` knows every key the contract accepts, most of them disabled. + * Checked before the write, so a disabled type is a 400 and not a tour the + * planner cannot show. + */ + private async assertTourTypeEnabled(key: string): Promise { + if (!(await this.tourTypesRepo.isEnabled(key))) throw new BadRequestException('Tour type is not available'); + } + /** All tours (the facet + owning place) for a trip, newest first. */ async listTours(tripId: string): Promise { const tid = toRowId(tripId); @@ -111,6 +122,7 @@ export class ToursService { async createTour(tripId: string, input: TourCreateRequest, socketId?: string): Promise { // The controller's TripAccessGuard already resolved this trip id. const tid = toRowId(tripId)!; + await this.assertTourTypeEnabled(input.tour_type); const start = input.route_geometry[0]; const fields = this.routeFields(input); @@ -144,6 +156,7 @@ export class ToursService { const tid = toRowId(tripId); const pid = toRowId(placeId); if (tid === null || pid === null) throw new NotFoundException('Tour not found'); + await this.assertTourTypeEnabled(input.tour_type); const start = input.route_geometry[0]; const fields = this.routeFields(input); @@ -226,7 +239,6 @@ export class ToursService { difficulty: null, wanderer_ref: null, match_confidence: matchConfidence, - tour_group_id: null, max_hiking_difficulty: 2, planned: 0, has_waypoints: 0, diff --git a/server/tests/helpers/tours-repos.ts b/server/tests/helpers/tours-repos.ts index e9ab4724a..5462b98be 100644 --- a/server/tests/helpers/tours-repos.ts +++ b/server/tests/helpers/tours-repos.ts @@ -2,6 +2,8 @@ import type Database from 'better-sqlite3'; import { sharedTestOrm } from './test-uow'; import { Tours } from '../../src/db/entities/Tours.entity'; import type { ToursRepository } from '../../src/db/repositories/Tours.repository'; +import { TourTypes } from '../../src/db/entities/TourTypes.entity'; +import type { TourTypesRepository } from '../../src/db/repositories/TourTypes.repository'; import { TourWaypoints } from '../../src/db/entities/TourWaypoints.entity'; import type { TourWaypointsRepository } from '../../src/db/repositories/TourWaypoints.repository'; @@ -15,6 +17,10 @@ export function createTestToursRepo(db: Database.Database): Promise t.repo(Tours)); } +export function createTestTourTypesRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(TourTypes)); +} + export function createTestTourWaypointsRepo(db: Database.Database): Promise { return sharedTestOrm(db).then((t) => t.repo(TourWaypoints)); } diff --git a/server/tests/unit/db/repositories/Tours.repository.test.ts b/server/tests/unit/db/repositories/Tours.repository.test.ts index 724134645..02e8c318f 100644 --- a/server/tests/unit/db/repositories/Tours.repository.test.ts +++ b/server/tests/unit/db/repositories/Tours.repository.test.ts @@ -60,12 +60,12 @@ describe('ToursRepository reads', () => { expect(await repo.listForTrip(trip.id)).toEqual([ { place_id: newer.id, name: 'Newer', tour_type: 'hike', distance: null, elevation_gain: null, elevation_loss: null, - duration: null, difficulty: null, wanderer_ref: null, match_confidence: null, tour_group_id: null, + duration: null, difficulty: null, wanderer_ref: null, match_confidence: null, max_hiking_difficulty: 5, planned: 0, has_waypoints: 1, }, { place_id: older.id, name: 'Older', tour_type: 'hike', distance: 5, elevation_gain: null, elevation_loss: null, - duration: null, difficulty: null, wanderer_ref: null, match_confidence: 0.3, tour_group_id: null, + duration: null, difficulty: null, wanderer_ref: null, match_confidence: 0.3, max_hiking_difficulty: 2, planned: 1, has_waypoints: 0, }, ]); @@ -128,7 +128,7 @@ describe('ToursRepository writes', () => { await repo.insertTour({ place_id: place.id, ...METRICS }); expect(await repo.findInTrip(trip.id, place.id)).toEqual({ - place_id: place.id, name: 'Drawn', ...METRICS, difficulty: null, wanderer_ref: null, tour_group_id: null, + place_id: place.id, name: 'Drawn', ...METRICS, difficulty: null, wanderer_ref: null, planned: 0, has_waypoints: 0, }); }); @@ -149,12 +149,12 @@ describe('ToursRepository writes', () => { const { trip, other } = fixture(); const source = createPlace(testDb, trip.id); createTour(testDb, source.id, { created_at: '2026-03-04 05:06:07', distance: 3.5, max_hiking_difficulty: 4 }); - testDb.prepare("UPDATE tours SET difficulty = 'T3', wanderer_ref = 'w-1', tour_group_id = 9 WHERE place_id = ?").run(source.id); + testDb.prepare("UPDATE tours SET difficulty = 'T3', wanderer_ref = 'w-1' WHERE place_id = ?").run(source.id); const rows = await repo.listRowsForTrip(trip.id); expect(rows).toEqual([{ place_id: source.id, tour_type: 'hike', distance: 3.5, elevation_gain: null, elevation_loss: null, duration: null, - difficulty: 'T3', wanderer_ref: 'w-1', match_confidence: null, tour_group_id: 9, created_at: '2026-03-04 05:06:07', + difficulty: 'T3', wanderer_ref: 'w-1', match_confidence: null, created_at: '2026-03-04 05:06:07', max_hiking_difficulty: 4, }]); expect(await repo.listRowsForTrip(other.id)).toEqual([]); diff --git a/server/tests/unit/db/tour-types-migration.test.ts b/server/tests/unit/db/tour-types-migration.test.ts new file mode 100644 index 000000000..1e86a49a0 --- /dev/null +++ b/server/tests/unit/db/tour-types-migration.test.ts @@ -0,0 +1,65 @@ +/** + * `Migration20200101042300_every_tour_type_the_contract_names`: `tour_types` + * gets a row for every key the shared contract accepts, and the never-written + * `tours.tour_group_id` column goes without touching the rows around it. + */ +import { describe, expect, it } from 'vitest'; +import { tourTypeKeySchema } from '@trek/shared'; +import { createMigrationOrm, migrateTo, pendingNames, rawExec, rawQuery } from '../../helpers/migration-step'; +import type { MikroORM } from '@mikro-orm/sqlite'; + +const TARGET = 'Migration20200101042300_every_tour_type_the_contract_names'; + +async function ormBeforeTarget(): Promise { + const orm = await createMigrationOrm(); + const names = await pendingNames(orm); + const idx = names.indexOf(TARGET); + expect(idx).toBeGreaterThan(0); + await migrateTo(orm, names[idx - 1]); + return orm; +} + +describe('Tour types migration', () => { + it('TOURTYPEMIG-001: every contract key has a row, only hike enabled, and the group column is gone', async () => { + const orm = await ormBeforeTarget(); + try { + await rawExec(orm, "INSERT INTO users (id, username, email, password_hash) VALUES (1, 'owner', 'owner@test', 'x')"); + await rawExec(orm, "INSERT INTO trips (id, user_id, title) VALUES (10, 1, 'Alps')"); + await rawExec(orm, "INSERT INTO places (id, trip_id, name) VALUES (100, 10, 'Ridge walk')"); + await rawExec(orm, "INSERT INTO tours (place_id, tour_type, distance, difficulty, wanderer_ref, tour_group_id, max_hiking_difficulty) VALUES (100, 'hike', 12.5, 'T3', 'w-1', 4, 3)"); + + await migrateTo(orm, TARGET); + + const types = await rawQuery<{ key: string; enabled: number; label_key: string }>( + orm, + 'SELECT key, enabled, label_key FROM tour_types ORDER BY sort_order', + ); + expect(types.map((t) => t.key)).toEqual([...tourTypeKeySchema.options]); + expect(types.filter((t) => t.enabled === 1).map((t) => t.key)).toEqual(['hike']); + for (const t of types) expect(t.label_key).toBe(`tourTypes.${t.key}`); + + const columns = await rawQuery<{ name: string }>(orm, "SELECT name FROM pragma_table_info('tours') ORDER BY cid"); + expect(columns.map((c) => c.name)).not.toContain('tour_group_id'); + expect(await rawQuery(orm, 'SELECT place_id, distance, difficulty, wanderer_ref, max_hiking_difficulty FROM tours')).toEqual([ + { place_id: 100, distance: 12.5, difficulty: 'T3', wanderer_ref: 'w-1', max_hiking_difficulty: 3 }, + ]); + } finally { + await orm.close(true); + } + }, 30000); + + it('TOURTYPEMIG-002: a type an instance already defined keeps its own values', async () => { + const orm = await ormBeforeTarget(); + try { + await rawExec(orm, "INSERT INTO tour_types (key, label_key, icon, color, routing_profile, enabled, sort_order) VALUES ('bike', 'tourTypes.bike', 'Bike', '#000000', 'bicycle', 1, 9)"); + + await migrateTo(orm, TARGET); + + expect(await rawQuery(orm, "SELECT color, enabled, sort_order FROM tour_types WHERE key = 'bike'")).toEqual([ + { color: '#000000', enabled: 1, sort_order: 9 }, + ]); + } finally { + await orm.close(true); + } + }, 30000); +}); diff --git a/server/tests/unit/nest/tours.gpx.atomic.test.ts b/server/tests/unit/nest/tours.gpx.atomic.test.ts index 2a19b6dab..509ba7d1d 100644 --- a/server/tests/unit/nest/tours.gpx.atomic.test.ts +++ b/server/tests/unit/nest/tours.gpx.atomic.test.ts @@ -52,7 +52,7 @@ import { createTestJourneysRepo, createTestJourneyContributorsRepo, createTestJourneyTripsRepo, createTestJourneyEntriesRepo, createTestJourneyPhotosRepo, createTestJourneyEntryPhotosRepo, } from '../../helpers/journey-repos'; -import { createTestToursRepo, createTestTourWaypointsRepo } from '../../helpers/tours-repos'; +import { createTestToursRepo, createTestTourTypesRepo, createTestTourWaypointsRepo } from '../../helpers/tours-repos'; const mixedGpx = Buffer.from(` Ignored POI @@ -111,7 +111,7 @@ beforeAll(async () => { uow = await createTestUnitOfWork(testDb); places = await makePlacesService(); tours = new ToursService( - uow, places, await createTestToursRepo(testDb), await createTestTourWaypointsRepo(testDb), await createTestPlacesRepo(testDb), + uow, places, await createTestToursRepo(testDb), await createTestTourTypesRepo(testDb), await createTestTourWaypointsRepo(testDb), await createTestPlacesRepo(testDb), ); }); diff --git a/server/tests/unit/nest/tours.permissions.boundaries.mock-only.test.ts b/server/tests/unit/nest/tours.permissions.boundaries.mock-only.test.ts index b515e4ed8..f2c17a66f 100644 --- a/server/tests/unit/nest/tours.permissions.boundaries.mock-only.test.ts +++ b/server/tests/unit/nest/tours.permissions.boundaries.mock-only.test.ts @@ -49,6 +49,7 @@ import type { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env import type { StorageService } from '../../../src/nest/storage/storage.service'; import type { UnitOfWork } from '../../../src/nest/database/unit-of-work'; import type { PlacesRepository } from '../../../src/db/repositories/Places.repository'; +import type { TourTypesRepository } from '../../../src/db/repositories/TourTypes.repository'; import type { ToursRepository } from '../../../src/db/repositories/Tours.repository'; import type { TourWaypointsRepository } from '../../../src/db/repositories/TourWaypoints.repository'; import type { User } from '../../../src/types'; @@ -203,7 +204,7 @@ function tourRepositoryFixture(ownerTripId: number) { const row = { place_id: 42, name: 'Ridge walk', tour_type: 'hike', distance: 3, elevation_gain: 30, elevation_loss: 0, duration: null, difficulty: null, - wanderer_ref: null, match_confidence: 1, tour_group_id: null, + wanderer_ref: null, match_confidence: 1, max_hiking_difficulty: 2, planned: 0, has_waypoints: 1, }; const uow = { transactional: vi.fn(async (fn: () => Promise): Promise => await fn()) }; @@ -226,6 +227,7 @@ function tourRepositoryFixture(ownerTripId: number) { uow as unknown as UnitOfWork, places as unknown as PlacesService, toursRepo as unknown as ToursRepository, + { isEnabled: vi.fn(async () => true) } as unknown as TourTypesRepository, waypointsRepo as unknown as TourWaypointsRepository, placesRepo as unknown as PlacesRepository, ); diff --git a/server/tests/unit/nest/tours.service.permissions.test.ts b/server/tests/unit/nest/tours.service.permissions.test.ts index 9da8adeeb..1e301d55a 100644 --- a/server/tests/unit/nest/tours.service.permissions.test.ts +++ b/server/tests/unit/nest/tours.service.permissions.test.ts @@ -31,6 +31,7 @@ import { tourCreateRequestSchema, type TourCreateRequest } from '@trek/shared'; import type { UnitOfWork } from '../../../src/nest/database/unit-of-work'; import type { PlacesService } from '../../../src/nest/places/places.service'; import type { PlacesRepository } from '../../../src/db/repositories/Places.repository'; +import type { TourTypesRepository } from '../../../src/db/repositories/TourTypes.repository'; import type { ToursRepository } from '../../../src/db/repositories/Tours.repository'; import type { TourWaypointsRepository } from '../../../src/db/repositories/TourWaypoints.repository'; @@ -50,7 +51,7 @@ function makeService() { const row = { place_id: 42, name: request.name, tour_type: 'hike', distance: 3, elevation_gain: 50, elevation_loss: 20, duration: 60, difficulty: null, - wanderer_ref: null, match_confidence: 1, tour_group_id: null, + wanderer_ref: null, match_confidence: 1, max_hiking_difficulty: 2, planned: 0, has_waypoints: 1, }; const place = { id: 42, trip_id: 7 }; @@ -88,6 +89,7 @@ function makeService() { uow as unknown as UnitOfWork, places as unknown as PlacesService, toursRepo as unknown as ToursRepository, + { isEnabled: vi.fn(async () => true) } as unknown as TourTypesRepository, waypointsRepo as unknown as TourWaypointsRepository, placesRepo as unknown as PlacesRepository, ); diff --git a/server/tests/unit/nest/tours.service.test.ts b/server/tests/unit/nest/tours.service.test.ts index f250a6407..62b258b0f 100644 --- a/server/tests/unit/nest/tours.service.test.ts +++ b/server/tests/unit/nest/tours.service.test.ts @@ -8,7 +8,7 @@ * real PlacesService, is covered by tours.gpx.atomic.test.ts. */ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; -import { Logger, NotFoundException } from '@nestjs/common'; +import { BadRequestException, Logger, NotFoundException } from '@nestjs/common'; vi.mock('../../../src/db/database', async () => { const { createSnapshotTestDb, buildDbMock } = await import('../../helpers/db-mock'); @@ -27,7 +27,7 @@ import type { PlacesService } from '../../../src/nest/places/places.service'; import { resetTestDb } from '../../helpers/test-db'; import { createPlace, createTrip, createUser } from '../../helpers/factories'; import { createTestPlacesRepo, createTestUnitOfWork, sharedTestOrm } from '../../helpers/test-uow'; -import { createTestToursRepo, createTestTourWaypointsRepo, createTour } from '../../helpers/tours-repos'; +import { createTestToursRepo, createTestTourTypesRepo, createTestTourWaypointsRepo, createTour } from '../../helpers/tours-repos'; const request: TourCreateRequest = { name: 'Ridge walk', @@ -59,6 +59,7 @@ beforeAll(async () => { await createTestUnitOfWork(testDb), { broadcast } as unknown as PlacesService, await createTestToursRepo(testDb), + await createTestTourTypesRepo(testDb), await createTestTourWaypointsRepo(testDb), await createTestPlacesRepo(testDb), ); @@ -133,6 +134,30 @@ describe('ToursService planner creation', () => { expect(result.tour.duration).toBeNull(); }); + it('TOURS-SVC-010: a type the catalogue holds but has not enabled is a 400 without a write', async () => { + // The contract only lets `hike` through today; this pins what happens the + // day it lets more keys in before the planner offers them. + const bike = { ...request, tour_type: 'bike' } as unknown as typeof request; + + await expect(service.createTour(tripId, bike)).rejects.toBeInstanceOf(BadRequestException); + + expect(count('places')).toBe(0); + expect(count('tours')).toBe(0); + expect(broadcast).not.toHaveBeenCalled(); + }); + + it('TOURS-SVC-011: an edit to a disabled type is a 400 and leaves the saved tour alone', async () => { + const created = await service.createTour(tripId, request); + broadcast.mockReset(); + const bike = { ...request, name: 'Renamed', tour_type: 'bike' } as unknown as typeof request; + + await expect(service.updateTour(tripId, String(created.tour.place_id), bike)).rejects.toBeInstanceOf(BadRequestException); + + expect(testDb.prepare('SELECT t.tour_type, p.name FROM tours t JOIN places p ON p.id = t.place_id').get()) + .toEqual({ tour_type: 'hike', name: 'Ridge walk' }); + expect(broadcast).not.toHaveBeenCalled(); + }); + it('keeps a committed create successful when Tours invalidation publication fails', async () => { const warn = vi.spyOn(Logger.prototype, 'warn').mockImplementation(() => {}); broadcast.mockImplementation((_tripId: string, event: string) => { diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index 6e889c28b..b78f336cf 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -1900,7 +1900,7 @@ describe('copy: Tours (#2586)', () => { const place = createPlace(testDb, tripId, { name }); testDb.prepare("UPDATE places SET transport_mode = 'walking', route_geometry = '[[47,11],[47.2,11.2]]' WHERE id = ?").run(place.id); createTour(testDb, place.id, { distance: 12.5, match_confidence: 0.9, max_hiking_difficulty: 4, created_at: '2025-01-02 03:04:05' }); - testDb.prepare("UPDATE tours SET elevation_gain = 800, elevation_loss = 790, duration = 5.5, difficulty = 'T3', wanderer_ref = 'w-1', tour_group_id = 7 WHERE place_id = ?").run(place.id); + testDb.prepare("UPDATE tours SET elevation_gain = 800, elevation_loss = 790, duration = 5.5, difficulty = 'T3', wanderer_ref = 'w-1' WHERE place_id = ?").run(place.id); const insert = testDb.prepare('INSERT INTO tour_waypoints (place_id, lat, lng, role, sequence) VALUES (?, ?, ?, ?, ?)'); insert.run(place.id, 47, 11, 'start', 0); insert.run(place.id, 47.1, 11.1, 'via', 1); @@ -1927,7 +1927,7 @@ describe('copy: Tours (#2586)', () => { const newPlain = testDb.prepare("SELECT id FROM places WHERE trip_id = ? AND name = 'Plain stop'").get(newTripId) as { id: number }; expect(newTour.id).not.toBe(tour.id); expect(tourColumns(newTour.id)).toEqual(tourColumns(tour.id)); - expect(tourColumns(newTour.id)).toMatchObject({ tour_type: 'hike', created_at: '2025-01-02 03:04:05', max_hiking_difficulty: 4, tour_group_id: 7 }); + expect(tourColumns(newTour.id)).toMatchObject({ tour_type: 'hike', created_at: '2025-01-02 03:04:05', max_hiking_difficulty: 4 }); expect(waypoints(newTour.id)).toEqual(waypoints(tour.id)); expect(waypoints(newTour.id)).toHaveLength(3); // The plain place stays plain, and the source keeps its own rows. diff --git a/shared/src/tours/tours.schema.ts b/shared/src/tours/tours.schema.ts index 6f41b641c..ab7565485 100644 --- a/shared/src/tours/tours.schema.ts +++ b/shared/src/tours/tours.schema.ts @@ -42,7 +42,6 @@ export const tourSchema = z.object({ // Nullable reference to a tour in wanderer. wanderer_ref: z.string().nullable(), match_confidence: z.number().min(0).max(1).nullable(), - tour_group_id: z.number().int().nullable(), max_hiking_difficulty: tourMaxHikingDifficultySchema, }); export type Tour = z.infer;