From e09061bd37c6496685ea0fdabe1d4d01f1b659ad Mon Sep 17 00:00:00 2001 From: Mohamed Bassem Date: Sun, 18 Jan 2026 21:38:21 +0000 Subject: feat: Add attachedBy field to update tags endpoint (#2281) * feat: Add attachedBy field to updateTags endpoint This change allows callers to specify the attachedBy field when updating tags on a bookmark. The field defaults to "human" if not provided, maintaining backward compatibility with existing code. Changes: - Added attachedBy field to zManipulatedTagSchema with default "human" - Updated updateTags endpoint to use the specified attachedBy value - Created mapping logic to correctly assign attachedBy to each tag * fix(cli): migrate bookmark source in migration command * fix * reduce queries --------- Co-authored-by: Claude --- packages/trpc/routers/bookmarks.test.ts | 68 +++++++++++++++++++++++++++++++++ packages/trpc/routers/bookmarks.ts | 54 ++++++++++++++++++++------ 2 files changed, 110 insertions(+), 12 deletions(-) (limited to 'packages/trpc') diff --git a/packages/trpc/routers/bookmarks.test.ts b/packages/trpc/routers/bookmarks.test.ts index 7e0e65e0..aaee5447 100644 --- a/packages/trpc/routers/bookmarks.test.ts +++ b/packages/trpc/routers/bookmarks.test.ts @@ -455,6 +455,74 @@ describe("Bookmark Routes", () => { expect(duplicateTagCount).toEqual(1); // Should only be attached once }); + test("updateTags with attachedBy field", async ({ + apiCallers, + }) => { + const api = apiCallers[0].bookmarks; + const bookmark = await api.createBookmark({ + url: "https://bookmark.com", + type: BookmarkTypes.LINK, + }); + + // Test 1: Attach tags with different attachedBy values + await api.updateTags({ + bookmarkId: bookmark.id, + attach: [ + { tagName: "ai-tag", attachedBy: "ai" }, + { tagName: "human-tag", attachedBy: "human" }, + { tagName: "default-tag" }, // Should default to "human" + ], + detach: [], + }); + + let b = await api.getBookmark({ bookmarkId: bookmark.id }); + expect(b.tags.length).toEqual(3); + + const aiTag = b.tags.find((t) => t.name === "ai-tag"); + const humanTag = b.tags.find((t) => t.name === "human-tag"); + const defaultTag = b.tags.find((t) => t.name === "default-tag"); + + expect(aiTag?.attachedBy).toEqual("ai"); + expect(humanTag?.attachedBy).toEqual("human"); + expect(defaultTag?.attachedBy).toEqual("human"); + + // Test 2: Attach existing tag by ID with different attachedBy + // First detach the ai-tag + await api.updateTags({ + bookmarkId: bookmark.id, + attach: [], + detach: [{ tagId: aiTag!.id }], + }); + + // Re-attach the same tag but as human + await api.updateTags({ + bookmarkId: bookmark.id, + attach: [{ tagId: aiTag!.id, attachedBy: "human" }], + detach: [], + }); + + b = await api.getBookmark({ bookmarkId: bookmark.id }); + const reAttachedTag = b.tags.find((t) => t.id === aiTag!.id); + expect(reAttachedTag?.attachedBy).toEqual("human"); + + // Test 3: Attach existing tag by name with AI attachedBy + const bookmark2 = await api.createBookmark({ + url: "https://bookmark2.com", + type: BookmarkTypes.LINK, + }); + + await api.updateTags({ + bookmarkId: bookmark2.id, + attach: [{ tagName: "ai-tag", attachedBy: "ai" }], + detach: [], + }); + + const b2 = await api.getBookmark({ bookmarkId: bookmark2.id }); + const aiTagOnB2 = b2.tags.find((t) => t.name === "ai-tag"); + expect(aiTagOnB2?.attachedBy).toEqual("ai"); + expect(aiTagOnB2?.id).toEqual(aiTag!.id); // Should be the same tag + }); + test("update bookmark text", async ({ apiCallers }) => { const api = apiCallers[0].bookmarks; const createdBookmark = await api.createBookmark({ diff --git a/packages/trpc/routers/bookmarks.ts b/packages/trpc/routers/bookmarks.ts index bf960748..37497bcf 100644 --- a/packages/trpc/routers/bookmarks.ts +++ b/packages/trpc/routers/bookmarks.ts @@ -714,10 +714,10 @@ export const bookmarksAppRouter = router({ ) .use(ensureBookmarkOwnership) .mutation(async ({ input, ctx }) => { - // Helper function to fetch tag IDs from a list of tag identifiers - const fetchTagIds = async ( + // Helper function to fetch tag IDs and their names from a list of tag identifiers + const fetchTagIdsWithNames = async ( tagIdentifiers: { tagId?: string; tagName?: string }[], - ): Promise => { + ): Promise<{ id: string; name: string }[]> => { const tagIds = tagIdentifiers.flatMap((t) => t.tagId ? [t.tagId] : [], ); @@ -729,7 +729,7 @@ export const bookmarksAppRouter = router({ const [byIds, byNames] = await Promise.all([ tagIds.length > 0 ? ctx.db - .select({ id: bookmarkTags.id }) + .select({ id: bookmarkTags.id, name: bookmarkTags.name }) .from(bookmarkTags) .where( and( @@ -740,7 +740,7 @@ export const bookmarksAppRouter = router({ : Promise.resolve([]), tagNames.length > 0 ? ctx.db - .select({ id: bookmarkTags.id }) + .select({ id: bookmarkTags.id, name: bookmarkTags.name }) .from(bookmarkTags) .where( and( @@ -751,15 +751,25 @@ export const bookmarksAppRouter = router({ : Promise.resolve([]), ]); - // Union results and deduplicate tag IDs - const results = [...byIds, ...byNames]; - return [...new Set(results.map((t) => t.id))]; + // Union results and deduplicate by tag ID + const seen = new Set(); + const results: { id: string; name: string }[] = []; + + for (const tag of [...byIds, ...byNames]) { + if (!seen.has(tag.id)) { + seen.add(tag.id); + results.push({ id: tag.id, name: tag.name }); + } + } + + return results; }; // Normalize tag names and create new tags outside transaction to reduce transaction duration const normalizedAttachTags = input.attach.map((tag) => ({ tagId: tag.tagId, tagName: tag.tagName ? normalizeTagName(tag.tagName) : undefined, + attachedBy: tag.attachedBy, })); { @@ -779,11 +789,31 @@ export const bookmarksAppRouter = router({ } // Fetch tag IDs for attachment/detachment now that we know that they all exist - const [allIdsToAttach, idsToRemove] = await Promise.all([ - fetchTagIds(normalizedAttachTags), - fetchTagIds(input.detach), + const [attachTagsWithNames, detachTagsWithNames] = await Promise.all([ + fetchTagIdsWithNames(normalizedAttachTags), + fetchTagIdsWithNames(input.detach), ]); + // Build the attachedBy map from the fetched results + const tagIdToAttachedBy = new Map(); + + for (const fetchedTag of attachTagsWithNames) { + // Find the corresponding input tag + const inputTag = normalizedAttachTags.find( + (t) => + (t.tagId && t.tagId === fetchedTag.id) || + (t.tagName && t.tagName === fetchedTag.name), + ); + + if (inputTag) { + tagIdToAttachedBy.set(fetchedTag.id, inputTag.attachedBy); + } + } + + // Extract just the IDs for the transaction + const allIdsToAttach = attachTagsWithNames.map((t) => t.id); + const idsToRemove = detachTagsWithNames.map((t) => t.id); + const res = await ctx.db.transaction(async (tx) => { // Detaches if (idsToRemove.length > 0) { @@ -805,7 +835,7 @@ export const bookmarksAppRouter = router({ allIdsToAttach.map((i) => ({ tagId: i, bookmarkId: input.bookmarkId, - attachedBy: "human" as const, + attachedBy: tagIdToAttachedBy.get(i) ?? "human", })), ) .onConflictDoNothing(); -- cgit v1.2.3-70-g09d2