diff --git a/mobile/lib/domain/services/remote_album.service.dart b/mobile/lib/domain/services/remote_album.service.dart index 3526c546d9..5141bb455f 100644 --- a/mobile/lib/domain/services/remote_album.service.dart +++ b/mobile/lib/domain/services/remote_album.service.dart @@ -175,12 +175,12 @@ class RemoteAlbumService { return _repository.getAssets(albumId); } - Future addAssets({required String albumId, required List assetIds}) async { + Future<({int added, int failed})> addAssets({required String albumId, required List assetIds}) async { final album = await _albumApiRepository.addAssets(albumId, assetIds); await _repository.addAssets(albumId, album.added); - return album.added.length; + return (added: album.added.length, failed: album.failed.length); } /// !TODO The name here is not clear as we have addAssets method above, @@ -196,7 +196,7 @@ class RemoteAlbumService { }) async { int addedCount = 0; if (candidates.remoteAssetIds.isNotEmpty) { - addedCount += await addAssets(albumId: albumId, assetIds: candidates.remoteAssetIds); + addedCount += (await addAssets(albumId: albumId, assetIds: candidates.remoteAssetIds)).added; } if (candidates.localAssetsToUpload.isNotEmpty) { addedCount += await _uploadAndAddLocals( diff --git a/mobile/lib/presentation/widgets/action_buttons/add_action_button.widget.dart b/mobile/lib/presentation/widgets/action_buttons/add_action_button.widget.dart index 1ab3f2039d..dc48ed57ec 100644 --- a/mobile/lib/presentation/widgets/action_buttons/add_action_button.widget.dart +++ b/mobile/lib/presentation/widgets/action_buttons/add_action_button.widget.dart @@ -2,6 +2,7 @@ import 'package:easy_localization/easy_localization.dart'; import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/extensions/build_context_extensions.dart'; +import 'package:immich_mobile/extensions/translate_extensions.dart'; import 'package:immich_mobile/presentation/widgets/action_buttons/base_action_button.widget.dart'; import 'package:immich_mobile/presentation/widgets/action_buttons/unarchive_action_button.widget.dart'; import 'package:immich_mobile/providers/asset_viewer/asset_viewer.provider.dart'; @@ -154,12 +155,8 @@ class _AddActionButtonState extends ConsumerState { return; } - if (result.count == 0) { - ImmichToast.show( - context: context, - msg: 'add_to_album_bottom_sheet_already_exists'.tr(namedArgs: {'album': album.name}), - ); - } else { + // Only report the failure when nothing was added; if some succeeded we show "added". + if (result.count > 0) { ImmichToast.show( context: context, msg: 'add_to_album_bottom_sheet_added'.tr(namedArgs: {'album': album.name}), @@ -167,6 +164,17 @@ class _AddActionButtonState extends ConsumerState { // Refresh the "Appears in" list on the asset's info panel. ref.invalidate(albumsContainingAssetProvider(latest.remoteId!)); + } else if (result.failedCount > 0) { + ImmichToast.show( + context: context, + msg: 'assets_cannot_be_added_to_album_count'.t(context: context, args: {'count': result.failedCount}), + toastType: ToastType.error, + ); + } else { + ImmichToast.show( + context: context, + msg: 'add_to_album_bottom_sheet_already_exists'.tr(namedArgs: {'album': album.name}), + ); } if (!context.mounted) { diff --git a/mobile/lib/presentation/widgets/bottom_sheet/favorite_bottom_sheet.widget.dart b/mobile/lib/presentation/widgets/bottom_sheet/favorite_bottom_sheet.widget.dart index 8438d5e8ac..bcb9fc6fe3 100644 --- a/mobile/lib/presentation/widgets/bottom_sheet/favorite_bottom_sheet.widget.dart +++ b/mobile/lib/presentation/widgets/bottom_sheet/favorite_bottom_sheet.widget.dart @@ -41,7 +41,7 @@ class FavoriteBottomSheet extends ConsumerWidget { } final remoteAssets = selectedAssets.whereType(); - final addedCount = await ref + final result = await ref .read(remoteAlbumProvider.notifier) .addAssets(album.id, remoteAssets.map((e) => e.id).toList()); @@ -52,15 +52,22 @@ class FavoriteBottomSheet extends ConsumerWidget { ); } - if (addedCount != remoteAssets.length) { + // Only report the failure when nothing was added; if some succeeded we show "added". + if (result.added > 0) { ImmichToast.show( context: context, - msg: 'add_to_album_bottom_sheet_already_exists'.t(args: {"album": album.name}), + msg: 'add_to_album_bottom_sheet_added'.t(args: {"album": album.name}), + ); + } else if (result.failed > 0) { + ImmichToast.show( + context: context, + msg: 'assets_cannot_be_added_to_album_count'.t(context: context, args: {'count': result.failed}), + toastType: ToastType.error, ); } else { ImmichToast.show( context: context, - msg: 'add_to_album_bottom_sheet_added'.t(args: {"album": album.name}), + msg: 'add_to_album_bottom_sheet_already_exists'.t(args: {"album": album.name}), ); } diff --git a/mobile/lib/providers/infrastructure/action.provider.dart b/mobile/lib/providers/infrastructure/action.provider.dart index d1b2c3af21..52e2d9e0b9 100644 --- a/mobile/lib/providers/infrastructure/action.provider.dart +++ b/mobile/lib/providers/infrastructure/action.provider.dart @@ -34,11 +34,19 @@ class ActionResult { final bool success; final String? error; final List remoteAssetIds; + final int failedCount; - const ActionResult({required this.count, required this.success, this.error, this.remoteAssetIds = const []}); + const ActionResult({ + required this.count, + required this.success, + this.error, + this.remoteAssetIds = const [], + this.failedCount = 0, + }); @override - String toString() => 'ActionResult(count: $count, success: $success, error: $error, remoteAssetIds: $remoteAssetIds)'; + String toString() => + 'ActionResult(count: $count, success: $success, error: $error, remoteAssetIds: $remoteAssetIds, failedCount: $failedCount)'; } class ActionNotifier extends Notifier { @@ -366,9 +374,12 @@ class ActionNotifier extends Notifier { final albumNotifier = ref.read(remoteAlbumProvider.notifier); int addedRemote = 0; + int failedRemote = 0; if (remoteIds.isNotEmpty) { try { - addedRemote = await albumNotifier.addAssets(album.id, remoteIds); + final result = await albumNotifier.addAssets(album.id, remoteIds); + addedRemote = result.added; + failedRemote = result.failed; } catch (error, stack) { _logger.severe('Failed to add assets to album ${album.id}', error, stack); return ActionResult(count: 0, success: false, error: error.toString()); @@ -382,7 +393,7 @@ class ActionNotifier extends Notifier { } if (localAssets.isEmpty) { - return ActionResult(count: addedRemote, success: true); + return ActionResult(count: addedRemote, success: true, failedCount: failedRemote); } final uploadResult = await upload( @@ -397,6 +408,7 @@ class ActionNotifier extends Notifier { count: addedRemote + uploadResult.count, success: uploadResult.success, error: uploadResult.error, + failedCount: failedRemote, ); } diff --git a/mobile/lib/providers/infrastructure/remote_album.provider.dart b/mobile/lib/providers/infrastructure/remote_album.provider.dart index 498fdd50dc..b392f5f401 100644 --- a/mobile/lib/providers/infrastructure/remote_album.provider.dart +++ b/mobile/lib/providers/infrastructure/remote_album.provider.dart @@ -200,12 +200,12 @@ class RemoteAlbumNotifier extends Notifier { return _remoteAlbumService.getAssets(albumId); } - Future addAssets(String albumId, List assetIds) async { - final added = await _remoteAlbumService.addAssets(albumId: albumId, assetIds: assetIds); - if (added > 0) { + Future<({int added, int failed})> addAssets(String albumId, List assetIds) async { + final result = await _remoteAlbumService.addAssets(albumId: albumId, assetIds: assetIds); + if (result.added > 0) { await _refreshAlbumInState(albumId); } - return added; + return result; } /// Links a freshly-uploaded local asset to an album using its new remote ID, diff --git a/mobile/lib/repositories/drift_album_api_repository.dart b/mobile/lib/repositories/drift_album_api_repository.dart index ee57352fb1..3a654b7511 100644 --- a/mobile/lib/repositories/drift_album_api_repository.dart +++ b/mobile/lib/repositories/drift_album_api_repository.dart @@ -59,7 +59,7 @@ class DriftAlbumApiRepository extends ApiRepository { for (final dto in response) { if (dto.success) { added.add(dto.id); - } else { + } else if (dto.error.orElse(null) != BulkIdErrorReason.duplicate) { failed.add(dto.id); } } diff --git a/mobile/test/repositories/drift_album_api_repository_test.dart b/mobile/test/repositories/drift_album_api_repository_test.dart new file mode 100644 index 0000000000..93c45c6687 --- /dev/null +++ b/mobile/test/repositories/drift_album_api_repository_test.dart @@ -0,0 +1,82 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_mobile/repositories/drift_album_api_repository.dart'; +import 'package:mocktail/mocktail.dart'; +import 'package:openapi/api.dart'; + +class _MockAlbumsApi extends Mock implements AlbumsApi {} + +void main() { + late _MockAlbumsApi api; + late DriftAlbumApiRepository repo; + + setUpAll(() { + registerFallbackValue(BulkIdsDto(ids: const [])); + }); + + setUp(() { + api = _MockAlbumsApi(); + repo = DriftAlbumApiRepository(api); + }); + + void stubResponse(List response) { + when( + () => api.addAssetsToAlbum(any(), any(), abortTrigger: any(named: 'abortTrigger')), + ).thenAnswer((_) async => response); + } + + test('no_permission failure surfaces as failed, not added (the #22342 bug)', () async { + stubResponse([ + BulkIdResponseDto(id: 'a1', success: false, error: const Optional.present(BulkIdErrorReason.noPermission)), + ]); + + final result = await repo.addAssets('album1', ['a1']); + + expect(result.added, isEmpty); + expect(result.failed, ['a1']); + }); + + test('duplicate is neither added nor failed (genuinely already in album)', () async { + stubResponse([ + BulkIdResponseDto(id: 'a1', success: false, error: const Optional.present(BulkIdErrorReason.duplicate)), + ]); + + final result = await repo.addAssets('album1', ['a1']); + + expect(result.added, isEmpty); + expect(result.failed, isEmpty); + }); + + test('success is added', () async { + stubResponse([BulkIdResponseDto(id: 'a1', success: true)]); + + final result = await repo.addAssets('album1', ['a1']); + + expect(result.added, ['a1']); + expect(result.failed, isEmpty); + }); + + test('not_found and unknown count as failures', () async { + stubResponse([ + BulkIdResponseDto(id: 'a1', success: false, error: const Optional.present(BulkIdErrorReason.notFound)), + BulkIdResponseDto(id: 'a2', success: false, error: const Optional.present(BulkIdErrorReason.unknown)), + ]); + + final result = await repo.addAssets('album1', ['a1', 'a2']); + + expect(result.added, isEmpty); + expect(result.failed, ['a1', 'a2']); + }); + + test('mixed: added kept, no_permission failed, duplicate dropped', () async { + stubResponse([ + BulkIdResponseDto(id: 'ok', success: true), + BulkIdResponseDto(id: 'perm', success: false, error: const Optional.present(BulkIdErrorReason.noPermission)), + BulkIdResponseDto(id: 'dup', success: false, error: const Optional.present(BulkIdErrorReason.duplicate)), + ]); + + final result = await repo.addAssets('album1', ['ok', 'perm', 'dup']); + + expect(result.added, ['ok']); + expect(result.failed, ['perm']); + }); +}