feat(selection): add batch operations for image management#39
Conversation
Implement batch delete, copy, move, and category assignment for selected images. - Add batch delete handler that sends selected images to trash - Add batch copy handler with folder selection dialog - Add batch move handler with folder selection dialog - Add batch category assignment buttons for each defined category - Filter out directory paths from batch operations to prevent errors - Fix folder icons disappearing when all images are deleted - Add Rust backend commands: copy_image and move_image - Add comprehensive tests for all new batch operations - Add Rust tests for copy_image and move_image functions The batch operations validate that only actual image paths are processed, excluding any directory paths that might be in the selection set.
|
Caution Review failedThe pull request is closed. WalkthroughThis PR adds backend Tauri commands for copying and moving images with tests, and implements frontend batch file actions (delete/copy/move) and batch category toggling in selection mode; it also includes minor UI/formatting changes and a small state cleanup tweak. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as ImageGridSelection
participant Tauri as Tauri Backend
participant FS as File System
participant State as State Manager
User->>UI: Selects images & chooses batch action
rect rgb(220, 240, 255)
Note over UI,Tauri: Batch Copy/Move Flow
UI->>User: Open folder picker
User-->>UI: Select destination
UI->>Tauri: Invoke copy_image/move_image (per-file)
Tauri->>FS: Copy/Move file(s)
FS-->>Tauri: Success/Error
Tauri-->>UI: Result
end
rect rgb(240, 220, 255)
Note over UI,State: State Update & Cleanup
UI->>State: Update loadedImages, allImagePaths
UI->>State: Clear selectedImages
State-->>UI: State updated
end
UI->>User: Show notification (success/error)
sequenceDiagram
participant User
participant UI as ImageGridSelection
participant State as State Manager
rect rgb(240, 255, 220)
Note over UI,State: Batch Category Toggle
User->>UI: Click category checkbox (in selection UI)
UI->>UI: Compute categoryState (useMemo)
alt All images assigned
UI->>State: Unassign category from selected images
else Some/None assigned
UI->>State: Assign category to selected images
end
State-->>UI: imageCategoriesAtom updated
UI->>User: Show contextual notification
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src-tauri/src/lib.rs (2)
349-353:fs::renamefails across filesystems.
fs::renameonly works when source and destination are on the same filesystem. If a user selects a destination on a different mount/drive, this will fail with a confusing error message. Consider implementing a fallback that copies then deletes the source file.// Move the file (rename is used for moving files on the same filesystem) match fs::rename(source_path, &dest_path) { Ok(_) => Ok(()), - Err(e) => Err(format!("Failed to move image: {}", e)), + Err(e) => { + // If rename fails (e.g., cross-filesystem), try copy + delete + if e.raw_os_error() == Some(libc::EXDEV) || e.kind() == std::io::ErrorKind::CrossesDevices { + match fs::copy(source_path, &dest_path) { + Ok(_) => { + fs::remove_file(source_path) + .map_err(|del_err| format!("Copied but failed to remove source: {}", del_err)) + } + Err(copy_err) => Err(format!("Failed to move image: {}", copy_err)), + } + } else { + Err(format!("Failed to move image: {}", e)) + } + } }Note:
std::io::ErrorKind::CrossesDevicesis unstable. On stable Rust, you may need to check the raw OS error code instead (e.g.,EXDEVon Unix,ERROR_NOT_SAME_DEVICEon Windows).
296-303: Consider warning or prompting before overwriting existing files.Both
copy_imageandmove_imagesilently overwrite existing files at the destination. While the tests confirm this is intentional, it could lead to accidental data loss. Consider either:
- Returning an error if the destination file exists (letting the frontend handle the confirmation)
- Adding an
overwrite: boolparameter to control this behavior- Returning the destination path in the result so the frontend can inform the user
This is optional since the tests indicate the behavior is by design.
src/components/ImageGridSelection.tsx (2)
126-144: Sequential deletion is safe but could be slow for large selections.The sequential approach ensures each deletion completes before the next, which is reliable. For better UX with large batches, consider using
Promise.allSettledfor parallel execution:const results = await Promise.allSettled( imagePaths.map(async (imagePath) => { await invokeTauri("delete_image", { imagePath }); deleteFromAtomMap(loadedImagesAtom, imagePath); return imagePath; }) ); // Then batch-update allImagePathsAtom once with all successful pathsThis is optional since sequential is safer and the current implementation works correctly.
429-474: Consider extracting inline styles to CSS classes.The category label styling uses extensive inline styles. For consistency with the rest of the codebase and easier maintenance, consider moving these to CSS classes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src-tauri/src/lib.rs(3 hunks)src/components/CategoryDialog.tsx(9 hunks)src/components/HotkeyList.tsx(1 hunks)src/components/ImageGrid.tsx(1 hunks)src/components/ImageGridSelection.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
Use soft tabs (spaces) for indentation, never hard tabs, following the existing codebase style of typically 2 spaces for TypeScript/React files
Files:
src/components/ImageGrid.tsxsrc/components/HotkeyList.tsxsrc/components/ImageGridSelection.tsxsrc/components/CategoryDialog.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
When calling Tauri commands from TypeScript, use
camelCasefor parameters (e.g.,dataFilePath: value). Do not use snake_case in TypeScript
Files:
src/components/ImageGrid.tsxsrc/components/HotkeyList.tsxsrc/components/ImageGridSelection.tsxsrc/components/CategoryDialog.tsx
**/*.rs
📄 CodeRabbit inference engine (.cursorrules)
When defining Tauri commands in Rust, use
snake_casefor parameters (e.g.,data_file_path: String). Tauri automatically converts to camelCase in TypeScript
Files:
src-tauri/src/lib.rs
🧬 Code graph analysis (3)
src/components/HotkeyList.tsx (1)
src/ui/hotkeys.ts (1)
formatActionLabel(364-411)
src/components/ImageGridSelection.tsx (5)
src/state.ts (6)
selectionModeAtom(55-55)selectedImagesAtom(56-56)allImagePathsAtom(5-5)categoriesAtom(14-14)imageCategoriesAtom(15-15)toggleImageSelectionAtom(57-57)src/utils/jotaiStore.ts (2)
store(6-6)deleteFromAtomMap(38-46)src/utils/tauri.ts (1)
invokeTauri(18-26)src/utils/dialog.ts (1)
open(2-7)src/ui/categories.ts (1)
toggleImageCategory(456-506)
src/components/CategoryDialog.tsx (5)
src/components/NotificationBar.tsx (1)
showError(42-59)src/ui/notification.ts (1)
showError(3-3)src/ui/error.ts (1)
showError(9-11)src/ui/categories.ts (2)
isCategoryNameDuplicate(607-618)saveAppData(167-192)src/ui/hotkeys.ts (1)
autoAssignHotkeyToCategory(327-356)
🔇 Additional comments (12)
src/components/HotkeyList.tsx (1)
43-45: LGTM!Formatting improvement that wraps the JSX content across multiple lines for better readability.
src/components/CategoryDialog.tsx (2)
22-27: LGTM!Good optimization using the native
crypto.randomUUID()when available, with a proper fallback to theuuidlibrary for environments that don't support it.
4-8: LGTM!Import statement formatting for improved readability.
src-tauri/src/lib.rs (3)
269-304: Well-structured command implementation.The
copy_imagecommand has thorough input validation, clear error messages, and properly preserves the filename. The comprehensive test coverage (lines 2082-2194) validates all edge cases.
319-354: Well-structured command implementation with good validation.The
move_imagecommand mirrorscopy_image's validation pattern. Tests at lines 2196-2332 provide excellent coverage including the overwrite scenario.
843-843: LGTM!New commands properly registered in the handler list.
src/components/ImageGrid.tsx (1)
99-100: LGTM!Good fix preserving directory items when all images are cleared. The comment clearly documents the intent, aligning with the PR objective of "preserving folder icons when all images are deleted."
src/components/ImageGridSelection.tsx (5)
21-57: LGTM!The
categoryStatescomputation correctly handles all three checkbox states (checked, unchecked, indeterminate) based on how many selected images have each category assigned. The validation againstallImagePathsproperly excludes directory paths.
167-237: LGTM!The batch copy implementation correctly:
- Opens folder picker and handles both string and array return types
- Filters out directory paths from selection
- Does not clear selection or update state (since source files remain)
- Provides clear error aggregation and notifications
239-321: LGTM!The batch move implementation correctly clears the image from local state after successful moves, since the source file no longer exists. The cleanup pattern matches
handleBatchDelete.
323-387: LGTM!The batch category toggle implementation correctly:
- Determines assign vs unassign based on current checkbox state
- Optimizes by skipping images already in the desired state
- Uses the existing
toggleImageCategoryutility for consistency- Provides clear feedback about the action taken
448-461: Good use of ref callback for indeterminate state.The ref callback pattern correctly sets the
indeterminateproperty on the checkbox, which cannot be set via JSX attributes. This is the proper React approach for this browser API.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #39 +/- ##
========================================
Coverage 94.45% 94.46%
========================================
Files 19 19
Lines 3121 3358 +237
Branches 471 568 +97
========================================
+ Hits 2948 3172 +224
- Misses 173 186 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Implement fallback for fs::rename when moving files across different filesystems or mount points. fs::rename only works on the same filesystem, so when it fails with a cross-device error (EXDEV on Unix, ERROR_NOT_SAME_DEVICE on Windows), the function now falls back to copying the file and then deleting the source. - Add cross-platform error detection for cross-device errors - Implement copy + delete fallback for cross-filesystem moves - Preserve error messages for non-cross-device errors - Handle edge case where copy succeeds but delete fails
Implements batch file actions in selection mode as requested in #29.
Changes
Technical Details
Frontend (TypeScript/React)
handleBatchDelete,handleBatchCopy,handleBatchMovehandlers inImageGridSelection.tsxhandleBatchAssignCategoryfor batch category assignmentImageGrid.tsxto preserve directory items when images are deletedBackend (Rust)
copy_imageTauri command to copy files to destination directorymove_imageTauri command to move files to destination directoryTests
ImageGridSelectioncomponent (25 tests)copy_imageandmove_image(14 tests)Testing
All functionality has been tested and verified:
Resolves #29
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.