From bba0f69f625daec55f71ec64de7b354d39af273d Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 10 Mar 2023 12:38:25 +0800 Subject: [PATCH] Add top percentage to the reading position in API --- packages/api/src/elastic/pages.ts | 4 +- packages/api/src/elastic/types.ts | 2 + packages/api/src/generated/graphql.ts | 5 ++ packages/api/src/generated/schema.graphql | 3 + packages/api/src/resolvers/article/index.ts | 52 +++++++---- packages/api/src/schema.ts | 3 + packages/api/test/resolvers/article.test.ts | 86 +++++++++++-------- .../db/elastic_migrations/index_settings.json | 5 +- 8 files changed, 101 insertions(+), 59 deletions(-) diff --git a/packages/api/src/elastic/pages.ts b/packages/api/src/elastic/pages.ts index d6307007d..49d1bda5e 100644 --- a/packages/api/src/elastic/pages.ts +++ b/packages/api/src/elastic/pages.ts @@ -256,7 +256,7 @@ export const updatePage = async ( ctx: PageContext ): Promise => { try { - const { body } = await client.update({ + await client.update({ index: INDEX_ALIAS, id, body: { @@ -269,8 +269,6 @@ export const updatePage = async ( retry_on_conflict: 3, }) - if (body.result !== 'updated') return false - if (page.state === ArticleSavingRequestStatus.Deleted) { await ctx.pubsub.entityDeleted(EntityType.PAGE, id, ctx.uid) return true diff --git a/packages/api/src/elastic/types.ts b/packages/api/src/elastic/types.ts index 39cdadba2..1ec528e54 100644 --- a/packages/api/src/elastic/types.ts +++ b/packages/api/src/elastic/types.ts @@ -242,6 +242,7 @@ export interface Page { originalHtml?: string | null slug: string labels?: Label[] + readingProgressTopPercent?: number readingProgressPercent: number readingProgressAnchorIndex: number createdAt: Date @@ -283,6 +284,7 @@ export interface SearchItem { uploadFileId?: string | null url: string archivedAt?: Date | null + readingProgressTopPercent?: number readingProgressPercent: number readingProgressAnchorIndex: number userId: string diff --git a/packages/api/src/generated/graphql.ts b/packages/api/src/generated/graphql.ts index 44c16e204..d3bfcc675 100644 --- a/packages/api/src/generated/graphql.ts +++ b/packages/api/src/generated/graphql.ts @@ -110,6 +110,7 @@ export type Article = { readAt?: Maybe; readingProgressAnchorIndex: Scalars['Int']; readingProgressPercent: Scalars['Float']; + readingProgressTopPercent?: Maybe; recommendations?: Maybe>; savedAt: Scalars['Date']; savedByViewer?: Maybe; @@ -2125,6 +2126,7 @@ export type SaveArticleReadingProgressInput = { id: Scalars['ID']; readingProgressAnchorIndex: Scalars['Int']; readingProgressPercent: Scalars['Float']; + readingProgressTopPercent?: InputMaybe; }; export type SaveArticleReadingProgressResult = SaveArticleReadingProgressError | SaveArticleReadingProgressSuccess; @@ -2233,6 +2235,7 @@ export type SearchItem = { readAt?: Maybe; readingProgressAnchorIndex: Scalars['Int']; readingProgressPercent: Scalars['Float']; + readingProgressTopPercent?: Maybe; recommendations?: Maybe>; savedAt: Scalars['Date']; shortId?: Maybe; @@ -4142,6 +4145,7 @@ export type ArticleResolvers, ParentType, ContextType>; readingProgressAnchorIndex?: Resolver; readingProgressPercent?: Resolver; + readingProgressTopPercent?: Resolver, ParentType, ContextType>; recommendations?: Resolver>, ParentType, ContextType>; savedAt?: Resolver; savedByViewer?: Resolver, ParentType, ContextType>; @@ -5365,6 +5369,7 @@ export type SearchItemResolvers, ParentType, ContextType>; readingProgressAnchorIndex?: Resolver; readingProgressPercent?: Resolver; + readingProgressTopPercent?: Resolver, ParentType, ContextType>; recommendations?: Resolver>, ParentType, ContextType>; savedAt?: Resolver; shortId?: Resolver, ParentType, ContextType>; diff --git a/packages/api/src/generated/schema.graphql b/packages/api/src/generated/schema.graphql index 7c7b36037..96d6b560c 100644 --- a/packages/api/src/generated/schema.graphql +++ b/packages/api/src/generated/schema.graphql @@ -86,6 +86,7 @@ type Article { readAt: Date readingProgressAnchorIndex: Int! readingProgressPercent: Float! + readingProgressTopPercent: Float recommendations: [Recommendation!] savedAt: Date! savedByViewer: Boolean @@ -1532,6 +1533,7 @@ input SaveArticleReadingProgressInput { id: ID! readingProgressAnchorIndex: Int! readingProgressPercent: Float! + readingProgressTopPercent: Float } union SaveArticleReadingProgressResult = SaveArticleReadingProgressError | SaveArticleReadingProgressSuccess @@ -1633,6 +1635,7 @@ type SearchItem { readAt: Date readingProgressAnchorIndex: Int! readingProgressPercent: Float! + readingProgressTopPercent: Float recommendations: [Recommendation!] savedAt: Date! shortId: String diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 40d1ca256..726a88399 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -764,7 +764,14 @@ export const saveArticleReadingProgressResolver = authorized< >( async ( _, - { input: { id, readingProgressPercent, readingProgressAnchorIndex } }, + { + input: { + id, + readingProgressPercent, + readingProgressAnchorIndex, + readingProgressTopPercent, + }, + }, { claims: { uid }, pubsub } ) => { const page = await getPageByParam({ userId: uid, _id: id }) @@ -774,31 +781,42 @@ export const saveArticleReadingProgressResolver = authorized< } if ( - (!readingProgressPercent && readingProgressPercent !== 0) || readingProgressPercent < 0 || - readingProgressPercent > 100 + readingProgressPercent > 100 || + (readingProgressTopPercent && + (readingProgressTopPercent < 0 || + readingProgressTopPercent > readingProgressPercent)) ) { return { errorCodes: [SaveArticleReadingProgressErrorCode.BadData] } } - + // If we have a top percent, we only save it if it's greater than the current top percent + // or set to zero if the top percent is zero. + const readingProgressTopPercentToSave = readingProgressTopPercent + ? readingProgressTopPercent === 0 + ? 0 + : Math.max( + readingProgressTopPercent, + page.readingProgressTopPercent || 0 + ) + : undefined // If setting to zero we accept the update, otherwise we require it // be greater than the current reading progress. - const shouldUpdate = - readingProgressPercent === 0 || - page.readingProgressPercent < readingProgressPercent || - page.readingProgressAnchorIndex < readingProgressAnchorIndex - const updatedPart = { - readingProgressPercent: shouldUpdate - ? readingProgressPercent - : page.readingProgressPercent, - readingProgressAnchorIndex: shouldUpdate - ? readingProgressAnchorIndex - : page.readingProgressAnchorIndex, + readingProgressPercent: + readingProgressPercent === 0 + ? 0 + : Math.max(readingProgressPercent, page.readingProgressPercent), + readingProgressAnchorIndex: Math.max( + readingProgressAnchorIndex, + page.readingProgressAnchorIndex + ), + readingProgressTopPercent: readingProgressTopPercentToSave, readAt: new Date(), } - - await updatePage(id, updatedPart, { pubsub, uid }) + const updated = await updatePage(id, updatedPart, { pubsub, uid }) + if (!updated) { + return { errorCodes: [SaveArticleReadingProgressErrorCode.NotFound] } + } return { updatedArticle: { diff --git a/packages/api/src/schema.ts b/packages/api/src/schema.ts index 1a1efa40d..1c43b4e22 100755 --- a/packages/api/src/schema.ts +++ b/packages/api/src/schema.ts @@ -363,6 +363,7 @@ const schema = gql` savedAt: Date! updatedAt: Date! publishedAt: Date + readingProgressTopPercent: Float readingProgressPercent: Float! readingProgressAnchorIndex: Int! sharedComment: String @@ -609,6 +610,7 @@ const schema = gql` | SaveArticleReadingProgressError input SaveArticleReadingProgressInput { id: ID! + readingProgressTopPercent: Float readingProgressPercent: Float! readingProgressAnchorIndex: Int! } @@ -1526,6 +1528,7 @@ const schema = gql` createdAt: Date! updatedAt: Date isArchived: Boolean! + readingProgressTopPercent: Float readingProgressPercent: Float! readingProgressAnchorIndex: Int! author: String diff --git a/packages/api/test/resolvers/article.test.ts b/packages/api/test/resolvers/article.test.ts index a7c3caff2..feab7963e 100644 --- a/packages/api/test/resolvers/article.test.ts +++ b/packages/api/test/resolvers/article.test.ts @@ -298,7 +298,8 @@ const setBookmarkQuery = (articleId: string, bookmark: boolean) => { const saveArticleReadingProgressQuery = ( articleId: string, - progress: number + progress: number, + topPercent: number | null = null ) => { return ` mutation { @@ -307,6 +308,7 @@ const saveArticleReadingProgressQuery = ( id: "${articleId}", readingProgressPercent: ${progress} readingProgressAnchorIndex: 0 + readingProgressTopPercent: ${topPercent} } ) { ... on SaveArticleReadingProgressSuccess { @@ -314,6 +316,7 @@ const saveArticleReadingProgressQuery = ( id readingProgressPercent readAt + readingProgressTopPercent } } ... on SaveArticleReadingProgressError { @@ -695,9 +698,9 @@ describe('Article API', () => { describe('saveArticleReadingProgressResolver', () => { let query = '' - let articleId = '' - let progress = 0.5 let pageId = '' + let progress = 0.5 + let topPercent: number | null = null before(async () => { pageId = (await createTestElasticPage(user.id)).id! @@ -707,46 +710,53 @@ describe('Article API', () => { await deletePage(pageId, ctx) }) - beforeEach(() => { - query = saveArticleReadingProgressQuery(articleId, progress) + it('saves a reading progress on an article', async () => { + query = saveArticleReadingProgressQuery(pageId, progress, topPercent) + const res = await graphqlRequest(query, authToken).expect(200) + expect( + res.body.data.saveArticleReadingProgress.updatedArticle + .readingProgressPercent + ).to.eq(progress) + expect(res.body.data.saveArticleReadingProgress.updatedArticle.readAt).not + .null }) - context('when we save a reading progress on an article', () => { - before(async () => { - articleId = pageId - progress = 0.5 - }) + it('should not allow setting the reading progress lower than current progress', async () => { + const firstQuery = saveArticleReadingProgressQuery(pageId, 75) + const firstRes = await graphqlRequest(firstQuery, authToken).expect(200) + expect( + firstRes.body.data.saveArticleReadingProgress.updatedArticle + .readingProgressPercent + ).to.eq(75) + await refreshIndex() - it('should save a reading progress on an article', async () => { - const res = await graphqlRequest(query, authToken).expect(200) - expect( - res.body.data.saveArticleReadingProgress.updatedArticle - .readingProgressPercent - ).to.eq(progress) - expect(res.body.data.saveArticleReadingProgress.updatedArticle.readAt) - .not.null - }) + // Now try to set to a lower value (50), value should not be updated + // refresh index to ensure the reading progress is updated + const secondQuery = saveArticleReadingProgressQuery(pageId, 50) + const secondRes = await graphqlRequest(secondQuery, authToken).expect(200) + expect( + secondRes.body.data.saveArticleReadingProgress.updatedArticle + .readingProgressPercent + ).to.eq(75) + }) - it('should not allow setting the reading progress lower than current progress', async () => { - const firstQuery = saveArticleReadingProgressQuery(articleId, 75) - const firstRes = await graphqlRequest(firstQuery, authToken).expect(200) - expect( - firstRes.body.data.saveArticleReadingProgress.updatedArticle - .readingProgressPercent - ).to.eq(75) - await refreshIndex() + it('does not save topPercent if not undefined', async () => { + query = saveArticleReadingProgressQuery(pageId, progress, null) + const res = await graphqlRequest(query, authToken).expect(200) + expect( + res.body.data.saveArticleReadingProgress.updatedArticle + .readingProgressTopPercent + ).to.be.null + }) - // Now try to set to a lower value (50), value should not be updated - // refresh index to ensure the reading progress is updated - const secondQuery = saveArticleReadingProgressQuery(articleId, 50) - const secondRes = await graphqlRequest(secondQuery, authToken).expect( - 200 - ) - expect( - secondRes.body.data.saveArticleReadingProgress.updatedArticle - .readingProgressPercent - ).to.eq(75) - }) + it('saves topPercent if defined', async () => { + const topPercent = 0.2 + query = saveArticleReadingProgressQuery(pageId, progress, topPercent) + const res = await graphqlRequest(query, authToken).expect(200) + expect( + res.body.data.saveArticleReadingProgress.updatedArticle + .readingProgressTopPercent + ).to.eql(topPercent) }) }) diff --git a/packages/db/elastic_migrations/index_settings.json b/packages/db/elastic_migrations/index_settings.json index cebf4c58b..8671385d5 100644 --- a/packages/db/elastic_migrations/index_settings.json +++ b/packages/db/elastic_migrations/index_settings.json @@ -126,6 +126,9 @@ } } }, + "readingProgressTopPercent": { + "type": "float" + }, "readingProgressPercent": { "type": "float" }, @@ -206,4 +209,4 @@ } } } -} \ No newline at end of file +}