fix removeAlbum + tests

mobile/lib/domain/services/sync.service.dart

@@ -5,7 +5,8 @@ import 'package:immich_mobile/domain/interfaces/album_media.interface.dart';
 import 'package:immich_mobile/domain/interfaces/local_album.interface.dart';
 import 'package:immich_mobile/domain/interfaces/local_album_asset.interface.dart';
 import 'package:immich_mobile/domain/interfaces/local_asset.interface.dart';
-import 'package:immich_mobile/domain/models/asset/asset.model.dart';
+import 'package:immich_mobile/domain/models/asset/asset.model.dart'
+    hide AssetType;
 import 'package:immich_mobile/domain/models/local_album.model.dart';
 import 'package:immich_mobile/utils/diff.dart';
 import 'package:logging/logging.dart';
@@ -114,18 +115,43 @@ class SyncService {
     }
   }
 
-  Future<void> removeLocalAlbum(LocalAlbum album) async {
+  Future<void> removeLocalAlbum(LocalAlbum a) async {
-    _log.info("Removing device album ${album.name}");
+    _log.info("Removing device album ${a.name}");
     try {
+      // Do not request title to speed things up on iOS
+      final filter = albumFilter;
+      filter.setOption(
+        AssetType.image,
+        filter.getOption(AssetType.image).copyWith(needTitle: false),
+      );
+      filter.setOption(
+        AssetType.video,
+        filter.getOption(AssetType.video).copyWith(needTitle: false),
+      );
+      final deviceAlbum =
+          await _albumMediaRepository.refresh(a.id, filter: filter);
+      final assetsToDelete =
+          (await _localAlbumAssetRepository.getAssetsForAlbum(deviceAlbum.id))
+              .map((asset) => asset.localId)
+              .toSet();
+
       // Remove all assets that are only in this particular album
       // We cannot remove all assets in the album because they might be in other albums in iOS
-      final assetsToDelete =
+      final assetsOnlyInAlbum = assetsToDelete.isEmpty
-          await _localAlbumRepository.getAssetIdsOnlyInAlbum(album.id);
+          ? <String>{}
+          : (await _localAlbumRepository.getAssetIdsOnlyInAlbum(deviceAlbum.id))
+              .toSet();
       await _localAlbumRepository.transaction(() async {
-        if (assetsToDelete.isNotEmpty) {
+        // Delete all assets that are only in this particular album
-          await _localAssetRepository.deleteIds(assetsToDelete);
+        await _localAssetRepository.deleteIds(
-        }
+          assetsToDelete.intersection(assetsOnlyInAlbum),
-        await _localAlbumRepository.delete(album.id);
+        );
+        // Unlink the others
+        await _localAlbumAssetRepository.unlinkAssetsFromAlbum(
+          deviceAlbum.id,
+          assetsToDelete.difference(assetsOnlyInAlbum),
+        );
+        await _localAlbumRepository.delete(deviceAlbum.id);
       });
     } catch (e, s) {
       _log.warning("Error while removing device album", e, s);
@@ -312,7 +338,8 @@ class SyncService {
         backupSelection: dbAlbum.backupSelection,
       );
 
-      // Only query for assets unique to album if we have assets to delete
+      // Remove all assets that are only in this particular album
+      // We cannot remove all assets in the album because they might be in other albums in iOS
       final assetsOnlyInAlbum = assetsToDelete.isEmpty
           ? <String>{}
           : (await _localAlbumRepository.getAssetIdsOnlyInAlbum(deviceAlbum.id))
@@ -326,8 +353,7 @@ class SyncService {
           assetsToAdd.map((a) => a.localId),
         );
         await _localAlbumRepository.upsert(updatedAlbum);
-        // Remove all assets that are only in this particular album
+        // Delete all assets that are only in this particular album
-        // We cannot remove all assets in the album because they might be in other albums in iOS
         await _localAssetRepository.deleteIds(
           assetsToDelete.intersection(assetsOnlyInAlbum),
         );

mobile/test/domain/services/sync_service_test.dart

@@ -1,3 +1,4 @@
+import 'package:collection/collection.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:immich_mobile/domain/interfaces/album_media.interface.dart';
 import 'package:immich_mobile/domain/interfaces/local_album.interface.dart';
@@ -120,8 +121,16 @@ void main() {
       when(() => mockLocalAlbumAssetRepo.linkAssetsToAlbum(any(), any()))
           .thenAnswer((_) async => {});
 
+      when(() => mockLocalAlbumAssetRepo.unlinkAssetsFromAlbum(any(), any()))
+          .thenAnswer((_) async => {});
+
       when(() => mockLocalAlbumRepo.upsert(any())).thenAnswer((_) async => {});
 
+      when(() => mockLocalAlbumRepo.delete(any())).thenAnswer((_) async => {});
+
+      when(() => mockLocalAssetRepo.deleteIds(any()))
+          .thenAnswer((_) async => {});
+
       when(() => mockLocalAlbumRepo.transaction<Null>(any()))
           .thenAnswer((_) async {
         final capturedCallback = verify(
@@ -136,28 +145,18 @@ void main() {
     test(
       'album filter should be properly configured with expected settings',
       () {
-        // Access the album filter from the service
         final albumFilter = sut.albumFilter;
 
-        // Verify image option settings
         final imageOption = albumFilter.getOption(AssetType.image);
         expect(imageOption.needTitle, isTrue);
         expect(imageOption.sizeConstraint.ignoreSize, isTrue);
-
-        // Verify video option settings
         final videoOption = albumFilter.getOption(AssetType.video);
         expect(videoOption.needTitle, isTrue);
         expect(videoOption.sizeConstraint.ignoreSize, isTrue);
         expect(videoOption.durationConstraint.allowNullable, isTrue);
-
-        // Verify containsPathModified flag
         expect(albumFilter.containsPathModified, isTrue);
-
-        // Verify time conditions are ignored
         expect(albumFilter.createTimeCond.ignore, isTrue);
         expect(albumFilter.updateTimeCond.ignore, isTrue);
-
-        // Verify ordering
         expect(albumFilter.orders.length, 1);
         expect(
           albumFilter.orders.firstOrNull?.type,
@@ -183,7 +182,6 @@ void main() {
             deviceAlbum.copyWith(updatedAt: earlier),
           );
 
-          // Verify: method returns without making any changes
           expect(result, isFalse);
           verifyNever(() => mockAlbumMediaRepo.getAssetsForAlbum(any()));
           verifyNever(() => mockLocalAssetRepo.upsertAll(any()));
@@ -197,7 +195,6 @@ void main() {
       test(
         'early return when device album has fewer assets than DB album',
         () async {
-          // Execute
           final result = await sut.handleOnlyAssetsAdded(
             dbAlbum,
             deviceAlbum.copyWith(assetCount: dbAlbum.assetCount - 1),
@@ -345,9 +342,9 @@ void main() {
               verify(() => mockLocalAlbumRepo.upsert(captureAny()));
           albumUpsertCall.called(1);
           verify(() => mockLocalAssetRepo.upsertAll(any())).called(1);
-          verify(
+          verify(() =>
-            () => mockLocalAlbumAssetRepo.linkAssetsToAlbum(albumId, any()),
+                  mockLocalAlbumAssetRepo.linkAssetsToAlbum(albumId, any()))
-          ).called(1);
+              .called(1);
 
           final capturedAlbum =
               albumUpsertCall.captured.singleOrNull as LocalAlbum?;
@@ -357,5 +354,83 @@ void main() {
         },
       );
     });
+
+    group('removeLocalAlbum: ', () {
+      test(
+        'removing album with no assets correctly calls delete',
+        () async {
+          when(() => mockLocalAlbumAssetRepo.getAssetsForAlbum(albumId))
+              .thenAnswer((_) async => []);
+          when(() => mockLocalAlbumRepo.getAssetIdsOnlyInAlbum(albumId))
+              .thenAnswer((_) async => []);
+
+          await sut.removeLocalAlbum(deviceAlbum);
+
+          verify(() => mockLocalAssetRepo.deleteIds([])).called(1);
+          verify(() => mockLocalAlbumRepo.delete(albumId)).called(1);
+          verify(
+            () => mockLocalAlbumAssetRepo.unlinkAssetsFromAlbum(albumId, {}),
+          ).called(1);
+        },
+      );
+
+      test(
+        'removing album with assets unique to that album deletes those assets',
+        () async {
+          final uniqueAssetIds = deviceAssets.map((a) => a.localId).toList();
+          when(() => mockLocalAlbumRepo.getAssetIdsOnlyInAlbum(albumId))
+              .thenAnswer((_) async => uniqueAssetIds);
+
+          await sut.removeLocalAlbum(deviceAlbum);
+
+          verify(() => mockLocalAssetRepo.deleteIds(uniqueAssetIds)).called(1);
+          verify(() => mockLocalAlbumRepo.delete(albumId)).called(1);
+          verify(() => mockLocalAlbumAssetRepo.unlinkAssetsFromAlbum(any(), {}))
+              .called(1);
+        },
+      );
+
+      test(
+        'removing album with shared assets unlinks those assets',
+        () async {
+          final assetIds = deviceAssets.map((a) => a.localId).toList();
+          when(() => mockLocalAlbumRepo.getAssetIdsOnlyInAlbum(albumId))
+              .thenAnswer((_) async => []);
+
+          await sut.removeLocalAlbum(deviceAlbum);
+
+          verify(() => mockLocalAssetRepo.deleteIds([])).called(1);
+          verify(
+            () => mockLocalAlbumAssetRepo.unlinkAssetsFromAlbum(
+              albumId,
+              assetIds,
+            ),
+          ).called(1);
+          verify(() => mockLocalAlbumRepo.delete(albumId)).called(1);
+        },
+      );
+
+      test(
+        'removing album with mixed assets (some unique, some shared)',
+        () async {
+          final uniqueAssetIds = ['asset-1', 'asset-2'];
+          final sharedAssetIds = ['asset-0', 'asset-3', 'asset-4'];
+
+          when(() => mockLocalAlbumRepo.getAssetIdsOnlyInAlbum(albumId))
+              .thenAnswer((_) async => uniqueAssetIds);
+
+          await sut.removeLocalAlbum(deviceAlbum);
+
+          verify(() => mockLocalAssetRepo.deleteIds(uniqueAssetIds)).called(1);
+          verify(
+            () => mockLocalAlbumAssetRepo.unlinkAssetsFromAlbum(
+              albumId,
+              sharedAssetIds,
+            ),
+          ).called(1);
+          verify(() => mockLocalAlbumRepo.delete(albumId)).called(1);
+        },
+      );
+    });
   });
 }