From c99c1db57e944d4e130dc7bee5b59aa146f1ef75 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 28 Jul 2022 10:26:15 +0800 Subject: [PATCH 1/4] Add support to the case when from address is in Name
format --- packages/api/src/routers/svc/emails.ts | 8 +++++--- packages/api/src/utils/parser.ts | 12 ++++++++++++ packages/api/test/db.ts | 6 +++--- .../api/test/resolvers/newsletters.test.ts | 4 ++-- packages/api/test/routers/auth.test.ts | 18 +++++------------- .../api/test/routers/pdf_attachments.test.ts | 2 +- packages/api/test/utils/parser.test.ts | 11 +++++++++++ 7 files changed, 39 insertions(+), 22 deletions(-) diff --git a/packages/api/src/routers/svc/emails.ts b/packages/api/src/routers/svc/emails.ts index 0c166cdf6..e81c118ca 100644 --- a/packages/api/src/routers/svc/emails.ts +++ b/packages/api/src/routers/svc/emails.ts @@ -13,6 +13,7 @@ import { getTitleFromEmailSubject, isProbablyArticle, isProbablyNewsletter, + parseEmailAddress, } from '../../utils/parser' import { saveNewsletterEmail } from '../../services/save_newsletter_email' import { saveEmail } from '../../services/save_email' @@ -75,6 +76,7 @@ export function emailsServiceRouter() { } const user = newsletterEmail.user const ctx = { pubsub: createPubSubClient(), uid: user.id } + const parsedFrom = parseEmailAddress(data.from) if (await isProbablyNewsletter(data.html)) { logger.info('handling as newsletter', data) @@ -83,7 +85,7 @@ export function emailsServiceRouter() { email: data.to, title: data.subject, content: data.html, - author: data.from, + author: parsedFrom.name, url: (await findNewsletterUrl(data.html)) || generateUniqueUrl(), unsubMailTo: data.unsubMailTo, unsubHttpUrl: data.unsubHttpUrl, @@ -95,11 +97,11 @@ export function emailsServiceRouter() { return } - if (await isProbablyArticle(data.from, data.subject)) { + if (await isProbablyArticle(parsedFrom.address, data.subject)) { logger.info('handling as article', data) await saveEmail(ctx, { title: getTitleFromEmailSubject(data.subject), - author: data.from, + author: parsedFrom.name, url: generateUniqueUrl(), originalContent: data.html, }) diff --git a/packages/api/src/utils/parser.ts b/packages/api/src/utils/parser.ts index 89fe4b018..abe134b9b 100644 --- a/packages/api/src/utils/parser.ts +++ b/packages/api/src/utils/parser.ts @@ -19,6 +19,7 @@ import { getRepository } from '../entity/utils' import { User } from '../entity/user' import { ILike } from 'typeorm' import { v4 as uuid } from 'uuid' +import addressparser from 'addressparser' const logger = buildLogger('utils.parse') @@ -567,3 +568,14 @@ export const getTitleFromEmailSubject = (subject: string) => { const title = subject.replace(ARTICLE_PREFIX, '') return title.trim() } + +export const parseEmailAddress = (from: string): addressparser.EmailAddress => { + // get author name from email + // e.g. 'Jackson Harper from Omnivore App ' + // or 'Mike Allen ' + const parsed = addressparser(from) + if (parsed.length > 0) { + return parsed[0] + } + return { name: from, address: from } +} diff --git a/packages/api/test/db.ts b/packages/api/test/db.ts index 4af92259b..d1e752c17 100644 --- a/packages/api/test/db.ts +++ b/packages/api/test/db.ts @@ -64,7 +64,7 @@ export const deleteTestUser = async (name: string) => { await AppDataSource.createQueryBuilder() .delete() .from(User) - .where({ email: `${name}@fake.com` }) + .where({ email: `${name}@omnivore.app` }) .execute() } @@ -77,7 +77,7 @@ export const createTestUser = async ( const [newUser] = await createUser({ provider: 'GOOGLE', sourceUserId: 'fake-user-id-' + name, - email: `${name}@fake.com`, + email: `${name}@omnivore.app`, username: name, bio: `i am ${name}`, name: name, @@ -93,7 +93,7 @@ export const createUserWithoutProfile = async (name: string): Promise => { return getRepository(User).save({ source: 'GOOGLE', sourceUserId: 'fake-user-id-' + name, - email: `${name}@fake.com`, + email: `${name}@omnivore.app`, name: name, }) } diff --git a/packages/api/test/resolvers/newsletters.test.ts b/packages/api/test/resolvers/newsletters.test.ts index e82bc003e..465fcc9aa 100644 --- a/packages/api/test/resolvers/newsletters.test.ts +++ b/packages/api/test/resolvers/newsletters.test.ts @@ -28,11 +28,11 @@ describe('Newsletters API', () => { // create test newsletter emails const newsletterEmail1 = await createTestNewsletterEmail( user, - 'Test_email_address_1@fake-email.com' + 'Test_email_address_1@omnivore.app' ) const newsletterEmail2 = await createTestNewsletterEmail( user, - 'Test_email_address_2@fake-email.com' + 'Test_email_address_2@omnivore.app' ) newsletterEmails = [newsletterEmail1, newsletterEmail2] }) diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index b45fe926c..9dfb301ee 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -47,7 +47,7 @@ describe('auth router', () => { before(() => { password = validPassword username = 'Some_username' - email = `${username}@fake.com` + email = `${username}@omnivore.app` name = 'Some name' }) @@ -397,9 +397,7 @@ describe('auth router', () => { it('redirects to forgot-password page with success message', async () => { const res = await emailResetPasswordReq(email).expect(302) - expect(res.header.location).to.endWith( - '/auth/reset-sent' - ) + expect(res.header.location).to.endWith('/auth/reset-sent') }) }) @@ -434,9 +432,7 @@ describe('auth router', () => { it('redirects to email-login page with error code PENDING_VERIFICATION', async () => { const res = await emailResetPasswordReq(email).expect(302) - expect(res.header.location).to.endWith( - '/auth/reset-sent' - ) + expect(res.header.location).to.endWith('/auth/reset-sent') }) }) }) @@ -448,9 +444,7 @@ describe('auth router', () => { it('redirects to forgot-password page with error code USER_NOT_FOUND', async () => { const res = await emailResetPasswordReq(email).expect(302) - expect(res.header.location).to.endWith( - '/auth/reset-sent' - ) + expect(res.header.location).to.endWith('/auth/reset-sent') }) }) }) @@ -501,9 +495,7 @@ describe('auth router', () => { const res = await resetPasswordRequest(token, 'new_password').expect( 302 ) - expect(res.header.location).to.contain( - '/api/client/auth?tok' - ) + expect(res.header.location).to.contain('/api/client/auth?tok') }) it('resets password', async () => { diff --git a/packages/api/test/routers/pdf_attachments.test.ts b/packages/api/test/routers/pdf_attachments.test.ts index 06053be4a..6110ef4e3 100644 --- a/packages/api/test/routers/pdf_attachments.test.ts +++ b/packages/api/test/routers/pdf_attachments.test.ts @@ -12,7 +12,7 @@ import { getPageById } from '../../src/elastic/pages' describe('PDF attachments Router', () => { const username = 'fakeUser' - const newsletterEmail = 'fakeEmail@fake-email.com' + const newsletterEmail = 'fakeEmail@omnivore.app' let user: User let authToken: string diff --git a/packages/api/test/utils/parser.test.ts b/packages/api/test/utils/parser.test.ts index 5c79e8619..9b78d77b4 100644 --- a/packages/api/test/utils/parser.test.ts +++ b/packages/api/test/utils/parser.test.ts @@ -9,6 +9,7 @@ import { getTitleFromEmailSubject, isProbablyArticle, isProbablyNewsletter, + parseEmailAddress, parsePageMetadata, parsePreparedContent, } from '../../src/utils/parser' @@ -179,3 +180,13 @@ describe('getTitleFromEmailSubject', () => { expect(getTitleFromEmailSubject(subject)).to.eql(title) }) }) + +describe('parseEmailAddress', () => { + const email = 'Tester ' + + it('returns the name and address', () => { + const { name, address } = parseEmailAddress(email) + expect(name).to.eql('Tester') + expect(address).to.eql('tester@omnivore.app') + }) +}) From 3a120b8f4751566088ced8e5aa7d5c4d7937610c Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 28 Jul 2022 10:37:28 +0800 Subject: [PATCH 2/4] Return empty name if name not found in from header --- packages/api/src/utils/parser.ts | 2 +- packages/api/test/utils/parser.test.ts | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/api/src/utils/parser.ts b/packages/api/src/utils/parser.ts index abe134b9b..6c2ba1ac3 100644 --- a/packages/api/src/utils/parser.ts +++ b/packages/api/src/utils/parser.ts @@ -577,5 +577,5 @@ export const parseEmailAddress = (from: string): addressparser.EmailAddress => { if (parsed.length > 0) { return parsed[0] } - return { name: from, address: from } + return { name: '', address: from } } diff --git a/packages/api/test/utils/parser.test.ts b/packages/api/test/utils/parser.test.ts index 9b78d77b4..83bd23294 100644 --- a/packages/api/test/utils/parser.test.ts +++ b/packages/api/test/utils/parser.test.ts @@ -182,11 +182,18 @@ describe('getTitleFromEmailSubject', () => { }) describe('parseEmailAddress', () => { - const email = 'Tester ' + it('returns the name and address when in name
format', () => { + const name = 'test name' + const address = 'tester@omnivore.app' + const parsed = parseEmailAddress(`${name} <${address}>`) + expect(parsed.name).to.eql(name) + expect(parsed.address).to.eql(address) + }) - it('returns the name and address', () => { - const { name, address } = parseEmailAddress(email) - expect(name).to.eql('Tester') - expect(address).to.eql('tester@omnivore.app') + it('returns the address when in address format', () => { + const address = 'tester@omnivore.app' + const parsed = parseEmailAddress(address) + expect(parsed.name).to.eql('') + expect(parsed.address).to.eql(address) }) }) From ec9247bc8af47156cc61def4f19ce8ef7170976b Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 28 Jul 2022 10:44:44 +0800 Subject: [PATCH 3/4] Add addressparser dependency --- packages/api/package.json | 1 + yarn.lock | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/api/package.json b/packages/api/package.json index 2d528d197..dbb73ca3d 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -36,6 +36,7 @@ "@sentry/integrations": "^6.19.1", "@sentry/node": "^5.26.0", "@sentry/tracing": "^5.26.0", + "addressparser": "^1.0.1", "analytics-node": "^6.0.0", "apollo-datasource": "^3.3.1", "apollo-server-express": "^3.6.3", diff --git a/yarn.lock b/yarn.lock index 8334e2744..963a999da 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8682,7 +8682,7 @@ address@^1.0.1: addressparser@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/addressparser/-/addressparser-1.0.1.tgz#47afbe1a2a9262191db6838e4fd1d39b40821746" - integrity sha1-R6++GiqSYhkdtoOOT9HTm0CCF0Y= + integrity sha512-aQX7AISOMM7HFE0iZ3+YnD07oIeJqWGVnJ+ZIKaBZAk03ftmVYVqsGas/rbXKR21n4D/hKCSHypvcyOkds/xzg== agent-base@6: version "6.0.1" From 469910f2bd8763cf04356092c662ecb06b06d3ca Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 28 Jul 2022 11:37:14 +0800 Subject: [PATCH 4/4] Add addressparser types dependency --- packages/api/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/api/package.json b/packages/api/package.json index dbb73ca3d..258ae3310 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -88,6 +88,7 @@ "devDependencies": { "@babel/register": "^7.14.5", "@istanbuljs/nyc-config-typescript": "^1.0.2", + "@types/addressparser": "^1.0.1", "@types/analytics-node": "^3.1.7", "@types/bcryptjs": "^2.4.2", "@types/chai": "^4.2.18",