feat: Bulk AI Tagging & Progress Tracking for Folder Management (#725)#728
feat: Bulk AI Tagging & Progress Tracking for Folder Management (#725)#728Jitmisra wants to merge 1 commit intoAOSSIE-Org:mainfrom
Conversation
…IE-Org#725) - Add progress summary showing overall AI tagging status - Add bulk action buttons (AI Tag All, Tag Selected, Disable All) - Add smart sorting with collapsible sections (Completed/In Progress/Pending) - Add multi-select functionality with checkboxes - Add Checkbox UI component - Add utility functions for folder grouping and stats - Add unit tests for folder utilities Closes AOSSIE-Org#725
|
|
WalkthroughIntroduces bulk folder management UI and state handling. Adds new React components for bulk actions and progress display, a selection hook, bulk mutation operations, folder utility functions for status grouping and statistics, and refactors FolderManagementCard to integrate the new bulk management interface with collapsible sections. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (3)
frontend/src/pages/SettingsPage/components/FolderManagementCard.tsx (2)
109-120: Clarify the re-enable behavior for already-tagged folders.When all selected folders already have AI tagging enabled, this code re-triggers
bulkEnableAITaggingon them. While this may be intentional (to re-process), consider if this is the desired UX. A user clicking "Tag Selected" when all selections are already tagged might expect a no-op or informative message instead.If re-triggering is not intended, consider showing feedback or skipping the operation:
const handleEnableSelected = useCallback(() => { const selectedFolders = getSelectedFolders(foldersWithStatus); const pendingSelectedIds = selectedFolders .filter((f) => !f.AI_Tagging) .map((f) => f.folder_id); if (pendingSelectedIds.length > 0) { bulkEnableAITagging(pendingSelectedIds); - } else { - // If all selected already have AI tagging, enable anyway (re-trigger) - bulkEnableAITagging(selectedFolders.map((f) => f.folder_id)); } }, [getSelectedFolders, foldersWithStatus, bulkEnableAITagging]);
63-65: Consider guarding against redundant saves on initial mount.The
useEffectwill trigger on mount, saving the preferences loaded from storage back to storage. While harmless, you could add auseRefto skip the initial effect if minimizing storage writes is desired.frontend/src/hooks/useFolderOperations.tsx (1)
200-219: Consider adding concurrent mutation protection.If a user rapidly triggers multiple bulk operations, concurrent mutations could occur. While the backend may handle this gracefully, you might want to guard against re-entry when a mutation is already pending.
const bulkEnableAITagging = useCallback( (folderIds: string[]) => { - if (folderIds.length > 0) { + if (folderIds.length > 0 && !bulkEnableAITaggingMutation.isPending) { bulkEnableAITaggingMutation.mutate(folderIds); } }, - [bulkEnableAITaggingMutation], + [bulkEnableAITaggingMutation.mutate, bulkEnableAITaggingMutation.isPending], ); const bulkDisableAITagging = useCallback( (folderIds: string[]) => { - if (folderIds.length > 0) { + if (folderIds.length > 0 && !bulkDisableAITaggingMutation.isPending) { bulkDisableAITaggingMutation.mutate(folderIds); } }, - [bulkDisableAITaggingMutation], + [bulkDisableAITaggingMutation.mutate, bulkDisableAITaggingMutation.isPending], );
📜 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 (11)
frontend/package.json(1 hunks)frontend/src/components/FolderManagement/FolderBulkActions.tsx(1 hunks)frontend/src/components/FolderManagement/FolderProgressSummary.tsx(1 hunks)frontend/src/components/FolderManagement/FolderSection.tsx(1 hunks)frontend/src/components/FolderManagement/index.ts(1 hunks)frontend/src/components/ui/checkbox.tsx(1 hunks)frontend/src/hooks/useBulkFolderSelection.ts(1 hunks)frontend/src/hooks/useFolderOperations.tsx(3 hunks)frontend/src/pages/SettingsPage/components/FolderManagementCard.tsx(1 hunks)frontend/src/utils/__tests__/folderUtils.test.ts(1 hunks)frontend/src/utils/folderUtils.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/src/components/FolderManagement/FolderBulkActions.tsx (2)
frontend/src/components/FolderManagement/index.ts (1)
FolderBulkActions(2-2)frontend/src/components/ui/checkbox.tsx (1)
Checkbox(27-27)
frontend/src/hooks/useBulkFolderSelection.ts (1)
frontend/src/utils/folderUtils.ts (1)
FolderWithStatus(4-6)
frontend/src/components/FolderManagement/FolderProgressSummary.tsx (3)
frontend/src/utils/folderUtils.ts (1)
FolderStats(14-20)frontend/src/components/FolderManagement/index.ts (1)
FolderProgressSummary(1-1)frontend/src/components/ui/progress.tsx (1)
Progress(37-37)
🔇 Additional comments (25)
frontend/package.json (1)
28-28: LGTM!The new
@radix-ui/react-checkboxdependency is correctly placed alphabetically among other Radix UI packages, and the version constraint (^1.3.3) follows the existing pattern used for other@radix-uidependencies.frontend/src/components/FolderManagement/index.ts (1)
1-3: LGTM!Clean barrel file that consolidates the FolderManagement component exports, enabling cleaner imports in consuming modules.
frontend/src/utils/__tests__/folderUtils.test.ts (1)
1-144: LGTM!Comprehensive test coverage for the folder utilities including edge cases (empty arrays, missing status data). The test structure is clear and well-organized.
frontend/src/components/ui/checkbox.tsx (1)
6-27: LGTM!Well-structured Checkbox component following the established Radix UI + Tailwind pattern. The
forwardRefimplementation, styling, anddisplayNameassignment are all correctly implemented.frontend/src/pages/SettingsPage/components/FolderManagementCard.tsx (1)
46-213: LGTM on the overall component structure.Well-organized component with appropriate separation of concerns:
- Memoized derived state (
foldersWithStatus,groupedFolders,stats)- Properly wrapped callbacks with
useCallback- Clean composition with the new FolderManagement subcomponents
- Persistence of user preferences
frontend/src/hooks/useFolderOperations.tsx (1)
163-219: LGTM on bulk mutation implementation.Well-implemented bulk operations with:
- Proper guards against empty array submissions
useCallbackwrapping for stable references- Appropriate feedback messages distinguishing bulk vs single operations
- Exposed pending states for UI disabling
frontend/src/components/FolderManagement/FolderProgressSummary.tsx (2)
1-18: LGTM!The imports, interface definition, and early return guard are appropriate.
39-60: LGTM!The metric rows display correctly with appropriate icons and styling.
frontend/src/hooks/useBulkFolderSelection.ts (5)
1-24: LGTM!The hook initialization and interface definition are well-structured.
25-40: LGTM!The selection state management correctly handles Set immutability.
42-48: LGTM!The bulk selection operations are implemented correctly.
50-64: LGTM!The status-based selection logic correctly matches the folder grouping logic in folderUtils.
66-94: LGTM!The computed values and memoization dependencies are correctly implemented.
frontend/src/components/FolderManagement/FolderBulkActions.tsx (2)
1-43: LGTM!The component setup and loading state management are appropriate.
69-124: LGTM!The action buttons are appropriately rendered with correct conditional logic and loading state handling.
frontend/src/components/FolderManagement/FolderSection.tsx (4)
1-52: LGTM!The imports, type definitions, and section configuration are well-structured.
54-111: LGTM!The section component correctly handles collapsible state and renders folders with appropriate props.
113-148: LGTM!The FolderItem component structure and conditional styling are appropriate.
150-209: LGTM!The folder item content correctly displays information and handles user interactions with appropriate event handlers and disabled states.
frontend/src/utils/folderUtils.ts (6)
1-20: LGTM!The type definitions are clear and appropriate for the folder management domain.
22-33: LGTM!The merge function correctly handles missing tagging status with appropriate fallback values.
35-59: LGTM!The folder grouping logic correctly categorizes folders by their tagging status.
61-85: LGTM!The statistics calculation correctly computes the average tagging percentage across enabled folders. The logic aligns with the progress display in FolderProgressSummary.
87-106: LGTM!The preferences structure and defaults are well-defined with appropriate namespacing.
120-126: LGTM!The save function correctly serializes preferences to localStorage with appropriate error handling.
| <div | ||
| className="flex cursor-pointer items-center gap-2" | ||
| onClick={isAllSelected ? onDeselectAll : onSelectAll} | ||
| > | ||
| <Checkbox | ||
| checked={isAllSelected} | ||
| onCheckedChange={isAllSelected ? onDeselectAll : onSelectAll} | ||
| className="cursor-pointer" | ||
| /> |
There was a problem hiding this comment.
Remove duplicate event handlers to prevent double-triggering.
The parent div has an onClick handler (line 50) and the child Checkbox has an onCheckedChange handler (line 54), both triggering the same action. Clicking the checkbox will fire both handlers, causing the toggle action to execute twice and cancel itself out.
Apply this diff to remove the redundant onClick handler:
- <div
- className="flex cursor-pointer items-center gap-2"
- onClick={isAllSelected ? onDeselectAll : onSelectAll}
- >
+ <div className="flex cursor-pointer items-center gap-2">
<Checkbox
checked={isAllSelected}
onCheckedChange={isAllSelected ? onDeselectAll : onSelectAll}
className="cursor-pointer"
/>Alternatively, if you want the entire div to be clickable, remove the onCheckedChange from the Checkbox and keep only the div's onClick:
<div
className="flex cursor-pointer items-center gap-2"
onClick={isAllSelected ? onDeselectAll : onSelectAll}
>
<Checkbox
checked={isAllSelected}
- onCheckedChange={isAllSelected ? onDeselectAll : onSelectAll}
className="cursor-pointer"
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div | |
| className="flex cursor-pointer items-center gap-2" | |
| onClick={isAllSelected ? onDeselectAll : onSelectAll} | |
| > | |
| <Checkbox | |
| checked={isAllSelected} | |
| onCheckedChange={isAllSelected ? onDeselectAll : onSelectAll} | |
| className="cursor-pointer" | |
| /> | |
| <div className="flex cursor-pointer items-center gap-2"> | |
| <Checkbox | |
| checked={isAllSelected} | |
| onCheckedChange={isAllSelected ? onDeselectAll : onSelectAll} | |
| className="cursor-pointer" | |
| /> |
🤖 Prompt for AI Agents
In frontend/src/components/FolderManagement/FolderBulkActions.tsx around lines
48 to 56, the parent div's onClick and the Checkbox's onCheckedChange both
trigger the select/deselect action causing double execution; remove the
redundant handler so the action only fires once — recommended: delete the div's
onClick and keep Checkbox.onCheckedChange (or alternatively keep div.onClick and
remove Checkbox.onCheckedChange) so only a single event handler remains for
toggling selection.
| export const loadFolderPreferences = (): FolderPreferences => { | ||
| try { | ||
| const stored = localStorage.getItem(FOLDER_PREFS_KEY); | ||
| if (stored) { | ||
| return { ...defaultFolderPreferences, ...JSON.parse(stored) }; | ||
| } | ||
| } catch (e) { | ||
| console.warn('Failed to load folder preferences:', e); | ||
| } | ||
| return defaultFolderPreferences; | ||
| }; |
There was a problem hiding this comment.
Fix shallow merge to handle nested preferences correctly.
Line 112 uses a shallow spread merge that could cause issues with the nested collapsedSections object. If the stored preferences have a partial collapsedSections object, the merge will replace the entire nested object rather than deep merging, potentially losing default values for missing keys.
Apply this diff to properly merge nested preferences:
export const loadFolderPreferences = (): FolderPreferences => {
try {
const stored = localStorage.getItem(FOLDER_PREFS_KEY);
if (stored) {
- return { ...defaultFolderPreferences, ...JSON.parse(stored) };
+ const parsed = JSON.parse(stored);
+ return {
+ ...defaultFolderPreferences,
+ ...parsed,
+ collapsedSections: {
+ ...defaultFolderPreferences.collapsedSections,
+ ...parsed.collapsedSections,
+ },
+ };
}
} catch (e) {
console.warn('Failed to load folder preferences:', e);
}
return defaultFolderPreferences;
};🤖 Prompt for AI Agents
In frontend/src/utils/folderUtils.ts around lines 108 to 118, the current
shallow spread merge can overwrite the nested collapsedSections object; instead
parse stored prefs into a variable, then return a merged object that deep-merges
collapsedSections by combining defaultFolderPreferences.collapsedSections with
parsed.collapsedSections (falling back when undefined) while spreading other
top-level fields from defaultFolderPreferences and parsed so missing nested keys
are preserved; keep the try/catch and return defaultFolderPreferences on error.
|
Team name bigbull |
|
|
2 participants
Problem
The folder management interface required manual interaction with each folder's AI tagging toggle, making it tedious to manage multiple folders. Users had no overview of tagging progress across their collection.
Solution
Visual progress bar showing overall AI tagging status
Stats breakdown: Completed | In Progress | Pending folders
2. Bulk Action Buttons
"AI Tag All" - Enable AI tagging for all pending folders at once
"Select All" - Checkbox to select multiple folders
"Tag Selected" - Apply AI tagging to selected folders only
"Disable All" / "Disable Selected" - Bulk disable options
3. Smart Sorting by Status
Folders auto-organized into collapsible sections:
In Progress (currently being tagged)
Pending (not yet enabled)
Completed (100% tagged)
Section collapse state persists across sessions
4. Multi-Select System
Individual folder checkboxes
Visual indication of selected folders
Bulk actions enabled when folders are selected
Technical Notes
No backend changes required (existing API already supports batch operations)
Added @radix-ui/react-checkbox dependency
#725
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.