mirror of
https://github.com/immich-app/immich.git
synced 2026-07-09 05:39:33 -07:00
Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 07c9606a76 |
@@ -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']);
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user