fix: still part of live photo hidden on bg upload (#29884)

* fix: still part of live photo hidden on bg upload

* add foreground tests

---------

Co-authored-by: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com>

mobile/lib/services/background_upload.service.dart

@@ -317,6 +317,8 @@ class BackgroundUploadService {
       priority: priority,
       isFavorite: asset.isFavorite,
       requiresWiFi: requiresWiFi,
+      // Visibility hidden on upload to prevent the server from running regular jobs on the live photo asset
+      fields: entity.isLivePhoto ? {'visibility': api.AssetVisibility.hidden.value} : null,
       cloudId: entity.isLivePhoto ? null : asset.cloudId,
       adjustmentTime: entity.isLivePhoto ? null : asset.adjustmentTime?.toIso8601String(),
       latitude: entity.isLivePhoto ? null : asset.latitude?.toString(),
@@ -336,8 +338,7 @@ class BackgroundUploadService {
       return null;
     }
 
-    // Visibility hidden on upload to prevent the server from running regular jobs on the live photo asset
-    final fields = {'livePhotoVideoId': livePhotoVideoId, 'visibility': api.AssetVisibility.hidden.value};
+    final fields = {'livePhotoVideoId': livePhotoVideoId};
 
     final requiresWiFi = _shouldRequireWiFi(asset);
     final originalFileName = await _assetMediaRepository.getOriginalFilename(asset.id) ?? asset.name;

mobile/lib/services/foreground_upload.service.dart

@@ -2,6 +2,7 @@ import 'dart:async';
 import 'dart:convert';
 import 'dart:io';
 
+import 'package:flutter/foundation.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
 import 'package:immich_mobile/domain/models/asset/asset_metadata.model.dart';
 import 'package:immich_mobile/domain/models/asset/base_asset.model.dart' hide AssetVisibility;
@@ -100,7 +101,7 @@ class ForegroundUploadService {
           final requireWifi = _shouldRequireWiFi(asset);
           return requireWifi && !hasWifi;
         },
-        processItem: (asset) => _uploadSingleAsset(asset, cancelToken, callbacks: callbacks),
+        processItem: (asset) => uploadSingleAsset(asset, cancelToken, callbacks: callbacks),
       );
     }
   }
@@ -126,7 +127,7 @@ class ForegroundUploadService {
         continue;
       }
 
-      await _uploadSingleAsset(asset, cancelToken, callbacks: callbacks);
+      await uploadSingleAsset(asset, cancelToken, callbacks: callbacks);
     }
   }
 
@@ -143,7 +144,7 @@ class ForegroundUploadService {
     await _executeWithWorkerPool<LocalAsset>(
       items: localAssets,
       cancelToken: cancelToken,
-      processItem: (asset) => _uploadSingleAsset(asset, cancelToken, callbacks: callbacks),
+      processItem: (asset) => uploadSingleAsset(asset, cancelToken, callbacks: callbacks),
     );
   }
 
@@ -233,7 +234,8 @@ class ForegroundUploadService {
     await Future.wait(workerFutures);
   }
 
-  Future<void> _uploadSingleAsset(
+  @visibleForTesting
+  Future<void> uploadSingleAsset(
     LocalAsset asset,
     Completer<void>? cancelToken, {
     required UploadCallbacks callbacks,

mobile/test/api.mocks.dart

@@ -1,3 +1,4 @@
+import 'package:immich_mobile/platform/connectivity_api.g.dart';
 import 'package:immich_mobile/repositories/partner_api.repository.dart';
 import 'package:mocktail/mocktail.dart';
 import 'package:openapi/api.dart';
@@ -7,3 +8,5 @@ class MockSyncApi extends Mock implements SyncApi {}
 class MockServerApi extends Mock implements ServerApi {}
 
 class MockPartnerApiRepository extends Mock implements PartnerApiRepository {}
+
+class MockConnectivityApi extends Mock implements ConnectivityApi {}

mobile/test/services/background_upload.service_test.dart

@@ -118,8 +118,24 @@ void main() {
       expect(task, isNotNull);
       // For live photos, extension should be changed to match the video file
       expect(task!.fields['filename'], equals('OriginalLivePhoto.mov'));
+      expect(task.fields['visibility'], equals('hidden'));
       verify(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).called(1);
     });
+
+    test('should not set visibility for a regular photo', () async {
+      final asset = LocalAssetStub.image1;
+      final mockEntity = MockAssetEntity();
+      final mockFile = File('/path/to/file.jpg');
+
+      when(() => mockEntity.isLivePhoto).thenReturn(false);
+      when(() => mockStorageRepository.getAssetEntityForAsset(asset)).thenAnswer((_) async => mockEntity);
+      when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => mockFile);
+      when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'Regular.jpg');
+
+      final task = await sut.getUploadTask(asset);
+      expect(task, isNotNull);
+      expect(task!.fields.containsKey('visibility'), isFalse);
+    });
   });
 
   group('getLivePhotoUploadTask', () {
@@ -140,7 +156,7 @@ void main() {
       expect(task, isNotNull);
       expect(task!.fields['filename'], equals('OriginalLivePhoto.HEIC'));
       expect(task.fields['livePhotoVideoId'], equals('video-id-123'));
-      expect(task.fields['visibility'], equals('hidden'));
+      expect(task.fields.containsKey('visibility'), isFalse);
       verify(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).called(1);
     });
 
@@ -334,7 +350,7 @@ void main() {
       expect(task, isNotNull);
       expect(task!.fields.containsKey('metadata'), isTrue);
       expect(task.fields['livePhotoVideoId'], equals('video-123'));
-      expect(task.fields['visibility'], equals('hidden'));
+      expect(task.fields.containsKey('visibility'), isFalse);
 
       final metadata = jsonDecode(task.fields['metadata']!) as List;
       expect(metadata, hasLength(1));

mobile/test/services/foreground_upload.service_test.dart

@@ -0,0 +1,128 @@
+import 'dart:io';
+
+import 'package:drift/drift.dart' hide isNull, isNotNull;
+import 'package:drift/native.dart';
+import 'package:flutter/services.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:immich_mobile/domain/models/store.model.dart';
+import 'package:immich_mobile/domain/services/store.service.dart';
+import 'package:immich_mobile/entities/store.entity.dart';
+import 'package:immich_mobile/infrastructure/repositories/db.repository.dart';
+import 'package:immich_mobile/infrastructure/repositories/settings.repository.dart';
+import 'package:immich_mobile/infrastructure/repositories/store.repository.dart';
+import 'package:immich_mobile/repositories/upload.repository.dart';
+import 'package:immich_mobile/services/foreground_upload.service.dart';
+import 'package:mocktail/mocktail.dart';
+
+import '../api.mocks.dart';
+import '../fixtures/asset.stub.dart';
+import '../infrastructure/repository.mock.dart';
+import '../mocks/asset_entity.mock.dart';
+import '../repository.mocks.dart';
+
+void main() {
+  late ForegroundUploadService sut;
+  late MockUploadRepository mockUploadRepository;
+  late MockStorageRepository mockStorageRepository;
+  late MockDriftBackupRepository mockBackupRepository;
+  late MockConnectivityApi mockConnectivityApi;
+  late MockAssetMediaRepository mockAssetMediaRepository;
+  late Drift db;
+
+  setUpAll(() async {
+    TestWidgetsFlutterBinding.ensureInitialized();
+    TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockMethodCallHandler(
+      const MethodChannel('plugins.flutter.io/path_provider'),
+      (MethodCall methodCall) async => 'test',
+    );
+    db = Drift(DatabaseConnection(NativeDatabase.memory(), closeStreamsSynchronously: true));
+    await StoreService.init(storeRepository: DriftStoreRepository(db));
+    await SettingsRepository.ensureInitialized(db);
+
+    await Store.put(StoreKey.serverEndpoint, 'http://demo.immich.app');
+    await Store.put(StoreKey.deviceId, 'device-id');
+
+    registerFallbackValue(File('file'));
+    registerFallbackValue(<String, String>{});
+  });
+
+  setUp(() {
+    mockUploadRepository = MockUploadRepository();
+    mockStorageRepository = MockStorageRepository();
+    mockBackupRepository = MockDriftBackupRepository();
+    mockConnectivityApi = MockConnectivityApi();
+    mockAssetMediaRepository = MockAssetMediaRepository();
+
+    sut = ForegroundUploadService(
+      mockUploadRepository,
+      mockStorageRepository,
+      mockBackupRepository,
+      mockConnectivityApi,
+      mockAssetMediaRepository,
+    );
+  });
+
+  List<Map<String, String>> captureFields() {
+    final captured = <Map<String, String>>[];
+    when(
+      () => mockUploadRepository.uploadFile(
+        file: any(named: 'file'),
+        originalFileName: any(named: 'originalFileName'),
+        fields: any(named: 'fields'),
+        cancelToken: any(named: 'cancelToken'),
+        onProgress: any(named: 'onProgress'),
+        logContext: any(named: 'logContext'),
+      ),
+    ).thenAnswer((invocation) async {
+      final fields = invocation.namedArguments[#fields] as Map<String, String>;
+      captured.add(Map.of(fields));
+      return UploadResult.success(remoteAssetId: 'remote-${captured.length}');
+    });
+    return captured;
+  }
+
+  group('uploadSingleAsset', () {
+    test('should upload the motion part hidden and keep the still image visible', () async {
+      final asset = LocalAssetStub.image1;
+      final mockEntity = MockAssetEntity();
+      final stillFile = File('/path/to/still.heic');
+      final videoFile = File('/path/to/motion.mov');
+
+      when(() => mockEntity.isLivePhoto).thenReturn(true);
+      when(() => mockStorageRepository.getAssetEntityForAsset(asset)).thenAnswer((_) async => mockEntity);
+      when(() => mockStorageRepository.isAssetAvailableLocally(asset.id)).thenAnswer((_) async => true);
+      when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => stillFile);
+      when(() => mockStorageRepository.getMotionFileForAsset(asset)).thenAnswer((_) async => videoFile);
+      when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'live.heic');
+
+      final captured = captureFields();
+
+      await sut.uploadSingleAsset(asset, null, callbacks: const UploadCallbacks());
+
+      expect(captured, hasLength(2));
+      expect(captured[0]['visibility'], equals('hidden'));
+      expect(captured[0].containsKey('livePhotoVideoId'), isFalse);
+      expect(captured[1].containsKey('visibility'), isFalse);
+      expect(captured[1]['livePhotoVideoId'], equals('remote-1'));
+    });
+
+    test('should not set visibility for a regular photo', () async {
+      final asset = LocalAssetStub.image1;
+      final mockEntity = MockAssetEntity();
+      final stillFile = File('/path/to/photo.jpg');
+
+      when(() => mockEntity.isLivePhoto).thenReturn(false);
+      when(() => mockStorageRepository.getAssetEntityForAsset(asset)).thenAnswer((_) async => mockEntity);
+      when(() => mockStorageRepository.isAssetAvailableLocally(asset.id)).thenAnswer((_) async => true);
+      when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => stillFile);
+      when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'photo.jpg');
+
+      final captured = captureFields();
+
+      await sut.uploadSingleAsset(asset, null, callbacks: const UploadCallbacks());
+
+      expect(captured, hasLength(1));
+      expect(captured[0].containsKey('visibility'), isFalse);
+    });
+  });
+}

server/src/schema/migrations/1782500000000-RestoreLivePhotoStillVisibility.ts

@@ -0,0 +1,16 @@
+import { Kysely, sql } from 'kysely';
+
+export async function up(db: Kysely<any>): Promise<void> {
+  // (#29836) Reset the visibility of any still images that are hidden and have a motion part
+  await sql`
+    UPDATE "asset"
+    SET "visibility" = 'timeline'
+    WHERE "type" = 'IMAGE'
+      AND "visibility" = 'hidden'
+      AND "livePhotoVideoId" IS NOT NULL
+  `.execute(db);
+}
+
+export async function down(): Promise<void> {
+  // Not implemented: the previous 'hidden' value was itself the bug.
+}