From bce7afde50d3742d4a49255295215382adbfb6da Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 24 Apr 2023 12:53:40 +0800 Subject: [PATCH 1/3] Eagerly load profile of the user --- packages/api/src/entity/user.ts | 2 +- packages/api/src/resolvers/save/index.ts | 13 ++++++++----- .../src/services/create_page_save_request.ts | 19 +++++++++++-------- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/packages/api/src/entity/user.ts b/packages/api/src/entity/user.ts index b03c36e11..60859dc91 100644 --- a/packages/api/src/entity/user.ts +++ b/packages/api/src/entity/user.ts @@ -40,7 +40,7 @@ export class User { @OneToMany(() => NewsletterEmail, (newsletterEmail) => newsletterEmail.user) newsletterEmails?: NewsletterEmail[] - @OneToOne(() => Profile, (profile) => profile.user) + @OneToOne(() => Profile, (profile) => profile.user, { eager: true }) profile!: Profile @Column('varchar', { length: 255, nullable: true }) diff --git a/packages/api/src/resolvers/save/index.ts b/packages/api/src/resolvers/save/index.ts index ea6960049..d5c2abb29 100644 --- a/packages/api/src/resolvers/save/index.ts +++ b/packages/api/src/resolvers/save/index.ts @@ -1,3 +1,6 @@ +import { User } from '../../entity/user' +import { getRepository } from '../../entity/utils' +import { env } from '../../env' import { MutationSaveFileArgs, MutationSavePageArgs, @@ -6,12 +9,11 @@ import { SaveErrorCode, SaveSuccess, } from '../../generated/graphql' +import { saveFile } from '../../services/save_file' import { savePage } from '../../services/save_page' import { saveUrl } from '../../services/save_url' -import { saveFile } from '../../services/save_file' -import { authorized, userDataToUser } from '../../utils/helpers' import { analytics } from '../../utils/analytics' -import { env } from '../../env' +import { authorized, userDataToUser } from '../../utils/helpers' export const savePageResolver = authorized< SaveSuccess, @@ -51,7 +53,6 @@ export const saveUrlResolver = authorized< MutationSaveUrlArgs >(async (_, { input }, ctx) => { const { - models, claims: { uid }, } = ctx @@ -66,7 +67,9 @@ export const saveUrlResolver = authorized< }, }) - const user = userDataToUser(await models.user.get(uid)) + const user = await getRepository(User).findOneBy({ + id: uid, + }) if (!user) { return { errorCodes: [SaveErrorCode.Unauthorized] } } diff --git a/packages/api/src/services/create_page_save_request.ts b/packages/api/src/services/create_page_save_request.ts index 6285c1d81..4aec90219 100644 --- a/packages/api/src/services/create_page_save_request.ts +++ b/packages/api/src/services/create_page_save_request.ts @@ -26,6 +26,7 @@ interface PageSaveRequest { archivedAt?: Date | null labels?: Label[] priority?: 'low' | 'high' + user?: User | null } const SAVING_CONTENT = 'Your link is being saved...' @@ -76,6 +77,7 @@ export const createPageSaveRequest = async ({ archivedAt, priority, labels, + user, }: PageSaveRequest): Promise => { try { validateUrl(url) @@ -85,16 +87,17 @@ export const createPageSaveRequest = async ({ errorCode: CreateArticleSavingRequestErrorCode.BadData, }) } - - const user = await getRepository(User).findOne({ - where: { id: userId }, - relations: ['profile'], - }) + // if user is not specified, get it from the database if (!user) { - console.log('User not found', userId) - return Promise.reject({ - errorCode: CreateArticleSavingRequestErrorCode.BadData, + user = await getRepository(User).findOneBy({ + id: userId, }) + if (!user) { + console.log('User not found', userId) + return Promise.reject({ + errorCode: CreateArticleSavingRequestErrorCode.BadData, + }) + } } // get priority by checking rate limit if not specified From 69968e52888a18fea965c5343b8653acea3f0ecd Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 24 Apr 2023 12:54:23 +0800 Subject: [PATCH 2/3] Save url if the email subject is a parsable url --- packages/api/src/routers/svc/newsletters.ts | 45 +++++++++++++------ .../api/src/services/save_newsletter_email.ts | 4 -- packages/api/src/services/save_url.ts | 44 ++++++++++++++---- packages/api/src/utils/helpers.ts | 9 ++++ 4 files changed, 76 insertions(+), 26 deletions(-) diff --git a/packages/api/src/routers/svc/newsletters.ts b/packages/api/src/routers/svc/newsletters.ts index d8e81f639..cc67a935a 100644 --- a/packages/api/src/routers/svc/newsletters.ts +++ b/packages/api/src/routers/svc/newsletters.ts @@ -1,14 +1,19 @@ import express from 'express' -import { readPushSubscription } from '../../datalayer/pubsub' +import { + createPubSubClient, + readPushSubscription, +} from '../../datalayer/pubsub' import { getNewsletterEmail, updateConfirmationCode, } from '../../services/newsletters' +import { updateReceivedEmail } from '../../services/received_emails' import { NewsletterMessage, saveNewsletterEmail, } from '../../services/save_newsletter_email' -import { updateReceivedEmail } from '../../services/received_emails' +import { saveUrlFromEmail } from '../../services/save_url' +import { isUrl } from '../../utils/helpers' interface SetConfirmationCodeMessage { emailAddress: string @@ -102,27 +107,41 @@ export function newsletterServiceRouter() { const newsletterEmail = await getNewsletterEmail(data.email) if (!newsletterEmail) { console.log('newsletter email not found', data.email) - return false + return res.status(200).send('Not Found') } - const result = await saveNewsletterEmail(data, newsletterEmail) - if (!result) { - console.log( - 'Error creating newsletter link from data', - data.email, + const saveCtx = { + pubsub: createPubSubClient(), + uid: newsletterEmail.user.id, + } + if (isUrl(data.title)) { + // save url if the title is a parsable url + const result = await saveUrlFromEmail( + saveCtx, data.title, - data.author + data.receivedEmailId ) + if (!result) { + return res.status(500).send('Error saving url from email') + } + } else { + // save newsletter instead + const result = await saveNewsletterEmail(data, newsletterEmail, saveCtx) + if (!result) { + console.log( + 'Error creating newsletter link from data', + data.email, + data.title, + data.author + ) - res.status(500).send('Error creating newsletter link') - return + return res.status(500).send('Error creating newsletter link') + } } // update received email type await updateReceivedEmail(data.receivedEmailId, 'article') - // We always send 200 if it was a valid message - // because we don't want the res.status(200).send('newsletter created') } catch (e) { console.log(e) diff --git a/packages/api/src/services/save_newsletter_email.ts b/packages/api/src/services/save_newsletter_email.ts index fdba98824..fd558ae14 100644 --- a/packages/api/src/services/save_newsletter_email.ts +++ b/packages/api/src/services/save_newsletter_email.ts @@ -10,7 +10,6 @@ import { analytics } from '../utils/analytics' import { isBase64Image } from '../utils/helpers' import { fetchFavicon } from '../utils/parser' import { addLabelToPage } from './labels' -import { updateReceivedEmail } from './received_emails' import { SaveContext, saveEmail, SaveEmailInput } from './save_email' import { saveSubscription } from './subscriptions' @@ -63,9 +62,6 @@ export const saveNewsletterEmail = async ( return false } - // update received email type - await updateReceivedEmail(data.receivedEmailId, 'article') - if (!page.siteIcon || isBase64Image(page.siteIcon)) { // fetch favicon if not already set or is a base64 image const favicon = await fetchFavicon(page.url) diff --git a/packages/api/src/services/save_url.ts b/packages/api/src/services/save_url.ts index 7b5695285..93e981d8c 100644 --- a/packages/api/src/services/save_url.ts +++ b/packages/api/src/services/save_url.ts @@ -1,20 +1,20 @@ import { PubsubClient } from '../datalayer/pubsub' -import { UserData } from '../datalayer/user/model' +import { ArticleSavingRequestStatus } from '../elastic/types' +import { User } from '../entity/user' +import { getRepository } from '../entity/utils' import { homePageURL } from '../env' import { SaveErrorCode, SaveResult, SaveUrlInput } from '../generated/graphql' -import { DataModels } from '../resolvers/types' import { createPageSaveRequest } from './create_page_save_request' -import { ArticleSavingRequestStatus } from '../elastic/types' import { createLabels } from './labels' -type SaveContext = { +interface SaveContext { pubsub: PubsubClient - models: DataModels + uid: string } export const saveUrl = async ( ctx: SaveContext, - saver: UserData, + user: User, input: SaveUrlInput ): Promise => { try { @@ -23,28 +23,54 @@ export const saveUrl = async ( input.state === ArticleSavingRequestStatus.Archived ? new Date() : null // add labels to page const labels = input.labels - ? await createLabels({ ...ctx, uid: saver.id }, input.labels) + ? await createLabels({ ...ctx, uid: ctx.uid }, input.labels) : undefined const pageSaveRequest = await createPageSaveRequest({ - userId: saver.id, + userId: ctx.uid, url: input.url, pubsub: ctx.pubsub, articleSavingRequestId: input.clientRequestId, archivedAt, labels, + user, }) return { clientRequestId: pageSaveRequest.id, - url: `${homePageURL()}/${saver.profile.username}/links/${ + url: `${homePageURL()}/${user.profile.username}/links/${ pageSaveRequest.id }`, } } catch (error) { console.log('error enqueuing request', error) return { + __typename: 'SaveError', errorCodes: [SaveErrorCode.Unknown], } } } + +export const saveUrlFromEmail = async ( + ctx: SaveContext, + url: string, + clientRequestId: string +): Promise => { + const user = await getRepository(User).findOneBy({ + id: ctx.uid, + }) + if (!user) { + return false + } + + const result = await saveUrl(ctx, user, { + url, + clientRequestId, + source: 'email', + }) + if (result.__typename === 'SaveError') { + return false + } + + return true +} diff --git a/packages/api/src/utils/helpers.ts b/packages/api/src/utils/helpers.ts index ecf51afc0..0f0861c99 100644 --- a/packages/api/src/utils/helpers.ts +++ b/packages/api/src/utils/helpers.ts @@ -289,3 +289,12 @@ export const generateRandomColor = (): string => { export const unescapeHtml = (html: string): string => { return _.unescape(html) } + +export const isUrl = (str: string): boolean => { + try { + new URL(str) + return true + } catch { + return false + } +} From e512ba0badf7d3bcd9387e03e8c95fc4883bd7d5 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Mon, 24 Apr 2023 18:04:18 +0800 Subject: [PATCH 3/3] Fix tests --- packages/api/src/resolvers/recent_emails/index.ts | 15 +++++++++------ packages/api/test/services/create_user.test.ts | 14 ++++++++++---- .../test/services/save_newsletter_email.test.ts | 6 ------ 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/packages/api/src/resolvers/recent_emails/index.ts b/packages/api/src/resolvers/recent_emails/index.ts index b8438c9cf..b3c28e8aa 100644 --- a/packages/api/src/resolvers/recent_emails/index.ts +++ b/packages/api/src/resolvers/recent_emails/index.ts @@ -1,3 +1,8 @@ +import { ILike } from 'typeorm' +import { NewsletterEmail } from '../../entity/newsletter_email' +import { ReceivedEmail } from '../../entity/received_email' +import { getRepository } from '../../entity/utils' +import { env } from '../../env' import { MarkEmailAsItemError, MarkEmailAsItemErrorCode, @@ -7,15 +12,11 @@ import { RecentEmailsErrorCode, RecentEmailsSuccess, } from '../../generated/graphql' -import { authorized } from '../../utils/helpers' -import { getRepository } from '../../entity/utils' -import { ReceivedEmail } from '../../entity/received_email' +import { updateReceivedEmail } from '../../services/received_emails' import { saveNewsletterEmail } from '../../services/save_newsletter_email' -import { NewsletterEmail } from '../../entity/newsletter_email' +import { authorized } from '../../utils/helpers' import { generateUniqueUrl, parseEmailAddress } from '../../utils/parser' import { sendEmail } from '../../utils/sendEmail' -import { env } from '../../env' -import { ILike } from 'typeorm' export const recentEmailsResolver = authorized< RecentEmailsSuccess, @@ -114,6 +115,8 @@ export const markEmailAsItemResolver = authorized< }, newsletterEmail ) + // update received email type + await updateReceivedEmail(recentEmail.id, 'article') const text = `A recent email marked as a library item by: ${claims.uid} diff --git a/packages/api/test/services/create_user.test.ts b/packages/api/test/services/create_user.test.ts index 1aa4da1ff..4458723c9 100644 --- a/packages/api/test/services/create_user.test.ts +++ b/packages/api/test/services/create_user.test.ts @@ -40,16 +40,22 @@ describe('create user', () => { const testUser = 'testuser' const adminUser = await createTestUser(testOwner) + const admninIds = [adminUser.id] const [, invite] = await createGroup({ admin: adminUser, name: 'testgroup', }) const user = await createTestUser(testUser, invite.code) + const userIds = [user.id] - expect(await getUserFollowers(user)).to.eql([adminUser]) - expect(await getUserFollowing(user)).to.eql([adminUser]) - expect(await getUserFollowers(adminUser)).to.eql([user]) - expect(await getUserFollowing(adminUser)).to.eql([user]) + const userFollowers = await getUserFollowers(user) + const userFollowing = await getUserFollowing(user) + const adminUserFollowers = await getUserFollowers(adminUser) + const adminUserFollowing = await getUserFollowing(adminUser) + expect(userFollowers.map(u => u.id)).to.eql(admninIds) + expect(userFollowing.map(u => u.id)).to.eql(admninIds) + expect(adminUserFollowers.map(u => u.id)).to.eql(userIds) + expect(adminUserFollowing.map(u => u.id)).to.eql(userIds) }) it('creates profile when user exists but profile not', async () => { diff --git a/packages/api/test/services/save_newsletter_email.test.ts b/packages/api/test/services/save_newsletter_email.test.ts index 8d9d28a2c..28df95624 100644 --- a/packages/api/test/services/save_newsletter_email.test.ts +++ b/packages/api/test/services/save_newsletter_email.test.ts @@ -80,12 +80,6 @@ describe('saveNewsletterEmail', () => { newsletterEmail: { id: newsletterEmail.id }, }) expect(subscriptions).not.to.be.empty - - // check if the received email was updated - const updatedReceivedEmail = await getRepository(ReceivedEmail).findOneBy({ - id: receivedEmail.id, - }) - expect(updatedReceivedEmail?.type).to.equal('article') }) it('adds a Newsletter label to that page', async () => {