refactor: added the bookmark type to the database (#256)

* refactoring asset types
Extracted out functions to silently delete assets and to update them after crawling
Generalized the mapping of assets to bookmark fields to make extending them easier

* Added the bookmark type to the database
Introduced an enum to have better type safety
cleaned up the code and based some code on the type directly

* add BookmarkType.UNKNWON

* lint and remove unused function

---------

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

2 files changed, 80 insertions, 63 deletions

diff --git a/packages/trpc/routers/bookmarks.test.ts b/packages/trpc/routers/bookmarks.test.ts
index c9627626..67e73ad1 100644
--- a/packages/trpc/routers/bookmarks.test.ts
+++ b/packages/trpc/routers/bookmarks.test.ts
@@ -1,6 +1,7 @@
 import { assert, beforeEach, describe, expect, test } from "vitest";
 
 import { bookmarks } from "@hoarder/db/schema";
+import { BookmarkTypes } from "@hoarder/shared/types/bookmarks";
 
 import type { CustomTestContext } from "../testUtils";
 import { defaultBeforeEach } from "../testUtils";
@@ -12,15 +13,15 @@ describe("Bookmark Routes", () => {
     const api = apiCallers[0].bookmarks;
     const bookmark = await api.createBookmark({
       url: "https://google.com",
-      type: "link",
+      type: BookmarkTypes.LINK,
     });
 
     const res = await api.getBookmark({ bookmarkId: bookmark.id });
-    assert(res.content.type == "link");
+    assert(res.content.type == BookmarkTypes.LINK);
     expect(res.content.url).toEqual("https://google.com");
     expect(res.favourited).toEqual(false);
     expect(res.archived).toEqual(false);
-    expect(res.content.type).toEqual("link");
+    expect(res.content.type).toEqual(BookmarkTypes.LINK);
   });
 
   test<CustomTestContext>("delete bookmark", async ({ apiCallers }) => {
@@ -29,7 +30,7 @@ describe("Bookmark Routes", () => {
     // Create the bookmark
     const bookmark = await api.createBookmark({
       url: "https://google.com",
-      type: "link",
+      type: BookmarkTypes.LINK,
     });
 
     // It should exist
@@ -50,7 +51,7 @@ describe("Bookmark Routes", () => {
     // Create the bookmark
     const bookmark = await api.createBookmark({
       url: "https://google.com",
-      type: "link",
+      type: BookmarkTypes.LINK,
     });
 
     await api.updateBookmark({
@@ -71,12 +72,12 @@ describe("Bookmark Routes", () => {
 
     const bookmark1 = await api.createBookmark({
       url: "https://google.com",
-      type: "link",
+      type: BookmarkTypes.LINK,
     });
 
     const bookmark2 = await api.createBookmark({
       url: "https://google2.com",
-      type: "link",
+      type: BookmarkTypes.LINK,
     });
 
     {
@@ -120,7 +121,7 @@ describe("Bookmark Routes", () => {
     const api = apiCallers[0].bookmarks;
     const createdBookmark = await api.createBookmark({
       url: "https://google.com",
-      type: "link",
+      type: BookmarkTypes.LINK,
     });
 
     await api.updateTags({
@@ -161,7 +162,7 @@ describe("Bookmark Routes", () => {
     const api = apiCallers[0].bookmarks;
     const createdBookmark = await api.createBookmark({
       text: "HELLO WORLD",
-      type: "text",
+      type: BookmarkTypes.TEXT,
     });
 
     await api.updateBookmarkText({
@@ -170,17 +171,17 @@ describe("Bookmark Routes", () => {
     });
 
     const bookmark = await api.getBookmark({ bookmarkId: createdBookmark.id });
-    assert(bookmark.content.type == "text");
+    assert(bookmark.content.type == BookmarkTypes.TEXT);
     expect(bookmark.content.text).toEqual("WORLD HELLO");
   });
 
   test<CustomTestContext>("privacy", async ({ apiCallers }) => {
     const user1Bookmark = await apiCallers[0].bookmarks.createBookmark({
-      type: "link",
+      type: BookmarkTypes.LINK,
       url: "https://google.com",
     });
     const user2Bookmark = await apiCallers[1].bookmarks.createBookmark({
-      type: "link",
+      type: BookmarkTypes.LINK,
       url: "https://google.com",
     });
 
@@ -219,20 +220,20 @@ describe("Bookmark Routes", () => {
     // Two users with google in their bookmarks
     const bookmark1User1 = await apiCallers[0].bookmarks.createBookmark({
       url: "https://google.com",
-      type: "link",
+      type: BookmarkTypes.LINK,
     });
     expect(bookmark1User1.alreadyExists).toEqual(false);
 
     const bookmark1User2 = await apiCallers[1].bookmarks.createBookmark({
       url: "https://google.com",
-      type: "link",
+      type: BookmarkTypes.LINK,
     });
     expect(bookmark1User2.alreadyExists).toEqual(false);
 
     // User1 attempting to re-add google. Should return the existing bookmark
     const bookmark2User1 = await apiCallers[0].bookmarks.createBookmark({
       url: "https://google.com",
-      type: "link",
+      type: BookmarkTypes.LINK,
     });
     expect(bookmark2User1.alreadyExists).toEqual(true);
     expect(bookmark2User1.id).toEqual(bookmark1User1.id);
@@ -240,7 +241,7 @@ describe("Bookmark Routes", () => {
     // User2 attempting to re-add google. Should return the existing bookmark
     const bookmark2User2 = await apiCallers[1].bookmarks.createBookmark({
       url: "https://google.com",
-      type: "link",
+      type: BookmarkTypes.LINK,
     });
     expect(bookmark2User2.alreadyExists).toEqual(true);
     expect(bookmark2User2.id).toEqual(bookmark1User2.id);
@@ -248,7 +249,7 @@ describe("Bookmark Routes", () => {
     // User1 adding google2. Should not return an existing bookmark
     const bookmark3User1 = await apiCallers[0].bookmarks.createBookmark({
       url: "https://google2.com",
-      type: "link",
+      type: BookmarkTypes.LINK,
     });
     expect(bookmark3User1.alreadyExists).toEqual(false);
   });
@@ -261,6 +262,7 @@ describe("Bookmark Routes", () => {
     const bookmarkWithDate = (date_ms: number) => ({
       userId: user.id,
       createdAt: new Date(date_ms),
+      type: BookmarkTypes.TEXT as const,
     });
 
     // One normal bookmark
@@ -274,7 +276,7 @@ describe("Bookmark Routes", () => {
     for (let i = 0; i < 10; i++) {
       values.push(bookmarkWithDate(now));
     }
-    // And then another one with a second afterards
+    // And then another one with a second afterwards
     for (let i = 0; i < 10; i++) {
       now -= 1000;
       values.push(bookmarkWithDate(now));
diff --git a/packages/trpc/routers/bookmarks.ts b/packages/trpc/routers/bookmarks.ts
index 8644bbcf..edf196bb 100644
--- a/packages/trpc/routers/bookmarks.ts
+++ b/packages/trpc/routers/bookmarks.ts
@@ -29,6 +29,7 @@ import {
 } from "@hoarder/shared/queues";
 import { getSearchIdxClient } from "@hoarder/shared/search";
 import {
+  BookmarkTypes,
   DEFAULT_NUM_BOOKMARKS_PER_PAGE,
   zBareBookmarkSchema,
   zBookmarkSchema,
@@ -176,25 +177,29 @@ async function cleanupAssetForBookmark(
 function toZodSchema(bookmark: BookmarkQueryReturnType): ZBookmark {
   const { tagsOnBookmarks, link, text, asset, assets, ...rest } = bookmark;
 
-  let content: ZBookmarkContent;
-  if (link) {
-    content = {
-      type: "link",
-      ...mapAssetsToBookmarkFields(assets),
-      ...link,
-    };
-  } else if (text) {
-    content = { type: "text", text: text.text ?? "" };
-  } else if (asset) {
-    content = {
-      type: "asset",
-      assetType: asset.assetType,
-      assetId: asset.assetId,
-      fileName: asset.fileName,
-      sourceUrl: asset.sourceUrl,
-    };
-  } else {
-    content = { type: "unknown" };
+  let content: ZBookmarkContent = {
+    type: BookmarkTypes.UNKNWON,
+  };
+  switch (bookmark.type) {
+    case BookmarkTypes.LINK:
+      content = {
+        type: bookmark.type,
+        ...mapAssetsToBookmarkFields(assets),
+        ...link,
+      };
+      break;
+    case BookmarkTypes.TEXT:
+      content = { type: bookmark.type, text: text.text ?? "" };
+      break;
+    case BookmarkTypes.ASSET:
+      content = {
+        type: bookmark.type,
+        assetType: asset.assetType,
+        assetId: asset.assetId,
+        fileName: asset.fileName,
+        sourceUrl: asset.sourceUrl,
+      };
+      break;
   }
 
   return {
@@ -218,19 +223,23 @@ export const bookmarksAppRouter = router({
       ),
     )
     .mutation(async ({ input, ctx }) => {
-      if (input.type == "link") {
+      if (input.type == BookmarkTypes.LINK) {
         // This doesn't 100% protect from duplicates because of races, but it's more than enough for this usecase.
         const alreadyExists = await attemptToDedupLink(ctx, input.url);
         if (alreadyExists) {
           return { ...alreadyExists, alreadyExists: true };
         }
       }
+      if (input.type == BookmarkTypes.UNKNWON) {
+        throw new TRPCError({ code: "BAD_REQUEST" });
+      }
       const bookmark = await ctx.db.transaction(async (tx) => {
         const bookmark = (
           await tx
             .insert(bookmarks)
             .values({
               userId: ctx.user.id,
+              type: input.type,
             })
             .returning()
         )[0];
@@ -238,7 +247,7 @@ export const bookmarksAppRouter = router({
         let content: ZBookmarkContent;
 
         switch (input.type) {
-          case "link": {
+          case BookmarkTypes.LINK: {
             const link = (
               await tx
                 .insert(bookmarkLinks)
@@ -249,12 +258,12 @@ export const bookmarksAppRouter = router({
                 .returning()
             )[0];
             content = {
-              type: "link",
+              type: BookmarkTypes.LINK,
               ...link,
             };
             break;
           }
-          case "text": {
+          case BookmarkTypes.TEXT: {
             const text = (
               await tx
                 .insert(bookmarkTexts)
@@ -262,12 +271,12 @@ export const bookmarksAppRouter = router({
                 .returning()
             )[0];
             content = {
-              type: "text",
+              type: BookmarkTypes.TEXT,
               text: text.text ?? "",
             };
             break;
           }
-          case "asset": {
+          case BookmarkTypes.ASSET: {
             const [asset] = await tx
               .insert(bookmarkAssets)
               .values({
@@ -281,15 +290,12 @@ export const bookmarksAppRouter = router({
               })
               .returning();
             content = {
-              type: "asset",
+              type: BookmarkTypes.ASSET,
               assetType: asset.assetType,
               assetId: asset.assetId,
             };
             break;
           }
-          case "unknown": {
-            throw new TRPCError({ code: "BAD_REQUEST" });
-          }
         }
 
         return {
@@ -302,15 +308,15 @@ export const bookmarksAppRouter = router({
 
       // Enqueue crawling request
       switch (bookmark.content.type) {
-        case "link": {
+        case BookmarkTypes.LINK: {
           // The crawling job triggers openai when it's done
           await LinkCrawlerQueue.add("crawl", {
             bookmarkId: bookmark.id,
           });
           break;
         }
-        case "text":
-        case "asset": {
+        case BookmarkTypes.TEXT:
+        case BookmarkTypes.ASSET: {
           await OpenAIQueue.add("openai", {
             bookmarkId: bookmark.id,
           });
@@ -565,19 +571,28 @@ export const bookmarksAppRouter = router({
           const bookmarkId = row.bookmarksSq.id;
           if (!acc[bookmarkId]) {
             let content: ZBookmarkContent;
-            if (row.bookmarkLinks) {
-              content = { type: "link", ...row.bookmarkLinks };
-            } else if (row.bookmarkTexts) {
-              content = { type: "text", text: row.bookmarkTexts.text ?? "" };
-            } else if (row.bookmarkAssets) {
-              content = {
-                type: "asset",
-                assetId: row.bookmarkAssets.assetId,
-                assetType: row.bookmarkAssets.assetType,
-                fileName: row.bookmarkAssets.fileName,
-              };
-            } else {
-              content = { type: "unknown" };
+            switch (row.bookmarksSq.type) {
+              case BookmarkTypes.LINK: {
+                content = { type: row.bookmarksSq.type, ...row.bookmarkLinks! };
+                break;
+              }
+              case BookmarkTypes.TEXT: {
+                content = {
+                  type: row.bookmarksSq.type,
+                  text: row.bookmarkTexts?.text ?? "",
+                };
+                break;
+              }
+              case BookmarkTypes.ASSET: {
+                const bookmarkAssets = row.bookmarkAssets!;
+                content = {
+                  type: row.bookmarksSq.type,
+                  assetId: bookmarkAssets.assetId,
+                  assetType: bookmarkAssets.assetType,
+                  fileName: bookmarkAssets.fileName,
+                };
+                break;
+              }
             }
             acc[bookmarkId] = {
               ...row.bookmarksSq,