From 6e4fda897f8532900e00f559bbaa6c6cdf523407 Mon Sep 17 00:00:00 2001 From: Maurice Date: Tue, 18 Aug 2026 21:07:47 +0200 Subject: [PATCH] refactor(journey): stop pulling trip photos into journeys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Photos live in journeys (#1614). Linking a trip used to copy its trip_photos rows into the journey gallery, which is how a photo one member had chosen not to share could reach a journey at all — PR #1967 filtered that at the door, and this removes the door. Nothing is lost by it: the trip-photo surface lost its UI in 3.1.0, so nothing writes to trip_photos on an install newer than that, and the client calls none of its routes. The table and those routes stay for one more release rather than being dropped in an append-only migration; the read paths keep working for installs that still hold rows. --- .../nest/journey/journey-domain.service.ts | 37 +++-------------- .../unit/nest/journey-domain.service.test.ts | 41 +++++++------------ 2 files changed, 21 insertions(+), 57 deletions(-) diff --git a/server/src/nest/journey/journey-domain.service.ts b/server/src/nest/journey/journey-domain.service.ts index 6b7f74bdb..e6a326343 100644 --- a/server/src/nest/journey/journey-domain.service.ts +++ b/server/src/nest/journey/journey-domain.service.ts @@ -359,8 +359,12 @@ export class JourneyDomainService { // sync skeleton entries for all places in this trip this.syncTripPlaces(journeyId, tripId, userId); - // import existing trip photos (Immich/Synology) with sharing settings - this.syncTripPhotos(journeyId, tripId); + // Trip photos are deliberately NOT pulled in any more (#1614). Photos live in + // journeys now: the trip-photo surface lost its UI in 3.1.0, nothing writes to + // it on an install newer than that, and copying rows between the two was what + // let a photo one member had chosen not to share reach a journey at all. The + // table and its (unreferenced) routes stay for one more release rather than + // being dropped in an append-only migration. this.broadcastJourneyEvent(journeyId, 'journey:trip:synced', { tripId }); return true; } @@ -445,35 +449,6 @@ export class JourneyDomainService { } } - // import trip_photos into journey gallery when a trip is linked - // - // Only the ones the owner actually shared with the trip. A photo left unshared is - // denied on the trip path (memories-access checks tp.shared = 1), and copying it - // here handed it to every journey contributor and, with an open gallery, to - // anonymous visitors of the share link — journey_photos.shared cannot gate that - // afterwards, because a direct upload writes 0 into the same column. - private syncTripPhotos(journeyId: number, tripId: number) { - const tripPhotos = this.db - .prepare('SELECT tp.photo_id, tp.shared FROM trip_photos tp WHERE tp.trip_id = ? AND tp.shared = 1') - .all(tripId) as { photo_id: number; shared: number }[]; - if (!tripPhotos.length) return; - - const now = this.ts(); - const maxOrderRow = this.db - .prepare('SELECT MAX(sort_order) as m FROM journey_photos WHERE journey_id = ?') - .get(journeyId) as { m: number | null }; - let nextOrder = (maxOrderRow?.m ?? -1) + 1; - - for (const tp of tripPhotos) { - this.db.prepare( - ` - INSERT OR IGNORE INTO journey_photos (journey_id, photo_id, shared, sort_order, created_at) - VALUES (?, ?, ?, ?, ?) - `, - ).run(journeyId, tp.photo_id, tp.shared, nextOrder++, now); - } - } - // called when a trip place is created onPlaceCreated(tripId: number, placeId: number) { const links = this.db.prepare('SELECT journey_id FROM journey_trips WHERE trip_id = ?').all(tripId) as { diff --git a/server/tests/unit/nest/journey-domain.service.test.ts b/server/tests/unit/nest/journey-domain.service.test.ts index 9853a5ebc..a9c7f3373 100644 --- a/server/tests/unit/nest/journey-domain.service.test.ts +++ b/server/tests/unit/nest/journey-domain.service.test.ts @@ -1411,7 +1411,11 @@ describe('Edge cases', () => { expect(JSON.parse(raw.pros_cons)).toEqual({ pros: ['Fun'], cons: [] }); }); - it('JOURNEY-SVC-086: addTripToJourney syncs trip photos when present', () => { + // #1614 — photos live in journeys now. Linking a trip used to copy its + // trip_photos into the gallery; that surface lost its UI in 3.1.0, nothing + // writes to it on a newer install, and the copy was how a photo one member had + // chosen not to share could reach a journey at all. + it('JOURNEY-SVC-086: addTripToJourney no longer pulls trip photos into the gallery', () => { const { user } = createUser(testDb); const journey = createJourney(testDb, user.id); const trip = createTrip(testDb, user.id, { @@ -1421,16 +1425,10 @@ describe('Edge cases', () => { }); addTripPhoto(testDb, trip.id, user.id, 'immich-photo-1', 'immich', { shared: true }); - svc.addTripToJourney(journey.id, trip.id, user.id); + expect(svc.addTripToJourney(journey.id, trip.id, user.id)).toBe(true); - // Trip photos now go straight into the journey gallery (no wrapper entry). - const photos = testDb.prepare(` - SELECT jp.*, tkp.asset_id FROM journey_photos jp - JOIN trek_photos tkp ON tkp.id = jp.photo_id - WHERE jp.journey_id = ? - `).all(journey.id); - expect(photos.length).toBe(1); - expect((photos[0] as any).asset_id).toBe('immich-photo-1'); + const photos = testDb.prepare('SELECT 1 FROM journey_photos WHERE journey_id = ?').all(journey.id); + expect(photos).toHaveLength(0); }); it('JOURNEY-SVC-087: removeTripFromJourney detaches filled entries, deletes skeletons', () => { @@ -2106,28 +2104,19 @@ describe('addTripToJourney guards', () => { expect(svc.addTripToJourney(journey.id, trip.id, helper.id)).toBe(true); }); - it('JOURNEY-SVC-102: only trip photos the owner shared are copied into the gallery', () => { + it('JOURNEY-SVC-102: not even a shared trip photo is copied any more', () => { const { user } = createUser(testDb); const journey = createJourney(testDb, user.id, { title: 'Photo journey' }); const trip = createTrip(testDb, user.id, { title: 'Photo trip' }); - const mk = (assetId: string) => { - const r = testDb.prepare( - "INSERT INTO trek_photos (provider, asset_id, owner_id, media_type) VALUES ('immich', ?, ?, 'image')", - ).run(assetId, user.id); - return Number(r.lastInsertRowid); - }; - const sharedPhoto = mk('shared-asset'); - const privatePhoto = mk('private-asset'); - const ins = testDb.prepare('INSERT INTO trip_photos (trip_id, user_id, photo_id, shared) VALUES (?, ?, ?, ?)'); - ins.run(trip.id, user.id, sharedPhoto, 1); - ins.run(trip.id, user.id, privatePhoto, 0); + const r = testDb.prepare( + "INSERT INTO trek_photos (provider, asset_id, owner_id, media_type) VALUES ('immich', 'shared-asset', ?, 'image')", + ).run(user.id); + testDb.prepare('INSERT INTO trip_photos (trip_id, user_id, photo_id, shared) VALUES (?, ?, ?, 1)') + .run(trip.id, user.id, Number(r.lastInsertRowid)); svc.addTripToJourney(journey.id, trip.id, user.id); - const copied = testDb - .prepare('SELECT photo_id FROM journey_photos WHERE journey_id = ? ORDER BY photo_id') - .all(journey.id) as { photo_id: number }[]; - expect(copied.map(r => r.photo_id)).toEqual([sharedPhoto]); + expect(testDb.prepare('SELECT 1 FROM journey_photos WHERE journey_id = ?').all(journey.id)).toHaveLength(0); }); });