fix(mobile): show real error when an asset can't be added to an album (#29754)

* fix(mobile): show real error when an asset can't be added to an album

* fix(mobile): show added toast when an add-to-album partly succeeds

* docs(mobile): explain why a partial add-to-album shows added
This commit is contained in:
Santo Shakil 2026-07-21 21:56:50 +06:00 committed by GitHub
parent ee4bd3f833
commit 943bfafb7a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 131 additions and 22 deletions

View file

@ -175,12 +175,12 @@ class RemoteAlbumService {
return _repository.getAssets(albumId);
}
Future<int> addAssets({required String albumId, required List<String> assetIds}) async {
Future<({int added, int failed})> addAssets({required String albumId, required List<String> 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(

View file

@ -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<AddActionButton> {
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<AddActionButton> {
// 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) {

View file

@ -41,7 +41,7 @@ class FavoriteBottomSheet extends ConsumerWidget {
}
final remoteAssets = selectedAssets.whereType<RemoteAsset>();
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}),
);
}

View file

@ -34,11 +34,19 @@ class ActionResult {
final bool success;
final String? error;
final List<String> 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<void> {
@ -366,9 +374,12 @@ class ActionNotifier extends Notifier<void> {
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<void> {
}
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<void> {
count: addedRemote + uploadResult.count,
success: uploadResult.success,
error: uploadResult.error,
failedCount: failedRemote,
);
}

View file

@ -200,12 +200,12 @@ class RemoteAlbumNotifier extends Notifier<RemoteAlbumState> {
return _remoteAlbumService.getAssets(albumId);
}
Future<int> addAssets(String albumId, List<String> assetIds) async {
final added = await _remoteAlbumService.addAssets(albumId: albumId, assetIds: assetIds);
if (added > 0) {
Future<({int added, int failed})> addAssets(String albumId, List<String> 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,

View file

@ -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);
}
}

View file

@ -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<BulkIdResponseDto> 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']);
});
}