From e94a61a9bcb76139b90ef706eda5798dd3562db3 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 23 Oct 2023 17:44:38 +0800 Subject: [PATCH 1/8] 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, + { + ...itemToSave, + id: undefined, + slug: undefined, + } as QueryDeepPartialEntity, 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', () => { From 27b352f9ae897df936725c98adc76310d701734c Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 23 Oct 2023 19:07:47 +0800 Subject: [PATCH 2/8] fix: updates since api returns error for android client if invalid date is supplied --- packages/api/src/resolvers/article/index.ts | 8 +++++++- packages/api/test/resolvers/article.test.ts | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 85a4bd4de..f0e4cd327 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -46,6 +46,7 @@ import { TypeaheadSearchSuccess, UpdateReason, UpdatesSinceError, + UpdatesSinceErrorCode, UpdatesSinceSuccess, } from '../../generated/graphql' import { getColumns } from '../../repository' @@ -702,7 +703,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), diff --git a/packages/api/test/resolvers/article.test.ts b/packages/api/test/resolvers/article.test.ts index fbf3b2d86..4607044f4 100644 --- a/packages/api/test/resolvers/article.test.ts +++ b/packages/api/test/resolvers/article.test.ts @@ -1670,6 +1670,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', () => { From 0c60706503887e1b96a003ef17fa089824778a58 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 23 Oct 2023 19:34:41 +0800 Subject: [PATCH 3/8] fix: date filter in bulk action api --- packages/api/src/services/save_page.ts | 8 +++----- packages/api/src/utils/search.ts | 6 +++--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/api/src/services/save_page.ts b/packages/api/src/services/save_page.ts index 916d998c4..68af11b2c 100644 --- a/packages/api/src/services/save_page.ts +++ b/packages/api/src/services/save_page.ts @@ -1,6 +1,7 @@ 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' @@ -166,12 +167,9 @@ export const savePage = async ( } if (parseResult.highlightData) { - const highlight = { - updatedAt: new Date(), - createdAt: new Date(), - userId: user.id, + const highlight: DeepPartial = { ...parseResult.highlightData, - type: HighlightType.Highlight, + user: { id: user.id }, } if (!(await createHighlight(highlight, clientRequestId, user.id))) { diff --git a/packages/api/src/utils/search.ts b/packages/api/src/utils/search.ts index c4799ba7b..96abd859b 100644 --- a/packages/api/src/utils/search.ts +++ b/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 { From e673db423b8503cf8b078d8ecbf6f8c9f99c41b8 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 23 Oct 2023 21:11:37 +0800 Subject: [PATCH 4/8] limit max content size to 10MB --- packages/api/src/schema.ts | 6 ++++-- packages/api/src/services/library_item.ts | 17 ++++++++++++++--- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/packages/api/src/schema.ts b/packages/api/src/schema.ts index de3f5209c..9bfe8406b 100755 --- a/packages/api/src/schema.ts +++ b/packages/api/src/schema.ts @@ -543,7 +543,8 @@ const schema = gql` title: String! byline: String dir: String - content: String! + # max length of 10MB + content: String! @sanitize(maxLength: 10485760) textContent: String! length: Int! excerpt: String! @@ -559,7 +560,8 @@ const schema = gql` source: String! clientRequestId: ID! title: String - originalContent: String! + # max length of 10MB + originalContent: String! @sanitize(maxLength: 10485760) parseResult: ParseResult state: ArticleSavingRequestStatus labels: [CreateLabelInput!] diff --git a/packages/api/src/services/library_item.ts b/packages/api/src/services/library_item.ts index 8dab74fc5..e662ecb69 100644 --- a/packages/api/src/services/library_item.ts +++ b/packages/api/src/services/library_item.ts @@ -431,7 +431,13 @@ export const updateLibraryItem = async ( await pubsub.entityUpdated>( EntityType.PAGE, - { ...libraryItem, id }, + { + ...libraryItem, + id, + // don't send original content and readable content + originalContent: undefined, + readableContent: undefined, + }, userId ) @@ -535,9 +541,14 @@ export const createLibraryItem = async ( userId ) - await pubsub.entityCreated( + await pubsub.entityCreated>( EntityType.PAGE, - newLibraryItem, + { + ...newLibraryItem, + // don't send original content and readable content + originalContent: undefined, + readableContent: undefined, + }, userId ) From 79a647deb45ae50593c972d81d85ad3ef0004b75 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 23 Oct 2023 21:27:16 +0800 Subject: [PATCH 5/8] do not publish content and original html in the queue --- packages/api/src/resolvers/article/index.ts | 2 +- packages/api/src/schema.ts | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index f0e4cd327..c45b4b67d 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -623,7 +623,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 diff --git a/packages/api/src/schema.ts b/packages/api/src/schema.ts index 9bfe8406b..de3f5209c 100755 --- a/packages/api/src/schema.ts +++ b/packages/api/src/schema.ts @@ -543,8 +543,7 @@ const schema = gql` title: String! byline: String dir: String - # max length of 10MB - content: String! @sanitize(maxLength: 10485760) + content: String! textContent: String! length: Int! excerpt: String! @@ -560,8 +559,7 @@ const schema = gql` source: String! clientRequestId: ID! title: String - # max length of 10MB - originalContent: String! @sanitize(maxLength: 10485760) + originalContent: String! parseResult: ParseResult state: ArticleSavingRequestStatus labels: [CreateLabelInput!] From 3f14ac4f9b0da3ddafe01761a0b5daeba91b6845 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 23 Oct 2023 21:31:38 +0800 Subject: [PATCH 6/8] skip publishing event for importing --- packages/api/src/services/library_item.ts | 7 ++++++- packages/api/src/services/save_page.ts | 8 ++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/api/src/services/library_item.ts b/packages/api/src/services/library_item.ts index e662ecb69..7fec735e0 100644 --- a/packages/api/src/services/library_item.ts +++ b/packages/api/src/services/library_item.ts @@ -527,7 +527,8 @@ export const createLibraryItems = async ( export const createLibraryItem = async ( libraryItem: DeepPartial, userId: string, - pubsub = createPubSubClient() + pubsub = createPubSubClient(), + skipPubSub = false ): Promise => { const newLibraryItem = await authTrx( async (tx) => @@ -541,6 +542,10 @@ export const createLibraryItem = async ( userId ) + if (skipPubSub) { + return newLibraryItem + } + await pubsub.entityCreated>( EntityType.PAGE, { diff --git a/packages/api/src/services/save_page.ts b/packages/api/src/services/save_page.ts index 68af11b2c..e14ca4850 100644 --- a/packages/api/src/services/save_page.ts +++ b/packages/api/src/services/save_page.ts @@ -7,7 +7,6 @@ import { User } from '../entity/user' import { homePageURL } from '../env' import { ArticleSavingRequestStatus, - HighlightType, Maybe, PreparedDocumentInput, SaveErrorCode, @@ -144,7 +143,12 @@ export const savePage = async ( ) } 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 } From f497c0ac5ba917e0c0e38670c193b919510bbe06 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 23 Oct 2023 21:43:01 +0800 Subject: [PATCH 7/8] add test cases --- packages/api/test/resolvers/article.test.ts | 60 +++++++++++++++++---- 1 file changed, 49 insertions(+), 11 deletions(-) diff --git a/packages/api/test/resolvers/article.test.ts b/packages/api/test/resolvers/article.test.ts index 4607044f4..c50466e13 100644 --- a/packages/api/test/resolvers/article.test.ts +++ b/packages/api/test/resolvers/article.test.ts @@ -1741,18 +1741,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: '

test

', + slug: 'test-item', + originalUrl: `https://blog.omnivore.app/p/bulk-action-archive`, + publishedAt: new Date('2023-10-01'), + }, + { + user, + title: 'test item 2', + readableContent: '

test

', + 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 () => { From 41866d5cb7213f3bd1c331be7c145f9764b9e517 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 23 Oct 2023 22:02:43 +0800 Subject: [PATCH 8/8] cont --- packages/api/src/services/save_page.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/api/src/services/save_page.ts b/packages/api/src/services/save_page.ts index e14ca4850..80206277b 100644 --- a/packages/api/src/services/save_page.ts +++ b/packages/api/src/services/save_page.ts @@ -174,6 +174,7 @@ export const savePage = async ( const highlight: DeepPartial = { ...parseResult.highlightData, user: { id: user.id }, + libraryItem: { id: clientRequestId }, } if (!(await createHighlight(highlight, clientRequestId, user.id))) {