From 80d0146f72503c46376d3694d1878558e149a8fc Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 4 May 2022 11:55:16 +0800 Subject: [PATCH 01/16] Show unable to parse if page is failed to save within 30 seconds --- packages/api/src/resolvers/article/index.ts | 4 ++-- packages/api/src/services/create_page_save_request.ts | 1 - .../web/components/templates/homeFeed/HomeFeedContainer.tsx | 6 +++++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 02312f8b6..37d95d9ae 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -102,7 +102,7 @@ const FORCE_PUPPETEER_URLS = [ /twitter\.com\/(?:#!\/)?(\w+)\/status(?:es)?\/(\d+)(?:\/.*)?/, /^((?:https?:)?\/\/)?((?:www|m)\.)?((?:youtube\.com|youtu.be))(\/(?:[\w-]+\?v=|embed\/|v\/)?)([\w-]+)(\S+)?$/, ] -const UNPARSEABLE_CONTENT = 'We were unable to parse this page.' +const UNPARSEABLE_CONTENT = '

We were unable to parse this page.

' export type CreateArticlesSuccessPartial = Merge< CreateArticleSuccess, @@ -434,7 +434,7 @@ export const getArticleResolver: ResolverFn< if ( page.state === ArticleSavingRequestStatus.Processing && - new Date(page.createdAt).getTime() < new Date().getTime() - 1000 * 60 + new Date(page.createdAt).getTime() < new Date().getTime() - 1000 * 30 ) { page.content = UNPARSEABLE_CONTENT page.description = UNPARSEABLE_CONTENT diff --git a/packages/api/src/services/create_page_save_request.ts b/packages/api/src/services/create_page_save_request.ts index 4c0170012..b5a70f784 100644 --- a/packages/api/src/services/create_page_save_request.ts +++ b/packages/api/src/services/create_page_save_request.ts @@ -96,7 +96,6 @@ export const createPageSaveRequest = async ( const existingPage = await getPageByParam({ userId, url, - state: ArticleSavingRequestStatus.Succeeded, }) if (existingPage) { console.log('Page already exists', url) diff --git a/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx b/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx index 916f02d05..fed9a1337 100644 --- a/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx +++ b/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx @@ -259,7 +259,11 @@ export function HomeFeedContainer(props: HomeFeedContainerProps): JSX.Element { const username = viewerData?.me?.profile.username if (username) { setActiveCardId(item.node.id) - if (item.node.state === State.PROCESSING) { + if ( + item.node.state === State.PROCESSING && + new Date(item.node.createdAt).getTime() >= + new Date().getTime() - 1000 * 30 + ) { router.push(`/${username}/links/${item.node.id}`) } else { router.push(`/${username}/${item.node.slug}`) From 329efacbe5f15eb6d84d2813f97cdf5c902d56f7 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 4 May 2022 12:12:20 +0800 Subject: [PATCH 02/16] Redirect to slug page --- .../templates/homeFeed/HomeFeedContainer.tsx | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx b/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx index fed9a1337..1d7659d57 100644 --- a/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx +++ b/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx @@ -40,7 +40,6 @@ import { Label } from '../../../lib/networking/fragments/labelFragment' import { isVipUser } from '../../../lib/featureFlag' import { EmptyLibrary } from './EmptyLibrary' import TopBarProgress from 'react-topbar-progress-indicator' -import { State } from '../../../lib/networking/fragments/articleFragment' export type LayoutType = 'LIST_LAYOUT' | 'GRID_LAYOUT' @@ -259,15 +258,7 @@ export function HomeFeedContainer(props: HomeFeedContainerProps): JSX.Element { const username = viewerData?.me?.profile.username if (username) { setActiveCardId(item.node.id) - if ( - item.node.state === State.PROCESSING && - new Date(item.node.createdAt).getTime() >= - new Date().getTime() - 1000 * 30 - ) { - router.push(`/${username}/links/${item.node.id}`) - } else { - router.push(`/${username}/${item.node.slug}`) - } + router.push(`/${username}/${item.node.slug}`) } break case 'showOriginal': From 2aa0618d28d3c51c80778ede8a71e7e9071d3fa4 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 4 May 2022 12:16:21 +0800 Subject: [PATCH 03/16] Use id instead of slug --- .../web/components/templates/homeFeed/HomeFeedContainer.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx b/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx index 1d7659d57..5e9880fac 100644 --- a/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx +++ b/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx @@ -258,7 +258,7 @@ export function HomeFeedContainer(props: HomeFeedContainerProps): JSX.Element { const username = viewerData?.me?.profile.username if (username) { setActiveCardId(item.node.id) - router.push(`/${username}/${item.node.slug}`) + router.push(`/${username}/${item.node.id}`) } break case 'showOriginal': From ca0f58ee22d7ac8dd48c7253bc14c14f46696063 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 4 May 2022 12:21:41 +0800 Subject: [PATCH 04/16] Update page content and state if failed to parse --- packages/api/src/resolvers/article/index.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 37d95d9ae..844826a4c 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -408,7 +408,7 @@ export const getArticleResolver: ResolverFn< Record, WithDataSourcesContext, QueryArticleArgs -> = async (_obj, { slug }, { claims }) => { +> = async (_obj, { slug }, { claims, pubsub }) => { try { if (!claims?.uid) { return { errorCodes: [ArticleErrorCode.Unauthorized] } @@ -438,6 +438,11 @@ export const getArticleResolver: ResolverFn< ) { page.content = UNPARSEABLE_CONTENT page.description = UNPARSEABLE_CONTENT + page.state = ArticleSavingRequestStatus.Failed + await updatePage(page.id, page, { + uid: claims.uid, + pubsub, + }) } return { From 0b0100f0e59ae541252c6ac9347aed73068b6121 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 4 May 2022 12:25:10 +0800 Subject: [PATCH 05/16] Revert web change --- .../components/templates/homeFeed/HomeFeedContainer.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx b/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx index 5e9880fac..916f02d05 100644 --- a/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx +++ b/packages/web/components/templates/homeFeed/HomeFeedContainer.tsx @@ -40,6 +40,7 @@ import { Label } from '../../../lib/networking/fragments/labelFragment' import { isVipUser } from '../../../lib/featureFlag' import { EmptyLibrary } from './EmptyLibrary' import TopBarProgress from 'react-topbar-progress-indicator' +import { State } from '../../../lib/networking/fragments/articleFragment' export type LayoutType = 'LIST_LAYOUT' | 'GRID_LAYOUT' @@ -258,7 +259,11 @@ export function HomeFeedContainer(props: HomeFeedContainerProps): JSX.Element { const username = viewerData?.me?.profile.username if (username) { setActiveCardId(item.node.id) - router.push(`/${username}/${item.node.id}`) + if (item.node.state === State.PROCESSING) { + router.push(`/${username}/links/${item.node.id}`) + } else { + router.push(`/${username}/${item.node.slug}`) + } } break case 'showOriginal': From 9048ccbab3d5a01f8e6710d6b11b153b922285cc Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 4 May 2022 12:46:57 +0800 Subject: [PATCH 06/16] Show failed pages in library --- packages/api/src/elastic/pages.ts | 8 +------- packages/api/src/services/create_page_save_request.ts | 1 + 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/api/src/elastic/pages.ts b/packages/api/src/elastic/pages.ts index f0a19434a..5434dcfdf 100644 --- a/packages/api/src/elastic/pages.ts +++ b/packages/api/src/elastic/pages.ts @@ -377,13 +377,7 @@ export const searchPages = async ( }, ], should: [], - must_not: [ - { - term: { - state: ArticleSavingRequestStatus.Failed, - }, - }, - ], + must_not: [], }, }, sort: [ diff --git a/packages/api/src/services/create_page_save_request.ts b/packages/api/src/services/create_page_save_request.ts index b5a70f784..4c0170012 100644 --- a/packages/api/src/services/create_page_save_request.ts +++ b/packages/api/src/services/create_page_save_request.ts @@ -96,6 +96,7 @@ export const createPageSaveRequest = async ( const existingPage = await getPageByParam({ userId, url, + state: ArticleSavingRequestStatus.Succeeded, }) if (existingPage) { console.log('Page already exists', url) From 98ea1016d5f133a1510fe070392f9a1c48bccdf6 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 4 May 2022 12:50:22 +0800 Subject: [PATCH 07/16] Remove

tag in description --- packages/api/src/resolvers/article/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 844826a4c..84be0ce45 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -102,7 +102,7 @@ const FORCE_PUPPETEER_URLS = [ /twitter\.com\/(?:#!\/)?(\w+)\/status(?:es)?\/(\d+)(?:\/.*)?/, /^((?:https?:)?\/\/)?((?:www|m)\.)?((?:youtube\.com|youtu.be))(\/(?:[\w-]+\?v=|embed\/|v\/)?)([\w-]+)(\S+)?$/, ] -const UNPARSEABLE_CONTENT = '

We were unable to parse this page.

' +const UNPARSEABLE_CONTENT = 'We were unable to parse this page.' export type CreateArticlesSuccessPartial = Merge< CreateArticleSuccess, @@ -436,7 +436,7 @@ export const getArticleResolver: ResolverFn< page.state === ArticleSavingRequestStatus.Processing && new Date(page.createdAt).getTime() < new Date().getTime() - 1000 * 30 ) { - page.content = UNPARSEABLE_CONTENT + page.content = `

${UNPARSEABLE_CONTENT}

` page.description = UNPARSEABLE_CONTENT page.state = ArticleSavingRequestStatus.Failed await updatePage(page.id, page, { From a42bb7ab38383f826d83aa0d8177d74f938e6195 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 4 May 2022 12:59:07 +0800 Subject: [PATCH 08/16] Replace createdAt with savedAt --- packages/api/src/resolvers/article/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 84be0ce45..6a1af468d 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -434,7 +434,8 @@ export const getArticleResolver: ResolverFn< if ( page.state === ArticleSavingRequestStatus.Processing && - new Date(page.createdAt).getTime() < new Date().getTime() - 1000 * 30 + page.savedAt && + new Date(page.savedAt).getTime() < new Date().getTime() - 1000 * 30 ) { page.content = `

${UNPARSEABLE_CONTENT}

` page.description = UNPARSEABLE_CONTENT From cf65a635320110cf7d0698621841e084d096a8cd Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 4 May 2022 14:30:54 +0800 Subject: [PATCH 09/16] Replace getArticleSavingRequest with getArticle --- packages/web/pages/[username]/links/[id].tsx | 88 +++++++++++--------- 1 file changed, 49 insertions(+), 39 deletions(-) diff --git a/packages/web/pages/[username]/links/[id].tsx b/packages/web/pages/[username]/links/[id].tsx index 343ecf773..1ff941ad6 100644 --- a/packages/web/pages/[username]/links/[id].tsx +++ b/packages/web/pages/[username]/links/[id].tsx @@ -1,10 +1,9 @@ import { useRouter } from 'next/router' import { useEffect, useState } from 'react' -import { useGetArticleSavingStatus } from '../../../lib/networking/queries/useGetArticleSavingStatus' import { PrimaryLayout } from '../../../components/templates/PrimaryLayout' import { - Loader, ErrorComponent, + Loader, } from '../../../components/templates/SavingRequest' import { ArticleActionsMenu } from '../../../components/templates/article/ArticleActionsMenu' import { VStack } from '../../../components/elements/LayoutPrimitives' @@ -13,6 +12,7 @@ import { applyStoredTheme } from '../../../lib/themeUpdater' import { useReaderSettings } from '../../../lib/hooks/useReaderSettings' import { SkeletonArticleContainer } from '../../../components/templates/article/SkeletonArticleContainer' import TopBarProgress from 'react-topbar-progress-indicator' +import { useGetArticleQuery } from '../../../lib/networking/queries/useGetArticleQuery' export default function ArticleSavingRequestPage(): JSX.Element { const router = useRouter() @@ -32,7 +32,7 @@ export default function ArticleSavingRequestPage(): JSX.Element { headerToolbarControl={ - - + - - {articleId ? : } - + {articleId ? ( + + ) : ( + + )} + ) @@ -91,14 +101,16 @@ export default function ArticleSavingRequestPage(): JSX.Element { type PrimaryContentProps = { articleId: string + username: string } function PrimaryContent(props: PrimaryContentProps): JSX.Element { const router = useRouter() const [timedOut, setTimedOut] = useState(false) - const { successRedirectPath, error } = useGetArticleSavingStatus({ - id: props.articleId, + const { articleData, articleFetchError } = useGetArticleQuery({ + username: props.username, + slug: props.articleId, }) useEffect(() => { @@ -111,21 +123,19 @@ function PrimaryContent(props: PrimaryContentProps): JSX.Element { } }, []) - if (error === 'unauthorized') { + if (articleFetchError === 'Unauthorized') { router.replace('/login') } - if (timedOut || error) { + if (timedOut || articleFetchError) { return ( ) } - if (successRedirectPath) { - router.replace(successRedirectPath) + if (articleData && articleData.article.article.state === 'SUCCEEDED') { + router.replace(`/${props.username}/${articleData.article.article.slug}`) } - return ( - - ) + return } From 0bff046733c375bbe7d21516268c89a670e20655 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 4 May 2022 17:43:37 +0800 Subject: [PATCH 10/16] Revert "Replace getArticleSavingRequest with getArticle" This reverts commit cf65a635320110cf7d0698621841e084d096a8cd. --- packages/web/pages/[username]/links/[id].tsx | 88 +++++++++----------- 1 file changed, 39 insertions(+), 49 deletions(-) diff --git a/packages/web/pages/[username]/links/[id].tsx b/packages/web/pages/[username]/links/[id].tsx index 1ff941ad6..343ecf773 100644 --- a/packages/web/pages/[username]/links/[id].tsx +++ b/packages/web/pages/[username]/links/[id].tsx @@ -1,9 +1,10 @@ import { useRouter } from 'next/router' import { useEffect, useState } from 'react' +import { useGetArticleSavingStatus } from '../../../lib/networking/queries/useGetArticleSavingStatus' import { PrimaryLayout } from '../../../components/templates/PrimaryLayout' import { - ErrorComponent, Loader, + ErrorComponent, } from '../../../components/templates/SavingRequest' import { ArticleActionsMenu } from '../../../components/templates/article/ArticleActionsMenu' import { VStack } from '../../../components/elements/LayoutPrimitives' @@ -12,7 +13,6 @@ import { applyStoredTheme } from '../../../lib/themeUpdater' import { useReaderSettings } from '../../../lib/hooks/useReaderSettings' import { SkeletonArticleContainer } from '../../../components/templates/article/SkeletonArticleContainer' import TopBarProgress from 'react-topbar-progress-indicator' -import { useGetArticleQuery } from '../../../lib/networking/queries/useGetArticleQuery' export default function ArticleSavingRequestPage(): JSX.Element { const router = useRouter() @@ -32,7 +32,7 @@ export default function ArticleSavingRequestPage(): JSX.Element { headerToolbarControl={ - - - - {articleId ? ( - - ) : ( - - )} - + + {articleId ? : } + ) @@ -101,16 +91,14 @@ export default function ArticleSavingRequestPage(): JSX.Element { type PrimaryContentProps = { articleId: string - username: string } function PrimaryContent(props: PrimaryContentProps): JSX.Element { const router = useRouter() const [timedOut, setTimedOut] = useState(false) - const { articleData, articleFetchError } = useGetArticleQuery({ - username: props.username, - slug: props.articleId, + const { successRedirectPath, error } = useGetArticleSavingStatus({ + id: props.articleId, }) useEffect(() => { @@ -123,19 +111,21 @@ function PrimaryContent(props: PrimaryContentProps): JSX.Element { } }, []) - if (articleFetchError === 'Unauthorized') { + if (error === 'unauthorized') { router.replace('/login') } - if (timedOut || articleFetchError) { + if (timedOut || error) { return ( ) } - if (articleData && articleData.article.article.state === 'SUCCEEDED') { - router.replace(`/${props.username}/${articleData.article.article.slug}`) + if (successRedirectPath) { + router.replace(successRedirectPath) } - return + return ( + + ) } From afe8b6e9482f37d19cfa1467fa8c11c71d5b2a04 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 4 May 2022 17:54:59 +0800 Subject: [PATCH 11/16] make savedAt a required field in page --- packages/api/src/elastic/types.ts | 2 +- packages/api/src/resolvers/article/index.ts | 1 - packages/api/src/routers/svc/pdf_attachments.ts | 1 + packages/api/src/services/create_page_save_request.ts | 8 ++++---- packages/api/src/services/save_email.ts | 1 + packages/api/src/services/save_file.ts | 1 + packages/api/src/services/save_page.ts | 3 ++- packages/api/test/elastic/index.test.ts | 3 +++ packages/api/test/resolvers/article.test.ts | 3 ++- .../api/test/resolvers/article_saving_request.test.ts | 2 +- packages/api/test/util.ts | 1 + 11 files changed, 17 insertions(+), 9 deletions(-) diff --git a/packages/api/src/elastic/types.ts b/packages/api/src/elastic/types.ts index c6909eb97..cd829d54a 100644 --- a/packages/api/src/elastic/types.ts +++ b/packages/api/src/elastic/types.ts @@ -196,7 +196,7 @@ export interface Page { createdAt: Date updatedAt?: Date publishedAt?: Date - savedAt?: Date + savedAt: Date sharedAt?: Date archivedAt?: Date | null siteName?: string diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 6a1af468d..3e97b9e62 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -434,7 +434,6 @@ export const getArticleResolver: ResolverFn< if ( page.state === ArticleSavingRequestStatus.Processing && - page.savedAt && new Date(page.savedAt).getTime() < new Date().getTime() - 1000 * 30 ) { page.content = `

${UNPARSEABLE_CONTENT}

` diff --git a/packages/api/src/routers/svc/pdf_attachments.ts b/packages/api/src/routers/svc/pdf_attachments.ts index 4fcba0176..c408d821b 100644 --- a/packages/api/src/routers/svc/pdf_attachments.ts +++ b/packages/api/src/routers/svc/pdf_attachments.ts @@ -155,6 +155,7 @@ export function pdfAttachmentsRouter() { slug: generateSlug(title), id: '', createdAt: new Date(), + savedAt: new Date(), readingProgressPercent: 0, readingProgressAnchorIndex: 0, state: ArticleSavingRequestStatus.Succeeded, diff --git a/packages/api/src/services/create_page_save_request.ts b/packages/api/src/services/create_page_save_request.ts index 4c0170012..295e955a3 100644 --- a/packages/api/src/services/create_page_save_request.ts +++ b/packages/api/src/services/create_page_save_request.ts @@ -14,7 +14,7 @@ import { ArticleSavingRequestStatus, Page, PageType } from '../elastic/types' import { createPubSubClient, PubsubClient } from '../datalayer/pubsub' import normalizeUrl from 'normalize-url' -const SAVING_DESCRIPTION = 'Your link is being saved...' +const SAVING_CONTENT = 'Your link is being saved...' const isPrivateIP = privateIpLib.default @@ -107,8 +107,7 @@ export const createPageSaveRequest = async ( const page: Page = { id: articleSavingRequestId, userId, - content: SAVING_DESCRIPTION, - createdAt: new Date(), + content: SAVING_CONTENT, hash: '', pageType: PageType.Unknown, readingProgressAnchorIndex: 0, @@ -118,7 +117,8 @@ export const createPageSaveRequest = async ( url, taskName: createdTaskName, state: ArticleSavingRequestStatus.Processing, - description: SAVING_DESCRIPTION, + createdAt: new Date(), + savedAt: new Date(), } const pageId = await createPage(page, { pubsub, uid: userId }) diff --git a/packages/api/src/services/save_email.ts b/packages/api/src/services/save_email.ts index 9088fa0e8..e62175c47 100644 --- a/packages/api/src/services/save_email.ts +++ b/packages/api/src/services/save_email.ts @@ -66,6 +66,7 @@ export const saveEmail = async ( publishedAt: validatedDate(parseResult.parsedContent?.publishedDate), slug: slug, createdAt: new Date(), + savedAt: new Date(), readingProgressAnchorIndex: 0, readingProgressPercent: 0, subscription: input.author, diff --git a/packages/api/src/services/save_file.ts b/packages/api/src/services/save_file.ts index b8cf39307..874ca8e1d 100644 --- a/packages/api/src/services/save_file.ts +++ b/packages/api/src/services/save_file.ts @@ -94,6 +94,7 @@ export const saveFile = async ( userId: saver.id, id: input.clientRequestId, createdAt: new Date(), + savedAt: new Date(), readingProgressPercent: 0, readingProgressAnchorIndex: 0, state: ArticleSavingRequestStatus.Succeeded, diff --git a/packages/api/src/services/save_page.ts b/packages/api/src/services/save_page.ts index f54cc351e..e6fd79110 100644 --- a/packages/api/src/services/save_page.ts +++ b/packages/api/src/services/save_page.ts @@ -91,10 +91,11 @@ export const savePage = async ( hash: stringToHash(parseResult.parsedContent?.content || input.url), image: parseResult.parsedContent?.previewImage, publishedAt: validatedDate(parseResult.parsedContent?.publishedDate), - createdAt: new Date(), readingProgressPercent: 0, readingProgressAnchorIndex: 0, state: ArticleSavingRequestStatus.Succeeded, + createdAt: new Date(), + savedAt: new Date(), } const existingPage = await getPageByParam({ diff --git a/packages/api/test/elastic/index.test.ts b/packages/api/test/elastic/index.test.ts index 7815db81b..c313ad556 100644 --- a/packages/api/test/elastic/index.test.ts +++ b/packages/api/test/elastic/index.test.ts @@ -46,6 +46,7 @@ describe('elastic api', () => { slug: 'test slug', createdAt: new Date(), updatedAt: new Date(), + savedAt: new Date(), readingProgressPercent: 100, readingProgressAnchorIndex: 0, url: 'https://blog.omnivore.app/p/getting-started-with-omnivore', @@ -98,6 +99,7 @@ describe('elastic api', () => { slug: 'test', createdAt: new Date(), updatedAt: new Date(), + savedAt: new Date(), readingProgressPercent: 0, readingProgressAnchorIndex: 0, url: 'https://blog.omnivore.app/testUrl', @@ -202,6 +204,7 @@ describe('elastic api', () => { content: 'test', slug: 'test', createdAt: new Date(createdAt), + savedAt: new Date(), readingProgressPercent: 0, readingProgressAnchorIndex: 0, url: 'https://blog.omnivore.app/testCount', diff --git a/packages/api/test/resolvers/article.test.ts b/packages/api/test/resolvers/article.test.ts index c6b4e10fe..be28848cd 100644 --- a/packages/api/test/resolvers/article.test.ts +++ b/packages/api/test/resolvers/article.test.ts @@ -434,7 +434,7 @@ describe('Article API', () => { pageId, { state: ArticleSavingRequestStatus.Processing, - createdAt: new Date(Date.now() - 1000 * 60), + savedAt: new Date(Date.now() - 1000 * 60), }, ctx ) @@ -731,6 +731,7 @@ describe('Article API', () => { title: 'test title', content: '

test

', createdAt: new Date(), + savedAt: new Date(), url: 'https://blog.omnivore.app/setBookmarkArticle', slug: 'test-with-omnivore', readingProgressPercent: 0, diff --git a/packages/api/test/resolvers/article_saving_request.test.ts b/packages/api/test/resolvers/article_saving_request.test.ts index 43cb5cfa8..cc8aabc1e 100644 --- a/packages/api/test/resolvers/article_saving_request.test.ts +++ b/packages/api/test/resolvers/article_saving_request.test.ts @@ -96,7 +96,7 @@ describe('ArticleSavingRequest API', () => { const page = await getPageById( res.body.data.createArticleSavingRequest.articleSavingRequest.id ) - expect(page?.description).to.eq('Your link is being saved...') + expect(page?.content).to.eq('Your link is being saved...') }) it('returns an error if the url is invalid', async () => { diff --git a/packages/api/test/util.ts b/packages/api/test/util.ts index d235a2c66..8f1bab161 100644 --- a/packages/api/test/util.ts +++ b/packages/api/test/util.ts @@ -51,6 +51,7 @@ export const createTestElasticPage = async ( title: 'test title', content: '

test content

', createdAt: new Date(), + savedAt: new Date(), url: 'https://example.com/test-url', slug: 'test-with-omnivore', labels: labels, From 9e3db0e053bf5b786286fadaf29c95b01d987235 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 4 May 2022 19:12:22 +0800 Subject: [PATCH 12/16] Fix a bug to have multiple pages with the same url in lib --- packages/api/src/resolvers/article/index.ts | 94 ++++++++----------- .../src/services/create_page_save_request.ts | 71 +++++++------- 2 files changed, 72 insertions(+), 93 deletions(-) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 3e97b9e62..55e548115 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -246,8 +246,8 @@ export const createArticleResolver = authorized< const saveTime = new Date() const slug = generateSlug(parsedContent?.title || croppedPathname) - let articleToSave: Page = { - id: '', + const articleToSave: Page = { + id: pageId || '', userId: uid, originalHtml: domContent, content: parsedContent?.content || '', @@ -317,63 +317,47 @@ export const createArticleResolver = authorized< ) } - const existingPage = await getPageByParam({ - userId: uid, - url: articleToSave.url, - state: ArticleSavingRequestStatus.Succeeded, - }) - if (existingPage) { - // update existing page in elastic - existingPage.slug = slug - existingPage.savedAt = saveTime - existingPage.archivedAt = archive ? saveTime : undefined - existingPage.url = uploadFileUrlOverride || articleToSave.url - existingPage.hash = articleToSave.hash - - await updatePage(existingPage.id, existingPage, { ...ctx, uid }) - - log.info('page updated in elastic', existingPage.id) - articleToSave = existingPage - } else { - // create new page in elastic - if (!pageId) { - pageId = await createPage(articleToSave, { ...ctx, uid }) - if (!pageId) { - return pageError( - { - errorCodes: [CreateArticleErrorCode.ElasticError], - }, - ctx, - pageId - ) - } - } else { - const updated = await updatePage(pageId, articleToSave, { - ...ctx, - uid, - }) - - if (!updated) { - return pageError( - { - errorCodes: [CreateArticleErrorCode.ElasticError], - }, - ctx, - pageId - ) - } + // create new page in elastic + if (!pageId) { + const newPageId = await createPage(articleToSave, { ...ctx, uid }) + if (!newPageId) { + return pageError( + { + errorCodes: [CreateArticleErrorCode.ElasticError], + }, + ctx, + pageId + ) } + articleToSave.id = newPageId + } else { + // update existing page's state from processing to succeeded + articleToSave.archivedAt = archive ? saveTime : undefined + articleToSave.url = uploadFileUrlOverride || articleToSave.url + const updated = await updatePage(pageId, articleToSave, { + ...ctx, + uid, + }) - log.info( - 'page created in elastic', - pageId, - articleToSave.url, - articleToSave.slug, - articleToSave.title - ) - articleToSave.id = pageId + if (!updated) { + return pageError( + { + errorCodes: [CreateArticleErrorCode.ElasticError], + }, + ctx, + pageId + ) + } } + log.info( + 'page created in elastic', + articleToSave.id, + articleToSave.url, + articleToSave.slug, + articleToSave.title + ) + const createdArticle: PartialArticle = { ...articleToSave, isArchived: !!articleToSave.archivedAt, diff --git a/packages/api/src/services/create_page_save_request.ts b/packages/api/src/services/create_page_save_request.ts index 295e955a3..ca8c0c357 100644 --- a/packages/api/src/services/create_page_save_request.ts +++ b/packages/api/src/services/create_page_save_request.ts @@ -10,7 +10,7 @@ import { import { generateSlug, pageToArticleSavingRequest } from '../utils/helpers' import * as privateIpLib from 'private-ip' import { countByCreatedAt, createPage, getPageByParam } from '../elastic/pages' -import { ArticleSavingRequestStatus, Page, PageType } from '../elastic/types' +import { ArticleSavingRequestStatus, PageType } from '../elastic/types' import { createPubSubClient, PubsubClient } from '../datalayer/pubsub' import normalizeUrl from 'normalize-url' @@ -82,52 +82,47 @@ export const createPageSaveRequest = async ( // get priority by checking rate limit if not specified priority = priority || (await getPriorityByRateLimit(userId)) + // look for existing page url = normalizeUrl(url, { stripHash: true, stripWWW: false, }) - const createdTaskName = await enqueueParseRequest( - url, - userId, - articleSavingRequestId, - priority - ) - - const existingPage = await getPageByParam({ + let page = await getPageByParam({ userId, url, - state: ArticleSavingRequestStatus.Succeeded, }) - if (existingPage) { - console.log('Page already exists', url) - existingPage.taskName = createdTaskName - return pageToArticleSavingRequest(user, existingPage) + if (page) { + console.log('Page already exists', page) + articleSavingRequestId = page.id + } else { + page = { + id: articleSavingRequestId, + userId, + content: SAVING_CONTENT, + hash: '', + pageType: PageType.Unknown, + readingProgressAnchorIndex: 0, + readingProgressPercent: 0, + slug: generateSlug(url), + title: url, + url, + state: ArticleSavingRequestStatus.Processing, + createdAt: new Date(), + savedAt: new Date(), + } + + // create processing page + const pageId = await createPage(page, { pubsub, uid: userId }) + if (!pageId) { + console.log('Failed to create page', page) + return Promise.reject({ + errorCode: CreateArticleSavingRequestErrorCode.BadData, + }) + } } - const page: Page = { - id: articleSavingRequestId, - userId, - content: SAVING_CONTENT, - hash: '', - pageType: PageType.Unknown, - readingProgressAnchorIndex: 0, - readingProgressPercent: 0, - slug: generateSlug(url), - title: url, - url, - taskName: createdTaskName, - state: ArticleSavingRequestStatus.Processing, - createdAt: new Date(), - savedAt: new Date(), - } - - const pageId = await createPage(page, { pubsub, uid: userId }) - if (!pageId) { - console.log('Failed to create page', page) - return Promise.reject({ - errorCode: CreateArticleSavingRequestErrorCode.BadData, - }) - } + // enqueue task to parse page + await enqueueParseRequest(url, userId, articleSavingRequestId, priority) return pageToArticleSavingRequest(user, page) } From bc1ed3f054170eb5320038a25433a4531fde47a5 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 4 May 2022 19:33:13 +0800 Subject: [PATCH 13/16] Check state in articleSavingRequest --- packages/api/src/resolvers/article/index.ts | 16 ++++------------ .../resolvers/article_saving_request/index.ts | 13 +++++++++++-- packages/api/src/utils/helpers.ts | 8 ++++++++ 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 55e548115..aa1a0e5ca 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -46,6 +46,7 @@ import { ContentParseError } from '../../utils/errors' import { authorized, generateSlug, + isParsingTimeout, pageError, stringToHash, userDataToUser, @@ -102,7 +103,7 @@ const FORCE_PUPPETEER_URLS = [ /twitter\.com\/(?:#!\/)?(\w+)\/status(?:es)?\/(\d+)(?:\/.*)?/, /^((?:https?:)?\/\/)?((?:www|m)\.)?((?:youtube\.com|youtu.be))(\/(?:[\w-]+\?v=|embed\/|v\/)?)([\w-]+)(\S+)?$/, ] -const UNPARSEABLE_CONTENT = 'We were unable to parse this page.' +const UNPARSEABLE_CONTENT = '

We were unable to parse this page.

' export type CreateArticlesSuccessPartial = Merge< CreateArticleSuccess, @@ -416,17 +417,8 @@ export const getArticleResolver: ResolverFn< return { errorCodes: [ArticleErrorCode.NotFound] } } - if ( - page.state === ArticleSavingRequestStatus.Processing && - new Date(page.savedAt).getTime() < new Date().getTime() - 1000 * 30 - ) { - page.content = `

${UNPARSEABLE_CONTENT}

` - page.description = UNPARSEABLE_CONTENT - page.state = ArticleSavingRequestStatus.Failed - await updatePage(page.id, page, { - uid: claims.uid, - pubsub, - }) + if (isParsingTimeout(page)) { + page.content = UNPARSEABLE_CONTENT } return { diff --git a/packages/api/src/resolvers/article_saving_request/index.ts b/packages/api/src/resolvers/article_saving_request/index.ts index d4a542fb5..1b8f2764e 100644 --- a/packages/api/src/resolvers/article_saving_request/index.ts +++ b/packages/api/src/resolvers/article_saving_request/index.ts @@ -2,6 +2,7 @@ import { ArticleSavingRequestError, ArticleSavingRequestErrorCode, + ArticleSavingRequestStatus, ArticleSavingRequestSuccess, CreateArticleSavingRequestError, CreateArticleSavingRequestErrorCode, @@ -9,7 +10,11 @@ import { MutationCreateArticleSavingRequestArgs, QueryArticleSavingRequestArgs, } from '../../generated/graphql' -import { authorized, pageToArticleSavingRequest } from '../../utils/helpers' +import { + authorized, + isParsingTimeout, + pageToArticleSavingRequest, +} from '../../utils/helpers' import { createPageSaveRequest } from '../../services/create_page_save_request' import { getPageById } from '../../elastic/pages' import { isErrorWithCode } from '../user' @@ -62,8 +67,12 @@ export const articleSavingRequestResolver = authorized< user = await models.user.get(page.userId) // eslint-disable-next-line no-empty } catch (error) {} - if (user && page) + if (user && page) { + if (isParsingTimeout(page)) { + page.state = ArticleSavingRequestStatus.Succeeded + } return { articleSavingRequest: pageToArticleSavingRequest(user, page) } + } return { errorCodes: [ArticleSavingRequestErrorCode.NotFound] } }) diff --git a/packages/api/src/utils/helpers.ts b/packages/api/src/utils/helpers.ts index f5499fd56..73c0abdc9 100644 --- a/packages/api/src/utils/helpers.ts +++ b/packages/api/src/utils/helpers.ts @@ -202,6 +202,14 @@ export const pageToArticleSavingRequest = ( updatedAt: page.updatedAt || new Date(), }) +export const isParsingTimeout = (page: Page): boolean => { + return ( + // page processed more than 30 seconds ago + page.state === ArticleSavingRequestStatus.Processing && + new Date(page.savedAt).getTime() < new Date().getTime() - 1000 * 30 + ) +} + export const validatedDate = ( date: Date | string | undefined ): Date | undefined => { From 93ab6bc87af36e44231e0cef481818b2d0aadf21 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 4 May 2022 21:28:06 +0800 Subject: [PATCH 14/16] Add includePending=true in search API --- packages/api/src/resolvers/article/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index aa1a0e5ca..b7865a5c6 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -855,6 +855,7 @@ export const searchResolver = authorized< savedDateFilter: searchQuery.savedDateFilter, publishedDateFilter: searchQuery.publishedDateFilter, subscriptionFilter: searchQuery.subscriptionFilter, + includePending: true, }, claims.uid )) || [[], 0] From 21ff9a5ae78e6a6d6ecf485e9dad2c5364a2b637 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 4 May 2022 21:42:59 +0800 Subject: [PATCH 15/16] Add test for state=failed or processing --- packages/api/test/resolvers/article.test.ts | 32 +++++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/packages/api/test/resolvers/article.test.ts b/packages/api/test/resolvers/article.test.ts index be28848cd..9fdc33baf 100644 --- a/packages/api/test/resolvers/article.test.ts +++ b/packages/api/test/resolvers/article.test.ts @@ -444,7 +444,7 @@ describe('Article API', () => { const res = await graphqlRequest(query, authToken).expect(200) expect(res.body.data.article.article.content).to.eql( - 'We were unable to parse this page.' + '

We were unable to parse this page.

' ) }) }) @@ -474,7 +474,7 @@ describe('Article API', () => { before(async () => { // Create some test pages for (let i = 0; i < 15; i++) { - const page = { + const page: Page = { id: '', hash: 'test hash', userId: user.id, @@ -488,6 +488,7 @@ describe('Article API', () => { readingProgressAnchorIndex: 0, url: url, savedAt: new Date(), + state: ArticleSavingRequestStatus.Succeeded, } as Page const pageId = await createPage(page, ctx) if (!pageId) { @@ -583,7 +584,7 @@ describe('Article API', () => { ) expect( res.body.data.articles.pageInfo.startCursor, - 'startCursor' + 'st artCursor' ).to.eql('5') expect(res.body.data.articles.pageInfo.endCursor, 'endCursor').to.eql( '10' @@ -596,6 +597,31 @@ describe('Article API', () => { // expect(res.body.data.articles.pageInfo.hasPreviousPage).to.eql(true) }) }) + + context('when there are pages with failed state', () => { + before(async () => { + for (let i = 0; i < 5; i++) { + await updatePage( + pages[i].id, + { + state: ArticleSavingRequestStatus.Failed, + }, + ctx + ) + } + after = '10' + }) + it('should include state=failed pages', async () => { + const res = await graphqlRequest(query, authToken).expect(200) + + expect(res.body.data.articles.edges.length).to.eql(5) + expect(res.body.data.articles.edges[0].node.id).to.eql(pages[4].id) + expect(res.body.data.articles.edges[1].node.id).to.eql(pages[3].id) + expect(res.body.data.articles.edges[2].node.id).to.eql(pages[2].id) + expect(res.body.data.articles.edges[3].node.id).to.eql(pages[1].id) + expect(res.body.data.articles.edges[4].node.id).to.eql(pages[0].id) + }) + }) }) describe('SavePage', () => { From 8f60a9f905474e8d991e815b37204b00a368a402 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 4 May 2022 21:54:28 +0800 Subject: [PATCH 16/16] Add default state to migration script --- packages/db/migrate.ts | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/packages/db/migrate.ts b/packages/db/migrate.ts index 04659182b..9d5b2bfd6 100755 --- a/packages/db/migrate.ts +++ b/packages/db/migrate.ts @@ -87,7 +87,7 @@ const logAppliedMigrations = ( } export const INDEX_ALIAS = 'pages_alias' -export const client = new Client({ +export const esClient = new Client({ node: process.env.ELASTIC_URL || 'http://localhost:9200', auth: { username: process.env.ELASTIC_USERNAME || '', @@ -103,7 +103,7 @@ const updateMappings = async (): Promise => { ) // update mappings - await client.indices.putMapping({ + await esClient.indices.putMapping({ index: INDEX_ALIAS, body: JSON.parse(indexSettings).mappings, }) @@ -123,8 +123,39 @@ postgrator log('Starting updating elasticsearch index mappings...') updateMappings() - .then(() => console.log('\nUpdating elastic completed.')) + .then(() => console.log('\nUpdating elastic mappings completed.')) .catch((error) => { log(`${chalk.red('Updating failed: ')}${error.message}`, chalk.red) process.exit(1) }) + +log('Starting adding default state to pages in elasticsearch...') +esClient + .update_by_query({ + index: INDEX_ALIAS, + body: { + script: { + source: 'ctx._source.state = params.state', + lang: 'painless', + params: { + state: 'SUCCEEDED', + }, + }, + query: { + bool: { + must_not: [ + { + exists: { + field: 'state', + }, + }, + ], + }, + }, + }, + }) + .then(() => console.log('\nAdding default state completed.')) + .catch((error) => { + log(`${chalk.red('Adding failed: ')}${error.message}`, chalk.red) + process.exit(1) + })