Merge pull request #2995 from omnivore-app/fix/item-id-update

fix: library item id could be updated if a different client request id supplied in save page api payload

packages/api/src/resolvers/article/index.ts

@@ -46,6 +46,7 @@ import {
   TypeaheadSearchSuccess,
   UpdateReason,
   UpdatesSinceError,
+  UpdatesSinceErrorCode,
   UpdatesSinceSuccess,
 } from '../../generated/graphql'
 import { getColumns } from '../../repository'
@@ -644,7 +645,7 @@ export const searchResolver = authorized<
       size: first + 1, // fetch one more item to get next cursor
       sort: searchQuery.sort,
       includePending: true,
-      includeContent: params.includeContent || false,
+      includeContent: !!params.includeContent,
       ...searchQuery,
     },
     uid
@@ -724,7 +725,12 @@ export const updatesSinceResolver = authorized<
 
   const startCursor = after || ''
   const size = first || 10
-  const startDate = new Date(since)
+  let startDate = new Date(since)
+  if (isNaN(startDate.getTime())) {
+    // for android app compatibility
+    startDate = new Date(0)
+  }
+
   const { libraryItems, count } = await searchLibraryItems(
     {
       from: Number(startCursor),

packages/api/src/services/library_item.ts

@@ -431,7 +431,13 @@ export const updateLibraryItem = async (
 
   await pubsub.entityUpdated<QueryDeepPartialEntity<LibraryItem>>(
     EntityType.PAGE,
-    { ...libraryItem, id },
+    {
+      ...libraryItem,
+      id,
+      // don't send original content and readable content
+      originalContent: undefined,
+      readableContent: undefined,
+    },
     userId
   )
 
@@ -521,7 +527,8 @@ export const createLibraryItems = async (
 export const createLibraryItem = async (
   libraryItem: DeepPartial<LibraryItem>,
   userId: string,
-  pubsub = createPubSubClient()
+  pubsub = createPubSubClient(),
+  skipPubSub = false
 ): Promise<LibraryItem> => {
   const newLibraryItem = await authTrx(
     async (tx) =>
@@ -535,9 +542,18 @@ export const createLibraryItem = async (
     userId
   )
 
-  await pubsub.entityCreated<LibraryItem>(
+  if (skipPubSub) {
+    return newLibraryItem
+  }
+
+  await pubsub.entityCreated<DeepPartial<LibraryItem>>(
     EntityType.PAGE,
-    newLibraryItem,
+    {
+      ...newLibraryItem,
+      // don't send original content and readable content
+      originalContent: undefined,
+      readableContent: undefined,
+    },
     userId
   )
 

packages/api/src/services/save_page.ts

@@ -1,12 +1,12 @@
 import { Readability } from '@omnivore/readability'
 import { DeepPartial } from 'typeorm'
 import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity'
+import { Highlight } from '../entity/highlight'
 import { LibraryItem, LibraryItemState } from '../entity/library_item'
 import { User } from '../entity/user'
 import { homePageURL } from '../env'
 import {
   ArticleSavingRequestStatus,
-  HighlightType,
   Maybe,
   PreparedDocumentInput,
   SaveErrorCode,
@@ -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,16 +131,24 @@ 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 {
       // do not publish a pubsub event if the item is imported
-      const newItem = await createLibraryItem(itemToSave, user.id)
+      const newItem = await createLibraryItem(
+        itemToSave,
+        user.id,
+        undefined,
+        isImported
+      )
       clientRequestId = newItem.id
     }
 
@@ -159,12 +171,10 @@ export const savePage = async (
   }
 
   if (parseResult.highlightData) {
-    const highlight = {
-      updatedAt: new Date(),
-      createdAt: new Date(),
-      userId: user.id,
+    const highlight: DeepPartial<Highlight> = {
       ...parseResult.highlightData,
-      type: HighlightType.Highlight,
+      user: { id: user.id },
+      libraryItem: { id: clientRequestId },
     }
 
     if (!(await createHighlight(highlight, clientRequestId, user.id))) {
@@ -256,5 +266,7 @@ export const parsedContentToLibraryItem = ({
     wordCount: wordsCount(parsedContent?.textContent || ''),
     contentReader: contentReaderForLibraryItem(itemType, uploadFileId),
     subscription: rssFeedUrl,
+    archivedAt:
+      state === ArticleSavingRequestStatus.Archived ? new Date() : undefined,
   }
 }

packages/api/src/utils/search.ts

@@ -249,13 +249,13 @@ const parseDateFilter = (
 
   switch (field.toUpperCase()) {
     case 'PUBLISHED':
-      field = 'publishedAt'
+      field = 'published_at'
       break
     case 'SAVED':
-      field = 'savedAt'
+      field = 'saved_at'
       break
     case 'UPDATED':
-      field = 'updatedAt'
+      field = 'updated_at'
   }
 
   return {

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'
@@ -581,22 +581,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(
@@ -662,22 +666,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', () => {
@@ -1715,6 +1703,21 @@ describe('Article API', () => {
         UpdateReason.Deleted
       )
     })
+
+    context('when since is -1000000000-01-01T00:00:00Z from android app', () => {
+      before(() => {
+        since = '-1000000000-01-01T00:00:00Z'
+      })
+
+      it('returns all', async () => {
+        const res = await graphqlRequest(
+          updatesSinceQuery(since),
+          authToken
+        ).expect(200)
+
+        expect(res.body.data.updatesSince.edges.length).to.eql(5)
+      })
+    })
   })
 
   describe('BulkAction API', () => {
@@ -1771,18 +1774,56 @@ describe('Article API', () => {
       })
     })
 
-    context('when action is Archive', () => {
-      it('archives all items', async () => {
-        const res = await graphqlRequest(
-          bulkActionQuery(BulkActionType.Archive),
-          authToken
-        ).expect(200)
-        expect(res.body.data.bulkAction.success).to.be.true
+    context(
+      'when action is Archive and query is published:*..2023-10-01',
+      () => {
+        let items: LibraryItem[] = []
 
-        const items = await graphqlRequest(searchQuery(), authToken).expect(200)
-        expect(items.body.data.search.pageInfo.totalCount).to.eql(0)
-      })
-    })
+        before(async () => {
+          items = await createLibraryItems(
+            [
+              {
+                user,
+                title: 'test item',
+                readableContent: '<p>test</p>',
+                slug: 'test-item',
+                originalUrl: `https://blog.omnivore.app/p/bulk-action-archive`,
+                publishedAt: new Date('2023-10-01'),
+              },
+              {
+                user,
+                title: 'test item 2',
+                readableContent: '<p>test</p>',
+                slug: 'test-item-2',
+                originalUrl: `https://blog.omnivore.app/p/bulk-action-archive-2`,
+                publishedAt: new Date('2023-10-02'),
+              },
+            ],
+            user.id
+          )
+        })
+
+        after(async () => {
+          // Delete all items
+          await deleteLibraryItems(items, user.id)
+        })
+
+        it('archives old items', async () => {
+          const res = await graphqlRequest(
+            bulkActionQuery(BulkActionType.Archive, 'published:*..2023-10-01'),
+            authToken
+          ).expect(200)
+          expect(res.body.data.bulkAction.success).to.be.true
+
+          const response = await graphqlRequest(
+            searchQuery('in:archive'),
+            authToken
+          ).expect(200)
+          expect(response.body.data.search.pageInfo.totalCount).to.eql(1)
+          expect(response.body.data.search.edges[0].node.id).to.eql(items[0].id)
+        })
+      }
+    )
 
     context('when action is Delete', () => {
       it('deletes all items', async () => {