Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 66 additions & 7 deletions src/components/workspace-canvas/FolderCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ interface FolderCardProps {
onOpenFolder: (folderId: string) => void;
onUpdateItem: (itemId: string, updates: Partial<Item>) => void;
onDeleteItem: (itemId: string) => void;
onDeleteFolderWithContents?: (folderId: string) => void; // Callback to delete folder and all items inside
onMoveItem?: (itemId: string, folderId: string | null) => void; // Callback to move folder to another location
}

Expand All @@ -70,10 +71,12 @@ function FolderCardComponent({
onOpenFolder,
onUpdateItem,
onDeleteItem,
onDeleteFolderWithContents,
onMoveItem,
}: FolderCardProps) {
const [showColorPicker, setShowColorPicker] = useState(false);
const [showDeleteConfirm, setShowDeleteConfirm] = useState(false);
const [deleteOption, setDeleteOption] = useState<'keep' | 'delete' | null>(null);
const [showMoveDialog, setShowMoveDialog] = useState(false);
const [isDropdownOpen, setIsDropdownOpen] = useState(false);
const [isDragHover, setIsDragHover] = useState(false);
Expand Down Expand Up @@ -210,9 +213,21 @@ function FolderCardComponent({
}, [item.id, onUpdateItem]);

const handleDelete = useCallback(() => {
onDeleteItem(item.id);
if (deleteOption === 'delete' && onDeleteFolderWithContents) {
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
onDeleteFolderWithContents(item.id);
} else {
onDeleteItem(item.id);
}
setShowDeleteConfirm(false);
}, [item.id, onDeleteItem]);
setDeleteOption(null);
}, [item.id, onDeleteItem, onDeleteFolderWithContents, deleteOption]);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Reset delete option when dialog closes
useEffect(() => {
if (!showDeleteConfirm) {
setDeleteOption(null);
}
}, [showDeleteConfirm]);

// Calculate colors using the same utilities as WorkspaceCard
const bodyBgColor = getCardColorCSS(folderColor, 0.4); // Body is more transparent
Expand Down Expand Up @@ -436,14 +451,57 @@ function FolderCardComponent({
>
<AlertDialogHeader>
<AlertDialogTitle>Delete Folder</AlertDialogTitle>
<AlertDialogDescription>
Are you sure you want to delete &quot;{item.name}&quot;? Items in this
folder will be moved out, but not deleted.
<AlertDialogDescription asChild>
<div className="space-y-4">
<div>
Choose what happens to the {itemCount} {itemCount === 1 ? 'item' : 'items'} in &quot;{item.name}&quot;:
</div>
<div className="space-y-3 pt-2">
<label className="flex items-start space-x-3 cursor-pointer group">
<input
type="radio"
name="deleteOption"
value="keep"
checked={deleteOption === 'keep'}
onChange={() => setDeleteOption('keep')}
className="mt-1 h-4 w-4 text-primary focus:ring-primary border-gray-300"
onClick={(e) => e.stopPropagation()}
/>
<div className="flex-1">
<div className="font-medium text-sm">Keep items</div>
<div className="text-xs text-muted-foreground">
Move items out of folder before deleting
</div>
</div>
</label>
<label className={`flex items-start space-x-3 group ${onDeleteFolderWithContents ? 'cursor-pointer' : 'cursor-not-allowed opacity-50'}`}>
<input
type="radio"
name="deleteOption"
value="delete"
checked={deleteOption === 'delete'}
onChange={() => setDeleteOption('delete')}
disabled={!onDeleteFolderWithContents}
className="mt-1 h-4 w-4 text-destructive focus:ring-destructive border-gray-300 disabled:opacity-50"
onClick={(e) => e.stopPropagation()}
/>
<div className="flex-1">
<div className="font-medium text-sm text-destructive">Delete items</div>
<div className="text-xs text-muted-foreground">
Permanently delete folder and all {itemCount} {itemCount === 1 ? 'item' : 'items'} inside
</div>
</div>
</label>
</div>
</div>
</AlertDialogDescription>
</AlertDialogHeader>
<AlertDialogFooter>
<AlertDialogCancel
onClick={(e) => e.stopPropagation()}
onClick={(e) => {
e.stopPropagation();
setDeleteOption(null);
}}
onMouseDown={(e) => e.stopPropagation()}
>
Cancel
Expand All @@ -454,7 +512,8 @@ function FolderCardComponent({
handleDelete();
}}
onMouseDown={(e) => e.stopPropagation()}
className="bg-destructive text-destructive-foreground hover:bg-destructive/90"
disabled={deleteOption === null}
className="bg-destructive text-destructive-foreground hover:bg-destructive/90 disabled:opacity-50 disabled:cursor-not-allowed"
>
Delete
</AlertDialogAction>
Expand Down
3 changes: 3 additions & 0 deletions src/components/workspace-canvas/WorkspaceContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ interface WorkspaceContentProps {
onMoveItem?: (itemId: string, folderId: string | null) => void; // Callback to move item to folder
onMoveItems?: (itemIds: string[], folderId: string | null) => void; // Callback to move multiple items to folder (bulk move)
onOpenFolder?: (folderId: string) => void; // Callback when folder is clicked
onDeleteFolderWithContents?: (folderId: string) => void; // Callback to delete folder and all items inside
onPDFUpload?: (files: File[]) => Promise<void>; // Function to handle PDF upload
}

Expand All @@ -55,6 +56,7 @@ export default function WorkspaceContent({
onMoveItem,
onMoveItems,
onOpenFolder,
onDeleteFolderWithContents,
onPDFUpload,
}: WorkspaceContentProps) {
// Use external ref if provided (from dashboard page), otherwise create local one
Expand Down Expand Up @@ -406,6 +408,7 @@ export default function WorkspaceContent({
onMoveItem={onMoveItem}
onMoveItems={onMoveItems}
onOpenFolder={handleOpenFolder}
onDeleteFolderWithContents={onDeleteFolderWithContents}
addItem={addItem}
onPDFUpload={onPDFUpload}
setOpenModalItemId={setOpenModalItemId}
Expand Down
4 changes: 4 additions & 0 deletions src/components/workspace-canvas/WorkspaceGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ interface WorkspaceGridProps {
onMoveItem?: (itemId: string, folderId: string | null) => void; // Callback to move item to folder
onMoveItems?: (itemIds: string[], folderId: string | null) => void; // Callback to move multiple items to folder (bulk move)
onOpenFolder?: (folderId: string) => void; // Callback when folder is clicked
onDeleteFolderWithContents?: (folderId: string) => void; // Callback to delete folder and all items inside
addItem?: (type: CardType, name?: string, initialData?: Partial<Item['data']>) => string | void; // Function to add new items
onPDFUpload?: (files: File[]) => Promise<void>; // Function to handle PDF upload
setOpenModalItemId?: (id: string | null) => void; // Function to open modal for newly created items
Expand Down Expand Up @@ -59,6 +60,7 @@ export function WorkspaceGrid({
onMoveItem,
onMoveItems,
onOpenFolder,
onDeleteFolderWithContents,
addItem,
onPDFUpload,
setOpenModalItemId,
Expand Down Expand Up @@ -531,6 +533,7 @@ export function WorkspaceGrid({
onOpenFolder={handleOpenFolder}
onUpdateItem={handleUpdateItem}
onDeleteItem={handleDeleteItem}
onDeleteFolderWithContents={onDeleteFolderWithContents}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing onDeleteFolderWithContents in useMemo dependency array.

The onDeleteFolderWithContents callback is passed to FolderCard (line 536) but is not included in the useMemo dependency array for children (lines 569-580). If the callback reference changes, the memoized children will use a stale closure.

🔧 Suggested fix
   // eslint-disable-next-line react-hooks/exhaustive-deps
 }, [
   itemsKey, // Stable key based on item IDs/names/types - only changes when items actually change
   handleUpdateItem,
   handleDeleteItem,
   handleOpenModal,
   existingColors,
   onMoveItem,
   handleOpenFolder,
   folderItemCounts,
   workspaceName,
   workspaceColor,
+  onDeleteFolderWithContents,
 ]);

Also applies to: 569-580

🤖 Prompt for AI Agents
In `@src/components/workspace-canvas/WorkspaceGrid.tsx` at line 536, The memoized
children in WorkspaceGrid (the useMemo that builds FolderCard instances)
currently omits onDeleteFolderWithContents from its dependency array causing
stale closures for FolderCard props; update the dependency array for that
useMemo (the variable computing children) to include onDeleteFolderWithContents
so the memo invalidates when the callback reference changes and FolderCard
receives the latest prop.

@cubic-dev-ai cubic-dev-ai Bot Jan 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The children useMemo now reads onDeleteFolderWithContents but the dependency array wasn’t updated. This can leave FolderCard with a stale delete handler if the prop changes. Add onDeleteFolderWithContents to the useMemo dependency list.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/workspace-canvas/WorkspaceGrid.tsx, line 536:

<comment>The children useMemo now reads onDeleteFolderWithContents but the dependency array wasn’t updated. This can leave FolderCard with a stale delete handler if the prop changes. Add onDeleteFolderWithContents to the useMemo dependency list.</comment>

<file context>
@@ -531,6 +533,7 @@ export function WorkspaceGrid({
             onOpenFolder={handleOpenFolder}
             onUpdateItem={handleUpdateItem}
             onDeleteItem={handleDeleteItem}
+            onDeleteFolderWithContents={onDeleteFolderWithContents}
             onMoveItem={onMoveItem}
           />
</file context>
Fix with Cubic

onMoveItem={onMoveItem}
/>
) : item.type === 'flashcard' ? (
Expand Down Expand Up @@ -570,6 +573,7 @@ export function WorkspaceGrid({
handleOpenModal,
existingColors,
onMoveItem,
onDeleteFolderWithContents,
handleOpenFolder,
folderItemCounts,
workspaceName,
Expand Down
1 change: 1 addition & 0 deletions src/components/workspace-canvas/WorkspaceSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,7 @@ export function WorkspaceSection({
workspaceColor={workspaceColor}
onMoveItem={operations?.moveItemToFolder}
onMoveItems={operations?.moveItemsToFolder}
onDeleteFolderWithContents={operations?.deleteFolderWithContents}
onPDFUpload={handlePDFUpload}
/>
)}
Expand Down
83 changes: 83 additions & 0 deletions src/hooks/workspace/use-workspace-operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export interface WorkspaceOperations {
createFolderWithItems: (name: string, itemIds: string[], color?: CardColor) => string;
updateFolder: (folderId: string, changes: Partial<Item>) => void;
deleteFolder: (folderId: string) => void;
deleteFolderWithContents: (folderId: string) => void;
moveItemToFolder: (itemId: string, folderId: string | null) => void;
moveItemsToFolder: (itemIds: string[], folderId: string | null) => void;
isPending: boolean;
Expand Down Expand Up @@ -484,6 +485,87 @@ export function useWorkspaceOperations(
[deleteItem, currentState.items]
);

// Helper to recursively find all descendant IDs (items in folder and nested subfolders)
const getAllDescendantIds = useCallback(
(folderId: string, items: Item[]): string[] => {
const directChildren = items.filter(item => item.folderId === folderId);
const descendantIds: string[] = [];

for (const child of directChildren) {
descendantIds.push(child.id);
// Recursively get descendants of nested folders
if (child.type === 'folder') {
descendantIds.push(...getAllDescendantIds(child.id, items));
}
}

return descendantIds;
},
[]
);

// deleteFolderWithContents deletes the folder and all items inside it (including nested)
// Uses atomic bulk update pattern (same as handleBulkDelete in WorkspaceSection)
const deleteFolderWithContents = useCallback(
(folderId: string) => {
// CRITICAL: Read latest state from cache to avoid stale data issues
// (same pattern as updateAllItems - currentState prop can be stale)
let latestItems: Item[];
if (workspaceId) {
const cacheData = queryClient.getQueryData<EventResponse>([
"workspace",
workspaceId,
"events",
]);
if (cacheData?.events) {
const latestState = replayEvents(cacheData.events, workspaceId, cacheData.snapshot?.state);
latestItems = latestState.items;
} else {
latestItems = currentState.items;
}
} else {
latestItems = currentState.items;
}

const folder = latestItems.find(i => i.id === folderId && i.type === 'folder');
logger.debug("📁 [FOLDER-DELETE-WITH-CONTENTS] Deleting folder and contents:", { folderId, folderName: folder?.name });

// Find all descendant items recursively (handles nested folders)
const allDescendantIds = getAllDescendantIds(folderId, latestItems);

// Create set of all IDs to delete (descendants + folder itself)
const idsToDelete = new Set([...allDescendantIds, folderId]);
const itemCount = allDescendantIds.length;

logger.debug("📁 [FOLDER-DELETE-WITH-CONTENTS] Found items to delete:", { itemCount, itemIds: [...idsToDelete] });

// Delete PDF files from storage (fire-and-forget, non-blocking)
// This is best-effort cleanup - files may become orphaned if this fails
const itemsToDelete = latestItems.filter(item => idsToDelete.has(item.id));
for (const item of itemsToDelete) {
if (item.type === 'pdf') {
const pdfData = item.data as { fileUrl?: string };
if (pdfData?.fileUrl) {
fetch(`/api/delete-file?url=${encodeURIComponent(pdfData.fileUrl)}`, {
method: 'DELETE',
}).catch(err => logger.warn("📁 [FOLDER-DELETE-WITH-CONTENTS] Failed to delete PDF file:", err));
}
}
}

// Atomic bulk delete using updateAllItems pattern (single BULK_ITEMS_UPDATED event)
const remainingItems = latestItems.filter(item => !idsToDelete.has(item.id));
updateAllItems(remainingItems);

toast.success(
folder
? `Folder "${folder.name}" and ${itemCount} ${itemCount === 1 ? 'item' : 'items'} deleted`
: `Folder and ${itemCount} ${itemCount === 1 ? 'item' : 'items'} deleted`
);
},
[workspaceId, queryClient, currentState.items, getAllDescendantIds, updateAllItems]
);

const moveItemToFolder = useCallback(
(itemId: string, folderId: string | null) => {
logger.debug("📁 [ITEM-MOVE] Moving item to folder:", { itemId, folderId });
Expand Down Expand Up @@ -566,6 +648,7 @@ export function useWorkspaceOperations(
createFolderWithItems,
updateFolder,
deleteFolder,
deleteFolderWithContents,
moveItemToFolder,
moveItemsToFolder,
isPending: mutation.isPending,
Expand Down
29 changes: 25 additions & 4 deletions src/lib/workspace/event-reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,34 @@ export function eventReducer(state: AgentState, event: WorkspaceEvent): AgentSta
workspaceId: state.workspaceId, // Preserve workspace ID
};

case 'BULK_ITEMS_UPDATED':
// Used for layout changes (drag/resize) and reordering
case 'BULK_ITEMS_UPDATED': {
// Used for layout changes (drag/resize), reordering, and bulk delete
// Support both new format (layoutUpdates only) and legacy format (full items array)
if (event.payload.items) {
// Legacy format: full items array (for backwards compatibility)
// Full items array format (used for bulk delete and item add/remove operations)
const newItems = event.payload.items;

// Find folders that were deleted (existed in old state but not in new state)
const newItemIds = new Set(newItems.map(item => item.id));
const deletedFolderIds = new Set(
state.items
.filter(item => item.type === 'folder' && !newItemIds.has(item.id))
.map(item => item.id)
);

// If any folders were deleted, clear folderId on orphaned items
// This prevents items from pointing to non-existent folders
const cleanedItems = deletedFolderIds.size > 0
? newItems.map(item =>
item.folderId && deletedFolderIds.has(item.folderId)
? { ...item, folderId: undefined }
: item
)
: newItems;

return {
...state,
items: event.payload.items,
items: cleanedItems,
};
} else {
// New format: only layout changes - apply to existing items
Expand All @@ -107,6 +127,7 @@ export function eventReducer(state: AgentState, event: WorkspaceEvent): AgentSta
}),
};
}
}

case 'BULK_ITEMS_CREATED':
// Create multiple items atomically in a single event
Expand Down