From 3c838ddb26c1e86d3f201ce71f13c834be705f69 Mon Sep 17 00:00:00 2001 From: Mohamed Bassem Date: Wed, 4 Feb 2026 09:44:18 +0000 Subject: feat: Import workflow v3 (#2378) * feat: import workflow v3 * batch stage * revert migration * cleanups * pr comments * move to models * add allowed workers * e2e tests * import list ids * add missing indicies * merge test * more fixes * add resume/pause to UI * fix ui states * fix tests * simplify progress tracking * remove backpressure * fix list imports * fix race on claiming bookmarks * remove the codex file --- packages/shared/import-export/importer.test.ts | 414 ++++++++++++------------- 1 file changed, 198 insertions(+), 216 deletions(-) (limited to 'packages/shared/import-export/importer.test.ts') diff --git a/packages/shared/import-export/importer.test.ts b/packages/shared/import-export/importer.test.ts index 7e381f20..f097f8d5 100644 --- a/packages/shared/import-export/importer.test.ts +++ b/packages/shared/import-export/importer.test.ts @@ -1,13 +1,14 @@ import { describe, expect, it, vi } from "vitest"; -import { importBookmarksFromFile, ParsedBookmark } from "."; +import type { StagedBookmark } from "."; +import { importBookmarksFromFile } from "."; const fakeFile = { text: vi.fn().mockResolvedValue("fake file content"), } as unknown as File; describe("importBookmarksFromFile", () => { - it("creates root list, folders and imports bookmarks with progress", async () => { + it("creates root list, folders and stages bookmarks with progress", async () => { const parsers = { pocket: vi.fn().mockReturnValue([ { @@ -61,32 +62,23 @@ describe("importBookmarksFromFile", () => { }, ); - const createdBookmarks: ParsedBookmark[] = []; - const addedToLists: { bookmarkId: string; listIds: string[] }[] = []; - const updatedTags: { bookmarkId: string; tags: string[] }[] = []; - - const createBookmark = vi.fn(async (bookmark: ParsedBookmark) => { - createdBookmarks.push(bookmark); - return { - id: `bookmark-${createdBookmarks.length}`, - alreadyExists: false, - }; - }); - - const addBookmarkToLists = vi.fn( - async (input: { bookmarkId: string; listIds: string[] }) => { - addedToLists.push(input); + const stagedBookmarks: StagedBookmark[] = []; + const stageImportedBookmarks = vi.fn( + async (input: { + importSessionId: string; + bookmarks: StagedBookmark[]; + }) => { + stagedBookmarks.push(...input.bookmarks); }, ); - const updateBookmarkTags = vi.fn( - async (input: { bookmarkId: string; tags: string[] }) => { - updatedTags.push(input); - }, + const finalizeImportStaging = vi.fn(); + const createImportSession = vi.fn( + async (_input: { name: string; rootListId: string }) => ({ + id: "session-1", + }), ); - const createImportSession = vi.fn(async () => ({ id: "session-1" })); - const progress: number[] = []; const res = await importBookmarksFromFile( { @@ -95,9 +87,8 @@ describe("importBookmarksFromFile", () => { rootListName: "Imported", deps: { createList, - createBookmark, - addBookmarkToLists, - updateBookmarkTags, + stageImportedBookmarks, + finalizeImportStaging, createImportSession, }, onProgress: (d, t) => progress.push(d / t), @@ -106,12 +97,14 @@ describe("importBookmarksFromFile", () => { ); expect(res.rootListId).toBe("Imported"); + expect(res.importSessionId).toBe("session-1"); expect(res.counts).toEqual({ - successes: 5, + successes: 0, failures: 0, alreadyExisted: 0, total: 5, // Using custom parser, no deduplication }); + // Root + all unique folders from paths expect(createdLists).toEqual([ { name: "Imported", icon: "ā¬†ļø" }, @@ -122,38 +115,43 @@ describe("importBookmarksFromFile", () => { { name: "Tech", parentId: "Imported/Reading", icon: "šŸ“" }, { name: "Duplicates", parentId: "Imported/Development", icon: "šŸ“" }, ]); - // Verify we have 5 created bookmarks (no deduplication with custom parser) - expect(createdBookmarks).toHaveLength(5); - // Verify GitHub bookmark exists (will be two separate bookmarks since no deduplication) - const githubBookmarks = createdBookmarks.filter( - (bookmark) => - bookmark.content?.type === "link" && - bookmark.content.url === "https://github.com/example/repo", - ); - expect(githubBookmarks).toHaveLength(2); - // Verify text bookmark exists - const textBookmark = createdBookmarks.find( - (bookmark) => bookmark.content?.type === "text", + + // Verify 5 bookmarks were staged (in 1 batch since < 50) + expect(stagedBookmarks).toHaveLength(5); + expect(stageImportedBookmarks).toHaveBeenCalledTimes(1); + + // Verify GitHub link bookmark was staged correctly + const githubBookmark = stagedBookmarks.find( + (b) => b.url === "https://github.com/example/repo" && b.type === "link", ); + expect(githubBookmark).toBeDefined(); + if (!githubBookmark) { + throw new Error("Expected GitHub bookmark to be staged"); + } + expect(githubBookmark.title).toBe("GitHub Repository"); + expect(githubBookmark.tags).toEqual(["dev", "github"]); + expect(githubBookmark.listIds).toEqual(["Imported/Development/Projects"]); + + // Verify text bookmark was staged correctly + const textBookmark = stagedBookmarks.find((b) => b.type === "text"); expect(textBookmark).toBeDefined(); - expect(textBookmark!.archived).toBe(true); - expect(textBookmark!.notes).toBe("Additional context"); - // Verify bookmark with no path goes to root - const noCategoryBookmark = createdBookmarks.find( - (bookmark) => - bookmark.content?.type === "link" && - bookmark.content.url === "https://example.com/misc", + if (!textBookmark) { + throw new Error("Expected text bookmark to be staged"); + } + expect(textBookmark.content).toBe("Important notes about the project"); + expect(textBookmark.note).toBe("Additional context"); + expect(textBookmark.listIds).toEqual(["Imported/Personal"]); + + // Verify bookmark with empty paths gets root list ID + const noCategoryBookmark = stagedBookmarks.find( + (b) => b.url === "https://example.com/misc", ); expect(noCategoryBookmark).toBeDefined(); - // Find the corresponding list assignment for this bookmark - const noCategoryBookmarkId = `bookmark-${createdBookmarks.indexOf(noCategoryBookmark!) + 1}`; - const listAssignment = addedToLists.find( - (a) => a.bookmarkId === noCategoryBookmarkId, - ); - expect(listAssignment!.listIds).toEqual(["Imported"]); + expect(noCategoryBookmark!.listIds).toEqual(["Imported"]); + + // Verify finalizeImportStaging was called + expect(finalizeImportStaging).toHaveBeenCalledWith("session-1"); - // Verify that tags were updated for bookmarks that have tags - expect(updatedTags.length).toBeGreaterThan(0); expect(progress).toContain(0); expect(progress.at(-1)).toBe(1); }); @@ -167,9 +165,8 @@ describe("importBookmarksFromFile", () => { rootListName: "Imported", deps: { createList: vi.fn(), - createBookmark: vi.fn(), - addBookmarkToLists: vi.fn(), - updateBookmarkTags: vi.fn(), + stageImportedBookmarks: vi.fn(), + finalizeImportStaging: vi.fn(), createImportSession: vi.fn(async () => ({ id: "session-1" })), }, }, @@ -182,29 +179,29 @@ describe("importBookmarksFromFile", () => { }); }); - it("continues import when individual bookmarks fail", async () => { + it("stages all bookmarks successfully", async () => { const parsers = { pocket: vi.fn().mockReturnValue([ { - title: "Success Bookmark 1", - content: { type: "link", url: "https://example.com/success1" }, - tags: ["success"], + title: "Bookmark 1", + content: { type: "link", url: "https://example.com/1" }, + tags: ["tag1"], addDate: 100, - paths: [["Success"]], + paths: [["Category1"]], }, { - title: "Failure Bookmark", - content: { type: "link", url: "https://example.com/failure" }, - tags: ["failure"], + title: "Bookmark 2", + content: { type: "link", url: "https://example.com/2" }, + tags: ["tag2"], addDate: 200, - paths: [["Failure"]], + paths: [["Category2"]], }, { - title: "Success Bookmark 2", - content: { type: "link", url: "https://example.com/success2" }, - tags: ["success"], + title: "Bookmark 3", + content: { type: "link", url: "https://example.com/3" }, + tags: ["tag3"], addDate: 300, - paths: [["Success"]], + paths: [["Category1"]], }, ]), }; @@ -220,37 +217,23 @@ describe("importBookmarksFromFile", () => { }, ); - const createdBookmarks: ParsedBookmark[] = []; - const addedToLists: { bookmarkId: string; listIds: string[] }[] = []; - const updatedTags: { bookmarkId: string; tags: string[] }[] = []; - - const createBookmark = vi.fn(async (bookmark: ParsedBookmark) => { - // Simulate failure for the "Failure Bookmark" - if (bookmark.title === "Failure Bookmark") { - throw new Error("Simulated bookmark creation failure"); - } - - createdBookmarks.push(bookmark); - return { - id: `bookmark-${createdBookmarks.length}`, - alreadyExists: false, - }; - }); - - const addBookmarkToLists = vi.fn( - async (input: { bookmarkId: string; listIds: string[] }) => { - addedToLists.push(input); + const stagedBookmarks: StagedBookmark[] = []; + const stageImportedBookmarks = vi.fn( + async (input: { + importSessionId: string; + bookmarks: StagedBookmark[]; + }) => { + stagedBookmarks.push(...input.bookmarks); }, ); - const updateBookmarkTags = vi.fn( - async (input: { bookmarkId: string; tags: string[] }) => { - updatedTags.push(input); - }, + const finalizeImportStaging = vi.fn(); + const createImportSession = vi.fn( + async (_input: { name: string; rootListId: string }) => ({ + id: "session-1", + }), ); - const createImportSession = vi.fn(async () => ({ id: "session-1" })); - const progress: number[] = []; const res = await importBookmarksFromFile( { @@ -259,9 +242,8 @@ describe("importBookmarksFromFile", () => { rootListName: "Imported", deps: { createList, - createBookmark, - addBookmarkToLists, - updateBookmarkTags, + stageImportedBookmarks, + finalizeImportStaging, createImportSession, }, onProgress: (d, t) => progress.push(d / t), @@ -269,63 +251,57 @@ describe("importBookmarksFromFile", () => { { parsers }, ); - // Should still create the root list expect(res.rootListId).toBe("Imported"); - - // Should track both successes and failures + expect(res.importSessionId).toBe("session-1"); expect(res.counts).toEqual({ - successes: 2, // Two successful bookmarks - failures: 1, // One failed bookmark + successes: 0, + failures: 0, alreadyExisted: 0, total: 3, }); - // Should create folders for all bookmarks (including failed ones) + // Should create folders for all bookmarks expect(createdLists).toEqual([ { name: "Imported", icon: "ā¬†ļø" }, - { name: "Success", parentId: "Imported", icon: "šŸ“" }, - { name: "Failure", parentId: "Imported", icon: "šŸ“" }, + { name: "Category1", parentId: "Imported", icon: "šŸ“" }, + { name: "Category2", parentId: "Imported", icon: "šŸ“" }, ]); - // Only successful bookmarks should be created - expect(createdBookmarks).toHaveLength(2); - expect(createdBookmarks.map((b) => b.title)).toEqual([ - "Success Bookmark 1", - "Success Bookmark 2", - ]); + // All bookmarks should be staged (in 1 batch since < 50) + expect(stagedBookmarks).toHaveLength(3); + expect(stageImportedBookmarks).toHaveBeenCalledTimes(1); - // Only successful bookmarks should be added to lists and have tags updated - expect(addedToLists).toHaveLength(2); - expect(updatedTags).toHaveLength(2); + // Verify finalizeImportStaging was called + expect(finalizeImportStaging).toHaveBeenCalledWith("session-1"); - // Progress should complete even with failures + // Progress should complete expect(progress).toContain(0); expect(progress.at(-1)).toBe(1); }); - it("handles failures in different stages of bookmark import", async () => { + it("stages bookmarks with different paths", async () => { const parsers = { pocket: vi.fn().mockReturnValue([ { - title: "Success Bookmark", - content: { type: "link", url: "https://example.com/success" }, - tags: ["success"], + title: "Bookmark 1", + content: { type: "link", url: "https://example.com/1" }, + tags: ["tag1"], addDate: 100, - paths: [["Success"]], + paths: [["Path1"]], }, { - title: "Fail at List Assignment", - content: { type: "link", url: "https://example.com/fail-list" }, - tags: ["fail"], + title: "Bookmark 2", + content: { type: "link", url: "https://example.com/2" }, + tags: ["tag2"], addDate: 200, - paths: [["Failure"]], + paths: [["Path2"]], }, { - title: "Fail at Tag Update", - content: { type: "link", url: "https://example.com/fail-tag" }, - tags: ["fail-tag"], + title: "Bookmark 3", + content: { type: "link", url: "https://example.com/3" }, + tags: ["tag3"], addDate: 300, - paths: [["Failure"]], + paths: [["Path2"]], }, ]), }; @@ -338,31 +314,23 @@ describe("importBookmarksFromFile", () => { }, ); - let bookmarkIdCounter = 1; - const createBookmark = vi.fn(async () => { - return { id: `bookmark-${bookmarkIdCounter++}`, alreadyExists: false }; - }); - - const addBookmarkToLists = vi.fn( - async (input: { bookmarkId: string; listIds: string[] }) => { - // Simulate failure for specific bookmark - if (input.bookmarkId === "bookmark-2") { - throw new Error("Failed to add bookmark to lists"); - } + const stagedBookmarks: StagedBookmark[] = []; + const stageImportedBookmarks = vi.fn( + async (input: { + importSessionId: string; + bookmarks: StagedBookmark[]; + }) => { + stagedBookmarks.push(...input.bookmarks); }, ); - const updateBookmarkTags = vi.fn( - async (input: { bookmarkId: string; tags: string[] }) => { - // Simulate failure for specific bookmark - if (input.bookmarkId === "bookmark-3") { - throw new Error("Failed to update bookmark tags"); - } - }, + const finalizeImportStaging = vi.fn(); + const createImportSession = vi.fn( + async (_input: { name: string; rootListId: string }) => ({ + id: "session-1", + }), ); - const createImportSession = vi.fn(async () => ({ id: "session-1" })); - const progress: number[] = []; const res = await importBookmarksFromFile( { @@ -371,9 +339,8 @@ describe("importBookmarksFromFile", () => { rootListName: "Imported", deps: { createList, - createBookmark, - addBookmarkToLists, - updateBookmarkTags, + stageImportedBookmarks, + finalizeImportStaging, createImportSession, }, onProgress: (d, t) => progress.push(d / t), @@ -383,23 +350,19 @@ describe("importBookmarksFromFile", () => { expect(res.rootListId).toBe("Imported"); expect(res.importSessionId).toBe("session-1"); - - // All bookmarks are created successfully, but 2 fail in post-processing expect(res.counts).toEqual({ - successes: 1, // Only one fully successful bookmark - failures: 2, // Two failed in post-processing steps + successes: 0, + failures: 0, alreadyExisted: 0, total: 3, }); - // All bookmarks should be created (failures happen after bookmark creation) - expect(createBookmark).toHaveBeenCalledTimes(3); - - // addBookmarkToLists should be called 3 times (but one fails) - expect(addBookmarkToLists).toHaveBeenCalledTimes(3); + // All bookmarks should be staged (in 1 batch since < 50) + expect(stagedBookmarks).toHaveLength(3); + expect(stageImportedBookmarks).toHaveBeenCalledTimes(1); - // updateBookmarkTags should be called 2 times (once fails at list assignment, one fails at tag update) - expect(updateBookmarkTags).toHaveBeenCalledTimes(2); + // Verify finalizeImportStaging was called + expect(finalizeImportStaging).toHaveBeenCalledWith("session-1"); }); it("handles HTML bookmarks with empty folder names", async () => { @@ -432,14 +395,22 @@ describe("importBookmarksFromFile", () => { }, ); - const createdBookmarks: ParsedBookmark[] = []; - const createBookmark = vi.fn(async (bookmark: ParsedBookmark) => { - createdBookmarks.push(bookmark); - return { - id: `bookmark-${createdBookmarks.length}`, - alreadyExists: false, - }; - }); + const stagedBookmarks: StagedBookmark[] = []; + const stageImportedBookmarks = vi.fn( + async (input: { + importSessionId: string; + bookmarks: StagedBookmark[]; + }) => { + stagedBookmarks.push(...input.bookmarks); + }, + ); + + const finalizeImportStaging = vi.fn(); + const createImportSession = vi.fn( + async (_input: { name: string; rootListId: string }) => ({ + id: "session-1", + }), + ); const res = await importBookmarksFromFile({ file: mockFile, @@ -447,15 +418,14 @@ describe("importBookmarksFromFile", () => { rootListName: "HTML Import", deps: { createList, - createBookmark, - addBookmarkToLists: vi.fn(), - updateBookmarkTags: vi.fn(), - createImportSession: vi.fn(async () => ({ id: "session-1" })), + stageImportedBookmarks, + finalizeImportStaging, + createImportSession, }, }); expect(res.counts).toEqual({ - successes: 1, + successes: 0, failures: 0, alreadyExisted: 0, total: 1, @@ -472,16 +442,18 @@ describe("importBookmarksFromFile", () => { }, ]); - // Verify the bookmark was created and assigned to the correct path - expect(createdBookmarks).toHaveLength(1); - expect(createdBookmarks[0]).toMatchObject({ + // Verify the bookmark was staged with correct listIds + expect(stagedBookmarks).toHaveLength(1); + expect(stagedBookmarks[0]).toMatchObject({ title: "Example Product", - content: { - type: "link", - url: "https://www.example.com/product.html", - }, + url: "https://www.example.com/product.html", + type: "link", tags: [], + listIds: ["HTML Import/Bluetooth Fernbedienung/Unnamed"], }); + + // Verify finalizeImportStaging was called + expect(finalizeImportStaging).toHaveBeenCalledWith("session-1"); }); it("parses mymind CSV export correctly", async () => { @@ -495,14 +467,22 @@ describe("importBookmarksFromFile", () => { text: vi.fn().mockResolvedValue(mymindCsv), } as unknown as File; - const createdBookmarks: ParsedBookmark[] = []; - const createBookmark = vi.fn(async (bookmark: ParsedBookmark) => { - createdBookmarks.push(bookmark); - return { - id: `bookmark-${createdBookmarks.length}`, - alreadyExists: false, - }; - }); + const stagedBookmarks: StagedBookmark[] = []; + const stageImportedBookmarks = vi.fn( + async (input: { + importSessionId: string; + bookmarks: StagedBookmark[]; + }) => { + stagedBookmarks.push(...input.bookmarks); + }, + ); + + const finalizeImportStaging = vi.fn(); + const createImportSession = vi.fn( + async (_input: { name: string; rootListId: string }) => ({ + id: "session-1", + }), + ); const res = await importBookmarksFromFile({ file: mockFile, @@ -514,52 +494,54 @@ describe("importBookmarksFromFile", () => { id: `${input.parentId ? input.parentId + "/" : ""}${input.name}`, }), ), - createBookmark, - addBookmarkToLists: vi.fn(), - updateBookmarkTags: vi.fn(), - createImportSession: vi.fn(async () => ({ id: "session-1" })), + stageImportedBookmarks, + finalizeImportStaging, + createImportSession, }, }); expect(res.counts).toEqual({ - successes: 3, + successes: 0, failures: 0, alreadyExisted: 0, total: 3, }); - // Verify first bookmark (WebPage with URL) - expect(createdBookmarks[0]).toMatchObject({ + // Verify 3 bookmarks were staged + expect(stagedBookmarks).toHaveLength(3); + + // Verify first bookmark (WebPage with URL) - mymind has no paths, so root list + expect(stagedBookmarks[0]).toMatchObject({ title: "mymind", - content: { - type: "link", - url: "https://access.mymind.com/everything", - }, + url: "https://access.mymind.com/everything", + type: "link", tags: ["Wellness", "Self-Improvement", "Psychology"], + listIds: ["mymind Import"], }); - expect(createdBookmarks[0].addDate).toBeCloseTo( - new Date("2024-12-04T23:02:10Z").getTime() / 1000, + expect(stagedBookmarks[0].sourceAddedAt).toEqual( + new Date("2024-12-04T23:02:10Z"), ); // Verify second bookmark (WebPage with note) - expect(createdBookmarks[1]).toMatchObject({ + expect(stagedBookmarks[1]).toMatchObject({ title: "Movies / TV / Anime", - content: { - type: "link", - url: "https://fmhy.pages.dev/videopiracyguide", - }, + url: "https://fmhy.pages.dev/videopiracyguide", + type: "link", tags: ["Tools", "media", "Entertainment"], - notes: "Free Media!", + note: "Free Media!", + listIds: ["mymind Import"], }); // Verify third bookmark (Note with text content) - expect(createdBookmarks[2]).toMatchObject({ + expect(stagedBookmarks[2]).toMatchObject({ title: "", - content: { - type: "text", - text: "ā€¢ Critical Thinking\nā€¢ Empathy", - }, + content: "ā€¢ Critical Thinking\nā€¢ Empathy", + type: "text", tags: [], + listIds: ["mymind Import"], }); + + // Verify finalizeImportStaging was called + expect(finalizeImportStaging).toHaveBeenCalledWith("session-1"); }); }); -- cgit v1.2.3-70-g09d2