pr feedback

i18n/en.json

@@ -778,6 +778,7 @@
   "clear": "Clear",
   "clear_all": "Clear all",
   "clear_all_recent_searches": "Clear all recent searches",
+  "clear_failed_count": "Clear failed ({count})",
   "clear_file_cache": "Clear File Cache",
   "clear_message": "Clear message",
   "clear_value": "Clear value",

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

@@ -235,11 +235,23 @@ class RemoteAlbumService {
     final localById = {for (final a in localAssets) a.id: a};
 
     final wrappedCallbacks = UploadCallbacks(
-      onProgress: userCallbacks.onProgress,
+      onProgress: (localId, filename, bytes, totalBytes) => _runUploadCallback(
-      onICloudProgress: userCallbacks.onICloudProgress,
+        'Upload progress callback failed for $localId',
-      onError: userCallbacks.onError,
+        () => userCallbacks.onProgress?.call(localId, filename, bytes, totalBytes),
+      ),
+      onICloudProgress: (localId, progress) => _runUploadCallback(
+        'iCloud progress callback failed for $localId',
+        () => userCallbacks.onICloudProgress?.call(localId, progress),
+      ),
+      onError: (localId, errorMessage) => _runUploadCallback(
+        'Upload error callback failed for $localId',
+        () => userCallbacks.onError?.call(localId, errorMessage),
+      ),
       onSuccess: (localId, remoteId) {
-        userCallbacks.onSuccess?.call(localId, remoteId);
+        _runUploadCallback(
+          'Upload success callback failed for $localId',
+          () => userCallbacks.onSuccess?.call(localId, remoteId),
+        );
         final source = localById[localId];
         if (source == null) {
           _logger.warning('Upload success for $localId but source LocalAsset missing; skipping album link');
@@ -262,6 +274,14 @@ class RemoteAlbumService {
     return addedCount;
   }
 
+  void _runUploadCallback(String message, void Function() callback) {
+    try {
+      callback();
+    } catch (error, stack) {
+      _logger.warning(message, error, stack);
+    }
+  }
+
   /// Links a freshly-uploaded asset to an album, ensuring the local DB
   /// reflects the change without waiting for the next sync. We call the API
   /// (server is the source of truth), then upsert a placeholder
@@ -273,6 +293,7 @@ class RemoteAlbumService {
     if (result.added.isEmpty) {
       return 0;
     }
+
     await _repository.upsertRemoteAssetStub(remoteId: remoteId, ownerId: uploader.id, source: source);
     await _repository.addAssets(albumId, result.added);
     return result.added.length;

mobile/lib/infrastructure/repositories/remote_album.repository.dart

@@ -160,7 +160,7 @@ class DriftRemoteAlbumRepository extends DriftDatabaseRepository {
         createdAt: Value(album.createdAt),
         updatedAt: Value(album.updatedAt),
         description: Value(album.description),
-        thumbnailAssetId: Value(album.thumbnailAssetId),
+        thumbnailAssetId: Value(album.thumbnailAssetId ?? (assetIds.isNotEmpty ? assetIds.first : null)),
         isActivityEnabled: Value(album.isActivityEnabled),
         order: Value(album.order),
       );
@@ -275,12 +275,23 @@ class DriftRemoteAlbumRepository extends DriftDatabaseRepository {
   }
 
   Future<int> addAssets(String albumId, List<String> assetIds) async {
+    if (assetIds.isEmpty) {
+      return 0;
+    }
+
     final albumAssets = assetIds.map(
       (assetId) => RemoteAlbumAssetEntityCompanion(albumId: Value(albumId), assetId: Value(assetId)),
     );
 
-    await _db.batch((batch) {
+    await _db.transaction(() async {
-      batch.insertAll(_db.remoteAlbumAssetEntity, albumAssets);
+      await _db.batch((batch) {
+        batch.insertAll(_db.remoteAlbumAssetEntity, albumAssets);
+      });
+
+      final album = _db.update(_db.remoteAlbumEntity)
+        ..where((row) => row.id.equals(albumId) & row.thumbnailAssetId.isNull());
+
+      await album.write(RemoteAlbumEntityCompanion(thumbnailAssetId: Value(assetIds.first)));
     });
 
     return assetIds.length;
@@ -301,7 +312,7 @@ class DriftRemoteAlbumRepository extends DriftDatabaseRepository {
           RemoteAssetEntityCompanion(
             id: Value(remoteId),
             ownerId: Value(ownerId),
-            checksum: Value(source.checksum ?? ''),
+            checksum: Value(source.checksum ?? remoteId),
             name: Value(source.name),
             type: Value(source.type),
             createdAt: Value(source.createdAt),

mobile/lib/presentation/widgets/album/pending_uploads_banner.widget.dart

@@ -157,7 +157,9 @@ class _PendingUploadsSheet extends ConsumerWidget {
     // Auto-dismiss when the queue empties.
     if (pending.isEmpty) {
       WidgetsBinding.instance.addPostFrameCallback((_) {
-        if (Navigator.of(context).canPop()) Navigator.of(context).pop();
+        if (Navigator.of(context).canPop()) {
+          Navigator.of(context).pop();
+        }
       });
       return const SizedBox.shrink();
     }
@@ -185,7 +187,7 @@ class _PendingUploadsSheet extends ConsumerWidget {
                     TextButton.icon(
                       onPressed: () => ref.read(pendingAlbumUploadsProvider(albumId).notifier).clearFailed(),
                       icon: const Icon(Icons.clear_rounded, size: 18),
-                      label: Text('Clear failed ($failedCount)'),
+                      label: Text('clear_failed_count'.t(context: context, args: {'count': failedCount})),
                       style: TextButton.styleFrom(foregroundColor: context.colorScheme.error),
                     ),
                 ],

mobile/lib/providers/album/pending_album_uploads.provider.dart

@@ -12,15 +12,28 @@ class PendingAlbumUpload {
       PendingAlbumUpload(asset: asset, progress: progress ?? this.progress, failed: failed ?? this.failed);
 }
 
-class AlbumPendingUploadsNotifier extends FamilyNotifier<List<PendingAlbumUpload>, String> {
+class AlbumPendingUploadsNotifier extends AutoDisposeFamilyNotifier<List<PendingAlbumUpload>, String> {
+  KeepAliveLink? _keepAliveLink;
+
   @override
-  List<PendingAlbumUpload> build(String albumId) => const [];
+  List<PendingAlbumUpload> build(String albumId) {
+    ref.onDispose(() {
+      _keepAliveLink?.close();
+      _keepAliveLink = null;
+    });
+
+    return const [];
+  }
 
   void enqueue(Iterable<LocalAsset> assets) {
-    if (assets.isEmpty) return;
+    if (assets.isEmpty) {
+      return;
+    }
+
     final existingIds = state.map((e) => e.asset.id).toSet();
     final additions = assets.where((a) => !existingIds.contains(a.id)).map((a) => PendingAlbumUpload(asset: a));
     state = [...state, ...additions];
+    _syncKeepAlive();
   }
 
   void updateProgress(String localAssetId, double progress) {
@@ -28,6 +41,7 @@ class AlbumPendingUploadsNotifier extends FamilyNotifier<List<PendingAlbumUpload
       for (final entry in state)
         if (entry.asset.id == localAssetId) entry.copyWith(progress: progress, failed: false) else entry,
     ];
+    _syncKeepAlive();
   }
 
   void markFailed(String localAssetId) {
@@ -35,18 +49,33 @@ class AlbumPendingUploadsNotifier extends FamilyNotifier<List<PendingAlbumUpload
       for (final entry in state)
         if (entry.asset.id == localAssetId) entry.copyWith(failed: true) else entry,
     ];
+    _syncKeepAlive();
+  }
+
+  void markAllFailed() {
+    state = [for (final entry in state) entry.copyWith(failed: true)];
+    _syncKeepAlive();
   }
 
   void remove(String localAssetId) {
     state = state.where((e) => e.asset.id != localAssetId).toList();
+    _syncKeepAlive();
   }
 
   void clearFailed() {
     state = state.where((e) => !e.failed).toList();
+    _syncKeepAlive();
+  }
+
+  void _syncKeepAlive() {
+    if (state.isEmpty) {
+      _keepAliveLink?.close();
+      _keepAliveLink = null;
+    } else {
+      _keepAliveLink ??= ref.keepAlive();
+    }
   }
 }
 
-final pendingAlbumUploadsProvider =
+final pendingAlbumUploadsProvider = NotifierProvider.autoDispose
-    NotifierProvider.family<AlbumPendingUploadsNotifier, List<PendingAlbumUpload>, String>(
+    .family<AlbumPendingUploadsNotifier, List<PendingAlbumUpload>, String>(AlbumPendingUploadsNotifier.new);
-      AlbumPendingUploadsNotifier.new,
-    );

mobile/lib/providers/infrastructure/remote_album.provider.dart

@@ -1,3 +1,5 @@
+import 'dart:async';
+
 import 'package:collection/collection.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
 import 'package:immich_mobile/domain/models/album/album.model.dart';
@@ -108,8 +110,8 @@ class RemoteAlbumNotifier extends Notifier<RemoteAlbumState> {
   }
 
   /// Creates an album from a heterogeneous asset selection. Already-remote
-  /// assets seed the album immediately; local-only assets are uploaded and
+  /// assets seed the album immediately; local-only assets are uploaded in the
-  /// linked one-by-one as each upload completes.
+  /// background and linked one-by-one as each upload completes.
   Future<RemoteAlbum?> createAlbumWithAssets({
     required String title,
     String? description,
@@ -122,20 +124,22 @@ class RemoteAlbumNotifier extends Notifier<RemoteAlbumState> {
       }
 
       final candidates = RemoteAlbumService.categorizeCandidates(assets);
-      final album = await _remoteAlbumService.createAlbumWithAssets(
+      final album = await _remoteAlbumService.createAlbum(
         title: title,
         owner: currentUser,
         description: description,
-        candidates: candidates,
+        assetIds: candidates.remoteAssetIds,
       );
 
       state = state.copyWith(albums: [...state.albums, album]);
 
-      // The createAlbum API returns the album with its initial asset count, but
-      // any local-only assets are uploaded and linked afterward — re-read to
-      // pick up the post-upload junction rows.
       if (candidates.localAssetsToUpload.isNotEmpty) {
-        await _refreshAlbumInState(album.id);
+        unawaited(
+          addAssetsToAlbum(
+            album.id,
+            candidates.localAssetsToUpload,
+          ).then<void>((_) {}).catchError((Object _, StackTrace _) {}),
+        );
       }
 
       return album;
@@ -236,6 +240,9 @@ class RemoteAlbumNotifier extends Notifier<RemoteAlbumState> {
       }
       return added;
     } catch (error, stack) {
+      if (candidates.localAssetsToUpload.isNotEmpty) {
+        pendingNotifier.markAllFailed();
+      }
       _logger.severe('Failed to add assets to album $albumId', error, stack);
       rethrow;
     }
@@ -246,7 +253,10 @@ class RemoteAlbumNotifier extends Notifier<RemoteAlbumState> {
   /// latest junction-table changes without a full `refresh()`.
   Future<void> _refreshAlbumInState(String albumId) async {
     final updated = await _remoteAlbumService.get(albumId);
-    if (updated == null) return;
+    if (updated == null) {
+      return;
+    }
+
     state = state.copyWith(albums: state.albums.map((album) => album.id == albumId ? updated : album).toList());
   }
 

mobile/test/medium/repositories/remote_album_repository_test.dart

@@ -17,6 +17,33 @@ void main() {
     await ctx.dispose();
   });
 
+  group('addAssets', () {
+    test('sets the first added asset as thumbnail when the album has no thumbnail', () async {
+      final user = await ctx.newUser();
+      final album = await ctx.newRemoteAlbum(ownerId: user.id);
+      final asset = await ctx.newRemoteAsset(ownerId: user.id);
+
+      await sut.addAssets(album.id, [asset.id]);
+
+      final updated = await sut.get(album.id);
+      expect(updated?.thumbnailAssetId, asset.id);
+      expect(updated?.assetCount, 1);
+    });
+
+    test('preserves an existing thumbnail when adding assets', () async {
+      final user = await ctx.newUser();
+      final thumbnail = await ctx.newRemoteAsset(ownerId: user.id);
+      final album = await ctx.newRemoteAlbum(ownerId: user.id, thumbnailAssetId: thumbnail.id);
+      final asset = await ctx.newRemoteAsset(ownerId: user.id);
+
+      await sut.addAssets(album.id, [asset.id]);
+
+      final updated = await sut.get(album.id);
+      expect(updated?.thumbnailAssetId, thumbnail.id);
+      expect(updated?.assetCount, 1);
+    });
+  });
+
   group('getSortedAlbumIds', () {
     late String userId;