From e53cc28683365fffce6c7ce685a4f7e9970b182c Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Mon, 5 Jun 2023 14:44:47 +0800 Subject: [PATCH 1/3] Fix issue where we return EPUB content reader for items marked as book in metadata If we parse a page with in it now, we incorrectly set this to ContentReader.EPUB, which then causes syncing issues on iOS which doesn't have an EPUB reader type defined in its GraphQL schema yet. --- packages/api/src/resolvers/article/index.ts | 8 ++++---- packages/api/src/resolvers/function_resolvers.ts | 9 ++++++--- packages/api/src/services/speech.ts | 7 ++++--- packages/api/src/utils/uploads.ts | 9 +++++++-- 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 157e4b489..d9706bdfa 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -100,7 +100,7 @@ import { } from '../../utils/parser' import { parseSearchQuery, SortBy, SortOrder } from '../../utils/search' import { - contentReaderForPageType, + contentReaderForPage, getStorageFileDetails, makeStorageFilePublic, } from '../../utils/uploads' @@ -963,7 +963,7 @@ export const searchResolver = authorized< ...r, image: r.image && createImageProxyUrl(r.image, 260, 260), isArchived: !!r.archivedAt, - contentReader: contentReaderForPageType(r.pageType), + contentReader: contentReaderForPage(r.pageType, r.uploadFileId), originalArticleUrl: r.url, publishedAt: validatedDate(r.publishedAt), ownedByViewer: r.userId === claims.uid, @@ -1007,7 +1007,7 @@ export const typeaheadSearchResolver = authorized< const results = await searchAsYouType(claims.uid, query, first || undefined) const items: TypeaheadSearchItem[] = results.map((r) => ({ ...r, - contentReader: contentReaderForPageType(r.pageType), + contentReader: contentReaderForPage(r.pageType, r.uploadFileId), })) return { items } @@ -1072,7 +1072,7 @@ export const updatesSinceResolver = authorized< ...p, image: p.image && createImageProxyUrl(p.image, 260, 260), isArchived: !!p.archivedAt, - contentReader: contentReaderForPageType(p.pageType), + contentReader: contentReaderForPage(p.pageType, p.uploadFileId), } as SearchItem, cursor: endCursor, itemID: p.id, diff --git a/packages/api/src/resolvers/function_resolvers.ts b/packages/api/src/resolvers/function_resolvers.ts index be95f5d15..0000cf4cf 100644 --- a/packages/api/src/resolvers/function_resolvers.ts +++ b/packages/api/src/resolvers/function_resolvers.ts @@ -19,7 +19,7 @@ import { import { userDataToUser, validatedDate, wordsCount } from '../utils/helpers' import { createImageProxyUrl } from '../utils/imageproxy' import { - contentReaderForPageType, + contentReaderForPage, generateDownloadSignedUrl, generateUploadFilePathName, } from '../utils/uploads' @@ -468,8 +468,11 @@ export const functionResolvers = { }) return !!page?.archivedAt || false }, - contentReader(article: { pageType: PageType }) { - return contentReaderForPageType(article.pageType) + contentReader(article: { + pageType: PageType + uploadFileId: string | undefined + }) { + return contentReaderForPage(article.pageType, article.uploadFileId) }, highlights( article: { id: string; userId?: string; highlights?: Highlight[] }, diff --git a/packages/api/src/services/speech.ts b/packages/api/src/services/speech.ts index 34da9b489..d36fe66ab 100644 --- a/packages/api/src/services/speech.ts +++ b/packages/api/src/services/speech.ts @@ -1,6 +1,6 @@ -import { Page, PageType } from '../elastic/types' +import { Page } from '../elastic/types' import { ContentReader } from '../generated/graphql' -import { contentReaderForPageType } from '../utils/uploads' +import { contentReaderForPage } from '../utils/uploads' import { FeatureName, isOptedIn } from './features' /* @@ -11,7 +11,8 @@ export const shouldSynthesize = async ( page: Page ): Promise => { if ( - contentReaderForPageType(page.pageType) !== ContentReader.Web || + contentReaderForPage(page.pageType, page.uploadFileId) !== + ContentReader.Web || !page.content ) { // we don't synthesize files for now diff --git a/packages/api/src/utils/uploads.ts b/packages/api/src/utils/uploads.ts index 738704662..eeb1dfa0d 100644 --- a/packages/api/src/utils/uploads.ts +++ b/packages/api/src/utils/uploads.ts @@ -4,8 +4,13 @@ import { File, GetSignedUrlConfig, Storage } from '@google-cloud/storage' import { env } from '../env' import { ContentReader, PageType } from '../generated/graphql' -export const contentReaderForPageType = (pageType: PageType) => { - console.log('getting content reader: ', pageType) +export const contentReaderForPage = ( + pageType: PageType, + uploadFileId: string | null | undefined +) => { + if (!uploadFileId) { + return ContentReader.Web + } switch (pageType) { case PageType.Book: return ContentReader.Epub From 51186a195f1b3e7bf98b01131bea6e060a76dbcc Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Mon, 5 Jun 2023 14:59:27 +0800 Subject: [PATCH 2/3] Add test for content type func --- packages/api/test/utils/uploads.test.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 packages/api/test/utils/uploads.test.ts diff --git a/packages/api/test/utils/uploads.test.ts b/packages/api/test/utils/uploads.test.ts new file mode 100644 index 000000000..d3f601d20 --- /dev/null +++ b/packages/api/test/utils/uploads.test.ts @@ -0,0 +1,20 @@ +import 'mocha' +import { expect } from 'chai' +import 'chai/register-should' +import { contentReaderForPage } from '../../src/utils/uploads' +import { ContentReader, PageType } from '../../src/generated/graphql' + +describe('contentReaderForPage', () => { + it('returns web if there is no uploadFileId', () => { + const result = contentReaderForPage(PageType.Book, undefined) + expect(result).to.eq(ContentReader.Web) + }) + it('returns Epub if there is an uploadFileId and type is book', () => { + const result = contentReaderForPage(PageType.Book, 'fakeUploadFileId') + expect(result).to.eq(ContentReader.Epub) + }) + it('returns PDF if there is an uploadFileId and type is not File', () => { + const result = contentReaderForPage(PageType.File, 'fakeUploadFileId') + expect(result).to.eq(ContentReader.Pdf) + }) +}) From bab3983383445c6455ebd11a422c275ac21a7d9d Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Mon, 5 Jun 2023 16:00:50 +0800 Subject: [PATCH 3/3] Fix typo in test description --- packages/api/test/utils/uploads.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/test/utils/uploads.test.ts b/packages/api/test/utils/uploads.test.ts index d3f601d20..a953191b9 100644 --- a/packages/api/test/utils/uploads.test.ts +++ b/packages/api/test/utils/uploads.test.ts @@ -13,7 +13,7 @@ describe('contentReaderForPage', () => { const result = contentReaderForPage(PageType.Book, 'fakeUploadFileId') expect(result).to.eq(ContentReader.Epub) }) - it('returns PDF if there is an uploadFileId and type is not File', () => { + it('returns PDF if there is an uploadFileId and type is File', () => { const result = contentReaderForPage(PageType.File, 'fakeUploadFileId') expect(result).to.eq(ContentReader.Pdf) })