feat: Add Batch Operations for Images #663
Conversation
WalkthroughImplements batch operations (delete, tag, move) for images with Redux-based selection state, frontend UI components including toolbar and dialogs, keyboard shortcuts (Ctrl+A, Escape), checkbox selection on image cards, and corresponding backend API endpoints with database functions and request/response schemas. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as UI Components
participant Redux as Redux Store
participant Backend as Backend API
participant DB as Database
User->>UI: Click "Select" in Navbar
UI->>Redux: dispatch(setSelectionMode(true))
Redux-->>UI: isSelectionMode = true
User->>UI: Click image checkbox(es)
UI->>Redux: dispatch(toggleImageSelection(imageId))
Redux-->>UI: Update selectedImageIds
User->>UI: Ctrl+A (or click Add Tags)
alt Ctrl+A
UI->>Redux: dispatch(selectAllImages([...]))
else Tag action
UI->>UI: Open BatchTagDialog
User->>UI: Enter tags, click Apply
UI->>UI: Collect tags & selectedImageIds
end
UI->>Backend: POST /images/batch-tag<br/>{image_ids, tags}
Backend->>DB: For each image_id:<br/>db_add_tags_to_image()
DB-->>Backend: Tag associations created
Backend-->>UI: BatchOperationResponse<br/>{success, processed, failed}
UI->>Redux: dispatch(deselectAllImages())
UI->>User: Show success toast
User->>UI: Press Escape
UI->>Redux: dispatch(deselectAllImages())
Redux-->>UI: Clear selection
UI-->>User: Exit selection mode
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (7)
backend/app/schemas/batch.py (1)
4-23: Consider adding field validation.The schemas lack validation rules to ensure data quality:
image_idscould be empty (all request models)tagscould be empty (BatchTagRequest)album_idcould be empty string (BatchMoveRequest)Consider adding Pydantic validators:
from pydantic import BaseModel -from typing import List +from typing import List +from pydantic import field_validator class BatchDeleteRequest(BaseModel): """Request model for batch delete operation""" image_ids: List[str] + + @field_validator('image_ids') + @classmethod + def validate_image_ids(cls, v): + if not v: + raise ValueError('image_ids cannot be empty') + return v class BatchTagRequest(BaseModel): """Request model for batch tag operation""" image_ids: List[str] tags: List[str] + + @field_validator('image_ids', 'tags') + @classmethod + def validate_lists(cls, v): + if not v: + raise ValueError('List cannot be empty') + return v class BatchMoveRequest(BaseModel): """Request model for batch move to album operation""" image_ids: List[str] album_id: str + + @field_validator('image_ids') + @classmethod + def validate_image_ids(cls, v): + if not v: + raise ValueError('image_ids cannot be empty') + return v + + @field_validator('album_id') + @classmethod + def validate_album_id(cls, v): + if not v or not v.strip(): + raise ValueError('album_id cannot be empty') + return v.strip()frontend/src/components/Media/ImageCard.tsx (1)
73-82: Use Checkbox's onCheckedChange prop instead of wrapper onClick.The current pattern wraps the Checkbox in a div with onClick handler. This is non-idiomatic for Radix UI components.
Apply this diff:
{isSelectionMode && ( - <div - className="absolute top-2 left-2 z-10" - onClick={handleCheckboxClick} - > - <Checkbox - checked={isImageSelected} - className="h-5 w-5 bg-white border-2 data-[state=checked]:bg-blue-600 data-[state=checked]:border-blue-600" - /> - </div> + <Checkbox + checked={isImageSelected} + onCheckedChange={() => dispatch(toggleImageSelection(image.id))} + className="absolute top-2 left-2 z-10 h-5 w-5 bg-white border-2 data-[state=checked]:bg-blue-600 data-[state=checked]:border-blue-600" + /> )}Then remove the
handleCheckboxClickfunction (lines 48-51) as it's no longer needed.frontend/src/components/Batch/BatchToolbar.tsx (2)
142-199: Consider adding a loading indicator during processing.While buttons are correctly disabled during processing, there's no visual feedback to users that an operation is in progress.
Consider adding a loading state to the buttons or a spinner:
<Button variant="ghost" size="sm" onClick={handleBatchDelete} disabled={isProcessing} className="hover:bg-red-50 hover:text-red-600 dark:hover:bg-red-900/20" > - <Trash2 className="h-4 w-4 mr-2" /> - Delete + {isProcessing ? ( + <Loader2 className="h-4 w-4 mr-2 animate-spin" /> + ) : ( + <Trash2 className="h-4 w-4 mr-2" /> + )} + {isProcessing ? 'Processing...' : 'Delete'} </Button>
34-72: Consider adding request timeout.The fetch calls lack timeout configuration, which could cause indefinite waiting if the backend is slow or unresponsive.
Add timeout to fetch requests:
+ const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); // 30 second timeout + const response = await fetch( `${backendUrl}/images/batch-delete`, { method: 'DELETE', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ image_ids: selectedImageIds }), + signal: controller.signal, } ); + + clearTimeout(timeoutId);Apply similar changes to the batch-tag fetch call.
backend/app/routes/images.py (1)
143-168: Consider using database transactions for atomicity.The current implementation processes deletions sequentially without a transaction wrapper. If a deletion fails midway through the batch, some images will be deleted while others remain, leaving the database in an inconsistent state.
Consider wrapping batch operations in a transaction:
from app.database.images import _connect @router.delete("/batch-delete", response_model=BatchOperationResponse) def batch_delete_images(request: BatchDeleteRequest): """Delete multiple images""" # ... validation code ... conn = _connect() cursor = conn.cursor() processed = 0 failed = 0 errors = [] try: for image_id in request.image_ids: try: cursor.execute("DELETE FROM images WHERE id = ?", (image_id,)) if cursor.rowcount > 0: processed += 1 else: failed += 1 errors.append(f"Image {image_id} not found") except Exception as e: failed += 1 errors.append(f"Failed to delete {image_id}: {str(e)}") logger.error(f"Error deleting image {image_id}: {e}") if failed == 0: conn.commit() else: conn.rollback() except Exception as e: conn.rollback() raise finally: conn.close() return BatchOperationResponse( success=failed == 0, processed=processed, failed=failed, errors=errors )Note: This approach provides all-or-nothing semantics. If you prefer partial success handling, consider documenting this behavior clearly for API consumers.
frontend/src/store/slices/selectionSlice.ts (1)
17-42: Consider using Set for better performance with large selections.The current implementation uses array methods like
indexOf,splice, andincludes, which have O(n) complexity. For large image collections (100+ selected images), this could become a performance bottleneck.Consider this optimization if you expect users to select many images:
interface SelectionState { - selectedImageIds: string[]; + selectedImageIds: Set<string>; isSelectionMode: boolean; } const initialState: SelectionState = { - selectedImageIds: [], + selectedImageIds: new Set<string>(), isSelectionMode: false, }; const selectionSlice = createSlice({ name: 'selection', initialState, reducers: { toggleImageSelection: (state, action: PayloadAction<string>) => { const imageId = action.payload; - const index = state.selectedImageIds.indexOf(imageId); - if (index > -1) { - state.selectedImageIds.splice(index, 1); + if (state.selectedImageIds.has(imageId)) { + state.selectedImageIds.delete(imageId); } else { - state.selectedImageIds.push(imageId); + state.selectedImageIds.add(imageId); } }, selectAllImages: (state, action: PayloadAction<string[]>) => { - state.selectedImageIds = action.payload; + state.selectedImageIds = new Set(action.payload); }, deselectAllImages: (state) => { - state.selectedImageIds = []; + state.selectedImageIds = new Set<string>(); }, setSelectionMode: (state, action: PayloadAction<boolean>) => { state.isSelectionMode = action.payload; if (!action.payload) { - state.selectedImageIds = []; + state.selectedImageIds = new Set<string>(); } }, removeFromSelection: (state, action: PayloadAction<string[]>) => { - state.selectedImageIds = state.selectedImageIds.filter( - (id) => !action.payload.includes(id) - ); + action.payload.forEach(id => state.selectedImageIds.delete(id)); }, }, });Note: You'll need to convert the Set to an array when consuming in components using
Array.from(selectedImageIds)or the spread operator[...selectedImageIds].frontend/src/components/Batch/BatchTagDialog.tsx (1)
61-110: Prevent dialog dismissal during processing.The dialog can be dismissed (via ESC key or clicking outside) while
isProcessingis true, which could interrupt the operation and leave the UI in an inconsistent state.-<Dialog open={isOpen} onOpenChange={onClose}> +<Dialog + open={isOpen} + onOpenChange={(open) => { + if (!open && !isProcessing) { + onClose(); + } + }} +> <DialogContent className="sm:max-w-md">Alternatively, you can use the
onInteractOutsideandonEscapeKeyDownprops from Radix UI:-<Dialog open={isOpen} onOpenChange={onClose}> - <DialogContent className="sm:max-w-md"> +<Dialog open={isOpen} onOpenChange={onClose}> + <DialogContent + className="sm:max-w-md" + onInteractOutside={(e) => { + if (isProcessing) e.preventDefault(); + }} + onEscapeKeyDown={(e) => { + if (isProcessing) e.preventDefault(); + }} + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (20)
README.md(1 hunks)backend/app/database/images.py(1 hunks)backend/app/routes/images.py(1 hunks)backend/app/schemas/batch.py(1 hunks)docs/backend/backend_python/openapi.json(2 hunks)frontend/jest.setup.ts(1 hunks)frontend/package.json(1 hunks)frontend/src/app/store.ts(2 hunks)frontend/src/components/Batch/BatchTagDialog.tsx(1 hunks)frontend/src/components/Batch/BatchToolbar.tsx(1 hunks)frontend/src/components/Media/ImageCard.tsx(2 hunks)frontend/src/components/Navigation/Navbar/Navbar.tsx(3 hunks)frontend/src/components/ui/checkbox.tsx(1 hunks)frontend/src/hooks/use-toast.ts(1 hunks)frontend/src/hooks/useBatchKeyboard.ts(1 hunks)frontend/src/pages/AITagging/AITagging.tsx(3 hunks)frontend/src/pages/Home/Home.tsx(3 hunks)frontend/src/pages/Home/MyFav.tsx(3 hunks)frontend/src/store/slices/selectionSlice.ts(1 hunks)frontend/src/utils/config.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
frontend/src/pages/Home/Home.tsx (2)
frontend/src/hooks/useBatchKeyboard.ts (1)
useBatchKeyboard(9-44)frontend/src/components/Batch/BatchToolbar.tsx (1)
BatchToolbar(15-211)
frontend/src/components/Batch/BatchToolbar.tsx (4)
frontend/src/hooks/use-toast.ts (1)
useToast(1-19)frontend/src/app/store.ts (1)
RootState(24-24)frontend/src/utils/config.ts (1)
getBackendUrl(1-3)frontend/src/components/Batch/BatchTagDialog.tsx (1)
BatchTagDialog(20-111)
frontend/src/components/Media/ImageCard.tsx (4)
frontend/src/hooks/useToggleFav.ts (1)
useToggleFav(5-18)frontend/src/app/store.ts (1)
RootState(24-24)frontend/src/lib/utils.ts (1)
cn(5-7)frontend/src/components/ui/checkbox.tsx (1)
Checkbox(28-28)
frontend/src/hooks/useBatchKeyboard.ts (1)
frontend/src/app/store.ts (1)
RootState(24-24)
frontend/src/pages/Home/MyFav.tsx (2)
frontend/src/hooks/useBatchKeyboard.ts (1)
useBatchKeyboard(9-44)frontend/src/components/Batch/BatchToolbar.tsx (1)
BatchToolbar(15-211)
frontend/src/components/Navigation/Navbar/Navbar.tsx (2)
frontend/src/app/store.ts (1)
RootState(24-24)frontend/src/components/ui/button.tsx (1)
Button(59-59)
backend/app/routes/images.py (2)
backend/app/schemas/batch.py (4)
BatchDeleteRequest(4-6)BatchTagRequest(8-11)BatchMoveRequest(13-16)BatchOperationResponse(18-23)backend/app/database/images.py (2)
db_delete_image(425-448)db_add_tags_to_image(451-515)
frontend/src/pages/AITagging/AITagging.tsx (2)
frontend/src/hooks/useBatchKeyboard.ts (1)
useBatchKeyboard(9-44)frontend/src/components/Batch/BatchToolbar.tsx (1)
BatchToolbar(15-211)
🪛 Checkov (3.2.334)
docs/backend/backend_python/openapi.json
[medium] 1566-1572: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🔇 Additional comments (22)
frontend/package.json (1)
28-28: LGTM! Appropriate dependency addition.The
@radix-ui/react-checkboxdependency aligns with the existing Radix UI component suite and supports the new checkbox functionality for image selection in batch operations.backend/app/database/images.py (1)
425-448: LGTM! Proper transaction handling.The
db_delete_imagefunction correctly handles single image deletion with CASCADE behavior for related records. Error handling, rollback, and connection cleanup are properly implemented.frontend/src/components/Navigation/Navbar/Navbar.tsx (3)
3-11: LGTM! Clean integration of selection mode imports.The new imports properly integrate the selection mode toggle functionality with Redux state management and UI components.
20-21: LGTM! Proper Redux state selection.Correctly reads the selection mode state from Redux using typed selector.
85-93: LGTM! Well-implemented selection toggle button.The button properly:
- Toggles selection mode via Redux action
- Provides visual feedback with conditional variant styling
- Updates label text contextually ("Select" vs "Cancel")
- Includes appropriate icon and sizing
frontend/src/pages/Home/Home.tsx (3)
16-17: LGTM! Appropriate imports for batch functionality.The imports correctly bring in the BatchToolbar component and keyboard shortcut hook needed for batch operations.
55-57: LGTM! Proper keyboard shortcut initialization.The batch keyboard shortcuts are correctly initialized with all available image IDs, enabling Ctrl+A and Escape functionality for the gallery.
87-89: LGTM! Clean BatchToolbar integration.The BatchToolbar component is properly positioned to provide batch operation controls when images are selected.
README.md (1)
35-43: LGTM! Comprehensive feature documentation.The updated features section accurately documents the new batch operations and keyboard shortcuts, providing clear descriptions that align with the implemented functionality.
frontend/src/components/ui/checkbox.tsx (1)
7-25: LGTM! Well-structured Checkbox component.The Checkbox component is properly implemented with ref forwarding, appropriate styling, and accessibility features. The integration with Radix UI primitives follows best practices.
frontend/src/pages/AITagging/AITagging.tsx (2)
46-48: LGTM! Batch keyboard integration is clean.The implementation correctly derives image IDs from the tagged images and enables batch keyboard shortcuts.
85-87: LGTM! BatchToolbar placement is appropriate.The toolbar is correctly positioned at the end of the component and will only render when images are selected.
frontend/src/app/store.ts (1)
9-9: LGTM! Selection state properly integrated.The selection reducer is correctly imported and added to the store, following the existing Redux Toolkit patterns.
Also applies to: 20-20
frontend/src/pages/Home/MyFav.tsx (2)
62-64: LGTM! Consistent batch integration.The batch keyboard shortcuts are properly enabled for favorite images, following the same pattern used in other pages.
118-119: LGTM! BatchToolbar placement is appropriate.The toolbar is correctly positioned and will only render when images are selected.
frontend/src/hooks/useBatchKeyboard.ts (1)
9-44: LGTM! Keyboard shortcuts well implemented.The hook properly manages keyboard shortcuts for batch selection with correct event handling and cleanup. The Ctrl+A toggle logic and Escape deselect functionality work as expected.
frontend/src/components/Media/ImageCard.tsx (1)
53-59: LGTM! Card click handler correctly switches behavior.The click handler appropriately toggles selection in selection mode and triggers the onClick prop in normal mode.
backend/app/routes/images.py (1)
134-141: LGTM!The imports are correctly structured and align with the batch operations implemented below.
frontend/src/store/slices/selectionSlice.ts (2)
1-11: LGTM!The state interface and initial state are well-defined and type-safe.
46-54: LGTM!The exports follow Redux Toolkit best practices.
frontend/src/components/Batch/BatchTagDialog.tsx (1)
1-18: LGTM!The imports and prop types are well-structured.
docs/backend/backend_python/openapi.json (1)
929-1054: LGTM!The batch endpoint definitions follow OpenAPI 3.1 standards and are consistent with the existing API structure. The operation IDs, request/response schemas, and HTTP methods are all appropriate.
|
@rahulharpal1603 can you please review this |
1 participant
Description
Adds ability to select multiple images and perform bulk actions (delete, tag, move to album).
Closes #662
Changes
Testing
Checklist
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.