Compare commits

...

1 Commits

Author SHA1 Message Date
Santo Shakil 07c9606a76 fix(mobile): show real error when an asset can't be added to an album 2026-07-09 15:52:35 +06:00
7 changed files with 122 additions and 15 deletions
@@ -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(
@@ -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,7 +155,13 @@ class _AddActionButtonState extends ConsumerState<AddActionButton> {
return;
}
if (result.count == 0) {
if (result.count == 0 && 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 if (result.count == 0) {
ImmichToast.show(
context: context,
msg: 'add_to_album_bottom_sheet_already_exists'.tr(namedArgs: {'album': album.name}),
@@ -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,7 +52,13 @@ class FavoriteBottomSheet extends ConsumerWidget {
);
}
if (addedCount != remoteAssets.length) {
if (result.added == 0 && 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 if (result.added != remoteAssets.length) {
ImmichToast.show(
context: context,
msg: 'add_to_album_bottom_sheet_already_exists'.t(args: {"album": album.name}),
@@ -37,11 +37,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> {
@@ -393,9 +401,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());
@@ -409,7 +420,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(
@@ -424,6 +435,7 @@ class ActionNotifier extends Notifier<void> {
count: addedRemote + uploadResult.count,
success: uploadResult.success,
error: uploadResult.error,
failedCount: failedRemote,
);
}
@@ -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,
@@ -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);
}
}
@@ -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']);
});
}