From 359c3afc211ea312871b745f0a6e28854967d5e8 Mon Sep 17 00:00:00 2001 From: Maurice Date: Tue, 18 Aug 2026 20:16:39 +0200 Subject: [PATCH] feat(photos): ask the provider where and when, and stop the album path dropping it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second half of the capture-metadata foundation (#1614). The album paths were the asymmetry: Immich's search has always carried exifInfo.latitude/longitude, but the album mapping quietly left them out, and the Synology album listing never asked for the gps block at all. So a photo picked out of an album could never be placed on a map, and the distance sort that PR #1632 added silently had nothing to sort by. Both now carry the pair. For the add itself the client only sends an asset id. Widening that contract would have meant trusting the client for something the provider can be asked for directly, and would have left the MCP path and every already-imported photo without it — so PhotoCaptureBackfillService asks afterwards instead, detached, skipping rows that already know both so an album import is not one provider call per photo. A slow or unreachable provider cannot hold up the add, and a photo without capture metadata stays the normal case. --- server/src/nest/journey/journey.controller.ts | 20 +- server/src/nest/memories/immich.service.ts | 5 + server/src/nest/memories/memories.module.ts | 4 +- .../photo-capture-backfill.service.ts | 53 ++++++ server/src/nest/memories/synology.service.ts | 8 +- server/src/types.ts | 5 + .../unit/nest/journey.controller.test.ts | 179 +++++++++--------- .../photo-capture-backfill.service.test.ts | 88 +++++++++ 8 files changed, 271 insertions(+), 91 deletions(-) create mode 100644 server/src/nest/memories/photo-capture-backfill.service.ts create mode 100644 server/tests/unit/nest/photo-capture-backfill.service.test.ts diff --git a/server/src/nest/journey/journey.controller.ts b/server/src/nest/journey/journey.controller.ts index b14c35169..27f3486e9 100644 --- a/server/src/nest/journey/journey.controller.ts +++ b/server/src/nest/journey/journey.controller.ts @@ -22,6 +22,7 @@ import fs from 'node:fs'; import crypto from 'node:crypto'; import type { User } from '../../types'; import { JourneyService } from './journey.service'; +import { PhotoCaptureBackfillService } from '../memories/photo-capture-backfill.service'; import { AddonGuard } from '../addons/addon.guard'; import { RequireAddon } from '../addons/require-addon.decorator'; import { ADDON_IDS } from '../../addons'; @@ -120,7 +121,20 @@ const VIDEO_UPLOAD = { @UseGuards(AddonGuard, JwtAuthGuard) @RequireAddon(ADDON_IDS.JOURNEY, 'Journey') export class JourneyController { - constructor(private readonly journey: JourneyService) {} + constructor( + private readonly journey: JourneyService, + private readonly captureBackfill: PhotoCaptureBackfillService, + ) {} + + // The add call carries only an asset id, so when and where the picture was taken + // are fetched from the provider afterwards rather than trusted from the client + // (#1614). Detached: a slow or unreachable provider must not hold up the add. + private backfillCapture(photos: unknown[], userId: number): void { + const ids = photos + .map(p => (p as { photo_id?: number } | null)?.photo_id) + .filter((id): id is number => typeof id === 'number'); + this.captureBackfill.schedule(ids, userId); + } // ── Static prefix routes (before /:id) ────────────────────────────────── @Get() @@ -211,6 +225,7 @@ export class JourneyController { const photo = this.journey.addProviderPhoto(Number(entryId), user.id, String(body.provider), String(id), body.caption as string | undefined, pp, mt); if (photo) added.push(photo); }); + this.backfillCapture(added, user.id); return { photos: added, added: added.length }; } if (!body.provider || !body.asset_id) { @@ -220,6 +235,7 @@ export class JourneyController { if (!photo) { throw new HttpException({ error: 'Not allowed or duplicate' }, 403); } + this.backfillCapture([photo], user.id); return photo; } @@ -325,6 +341,7 @@ export class JourneyController { const photo = this.journey.addProviderPhotoToGallery(Number(id), user.id, String(body.provider), String(aid), undefined, pp, mt); if (photo) added.push(photo); }); + this.backfillCapture(added, user.id); return { photos: added, added: added.length }; } if (!body.provider || !body.asset_id) { @@ -334,6 +351,7 @@ export class JourneyController { if (!photo) { throw new HttpException({ error: 'Not allowed or duplicate' }, 403); } + this.backfillCapture([photo], user.id); return photo; } diff --git a/server/src/nest/memories/immich.service.ts b/server/src/nest/memories/immich.service.ts index c52f71449..db6f5392a 100644 --- a/server/src/nest/memories/immich.service.ts +++ b/server/src/nest/memories/immich.service.ts @@ -519,6 +519,11 @@ export class ImmichService { takenAt: a.fileCreatedAt || a.createdAt, city: a.exifInfo?.city || null, country: a.exifInfo?.country || null, + // The search path has always carried these; dropping them here meant a + // photo picked out of an album could never be placed on a map, and the + // distance sort silently had nothing to sort by (#1614). + lat: typeof a.exifInfo?.latitude === 'number' ? a.exifInfo.latitude : null, + lng: typeof a.exifInfo?.longitude === 'number' ? a.exifInfo.longitude : null, mediaType: a.type === 'VIDEO' ? 'video' : 'image', })); return { assets }; diff --git a/server/src/nest/memories/memories.module.ts b/server/src/nest/memories/memories.module.ts index 3e7e39d8f..cc0be4c77 100644 --- a/server/src/nest/memories/memories.module.ts +++ b/server/src/nest/memories/memories.module.ts @@ -5,6 +5,7 @@ import { ImmichService } from './immich.service'; import { SynologyService } from './synology.service'; import { UnifiedMemoriesService } from './unified-memories.service'; import { PhotoResolverService } from './photo-resolver.service'; +import { PhotoCaptureBackfillService } from './photo-capture-backfill.service'; import { ThumbnailService } from './thumbnail.service'; import { TrekPhotoCacheService } from './trek-photo-cache.service'; import { TrekPhotoCacheJob } from './trek-photo-cache.job'; @@ -52,6 +53,7 @@ import { NotificationsModule } from '../notifications/notifications.module'; SynologyService, UnifiedMemoriesService, PhotoResolverService, + PhotoCaptureBackfillService, ThumbnailService, TrekPhotoCacheService, TrekPhotoCacheJob, @@ -64,6 +66,6 @@ import { NotificationsModule } from '../notifications/notifications.module'; inject: [ImmichPhotoProvider, SynologyPhotoProvider], }, ], - exports: [MemoriesAccessService, PhotoResolverService, ImmichService, SynologyService, PhotoProviderRegistry], + exports: [MemoriesAccessService, PhotoResolverService, PhotoCaptureBackfillService, ImmichService, SynologyService, PhotoProviderRegistry], }) export class MemoriesModule {} diff --git a/server/src/nest/memories/photo-capture-backfill.service.ts b/server/src/nest/memories/photo-capture-backfill.service.ts new file mode 100644 index 000000000..3d4184514 --- /dev/null +++ b/server/src/nest/memories/photo-capture-backfill.service.ts @@ -0,0 +1,53 @@ +import { Injectable } from '@nestjs/common'; +import { PhotoResolverService } from './photo-resolver.service'; +import { TrekPhotosRepository } from '../photos/trek-photos.repository'; + +/** + * Ask the provider when and where a photo was taken, and record it (#1614). + * + * The picker already sees these values, but the add call carries only the asset + * id — widening that contract would have meant trusting the client for something + * the provider can be asked for directly, and would have left the MCP path, the + * album sync and every already-imported photo without them. + * + * Runs detached and never throws: a provider that is slow, unreachable or simply + * does not know must not fail the add the user is waiting on. A photo without + * capture metadata is the normal case, not an error — it just will not appear on + * the map. + */ +@Injectable() +export class PhotoCaptureBackfillService { + constructor( + private readonly resolver: PhotoResolverService, + private readonly photos: TrekPhotosRepository, + ) {} + + /** Fire-and-forget for a batch that was just added. */ + schedule(trekPhotoIds: number[], userId: number): void { + if (!trekPhotoIds.length) return; + void this.run(trekPhotoIds, userId); + } + + /** Awaitable form, so tests do not have to chase a floating promise. */ + async run(trekPhotoIds: number[], userId: number): Promise { + for (const id of trekPhotoIds) { + try { + const photo = this.photos.resolve(id); + // A row that already knows both has nothing to gain, and a provider call + // per photo is the expensive part of an album import. + if (!photo || (photo.taken_at && photo.lat != null && photo.lng != null)) continue; + + const info = await this.resolver.getPhotoInfo(id, userId); + if (!info.success) continue; + + this.photos.recordCaptureMetadata(id, { + takenAt: info.data.takenAt ?? null, + lat: info.data.lat ?? null, + lng: info.data.lng ?? null, + }); + } catch (err) { + console.error(`[Photos] capture backfill failed for ${id}:`, err instanceof Error ? err.message : err); + } + } + } +} diff --git a/server/src/nest/memories/synology.service.ts b/server/src/nest/memories/synology.service.ts index d7c61f903..312ce0d62 100644 --- a/server/src/nest/memories/synology.service.ts +++ b/server/src/nest/memories/synology.service.ts @@ -507,8 +507,10 @@ export class SynologyService { while (true) { const params: ApiCallParams = passphrase - ? { api: 'SYNO.Foto.Browse.Item', method: 'list', version: 1, passphrase, offset, limit: pageSize, additional: ['thumbnail'] } - : { api: 'SYNO.Foto.Browse.Item', method: 'list', version: 1, album_id: Number(albumId), offset, limit: pageSize, additional: ['thumbnail'] }; + // 'gps' costs nothing extra on the same call and is the only way a + // photo picked from an album can ever reach the map (#1614). + ? { api: 'SYNO.Foto.Browse.Item', method: 'list', version: 1, passphrase, offset, limit: pageSize, additional: ['thumbnail', 'gps'] } + : { api: 'SYNO.Foto.Browse.Item', method: 'list', version: 1, album_id: Number(albumId), offset, limit: pageSize, additional: ['thumbnail', 'gps'] }; const result = await this._requestSynologyApi<{ list: SynologyPhotoItem[] }>(userId, params); if (!result.success) return result as ServiceResult; const items = result.data.list || []; @@ -520,6 +522,8 @@ export class SynologyService { const assets = allItems.map(item => ({ id: String(item.additional?.thumbnail?.cache_key || item.id || ''), takenAt: item.time ? new Date(item.time * 1000).toISOString() : '', + lat: typeof item.additional?.gps?.latitude === 'number' ? item.additional.gps.latitude : null, + lng: typeof item.additional?.gps?.longitude === 'number' ? item.additional.gps.longitude : null, })).filter(a => a.id); return success({ assets, total: assets.length, hasMore: false }); diff --git a/server/src/types.ts b/server/src/types.ts index 4187e4622..7990c3717 100644 --- a/server/src/types.ts +++ b/server/src/types.ts @@ -398,6 +398,11 @@ export interface TrekPhoto { media_type?: string | null; /** Optional video duration in milliseconds. */ duration_ms?: number | null; + /** When the picture was taken, as the provider or its EXIF reported it (#1614). */ + taken_at?: string | null; + /** Capture coordinates. Stored as a pair or not at all. */ + lat?: number | null; + lng?: number | null; created_at: string; } diff --git a/server/tests/unit/nest/journey.controller.test.ts b/server/tests/unit/nest/journey.controller.test.ts index 194179dc4..026c315c3 100644 --- a/server/tests/unit/nest/journey.controller.test.ts +++ b/server/tests/unit/nest/journey.controller.test.ts @@ -5,6 +5,7 @@ import path from 'node:path'; import fs from 'node:fs'; import { JourneyController } from '../../../src/nest/journey/journey.controller'; +import type { PhotoCaptureBackfillService } from '../../../src/nest/memories/photo-capture-backfill.service'; import { JourneyPublicController } from '../../../src/nest/journey/journey-public.controller'; import type { JourneyService } from '../../../src/nest/journey/journey.service'; import type { User } from '../../../src/types'; @@ -15,6 +16,10 @@ function svc(o: Partial = {}): JourneyService { return { journeyAddonEnabled: vi.fn().mockReturnValue(true), ...o } as unknown as JourneyService; } +// The capture backfill is detached and irrelevant to every case here — it only has +// to exist so the constructor can call it. +const captureBackfillStub = { schedule: vi.fn(), run: vi.fn() } as unknown as PhotoCaptureBackfillService; + function thrown(fn: () => unknown): { status: number; body: unknown } { try { fn(); } catch (err) { expect(err).toBeInstanceOf(HttpException); @@ -36,127 +41,127 @@ beforeEach(() => vi.clearAllMocks()); describe('JourneyController', () => { it('GET / lists; POST / 400 without title, else creates', () => { - expect(new JourneyController(svc({ listJourneys: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial)).list(user)).toEqual({ journeys: [{ id: 1 }] }); - expect(thrown(() => new JourneyController(svc()).create(user, { title: ' ' }))).toEqual({ status: 400, body: { error: 'Title is required' } }); + expect(new JourneyController(svc({ listJourneys: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial), captureBackfillStub).list(user)).toEqual({ journeys: [{ id: 1 }] }); + expect(thrown(() => new JourneyController(svc(), captureBackfillStub).create(user, { title: ' ' }))).toEqual({ status: 400, body: { error: 'Title is required' } }); const createJourney = vi.fn().mockReturnValue({ id: 9 }); - expect(new JourneyController(svc({ createJourney } as Partial)).create(user, { title: ' Trip ', trip_ids: [1, '2'] })).toEqual({ id: 9 }); + expect(new JourneyController(svc({ createJourney } as Partial), captureBackfillStub).create(user, { title: ' Trip ', trip_ids: [1, '2'] })).toEqual({ id: 9 }); expect(createJourney).toHaveBeenCalledWith(1, { title: 'Trip', subtitle: undefined, trip_ids: [1, 2] }); }); it('GET /suggestions + /available-trips', () => { - expect(new JourneyController(svc({ getSuggestions: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial)).suggestions(user)).toEqual({ trips: [{ id: 1 }] }); - expect(new JourneyController(svc({ listUserTrips: vi.fn().mockReturnValue([{ id: 2 }]) } as Partial)).availableTrips(user)).toEqual({ trips: [{ id: 2 }] }); + expect(new JourneyController(svc({ getSuggestions: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial), captureBackfillStub).suggestions(user)).toEqual({ trips: [{ id: 1 }] }); + expect(new JourneyController(svc({ listUserTrips: vi.fn().mockReturnValue([{ id: 2 }]) } as Partial), captureBackfillStub).availableTrips(user)).toEqual({ trips: [{ id: 2 }] }); }); it('PATCH/DELETE entries map 404', () => { - expect(thrown(() => new JourneyController(svc({ updateEntry: vi.fn().mockReturnValue(null) } as Partial)).updateEntry(user, '3', {}))).toEqual({ status: 404, body: { error: 'Entry not found' } }); - expect(new JourneyController(svc({ updateEntry: vi.fn().mockReturnValue({ id: 3 }) } as Partial)).updateEntry(user, '3', { title: 'x' })).toEqual({ id: 3 }); - expect(thrown(() => new JourneyController(svc({ deleteEntry: vi.fn().mockReturnValue(false) } as Partial)).deleteEntry(user, '3'))).toEqual({ status: 404, body: { error: 'Entry not found' } }); - expect(new JourneyController(svc({ deleteEntry: vi.fn().mockReturnValue(true) } as Partial)).deleteEntry(user, '3')).toEqual({ success: true }); + expect(thrown(() => new JourneyController(svc({ updateEntry: vi.fn().mockReturnValue(null) } as Partial), captureBackfillStub).updateEntry(user, '3', {}))).toEqual({ status: 404, body: { error: 'Entry not found' } }); + expect(new JourneyController(svc({ updateEntry: vi.fn().mockReturnValue({ id: 3 }) } as Partial), captureBackfillStub).updateEntry(user, '3', { title: 'x' })).toEqual({ id: 3 }); + expect(thrown(() => new JourneyController(svc({ deleteEntry: vi.fn().mockReturnValue(false) } as Partial), captureBackfillStub).deleteEntry(user, '3'))).toEqual({ status: 404, body: { error: 'Entry not found' } }); + expect(new JourneyController(svc({ deleteEntry: vi.fn().mockReturnValue(true) } as Partial), captureBackfillStub).deleteEntry(user, '3')).toEqual({ success: true }); }); it('provider-photos: batch, single 400/403, success', () => { const batch = svc({ addProviderPhoto: vi.fn().mockReturnValue({ id: 1 }) } as Partial); - expect(new JourneyController(batch).providerPhotos(user, '3', { provider: 'immich', asset_ids: ['a', 'b'] })).toEqual({ photos: [{ id: 1 }, { id: 1 }], added: 2 }); - expect(thrown(() => new JourneyController(svc()).providerPhotos(user, '3', { provider: 'immich' }))).toEqual({ status: 400, body: { error: 'provider and asset_id required' } }); - expect(thrown(() => new JourneyController(svc({ addProviderPhoto: vi.fn().mockReturnValue(null) } as Partial)).providerPhotos(user, '3', { provider: 'immich', asset_id: 'a' }))).toEqual({ status: 403, body: { error: 'Not allowed or duplicate' } }); + expect(new JourneyController(batch, captureBackfillStub).providerPhotos(user, '3', { provider: 'immich', asset_ids: ['a', 'b'] })).toEqual({ photos: [{ id: 1 }, { id: 1 }], added: 2 }); + expect(thrown(() => new JourneyController(svc(), captureBackfillStub).providerPhotos(user, '3', { provider: 'immich' }))).toEqual({ status: 400, body: { error: 'provider and asset_id required' } }); + expect(thrown(() => new JourneyController(svc({ addProviderPhoto: vi.fn().mockReturnValue(null) } as Partial), captureBackfillStub).providerPhotos(user, '3', { provider: 'immich', asset_id: 'a' }))).toEqual({ status: 403, body: { error: 'Not allowed or duplicate' } }); }); it('link-photo: 400 without id (accepts legacy photo_id), 403, success', () => { - expect(thrown(() => new JourneyController(svc()).linkPhoto(user, '3', {}))).toEqual({ status: 400, body: { error: 'journey_photo_id required' } }); + expect(thrown(() => new JourneyController(svc(), captureBackfillStub).linkPhoto(user, '3', {}))).toEqual({ status: 400, body: { error: 'journey_photo_id required' } }); const linkPhotoToEntry = vi.fn().mockReturnValue({ id: 5 }); - const c = new JourneyController(svc({ linkPhotoToEntry } as Partial)); + const c = new JourneyController(svc({ linkPhotoToEntry } as Partial), captureBackfillStub); expect(c.linkPhoto(user, '3', { photo_id: 5 })).toEqual({ id: 5 }); expect(linkPhotoToEntry).toHaveBeenCalledWith(3, 5, 1); // accepts the canonical journey_photo_id, 403 when the service refuses - expect(thrown(() => new JourneyController(svc({ linkPhotoToEntry: vi.fn().mockReturnValue(null) } as Partial)).linkPhoto(user, '3', { journey_photo_id: 9 }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(thrown(() => new JourneyController(svc({ linkPhotoToEntry: vi.fn().mockReturnValue(null) } as Partial), captureBackfillStub).linkPhoto(user, '3', { journey_photo_id: 9 }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); }); it('unlink photo (204) maps 404; delete photo 404 then unlinks file', () => { - expect(thrown(() => new JourneyController(svc({ unlinkPhotoFromEntry: vi.fn().mockReturnValue(false) } as Partial)).unlinkPhoto(user, '3', '7'))).toEqual({ status: 404, body: { error: 'Not found or not allowed' } }); - expect(new JourneyController(svc({ unlinkPhotoFromEntry: vi.fn().mockReturnValue(true) } as Partial)).unlinkPhoto(user, '3', '7')).toBeUndefined(); - expect(thrown(() => new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue(null) } as Partial)).deletePhoto(user, '7'))).toEqual({ status: 404, body: { error: 'Photo not found' } }); - expect(new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue({ id: 7, file_path: null }) } as Partial)).deletePhoto(user, '7')).toEqual({ success: true }); + expect(thrown(() => new JourneyController(svc({ unlinkPhotoFromEntry: vi.fn().mockReturnValue(false) } as Partial), captureBackfillStub).unlinkPhoto(user, '3', '7'))).toEqual({ status: 404, body: { error: 'Not found or not allowed' } }); + expect(new JourneyController(svc({ unlinkPhotoFromEntry: vi.fn().mockReturnValue(true) } as Partial), captureBackfillStub).unlinkPhoto(user, '3', '7')).toBeUndefined(); + expect(thrown(() => new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue(null) } as Partial), captureBackfillStub).deletePhoto(user, '7'))).toEqual({ status: 404, body: { error: 'Photo not found' } }); + expect(new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue({ id: 7, file_path: null }) } as Partial), captureBackfillStub).deletePhoto(user, '7')).toEqual({ success: true }); }); it('gallery upload 400 no files / 403 not allowed, else returns photos', () => { - expect(thrown(() => new JourneyController(svc()).uploadGalleryPhotos(user, '3', undefined))).toEqual({ status: 400, body: { error: 'No files uploaded' } }); - expect(thrown(() => new JourneyController(svc({ uploadGalleryPhotos: vi.fn().mockReturnValue([]) } as Partial)).uploadGalleryPhotos(user, '3', [{ filename: 'a.jpg' } as Express.Multer.File]))).toEqual({ status: 403, body: { error: 'Not allowed' } }); - expect(new JourneyController(svc({ uploadGalleryPhotos: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial)).uploadGalleryPhotos(user, '3', [{ filename: 'a.jpg' } as Express.Multer.File])).toEqual({ photos: [{ id: 1 }] }); + expect(thrown(() => new JourneyController(svc(), captureBackfillStub).uploadGalleryPhotos(user, '3', undefined))).toEqual({ status: 400, body: { error: 'No files uploaded' } }); + expect(thrown(() => new JourneyController(svc({ uploadGalleryPhotos: vi.fn().mockReturnValue([]) } as Partial), captureBackfillStub).uploadGalleryPhotos(user, '3', [{ filename: 'a.jpg' } as Express.Multer.File]))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(new JourneyController(svc({ uploadGalleryPhotos: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial), captureBackfillStub).uploadGalleryPhotos(user, '3', [{ filename: 'a.jpg' } as Express.Multer.File])).toEqual({ photos: [{ id: 1 }] }); }); it('gallery video: 400 no video, 403 not allowed, else stores the clip + poster (#823)', () => { const files = { video: [{ filename: 'v.mp4' } as Express.Multer.File], poster: [{ filename: 'p.jpg' } as Express.Multer.File] }; - expect(thrown(() => new JourneyController(svc()).uploadGalleryVideo(user, '3', {}, {}))).toEqual({ status: 400, body: { error: 'No video uploaded' } }); + expect(thrown(() => new JourneyController(svc(), captureBackfillStub).uploadGalleryVideo(user, '3', {}, {}))).toEqual({ status: 400, body: { error: 'No video uploaded' } }); // Rejected with real paths → the cleanup unlinks the orphaned bytes (the files // don't exist, so the best-effort catch swallows it). const withPaths = { video: [{ filename: 'v.mp4', path: '/nonexistent/v.mp4' } as Express.Multer.File], poster: [{ filename: 'p.jpg', path: '/nonexistent/p.jpg' } as Express.Multer.File] }; - expect(thrown(() => new JourneyController(svc({ uploadGalleryPhotos: vi.fn().mockReturnValue([]) } as Partial)).uploadGalleryVideo(user, '3', withPaths, { duration_ms: 'abc' }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(thrown(() => new JourneyController(svc({ uploadGalleryPhotos: vi.fn().mockReturnValue([]) } as Partial), captureBackfillStub).uploadGalleryVideo(user, '3', withPaths, { duration_ms: 'abc' }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); const up = vi.fn().mockReturnValue([{ id: 7 }]); - expect(new JourneyController(svc({ uploadGalleryPhotos: up } as Partial)).uploadGalleryVideo(user, '3', files, { duration_ms: '4200' })).toEqual({ photos: [{ id: 7 }] }); + expect(new JourneyController(svc({ uploadGalleryPhotos: up } as Partial), captureBackfillStub).uploadGalleryVideo(user, '3', files, { duration_ms: '4200' })).toEqual({ photos: [{ id: 7 }] }); expect(up).toHaveBeenCalledWith(3, 1, [{ path: 'journey/v.mp4', thumbnail: 'journey/p.jpg', mediaType: 'video', durationMs: 4200 }]); // No poster + no duration → thumbnail undefined, durationMs null. const up2 = vi.fn().mockReturnValue([{ id: 8 }]); - new JourneyController(svc({ uploadGalleryPhotos: up2 } as Partial)).uploadGalleryVideo(user, '3', { video: [{ filename: 'v2.mp4' } as Express.Multer.File] }, {}); + new JourneyController(svc({ uploadGalleryPhotos: up2 } as Partial), captureBackfillStub).uploadGalleryVideo(user, '3', { video: [{ filename: 'v2.mp4' } as Express.Multer.File] }, {}); expect(up2).toHaveBeenCalledWith(3, 1, [{ path: 'journey/v2.mp4', thumbnail: undefined, mediaType: 'video', durationMs: null }]); }); it('provider-photos forwards per-asset media_types for gallery and entries (#823)', () => { const add = vi.fn().mockReturnValue({ id: 1 }); - new JourneyController(svc({ addProviderPhotoToGallery: add } as Partial)).galleryProviderPhotos(user, '9', { provider: 'immich', asset_ids: ['a', 'b'], media_types: ['video', 'image'] }); + new JourneyController(svc({ addProviderPhotoToGallery: add } as Partial), captureBackfillStub).galleryProviderPhotos(user, '9', { provider: 'immich', asset_ids: ['a', 'b'], media_types: ['video', 'image'] }); expect(add).toHaveBeenNthCalledWith(1, 9, 1, 'immich', 'a', undefined, undefined, 'video'); expect(add).toHaveBeenNthCalledWith(2, 9, 1, 'immich', 'b', undefined, undefined, 'image'); const addOne = vi.fn().mockReturnValue({ id: 2 }); - new JourneyController(svc({ addProviderPhotoToGallery: addOne } as Partial)).galleryProviderPhotos(user, '9', { provider: 'immich', asset_id: 'c', media_type: 'video' }); + new JourneyController(svc({ addProviderPhotoToGallery: addOne } as Partial), captureBackfillStub).galleryProviderPhotos(user, '9', { provider: 'immich', asset_id: 'c', media_type: 'video' }); expect(addOne).toHaveBeenCalledWith(9, 1, 'immich', 'c', undefined, undefined, 'video'); // Entry path mirrors the gallery path. const eAdd = vi.fn().mockReturnValue({ id: 3 }); - new JourneyController(svc({ addProviderPhoto: eAdd } as Partial)).providerPhotos(user, '4', { provider: 'immich', asset_ids: ['x'], media_types: ['video'], caption: 'c' }); + new JourneyController(svc({ addProviderPhoto: eAdd } as Partial), captureBackfillStub).providerPhotos(user, '4', { provider: 'immich', asset_ids: ['x'], media_types: ['video'], caption: 'c' }); expect(eAdd).toHaveBeenNthCalledWith(1, 4, 1, 'immich', 'x', 'c', undefined, 'video'); const eOne = vi.fn().mockReturnValue({ id: 4 }); - new JourneyController(svc({ addProviderPhoto: eOne } as Partial)).providerPhotos(user, '4', { provider: 'immich', asset_id: 'y', media_type: 'video' }); + new JourneyController(svc({ addProviderPhoto: eOne } as Partial), captureBackfillStub).providerPhotos(user, '4', { provider: 'immich', asset_id: 'y', media_type: 'video' }); expect(eOne).toHaveBeenCalledWith(4, 1, 'immich', 'y', undefined, undefined, 'video'); }); it('GET/PATCH/DELETE /:id map 404', () => { - expect(thrown(() => new JourneyController(svc({ getJourneyFull: vi.fn().mockReturnValue(null) } as Partial)).get(user, '9'))).toEqual({ status: 404, body: { error: 'Journey not found' } }); - expect(new JourneyController(svc({ getJourneyFull: vi.fn().mockReturnValue({ id: 9 }) } as Partial)).get(user, '9')).toEqual({ id: 9 }); - expect(thrown(() => new JourneyController(svc({ updateJourney: vi.fn().mockReturnValue(null) } as Partial)).update(user, '9', {}))).toEqual({ status: 404, body: { error: 'Journey not found' } }); - expect(thrown(() => new JourneyController(svc({ deleteJourney: vi.fn().mockReturnValue(false) } as Partial)).remove(user, '9'))).toEqual({ status: 404, body: { error: 'Journey not found' } }); + expect(thrown(() => new JourneyController(svc({ getJourneyFull: vi.fn().mockReturnValue(null) } as Partial), captureBackfillStub).get(user, '9'))).toEqual({ status: 404, body: { error: 'Journey not found' } }); + expect(new JourneyController(svc({ getJourneyFull: vi.fn().mockReturnValue({ id: 9 }) } as Partial), captureBackfillStub).get(user, '9')).toEqual({ id: 9 }); + expect(thrown(() => new JourneyController(svc({ updateJourney: vi.fn().mockReturnValue(null) } as Partial), captureBackfillStub).update(user, '9', {}))).toEqual({ status: 404, body: { error: 'Journey not found' } }); + expect(thrown(() => new JourneyController(svc({ deleteJourney: vi.fn().mockReturnValue(false) } as Partial), captureBackfillStub).remove(user, '9'))).toEqual({ status: 404, body: { error: 'Journey not found' } }); }); it('trips: POST 400 without trip_id / 403, DELETE 403', () => { - expect(thrown(() => new JourneyController(svc()).addTrip(user, '9', {}))).toEqual({ status: 400, body: { error: 'trip_id required' } }); - expect(thrown(() => new JourneyController(svc({ addTripToJourney: vi.fn().mockReturnValue(false) } as Partial)).addTrip(user, '9', { trip_id: 2 }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); - expect(new JourneyController(svc({ addTripToJourney: vi.fn().mockReturnValue(true) } as Partial)).addTrip(user, '9', { trip_id: 2 })).toEqual({ success: true }); - expect(thrown(() => new JourneyController(svc({ removeTripFromJourney: vi.fn().mockReturnValue(false) } as Partial)).removeTrip(user, '9', '2'))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(thrown(() => new JourneyController(svc(), captureBackfillStub).addTrip(user, '9', {}))).toEqual({ status: 400, body: { error: 'trip_id required' } }); + expect(thrown(() => new JourneyController(svc({ addTripToJourney: vi.fn().mockReturnValue(false) } as Partial), captureBackfillStub).addTrip(user, '9', { trip_id: 2 }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(new JourneyController(svc({ addTripToJourney: vi.fn().mockReturnValue(true) } as Partial), captureBackfillStub).addTrip(user, '9', { trip_id: 2 })).toEqual({ success: true }); + expect(thrown(() => new JourneyController(svc({ removeTripFromJourney: vi.fn().mockReturnValue(false) } as Partial), captureBackfillStub).removeTrip(user, '9', '2'))).toEqual({ status: 403, body: { error: 'Not allowed' } }); }); it('entries under journey: list 404, create 400/404, reorder 400/403', () => { - expect(thrown(() => new JourneyController(svc({ listEntries: vi.fn().mockReturnValue(null) } as Partial)).listEntries(user, '9'))).toEqual({ status: 404, body: { error: 'Journey not found' } }); - expect(new JourneyController(svc({ listEntries: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial)).listEntries(user, '9')).toEqual({ entries: [{ id: 1 }] }); - expect(thrown(() => new JourneyController(svc()).createEntry(user, '9', {}))).toEqual({ status: 400, body: { error: 'entry_date is required' } }); - expect(thrown(() => new JourneyController(svc({ createEntry: vi.fn().mockReturnValue(null) } as Partial)).createEntry(user, '9', { entry_date: '2026-01-01' }))).toEqual({ status: 404, body: { error: 'Journey not found' } }); - expect(thrown(() => new JourneyController(svc()).reorderEntries(user, '9', { orderedIds: 'no' }))).toEqual({ status: 400, body: { error: 'orderedIds must be an array of numbers' } }); - expect(thrown(() => new JourneyController(svc({ reorderEntries: vi.fn().mockReturnValue(false) } as Partial)).reorderEntries(user, '9', { orderedIds: [1, 2] }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(thrown(() => new JourneyController(svc({ listEntries: vi.fn().mockReturnValue(null) } as Partial), captureBackfillStub).listEntries(user, '9'))).toEqual({ status: 404, body: { error: 'Journey not found' } }); + expect(new JourneyController(svc({ listEntries: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial), captureBackfillStub).listEntries(user, '9')).toEqual({ entries: [{ id: 1 }] }); + expect(thrown(() => new JourneyController(svc(), captureBackfillStub).createEntry(user, '9', {}))).toEqual({ status: 400, body: { error: 'entry_date is required' } }); + expect(thrown(() => new JourneyController(svc({ createEntry: vi.fn().mockReturnValue(null) } as Partial), captureBackfillStub).createEntry(user, '9', { entry_date: '2026-01-01' }))).toEqual({ status: 404, body: { error: 'Journey not found' } }); + expect(thrown(() => new JourneyController(svc(), captureBackfillStub).reorderEntries(user, '9', { orderedIds: 'no' }))).toEqual({ status: 400, body: { error: 'orderedIds must be an array of numbers' } }); + expect(thrown(() => new JourneyController(svc({ reorderEntries: vi.fn().mockReturnValue(false) } as Partial), captureBackfillStub).reorderEntries(user, '9', { orderedIds: [1, 2] }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); }); it('contributors: add 400/403, update 403, remove 403', () => { - expect(thrown(() => new JourneyController(svc()).addContributor(user, '9', {}))).toEqual({ status: 400, body: { error: 'user_id required' } }); - expect(thrown(() => new JourneyController(svc({ addContributor: vi.fn().mockReturnValue(false) } as Partial)).addContributor(user, '9', { user_id: 2 }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); - expect(new JourneyController(svc({ addContributor: vi.fn().mockReturnValue(true) } as Partial)).addContributor(user, '9', { user_id: 2 })).toEqual({ success: true }); - expect(thrown(() => new JourneyController(svc({ updateContributorRole: vi.fn().mockReturnValue(false) } as Partial)).updateContributor(user, '9', '2', { role: 'editor' }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); - expect(thrown(() => new JourneyController(svc({ removeContributor: vi.fn().mockReturnValue(false) } as Partial)).removeContributor(user, '9', '2'))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(thrown(() => new JourneyController(svc(), captureBackfillStub).addContributor(user, '9', {}))).toEqual({ status: 400, body: { error: 'user_id required' } }); + expect(thrown(() => new JourneyController(svc({ addContributor: vi.fn().mockReturnValue(false) } as Partial), captureBackfillStub).addContributor(user, '9', { user_id: 2 }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(new JourneyController(svc({ addContributor: vi.fn().mockReturnValue(true) } as Partial), captureBackfillStub).addContributor(user, '9', { user_id: 2 })).toEqual({ success: true }); + expect(thrown(() => new JourneyController(svc({ updateContributorRole: vi.fn().mockReturnValue(false) } as Partial), captureBackfillStub).updateContributor(user, '9', '2', { role: 'editor' }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(thrown(() => new JourneyController(svc({ removeContributor: vi.fn().mockReturnValue(false) } as Partial), captureBackfillStub).removeContributor(user, '9', '2'))).toEqual({ status: 403, body: { error: 'Not allowed' } }); }); it('preferences 403, share-link get/set/delete', () => { - expect(thrown(() => new JourneyController(svc({ updateJourneyPreferences: vi.fn().mockReturnValue(null) } as Partial)).preferences(user, '9', {}))).toEqual({ status: 403, body: { error: 'Not allowed' } }); - expect(new JourneyController(svc({ getJourneyShareLink: vi.fn().mockReturnValue({ token: 'abc' }) } as Partial)).getShareLink(user, '9')).toEqual({ link: { token: 'abc' } }); - expect(thrown(() => new JourneyController(svc({ createOrUpdateJourneyShareLink: vi.fn().mockReturnValue(null) } as Partial)).setShareLink(user, '9', {}))).toEqual({ status: 403, body: { error: 'Not allowed' } }); - expect(new JourneyController(svc({ createOrUpdateJourneyShareLink: vi.fn().mockReturnValue({ token: 'abc' }) } as Partial)).setShareLink(user, '9', { share_timeline: true })).toEqual({ token: 'abc' }); - expect(thrown(() => new JourneyController(svc({ deleteJourneyShareLink: vi.fn().mockReturnValue(false) } as Partial)).deleteShareLink(user, '9'))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(thrown(() => new JourneyController(svc({ updateJourneyPreferences: vi.fn().mockReturnValue(null) } as Partial), captureBackfillStub).preferences(user, '9', {}))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(new JourneyController(svc({ getJourneyShareLink: vi.fn().mockReturnValue({ token: 'abc' }) } as Partial), captureBackfillStub).getShareLink(user, '9')).toEqual({ link: { token: 'abc' } }); + expect(thrown(() => new JourneyController(svc({ createOrUpdateJourneyShareLink: vi.fn().mockReturnValue(null) } as Partial), captureBackfillStub).setShareLink(user, '9', {}))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(new JourneyController(svc({ createOrUpdateJourneyShareLink: vi.fn().mockReturnValue({ token: 'abc' }) } as Partial), captureBackfillStub).setShareLink(user, '9', { share_timeline: true })).toEqual({ token: 'abc' }); + expect(thrown(() => new JourneyController(svc({ deleteJourneyShareLink: vi.fn().mockReturnValue(false) } as Partial), captureBackfillStub).deleteShareLink(user, '9'))).toEqual({ status: 403, body: { error: 'Not allowed' } }); }); it('entry photo upload mirrors to Immich only when opted in', async () => { @@ -164,51 +169,51 @@ describe('JourneyController', () => { const uploadToImmich = vi.fn().mockResolvedValue('immich-1'); const setPhotoProvider = vi.fn(); const s = svc({ addPhoto, immichAutoUploadEnabled: vi.fn().mockReturnValue(true), uploadToImmich, setPhotoProvider } as Partial); - const res = await new JourneyController(s).uploadEntryPhotos(user, '3', [{ filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File], {}); + const res = await new JourneyController(s, captureBackfillStub).uploadEntryPhotos(user, '3', [{ filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File], {}); expect(setPhotoProvider).toHaveBeenCalledWith(5, 'immich', 'immich-1', 1); expect(res).toEqual({ photos: [{ id: 5, provider: 'immich', asset_id: 'immich-1', owner_id: 1 }] }); const noOptIn = svc({ addPhoto: vi.fn().mockReturnValue({ id: 6 }), immichAutoUploadEnabled: vi.fn().mockReturnValue(false), uploadToImmich } as Partial); - await new JourneyController(noOptIn).uploadEntryPhotos(user, '3', [{ filename: 'b.jpg', originalname: 'b.jpg' } as Express.Multer.File], {}); + await new JourneyController(noOptIn, captureBackfillStub).uploadEntryPhotos(user, '3', [{ filename: 'b.jpg', originalname: 'b.jpg' } as Express.Multer.File], {}); expect(uploadToImmich).toHaveBeenCalledTimes(1); // only the opted-in upload above }); it('entry photo upload: 400 no files, 403 when nothing added, swallows immich errors and empty ids', async () => { - expect(await thrownAsync(() => new JourneyController(svc()).uploadEntryPhotos(user, '3', undefined, {}))).toEqual({ status: 400, body: { error: 'No files uploaded' } }); - expect(await thrownAsync(() => new JourneyController(svc({ addPhoto: vi.fn().mockReturnValue(null) } as Partial)).uploadEntryPhotos(user, '3', [{ filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File], {}))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(await thrownAsync(() => new JourneyController(svc(), captureBackfillStub).uploadEntryPhotos(user, '3', undefined, {}))).toEqual({ status: 400, body: { error: 'No files uploaded' } }); + expect(await thrownAsync(() => new JourneyController(svc({ addPhoto: vi.fn().mockReturnValue(null) } as Partial), captureBackfillStub).uploadEntryPhotos(user, '3', [{ filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File], {}))).toEqual({ status: 403, body: { error: 'Not allowed' } }); // opted in but the immich upload throws → best-effort, the local photo still wins const setPhotoProvider = vi.fn(); const blowsUp = svc({ addPhoto: vi.fn().mockReturnValue({ id: 8 }), immichAutoUploadEnabled: vi.fn().mockReturnValue(true), uploadToImmich: vi.fn().mockRejectedValue(new Error('immich down')), setPhotoProvider } as Partial); - expect(await new JourneyController(blowsUp).uploadEntryPhotos(user, '3', [{ filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File], { caption: 'c' })).toEqual({ photos: [{ id: 8 }] }); + expect(await new JourneyController(blowsUp, captureBackfillStub).uploadEntryPhotos(user, '3', [{ filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File], { caption: 'c' })).toEqual({ photos: [{ id: 8 }] }); expect(setPhotoProvider).not.toHaveBeenCalled(); // opted in but immich returns a falsy id → no provider stamping const noId = svc({ addPhoto: vi.fn().mockReturnValue({ id: 9 }), immichAutoUploadEnabled: vi.fn().mockReturnValue(true), uploadToImmich: vi.fn().mockResolvedValue(''), setPhotoProvider } as Partial); - expect(await new JourneyController(noId).uploadEntryPhotos(user, '3', [{ filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File], {})).toEqual({ photos: [{ id: 9 }] }); + expect(await new JourneyController(noId, captureBackfillStub).uploadEntryPhotos(user, '3', [{ filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File], {})).toEqual({ photos: [{ id: 9 }] }); }); it('provider-photos batch passes the passphrase through when present', () => { const addProviderPhoto = vi.fn().mockReturnValue({ id: 1 }); - new JourneyController(svc({ addProviderPhoto } as Partial)).providerPhotos(user, '3', { provider: 'immich', asset_ids: ['a'], caption: 'cap', passphrase: 'secret' }); + new JourneyController(svc({ addProviderPhoto } as Partial), captureBackfillStub).providerPhotos(user, '3', { provider: 'immich', asset_ids: ['a'], caption: 'cap', passphrase: 'secret' }); expect(addProviderPhoto).toHaveBeenCalledWith(3, 1, 'immich', 'a', 'cap', 'secret', 'image'); // single-photo success path - expect(new JourneyController(svc({ addProviderPhoto: vi.fn().mockReturnValue({ id: 2 }) } as Partial)).providerPhotos(user, '3', { provider: 'immich', asset_id: 'a' })).toEqual({ id: 2 }); + expect(new JourneyController(svc({ addProviderPhoto: vi.fn().mockReturnValue({ id: 2 }) } as Partial), captureBackfillStub).providerPhotos(user, '3', { provider: 'immich', asset_id: 'a' })).toEqual({ id: 2 }); }); it('PATCH photos: 404 then returns the updated photo', () => { - expect(thrown(() => new JourneyController(svc({ updatePhoto: vi.fn().mockReturnValue(null) } as Partial)).updatePhoto(user, '7', { caption: 'x' }))).toEqual({ status: 404, body: { error: 'Photo not found' } }); - expect(new JourneyController(svc({ updatePhoto: vi.fn().mockReturnValue({ id: 7 }) } as Partial)).updatePhoto(user, '7', { caption: 'x' })).toEqual({ id: 7 }); + expect(thrown(() => new JourneyController(svc({ updatePhoto: vi.fn().mockReturnValue(null) } as Partial), captureBackfillStub).updatePhoto(user, '7', { caption: 'x' }))).toEqual({ status: 404, body: { error: 'Photo not found' } }); + expect(new JourneyController(svc({ updatePhoto: vi.fn().mockReturnValue({ id: 7 }) } as Partial), captureBackfillStub).updatePhoto(user, '7', { caption: 'x' })).toEqual({ id: 7 }); }); it('DELETE photo unlinks the file when a path exists', () => { const unlinkSpy = vi.spyOn(fs, 'unlinkSync').mockImplementation(() => undefined); try { - expect(new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue({ id: 7, file_path: 'journey/a.jpg' }) } as Partial)).deletePhoto(user, '7')).toEqual({ success: true }); + expect(new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue({ id: 7, file_path: 'journey/a.jpg' }) } as Partial), captureBackfillStub).deletePhoto(user, '7')).toEqual({ success: true }); expect(unlinkSpy).toHaveBeenCalledTimes(1); // a vanished file is swallowed unlinkSpy.mockImplementationOnce(() => { throw new Error('ENOENT'); }); - expect(new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue({ id: 8, file_path: 'journey/b.jpg' }) } as Partial)).deletePhoto(user, '8')).toEqual({ success: true }); + expect(new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue({ id: 8, file_path: 'journey/b.jpg' }) } as Partial), captureBackfillStub).deletePhoto(user, '8')).toEqual({ success: true }); } finally { unlinkSpy.mockRestore(); } @@ -216,68 +221,68 @@ describe('JourneyController', () => { it('gallery provider-photos: batch (with passphrase), single 400/403, success', () => { const addProviderPhotoToGallery = vi.fn().mockReturnValue({ id: 1 }); - const batch = new JourneyController(svc({ addProviderPhotoToGallery } as Partial)); + const batch = new JourneyController(svc({ addProviderPhotoToGallery } as Partial), captureBackfillStub); expect(batch.galleryProviderPhotos(user, '9', { provider: 'immich', asset_ids: ['a', 'b'], passphrase: 'pw' })).toEqual({ photos: [{ id: 1 }, { id: 1 }], added: 2 }); expect(addProviderPhotoToGallery).toHaveBeenCalledWith(9, 1, 'immich', 'a', undefined, 'pw', 'image'); - expect(thrown(() => new JourneyController(svc()).galleryProviderPhotos(user, '9', { provider: 'immich' }))).toEqual({ status: 400, body: { error: 'provider and asset_id required' } }); - expect(thrown(() => new JourneyController(svc({ addProviderPhotoToGallery: vi.fn().mockReturnValue(null) } as Partial)).galleryProviderPhotos(user, '9', { provider: 'immich', asset_id: 'a' }))).toEqual({ status: 403, body: { error: 'Not allowed or duplicate' } }); - expect(new JourneyController(svc({ addProviderPhotoToGallery: vi.fn().mockReturnValue({ id: 3 }) } as Partial)).galleryProviderPhotos(user, '9', { provider: 'immich', asset_id: 'a' })).toEqual({ id: 3 }); + expect(thrown(() => new JourneyController(svc(), captureBackfillStub).galleryProviderPhotos(user, '9', { provider: 'immich' }))).toEqual({ status: 400, body: { error: 'provider and asset_id required' } }); + expect(thrown(() => new JourneyController(svc({ addProviderPhotoToGallery: vi.fn().mockReturnValue(null) } as Partial), captureBackfillStub).galleryProviderPhotos(user, '9', { provider: 'immich', asset_id: 'a' }))).toEqual({ status: 403, body: { error: 'Not allowed or duplicate' } }); + expect(new JourneyController(svc({ addProviderPhotoToGallery: vi.fn().mockReturnValue({ id: 3 }) } as Partial), captureBackfillStub).galleryProviderPhotos(user, '9', { provider: 'immich', asset_id: 'a' })).toEqual({ id: 3 }); }); it('DELETE gallery photo: 404, then unlinks the file when present', () => { - expect(thrown(() => new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue(null) } as Partial)).deleteGalleryPhoto(user, '7'))).toEqual({ status: 404, body: { error: 'Photo not found or not allowed' } }); + expect(thrown(() => new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue(null) } as Partial), captureBackfillStub).deleteGalleryPhoto(user, '7'))).toEqual({ status: 404, body: { error: 'Photo not found or not allowed' } }); // no file_path → nothing to unlink, returns void - expect(new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue({ id: 7, file_path: null }) } as Partial)).deleteGalleryPhoto(user, '7')).toBeUndefined(); + expect(new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue({ id: 7, file_path: null }) } as Partial), captureBackfillStub).deleteGalleryPhoto(user, '7')).toBeUndefined(); const unlinkSpy = vi.spyOn(fs, 'unlinkSync').mockImplementation(() => undefined); try { - new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue({ id: 8, file_path: 'journey/g.jpg' }) } as Partial)).deleteGalleryPhoto(user, '8'); + new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue({ id: 8, file_path: 'journey/g.jpg' }) } as Partial), captureBackfillStub).deleteGalleryPhoto(user, '8'); expect(unlinkSpy).toHaveBeenCalledTimes(1); unlinkSpy.mockImplementationOnce(() => { throw new Error('ENOENT'); }); - expect(new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue({ id: 9, file_path: 'journey/h.jpg' }) } as Partial)).deleteGalleryPhoto(user, '9')).toBeUndefined(); + expect(new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue({ id: 9, file_path: 'journey/h.jpg' }) } as Partial), captureBackfillStub).deleteGalleryPhoto(user, '9')).toBeUndefined(); } finally { unlinkSpy.mockRestore(); } }); it('PATCH /:id returns the updated journey on success', () => { - expect(new JourneyController(svc({ updateJourney: vi.fn().mockReturnValue({ id: 9 }) } as Partial)).update(user, '9', { title: 'x' })).toEqual({ id: 9 }); + expect(new JourneyController(svc({ updateJourney: vi.fn().mockReturnValue({ id: 9 }) } as Partial), captureBackfillStub).update(user, '9', { title: 'x' })).toEqual({ id: 9 }); }); it('cover upload: 400 without file, 404 when the journey is gone, else returns the journey', () => { - expect(thrown(() => new JourneyController(svc()).cover(user, '9', undefined))).toEqual({ status: 400, body: { error: 'No file uploaded' } }); - expect(thrown(() => new JourneyController(svc({ updateJourney: vi.fn().mockReturnValue(null) } as Partial)).cover(user, '9', { filename: 'c.jpg' } as Express.Multer.File))).toEqual({ status: 404, body: { error: 'Journey not found' } }); + expect(thrown(() => new JourneyController(svc(), captureBackfillStub).cover(user, '9', undefined))).toEqual({ status: 400, body: { error: 'No file uploaded' } }); + expect(thrown(() => new JourneyController(svc({ updateJourney: vi.fn().mockReturnValue(null) } as Partial), captureBackfillStub).cover(user, '9', { filename: 'c.jpg' } as Express.Multer.File))).toEqual({ status: 404, body: { error: 'Journey not found' } }); const updateJourney = vi.fn().mockReturnValue({ id: 9, cover_image: 'journey/c.jpg' }); - expect(new JourneyController(svc({ updateJourney } as Partial)).cover(user, '9', { filename: 'c.jpg' } as Express.Multer.File)).toEqual({ id: 9, cover_image: 'journey/c.jpg' }); + expect(new JourneyController(svc({ updateJourney } as Partial), captureBackfillStub).cover(user, '9', { filename: 'c.jpg' } as Express.Multer.File)).toEqual({ id: 9, cover_image: 'journey/c.jpg' }); expect(updateJourney).toHaveBeenCalledWith(9, 1, { cover_image: 'journey/c.jpg' }); }); it('DELETE /:id and trips/contributors success paths', () => { - expect(new JourneyController(svc({ deleteJourney: vi.fn().mockReturnValue(true) } as Partial)).remove(user, '9')).toEqual({ success: true }); - expect(new JourneyController(svc({ removeTripFromJourney: vi.fn().mockReturnValue(true) } as Partial)).removeTrip(user, '9', '2')).toEqual({ success: true }); - expect(new JourneyController(svc({ updateContributorRole: vi.fn().mockReturnValue(true) } as Partial)).updateContributor(user, '9', '2', { role: 'editor' })).toEqual({ success: true }); - expect(new JourneyController(svc({ removeContributor: vi.fn().mockReturnValue(true) } as Partial)).removeContributor(user, '9', '2')).toEqual({ success: true }); + expect(new JourneyController(svc({ deleteJourney: vi.fn().mockReturnValue(true) } as Partial), captureBackfillStub).remove(user, '9')).toEqual({ success: true }); + expect(new JourneyController(svc({ removeTripFromJourney: vi.fn().mockReturnValue(true) } as Partial), captureBackfillStub).removeTrip(user, '9', '2')).toEqual({ success: true }); + expect(new JourneyController(svc({ updateContributorRole: vi.fn().mockReturnValue(true) } as Partial), captureBackfillStub).updateContributor(user, '9', '2', { role: 'editor' })).toEqual({ success: true }); + expect(new JourneyController(svc({ removeContributor: vi.fn().mockReturnValue(true) } as Partial), captureBackfillStub).removeContributor(user, '9', '2')).toEqual({ success: true }); }); it('addContributor defaults the role to viewer when omitted', () => { const addContributor = vi.fn().mockReturnValue(true); - new JourneyController(svc({ addContributor } as Partial)).addContributor(user, '9', { user_id: 2 }); + new JourneyController(svc({ addContributor } as Partial), captureBackfillStub).addContributor(user, '9', { user_id: 2 }); expect(addContributor).toHaveBeenCalledWith(9, 1, 2, 'viewer'); }); it('createEntry returns the entry when the journey exists', () => { - expect(new JourneyController(svc({ createEntry: vi.fn().mockReturnValue({ id: 4 }) } as Partial)).createEntry(user, '9', { entry_date: '2026-01-01' })).toEqual({ id: 4 }); + expect(new JourneyController(svc({ createEntry: vi.fn().mockReturnValue({ id: 4 }) } as Partial), captureBackfillStub).createEntry(user, '9', { entry_date: '2026-01-01' })).toEqual({ id: 4 }); }); it('reorderEntries succeeds for a numeric array', () => { - expect(new JourneyController(svc({ reorderEntries: vi.fn().mockReturnValue(true) } as Partial)).reorderEntries(user, '9', { orderedIds: [3, 1, 2] })).toEqual({ success: true }); + expect(new JourneyController(svc({ reorderEntries: vi.fn().mockReturnValue(true) } as Partial), captureBackfillStub).reorderEntries(user, '9', { orderedIds: [3, 1, 2] })).toEqual({ success: true }); }); it('preferences returns the result on success', () => { - expect(new JourneyController(svc({ updateJourneyPreferences: vi.fn().mockReturnValue({ ok: true }) } as Partial)).preferences(user, '9', { theme: 'dark' })).toEqual({ ok: true }); + expect(new JourneyController(svc({ updateJourneyPreferences: vi.fn().mockReturnValue({ ok: true }) } as Partial), captureBackfillStub).preferences(user, '9', { theme: 'dark' })).toEqual({ ok: true }); }); it('deleteShareLink returns success when removed', () => { - expect(new JourneyController(svc({ deleteJourneyShareLink: vi.fn().mockReturnValue(true) } as Partial)).deleteShareLink(user, '9')).toEqual({ success: true }); + expect(new JourneyController(svc({ deleteJourneyShareLink: vi.fn().mockReturnValue(true) } as Partial), captureBackfillStub).deleteShareLink(user, '9')).toEqual({ success: true }); }); }); diff --git a/server/tests/unit/nest/photo-capture-backfill.service.test.ts b/server/tests/unit/nest/photo-capture-backfill.service.test.ts new file mode 100644 index 000000000..74e05d1b0 --- /dev/null +++ b/server/tests/unit/nest/photo-capture-backfill.service.test.ts @@ -0,0 +1,88 @@ +/** + * PhotoCaptureBackfillService (#1614) — asking the provider when and where a + * photo was taken, after the add the user was waiting on has already answered. + */ +import { describe, it, expect, vi } from 'vitest'; +import { PhotoCaptureBackfillService } from '../../../src/nest/memories/photo-capture-backfill.service'; +import type { PhotoResolverService } from '../../../src/nest/memories/photo-resolver.service'; +import type { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository'; + +type Row = { id: number; taken_at?: string | null; lat?: number | null; lng?: number | null }; + +function build(rows: Row[], info: Record) { + const recordCaptureMetadata = vi.fn(); + const getPhotoInfo = vi.fn(async (id: number) => + info[id] ? { success: true, data: info[id] } : { success: false, error: 'nope', status: 404 }, + ); + const photos = { + resolve: (id: number) => rows.find(r => r.id === id) ?? null, + recordCaptureMetadata, + } as unknown as TrekPhotosRepository; + const resolver = { getPhotoInfo } as unknown as PhotoResolverService; + return { svc: new PhotoCaptureBackfillService(resolver, photos), recordCaptureMetadata, getPhotoInfo }; +} + +describe('PhotoCaptureBackfillService', () => { + it('CAPTURE-001: records what the provider knows', async () => { + const { svc, recordCaptureMetadata } = build( + [{ id: 7 }], + { 7: { takenAt: '2026-03-15T10:20:00Z', lat: 48.8584, lng: 2.2945 } }, + ); + + await svc.run([7], 1); + + expect(recordCaptureMetadata).toHaveBeenCalledWith(7, { + takenAt: '2026-03-15T10:20:00Z', lat: 48.8584, lng: 2.2945, + }); + }); + + it('CAPTURE-002: skips a row that already knows both, so an album import is not a provider call per photo', async () => { + const { svc, getPhotoInfo } = build( + [{ id: 7, taken_at: '2026-03-15T10:20:00Z', lat: 1, lng: 2 }], + { 7: { takenAt: 'x' } }, + ); + + await svc.run([7], 1); + + expect(getPhotoInfo).not.toHaveBeenCalled(); + }); + + it('CAPTURE-003: still asks when only half is known', async () => { + const { svc, getPhotoInfo } = build( + [{ id: 7, taken_at: '2026-03-15T10:20:00Z' }], + { 7: { takenAt: '2026-03-15T10:20:00Z', lat: 48.8, lng: 2.2 } }, + ); + + await svc.run([7], 1); + + expect(getPhotoInfo).toHaveBeenCalledTimes(1); + }); + + it('CAPTURE-004: a provider that refuses leaves the row alone and does not throw', async () => { + const { svc, recordCaptureMetadata } = build([{ id: 7 }], {}); + + await expect(svc.run([7], 1)).resolves.toBeUndefined(); + expect(recordCaptureMetadata).not.toHaveBeenCalled(); + }); + + it('CAPTURE-005: one failing photo does not take down the rest of the batch', async () => { + const { svc, recordCaptureMetadata } = build( + [{ id: 7 }, { id: 8 }], + { 8: { takenAt: '2026-03-16T08:00:00Z', lat: null, lng: null } }, + ); + // 7 has no info entry, so getPhotoInfo answers unsuccessfully for it. + + await svc.run([7, 8], 1); + + expect(recordCaptureMetadata).toHaveBeenCalledTimes(1); + expect(recordCaptureMetadata).toHaveBeenCalledWith(8, { + takenAt: '2026-03-16T08:00:00Z', lat: null, lng: null, + }); + }); + + it('CAPTURE-006: an empty batch touches nothing', () => { + const { svc, getPhotoInfo } = build([], {}); + svc.schedule([], 1); + expect(getPhotoInfo).not.toHaveBeenCalled(); + }); +});