diff --git a/mobile/lib/presentation/actions/delete.action.dart b/mobile/lib/presentation/actions/delete.action.dart index 7edac9aa4..9f2ca092f 100644 --- a/mobile/lib/presentation/actions/delete.action.dart +++ b/mobile/lib/presentation/actions/delete.action.dart @@ -10,12 +10,13 @@ import 'package:immich_mobile/providers/infrastructure/store.provider.dart'; import 'package:immich_mobile/providers/infrastructure/toast.provider.dart'; import 'package:immich_mobile/providers/server_info.provider.dart'; import 'package:immich_mobile/providers/user.provider.dart'; +import 'package:immich_mobile/repositories/permission.repository.dart'; import 'package:immich_mobile/services/cleanup.service.dart'; import 'package:immich_mobile/services/toast.service.dart'; import 'package:immich_mobile/utils/error_handler.dart'; import 'package:immich_mobile/widgets/common/confirm_dialog.dart'; -typedef _State = ({List localIds, List remoteIds, bool trash}); +typedef _State = ({List localIds, List remoteIds, bool trash, bool isLocalOnly}); final _stateProvider = Provider.family.autoDispose<_State?, ActionSource>((ref, source) { final assets = ref.watch(assetsActionProvider(source)); @@ -40,8 +41,10 @@ final _stateProvider = Provider.family.autoDispose<_State?, ActionSource>((ref, // Assets already in the trash or in the locked folder are deleted outright, irrespective of the server setting. final trash = ownedRemote.isEmpty || (trashEnabled && !ownedRemote.every((asset) => asset.isTrashed || asset.isLocked)); + final remoteIds = ownedRemote.map((asset) => asset.id).toList(growable: false); + final isLocalOnly = assets.any((asset) => asset.isLocalOnly); - return (localIds: localIds, remoteIds: ownedRemote.map((asset) => asset.id).toList(growable: false), trash: trash); + return (localIds: localIds, remoteIds: remoteIds, trash: trash, isLocalOnly: isLocalOnly); }, dependencies: [assetsActionProvider]); class DeleteAction extends AssetActionBuilder { @@ -67,7 +70,7 @@ class DeleteAction extends AssetActionBuilder { return; } - final (:localIds, :remoteIds, :trash) = state; + final (:localIds, :remoteIds, :trash, :isLocalOnly) = state; final assetService = ref.read(assetServiceProvider); final toastService = ref.read(toastServiceProvider); final clearSelection = ref.read(clearSelectionProvider(source)); @@ -77,9 +80,9 @@ class DeleteAction extends AssetActionBuilder { // Only trashing is reversible; a permanent delete and a device cleanup are not. ToastOption? undo; if (remoteIds.isEmpty) { - message = await _removeLocalAssets(context, ref, localIds); + message = await _removeLocalAssets(context, ref, localIds, isLocalOnly: isLocalOnly); } else if (trash) { - message = await _moveToTrash(context, ref, remoteIds, localIds); + message = await _moveToTrash(context, ref, remoteIds, localIds, isLocalOnly: isLocalOnly); undo = .new(onUndo: () => assetService.restoreTrash(remoteIds)); } else { message = await _deletePermanently(context, ref, remoteIds, localIds); @@ -96,8 +99,13 @@ class DeleteAction extends AssetActionBuilder { } } - Future _removeLocalAssets(BuildContext context, WidgetRef ref, List localIds) async { - final count = await _cleanupLocalAssets(context, ref, localIds); + Future _removeLocalAssets( + BuildContext context, + WidgetRef ref, + List localIds, { + required bool isLocalOnly, + }) async { + final count = await _cleanupLocalAssets(context, ref, localIds, isLocalOnly: isLocalOnly); if (count <= 0 || !context.mounted) { return null; } @@ -109,11 +117,12 @@ class DeleteAction extends AssetActionBuilder { BuildContext context, WidgetRef ref, List remoteIds, - List localIds, - ) async { + List localIds, { + required bool isLocalOnly, + }) async { final assetService = ref.read(assetServiceProvider); if (localIds.isNotEmpty) { - await _cleanupLocalAssets(context, ref, localIds); + await _cleanupLocalAssets(context, ref, localIds, isLocalOnly: isLocalOnly); if (!context.mounted) { return null; } @@ -201,38 +210,64 @@ class CleanupLocalAction extends AssetActionBuilder { } /// Removes the device copies of [assetIds], returning how many were deleted. -/// -/// iOS and Android without MANAGE_MEDIA prompt the user -/// with MANAGE_MEDIA, we do it ourselves unless [requestCustomPrompt] is false. Future _cleanupLocalAssets( BuildContext context, WidgetRef ref, List assetIds, { bool requestCustomPrompt = true, + bool isLocalOnly = false, }) async { if (assetIds.isEmpty) { return 0; } final cleanupService = ref.read(cleanupServiceProvider); - final requiresPrompt = - requestCustomPrompt && - CurrentPlatform.isAndroid && - ref.read(storeServiceProvider).get(.manageLocalMediaAndroid, false); - - if (requiresPrompt) { - final confirmed = await showDialog( - context: context, - builder: (_) => ConfirmDialog( - title: context.t.move_to_device_trash, - content: context.t.free_up_space_description, - ok: context.t.ok, - ), - ); - if (confirmed != true) { - return 0; - } + if (!await _allowsDeletion(context, ref, requestCustomPrompt: requestCustomPrompt, isLocalOnly: isLocalOnly)) { + return 0; } return cleanupService.deleteLocalAssets(assetIds); } + +/// iOS always shows a prompt and adds to on device trash +/// Android varies by API version: +/// < 30: Does not prompt by default. Immediately deletes. We provide a prompt to notify the user +/// 30: Prompts by default. Immediately deletes +/// > 30: When `MANAGE_MEDIA` is present, does not prompt by default. When `MANAGE_MEDIA` is not present, does prompt +/// by default. Either way adds to trash. We provide a prompt to notify the user in the `MANAGE_MEDIA` case +Future _allowsDeletion( + BuildContext context, + WidgetRef ref, { + required bool requestCustomPrompt, + required bool isLocalOnly, +}) async { + if (!requestCustomPrompt || !CurrentPlatform.isAndroid) { + return true; + } + + final manageMedia = ref.read(storeServiceProvider).get(.manageLocalMediaAndroid, false); + final versionRequiresPrompt = await ref.read(permissionRepositoryProvider).getAndroidSdkVersion() < 30; + if (!context.mounted) { + return false; + } + + final ConfirmDialog dialog; + if (versionRequiresPrompt) { + dialog = ConfirmDialog( + title: context.t.delete_dialog_title, + content: isLocalOnly ? context.t.delete_dialog_alert_local_non_backed_up : context.t.delete_dialog_alert_local, + ok: isLocalOnly ? context.t.delete_local_dialog_ok_force : context.t.delete_permanently, + ); + } else if (manageMedia) { + dialog = ConfirmDialog( + title: context.t.move_to_device_trash, + content: context.t.free_up_space_description, + ok: context.t.ok, + ); + } else { + return true; + } + + final confirmed = await showDialog(context: context, builder: (_) => dialog); + return confirmed == true; +} diff --git a/mobile/test/unit/presentation/actions/delete_action_test.dart b/mobile/test/unit/presentation/actions/delete_action_test.dart index b68e2096d..d949e2ae5 100644 --- a/mobile/test/unit/presentation/actions/delete_action_test.dart +++ b/mobile/test/unit/presentation/actions/delete_action_test.dart @@ -205,6 +205,8 @@ void main() { group('prompt handling', () { testWidgets('permanent delete shows a single app dialog', (tester) async { + debugDefaultTargetPlatformOverride = TargetPlatform.android; + when(context.repository.permission.getAndroidSdkVersion).thenAnswer((_) async => 28); final asset = owned(localId: 'local'); await pumpDelete(tester, {asset}, trashEnabled: false); @@ -214,9 +216,10 @@ void main() { await tester.tap(find.byType(TextButton).at(1)); await tester.pumpAndSettle(); - expect(find.text(StaticTranslations.instance.move_to_device_trash), findsNothing); + expect(find.byType(ConfirmDialog), findsNothing); verify(() => assetService.delete([asset.id])).called(1); verify(() => cleanupService.deleteLocalAssets(['local'])).called(1); + debugDefaultTargetPlatformOverride = null; }); testWidgets('local only delete on Android with MANAGE_MEDIA shows the prompt', (tester) async { @@ -247,6 +250,49 @@ void main() { verifyNever(() => cleanupService.deleteLocalAssets(any())); debugDefaultTargetPlatformOverride = null; }); + + testWidgets('local only delete on Android 30 shows no Flutter prompt', (tester) async { + debugDefaultTargetPlatformOverride = TargetPlatform.android; + when(context.repository.permission.getAndroidSdkVersion).thenAnswer((_) async => 30); + final asset = LocalAssetFactory.create(); + + await pumpDelete(tester, {asset}); + await tester.pumpAndSettle(); + + expect(find.byType(ConfirmDialog), findsNothing); + verify(() => cleanupService.deleteLocalAssets([asset.id])).called(1); + debugDefaultTargetPlatformOverride = null; + }); + + testWidgets('backed up device copy below API 30 shows the delete prompt, not the backup warning', (tester) async { + debugDefaultTargetPlatformOverride = TargetPlatform.android; + when(context.repository.permission.getAndroidSdkVersion).thenAnswer((_) async => 28); + final asset = LocalAssetFactory.create(remoteId: 'remote'); + + await pumpDelete(tester, {asset}); + await tester.pump(const Duration(milliseconds: 300)); + expect(find.text(StaticTranslations.instance.delete_dialog_alert_local), findsOneWidget); + await respondToDialog(tester, confirm: true); + + verify(() => cleanupService.deleteLocalAssets([asset.id])).called(1); + debugDefaultTargetPlatformOverride = null; + }); + + testWidgets('mixed selection below API 30 warns about the local only asset', (tester) async { + debugDefaultTargetPlatformOverride = TargetPlatform.android; + when(context.repository.permission.getAndroidSdkVersion).thenAnswer((_) async => 28); + final backedUp = owned(localId: 'local'); + final localOnly = LocalAssetFactory.create(); + + await pumpDelete(tester, {backedUp, localOnly}); + await tester.pump(const Duration(milliseconds: 300)); + expect(find.text(StaticTranslations.instance.delete_dialog_alert_local_non_backed_up), findsOneWidget); + await respondToDialog(tester, confirm: true); + + verify(() => assetService.trash([backedUp.id])).called(1); + verify(() => cleanupService.deleteLocalAssets(['local', localOnly.id])).called(1); + debugDefaultTargetPlatformOverride = null; + }); }); testWidgets('is hidden when nothing can be deleted', (tester) async { @@ -284,5 +330,23 @@ void main() { expect(find.byType(ImmichIconButton), findsNothing); }); + + testWidgets('asks before deleting on Android below API 30', (tester) async { + debugDefaultTargetPlatformOverride = TargetPlatform.android; + when(context.repository.permission.getAndroidSdkVersion).thenAnswer((_) async => 28); + final asset = LocalAssetFactory.create(remoteId: 'remote'); + + await tester.pumpTestAction( + context, + const CleanupLocalAction(source: .timeline), + overrides: context.selected({asset}), + ); + await tester.pump(const Duration(milliseconds: 300)); + expect(find.text(StaticTranslations.instance.delete_dialog_alert_local), findsOneWidget); + await respondToDialog(tester, confirm: true); + + verify(() => cleanupService.deleteLocalAssets([asset.id])).called(1); + debugDefaultTargetPlatformOverride = null; + }); }); } diff --git a/mobile/test/unit/presentation/presentation_context.dart b/mobile/test/unit/presentation/presentation_context.dart index 0bf18ed57..194d36ab0 100644 --- a/mobile/test/unit/presentation/presentation_context.dart +++ b/mobile/test/unit/presentation/presentation_context.dart @@ -25,6 +25,7 @@ import 'package:immich_mobile/providers/routes.provider.dart'; import 'package:immich_mobile/providers/timeline/multiselect.provider.dart'; import 'package:immich_mobile/providers/user.provider.dart'; import 'package:immich_mobile/repositories/asset_media.repository.dart'; +import 'package:immich_mobile/repositories/permission.repository.dart'; import 'package:immich_mobile/services/cleanup.service.dart'; import 'package:immich_mobile/services/gcast.service.dart'; import 'package:immich_mobile/services/server_info.service.dart'; @@ -64,6 +65,7 @@ class PresentationContext { serverInfoServiceProvider.overrideWithValue(service.serverInfo), inLockedViewProvider.overrideWithValue(false), assetMediaRepositoryProvider.overrideWithValue(repository.assetMedia.api), + permissionRepositoryProvider.overrideWithValue(repository.permission.repo), tagApiRepositoryProvider.overrideWithValue(service.tag.repo), ];