fix(web):Add validation on updateTags request to ensure that an empty tagName doesn't attach all tags to a bookmark #421 (#428)

* [Bug Report] Importing bookmarks adds all tags to all bookmarks #421
fixed an issue that caused all existing tags to be assigned to a new bookmark

* Add validation on the input of update tags to ensure that its not empty

---------

Co-authored-by: MohamedBassem <me@mbassem.com>

2 files changed, 39 insertions, 10 deletions

diff --git a/packages/trpc/routers/bookmarks.test.ts b/packages/trpc/routers/bookmarks.test.ts
index 5219e522..9fcbec52 100644
--- a/packages/trpc/routers/bookmarks.test.ts
+++ b/packages/trpc/routers/bookmarks.test.ts
@@ -169,7 +169,22 @@ describe("Bookmark Routes", () => {
       "tag2",
       "tag3",
       "tag4",
+      "tag5",
     ]);
+
+    await expect(() =>
+      api.updateTags({ bookmarkId: bookmark.id, attach: [{}], detach: [] }),
+    ).rejects.toThrow(/You must provide either a tagId or a tagName/);
+    await expect(() =>
+      api.updateTags({ bookmarkId: bookmark.id, attach: [], detach: [{}] }),
+    ).rejects.toThrow(/You must provide either a tagId or a tagName/);
+    await expect(() =>
+      api.updateTags({
+        bookmarkId: bookmark.id,
+        attach: [{ tagName: "" }],
+        detach: [{}],
+      }),
+    ).rejects.toThrow(/You must provide either a tagId or a tagName/);
   });
 
   test<CustomTestContext>("update bookmark text", async ({ apiCallers }) => {
diff --git a/packages/trpc/routers/bookmarks.ts b/packages/trpc/routers/bookmarks.ts
index 312c3acc..9990c6ed 100644
--- a/packages/trpc/routers/bookmarks.ts
+++ b/packages/trpc/routers/bookmarks.ts
@@ -678,18 +678,28 @@ export const bookmarksAppRouter = router({
       z.object({
         bookmarkId: z.string(),
         attach: z.array(
-          z.object({
-            // At least one of the two must be set
-            tagId: z.string().optional(), // If the tag already exists and we know its id we should pass it
-            tagName: z.string().optional(),
-          }),
+          z
+            .object({
+              // At least one of the two must be set
+              tagId: z.string().optional(), // If the tag already exists and we know its id we should pass it
+              tagName: z.string().optional(),
+            })
+            .refine((val) => !!val.tagId || !!val.tagName, {
+              message: "You must provide either a tagId or a tagName",
+              path: ["tagId", "tagName"],
+            }),
         ),
         detach: z.array(
-          z.object({
-            // At least one of the two must be set
-            tagId: z.string().optional(),
-            tagName: z.string().optional(), // Also allow removing by tagName, to make CLI usage easier
-          }),
+          z
+            .object({
+              // At least one of the two must be set
+              tagId: z.string().optional(),
+              tagName: z.string().optional(), // Also allow removing by tagName, to make CLI usage easier
+            })
+            .refine((val) => !!val.tagId || !!val.tagName, {
+              message: "You must provide either a tagId or a tagName",
+              path: ["tagId", "tagName"],
+            }),
         ),
       }),
     )
@@ -767,6 +777,9 @@ export const bookmarksAppRouter = router({
             .returning();
         }
 
+        // If there is nothing to add, the "or" statement will become useless and
+        // the query below will simply select all the existing tags for this user and assign them to the bookmark
+        invariant(toAddTagNames.length > 0 || toAddTagIds.length > 0);
         const allIds = (
           await tx.query.bookmarkTags.findMany({
             where: and(
@@ -797,6 +810,7 @@ export const bookmarksAppRouter = router({
             })),
           )
           .onConflictDoNothing();
+
         await triggerSearchReindex(input.bookmarkId);
         return {
           bookmarkId: input.bookmarkId,