From 0013150c26b2e41c1e40d5aa48baf417ae193cd3 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 5 Jun 2024 20:44:18 +0800 Subject: [PATCH 1/8] feat: highlights api --- packages/api/src/apollo.ts | 4 + packages/api/src/entity/highlight.ts | 3 + packages/api/src/generated/graphql.ts | 66 ++++++++++ packages/api/src/generated/schema.graphql | 21 ++++ .../api/src/resolvers/function_resolvers.ts | 47 ++++--- packages/api/src/resolvers/highlight/index.ts | 119 +++++++++--------- packages/api/src/resolvers/types.ts | 2 + packages/api/src/schema.ts | 21 ++++ packages/api/src/services/highlights.ts | 19 ++- packages/api/src/services/user.ts | 2 +- packages/api/test/routers/user.test.ts | 4 +- 11 files changed, 224 insertions(+), 84 deletions(-) diff --git a/packages/api/src/apollo.ts b/packages/api/src/apollo.ts index 9c0ce1271..b06bcd304 100644 --- a/packages/api/src/apollo.ts +++ b/packages/api/src/apollo.ts @@ -42,6 +42,7 @@ import { } from './services/service_usage' import { batchGetSubscriptionsByNames } from './services/subscriptions' import { batchGetUploadFilesByIds } from './services/upload_file' +import { findUsersByIds } from './services/user' import { tracer } from './tracing' import { getClaimsByToken, setAuthInCookie } from './utils/auth' import { SetClaimsRole } from './utils/dictionary' @@ -124,6 +125,9 @@ const contextFunc: ContextFunction = async ({ return batchGetSubscriptionsByNames(claims.uid, names as string[]) }), + users: new DataLoader(async (ids: readonly string[]) => + findUsersByIds(ids as string[]) + ), }, } diff --git a/packages/api/src/entity/highlight.ts b/packages/api/src/entity/highlight.ts index 60b8eed3a..cdbe26b65 100644 --- a/packages/api/src/entity/highlight.ts +++ b/packages/api/src/entity/highlight.ts @@ -36,6 +36,9 @@ export class Highlight { @JoinColumn({ name: 'user_id' }) user!: User + @Column('uuid') + userId!: string + @ManyToOne(() => LibraryItem, { onDelete: 'CASCADE' }) @JoinColumn({ name: 'library_item_id' }) libraryItem!: LibraryItem diff --git a/packages/api/src/generated/graphql.ts b/packages/api/src/generated/graphql.ts index 3ed794529..27cdcf62b 100644 --- a/packages/api/src/generated/graphql.ts +++ b/packages/api/src/generated/graphql.ts @@ -1275,6 +1275,12 @@ export type Highlight = { user: User; }; +export type HighlightEdge = { + __typename?: 'HighlightEdge'; + cursor: Scalars['String']; + node: Highlight; +}; + export type HighlightReply = { __typename?: 'HighlightReply'; createdAt: Scalars['Date']; @@ -1296,6 +1302,23 @@ export enum HighlightType { Redaction = 'REDACTION' } +export type HighlightsError = { + __typename?: 'HighlightsError'; + errorCodes: Array; +}; + +export enum HighlightsErrorCode { + BadRequest = 'BAD_REQUEST' +} + +export type HighlightsResult = HighlightsError | HighlightsSuccess; + +export type HighlightsSuccess = { + __typename?: 'HighlightsSuccess'; + edges: Array; + pageInfo: PageInfo; +}; + export type HomeEdge = { __typename?: 'HomeEdge'; cursor: Scalars['String']; @@ -2261,6 +2284,7 @@ export type Query = { groups: GroupsResult; hello?: Maybe; hiddenHomeSection: HiddenHomeSectionResult; + highlights: HighlightsResult; home: HomeResult; integration: IntegrationResult; integrations: IntegrationsResult; @@ -2311,6 +2335,13 @@ export type QueryGetDiscoverFeedArticlesArgs = { }; +export type QueryHighlightsArgs = { + after?: InputMaybe; + first?: InputMaybe; + query?: InputMaybe; +}; + + export type QueryHomeArgs = { after?: InputMaybe; first?: InputMaybe; @@ -4331,9 +4362,14 @@ export type ResolversTypes = { HiddenHomeSectionResult: ResolversTypes['HiddenHomeSectionError'] | ResolversTypes['HiddenHomeSectionSuccess']; HiddenHomeSectionSuccess: ResolverTypeWrapper; Highlight: ResolverTypeWrapper; + HighlightEdge: ResolverTypeWrapper; HighlightReply: ResolverTypeWrapper; HighlightStats: ResolverTypeWrapper; HighlightType: HighlightType; + HighlightsError: ResolverTypeWrapper; + HighlightsErrorCode: HighlightsErrorCode; + HighlightsResult: ResolversTypes['HighlightsError'] | ResolversTypes['HighlightsSuccess']; + HighlightsSuccess: ResolverTypeWrapper; HomeEdge: ResolverTypeWrapper; HomeError: ResolverTypeWrapper; HomeErrorCode: HomeErrorCode; @@ -4900,8 +4936,12 @@ export type ResolversParentTypes = { HiddenHomeSectionResult: ResolversParentTypes['HiddenHomeSectionError'] | ResolversParentTypes['HiddenHomeSectionSuccess']; HiddenHomeSectionSuccess: HiddenHomeSectionSuccess; Highlight: Highlight; + HighlightEdge: HighlightEdge; HighlightReply: HighlightReply; HighlightStats: HighlightStats; + HighlightsError: HighlightsError; + HighlightsResult: ResolversParentTypes['HighlightsError'] | ResolversParentTypes['HighlightsSuccess']; + HighlightsSuccess: HighlightsSuccess; HomeEdge: HomeEdge; HomeError: HomeError; HomeItem: HomeItem; @@ -6103,6 +6143,12 @@ export type HighlightResolvers; }; +export type HighlightEdgeResolvers = { + cursor?: Resolver; + node?: Resolver; + __isTypeOf?: IsTypeOfResolverFn; +}; + export type HighlightReplyResolvers = { createdAt?: Resolver; highlight?: Resolver; @@ -6118,6 +6164,21 @@ export type HighlightStatsResolvers; }; +export type HighlightsErrorResolvers = { + errorCodes?: Resolver, ParentType, ContextType>; + __isTypeOf?: IsTypeOfResolverFn; +}; + +export type HighlightsResultResolvers = { + __resolveType: TypeResolveFn<'HighlightsError' | 'HighlightsSuccess', ParentType, ContextType>; +}; + +export type HighlightsSuccessResolvers = { + edges?: Resolver, ParentType, ContextType>; + pageInfo?: Resolver; + __isTypeOf?: IsTypeOfResolverFn; +}; + export type HomeEdgeResolvers = { cursor?: Resolver; node?: Resolver; @@ -6581,6 +6642,7 @@ export type QueryResolvers; hello?: Resolver, ParentType, ContextType>; hiddenHomeSection?: Resolver; + highlights?: Resolver>; home?: Resolver>; integration?: Resolver>; integrations?: Resolver; @@ -7797,8 +7859,12 @@ export type Resolvers = { HiddenHomeSectionResult?: HiddenHomeSectionResultResolvers; HiddenHomeSectionSuccess?: HiddenHomeSectionSuccessResolvers; Highlight?: HighlightResolvers; + HighlightEdge?: HighlightEdgeResolvers; HighlightReply?: HighlightReplyResolvers; HighlightStats?: HighlightStatsResolvers; + HighlightsError?: HighlightsErrorResolvers; + HighlightsResult?: HighlightsResultResolvers; + HighlightsSuccess?: HighlightsSuccessResolvers; HomeEdge?: HomeEdgeResolvers; HomeError?: HomeErrorResolvers; HomeItem?: HomeItemResolvers; diff --git a/packages/api/src/generated/schema.graphql b/packages/api/src/generated/schema.graphql index bb2185cbf..d0e46a4b3 100644 --- a/packages/api/src/generated/schema.graphql +++ b/packages/api/src/generated/schema.graphql @@ -1147,6 +1147,11 @@ type Highlight { user: User! } +type HighlightEdge { + cursor: String! + node: Highlight! +} + type HighlightReply { createdAt: Date! highlight: Highlight! @@ -1166,6 +1171,21 @@ enum HighlightType { REDACTION } +type HighlightsError { + errorCodes: [HighlightsErrorCode!]! +} + +enum HighlightsErrorCode { + BAD_REQUEST +} + +union HighlightsResult = HighlightsError | HighlightsSuccess + +type HighlightsSuccess { + edges: [HighlightEdge!]! + pageInfo: PageInfo! +} + type HomeEdge { cursor: String! node: HomeSection! @@ -1738,6 +1758,7 @@ type Query { groups: GroupsResult! hello: String hiddenHomeSection: HiddenHomeSectionResult! + highlights(after: String, first: Int, query: String): HighlightsResult! home(after: String, first: Int): HomeResult! integration(name: String!): IntegrationResult! integrations: IntegrationsResult! diff --git a/packages/api/src/resolvers/function_resolvers.ts b/packages/api/src/resolvers/function_resolvers.ts index ba5079831..1a7d0ba40 100644 --- a/packages/api/src/resolvers/function_resolvers.ts +++ b/packages/api/src/resolvers/function_resolvers.ts @@ -5,7 +5,7 @@ /* eslint-disable @typescript-eslint/explicit-module-boundary-types */ import { createHmac } from 'crypto' import { isError } from 'lodash' -import { Highlight as HighlightEntity } from '../entity/highlight' +import { Highlight } from '../entity/highlight' import { LibraryItem } from '../entity/library_item' import { EXISTING_NEWSLETTER_FOLDER, @@ -16,10 +16,10 @@ import { DEFAULT_SUBSCRIPTION_FOLDER, Subscription, } from '../entity/subscription' +import { User as UserEntity } from '../entity/user' import { env } from '../env' import { Article, - Highlight, HomeItem, HomeItemSource, HomeItemSourceType, @@ -33,7 +33,6 @@ import { getAISummary } from '../services/ai-summaries' import { findUserFeatures } from '../services/features' import { Merge } from '../util' import { - highlightDataToHighlight, isBase64Image, recommandationDataToRecommendation, validatedDate, @@ -60,6 +59,7 @@ import { saveDiscoverArticleResolver, } from './discover_feeds' import { optInFeatureResolver } from './features' +import { highlightsResolver } from './highlight' import { hiddenHomeSectionResolver, homeResolver, @@ -376,6 +376,7 @@ export const functionResolvers = { home: homeResolver, subscription: subscriptionResolver, hiddenHomeSection: hiddenHomeSectionResolver, + highlights: highlightsResolver, }, User: { async intercomHash( @@ -414,6 +415,16 @@ export const functionResolvers = { return findUserFeatures(ctx.claims.uid) }, + picture: (user: UserEntity) => user.profile.pictureUrl, + // not implemented yet + friendsCount: () => 0, + followersCount: () => 0, + isFullUser: () => true, + viewerIsFollowing: () => false, + sharedArticles: () => [], + sharedArticlesCount: () => 0, + sharedHighlightsCount: () => 0, + sharedNotesCount: () => 0, }, Article: { async url(article: Article, _: unknown, ctx: WithDataSourcesContext) { @@ -465,16 +476,12 @@ export const functionResolvers = { ...readingProgressHandlers, }, Highlight: { - // async reactions( - // highlight: { id: string; reactions?: Reaction[] }, - // _: unknown, - // ctx: WithDataSourcesContext - // ) { - // const { reactions, id } = highlight - // if (reactions) return reactions - - // return await ctx.models.reaction.batchGetFromHighlight(id) - // }, + reactions: () => [], + replies: () => [], + type: (highlight: Highlight) => highlight.highlightType, + async user(highlight: Highlight, __: unknown, ctx: WithDataSourcesContext) { + return ctx.dataLoaders.users.load(highlight.userId) + }, createdByMe( highlight: { user: { id: string } }, __: unknown, @@ -483,15 +490,6 @@ export const functionResolvers = { return highlight.user.id === ctx.uid }, }, - // Reaction: { - // async user( - // reaction: { userId: string }, - // __: unknown, - // ctx: WithDataSourcesContext - // ) { - // return userDataToUser(await ctx.models.user.get(reaction.userId)) - // }, - // }, SearchItem: { async url(item: SearchItem, _: unknown, ctx: WithDataSourcesContext) { if ( @@ -570,7 +568,7 @@ export const functionResolvers = { if (item.highlights) return item.highlights const highlights = await ctx.dataLoaders.highlights.load(item.id) - return highlights.map(highlightDataToHighlight) + return highlights }, ...readingProgressHandlers, async content( @@ -585,7 +583,7 @@ export const functionResolvers = { ) { // convert html to the requested format if requested if (item.format && item.format !== ArticleFormat.Html && item.content) { - let highlights: HighlightEntity[] = [] + let highlights: Highlight[] = [] // load highlights if needed if ( item.format === ArticleFormat.HighlightedMarkdown && @@ -886,4 +884,5 @@ export const functionResolvers = { ...resultResolveTypeResolver('Subscription'), ...resultResolveTypeResolver('RefreshHome'), ...resultResolveTypeResolver('HiddenHomeSection'), + ...resultResolveTypeResolver('Highlights'), } diff --git a/packages/api/src/resolvers/highlight/index.ts b/packages/api/src/resolvers/highlight/index.ts index a142b3299..9d6d34359 100644 --- a/packages/api/src/resolvers/highlight/index.ts +++ b/packages/api/src/resolvers/highlight/index.ts @@ -3,7 +3,7 @@ /* eslint-disable @typescript-eslint/no-floating-promises */ import { DeepPartial } from 'typeorm' import { - Highlight as HighlightData, + Highlight as HighlightEntity, HighlightType, RepresentationType, } from '../../entity/highlight' @@ -16,6 +16,10 @@ import { DeleteHighlightError, DeleteHighlightErrorCode, DeleteHighlightSuccess, + HighlightEdge, + HighlightsError, + HighlightsErrorCode, + HighlightsSuccess, MergeHighlightError, MergeHighlightErrorCode, MergeHighlightSuccess, @@ -23,6 +27,7 @@ import { MutationDeleteHighlightArgs, MutationMergeHighlightArgs, MutationUpdateHighlightArgs, + QueryHighlightsArgs, UpdateHighlightError, UpdateHighlightErrorCode, UpdateHighlightSuccess, @@ -32,14 +37,15 @@ import { createHighlight, deleteHighlightById, mergeHighlights, + searchHighlights, updateHighlight, } from '../../services/highlights' +import { Merge } from '../../util' import { analytics } from '../../utils/analytics' import { authorized } from '../../utils/gql-utils' -import { highlightDataToHighlight } from '../../utils/helpers' export const createHighlightResolver = authorized< - CreateHighlightSuccess, + Merge, CreateHighlightError, MutationCreateHighlightArgs >(async (_, { input }, { log, pubsub, uid }) => { @@ -68,7 +74,7 @@ export const createHighlightResolver = authorized< }, }) - return { highlight: highlightDataToHighlight(newHighlight) } + return { highlight: newHighlight } } catch (err) { log.error('Error creating highlight', err) return { @@ -78,7 +84,7 @@ export const createHighlightResolver = authorized< }) export const mergeHighlightResolver = authorized< - MergeHighlightSuccess, + Merge, MergeHighlightError, MutationMergeHighlightArgs >(async (_, { input }, { authTrx, log, pubsub, uid }) => { @@ -123,7 +129,7 @@ export const mergeHighlightResolver = authorized< const color = newHighlightInput.color || mergedColors[mergedColors.length - 1] - const highlight: DeepPartial = { + const highlight: DeepPartial = { ...newHighlightInput, annotation: mergedAnnotations.length > 0 ? mergedAnnotations.join('\n') : null, @@ -154,7 +160,7 @@ export const mergeHighlightResolver = authorized< }) return { - highlight: highlightDataToHighlight(newHighlight), + highlight: newHighlight, overlapHighlightIdList: input.overlapHighlightIdList, } } catch (e) { @@ -166,7 +172,7 @@ export const mergeHighlightResolver = authorized< }) export const updateHighlightResolver = authorized< - UpdateHighlightSuccess, + Merge, UpdateHighlightError, MutationUpdateHighlightArgs >(async (_, { input }, { pubsub, uid, log }) => { @@ -183,7 +189,7 @@ export const updateHighlightResolver = authorized< pubsub ) - return { highlight: highlightDataToHighlight(updatedHighlight) } + return { highlight: updatedHighlight } } catch (error) { log.error('updateHighlightResolver error', error) return { @@ -193,7 +199,7 @@ export const updateHighlightResolver = authorized< }) export const deleteHighlightResolver = authorized< - DeleteHighlightSuccess, + Merge, DeleteHighlightError, MutationDeleteHighlightArgs >(async (_, { highlightId }, { log }) => { @@ -206,7 +212,7 @@ export const deleteHighlightResolver = authorized< } } - return { highlight: highlightDataToHighlight(deletedHighlight) } + return { highlight: deletedHighlight } } catch (error) { log.error('deleteHighlightResolver error', error) return { @@ -215,53 +221,54 @@ export const deleteHighlightResolver = authorized< } }) -// export const setShareHighlightResolver = authorized< -// SetShareHighlightSuccess, -// SetShareHighlightError, -// MutationSetShareHighlightArgs -// >(async (_, { input: { id, share } }, { pubsub, claims, log }) => { -// const highlight = await getHighlightById(id) +type PartialHighlightEdge = Merge< + HighlightEdge, + { + node: HighlightEntity + } +> +type PartialHighlightsSuccess = Merge< + HighlightsSuccess, + { + edges: PartialHighlightEdge[] + } +> +export const highlightsResolver = authorized< + PartialHighlightsSuccess, + HighlightsError, + QueryHighlightsArgs +>(async (_, { after, first }, { uid, log }) => { + const limit = first || 10 + const offset = parseInt(after || '0') + if (isNaN(offset) || offset < 0) { + log.error('Invalid after', { after }) -// if (!highlight?.id) { -// return { -// errorCodes: [SetShareHighlightErrorCode.NotFound], -// } -// } + return { + errorCodes: [HighlightsErrorCode.BadRequest], + } + } -// if (highlight.userId !== claims.uid) { -// return { -// errorCodes: [SetShareHighlightErrorCode.Forbidden], -// } -// } + const highlights = await searchHighlights(uid, limit + 1, offset) -// const sharedAt = share ? new Date() : null + const start = offset + const hasNextPage = highlights.length > limit + if (hasNextPage) { + highlights.pop() + } + const endCursor = String(start + highlights.length) -// log.info(`${share ? 'S' : 'Uns'}haring a highlight`, { -// highlight, -// labels: { -// source: 'resolver', -// resolver: 'setShareHighlightResolver', -// userId: highlight.userId, -// }, -// }) + const edges = highlights.map((highlight) => ({ + cursor: endCursor, + node: highlight, + })) -// const updatedHighlight: HighlightData = { -// ...highlight, -// sharedAt, -// updatedAt: new Date(), -// } - -// const updated = await updateHighlight(updatedHighlight, { -// pubsub, -// uid: claims.uid, -// refresh: true, -// }) - -// if (!updated) { -// return { -// errorCodes: [SetShareHighlightErrorCode.NotFound], -// } -// } - -// return { highlight: highlightDataToHighlight(updatedHighlight) } -// }) + return { + edges, + pageInfo: { + startCursor: String(start), + endCursor, + hasPreviousPage: start > 0, + hasNextPage, + }, + } +}) diff --git a/packages/api/src/resolvers/types.ts b/packages/api/src/resolvers/types.ts index 7aaf46322..cc833e556 100644 --- a/packages/api/src/resolvers/types.ts +++ b/packages/api/src/resolvers/types.ts @@ -13,6 +13,7 @@ import { PublicItem } from '../entity/public_item' import { Recommendation } from '../entity/recommendation' import { Subscription } from '../entity/subscription' import { UploadFile } from '../entity/upload_file' +import { User } from '../entity/user' import { HomeItem } from '../generated/graphql' import { PubsubClient } from '../pubsub' @@ -58,6 +59,7 @@ export interface RequestContext { libraryItems: DataLoader publicItems: DataLoader subscriptions: DataLoader + users: DataLoader } } diff --git a/packages/api/src/schema.ts b/packages/api/src/schema.ts index 14cdeddcd..e7d48d5a4 100755 --- a/packages/api/src/schema.ts +++ b/packages/api/src/schema.ts @@ -3226,6 +3226,26 @@ const schema = gql` PENDING } + union HighlightsResult = HighlightsSuccess | HighlightsError + + type HighlightsSuccess { + edges: [HighlightEdge!]! + pageInfo: PageInfo! + } + + type HighlightEdge { + cursor: String! + node: Highlight! + } + + type HighlightsError { + errorCodes: [HighlightsErrorCode!]! + } + + enum HighlightsErrorCode { + BAD_REQUEST + } + # Mutations type Mutation { googleLogin(input: GoogleLoginInput!): LoginResult! @@ -3425,6 +3445,7 @@ const schema = gql` home(first: Int, after: String): HomeResult! subscription(id: ID!): SubscriptionResult! hiddenHomeSection: HiddenHomeSectionResult! + highlights(after: String, first: Int, query: String): HighlightsResult! } schema { diff --git a/packages/api/src/services/highlights.ts b/packages/api/src/services/highlights.ts index dd6685362..2b2c89b71 100644 --- a/packages/api/src/services/highlights.ts +++ b/packages/api/src/services/highlights.ts @@ -1,5 +1,5 @@ import { diff_match_patch } from 'diff-match-patch' -import { DeepPartial, In } from 'typeorm' +import { DeepPartial, In, LessThan } from 'typeorm' import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity' import { EntityLabel } from '../entity/entity_label' import { Highlight } from '../entity/highlight' @@ -260,3 +260,20 @@ export const findHighlightsByLibraryItemId = async ( userId ) } + +export const searchHighlights = async ( + userId: string, + limit: number, + offset?: number +) => { + return authTrx( + async (tx) => + tx.withRepository(highlightRepository).find({ + where: { user: { id: userId } }, + take: limit, + skip: offset, + }), + undefined, + userId + ) +} diff --git a/packages/api/src/services/user.ts b/packages/api/src/services/user.ts index 3eabe002a..171b92f1d 100644 --- a/packages/api/src/services/user.ts +++ b/packages/api/src/services/user.ts @@ -60,7 +60,7 @@ export const findActiveUser = async (id: string): Promise => { return userRepository.findOneBy({ id, status: StatusType.Active }) } -export const findUsersById = async (ids: string[]): Promise => { +export const findUsersByIds = async (ids: string[]): Promise => { return userRepository.findBy({ id: In(ids) }) } diff --git a/packages/api/test/routers/user.test.ts b/packages/api/test/routers/user.test.ts index d6ca4a003..33b874efe 100644 --- a/packages/api/test/routers/user.test.ts +++ b/packages/api/test/routers/user.test.ts @@ -5,7 +5,7 @@ import { StatusType } from '../../src/entity/user' import { createUsers, deleteUsers, - findUsersById, + findUsersByIds, } from '../../src/services/user' import { request } from '../util' @@ -58,7 +58,7 @@ describe('User Service Router', () => { .send(data) .expect(200) - const deletedUsers = await findUsersById(toDeleteUserIds) + const deletedUsers = await findUsersByIds(toDeleteUserIds) expect(deletedUsers.length).to.equal(0) }) }) From c620ef38a2f1ffc50ab73c7b80e549826e72d81e Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 6 Jun 2024 12:08:15 +0800 Subject: [PATCH 2/8] clean up helpers --- packages/api/src/entity/library_item.ts | 3 + packages/api/src/resolvers/article/index.ts | 150 +++++------------ .../resolvers/article_saving_request/index.ts | 19 +-- .../api/src/resolvers/function_resolvers.ts | 132 ++++++++------- .../src/resolvers/recommendations/index.ts | 74 +++++---- packages/api/src/resolvers/update/index.ts | 8 +- packages/api/src/resolvers/user/index.ts | 46 +++--- packages/api/src/routers/article_router.ts | 19 +-- .../src/services/create_page_save_request.ts | 17 +- packages/api/src/services/groups.ts | 25 ++- packages/api/src/utils/helpers.ts | 152 +----------------- 11 files changed, 225 insertions(+), 420 deletions(-) diff --git a/packages/api/src/entity/library_item.ts b/packages/api/src/entity/library_item.ts index 8fb0edc94..ed8bc47ff 100644 --- a/packages/api/src/entity/library_item.ts +++ b/packages/api/src/entity/library_item.ts @@ -46,6 +46,9 @@ export class LibraryItem { @JoinColumn({ name: 'user_id' }) user!: User + @Column('uuid') + userId!: string + @Column('enum', { enum: LibraryItemState, default: LibraryItemState.Succeeded, diff --git a/packages/api/src/resolvers/article/index.ts b/packages/api/src/resolvers/article/index.ts index 026cb7988..99e1d29cd 100644 --- a/packages/api/src/resolvers/article/index.ts +++ b/packages/api/src/resolvers/article/index.ts @@ -5,7 +5,12 @@ /* eslint-disable @typescript-eslint/no-floating-promises */ import { Readability } from '@omnivore/readability' import graphqlFields from 'graphql-fields' -import { LibraryItem, LibraryItemState } from '../../entity/library_item' +import { + ContentReaderType, + LibraryItem, + LibraryItemState, +} from '../../entity/library_item' +import { User } from '../../entity/user' import { env } from '../../env' import { ArticleError, @@ -43,6 +48,7 @@ import { SaveArticleReadingProgressSuccess, SearchError, SearchErrorCode, + SearchItemEdge, SearchSuccess, SetBookmarkArticleError, SetBookmarkArticleErrorCode, @@ -87,6 +93,7 @@ import { setFileUploadComplete, } from '../../services/upload_file' import { traceAs } from '../../tracing' +import { Merge } from '../../util' import { analytics } from '../../utils/analytics' import { isSiteBlockedForParse } from '../../utils/blocked' import { enqueueBulkAction } from '../../utils/createTask' @@ -96,10 +103,7 @@ import { errorHandler, generateSlug, isParsingTimeout, - libraryItemToArticle, - libraryItemToSearchItem, titleForFilePath, - userDataToUser, } from '../../utils/helpers' import { getDistillerResult, @@ -126,7 +130,10 @@ const FORCE_PUPPETEER_URLS = [ const UNPARSEABLE_CONTENT = '

We were unable to parse this page.

' export const createArticleResolver = authorized< - CreateArticleSuccess, + Merge< + CreateArticleSuccess, + { user: User; createdArticle: Partial } + >, CreateArticleError, MutationCreateArticleArgs >( @@ -160,8 +167,8 @@ export const createArticleResolver = authorized< }, }) - const userData = await userRepository.findById(uid) - if (!userData) { + const user = await userRepository.findById(uid) + if (!user) { return errorHandler( { errorCodes: [CreateArticleErrorCode.Unauthorized], @@ -171,7 +178,6 @@ export const createArticleResolver = authorized< pubsub ) } - const user = userDataToUser(userData) try { if (isSiteBlockedForParse(url)) { @@ -203,25 +209,22 @@ export const createArticleResolver = authorized< let domContent = null let itemType = PageType.Unknown - const DUMMY_RESPONSE: CreateArticleSuccess = { + const DUMMY_RESPONSE = { user, created: false, createdArticle: { id: '', slug: '', createdAt: new Date(), - originalHtml: domContent, - content: '', + originalContent: domContent, + readableContent: '', description: '', title: '', - pageType: itemType, - contentReader: ContentReader.Web, + itemType, + contentReader: ContentReaderType.WEB, author: '', - url, - hash: '', - isArchived: false, - readingProgressAnchorIndex: 0, - readingProgressPercent: 0, + originalUrl: url, + textContentHash: '', highlights: [], savedAt: savedAt || new Date(), updatedAt: new Date(), @@ -257,7 +260,7 @@ export const createArticleResolver = authorized< FORCE_PUPPETEER_URLS.some((regex) => regex.test(url)) ) { await createPageSaveRequest({ - user: userData, + user: user, url, state: state || undefined, labels: inputLabels || undefined, @@ -282,7 +285,7 @@ export const createArticleResolver = authorized< // We have a URL but no document, so we try to send this to puppeteer // and return a dummy response. await createPageSaveRequest({ - user: userData, + user, url, state: state || undefined, labels: inputLabels || undefined, @@ -353,7 +356,7 @@ export const createArticleResolver = authorized< return { user, created: true, - createdArticle: libraryItemToArticle(libraryItemToReturn), + createdArticle: libraryItemToReturn, } } catch (error) { log.error('Error creating article', error) @@ -370,7 +373,7 @@ export const createArticleResolver = authorized< ) export const getArticleResolver = authorized< - ArticleSuccess, + Merge, ArticleError, QueryArticleArgs >(async (_obj, { slug, format }, { authTrx, uid, log }, info) => { @@ -439,7 +442,7 @@ export const getArticleResolver = authorized< } return { - article: libraryItemToArticle(libraryItem), + article: libraryItem, } } catch (error) { log.error(error) @@ -447,88 +450,8 @@ export const getArticleResolver = authorized< } }) -// type PaginatedPartialArticles = { -// edges: { cursor: string; node: PartialArticle }[] -// pageInfo: PageInfo -// } - -// export type SetShareArticleSuccessPartial = Merge< -// SetShareArticleSuccess, -// { -// updatedFeedArticle?: Omit< -// FeedArticle, -// | 'sharedBy' -// | 'article' -// | 'highlightsCount' -// | 'annotationsCount' -// | 'reactions' -// > -// updatedFeedArticleId?: string -// updatedArticle: PartialArticle -// } -// > - -// export const setShareArticleResolver = authorized< -// SetShareArticleSuccessPartial, -// SetShareArticleError, -// MutationSetShareArticleArgs -// >( -// async ( -// _, -// { input: { articleID, share, sharedComment, sharedWithHighlights } }, -// { models, authTrx, claims: { uid }, log } -// ) => { -// const article = await models.article.get(articleID) -// if (!article) { -// return { errorCodes: [SetShareArticleErrorCode.NotFound] } -// } - -// const sharedAt = share ? new Date() : null - -// log.info(`${share ? 'S' : 'Uns'}haring an article`, { -// article: Object.assign({}, article, { -// content: undefined, -// originalHtml: undefined, -// sharedAt, -// }), -// labels: { -// source: 'resolver', -// resolver: 'setShareArticleResolver', -// articleId: article.id, -// distinctId: uid, -// }, -// }) - -// const result = await authTrx((tx) => -// models.userArticle.updateByArticleId( -// uid, -// articleID, -// { sharedAt, sharedComment, sharedWithHighlights }, -// tx -// ) -// ) - -// if (!result) { -// return { errorCodes: [SetShareArticleErrorCode.NotFound] } -// } - -// // Make sure article.id instead of userArticle.id has passed. We use it for cache updates -// const updatedArticle = { -// ...result, -// ...article, -// postedByViewer: !!sharedAt, -// } -// const updatedFeedArticle = sharedAt ? { ...result, sharedAt } : undefined -// return { -// updatedFeedArticleId: result.id, -// updatedFeedArticle, -// updatedArticle, -// } -// } -// ) - export const setBookmarkArticleResolver = authorized< - SetBookmarkArticleSuccess, + Merge, SetBookmarkArticleError, MutationSetBookmarkArticleArgs >(async (_, { input: { articleID } }, { uid, log, pubsub }) => { @@ -556,12 +479,12 @@ export const setBookmarkArticleResolver = authorized< }) // Make sure article.id instead of userArticle.id has passed. We use it for cache updates return { - bookmarkedArticle: libraryItemToArticle(deletedLibraryItem), + bookmarkedArticle: deletedLibraryItem, } }) export const saveArticleReadingProgressResolver = authorized< - SaveArticleReadingProgressSuccess, + Merge, SaveArticleReadingProgressError, MutationSaveArticleReadingProgressArgs >( @@ -661,13 +584,15 @@ export const saveArticleReadingProgressResolver = authorized< } return { - updatedArticle: libraryItemToArticle(updatedItem), + updatedArticle: updatedItem, } } ) +export type PartialLibraryItem = Merge +type PartialSearchItemEdge = Merge export const searchResolver = authorized< - SearchSuccess, + Merge }>, SearchError, QuerySearchArgs >(async (_obj, params, { uid }) => { @@ -704,7 +629,10 @@ export const searchResolver = authorized< return { edges: libraryItems.map((item) => ({ - node: libraryItemToSearchItem(item, params.format as ArticleFormat), + node: { + ...item, + format: params.format || undefined, + }, cursor: endCursor, })), pageInfo: { @@ -738,7 +666,7 @@ export const typeaheadSearchResolver = authorized< }) export const updatesSinceResolver = authorized< - UpdatesSinceSuccess, + Merge }>, UpdatesSinceError, QueryUpdatesSinceArgs >(async (_obj, { since, first, after, sort: sortParams, folder }, { uid }) => { @@ -781,7 +709,7 @@ export const updatesSinceResolver = authorized< const edges = libraryItems.map((item) => { const updateReason = getUpdateReason(item, startDate) return { - node: libraryItemToSearchItem(item), + node: item, cursor: endCursor, itemID: item.id, updateReason, diff --git a/packages/api/src/resolvers/article_saving_request/index.ts b/packages/api/src/resolvers/article_saving_request/index.ts index 07d40aea5..3d99a8109 100644 --- a/packages/api/src/resolvers/article_saving_request/index.ts +++ b/packages/api/src/resolvers/article_saving_request/index.ts @@ -17,17 +17,17 @@ import { findLibraryItemById, findLibraryItemByUrl, } from '../../services/library_item' +import { Merge } from '../../util' import { analytics } from '../../utils/analytics' import { authorized } from '../../utils/gql-utils' -import { - cleanUrl, - isParsingTimeout, - libraryItemToArticleSavingRequest, -} from '../../utils/helpers' +import { cleanUrl, isParsingTimeout } from '../../utils/helpers' import { isErrorWithCode } from '../user' export const createArticleSavingRequestResolver = authorized< - CreateArticleSavingRequestSuccess, + Merge< + CreateArticleSavingRequestSuccess, + { articleSavingRequest: LibraryItem } + >, CreateArticleSavingRequestError, MutationCreateArticleSavingRequestArgs >(async (_, { input: { url } }, { uid, pubsub, log }) => { @@ -67,7 +67,7 @@ export const createArticleSavingRequestResolver = authorized< }) export const articleSavingRequestResolver = authorized< - ArticleSavingRequestSuccess, + Merge, ArticleSavingRequestError, QueryArticleSavingRequestArgs >(async (_, { id, url }, { uid, log }) => { @@ -109,10 +109,7 @@ export const articleSavingRequestResolver = authorized< libraryItem.state = LibraryItemState.Succeeded } return { - articleSavingRequest: libraryItemToArticleSavingRequest( - user, - libraryItem - ), + articleSavingRequest: libraryItem, } } catch (error) { log.error('articleSavingRequestResolver error', error) diff --git a/packages/api/src/resolvers/function_resolvers.ts b/packages/api/src/resolvers/function_resolvers.ts index 1a7d0ba40..a4fc71cf8 100644 --- a/packages/api/src/resolvers/function_resolvers.ts +++ b/packages/api/src/resolvers/function_resolvers.ts @@ -6,12 +6,14 @@ import { createHmac } from 'crypto' import { isError } from 'lodash' import { Highlight } from '../entity/highlight' +import { Label } from '../entity/label' import { LibraryItem } from '../entity/library_item' import { EXISTING_NEWSLETTER_FOLDER, NewsletterEmail, } from '../entity/newsletter_email' import { PublicItem } from '../entity/public_item' +import { Recommendation } from '../entity/recommendation' import { DEFAULT_SUBSCRIPTION_FOLDER, Subscription, @@ -19,25 +21,16 @@ import { import { User as UserEntity } from '../entity/user' import { env } from '../env' import { - Article, HomeItem, HomeItemSource, HomeItemSourceType, - Label, PageType, - Recommendation, - SearchItem, User, } from '../generated/graphql' import { getAISummary } from '../services/ai-summaries' import { findUserFeatures } from '../services/features' import { Merge } from '../util' -import { - isBase64Image, - recommandationDataToRecommendation, - validatedDate, - wordsCount, -} from '../utils/helpers' +import { isBase64Image, validatedDate, wordsCount } from '../utils/helpers' import { createImageProxyUrl } from '../utils/imageproxy' import { contentConverter } from '../utils/parser' import { @@ -48,6 +41,7 @@ import { ArticleFormat, emptyTrashResolver, fetchContentResolver, + PartialLibraryItem, } from './article' import { addDiscoverFeedResolver, @@ -192,7 +186,7 @@ const resultResolveTypeResolver = ( const readingProgressHandlers = { async readingProgressPercent( - article: { id: string; readingProgressPercent?: number }, + article: LibraryItem, _: unknown, ctx: WithDataSourcesContext ) { @@ -204,15 +198,15 @@ const readingProgressHandlers = { ) if (readingProgress) { return Math.max( - article.readingProgressPercent ?? 0, + article.readingProgressBottomPercent ?? 0, readingProgress.readingProgressPercent ) } } - return article.readingProgressPercent + return article.readingProgressBottomPercent }, async readingProgressAnchorIndex( - article: { id: string; readingProgressAnchorIndex?: number }, + article: LibraryItem, _: unknown, ctx: WithDataSourcesContext ) { @@ -224,15 +218,15 @@ const readingProgressHandlers = { ) if (readingProgress && readingProgress.readingProgressAnchorIndex) { return Math.max( - article.readingProgressAnchorIndex ?? 0, + article.readingProgressHighestReadAnchor ?? 0, readingProgress.readingProgressAnchorIndex ) } } - return article.readingProgressAnchorIndex + return article.readingProgressHighestReadAnchor }, async readingProgressTopPercent( - article: { id: string; readingProgressTopPercent?: number }, + article: LibraryItem, _: unknown, ctx: WithDataSourcesContext ) { @@ -427,10 +421,10 @@ export const functionResolvers = { sharedNotesCount: () => 0, }, Article: { - async url(article: Article, _: unknown, ctx: WithDataSourcesContext) { + async url(article: LibraryItem, _: unknown, ctx: WithDataSourcesContext) { if ( - (article.pageType == PageType.File || - article.pageType == PageType.Book) && + (article.itemType == PageType.File || + article.itemType == PageType.Book) && ctx.claims && article.uploadFileId ) { @@ -443,29 +437,33 @@ export const functionResolvers = { const filePath = generateUploadFilePathName(upload.id, upload.fileName) return generateDownloadSignedUrl(filePath) } - return article.url + return article.originalUrl }, - originalArticleUrl(article: { url: string }) { - return article.url + originalArticleUrl(article: LibraryItem) { + return article.originalUrl }, - hasContent(article: { - content: string | null - originalHtml: string | null - }) { - return !!article.originalHtml && !!article.content + hasContent(article: LibraryItem) { + return !!article.originalContent && !!article.readableContent }, publishedAt(article: { publishedAt: Date }) { return validatedDate(article.publishedAt) }, - image(article: { image?: string }): string | undefined { - return article.image && createImageProxyUrl(article.image, 320, 320) + image(article: LibraryItem): string | undefined { + if (article.thumbnail) { + return createImageProxyUrl(article.thumbnail, 320, 320) + } + + return undefined }, - wordsCount(article: { wordCount?: number; content?: string }) { + wordsCount(article: LibraryItem): number | undefined { if (article.wordCount) return article.wordCount - return article.content ? wordsCount(article.content) : undefined + + return article.readableContent + ? wordsCount(article.readableContent) + : undefined }, async labels( - article: { id: string; labels?: Label[] }, + article: LibraryItem, _: unknown, ctx: WithDataSourcesContext ) { @@ -473,6 +471,11 @@ export const functionResolvers = { return ctx.dataLoaders.labels.load(article.id) }, + content: (item: LibraryItem) => item.readableContent, + hash: (item: LibraryItem) => item.textContentHash || '', + isArchived: (item: LibraryItem) => !!item.archivedAt, + uploadFileId: (item: LibraryItem) => item.uploadFile?.id, + pageType: (item: LibraryItem) => item.itemType, ...readingProgressHandlers, }, Highlight: { @@ -491,9 +494,9 @@ export const functionResolvers = { }, }, SearchItem: { - async url(item: SearchItem, _: unknown, ctx: WithDataSourcesContext) { + async url(item: LibraryItem, _: unknown, ctx: WithDataSourcesContext) { if ( - (item.pageType == PageType.File || item.pageType == PageType.Book) && + (item.itemType == PageType.File || item.itemType == PageType.Book) && ctx.claims && item.uploadFileId ) { @@ -504,19 +507,19 @@ export const functionResolvers = { const filePath = generateUploadFilePathName(upload.id, upload.fileName) return generateDownloadSignedUrl(filePath) } - return item.url + return item.originalUrl }, - image(item: SearchItem) { - return item.image && createImageProxyUrl(item.image, 320, 320) + image(item: LibraryItem) { + return item.thumbnail && createImageProxyUrl(item.thumbnail, 320, 320) }, - originalArticleUrl(item: { url: string }) { - return item.url + originalArticleUrl(item: LibraryItem) { + return item.originalUrl }, - wordsCount(item: { wordCount?: number; content?: string }) { + wordsCount(item: LibraryItem) { if (item.wordCount) return item.wordCount - return item.content ? wordsCount(item.content) : undefined + return item.readableContent ? wordsCount(item.readableContent) : undefined }, - siteIcon(item: { siteIcon?: string }) { + siteIcon(item: LibraryItem) { if (item.siteIcon && !isBase64Image(item.siteIcon)) { return createImageProxyUrl(item.siteIcon, 128, 128) } @@ -546,9 +549,13 @@ export const functionResolvers = { const recommendations = await ctx.dataLoaders.recommendations.load( item.id ) - return recommendations.map(recommandationDataToRecommendation) + return recommendations }, - async aiSummary(item: SearchItem, _: unknown, ctx: WithDataSourcesContext) { + async aiSummary( + item: LibraryItem, + _: unknown, + ctx: WithDataSourcesContext + ) { return ( await getAISummary({ userId: ctx.uid, @@ -572,17 +579,16 @@ export const functionResolvers = { }, ...readingProgressHandlers, async content( - item: { - id: string - content?: string - highlightAnnotations?: string[] - format?: ArticleFormat - }, + item: PartialLibraryItem, _: unknown, ctx: WithDataSourcesContext ) { // convert html to the requested format if requested - if (item.format && item.format !== ArticleFormat.Html && item.content) { + if ( + item.format && + item.format !== ArticleFormat.Html && + item.readableContent + ) { let highlights: Highlight[] = [] // load highlights if needed if ( @@ -598,15 +604,17 @@ export const functionResolvers = { // convert html to the requested format const converter = contentConverter(item.format) if (converter) { - return converter(item.content, highlights) + return converter(item.readableContent, highlights) } } catch (error) { ctx.log.error('Error converting content', error) } } - return item.content + return item.readableContent }, + isArchived: (item: LibraryItem) => !!item.archivedAt, + pageType: (item: LibraryItem) => item.itemType, }, Subscription: { newsletterEmail(subscription: Subscription) { @@ -781,6 +789,22 @@ export const functionResolvers = { } }, }, + ArticleSavingRequest: { + status: (item: LibraryItem) => item.state, + url: (item: LibraryItem) => item.originalUrl, + }, + Recommendation: { + user: (recommendation: Recommendation) => { + return { + userId: recommendation.recommender.id, + username: recommendation.recommender.profile.username, + profileImageURL: recommendation.recommender.profile.pictureUrl, + name: recommendation.recommender.name, + } + }, + name: (recommendation: Recommendation) => recommendation.group.name, + recommendedAt: (recommendation: Recommendation) => recommendation.createdAt, + }, ...resultResolveTypeResolver('Login'), ...resultResolveTypeResolver('LogOut'), ...resultResolveTypeResolver('GoogleSignup'), diff --git a/packages/api/src/resolvers/recommendations/index.ts b/packages/api/src/resolvers/recommendations/index.ts index ef8b1b430..d1bd121eb 100644 --- a/packages/api/src/resolvers/recommendations/index.ts +++ b/packages/api/src/resolvers/recommendations/index.ts @@ -1,5 +1,6 @@ import { In } from 'typeorm' import { Group } from '../../entity/groups/group' +import { User } from '../../entity/user' import { env } from '../../env' import { CreateGroupError, @@ -19,6 +20,7 @@ import { MutationLeaveGroupArgs, MutationRecommendArgs, MutationRecommendHighlightsArgs, + RecommendationGroup, RecommendError, RecommendErrorCode, RecommendHighlightsError, @@ -38,26 +40,30 @@ import { leaveGroup, } from '../../services/groups' import { findLibraryItemById } from '../../services/library_item' +import { Merge } from '../../util' import { analytics } from '../../utils/analytics' import { enqueueRecommendation } from '../../utils/createTask' import { authorized } from '../../utils/gql-utils' -import { userDataToUser } from '../../utils/helpers' +export type PartialRecommendationGroup = Merge< + RecommendationGroup, + { admins: Array; members: Array } +> export const createGroupResolver = authorized< - CreateGroupSuccess, + Merge, CreateGroupError, MutationCreateGroupArgs >(async (_, { input }, { uid, log }) => { try { - const userData = await userRepository.findById(uid) - if (!userData) { + const user = await userRepository.findById(uid) + if (!user) { return { errorCodes: [CreateGroupErrorCode.Unauthorized], } } const [group, invite] = await createGroup({ - admin: userData, + admin: user, name: input.name, maxMembers: input.maxMembers, expiresInDays: input.expiresInDays, @@ -80,7 +86,6 @@ export const createGroupResolver = authorized< await createLabelAndRuleForGroup(uid, group.name) const inviteUrl = getInviteUrl(invite) - const user = userDataToUser(userData) return { group: { @@ -103,37 +108,38 @@ export const createGroupResolver = authorized< } }) -export const groupsResolver = authorized( - async (_, __, { uid, log }) => { - try { - const user = await userRepository.findById(uid) - if (!user) { - return { - errorCodes: [GroupsErrorCode.Unauthorized], - } - } - - const groups = await getRecommendationGroups(user) - +export const groupsResolver = authorized< + Merge }>, + GroupsError +>(async (_, __, { uid, log }) => { + try { + const user = await userRepository.findById(uid) + if (!user) { return { - groups, - } - } catch (error) { - log.error('Error getting groups', { - error, - labels: { - source: 'resolver', - resolver: 'groupsResolver', - uid, - }, - }) - - return { - errorCodes: [GroupsErrorCode.BadRequest], + errorCodes: [GroupsErrorCode.Unauthorized], } } + + const groups = await getRecommendationGroups(user) + + return { + groups, + } + } catch (error) { + log.error('Error getting groups', { + error, + labels: { + source: 'resolver', + resolver: 'groupsResolver', + uid, + }, + }) + + return { + errorCodes: [GroupsErrorCode.BadRequest], + } } -) +}) export const recommendResolver = authorized< RecommendSuccess, @@ -206,7 +212,7 @@ export const recommendResolver = authorized< }) export const joinGroupResolver = authorized< - JoinGroupSuccess, + Merge, JoinGroupError, MutationJoinGroupArgs >(async (_, { inviteCode }, { uid, log }) => { diff --git a/packages/api/src/resolvers/update/index.ts b/packages/api/src/resolvers/update/index.ts index c38019d8b..d17830505 100644 --- a/packages/api/src/resolvers/update/index.ts +++ b/packages/api/src/resolvers/update/index.ts @@ -1,15 +1,15 @@ -import { LibraryItemState } from '../../entity/library_item' +import { LibraryItem, LibraryItemState } from '../../entity/library_item' import { MutationUpdatePageArgs, UpdatePageError, UpdatePageSuccess, } from '../../generated/graphql' import { updateLibraryItem } from '../../services/library_item' -import { libraryItemToArticle } from '../../utils/helpers' +import { Merge } from '../../util' import { authorized } from '../../utils/gql-utils' export const updatePageResolver = authorized< - UpdatePageSuccess, + Merge, UpdatePageError, MutationUpdatePageArgs >(async (_, { input }, { uid }) => { @@ -29,6 +29,6 @@ export const updatePageResolver = authorized< uid ) return { - updatedPage: libraryItemToArticle(updatedPage), + updatedPage: updatedPage, } }) diff --git a/packages/api/src/resolvers/user/index.ts b/packages/api/src/resolvers/user/index.ts index 5f794aff0..6572c1f10 100644 --- a/packages/api/src/resolvers/user/index.ts +++ b/packages/api/src/resolvers/user/index.ts @@ -33,7 +33,6 @@ import { UpdateUserProfileErrorCode, UpdateUserProfileSuccess, UpdateUserSuccess, - User, UserErrorCode, UserResult, UsersError, @@ -43,13 +42,13 @@ import { userRepository } from '../../repository/user' import { createUser } from '../../services/create_user' import { sendAccountChangeEmail } from '../../services/send_emails' import { softDeleteUser } from '../../services/user' -import { userDataToUser } from '../../utils/helpers' +import { Merge } from '../../util' +import { authorized } from '../../utils/gql-utils' import { validateUsername } from '../../utils/usernamePolicy' import { WithDataSourcesContext } from '../types' -import { authorized } from '../../utils/gql-utils' export const updateUserResolver = authorized< - UpdateUserSuccess, + Merge, UpdateUserError, MutationUpdateUserArgs >(async (_, { input: { name, bio } }, { uid, authTrx }) => { @@ -83,11 +82,11 @@ export const updateUserResolver = authorized< }) ) - return { user: userDataToUser(updatedUser) } + return { user: updatedUser } }) export const updateUserProfileResolver = authorized< - UpdateUserProfileSuccess, + Merge, UpdateUserProfileError, MutationUpdateUserProfileArgs >(async (_, { input: { userId, username, pictureUrl } }, { uid, authTrx }) => { @@ -140,11 +139,11 @@ export const updateUserProfileResolver = authorized< }) ) - return { user: userDataToUser(updatedUser) } + return { user: updatedUser } }) export const googleLoginResolver: ResolverFn< - LoginResult, + Merge, unknown, WithDataSourcesContext, MutationGoogleLoginArgs @@ -167,7 +166,7 @@ export const googleLoginResolver: ResolverFn< // set auth cookie in response header await setAuth({ uid: user.id }) - return { me: userDataToUser(user) } + return { me: user } } export const validateUsernameResolver: ResolverFn< @@ -190,7 +189,7 @@ export const validateUsernameResolver: ResolverFn< } export const googleSignupResolver: ResolverFn< - GoogleSignupResult, + Merge, Record, WithDataSourcesContext, MutationGoogleSignupArgs @@ -205,7 +204,7 @@ export const googleSignupResolver: ResolverFn< } try { - const [user, profile] = await createUser({ + const [user] = await createUser({ email, sourceUserId, provider: 'GOOGLE', @@ -218,7 +217,7 @@ export const googleSignupResolver: ResolverFn< await setAuth({ uid: user.id }) return { - me: userDataToUser({ ...user, profile: { ...profile, private: false } }), + me: user, } } catch (err) { log.info('error signing up with google', err) @@ -245,7 +244,7 @@ export const logOutResolver: ResolverFn< } export const getMeUserResolver: ResolverFn< - User | undefined, + UserEntity | undefined, unknown, WithDataSourcesContext, unknown @@ -260,14 +259,14 @@ export const getMeUserResolver: ResolverFn< return undefined } - return userDataToUser(user) + return user } catch (error) { return undefined } } export const getUserResolver: ResolverFn< - UserResult, + Merge, unknown, WithDataSourcesContext, QueryUserArgs @@ -294,16 +293,17 @@ export const getUserResolver: ResolverFn< return { errorCodes: [UserErrorCode.UserNotFound] } } - return { user: userDataToUser(userRecord) } + return { user: userRecord } } -export const getAllUsersResolver = authorized( - async (_obj, _params) => { - const users = await userRepository.findTopUsers() - const result = { users: users.map((userData) => userDataToUser(userData)) } - return result - } -) +export const getAllUsersResolver = authorized< + Merge }>, + UsersError +>(async (_obj, _params) => { + const users = await userRepository.findTopUsers() + const result = { users } + return result +}) type ErrorWithCode = { errorCode: string diff --git a/packages/api/src/routers/article_router.ts b/packages/api/src/routers/article_router.ts index fbe9ea4d5..1adda960a 100644 --- a/packages/api/src/routers/article_router.ts +++ b/packages/api/src/routers/article_router.ts @@ -49,22 +49,23 @@ export function articleRouter() { return res.status(400).send('Bad Request') } - const result = await createPageSaveRequest({ user, url }) - if (isSiteBlockedForParse(url)) { return res .status(400) .send({ errorCode: CreateArticleErrorCode.NotAllowedToParse }) } - if (result.errorCode) { - return res.status(400).send({ errorCode: result.errorCode }) - } + try { + const result = await createPageSaveRequest({ user, url }) - return res.send({ - articleSavingRequestId: result.id, - url: result.url, - }) + return res.send({ + articleSavingRequestId: result.id, + url: result.originalUrl, + }) + } catch (error) { + logger.error('Error saving article:', error) + return res.status(500).send({ errorCode: 'INTERNAL_ERROR' }) + } }) router.get( diff --git a/packages/api/src/services/create_page_save_request.ts b/packages/api/src/services/create_page_save_request.ts index ce4ce137b..8e2470e2c 100644 --- a/packages/api/src/services/create_page_save_request.ts +++ b/packages/api/src/services/create_page_save_request.ts @@ -1,20 +1,16 @@ import * as privateIpLib from 'private-ip' -import { LibraryItemState } from '../entity/library_item' +import { LibraryItem, LibraryItemState } from '../entity/library_item' import { User } from '../entity/user' import { - ArticleSavingRequest, ArticleSavingRequestStatus, CreateArticleSavingRequestErrorCode, CreateLabelInput, PageType, } from '../generated/graphql' import { createPubSubClient, PubsubClient } from '../pubsub' +import { Merge } from '../util' import { enqueueParseRequest } from '../utils/createTask' -import { - cleanUrl, - generateSlug, - libraryItemToArticleSavingRequest, -} from '../utils/helpers' +import { cleanUrl, generateSlug } from '../utils/helpers' import { logger } from '../utils/logger' import { countBySavedAt, createOrUpdateLibraryItem } from './library_item' @@ -88,11 +84,12 @@ export const createPageSaveRequest = async ({ publishedAt, folder, subscription, -}: PageSaveRequest): Promise => { +}: PageSaveRequest): Promise => { try { validateUrl(url) } catch (error) { - logger.info('invalid url', { url, error }) + logger.error('invalid url', { url, error }) + return Promise.reject({ errorCode: CreateArticleSavingRequestErrorCode.BadData, }) @@ -140,5 +137,5 @@ export const createPageSaveRequest = async ({ rssFeedUrl: subscription, }) - return libraryItemToArticleSavingRequest(user, libraryItem) + return libraryItem } diff --git a/packages/api/src/services/groups.ts b/packages/api/src/services/groups.ts index cedfecd5a..ef1f61325 100644 --- a/packages/api/src/services/groups.ts +++ b/packages/api/src/services/groups.ts @@ -6,9 +6,8 @@ import { Invite } from '../entity/groups/invite' import { RuleActionType } from '../entity/rule' import { User } from '../entity/user' import { homePageURL } from '../env' -import { RecommendationGroup, User as GraphqlUser } from '../generated/graphql' import { getRepository } from '../repository' -import { userDataToUser } from '../utils/helpers' +import { PartialRecommendationGroup } from '../resolvers' import { findOrCreateLabels } from './labels' import { createRule } from './rules' @@ -70,22 +69,21 @@ export const createGroup = async (input: { export const getRecommendationGroups = async ( user: User -): Promise => { +): Promise> => { const groupMembers = await getRepository(GroupMembership).find({ where: { user: { id: user.id } }, relations: ['invite', 'group.members.user.profile'], }) return groupMembers.map((gm) => { - const admins: GraphqlUser[] = [] - const members: GraphqlUser[] = [] + const admins: Array = [] + const members: Array = [] // Return all members gm.group.members.forEach((m) => { - const user = userDataToUser(m.user) if (m.isAdmin) { - admins.push(user) + admins.push(m.user) } - members.push(user) + members.push(m.user) }) const canSeeMembers = gm.group.onlyAdminCanSeeMembers ? gm.isAdmin : true @@ -113,7 +111,7 @@ export const getInviteUrl = (invite: Invite) => { export const joinGroup = async ( user: User, inviteCode: string -): Promise => { +): Promise => { const invite = await appDataSource.transaction(async (t) => { // Check if the invite exists const invite = await t @@ -147,15 +145,14 @@ export const joinGroup = async ( where: { id: invite.group.id }, relations: ['members', 'members.user.profile'], }) - const admins: GraphqlUser[] = [] - const members: GraphqlUser[] = [] + const admins: Array = [] + const members: Array = [] // Return all members group.members.forEach((m) => { - const user = userDataToUser(m.user) if (m.isAdmin) { - admins.push(user) + admins.push(m.user) } - members.push(user) + members.push(m.user) }) return { diff --git a/packages/api/src/utils/helpers.ts b/packages/api/src/utils/helpers.ts index 30a821a5e..97f495859 100644 --- a/packages/api/src/utils/helpers.ts +++ b/packages/api/src/utils/helpers.ts @@ -7,30 +7,11 @@ import path from 'path' import _ from 'underscore' import slugify from 'voca/slugify' import wordsCounter from 'word-counting' -import { Highlight as HighlightData } from '../entity/highlight' import { LibraryItem, LibraryItemState } from '../entity/library_item' -import { Recommendation as RecommendationData } from '../entity/recommendation' -import { RegistrationType, User } from '../entity/user' -import { - Article, - ArticleSavingRequest, - ArticleSavingRequestStatus, - ContentReader, - CreateArticleError, - CreateArticleSuccess, - DirectionalityType, - FeedArticle, - Highlight, - PageType, - Profile, - Recommendation, - SearchItem, -} from '../generated/graphql' +import { CreateArticleError } from '../generated/graphql' import { createPubSubClient } from '../pubsub' -import { ArticleFormat } from '../resolvers' import { validateUrl } from '../services/create_page_save_request' import { updateLibraryItem } from '../services/library_item' -import { Merge } from '../util' import { logger } from './logger' interface InputObject { @@ -101,55 +82,6 @@ export const findDelimiter = ( return delimiter || defaultDelimiter } -// FIXME: Remove this Date stub after nullable types will be fixed -export const userDataToUser = ( - user: Merge< - User, - { - isFriend?: boolean - followersCount?: number - friendsCount?: number - sharedArticlesCount?: number - sharedHighlightsCount?: number - sharedNotesCount?: number - viewerIsFollowing?: boolean - } - > -): { - id: string - name: string - source: RegistrationType - email?: string | null - phone?: string | null - picture?: string | null - googleId?: string | null - createdAt: Date - isFriend?: boolean | null - isFullUser: boolean - viewerIsFollowing?: boolean | null - sourceUserId: string - friendsCount?: number - followersCount?: number - sharedArticles: FeedArticle[] - sharedArticlesCount?: number - sharedHighlightsCount?: number - sharedNotesCount?: number - profile: Profile -} => ({ - ...user, - source: user.source as RegistrationType, - createdAt: user.createdAt, - friendsCount: user.friendsCount || 0, - followersCount: user.followersCount || 0, - isFullUser: true, - viewerIsFollowing: user.viewerIsFollowing || user.isFriend || false, - picture: user.profile.pictureUrl, - sharedArticles: [], - sharedArticlesCount: user.sharedArticlesCount || 0, - sharedHighlightsCount: user.sharedHighlightsCount || 0, - sharedNotesCount: user.sharedNotesCount || 0, -}) - export const generateSlug = (title: string): string => { return slugify(title).substring(0, 64) + '-' + Date.now().toString(16) } @@ -161,7 +93,7 @@ export const errorHandler = async ( userId: string, pageId?: string | null, pubsub = createPubSubClient() -): Promise => { +): Promise => { if (!pageId) return result await updateLibraryItem( @@ -176,86 +108,6 @@ export const errorHandler = async ( return result } -export const highlightDataToHighlight = ( - highlight: HighlightData -): Highlight => ({ - ...highlight, - createdByMe: false, - reactions: [], - replies: [], - type: highlight.highlightType, - user: userDataToUser(highlight.user), -}) - -export const recommandationDataToRecommendation = ( - recommendation: RecommendationData -): Recommendation => ({ - ...recommendation, - user: { - userId: recommendation.recommender.id, - username: recommendation.recommender.profile.username, - profileImageURL: recommendation.recommender.profile.pictureUrl, - name: recommendation.recommender.name, - }, - name: recommendation.group.name, - recommendedAt: recommendation.createdAt, -}) - -export const libraryItemToArticleSavingRequest = ( - user: User, - item: LibraryItem -): ArticleSavingRequest => ({ - ...item, - user: userDataToUser(user), - status: item.state as unknown as ArticleSavingRequestStatus, - url: item.originalUrl, - userId: user.id, -}) - -export const libraryItemToArticle = (item: LibraryItem): Article => ({ - ...item, - url: item.originalUrl, - state: item.state as unknown as ArticleSavingRequestStatus, - content: item.readableContent, - hash: item.textContentHash || '', - isArchived: !!item.archivedAt, - recommendations: item.recommendations?.map( - recommandationDataToRecommendation - ), - image: item.thumbnail, - contentReader: item.contentReader as unknown as ContentReader, - readingProgressAnchorIndex: item.readingProgressHighestReadAnchor, - readingProgressPercent: item.readingProgressBottomPercent, - highlights: item.highlights?.map(highlightDataToHighlight) || [], - uploadFileId: item.uploadFile?.id, - pageType: item.itemType as unknown as PageType, - wordsCount: item.wordCount, - directionality: item.directionality as unknown as DirectionalityType, -}) - -export const libraryItemToSearchItem = ( - item: LibraryItem, - format?: ArticleFormat -): SearchItem => ({ - ...item, - url: item.originalUrl, - state: item.state as unknown as ArticleSavingRequestStatus, - content: item.readableContent, - isArchived: !!item.archivedAt, - pageType: item.itemType as unknown as PageType, - readingProgressPercent: item.readingProgressBottomPercent, - contentReader: item.contentReader as unknown as ContentReader, - readingProgressAnchorIndex: item.readingProgressHighestReadAnchor, - recommendations: item.recommendations?.map( - recommandationDataToRecommendation - ), - image: item.thumbnail, - highlights: item.highlights?.map(highlightDataToHighlight), - wordsCount: item.wordCount, - directionality: item.directionality as unknown as DirectionalityType, - format, -}) - export const isParsingTimeout = (libraryItem: LibraryItem): boolean => { return ( // item processed more than 30 seconds ago From 166b324d6397c51834368c47753057c494a9701f Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 6 Jun 2024 13:26:13 +0800 Subject: [PATCH 3/8] add tests --- packages/api/src/resolvers/highlight/index.ts | 2 +- packages/api/src/services/highlights.ts | 3 + packages/api/test/resolvers/highlight.test.ts | 69 +++++++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/packages/api/src/resolvers/highlight/index.ts b/packages/api/src/resolvers/highlight/index.ts index 9d6d34359..bdff13991 100644 --- a/packages/api/src/resolvers/highlight/index.ts +++ b/packages/api/src/resolvers/highlight/index.ts @@ -240,7 +240,7 @@ export const highlightsResolver = authorized< >(async (_, { after, first }, { uid, log }) => { const limit = first || 10 const offset = parseInt(after || '0') - if (isNaN(offset) || offset < 0) { + if (isNaN(offset) || offset < 0 || limit > 50) { log.error('Invalid after', { after }) return { diff --git a/packages/api/src/services/highlights.ts b/packages/api/src/services/highlights.ts index 2b2c89b71..6116acabc 100644 --- a/packages/api/src/services/highlights.ts +++ b/packages/api/src/services/highlights.ts @@ -270,6 +270,9 @@ export const searchHighlights = async ( async (tx) => tx.withRepository(highlightRepository).find({ where: { user: { id: userId } }, + order: { + updatedAt: 'DESC', + }, take: limit, skip: offset, }), diff --git a/packages/api/test/resolvers/highlight.test.ts b/packages/api/test/resolvers/highlight.test.ts index aef7aecae..b8760e410 100644 --- a/packages/api/test/resolvers/highlight.test.ts +++ b/packages/api/test/resolvers/highlight.test.ts @@ -3,13 +3,16 @@ import * as chai from 'chai' import { expect } from 'chai' import chaiString from 'chai-string' import 'mocha' +import { Highlight } from '../../src/entity/highlight' import { User } from '../../src/entity/user' +import { HighlightEdge } from '../../src/generated/graphql' import { createHighlight, deleteHighlightById, findHighlightById, } from '../../src/services/highlights' import { createLabel, saveLabelsInHighlight } from '../../src/services/labels' +import { deleteLibraryItemsByUserId } from '../../src/services/library_item' import { deleteUser } from '../../src/services/user' import { createTestLibraryItem, createTestUser } from '../db' import { @@ -344,4 +347,70 @@ describe('Highlights API', () => { ) }) }) + + describe('Get highlights API', () => { + const query = ` + query Highlights ($first: Int, $after: String) { + highlights (first: $first, after: $after) { + ... on HighlightsSuccess { + edges { + node { + id + user { + id + } + } + cursor + } + pageInfo { + hasNextPage + endCursor + } + } + ... on HighlightsError { + errorCodes + } + } + } + ` + let existingHighlights: Highlight[] + + before(async () => { + // create test library item + const item = await createTestLibraryItem(user.id) + + // create test highlights + const highlight1 = await createHighlight( + { + libraryItem: { id: item.id }, + shortId: generateFakeShortId(), + user: { id: user.id }, + }, + itemId, + user.id + ) + const highlight2 = await createHighlight( + { + libraryItem: { id: item.id }, + shortId: generateFakeShortId(), + user: { id: user.id }, + }, + itemId, + user.id + ) + existingHighlights = [highlight1, highlight2] + }) + + after(async () => { + await deleteLibraryItemsByUserId(user.id) + }) + + it('returns highlights', async () => { + const res = await graphqlRequest(query, authToken).expect(200) + const highlights = res.body.data.highlights.edges as Array + expect(highlights).to.have.lengthOf(existingHighlights.length) + expect(highlights[0].node.id).to.eq(existingHighlights[1].id) + expect(highlights[1].node.id).to.eq(existingHighlights[0].id) + }) + }) }) From 9259a9cfe3452c3e9146d6ad945ac16aeb9f3870 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 6 Jun 2024 14:53:14 +0800 Subject: [PATCH 4/8] fix tests --- packages/api/src/services/highlights.ts | 44 +++++++++++++------ packages/api/test/resolvers/highlight.test.ts | 31 ++++++++----- 2 files changed, 52 insertions(+), 23 deletions(-) diff --git a/packages/api/src/services/highlights.ts b/packages/api/src/services/highlights.ts index 6116acabc..3c5cacecd 100644 --- a/packages/api/src/services/highlights.ts +++ b/packages/api/src/services/highlights.ts @@ -1,5 +1,5 @@ import { diff_match_patch } from 'diff-match-patch' -import { DeepPartial, In, LessThan } from 'typeorm' +import { DeepPartial, In } from 'typeorm' import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity' import { EntityLabel } from '../entity/entity_label' import { Highlight } from '../entity/highlight' @@ -205,19 +205,26 @@ export const updateHighlight = async ( return updatedHighlight } -export const deleteHighlightById = async (highlightId: string) => { - const deletedHighlight = await authTrx(async (tx) => { - const highlightRepo = tx.withRepository(highlightRepository) - const highlight = await highlightRepo.findOneOrFail({ - where: { id: highlightId }, - relations: { - user: true, - }, - }) +export const deleteHighlightById = async ( + highlightId: string, + userId?: string +) => { + const deletedHighlight = await authTrx( + async (tx) => { + const highlightRepo = tx.withRepository(highlightRepository) + const highlight = await highlightRepo.findOneOrFail({ + where: { id: highlightId }, + relations: { + user: true, + }, + }) - await highlightRepo.delete(highlightId) - return highlight - }) + await highlightRepo.delete(highlightId) + return highlight + }, + undefined, + userId + ) await enqueueUpdateHighlight({ libraryItemId: deletedHighlight.libraryItemId, @@ -227,6 +234,17 @@ export const deleteHighlightById = async (highlightId: string) => { return deletedHighlight } +export const deleteHighlightsByIds = async ( + userId: string, + highlightIds: string[] +) => { + await authTrx( + async (tx) => tx.getRepository(Highlight).delete(highlightIds), + undefined, + userId + ) +} + export const findHighlightById = async ( highlightId: string, userId: string diff --git a/packages/api/test/resolvers/highlight.test.ts b/packages/api/test/resolvers/highlight.test.ts index b8760e410..df2dd9bad 100644 --- a/packages/api/test/resolvers/highlight.test.ts +++ b/packages/api/test/resolvers/highlight.test.ts @@ -9,10 +9,10 @@ import { HighlightEdge } from '../../src/generated/graphql' import { createHighlight, deleteHighlightById, + deleteHighlightsByIds, findHighlightById, } from '../../src/services/highlights' import { createLabel, saveLabelsInHighlight } from '../../src/services/labels' -import { deleteLibraryItemsByUserId } from '../../src/services/library_item' import { deleteUser } from '../../src/services/user' import { createTestLibraryItem, createTestUser } from '../db' import { @@ -168,8 +168,14 @@ describe('Highlights API', () => { }) context('createHighlightMutation', () => { + let highlightId: string + + afterEach(async () => { + await deleteHighlightById(highlightId, user.id) + }) + it('does not fail', async () => { - const highlightId = generateFakeUuid() + highlightId = generateFakeUuid() const shortHighlightId = '_short_id' const highlightPositionPercent = 35.0 const highlightPositionAnchorIndex = 15 @@ -197,31 +203,29 @@ describe('Highlights API', () => { context('when highlight position is null', () => { it('sets highlight position = 0', async () => { - const newHighlightId = generateFakeUuid() + highlightId = generateFakeUuid() const newShortHighlightId = '_short_id_5' const query = createHighlightQuery( itemId, - newHighlightId, + highlightId, newShortHighlightId ) const res = await graphqlRequest(query, authToken).expect(200) expect( res.body.data.createHighlight.highlight.highlightPositionPercent ).to.eq(0) - - await deleteHighlightById(newHighlightId) }) }) context('when the annotation has HTML reserved characters', () => { it('unescapes the annotation and creates', async () => { - const newHighlightId = generateFakeUuid() + highlightId = generateFakeUuid() const newShortHighlightId = '_short_id_4' const highlightPositionPercent = 50.0 const highlightPositionAnchorIndex = 25 const query = createHighlightQuery( itemId, - newHighlightId, + highlightId, newShortHighlightId, highlightPositionPercent, highlightPositionAnchorIndex, @@ -247,7 +251,7 @@ describe('Highlights API', () => { }) afterEach(async () => { - await deleteHighlightById(highlightId) + await deleteHighlightById(highlightId, user.id) }) it('should not fail', async () => { @@ -321,6 +325,10 @@ describe('Highlights API', () => { highlightId = highlight.id }) + after(async () => { + await deleteHighlightById(highlightId, user.id) + }) + it('updates the quote when the quote is in HTML format when the annotation has HTML reserved characters', async () => { const quote = '> This is a test' const query = updateHighlightQuery({ highlightId, quote }) @@ -402,7 +410,10 @@ describe('Highlights API', () => { }) after(async () => { - await deleteLibraryItemsByUserId(user.id) + await deleteHighlightsByIds( + user.id, + existingHighlights.map((h) => h.id) + ) }) it('returns highlights', async () => { From 51efff8b8ea1f4745588fc8e02bdbaaa2efae461 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 6 Jun 2024 15:23:56 +0800 Subject: [PATCH 5/8] allow returning library item information in highlight --- packages/api/src/generated/graphql.ts | 2 + packages/api/src/generated/schema.graphql | 1 + .../api/src/resolvers/function_resolvers.ts | 98 ++++++------------- packages/api/src/resolvers/highlight/index.ts | 25 +++-- packages/api/src/schema.ts | 1 + packages/api/src/services/highlights.ts | 5 +- 6 files changed, 52 insertions(+), 80 deletions(-) diff --git a/packages/api/src/generated/graphql.ts b/packages/api/src/generated/graphql.ts index 27cdcf62b..203364aa9 100644 --- a/packages/api/src/generated/graphql.ts +++ b/packages/api/src/generated/graphql.ts @@ -1261,6 +1261,7 @@ export type Highlight = { html?: Maybe; id: Scalars['ID']; labels?: Maybe>; + libraryItem: Article; patch?: Maybe; prefix?: Maybe; quote?: Maybe; @@ -6128,6 +6129,7 @@ export type HighlightResolvers, ParentType, ContextType>; id?: Resolver; labels?: Resolver>, ParentType, ContextType>; + libraryItem?: Resolver; patch?: Resolver, ParentType, ContextType>; prefix?: Resolver, ParentType, ContextType>; quote?: Resolver, ParentType, ContextType>; diff --git a/packages/api/src/generated/schema.graphql b/packages/api/src/generated/schema.graphql index d0e46a4b3..5692cb0aa 100644 --- a/packages/api/src/generated/schema.graphql +++ b/packages/api/src/generated/schema.graphql @@ -1133,6 +1133,7 @@ type Highlight { html: String id: ID! labels: [Label!] + libraryItem: Article! patch: String prefix: String quote: String diff --git a/packages/api/src/resolvers/function_resolvers.ts b/packages/api/src/resolvers/function_resolvers.ts index a4fc71cf8..aed2b51e8 100644 --- a/packages/api/src/resolvers/function_resolvers.ts +++ b/packages/api/src/resolvers/function_resolvers.ts @@ -6,7 +6,6 @@ import { createHmac } from 'crypto' import { isError } from 'lodash' import { Highlight } from '../entity/highlight' -import { Label } from '../entity/label' import { LibraryItem } from '../entity/library_item' import { EXISTING_NEWSLETTER_FOLDER, @@ -71,14 +70,12 @@ import { createHighlightResolver, createLabelResolver, createNewsletterEmailResolver, - // createReminderResolver, deleteAccountResolver, deleteFilterResolver, deleteHighlightResolver, deleteIntegrationResolver, deleteLabelResolver, deleteNewsletterEmailResolver, - // deleteReminderResolver, deleteRuleResolver, deleteWebhookResolver, deviceTokensResolver, @@ -88,11 +85,7 @@ import { generateApiKeyResolver, getAllUsersResolver, getArticleResolver, - // getFollowersResolver, - // getFollowingResolver, getMeUserResolver, - // getSharedArticleResolver, - // getUserFeedArticlesResolver, getUserPersonalizationResolver, getUserResolver, googleLoginResolver, @@ -112,7 +105,6 @@ import { newsletterEmailsResolver, recommendHighlightsResolver, recommendResolver, - // reminderResolver, reportItemResolver, revokeApiKeyResolver, rulesResolver, @@ -127,14 +119,11 @@ import { setBookmarkArticleResolver, setDeviceTokenResolver, setFavoriteArticleResolver, - // setFollowResolver, setIntegrationResolver, setLabelsForHighlightResolver, setLabelsResolver, setLinkArchivedResolver, setRuleResolver, - // setShareArticleResolver, - // setShareHighlightResolver, setUserPersonalizationResolver, setWebhookResolver, subscribeResolver, @@ -145,10 +134,7 @@ import { updateHighlightResolver, updateLabelResolver, updateNewsletterEmailResolver, - // updateLinkShareInfoResolver, updatePageResolver, - // updateReminderResolver, - // updateSharedCommentResolver, updatesSinceResolver, updateSubscriptionResolver, updateUserProfileResolver, @@ -259,30 +245,20 @@ export const functionResolvers = { updateUserProfile: updateUserProfileResolver, createArticle: createArticleResolver, createHighlight: createHighlightResolver, - // createReaction: createReactionResolver, - // deleteReaction: deleteReactionResolver, mergeHighlight: mergeHighlightResolver, updateHighlight: updateHighlightResolver, deleteHighlight: deleteHighlightResolver, uploadFileRequest: uploadFileRequestResolver, - // setShareArticle: setShareArticleResolver, - // updateSharedComment: updateSharedCommentResolver, - // setFollow: setFollowResolver, setBookmarkArticle: setBookmarkArticleResolver, setUserPersonalization: setUserPersonalizationResolver, createArticleSavingRequest: createArticleSavingRequestResolver, - // setShareHighlight: setShareHighlightResolver, reportItem: reportItemResolver, - // updateLinkShareInfo: updateLinkShareInfoResolver, setLinkArchived: setLinkArchivedResolver, createNewsletterEmail: createNewsletterEmailResolver, deleteNewsletterEmail: deleteNewsletterEmailResolver, saveUrl: saveUrlResolver, savePage: savePageResolver, saveFile: saveFileResolver, - // createReminder: createReminderResolver, - // updateReminder: updateReminderResolver, - // deleteReminder: deleteReminderResolver, setDeviceToken: setDeviceTokenResolver, createLabel: createLabelResolver, updateLabel: updateLabelResolver, @@ -340,14 +316,9 @@ export const functionResolvers = { users: getAllUsersResolver, validateUsername: validateUsernameResolver, article: getArticleResolver, - // sharedArticle: getSharedArticleResolver, - // feedArticles: getUserFeedArticlesResolver, - // getFollowers: getFollowersResolver, - // getFollowing: getFollowingResolver, getUserPersonalization: getUserPersonalizationResolver, articleSavingRequest: articleSavingRequestResolver, newsletterEmails: newsletterEmailsResolver, - // reminder: reminderResolver, labels: labelsResolver, search: searchResolver, subscriptions: subscriptionsResolver, @@ -373,11 +344,7 @@ export const functionResolvers = { highlights: highlightsResolver, }, User: { - async intercomHash( - user: User, - __: Record, - ctx: WithDataSourcesContext - ) { + async intercomHash(user: User) { if (env.intercom.secretKey) { const userIdentifier = user.id.toString() @@ -445,8 +412,8 @@ export const functionResolvers = { hasContent(article: LibraryItem) { return !!article.originalContent && !!article.readableContent }, - publishedAt(article: { publishedAt: Date }) { - return validatedDate(article.publishedAt) + publishedAt(article: LibraryItem) { + return validatedDate(article.publishedAt || undefined) }, image(article: LibraryItem): string | undefined { if (article.thumbnail) { @@ -471,6 +438,15 @@ export const functionResolvers = { return ctx.dataLoaders.labels.load(article.id) }, + async highlights( + article: LibraryItem, + _: unknown, + ctx: WithDataSourcesContext + ) { + if (article.highlights) return article.highlights + + return ctx.dataLoaders.highlights.load(article.id) + }, content: (item: LibraryItem) => item.readableContent, hash: (item: LibraryItem) => item.textContentHash || '', isArchived: (item: LibraryItem) => !!item.archivedAt, @@ -486,11 +462,18 @@ export const functionResolvers = { return ctx.dataLoaders.users.load(highlight.userId) }, createdByMe( - highlight: { user: { id: string } }, + highlight: Highlight, __: unknown, ctx: WithDataSourcesContext ) { - return highlight.user.id === ctx.uid + return highlight.userId === ctx.uid + }, + libraryItem(highlight: Highlight, _: unknown, ctx: WithDataSourcesContext) { + if (highlight.libraryItem) { + return highlight.libraryItem + } + + return ctx.dataLoaders.libraryItems.load(highlight.libraryItemId) }, }, SearchItem: { @@ -526,30 +509,19 @@ export const functionResolvers = { return item.siteIcon }, - async labels( - item: { id: string; labels?: Label[] }, - _: unknown, - ctx: WithDataSourcesContext - ) { + async labels(item: LibraryItem, _: unknown, ctx: WithDataSourcesContext) { if (item.labels) return item.labels - const labels = await ctx.dataLoaders.labels.load(item.id) - return labels + return ctx.dataLoaders.labels.load(item.id) }, async recommendations( - item: { - id: string - recommendations?: Recommendation[] - }, + item: LibraryItem, _: unknown, ctx: WithDataSourcesContext ) { if (item.recommendations) return item.recommendations - const recommendations = await ctx.dataLoaders.recommendations.load( - item.id - ) - return recommendations + return ctx.dataLoaders.recommendations.load(item.id) }, async aiSummary( item: LibraryItem, @@ -565,19 +537,14 @@ export const functionResolvers = { )?.summary }, async highlights( - item: { - id: string - highlights?: Highlight[] - }, + item: LibraryItem, _: unknown, ctx: WithDataSourcesContext ) { if (item.highlights) return item.highlights - const highlights = await ctx.dataLoaders.highlights.load(item.id) - return highlights + return ctx.dataLoaders.highlights.load(item.id) }, - ...readingProgressHandlers, async content( item: PartialLibraryItem, _: unknown, @@ -615,6 +582,7 @@ export const functionResolvers = { }, isArchived: (item: LibraryItem) => !!item.archivedAt, pageType: (item: LibraryItem) => item.itemType, + ...readingProgressHandlers, }, Subscription: { newsletterEmail(subscription: Subscription) { @@ -811,31 +779,21 @@ export const functionResolvers = { ...resultResolveTypeResolver('UpdateUser'), ...resultResolveTypeResolver('UpdateUserProfile'), ...resultResolveTypeResolver('Article'), - // ...resultResolveTypeResolver('SharedArticle'), ...resultResolveTypeResolver('Articles'), ...resultResolveTypeResolver('User'), ...resultResolveTypeResolver('Users'), ...resultResolveTypeResolver('SaveArticleReadingProgress'), - // ...resultResolveTypeResolver('FeedArticles'), ...resultResolveTypeResolver('CreateArticle'), ...resultResolveTypeResolver('CreateHighlight'), - // ...resultResolveTypeResolver('CreateReaction'), - // ...resultResolveTypeResolver('DeleteReaction'), ...resultResolveTypeResolver('MergeHighlight'), ...resultResolveTypeResolver('UpdateHighlight'), ...resultResolveTypeResolver('DeleteHighlight'), ...resultResolveTypeResolver('UploadFileRequest'), - // ...resultResolveTypeResolver('SetShareArticle'), - // ...resultResolveTypeResolver('UpdateSharedComment'), ...resultResolveTypeResolver('SetBookmarkArticle'), - // ...resultResolveTypeResolver('SetFollow'), - // ...resultResolveTypeResolver('GetFollowers'), - // ...resultResolveTypeResolver('GetFollowing'), ...resultResolveTypeResolver('GetUserPersonalization'), ...resultResolveTypeResolver('SetUserPersonalization'), ...resultResolveTypeResolver('ArticleSavingRequest'), ...resultResolveTypeResolver('CreateArticleSavingRequest'), - // ...resultResolveTypeResolver('SetShareHighlight'), ...resultResolveTypeResolver('ArchiveLink'), ...resultResolveTypeResolver('CreateNewsletterEmail'), ...resultResolveTypeResolver('NewsletterEmails'), diff --git a/packages/api/src/resolvers/highlight/index.ts b/packages/api/src/resolvers/highlight/index.ts index bdff13991..a52b5d06c 100644 --- a/packages/api/src/resolvers/highlight/index.ts +++ b/packages/api/src/resolvers/highlight/index.ts @@ -237,25 +237,34 @@ export const highlightsResolver = authorized< PartialHighlightsSuccess, HighlightsError, QueryHighlightsArgs ->(async (_, { after, first }, { uid, log }) => { +>(async (_, { after, first, query }, { uid, log }) => { const limit = first || 10 const offset = parseInt(after || '0') - if (isNaN(offset) || offset < 0 || limit > 50) { - log.error('Invalid after', { after }) + if ( + isNaN(offset) || + offset < 0 || + limit > 50 || + (query?.length && query.length > 1000) + ) { + log.error('Invalid args', { after, first, query }) return { errorCodes: [HighlightsErrorCode.BadRequest], } } - const highlights = await searchHighlights(uid, limit + 1, offset) + const highlights = await searchHighlights( + uid, + query || undefined, + limit + 1, + offset + ) - const start = offset const hasNextPage = highlights.length > limit if (hasNextPage) { highlights.pop() } - const endCursor = String(start + highlights.length) + const endCursor = String(offset + highlights.length) const edges = highlights.map((highlight) => ({ cursor: endCursor, @@ -265,9 +274,9 @@ export const highlightsResolver = authorized< return { edges, pageInfo: { - startCursor: String(start), + startCursor: String(offset), endCursor, - hasPreviousPage: start > 0, + hasPreviousPage: offset > 0, hasNextPage, }, } diff --git a/packages/api/src/schema.ts b/packages/api/src/schema.ts index e7d48d5a4..298bb7e90 100755 --- a/packages/api/src/schema.ts +++ b/packages/api/src/schema.ts @@ -751,6 +751,7 @@ const schema = gql` html: String color: String representation: RepresentationType! + libraryItem: Article! } input CreateHighlightInput { diff --git a/packages/api/src/services/highlights.ts b/packages/api/src/services/highlights.ts index 3c5cacecd..d6ad393c0 100644 --- a/packages/api/src/services/highlights.ts +++ b/packages/api/src/services/highlights.ts @@ -281,9 +281,10 @@ export const findHighlightsByLibraryItemId = async ( export const searchHighlights = async ( userId: string, - limit: number, + query?: string, + limit?: number, offset?: number -) => { +): Promise> => { return authTrx( async (tx) => tx.withRepository(highlightRepository).find({ From 24457d90365903471d6ebe10d54e8970abfaf859 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 6 Jun 2024 16:12:07 +0800 Subject: [PATCH 6/8] search highlights by labels --- packages/api/src/repository/index.ts | 20 ++ packages/api/src/services/highlights.ts | 189 +++++++++++++++++- packages/api/src/services/library_item.ts | 30 +-- packages/api/test/resolvers/highlight.test.ts | 57 +++++- 4 files changed, 260 insertions(+), 36 deletions(-) diff --git a/packages/api/src/repository/index.ts b/packages/api/src/repository/index.ts index 3a5db7e72..21c59becd 100644 --- a/packages/api/src/repository/index.ts +++ b/packages/api/src/repository/index.ts @@ -12,6 +12,26 @@ import { appDataSource } from '../data_source' import { Claims } from '../resolvers/types' import { SetClaimsRole } from '../utils/dictionary' +export enum SortOrder { + ASCENDING = 'ASC', + DESCENDING = 'DESC', +} + +export interface Sort { + by: string + order?: SortOrder + nulls?: 'NULLS FIRST' | 'NULLS LAST' +} + +export interface Select { + column: string + alias?: string +} + +export const paramtersToObject = (parameters: ObjectLiteral[]) => { + return parameters.reduce((a, b) => ({ ...a, ...b }), {}) +} + export const getColumns = ( repository: Repository ): (keyof T)[] => { diff --git a/packages/api/src/services/highlights.ts b/packages/api/src/services/highlights.ts index d6ad393c0..e154faf71 100644 --- a/packages/api/src/services/highlights.ts +++ b/packages/api/src/services/highlights.ts @@ -1,16 +1,18 @@ +import { ExpressionToken, LiqeQuery } from '@omnivore/liqe' import { diff_match_patch } from 'diff-match-patch' -import { DeepPartial, In } from 'typeorm' +import { DeepPartial, In, ObjectLiteral } from 'typeorm' import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity' import { EntityLabel } from '../entity/entity_label' import { Highlight } from '../entity/highlight' import { Label } from '../entity/label' import { homePageURL } from '../env' import { createPubSubClient, EntityEvent, EntityType } from '../pubsub' -import { authTrx } from '../repository' +import { authTrx, paramtersToObject } from '../repository' import { highlightRepository } from '../repository/highlight' import { Merge } from '../util' import { enqueueUpdateHighlight } from '../utils/createTask' import { deepDelete } from '../utils/helpers' +import { parseSearchQuery } from '../utils/search' import { ItemEvent } from './library_item' const columnsToDelete = ['user', 'sharedAt', 'libraryItem'] as const @@ -279,6 +281,149 @@ export const findHighlightsByLibraryItemId = async ( ) } +export const buildQueryString = ( + searchQuery: LiqeQuery, + parameters: ObjectLiteral[] = [] +) => { + const escapeQueryWithParameters = ( + query: string, + parameter: ObjectLiteral + ) => { + parameters.push(parameter) + return query + } + + const serializeImplicitField = ( + expression: ExpressionToken + ): string | null => { + if (expression.type !== 'LiteralExpression') { + throw new Error('Expected a literal expression') + } + + // not implemented yet + return null + } + + const serializeTagExpression = (ast: LiqeQuery): string | null => { + if (ast.type !== 'Tag') { + throw new Error('Expected a tag expression') + } + + const { field, expression } = ast + + if (field.type === 'ImplicitField') { + return serializeImplicitField(expression) + } else { + if (expression.type !== 'LiteralExpression') { + // ignore empty values + return null + } + + const value = expression.value?.toString() + if (!value) { + // ignore empty values + return null + } + + switch (field.name.toLowerCase()) { + case 'label': { + const labels = value.toLowerCase().split(',') + return ( + labels + .map((label) => { + const param = `label_${parameters.length}` + + const hasWildcard = label.includes('*') + if (hasWildcard) { + return escapeQueryWithParameters( + `label.name ILIKE :${param}`, + { + [param]: label.replace(/\*/g, '%'), + } + ) + } + + return escapeQueryWithParameters( + `LOWER(label.name) = :${param}`, + { + [param]: label.toLowerCase(), + } + ) + }) + .join(' OR ') + // wrap in brackets to avoid precedence issues + .replace(/^(.*)$/, '($1)') + ) + } + default: + // treat unknown fields as implicit fields + return serializeImplicitField({ + ...expression, + value: `${field.name}:${value}`, + }) + } + } + } + + const serialize = (ast: LiqeQuery): string | null => { + if (ast.type === 'Tag') { + return serializeTagExpression(ast) + } + + if (ast.type === 'LogicalExpression') { + let operator = '' + if (ast.operator.operator === 'AND') { + operator = 'AND' + } else if (ast.operator.operator === 'OR') { + operator = 'OR' + } else { + throw new Error('Unexpected operator') + } + + const left = serialize(ast.left) + const right = serialize(ast.right) + + if (!left && !right) { + return null + } + + if (!left) { + return right + } + + if (!right) { + return left + } + + return `${left} ${operator} ${right}` + } + + if (ast.type === 'UnaryOperator') { + const serialized = serialize(ast.operand) + + if (!serialized) { + return null + } + + return `NOT ${serialized}` + } + + if (ast.type === 'ParenthesizedExpression') { + const serialized = serialize(ast.expression) + + if (!serialized) { + return null + } + + return `(${serialized})` + } + + return null + } + + return serialize(searchQuery) +} + export const searchHighlights = async ( userId: string, query?: string, @@ -286,15 +431,37 @@ export const searchHighlights = async ( offset?: number ): Promise> => { return authTrx( - async (tx) => - tx.withRepository(highlightRepository).find({ - where: { user: { id: userId } }, - order: { - updatedAt: 'DESC', - }, - take: limit, - skip: offset, - }), + async (tx) => { + // TODO: parse query and search by it + const queryBuilder = tx + .getRepository(Highlight) + .createQueryBuilder('highlight') + + queryBuilder + .andWhere('highlight.userId = :userId', { userId }) + .orderBy('highlight.updatedAt', 'DESC') + .take(limit) + .skip(offset) + + if (query) { + const parameters: ObjectLiteral[] = [] + + const searchQuery = parseSearchQuery(query) + + // build query string and save parameters + const queryString = buildQueryString(searchQuery, parameters) + + if (queryString) { + // add where clause from query string + queryBuilder + .innerJoinAndSelect('highlight.labels', 'label') + .andWhere(`(${queryString})`) + .setParameters(paramtersToObject(parameters)) + } + } + + return queryBuilder.getMany() + }, undefined, userId ) diff --git a/packages/api/src/services/library_item.ts b/packages/api/src/services/library_item.ts index 4390e6128..c6ecbfc9f 100644 --- a/packages/api/src/services/library_item.ts +++ b/packages/api/src/services/library_item.ts @@ -18,7 +18,15 @@ import { env } from '../env' import { BulkActionType, InputMaybe, SortParams } from '../generated/graphql' import { createPubSubClient, EntityEvent, EntityType } from '../pubsub' import { redisDataSource } from '../redis_data_source' -import { authTrx, getColumns, queryBuilderToRawSql } from '../repository' +import { + authTrx, + getColumns, + paramtersToObject, + queryBuilderToRawSql, + Select, + Sort, + SortOrder, +} from '../repository' import { libraryItemRepository } from '../repository/library_item' import { Merge, PickTuple } from '../util' import { enqueueBulkUploadContentJob } from '../utils/createTask' @@ -122,22 +130,6 @@ export enum SortBy { WORDS_COUNT = 'wordscount', } -export enum SortOrder { - ASCENDING = 'ASC', - DESCENDING = 'DESC', -} - -export interface Sort { - by: string - order?: SortOrder - nulls?: 'NULLS FIRST' | 'NULLS LAST' -} - -interface Select { - column: string - alias?: string -} - const readingProgressDataSource = new ReadingProgressDataSource() export const batchGetLibraryItems = async (ids: readonly string[]) => { @@ -197,10 +189,6 @@ const handleNoCase = (value: string) => { throw new Error(`Unexpected keyword: ${value}`) } -const paramtersToObject = (parameters: ObjectLiteral[]) => { - return parameters.reduce((a, b) => ({ ...a, ...b }), {}) -} - export const sortParamsToSort = ( sortParams: InputMaybe | undefined ) => { diff --git a/packages/api/test/resolvers/highlight.test.ts b/packages/api/test/resolvers/highlight.test.ts index df2dd9bad..dc0a2c08e 100644 --- a/packages/api/test/resolvers/highlight.test.ts +++ b/packages/api/test/resolvers/highlight.test.ts @@ -12,7 +12,11 @@ import { deleteHighlightsByIds, findHighlightById, } from '../../src/services/highlights' -import { createLabel, saveLabelsInHighlight } from '../../src/services/labels' +import { + createLabel, + deleteLabels, + saveLabelsInHighlight, +} from '../../src/services/labels' import { deleteUser } from '../../src/services/user' import { createTestLibraryItem, createTestUser } from '../db' import { @@ -358,14 +362,24 @@ describe('Highlights API', () => { describe('Get highlights API', () => { const query = ` - query Highlights ($first: Int, $after: String) { - highlights (first: $first, after: $after) { + query Highlights ($first: Int, $after: String, $query: String) { + highlights (first: $first, after: $after, query: $query) { ... on HighlightsSuccess { edges { node { id user { id + name + } + labels { + id + name + color + } + libraryItem { + id + title } } cursor @@ -416,12 +430,47 @@ describe('Highlights API', () => { ) }) - it('returns highlights', async () => { + it('returns highlights in descending order', async () => { const res = await graphqlRequest(query, authToken).expect(200) const highlights = res.body.data.highlights.edges as Array expect(highlights).to.have.lengthOf(existingHighlights.length) expect(highlights[0].node.id).to.eq(existingHighlights[1].id) expect(highlights[1].node.id).to.eq(existingHighlights[0].id) + expect(highlights[0].node.user.id).to.eq(user.id) + expect(highlights[1].node.libraryItem.id).to.eq( + existingHighlights[0].libraryItemId + ) + }) + + it('returns highlights with pagination', async () => { + const res = await graphqlRequest(query, authToken, { + first: 1, + }).expect(200) + + const highlights = res.body.data.highlights.edges as Array + expect(highlights).to.have.lengthOf(1) + }) + + it('returns highlights with labels', async () => { + // create labels + const labelName = 'test_label' + const label = await createLabel(labelName, '#ff0000', user.id) + const labelName1 = 'test_label_1' + const label1 = await createLabel(labelName1, '#ff0001', user.id) + + // save labels in highlights + await saveLabelsInHighlight([label], existingHighlights[0].id, user.id) + await saveLabelsInHighlight([label1], existingHighlights[1].id, user.id) + + const res = await graphqlRequest(query, authToken, { + query: `label:${labelName},${labelName1}`, + }).expect(200) + const highlights = res.body.data.highlights.edges as Array + expect(highlights).to.have.lengthOf(2) + expect(highlights[1].node.labels?.[0].name).to.eq(labelName) + expect(highlights[0].node.labels?.[0].name).to.eq(labelName1) + + await deleteLabels([label.id, label1.id], user.id) }) }) }) From 70bc136d1556f1dffa4a64c195613210d9dd2650 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 6 Jun 2024 17:26:01 +0800 Subject: [PATCH 7/8] batch get labels from highlight id --- packages/api/src/apollo.ts | 6 +- .../api/src/resolvers/function_resolvers.ts | 9 + packages/api/src/resolvers/types.ts | 1 + packages/api/src/services/highlights.ts | 181 ++---------------- packages/api/src/services/labels.ts | 17 ++ packages/api/test/resolvers/highlight.test.ts | 2 +- 6 files changed, 51 insertions(+), 165 deletions(-) diff --git a/packages/api/src/apollo.ts b/packages/api/src/apollo.ts index b06bcd304..e51a6aef2 100644 --- a/packages/api/src/apollo.ts +++ b/packages/api/src/apollo.ts @@ -33,7 +33,10 @@ import ScalarResolvers from './scalars' import typeDefs from './schema' import { batchGetHighlightsFromLibraryItemIds } from './services/highlights' import { batchGetPublicItems } from './services/home' -import { batchGetLabelsFromLibraryItemIds } from './services/labels' +import { + batchGetLabelsFromHighlightIds, + batchGetLabelsFromLibraryItemIds, +} from './services/labels' import { batchGetLibraryItems } from './services/library_item' import { batchGetRecommendationsFromLibraryItemIds } from './services/recommendation' import { @@ -128,6 +131,7 @@ const contextFunc: ContextFunction = async ({ users: new DataLoader(async (ids: readonly string[]) => findUsersByIds(ids as string[]) ), + highlightLabels: new DataLoader(batchGetLabelsFromHighlightIds), }, } diff --git a/packages/api/src/resolvers/function_resolvers.ts b/packages/api/src/resolvers/function_resolvers.ts index aed2b51e8..6ed9de486 100644 --- a/packages/api/src/resolvers/function_resolvers.ts +++ b/packages/api/src/resolvers/function_resolvers.ts @@ -475,6 +475,15 @@ export const functionResolvers = { return ctx.dataLoaders.libraryItems.load(highlight.libraryItemId) }, + labels: async ( + highlight: Highlight, + _: unknown, + ctx: WithDataSourcesContext + ) => { + return ( + highlight.labels || ctx.dataLoaders.highlightLabels.load(highlight.id) + ) + }, }, SearchItem: { async url(item: LibraryItem, _: unknown, ctx: WithDataSourcesContext) { diff --git a/packages/api/src/resolvers/types.ts b/packages/api/src/resolvers/types.ts index cc833e556..16857d85d 100644 --- a/packages/api/src/resolvers/types.ts +++ b/packages/api/src/resolvers/types.ts @@ -60,6 +60,7 @@ export interface RequestContext { publicItems: DataLoader subscriptions: DataLoader users: DataLoader + highlightLabels: DataLoader } } diff --git a/packages/api/src/services/highlights.ts b/packages/api/src/services/highlights.ts index e154faf71..80a011589 100644 --- a/packages/api/src/services/highlights.ts +++ b/packages/api/src/services/highlights.ts @@ -1,18 +1,16 @@ -import { ExpressionToken, LiqeQuery } from '@omnivore/liqe' import { diff_match_patch } from 'diff-match-patch' -import { DeepPartial, In, ObjectLiteral } from 'typeorm' +import { DeepPartial, In } from 'typeorm' import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity' import { EntityLabel } from '../entity/entity_label' import { Highlight } from '../entity/highlight' import { Label } from '../entity/label' import { homePageURL } from '../env' import { createPubSubClient, EntityEvent, EntityType } from '../pubsub' -import { authTrx, paramtersToObject } from '../repository' +import { authTrx } from '../repository' import { highlightRepository } from '../repository/highlight' import { Merge } from '../util' import { enqueueUpdateHighlight } from '../utils/createTask' import { deepDelete } from '../utils/helpers' -import { parseSearchQuery } from '../utils/search' import { ItemEvent } from './library_item' const columnsToDelete = ['user', 'sharedAt', 'libraryItem'] as const @@ -281,149 +279,6 @@ export const findHighlightsByLibraryItemId = async ( ) } -export const buildQueryString = ( - searchQuery: LiqeQuery, - parameters: ObjectLiteral[] = [] -) => { - const escapeQueryWithParameters = ( - query: string, - parameter: ObjectLiteral - ) => { - parameters.push(parameter) - return query - } - - const serializeImplicitField = ( - expression: ExpressionToken - ): string | null => { - if (expression.type !== 'LiteralExpression') { - throw new Error('Expected a literal expression') - } - - // not implemented yet - return null - } - - const serializeTagExpression = (ast: LiqeQuery): string | null => { - if (ast.type !== 'Tag') { - throw new Error('Expected a tag expression') - } - - const { field, expression } = ast - - if (field.type === 'ImplicitField') { - return serializeImplicitField(expression) - } else { - if (expression.type !== 'LiteralExpression') { - // ignore empty values - return null - } - - const value = expression.value?.toString() - if (!value) { - // ignore empty values - return null - } - - switch (field.name.toLowerCase()) { - case 'label': { - const labels = value.toLowerCase().split(',') - return ( - labels - .map((label) => { - const param = `label_${parameters.length}` - - const hasWildcard = label.includes('*') - if (hasWildcard) { - return escapeQueryWithParameters( - `label.name ILIKE :${param}`, - { - [param]: label.replace(/\*/g, '%'), - } - ) - } - - return escapeQueryWithParameters( - `LOWER(label.name) = :${param}`, - { - [param]: label.toLowerCase(), - } - ) - }) - .join(' OR ') - // wrap in brackets to avoid precedence issues - .replace(/^(.*)$/, '($1)') - ) - } - default: - // treat unknown fields as implicit fields - return serializeImplicitField({ - ...expression, - value: `${field.name}:${value}`, - }) - } - } - } - - const serialize = (ast: LiqeQuery): string | null => { - if (ast.type === 'Tag') { - return serializeTagExpression(ast) - } - - if (ast.type === 'LogicalExpression') { - let operator = '' - if (ast.operator.operator === 'AND') { - operator = 'AND' - } else if (ast.operator.operator === 'OR') { - operator = 'OR' - } else { - throw new Error('Unexpected operator') - } - - const left = serialize(ast.left) - const right = serialize(ast.right) - - if (!left && !right) { - return null - } - - if (!left) { - return right - } - - if (!right) { - return left - } - - return `${left} ${operator} ${right}` - } - - if (ast.type === 'UnaryOperator') { - const serialized = serialize(ast.operand) - - if (!serialized) { - return null - } - - return `NOT ${serialized}` - } - - if (ast.type === 'ParenthesizedExpression') { - const serialized = serialize(ast.expression) - - if (!serialized) { - return null - } - - return `(${serialized})` - } - - return null - } - - return serialize(searchQuery) -} - export const searchHighlights = async ( userId: string, query?: string, @@ -432,32 +287,32 @@ export const searchHighlights = async ( ): Promise> => { return authTrx( async (tx) => { - // TODO: parse query and search by it const queryBuilder = tx .getRepository(Highlight) .createQueryBuilder('highlight') - - queryBuilder .andWhere('highlight.userId = :userId', { userId }) .orderBy('highlight.updatedAt', 'DESC') .take(limit) .skip(offset) if (query) { - const parameters: ObjectLiteral[] = [] + // parse query and search by it + const labelRegex = /label:"([^"]+)"/g + const labels = Array.from(query.matchAll(labelRegex)).map( + (match) => match[1] + ) - const searchQuery = parseSearchQuery(query) - - // build query string and save parameters - const queryString = buildQueryString(searchQuery, parameters) - - if (queryString) { - // add where clause from query string - queryBuilder - .innerJoinAndSelect('highlight.labels', 'label') - .andWhere(`(${queryString})`) - .setParameters(paramtersToObject(parameters)) - } + labels.forEach((label, index) => { + const alias = `label_${index}` + queryBuilder.innerJoin( + 'highlight.labels', + alias, + `LOWER(${alias}.name) = LOWER(:${alias})`, + { + [alias]: label, + } + ) + }) } return queryBuilder.getMany() diff --git a/packages/api/src/services/labels.ts b/packages/api/src/services/labels.ts index e3400520e..9e47ea65f 100644 --- a/packages/api/src/services/labels.ts +++ b/packages/api/src/services/labels.ts @@ -39,6 +39,23 @@ export const batchGetLabelsFromLibraryItemIds = async ( ) } +export const batchGetLabelsFromHighlightIds = async ( + highlightIds: readonly string[] +): Promise => { + const labels = await authTrx(async (tx) => + tx.getRepository(EntityLabel).find({ + where: { highlightId: In(highlightIds as string[]) }, + relations: ['label'], + }) + ) + + return highlightIds.map((highlightId) => + labels + .filter((label) => label.highlightId === highlightId) + .map((label) => label.label) + ) +} + export const findOrCreateLabels = async ( labels: CreateLabelInput[], userId: string diff --git a/packages/api/test/resolvers/highlight.test.ts b/packages/api/test/resolvers/highlight.test.ts index dc0a2c08e..34cc811eb 100644 --- a/packages/api/test/resolvers/highlight.test.ts +++ b/packages/api/test/resolvers/highlight.test.ts @@ -463,7 +463,7 @@ describe('Highlights API', () => { await saveLabelsInHighlight([label1], existingHighlights[1].id, user.id) const res = await graphqlRequest(query, authToken, { - query: `label:${labelName},${labelName1}`, + query: `label:"${labelName}" label:"${labelName1}"`, }).expect(200) const highlights = res.body.data.highlights.edges as Array expect(highlights).to.have.lengthOf(2) From e9f9f5ddedcba01ec3bc088b8daeeada32bb816c Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 6 Jun 2024 17:33:42 +0800 Subject: [PATCH 8/8] add index for user_id column on highlight table --- packages/api/test/resolvers/highlight.test.ts | 13 ++++++++----- .../0178.do.add_index_on_highlight_user_id.sql | 5 +++++ .../0178.undo.add_index_on_highlight_user_id.sql | 9 +++++++++ 3 files changed, 22 insertions(+), 5 deletions(-) create mode 100755 packages/db/migrations/0178.do.add_index_on_highlight_user_id.sql create mode 100755 packages/db/migrations/0178.undo.add_index_on_highlight_user_id.sql diff --git a/packages/api/test/resolvers/highlight.test.ts b/packages/api/test/resolvers/highlight.test.ts index 34cc811eb..b51e93b37 100644 --- a/packages/api/test/resolvers/highlight.test.ts +++ b/packages/api/test/resolvers/highlight.test.ts @@ -459,16 +459,19 @@ describe('Highlights API', () => { const label1 = await createLabel(labelName1, '#ff0001', user.id) // save labels in highlights - await saveLabelsInHighlight([label], existingHighlights[0].id, user.id) - await saveLabelsInHighlight([label1], existingHighlights[1].id, user.id) + await saveLabelsInHighlight( + [label, label1], + existingHighlights[0].id, + user.id + ) const res = await graphqlRequest(query, authToken, { query: `label:"${labelName}" label:"${labelName1}"`, }).expect(200) const highlights = res.body.data.highlights.edges as Array - expect(highlights).to.have.lengthOf(2) - expect(highlights[1].node.labels?.[0].name).to.eq(labelName) - expect(highlights[0].node.labels?.[0].name).to.eq(labelName1) + expect(highlights).to.have.lengthOf(1) + expect(highlights[0].node.labels?.[0].name).to.eq(labelName) + expect(highlights[0].node.labels?.[1].name).to.eq(labelName1) await deleteLabels([label.id, label1.id], user.id) }) diff --git a/packages/db/migrations/0178.do.add_index_on_highlight_user_id.sql b/packages/db/migrations/0178.do.add_index_on_highlight_user_id.sql new file mode 100755 index 000000000..b88fc8111 --- /dev/null +++ b/packages/db/migrations/0178.do.add_index_on_highlight_user_id.sql @@ -0,0 +1,5 @@ +-- Type: DO +-- Name: add_index_on_highlight_user_id +-- Description: Add index on user_id column to the highlight table + +CREATE INDEX CONCURRENTLY IF NOT EXISTS highlight_user_id_idx ON omnivore.highlight (user_id); diff --git a/packages/db/migrations/0178.undo.add_index_on_highlight_user_id.sql b/packages/db/migrations/0178.undo.add_index_on_highlight_user_id.sql new file mode 100755 index 000000000..387c61d6f --- /dev/null +++ b/packages/db/migrations/0178.undo.add_index_on_highlight_user_id.sql @@ -0,0 +1,9 @@ +-- Type: UNDO +-- Name: add_index_on_highlight_user_id +-- Description: Add index on user_id column to the highlight table + +BEGIN; + +DROP INDEX IF EXISTS highlight_user_id_idx; + +COMMIT;