From b339ef3f0487ea3536593b29890b0139acaac232 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 29 Aug 2026 21:04:54 +0200 Subject: [PATCH] fix(days): keep a place's stop type on both assignment loaders Three read paths built the same nested place object by hand, and two of them left `stop_type` out of it while their SQL was already selecting the column. The effect was visible: a petrol station added along a route drew correctly for a moment from the optimistic write, then the server's answer replaced it with a place that had no kind, and the road-trip rail redrew it as an ordinary numbered destination. `getAssignmentWithPlace` now goes through `formatAssignmentWithPlace` like the list path already did, which removes the third copy rather than adding the missing field to it. The two queries in the days service gain the column, and its hand-built shape gains the field it cannot yet be spared. --- .../nest/assignments/assignments.service.ts | 56 ++++--------------- server/src/nest/days/days.service.ts | 8 ++- server/tests/unit/nest/days.service.test.ts | 18 ++++++ 3 files changed, 34 insertions(+), 48 deletions(-) diff --git a/server/src/nest/assignments/assignments.service.ts b/server/src/nest/assignments/assignments.service.ts index 51bd1503b..155469b29 100644 --- a/server/src/nest/assignments/assignments.service.ts +++ b/server/src/nest/assignments/assignments.service.ts @@ -5,7 +5,7 @@ import { DatabaseService, type TripAccess } from '../database/database.service'; import { PermissionsService } from '../permissions/permissions.service'; import { QueryHelpersService } from '../query-helpers/query-helpers.service'; import { formatAssignmentWithPlace } from '../common/rowShape'; -import type { AssignmentRow, DayAssignment, User } from '../../types'; +import type { AssignmentRow, DayAssignment, User, Participant } from '../../types'; import { JourneyDomainService } from '../journey/journey-domain.service'; type Trip = TripAccess; @@ -60,7 +60,7 @@ export class AssignmentsService { COALESCE(da.assignment_time, p.place_time) as place_time, COALESCE(da.assignment_end_time, p.end_time) as end_time, p.duration_minutes, p.notes as place_notes, - p.image_url, p.transport_mode, p.google_place_id, p.google_ftid, p.osm_id, p.website, p.phone, + p.image_url, p.transport_mode, p.google_place_id, p.google_ftid, p.osm_id, p.website, p.phone, p.stop_type, c.name as category_name, c.color as category_color, c.icon as category_icon FROM day_assignments da JOIN places p ON da.place_id = p.id @@ -74,55 +74,19 @@ export class AssignmentsService { // one wire shape regardless of which read path produced it. const tags = this.queryHelpers.loadTagsByPlaceIds([a.place_id], { compact: true })[a.place_id] || []; - const participants = this.dbs.all(` + const participants = this.dbs.all(` SELECT ap.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar FROM assignment_participants ap JOIN users u ON ap.user_id = u.id WHERE ap.assignment_id = ? `, a.id); - return { - id: a.id, - day_id: a.day_id, - place_id: a.place_id, - order_index: a.order_index, - notes: a.notes, - assignment_time: a.assignment_time ?? null, - assignment_end_time: a.assignment_end_time ?? null, - leg_transport_mode: a.leg_transport_mode ?? null, - incoming_leg_transport_mode: a.incoming_leg_transport_mode ?? null, - participants, - created_at: a.created_at, - place: { - id: a.place_id, - name: a.place_name, - description: a.place_description, - lat: a.lat, - lng: a.lng, - address: a.address, - category_id: a.category_id, - price: a.price, - currency: a.place_currency, - place_time: a.place_time, - end_time: a.end_time, - duration_minutes: a.duration_minutes, - notes: a.place_notes, - image_url: a.image_url, - transport_mode: a.transport_mode, - google_place_id: a.google_place_id, - google_ftid: a.google_ftid, - osm_id: a.osm_id, - website: a.website, - phone: a.phone, - category: a.category_id ? { - id: a.category_id, - name: a.category_name, - color: a.category_color, - icon: a.category_icon, - } : null, - tags, - } - }; + // The same shaper the list path uses. It was spelled out here as a third hand-kept + // copy of the place shape, and the copy silently dropped `stop_type`: the optimistic + // row the client had drawn as a fuel stop was replaced, a beat later, by this answer + // without it — so a petrol station turned into an ordinary numbered place while you + // watched. + return formatAssignmentWithPlace(a, tags, participants); } listDayAssignments(dayId: string | number) { @@ -132,7 +96,7 @@ export class AssignmentsService { COALESCE(da.assignment_time, p.place_time) as place_time, COALESCE(da.assignment_end_time, p.end_time) as end_time, p.duration_minutes, p.notes as place_notes, - p.image_url, p.transport_mode, p.google_place_id, p.google_ftid, p.osm_id, p.website, p.phone, + p.image_url, p.transport_mode, p.google_place_id, p.google_ftid, p.osm_id, p.website, p.phone, p.stop_type, c.name as category_name, c.color as category_color, c.icon as category_icon FROM day_assignments da JOIN places p ON da.place_id = p.id diff --git a/server/src/nest/days/days.service.ts b/server/src/nest/days/days.service.ts index ce00d1b28..70479427a 100644 --- a/server/src/nest/days/days.service.ts +++ b/server/src/nest/days/days.service.ts @@ -91,7 +91,7 @@ export class DaysService { COALESCE(da.assignment_time, p.place_time) as place_time, COALESCE(da.assignment_end_time, p.end_time) as end_time, p.duration_minutes, p.notes as place_notes, - p.image_url, p.transport_mode, p.google_place_id, p.google_ftid, p.osm_id, p.website, p.phone, + p.image_url, p.transport_mode, p.google_place_id, p.google_ftid, p.osm_id, p.website, p.phone, p.stop_type, c.name as category_name, c.color as category_color, c.icon as category_icon FROM day_assignments da JOIN places p ON da.place_id = p.id @@ -135,6 +135,10 @@ export class DaysService { osm_id: a.osm_id, website: a.website, phone: a.phone, + // Hand-built here rather than through `formatAssignmentWithPlace`, so every + // place column has to be repeated twice — which is how the road-trip kind went + // missing and made a petrol station render as an ordinary numbered stop. + stop_type: a.stop_type ?? null, category: a.category_id ? { id: a.category_id, name: a.category_name, @@ -167,7 +171,7 @@ export class DaysService { COALESCE(da.assignment_time, p.place_time) as place_time, COALESCE(da.assignment_end_time, p.end_time) as end_time, p.duration_minutes, p.notes as place_notes, - p.image_url, p.transport_mode, p.google_place_id, p.google_ftid, p.osm_id, p.website, p.phone, + p.image_url, p.transport_mode, p.google_place_id, p.google_ftid, p.osm_id, p.website, p.phone, p.stop_type, c.name as category_name, c.color as category_color, c.icon as category_icon FROM day_assignments da JOIN places p ON da.place_id = p.id diff --git a/server/tests/unit/nest/days.service.test.ts b/server/tests/unit/nest/days.service.test.ts index 66e39f618..1a3f08fec 100644 --- a/server/tests/unit/nest/days.service.test.ts +++ b/server/tests/unit/nest/days.service.test.ts @@ -124,6 +124,24 @@ describe('getAssignmentsForDay', () => { expect(assignments[0].place.lat).toBe(48.8); }); + it('DAY-SVC-029 — a road-trip stop keeps its kind on both loaders', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id) as any; + const place = createPlace(testDb, trip.id, { name: 'Esso', lat: 53.8, lng: 10.4 }) as any; + testDb.prepare('UPDATE places SET stop_type = ? WHERE id = ?').run('fuel', place.id); + createDayAssignment(testDb, day.id, place.id, { order_index: 0 }); + + // Both queries build the same place shape by hand, and only one of them used to + // fetch the column — so the rail drew a petrol station as an ordinary numbered stop + // while the database had known it was fuel all along. + const single = svc.getAssignmentsForDay(day.id) as any[]; + expect(single[0].place.stop_type).toBe('fuel'); + + const listed = svc.list(trip.id) as any; + expect(listed.days[0].assignments[0].place.stop_type).toBe('fuel'); + }); + it('DAY-SVC-005 — assignment includes tags array (empty when place has none)', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id);