fix(mobile): permanently delete local copies when moving to the locked folder (#29730)

* permanently delete local copies when moving to the locked folder

* simplify the delete path and move the stubs into the mock defaults

* drop the kept count from the partial message and log the mismatch instead
This commit is contained in:
Santo Shakil authored and GitHub committed 2026-08-22 07:42:11 +05:30
1 parent 11cdfa9c5e
commit 2237b28813
7 files changed
+130 -19

No files matched your search

+2
View File
@@ -849,6 +849,7 @@
"delete_api_key_prompt": "Are you sure you want to delete this API key?",
"delete_dialog_alert": "These items will be permanently deleted from Immich and from your device",
"delete_dialog_alert_local": "These items will be permanently removed from your device but still be available on the Immich server",
"delete_dialog_alert_local_ios": "These items will be deleted from Photos, but will still be available on the Immich server. They will be in Recently Deleted for 30 days.",
"delete_dialog_alert_local_non_backed_up": "Some of the items aren't backed up to Immich and will be permanently removed from your device",
"delete_dialog_title": "Delete Permanently",
"delete_duplicates_confirmation": "Are you sure you want to permanently delete these duplicates?",
@@ -1482,6 +1483,7 @@
"move_to": "Move to",
"move_to_device_trash": "Move to device trash",
"move_to_lock_folder_action_prompt": "{count} added to the locked folder",
"move_to_lock_folder_partial_prompt": "{count} added to the locked folder, some local copies were kept",
"move_to_locked_folder": "Move to locked folder",
"move_to_locked_folder_confirmation": "These photos and video will be removed from all albums, and only viewable from the locked folder",
"moved_to_trash": "Moved to trash",
@@ -176,17 +176,17 @@ class AssetService {
}
}
Future<int> deleteLocal(List<String> localIds) async {
Future<int> deleteLocal(List<String> localIds, {bool trash = true}) async {
if (localIds.isEmpty) {
return 0;
}
final deletedIds = await _mediaRepository.deleteAll(localIds);
final deletedIds = await _mediaRepository.deleteAll(localIds, trash: trash);
if (deletedIds.isEmpty) {
return 0;
}
if (CurrentPlatform.isAndroid && Store.get(StoreKey.manageLocalMediaAndroid, false)) {
if (trash && CurrentPlatform.isAndroid && Store.get(StoreKey.manageLocalMediaAndroid, false)) {
await _trashedLocalRepository.applyTrashedAssets(deletedIds);
} else {
await _localRepository.deleteAssets(deletedIds);
@@ -1,12 +1,15 @@
import 'package:flutter/material.dart';
import 'package:hooks_riverpod/hooks_riverpod.dart';
import 'package:immich_mobile/constants/enums.dart';
import 'package:immich_mobile/extensions/platform_extensions.dart';
import 'package:immich_mobile/generated/translations.g.dart';
import 'package:immich_mobile/presentation/actions/action.dart';
import 'package:immich_mobile/providers/infrastructure/asset.provider.dart';
import 'package:immich_mobile/providers/infrastructure/toast.provider.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';
import 'package:logging/logging.dart';
typedef _State = ({bool shouldLock, List<String> assetIds, List<String> localIds});
@@ -29,6 +32,8 @@ final _stateProvider = Provider.family.autoDispose<_State?, ActionSource>((ref,
class LockAction extends AssetActionBuilder {
const LockAction({required super.source});
static final Logger _log = Logger('LockAction');
@override
ActionItem? create(BuildContext context, WidgetRef ref) {
final shouldLock = ref.watch(_stateProvider(source).select((state) => state?.shouldLock));
@@ -50,19 +55,44 @@ class LockAction extends AssetActionBuilder {
}
final (:shouldLock, :assetIds, :localIds) = state;
final message = shouldLock
? context.t.move_to_lock_folder_action_prompt(count: assetIds.length)
: context.t.remove_from_lock_folder_action_prompt(count: assetIds.length);
if (shouldLock && localIds.isNotEmpty) {
final confirmed = await showDialog<bool>(
context: context,
builder: (_) => ConfirmDialog(
title: context.t.move_to_locked_folder,
content: CurrentPlatform.isAndroid
? context.t.delete_dialog_alert_local
: context.t.delete_dialog_alert_local_ios,
ok: context.t.confirm,
),
);
if (confirmed != true || !context.mounted) {
return;
}
}
final assetService = ref.read(assetServiceProvider);
final toastService = ref.read(toastServiceProvider);
final clearSelection = ref.read(clearSelectionProvider(source));
try {
await assetService.update(assetIds, visibility: .some(shouldLock ? .locked : .timeline));
var keptCount = 0;
if (localIds.isNotEmpty) {
// A locked asset still sits in the device gallery, so offer to remove the local copy.
await assetService.deleteLocal(localIds);
keptCount = localIds.length - await assetService.deleteLocal(localIds, trash: false);
if (keptCount != 0) {
_log.warning('Only deleted ${localIds.length - keptCount} of ${localIds.length} local copies');
}
}
if (!context.mounted) {
return;
}
final message = shouldLock
? keptCount > 0
? context.t.move_to_lock_folder_partial_prompt(count: assetIds.length)
: context.t.move_to_lock_folder_action_prompt(count: assetIds.length)
: context.t.remove_from_lock_folder_action_prompt(count: assetIds.length);
// Unlocking is a sensitive action and requires an elevated session
final toast = shouldLock
? null
@@ -45,15 +45,11 @@ class AssetMediaRepository {
return false;
}
Future<List<String>> deleteAll(List<String> ids) async {
if (CurrentPlatform.isAndroid) {
if (await _androidSupportsTrash()) {
return PhotoManager.editor.android.moveToTrash(
ids.map((e) => AssetEntity(id: e, width: 1, height: 1, typeInt: 0)).toList(),
);
} else {
return PhotoManager.editor.deleteWithIds(ids);
}
Future<List<String>> deleteAll(List<String> ids, {bool trash = true}) async {
if (trash && CurrentPlatform.isAndroid && await _androidSupportsTrash()) {
return PhotoManager.editor.android.moveToTrash(
ids.map((e) => AssetEntity(id: e, width: 1, height: 1, typeInt: 0)).toList(),
);
}
return PhotoManager.editor.deleteWithIds(ids);
}
+14 -1
View File
@@ -65,6 +65,7 @@ class RepositoryMocks {
_stubAssetApiRepository();
_stubAssetMediaRepository();
_stubDownloadRepository();
_stubTrashedAssetRepository();
_stubPermissionRepository();
}
@@ -86,6 +87,7 @@ class RepositoryMocks {
void _stubLocalAssetRepository() {
when(localAsset.reconcileHashesFromCloudId).thenAnswer((_) async => {});
when(localAsset.updateHashes).thenAnswer((_) async => {});
when(localAsset.deleteAssets).thenAnswer((_) async => {});
}
void _stubNativeSyncApi() {
@@ -97,6 +99,7 @@ class RepositoryMocks {
}
void _stubAssetMediaRepository() {
when(assetMedia.deleteAll).thenAnswer((inv) async => inv.positionalArguments.first as List<String>);
when(assetMedia.shareAssets).thenAnswer((_) async => 1);
when(assetMedia.getOriginalFilename).thenAnswer((_) async => null);
}
@@ -105,6 +108,10 @@ class RepositoryMocks {
when(download.downloadAllAssets).thenAnswer((_) async => const []);
}
void _stubTrashedAssetRepository() {
when(() => trashedAsset.applyTrashedAssets(any())).thenAnswer((_) async => {});
}
void _stubPermissionRepository() {
when(permission.getStatus).thenAnswer((_) async => DevicePermissionStatus.denied);
when(permission.request).thenAnswer((_) async => DevicePermissionStatus.denied);
@@ -254,6 +261,9 @@ extension type const LocalAssetRepositoryStub(MockLocalAssetRepository repo)
Future<void> Function() get updateHashes =>
() => repo.updateHashes(any());
Future<void> Function() get deleteAssets =>
() => repo.deleteAssets(any());
}
extension type const RemoteAssetRepositoryStub(MockRemoteAssetRepository repo)
@@ -357,7 +367,7 @@ extension type const AssetServiceStub(MockAssetService service) implements Stub<
() => service.applyEdits(any(), any());
Future<int> Function() get deleteLocal =>
() => service.deleteLocal(any());
() => service.deleteLocal(any(), trash: any(named: 'trash'));
}
extension type const RemoteAlbumServiceStub(MockRemoteAlbumService service) implements Stub<MockRemoteAlbumService> {
@@ -393,6 +403,9 @@ extension type const AssetApiRepositoryStub(MockAssetApiRepository api) implemen
}
extension type const AssetMediaRepositoryStub(MockAssetMediaRepository api) implements Stub<MockAssetMediaRepository> {
Future<List<String>> Function() get deleteAll =>
() => api.deleteAll(any(), trash: any(named: 'trash'));
Future<int> Function() get shareAssets =>
() => api.shareAssets(
any(),
@@ -1,8 +1,10 @@
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:immich_mobile/domain/models/asset/base_asset.model.dart';
import 'package:immich_mobile/generated/translations.g.dart';
import 'package:immich_mobile/presentation/actions/action.widget.dart';
import 'package:immich_mobile/presentation/actions/lock.action.dart';
import 'package:immich_mobile/widgets/common/confirm_dialog.dart';
import 'package:immich_ui/immich_ui.dart';
import 'package:mocktail/mocktail.dart';
@@ -29,6 +31,13 @@ void main() {
Future<void> pumpLock(WidgetTester tester, Set<BaseAsset> selection) =>
tester.pumpTestAction(context, const LockAction(source: .timeline), overrides: context.selected(selection));
Future<void> respondToDialog(WidgetTester tester, {required bool confirm}) async {
await tester.pumpUntilFound(find.byType(ConfirmDialog));
expect(find.text(StaticTranslations.instance.move_to_locked_folder), findsOneWidget);
await tester.tap(find.text(confirm ? StaticTranslations.instance.confirm : StaticTranslations.instance.cancel));
await tester.pumpAndSettle();
}
group('LockAction', () {
testWidgets('locks the eligible owned assets', (tester) async {
final asset = owned();
@@ -79,8 +88,28 @@ void main() {
final remoteOnly = owned();
await pumpLock(tester, {merged, remoteOnly});
await respondToDialog(tester, confirm: true);
verify(() => assetService.deleteLocal(['local-1'])).called(1);
verify(() => assetService.deleteLocal(['local-1'], trash: false)).called(1);
});
testWidgets('does nothing when the warning is cancelled', (tester) async {
final merged = RemoteAssetFactory.create(ownerId: context.currentUser.id, localId: 'local-1');
await pumpLock(tester, {merged});
await respondToDialog(tester, confirm: false);
verifyNever(() => assetService.update(any(), visibility: any(named: 'visibility')));
verifyNever(() => assetService.deleteLocal(any(), trash: any(named: 'trash')));
});
testWidgets('locks remote-only assets without a warning', (tester) async {
final remoteOnly = owned();
await pumpLock(tester, {remoteOnly});
expect(find.byType(ConfirmDialog), findsNothing);
verify(() => assetService.update([remoteOnly.id], visibility: const .some(.locked))).called(1);
});
testWidgets('leaves the local copies alone when unlocking', (tester) async {
@@ -1,5 +1,13 @@
import 'package:drift/drift.dart' as drift;
import 'package:drift/native.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:immich_mobile/domain/models/store.model.dart';
import 'package:immich_mobile/domain/services/asset.service.dart';
import 'package:immich_mobile/domain/services/store.service.dart';
import 'package:immich_mobile/entities/store.entity.dart';
import 'package:immich_mobile/infrastructure/repositories/db.repository.dart';
import 'package:immich_mobile/infrastructure/repositories/store.repository.dart';
import 'package:mocktail/mocktail.dart';
import '../../infrastructure/repository.mock.dart';
@@ -12,6 +20,20 @@ void main() {
late MockAssetApiRepository apiRepository;
late MockRemoteAssetRepository remoteRepository;
late MockRemoteExifRepository exifRepository;
late Drift db;
setUpAll(() async {
TestWidgetsFlutterBinding.ensureInitialized();
debugDefaultTargetPlatformOverride = TargetPlatform.android;
db = Drift(drift.DatabaseConnection(NativeDatabase.memory(), closeStreamsSynchronously: true));
await StoreService.init(storeRepository: StoreRepository(db));
});
tearDownAll(() async {
debugDefaultTargetPlatformOverride = null;
await Store.clear();
await db.close();
});
setUp(() {
mocks = RepositoryMocks();
@@ -22,13 +44,17 @@ void main() {
sut = AssetService(
remoteRepository: remoteRepository,
exifRepository: exifRepository,
localRepository: MockLocalAssetRepository(),
localRepository: mocks.localAsset.repo,
apiRepository: apiRepository,
mediaRepository: mocks.assetMedia.api,
trashedLocalRepository: mocks.trashedAsset,
);
});
tearDown(() async {
await Store.delete(StoreKey.manageLocalMediaAndroid);
});
group('AssetService.updateDateTime', () {
const ids = ['asset_id_1'];
@@ -78,4 +104,19 @@ void main() {
verifyZeroInteractions(remoteRepository);
});
});
group('AssetService.deleteLocal', () {
const ids = ['l1', 'l2'];
test('permanently deletes local copies without trashing, even when Android trash handling is on', () async {
await Store.put(StoreKey.manageLocalMediaAndroid, true);
final result = await sut.deleteLocal(ids, trash: false);
expect(result, ids.length);
verify(() => mocks.assetMedia.api.deleteAll(ids, trash: false)).called(1);
verify(() => mocks.localAsset.repo.deleteAssets(ids)).called(1);
verifyNever(() => mocks.trashedAsset.applyTrashedAssets(any()));
});
});
}