diff options
| -rw-r--r-- | apps/web/components/dashboard/lists/DeleteListConfirmationDialog.tsx | 44 | ||||
| -rw-r--r-- | apps/web/lib/i18n/locales/en/translation.json | 6 | ||||
| -rw-r--r-- | packages/trpc/models/lists.ts | 35 | ||||
| -rw-r--r-- | packages/trpc/routers/lists.test.ts | 361 | ||||
| -rw-r--r-- | packages/trpc/routers/lists.ts | 7 |
5 files changed, 437 insertions, 16 deletions
diff --git a/apps/web/components/dashboard/lists/DeleteListConfirmationDialog.tsx b/apps/web/components/dashboard/lists/DeleteListConfirmationDialog.tsx index 7eb25e6d..4996ddf1 100644 --- a/apps/web/components/dashboard/lists/DeleteListConfirmationDialog.tsx +++ b/apps/web/components/dashboard/lists/DeleteListConfirmationDialog.tsx @@ -1,7 +1,11 @@ +import React from "react"; import { usePathname, useRouter } from "next/navigation"; import { ActionButton } from "@/components/ui/action-button"; import ActionConfirmingDialog from "@/components/ui/action-confirming-dialog"; +import { Label } from "@/components/ui/label"; +import { Switch } from "@/components/ui/switch"; import { toast } from "@/components/ui/use-toast"; +import { useTranslation } from "@/lib/i18n/client"; import type { ZBookmarkList } from "@karakeep/shared/types/lists"; import { useDeleteBookmarkList } from "@karakeep/shared-react/hooks/lists"; @@ -17,13 +21,15 @@ export default function DeleteListConfirmationDialog({ open: boolean; setOpen: (v: boolean) => void; }) { + const { t } = useTranslation(); const currentPath = usePathname(); const router = useRouter(); + const [deleteChildren, setDeleteChildren] = React.useState(false); const { mutate: deleteList, isPending } = useDeleteBookmarkList({ onSuccess: () => { toast({ - description: `List "${list.icon} ${list.name}" is deleted!`, + description: `List "${list.icon} ${list.name}" ${deleteChildren ? "and all its children are " : "is "} deleted!`, }); setOpen(false); if (currentPath.includes(list.id)) { @@ -42,16 +48,44 @@ export default function DeleteListConfirmationDialog({ <ActionConfirmingDialog open={open} setOpen={setOpen} - title={`Delete ${list.icon} ${list.name}?`} - description={`Are you sure you want to delete ${list.icon} ${list.name}?`} + title={t("lists.delete_list.title")} + description={ + <div className="space-y-3"> + <p className="text-balance"> + Are you sure you want to delete {list.icon} {list.name}? + </p> + <p className="text-balance text-sm text-muted-foreground"> + {t("lists.delete_list.description")} + </p> + + <div className="flex items-center justify-between rounded-lg border border-border/50 bg-muted/50 p-4"> + <div className="space-y-1"> + <Label + htmlFor="delete-children" + className="cursor-pointer text-sm font-medium" + > + {t("lists.delete_list.delete_children")} + </Label> + <p className="text-xs text-muted-foreground"> + {t("lists.delete_list.delete_children_description")} + </p> + </div> + <Switch + id="delete-children" + checked={deleteChildren} + onCheckedChange={setDeleteChildren} + /> + </div> + </div> + } actionButton={() => ( <ActionButton type="button" variant="destructive" loading={isPending} - onClick={() => deleteList({ listId: list.id })} + onClick={() => deleteList({ listId: list.id, deleteChildren })} > - Delete + {t("actions.delete")} </ActionButton> )} > diff --git a/apps/web/lib/i18n/locales/en/translation.json b/apps/web/lib/i18n/locales/en/translation.json index ce8d2839..ab1306be 100644 --- a/apps/web/lib/i18n/locales/en/translation.json +++ b/apps/web/lib/i18n/locales/en/translation.json @@ -428,6 +428,12 @@ "search_query": "Search Query", "search_query_help": "Learn more about the search query language.", "description": "Description (Optional)", + "delete_list": { + "title": "Delete List", + "description": "Deleting a list doesn't delete any bookmarks in that list.", + "delete_children": "Delete children lists (recursively)", + "delete_children_description": "If not checked, all direct children lists will become root lists" + }, "rss": { "title": "RSS Feed", "description": "Enable an RSS feed for this list", diff --git a/packages/trpc/models/lists.ts b/packages/trpc/models/lists.ts index c0e17bfc..48da5ed1 100644 --- a/packages/trpc/models/lists.ts +++ b/packages/trpc/models/lists.ts @@ -215,6 +215,41 @@ export abstract class List implements PrivacyAware { } } + async getChildren(): Promise<(ManualList | SmartList)[]> { + const lists = await List.getAll(this.ctx); + const listById = new Map(lists.map((l) => [l.list.id, l])); + + const adjecencyList = new Map<string, string[]>(); + + // Initialize all lists with empty arrays first + lists.forEach((l) => { + adjecencyList.set(l.list.id, []); + }); + + // Then populate the parent-child relationships + lists.forEach((l) => { + if (l.list.parentId) { + const currentChildren = adjecencyList.get(l.list.parentId) ?? []; + currentChildren.push(l.list.id); + adjecencyList.set(l.list.parentId, currentChildren); + } + }); + + const resultIds: string[] = []; + const queue: string[] = [this.list.id]; + + while (queue.length > 0) { + const id = queue.pop()!; + const children = adjecencyList.get(id) ?? []; + children.forEach((childId) => { + queue.push(childId); + resultIds.push(childId); + }); + } + + return resultIds.map((id) => listById.get(id)!); + } + async update( input: z.infer<typeof zEditBookmarkListSchemaWithValidation>, ): Promise<void> { diff --git a/packages/trpc/routers/lists.test.ts b/packages/trpc/routers/lists.test.ts index 9863fb38..8797b35e 100644 --- a/packages/trpc/routers/lists.test.ts +++ b/packages/trpc/routers/lists.test.ts @@ -10,19 +10,18 @@ import { zNewBookmarkListSchema } from "@karakeep/shared/types/lists"; import type { APICallerType, CustomTestContext } from "../testUtils"; import { defaultBeforeEach } from "../testUtils"; +async function createTestBookmark(api: APICallerType) { + const newBookmarkInput: z.infer<typeof zNewBookmarkRequestSchema> = { + type: BookmarkTypes.TEXT, + text: "Test bookmark text", + }; + const createdBookmark = await api.bookmarks.createBookmark(newBookmarkInput); + return createdBookmark.id; +} + beforeEach<CustomTestContext>(defaultBeforeEach(true)); describe("Lists Routes", () => { - async function createTestBookmark(api: APICallerType) { - const newBookmarkInput: z.infer<typeof zNewBookmarkRequestSchema> = { - type: BookmarkTypes.TEXT, - text: "Test bookmark text", - }; - const createdBookmark = - await api.bookmarks.createBookmark(newBookmarkInput); - return createdBookmark.id; - } - test<CustomTestContext>("create list", async ({ apiCallers }) => { const api = apiCallers[0].lists; const newListInput: z.infer<typeof zNewBookmarkListSchema> = { @@ -253,3 +252,345 @@ describe("Lists Routes", () => { expect(stats.stats.get(createdList.id)).toBeGreaterThan(0); }); }); + +describe("recursive delete", () => { + test<CustomTestContext>("non-recursive delete (deleteChildren=false)", async ({ + apiCallers, + }) => { + const api = apiCallers[0].lists; + + // Create parent list + const parentInput: z.infer<typeof zNewBookmarkListSchema> = { + name: "Parent List", + type: "manual", + icon: "📂", + }; + const parentList = await api.create(parentInput); + + // Create child list + const childInput: z.infer<typeof zNewBookmarkListSchema> = { + name: "Child List", + parentId: parentList.id, + type: "manual", + icon: "📄", + }; + const childList = await api.create(childInput); + + // Test both default behavior and explicit false + // Default (should be false) + await api.delete({ listId: parentList.id }); + + let lists = await api.list(); + expect(lists.lists.find((l) => l.id === parentList.id)).toBeUndefined(); + let remainingChild = lists.lists.find((l) => l.id === childList.id); + expect(remainingChild).toBeDefined(); + expect(remainingChild?.parentId).toBeNull(); + + // Create another parent-child pair to test explicit false + const parent2 = await api.create({ + name: "Parent List 2", + type: "manual", + icon: "📂", + }); + const child2 = await api.create({ + name: "Child List 2", + parentId: parent2.id, + type: "manual", + icon: "📄", + }); + + // Explicit deleteChildren=false + await api.delete({ listId: parent2.id, deleteChildren: false }); + + lists = await api.list(); + expect(lists.lists.find((l) => l.id === parent2.id)).toBeUndefined(); + remainingChild = lists.lists.find((l) => l.id === child2.id); + expect(remainingChild).toBeDefined(); + expect(remainingChild?.parentId).toBeNull(); + }); + + test<CustomTestContext>("recursive delete with multiple children", async ({ + apiCallers, + }) => { + const api = apiCallers[0].lists; + + // Create parent list + const parentInput: z.infer<typeof zNewBookmarkListSchema> = { + name: "Parent List", + type: "manual", + icon: "📂", + }; + const parentList = await api.create(parentInput); + + // Create multiple child lists + const child1Input: z.infer<typeof zNewBookmarkListSchema> = { + name: "Child List 1", + parentId: parentList.id, + type: "manual", + icon: "📄", + }; + const child1 = await api.create(child1Input); + + const child2Input: z.infer<typeof zNewBookmarkListSchema> = { + name: "Child List 2", + parentId: parentList.id, + type: "manual", + icon: "📄", + }; + const child2 = await api.create(child2Input); + + const child3Input: z.infer<typeof zNewBookmarkListSchema> = { + name: "Child List 3", + parentId: parentList.id, + type: "smart", + query: "is:fav", + icon: "⭐", + }; + const child3 = await api.create(child3Input); + + // Delete parent with deleteChildren=true + await api.delete({ listId: parentList.id, deleteChildren: true }); + + // Verify all lists are deleted + const lists = await api.list(); + expect(lists.lists.find((l) => l.id === parentList.id)).toBeUndefined(); + expect(lists.lists.find((l) => l.id === child1.id)).toBeUndefined(); + expect(lists.lists.find((l) => l.id === child2.id)).toBeUndefined(); + expect(lists.lists.find((l) => l.id === child3.id)).toBeUndefined(); + }); + + test<CustomTestContext>("recursive delete preserves bookmarks in deleted lists", async ({ + apiCallers, + }) => { + const api = apiCallers[0].lists; + + // Create a bookmark first + const bookmarkId = await createTestBookmark(apiCallers[0]); + + // Create parent list + const parentInput: z.infer<typeof zNewBookmarkListSchema> = { + name: "Parent List", + type: "manual", + icon: "📂", + }; + const parentList = await api.create(parentInput); + + // Create child list with bookmark + const childInput: z.infer<typeof zNewBookmarkListSchema> = { + name: "Child List", + parentId: parentList.id, + type: "manual", + icon: "📄", + }; + const childList = await api.create(childInput); + + // Add bookmark to child list + await api.addToList({ listId: childList.id, bookmarkId }); + + // Verify bookmark is in the list + const listsBeforeDelete = await api.getListsOfBookmark({ bookmarkId }); + expect( + listsBeforeDelete.lists.find((l) => l.id === childList.id), + ).toBeDefined(); + + // Delete parent with deleteChildren=true + await api.delete({ listId: parentList.id, deleteChildren: true }); + + // Verify lists are deleted + const allLists = await api.list(); + expect(allLists.lists.find((l) => l.id === parentList.id)).toBeUndefined(); + expect(allLists.lists.find((l) => l.id === childList.id)).toBeUndefined(); + + // Verify bookmark still exists but is not in any list + const listsAfterDelete = await api.getListsOfBookmark({ bookmarkId }); + expect(listsAfterDelete.lists).toHaveLength(0); + + // Verify the bookmark itself still exists by trying to access it + const bookmark = await apiCallers[0].bookmarks.getBookmark({ + bookmarkId, + }); + expect(bookmark).toBeDefined(); + expect(bookmark.id).toBe(bookmarkId); + }); + + test<CustomTestContext>("recursive delete with complex hierarchy", async ({ + apiCallers, + }) => { + const api = apiCallers[0].lists; + + // Create a complex tree structure: + // root + // / | \ + // A B C + // /| | |\ + // D E F G H + // | + // I + + const root = await api.create({ + name: "Root", + type: "manual", + icon: "🌳", + }); + + const listA = await api.create({ + name: "List A", + parentId: root.id, + type: "manual", + icon: "📂", + }); + + const listB = await api.create({ + name: "List B", + parentId: root.id, + type: "smart", + query: "is:fav", + icon: "📂", + }); + + const listC = await api.create({ + name: "List C", + parentId: root.id, + type: "manual", + icon: "📂", + }); + + const listD = await api.create({ + name: "List D", + parentId: listA.id, + type: "manual", + icon: "📄", + }); + + const listE = await api.create({ + name: "List E", + parentId: listA.id, + type: "smart", + query: "is:archived", + icon: "📄", + }); + + const listF = await api.create({ + name: "List F", + parentId: listB.id, + type: "manual", + icon: "📄", + }); + + const listG = await api.create({ + name: "List G", + parentId: listC.id, + type: "manual", + icon: "📄", + }); + + const listH = await api.create({ + name: "List H", + parentId: listC.id, + type: "smart", + query: "is:fav", + icon: "📄", + }); + + const listI = await api.create({ + name: "List I", + parentId: listF.id, + type: "manual", + icon: "📄", + }); + + const allCreatedIds = [ + root.id, + listA.id, + listB.id, + listC.id, + listD.id, + listE.id, + listF.id, + listG.id, + listH.id, + listI.id, + ]; + + // Delete root with deleteChildren=true + await api.delete({ listId: root.id, deleteChildren: true }); + + // Verify entire tree is deleted + const remainingLists = await api.list(); + allCreatedIds.forEach((id) => { + expect(remainingLists.lists.find((l) => l.id === id)).toBeUndefined(); + }); + }); + + test<CustomTestContext>("recursive delete edge cases", async ({ + apiCallers, + }) => { + const api = apiCallers[0].lists; + + // Test 1: Delete list with no children (should work fine) + const standaloneList = await api.create({ + name: "Standalone List", + type: "manual", + icon: "📄", + }); + + await api.delete({ listId: standaloneList.id, deleteChildren: true }); + let lists = await api.list(); + expect(lists.lists.find((l) => l.id === standaloneList.id)).toBeUndefined(); + + // Test 2: Delete child directly (no recursion needed) + const parent = await api.create({ + name: "Parent", + type: "manual", + icon: "📂", + }); + + const child = await api.create({ + name: "Child", + parentId: parent.id, + type: "manual", + icon: "📄", + }); + + await api.delete({ listId: child.id, deleteChildren: true }); + lists = await api.list(); + expect(lists.lists.find((l) => l.id === parent.id)).toBeDefined(); + expect(lists.lists.find((l) => l.id === child.id)).toBeUndefined(); + }); + + test<CustomTestContext>("partial recursive delete on middle node", async ({ + apiCallers, + }) => { + const api = apiCallers[0].lists; + + // Create hierarchy: grandparent -> parent -> child + const grandparent = await api.create({ + name: "Grandparent", + type: "manual", + icon: "📂", + }); + + const parent = await api.create({ + name: "Parent", + parentId: grandparent.id, + type: "manual", + icon: "📂", + }); + + const child = await api.create({ + name: "Child", + parentId: parent.id, + type: "manual", + icon: "📄", + }); + + // Delete middle node (parent) with deleteChildren=true + await api.delete({ listId: parent.id, deleteChildren: true }); + + // Verify parent and child are deleted, but grandparent remains + const lists = await api.list(); + expect(lists.lists.find((l) => l.id === grandparent.id)).toBeDefined(); + expect(lists.lists.find((l) => l.id === parent.id)).toBeUndefined(); + expect(lists.lists.find((l) => l.id === child.id)).toBeUndefined(); + }); +}); diff --git a/packages/trpc/routers/lists.ts b/packages/trpc/routers/lists.ts index 92392448..7118c608 100644 --- a/packages/trpc/routers/lists.ts +++ b/packages/trpc/routers/lists.ts @@ -57,10 +57,15 @@ export const listsAppRouter = router({ .input( z.object({ listId: z.string(), + deleteChildren: z.boolean().optional().default(false), }), ) .use(ensureListOwnership) - .mutation(async ({ ctx }) => { + .mutation(async ({ ctx, input }) => { + if (input.deleteChildren) { + const children = await ctx.list.getChildren(); + await Promise.all(children.map((l) => l.delete())); + } await ctx.list.delete(); }), addToList: authedProcedure |