From e94a61a9bcb76139b90ef706eda5798dd3562db3 Mon Sep 17 00:00:00 2001
From: Hongbo Wu <hongbo@omnivore.app>
Date: Mon, 23 Oct 2023 17:44:38 +0800
Subject: [PATCH] fix: library item id could be updated if a different client
 request id supplied in save page api payload

---
 packages/api/src/services/save_page.ts      | 15 +++++++--
 packages/api/test/resolvers/article.test.ts | 34 +++++++--------------
 2 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/packages/api/src/services/save_page.ts b/packages/api/src/services/save_page.ts
index 3e82374d7..916d998c4 100644
--- a/packages/api/src/services/save_page.ts
+++ b/packages/api/src/services/save_page.ts
@@ -78,6 +78,7 @@ export const savePage = async (
   let clientRequestId = input.clientRequestId
 
   const itemToSave = parsedContentToLibraryItem({
+    itemId: clientRequestId,
     url: input.url,
     title: input.title,
     userId: user.id,
@@ -119,6 +120,9 @@ export const savePage = async (
       })
     )
     if (existingLibraryItem) {
+      clientRequestId = existingLibraryItem.id
+      slug = existingLibraryItem.slug
+
       // we don't want to update an rss feed item if rss-feeder is tring to re-save it
       if (existingLibraryItem.subscription === input.rssFeedUrl) {
         return {
@@ -127,11 +131,14 @@ export const savePage = async (
         }
       }
 
-      clientRequestId = existingLibraryItem.id
-      slug = existingLibraryItem.slug
+      // update the item except for id and slug
       await updateLibraryItem(
         clientRequestId,
-        itemToSave as QueryDeepPartialEntity<LibraryItem>,
+        {
+          ...itemToSave,
+          id: undefined,
+          slug: undefined,
+        } as QueryDeepPartialEntity<LibraryItem>,
         user.id
       )
     } else {
@@ -256,5 +263,7 @@ export const parsedContentToLibraryItem = ({
     wordCount: wordsCount(parsedContent?.textContent || ''),
     contentReader: contentReaderForLibraryItem(itemType, uploadFileId),
     subscription: rssFeedUrl,
+    archivedAt:
+      state === ArticleSavingRequestStatus.Archived ? new Date() : undefined,
   }
 }
diff --git a/packages/api/test/resolvers/article.test.ts b/packages/api/test/resolvers/article.test.ts
index 4f8a3c96e..fbf3b2d86 100644
--- a/packages/api/test/resolvers/article.test.ts
+++ b/packages/api/test/resolvers/article.test.ts
@@ -16,7 +16,7 @@ import {
   PageType,
   SyncUpdatedItemEdge,
   UpdateReason,
-  UploadFileStatus,
+  UploadFileStatus
 } from '../../src/generated/graphql'
 import { getRepository } from '../../src/repository'
 import { createGroup, deleteGroup } from '../../src/services/groups'
@@ -24,7 +24,7 @@ import { createHighlight } from '../../src/services/highlights'
 import {
   createLabel,
   deleteLabels,
-  saveLabelsInLibraryItem,
+  saveLabelsInLibraryItem
 } from '../../src/services/labels'
 import {
   createLibraryItem,
@@ -35,7 +35,7 @@ import {
   deleteLibraryItemsByUserId,
   findLibraryItemById,
   findLibraryItemByUrl,
-  updateLibraryItem,
+  updateLibraryItem
 } from '../../src/services/library_item'
 import { deleteUser } from '../../src/services/user'
 import * as createTask from '../../src/utils/createTask'
@@ -579,22 +579,26 @@ describe('Article API', () => {
 
         // Now save the link again, and ensure it is returned
         await graphqlRequest(
-          savePageQuery(url, title, originalContent),
+          savePageQuery(url, title, originalContent, null, null, generateFakeUuid()),
           authToken
         ).expect(200)
 
         allLinks = await graphqlRequest(searchQuery(''), authToken).expect(200)
+        expect(allLinks.body.data.search.edges[0].node.id).to.eq(justSavedId)
         expect(allLinks.body.data.search.edges[0].node.url).to.eq(url)
       })
     })
 
-    xcontext('when we also want to save labels and archives the item', () => {
+    context('when we also want to save labels and archives the item', () => {
+      before(() => {
+        url = 'https://blog.omnivore.app/new-url-2'
+      })
+
       after(async () => {
-        await deleteLibraryItemById(url, user.id)
+        await deleteLibraryItemByUrl(url, user.id)
       })
 
       it('saves the labels and archives the item', async () => {
-        url = 'https://blog.omnivore.app/new-url-2'
         const state = ArticleSavingRequestStatus.Archived
         const labels = ['test name', 'test name 2']
         await graphqlRequest(
@@ -660,22 +664,6 @@ describe('Article API', () => {
         )
       })
     })
-
-    xcontext('when we save labels', () => {
-      it('saves the labels and archives the item', async () => {
-        url = 'https://blog.omnivore.app/new-url-2'
-        const state = ArticleSavingRequestStatus.Archived
-        const labels = ['test name', 'test name 2']
-        await graphqlRequest(
-          saveUrlQuery(url, state, labels),
-          authToken
-        ).expect(200)
-
-        const savedItem = await findLibraryItemByUrl(url, user.id)
-        expect(savedItem?.archivedAt).to.not.be.null
-        expect(savedItem?.labels?.map((l) => l.name)).to.eql(labels)
-      })
-    })
   })
 
   describe('setBookmarkArticle', () => {