-
Notifications
You must be signed in to change notification settings - Fork 365
Feat/android batch move assets #1319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Add moveAssetsToPath API for Android 11+ (API 30+) that uses MediaStore.createWriteRequest() to request user permission for batch file modifications. This provides a better UX by showing a single system permission dialog for multiple assets. Key features: - Batch operations: Move multiple assets with one permission dialog - PhotoManagerWriteManager: New manager following DeleteManager pattern - Activity result handling: Integrated with existing plugin architecture - Android 11+ compatibility: Uses createWriteRequest API - Example included: Sample code demonstrating usage API: PhotoManager.editor.android.moveAssetsToPath( entities: [asset1, asset2, ...], targetPath: 'Pictures/MyAlbum', ) Benefits: - Single permission prompt for 20+ images - Better user experience than individual moves - Follows existing photo_manager patterns - Works alongside existing moveAssetToAnother API Related to Android 11+ scoped storage restrictions where direct ContentResolver.update() is blocked and requires createWriteRequest for modifications.
- Add MoveAssetsBatchTestPage: Interactive UI test page for moveAssetsToPath API - Grid view with multi-select capability - Target path input field - Real-time status updates - Toast notifications for results - Add move_assets_example.dart: Comprehensive usage examples - Basic batch move example - Filtered assets move example (by date) - Important notes and best practices - Update index_page.dart: Add test page to menu This test page allows developers to validate the new moveAssetsToPath API on Android 11+ devices and verify the single permission dialog behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a batch move feature for Android 11+ (API 30+) that allows multiple assets to be moved to a different album with a single user permission dialog, significantly improving user experience when moving multiple files. The implementation leverages Android's MediaStore.createWriteRequest() API for batch operations.
Key Changes:
- Added
moveAssetsToPathmethod for batch moving assets on Android 11+ with single permission dialog - Introduced
PhotoManagerWriteManagerclass to handle write operations and permission requests - Included comprehensive example code and documentation for the new API
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/src/internal/plugin.dart | Added androidMoveAssetsToPath method to handle batch move requests with error handling |
| lib/src/internal/editor.dart | Added public moveAssetsToPath API with documentation and examples |
| lib/src/internal/constants.dart | Registered new method constant mMoveAssetsToPath |
| example/lib/page/move_assets_page.dart | Created interactive test page demonstrating batch move functionality with asset selection |
| example/lib/page/move_assets_example.dart | Added comprehensive documentation examples for the batch move API |
| example/lib/page/index_page.dart | Added navigation entry for the new move assets example page |
| android/.../PhotoManagerWriteManager.kt | Implemented write manager for handling batch permission requests and move operations |
| android/.../PhotoManagerPlugin.kt | Integrated write manager with Android 11+ version checks and URI mapping |
| android/.../Methods.kt | Added moveAssetsToPath method constant |
| android/PhotoManagerPlugin.kt | Registered writeManager as activity result listener for lifecycle management |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| try { | ||
| val pendingIntent = MediaStore.createWriteRequest(cr, uris) | ||
| activity?.startIntentSenderForResult( |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant null-safe call on activity. The method already checks if activity == null at line 167 and returns early if it is. The null-safe call activity?.startIntentSenderForResult is unnecessary here and should be activity.startIntentSenderForResult since activity is guaranteed to be non-null at this point.
This makes the code clearer and avoids confusion about whether the operation could silently fail.
| activity?.startIntentSenderForResult( | |
| activity.startIntentSenderForResult( |
| }, | ||
| ); | ||
| return result == true; | ||
| } catch (e) { |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The catch block silently swallows all exceptions and returns false. This makes debugging difficult as callers cannot distinguish between different failure scenarios (e.g., Android version too low, invalid asset IDs, permission denied, network issues).
Consider logging the exception or propagating specific error information to help with debugging:
} catch (e) {
// Log error for debugging
debugPrint('Failed to move assets: $e');
return false;
}| } catch (e) { | |
| } catch (e) { | |
| debugPrint('Failed to move assets: $e'); |
| val assetIds = call.argument<List<String>>("assetIds")!! | ||
| val targetPath = call.argument<String>("targetPath")!! | ||
|
|
||
| val uris = assetIds.mapNotNull { photoManager.getUri(it) } |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mapNotNull operation doesn't handle exceptions thrown by photoManager.getUri(it). The getUri method can throw a RuntimeException when an asset is not found (line 257 in PhotoManager.kt). These exceptions won't be caught by the outer try-catch block and could crash the operation.
Consider wrapping the mapping operation in a try-catch:
val uris = assetIds.mapNotNull { assetId ->
try {
photoManager.getUri(assetId)
} catch (e: Exception) {
LogUtils.error("Failed to get URI for asset: $assetId", e)
null
}
}| val uris = assetIds.mapNotNull { photoManager.getUri(it) } | |
| val uris = assetIds.mapNotNull { assetId -> | |
| try { | |
| photoManager.getUri(assetId) | |
| } catch (e: Exception) { | |
| LogUtils.error("Failed to get URI for asset: $assetId", e) | |
| null | |
| } | |
| } |
|
|
||
| try { | ||
| val pendingIntent = MediaStore.createWriteRequest(cr, uris) | ||
| activity?.startIntentSenderForResult( |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant null-safe call on activity. The method already checks if activity == null at line 90 and returns early if it is. The null-safe call activity?.startIntentSenderForResult is unnecessary here and should be activity.startIntentSenderForResult since activity is guaranteed to be non-null at this point.
This makes the code clearer and avoids confusion about whether the operation could silently fail.
| activity?.startIntentSenderForResult( | |
| activity.startIntentSenderForResult( |
|
@copilot open a new pull request to apply changes based on the comments in this thread |
This pull request introduces a new batch move feature for Android 11+ (API 30+) in the photo manager plugin, allowing multiple assets to be moved to a different album with a single user permission dialog. The implementation includes both platform-side (Kotlin) and Dart-side changes, as well as an example usage for developers. The main areas of change are the addition of the batch move API, integration of the new manager, and updates to the example app.
Batch Move Feature Implementation
moveAssetsToPathto support batch moving of assets to a target album/folder, which triggers only one system permission dialog for all selected assets. [1] [2]PhotoManagerWriteManagerclass inPhotoManagerWriteManager.ktto handle write requests and batch move operations, including permission handling and actual asset relocation logic.Plugin Integration
writeManageras anActivityResultListenerin the plugin lifecycle, ensuring proper permission and result handling for batch move operations. [1] [2] [3]writeManagerfor context and activity reference management.Example and Documentation
move_assets_example.dartwith clear documentation and sample code demonstrating how to use the new batch move API, including important notes on Android version requirements, path formats, and error handling.