Merge pull request #1021 from omnivore-app/feature/save-from-email

Save articles by sending emails to omnivore inbox

packages/api/src/routers/svc/emails.ts

@@ -1,12 +1,22 @@
 import express from 'express'
-import { readPushSubscription } from '../../datalayer/pubsub'
+import {
+  createPubSubClient,
+  readPushSubscription,
+} from '../../datalayer/pubsub'
 import { sendEmail } from '../../utils/sendEmail'
 import { analytics } from '../../utils/analytics'
 import { getNewsletterEmail } from '../../services/newsletters'
 import { env } from '../../env'
-import { v4 as uuid } from 'uuid'
-import { findNewsletterUrl, isProbablyNewsletter } from '../../utils/parser'
+import {
+  findNewsletterUrl,
+  generateUniqueUrl,
+  getTitleFromEmailSubject,
+  isProbablyArticle,
+  isProbablyNewsletter,
+} from '../../utils/parser'
 import { saveNewsletterEmail } from '../../services/save_newsletter_email'
+import { saveEmail } from '../../services/save_email'
+import { buildLogger } from '../../utils/logger'
 
 interface ForwardEmailMessage {
   from: string
@@ -17,15 +27,17 @@ interface ForwardEmailMessage {
   unsubHttpUrl?: string
 }
 
+const logger = buildLogger('app.dispatch')
+
 export function emailsServiceRouter() {
   const router = express.Router()
 
   // eslint-disable-next-line @typescript-eslint/no-misused-promises
   router.post('/forward', async (req, res) => {
-    console.log('forward')
+    logger.info('email forward router')
 
     const { message, expired } = readPushSubscription(req)
-    console.log('pubsub message:', message, 'expired:', expired)
+    logger.info('pubsub message:', message, 'expired:', expired)
 
     if (!message) {
       res.status(400).send('Bad Request')
@@ -33,7 +45,7 @@ export function emailsServiceRouter() {
     }
 
     if (expired) {
-      console.log('discards expired message:', message)
+      logger.log('discards expired message:', message)
       res.status(200).send('Expired')
       return
     }
@@ -48,39 +60,55 @@ export function emailsServiceRouter() {
         !('subject' in data) ||
         !('html' in data)
       ) {
-        console.log('Invalid message')
+        logger.info('Invalid message')
         res.status(400).send('Bad Request')
         return
       }
 
-      if (await isProbablyNewsletter(data.html)) {
-        console.log('handling as newsletter', data)
-        await saveNewsletterEmail({
-          email: data.to,
-          title: data.subject,
-          content: data.html,
-          author: data.from,
-          url:
-            (await findNewsletterUrl(data.html)) ||
-            'https://omnivore.app/no_url?q' + uuid(),
-          unsubMailTo: data.unsubMailTo,
-          unsubHttpUrl: data.unsubHttpUrl,
-        })
-        res.status(200).send('Newsletter')
-        return
-      }
-
       // get user from newsletter email
       const newsletterEmail = await getNewsletterEmail(data.to)
 
       if (!newsletterEmail) {
-        console.log('newsletter email not found', data.to)
+        logger.info('newsletter email not found', data.to)
         res.status(200).send('Not Found')
         return
       }
+      const user = newsletterEmail.user
+      const ctx = { pubsub: createPubSubClient(), uid: user.id }
+
+      if (await isProbablyNewsletter(data.html)) {
+        logger.info('handling as newsletter', data)
+        await saveNewsletterEmail(
+          {
+            email: data.to,
+            title: data.subject,
+            content: data.html,
+            author: data.from,
+            url: (await findNewsletterUrl(data.html)) || generateUniqueUrl(),
+            unsubMailTo: data.unsubMailTo,
+            unsubHttpUrl: data.unsubHttpUrl,
+            newsletterEmail,
+          },
+          ctx
+        )
+        res.status(200).send('Newsletter')
+        return
+      }
+
+      if (await isProbablyArticle(data.from, data.subject)) {
+        logger.info('handling as article', data)
+        await saveEmail(ctx, {
+          title: getTitleFromEmailSubject(data.subject),
+          author: data.from,
+          url: generateUniqueUrl(),
+          originalContent: data.html,
+        })
+        res.status(200).send('Article')
+        return
+      }
 
       analytics.track({
-        userId: newsletterEmail.user.id,
+        userId: user.id,
         event: 'non_newsletter_email_received',
         properties: {
           env: env.server.apiEnv,
@@ -90,21 +118,21 @@ export function emailsServiceRouter() {
       // forward non-newsletter emails to the registered email address
       const result = await sendEmail({
         from: env.sender.message,
-        to: newsletterEmail.user.email,
+        to: user.email,
         subject: `Fwd: ${data.subject}`,
         html: data.html,
         replyTo: data.from,
       })
 
       if (!result) {
-        console.log('Email not forwarded', data)
+        logger.info('Email not forwarded', data)
         res.status(200).send('Failed to send email')
         return
       }
 
       res.status(200).send('Email forwarded')
     } catch (e) {
-      console.log(e)
+      logger.info(e)
       if (e instanceof SyntaxError) {
         // when message is not a valid json string
         res.status(400).send(e)

packages/api/src/routers/svc/newsletters.ts

@@ -96,7 +96,7 @@ export function newsletterServiceRouter() {
 
       const result = await saveNewsletterEmail(data)
       if (!result) {
-        console.log('Error createing newsletter link from data', data)
+        console.log('Error creating newsletter link from data', data)
         res.status(500).send('Error creating newsletter link')
         return
       }

packages/api/src/services/save_newsletter_email.ts

@@ -11,6 +11,7 @@ import { getDeviceTokensByUserId } from './user_device_tokens'
 import { Page } from '../elastic/types'
 import { addLabelToPage } from './labels'
 import { saveSubscription } from './subscriptions'
+import { NewsletterEmail } from '../entity/newsletter_email'
 
 interface NewsletterMessage {
   email: string
@@ -20,6 +21,7 @@ interface NewsletterMessage {
   author: string
   unsubMailTo?: string
   unsubHttpUrl?: string
+  newsletterEmail?: NewsletterEmail
 }
 
 // Returns true if the link was created successfully. Can still fail to
@@ -29,7 +31,8 @@ export const saveNewsletterEmail = async (
   ctx?: SaveContext
 ): Promise<boolean> => {
   // get user from newsletter email
-  const newsletterEmail = await getNewsletterEmail(data.email)
+  const newsletterEmail =
+    data.newsletterEmail || (await getNewsletterEmail(data.email))
 
   if (!newsletterEmail) {
     console.log('newsletter email not found', data.email)

packages/api/src/utils/parser.ts

@@ -15,6 +15,10 @@ import { GolangHandler } from './golang-handler'
 import * as hljs from 'highlightjs'
 import { decode } from 'html-entities'
 import { parseHTML } from 'linkedom'
+import { getRepository } from '../entity/utils'
+import { User } from '../entity/user'
+import { ILike } from 'typeorm'
+import { v4 as uuid } from 'uuid'
 
 const logger = buildLogger('utils.parse')
 
@@ -37,6 +41,7 @@ const DOM_PURIFY_CONFIG = {
     'data-feature',
   ],
 }
+const ARTICLE_PREFIX = 'omnivore:'
 
 interface ContentHandler {
   shouldPrehandle: (url: URL, dom: Document) => boolean
@@ -545,3 +550,20 @@ export const findNewsletterUrl = async (
 
   return undefined
 }
+
+export const isProbablyArticle = async (
+  email: string,
+  subject: string
+): Promise<boolean> => {
+  const user = await getRepository(User).findOneBy({
+    email: ILike(email),
+  })
+  return !!user || subject.includes(ARTICLE_PREFIX)
+}
+
+export const generateUniqueUrl = () => 'https://omnivore.app/no_url?q=' + uuid()
+
+export const getTitleFromEmailSubject = (subject: string) => {
+  const title = subject.replace(ARTICLE_PREFIX, '')
+  return title.trim()
+}

packages/api/test/routers/auth.test.ts

@@ -1,6 +1,5 @@
 import { createTestUser, deleteTestUser } from '../db'
 import { generateFakeUuid, request } from '../util'
-import { expect } from 'chai'
 import { StatusType } from '../../src/datalayer/user/model'
 import { getRepository } from '../../src/entity/utils'
 import { User } from '../../src/entity/user'
@@ -13,6 +12,10 @@ import {
   generateVerificationToken,
   hashPassword,
 } from '../../src/utils/auth'
+import sinonChai from 'sinon-chai'
+import chai, { expect } from 'chai'
+
+chai.use(sinonChai)
 
 describe('auth router', () => {
   const route = '/api/auth'

packages/api/test/routers/emails.test.ts

@@ -0,0 +1,131 @@
+import {
+  createTestNewsletterEmail,
+  createTestUser,
+  deleteTestUser,
+} from '../db'
+import { User } from '../../src/entity/user'
+import 'mocha'
+import sinon from 'sinon'
+import { expect } from 'chai'
+import { request } from '../util'
+import * as parser from '../../src/utils/parser'
+import * as sendNotification from '../../src/utils/sendNotification'
+import * as sendEmail from '../../src/utils/sendEmail'
+
+describe('Emails Router', () => {
+  const username = 'fakeUser'
+  const newsletterEmail = 'fakeUser@omnivore.app'
+
+  let user: User
+  let token: string
+
+  before(async () => {
+    // create test user and login
+    user = await createTestUser(username)
+
+    await createTestNewsletterEmail(user, newsletterEmail)
+    token = process.env.PUBSUB_VERIFICATION_TOKEN!
+  })
+
+  after(async () => {
+    // clean up
+    await deleteTestUser(username)
+    sinon.restore()
+  })
+
+  describe('forward', () => {
+    const from = 'from@omnivore.app'
+    const to = newsletterEmail
+    const subject = 'test subject'
+    const html = 'test html'
+
+    beforeEach(async () => {
+      sinon.replace(
+        sendNotification,
+        'sendMulticastPushNotifications',
+        sinon.fake.resolves(undefined)
+      )
+      sinon.replace(sendEmail, 'sendEmail', sinon.fake.resolves(true))
+    })
+
+    afterEach(() => {
+      sinon.restore()
+    })
+
+    context('when email is a newsletter', () => {
+      before(() => {
+        sinon.replace(parser, 'isProbablyNewsletter', sinon.fake.resolves(true))
+      })
+
+      it('saves the email as a newsletter', async () => {
+        const data = {
+          message: {
+            data: Buffer.from(
+              JSON.stringify({ from, to, subject, html })
+            ).toString('base64'),
+            publishTime: new Date().toISOString(),
+          },
+        }
+        const res = await request
+          .post(`/svc/pubsub/emails/forward?token=${token}`)
+          .send(data)
+          .expect(200)
+        expect(res.text).to.eql('Newsletter')
+      })
+    })
+
+    context('when email is an article', () => {
+      before(() => {
+        sinon.replace(
+          parser,
+          'isProbablyNewsletter',
+          sinon.fake.resolves(false)
+        )
+        sinon.replace(parser, 'isProbablyArticle', sinon.fake.resolves(true))
+      })
+
+      it('saves the email as an article', async () => {
+        const data = {
+          message: {
+            data: Buffer.from(
+              JSON.stringify({ from, to, subject, html })
+            ).toString('base64'),
+            publishTime: new Date().toISOString(),
+          },
+        }
+        const res = await request
+          .post(`/svc/pubsub/emails/forward?token=${token}`)
+          .send(data)
+          .expect(200)
+        expect(res.text).to.eql('Article')
+      })
+    })
+
+    context('when email is a regular email', () => {
+      before(() => {
+        sinon.replace(
+          parser,
+          'isProbablyNewsletter',
+          sinon.fake.resolves(false)
+        )
+        sinon.replace(parser, 'isProbablyArticle', sinon.fake.resolves(false))
+      })
+
+      it('forwards the email', async () => {
+        const data = {
+          message: {
+            data: Buffer.from(
+              JSON.stringify({ from, to, subject, html })
+            ).toString('base64'),
+            publishTime: new Date().toISOString(),
+          },
+        }
+        const res = await request
+          .post(`/svc/pubsub/emails/forward?token=${token}`)
+          .send(data)
+          .expect(200)
+        expect(res.text).to.eql('Email forwarded')
+      })
+    })
+  })
+})

packages/api/test/utils/parser.test.ts

@@ -5,12 +5,17 @@ import 'chai/register-should'
 import fs from 'fs'
 import {
   findNewsletterUrl,
+  generateUniqueUrl,
+  getTitleFromEmailSubject,
+  isProbablyArticle,
   isProbablyNewsletter,
   parsePageMetadata,
   parsePreparedContent,
 } from '../../src/utils/parser'
 import nock from 'nock'
 import chaiAsPromised from 'chai-as-promised'
+import { User } from '../../src/entity/user'
+import { createTestUser, deleteTestUser } from '../db'
 
 chai.use(chaiAsPromised)
 
@@ -135,3 +140,42 @@ describe('parsePreparedContent', async () => {
     )
   })
 })
+
+describe('isProbablyArticle', () => {
+  let user: User
+
+  before(async () => {
+    user = await createTestUser('fakeUser')
+  })
+
+  after(async () => {
+    await deleteTestUser(user.name)
+  })
+
+  it('returns true when email is signed up with us', async () => {
+    const email = user.email
+    expect(await isProbablyArticle(email, 'test subject')).to.be.true
+  })
+
+  it('returns true when subject has omnivore: prefix', async () => {
+    const subject = 'omnivore: test subject'
+    expect(await isProbablyArticle('test-email', subject)).to.be.true
+  })
+})
+
+describe('generateUniqueUrl', () => {
+  it('generates a unique URL', () => {
+    const url1 = generateUniqueUrl()
+    const url2 = generateUniqueUrl()
+
+    expect(url1).to.not.eql(url2)
+  })
+})
+
+describe('getTitleFromEmailSubject', () => {
+  it('returns the title from the email subject', () => {
+    const title = 'test subject'
+    const subject = `omnivore: ${title}`
+    expect(getTitleFromEmailSubject(subject)).to.eql(title)
+  })
+})

packages/inbound-email-handler/src/index.ts

@@ -70,10 +70,10 @@ export const inboundEmailHandler = Sentry.GCPFunction.wrapHttpFunction(
       const html = parsed.html
       const text = parsed.text
 
-      const forwardedAddress = headers['x-forwarded-to']
-      const recipientAddress = forwardedAddress?.toString() || parsed.to
+      const forwardedAddress = headers['x-forwarded-to']?.toString()
+      const recipientAddress = forwardedAddress || parsed.to
       const postHeader = headers['list-post']?.toString()
-      const unSubHeader = headers['list-unsubscribe'].toString()
+      const unSubHeader = headers['list-unsubscribe']?.toString()
 
       try {
         // check if it is a forwarding confirmation email or newsletter
@@ -93,36 +93,43 @@ export const inboundEmailHandler = Sentry.GCPFunction.wrapHttpFunction(
             from,
             unSubHeader
           )
-        } else {
-          console.log('non-newsletter email from:', from, recipientAddress)
-
-          if (isConfirmationEmail(from)) {
-            console.log('handleConfirmation', from)
-            await handleConfirmation(recipientAddress, subject)
-          } else if (pdfAttachment) {
-            console.log('handle PDF attachment', from, recipientAddress)
-            await handlePdfAttachment(
-              recipientAddress,
-              pdfAttachmentName,
-              pdfAttachment,
-              subject
-            )
-          }
-
-          const unsubscribe = parseUnsubscribe(unSubHeader)
-          // queue non-newsletter emails
-          await pubsub.topic(NON_NEWSLETTER_EMAIL_TOPIC).publishMessage({
-            json: {
-              from: from,
-              to: recipientAddress,
-              subject: subject,
-              html: html,
-              text: text,
-              unsubMailTo: unsubscribe.mailTo,
-              unsubHttpUrl: unsubscribe.httpUrl,
-            },
-          })
+          return res.send('ok')
         }
+
+        console.log('non-newsletter email from:', from, recipientAddress)
+
+        if (isConfirmationEmail(from)) {
+          console.log('handleConfirmation', from)
+          await handleConfirmation(recipientAddress, subject)
+          return res.send('ok')
+        }
+
+        if (pdfAttachment) {
+          console.log('handle PDF attachment', from, recipientAddress)
+          await handlePdfAttachment(
+            recipientAddress,
+            pdfAttachmentName,
+            pdfAttachment,
+            subject
+          )
+          return res.send('ok')
+        }
+
+        const unsubscribe = parseUnsubscribe(unSubHeader)
+        // queue non-newsletter emails
+        await pubsub.topic(NON_NEWSLETTER_EMAIL_TOPIC).publishMessage({
+          json: {
+            from,
+            to: recipientAddress,
+            subject,
+            html,
+            text,
+            unsubMailTo: unsubscribe.mailTo,
+            unsubHttpUrl: unsubscribe.httpUrl,
+          },
+        })
+
+        res.send('ok')
       } catch (error) {
         console.log(
           'error handling emails, will forward.',
@@ -133,16 +140,14 @@ export const inboundEmailHandler = Sentry.GCPFunction.wrapHttpFunction(
         // queue error emails
         await pubsub.topic(NON_NEWSLETTER_EMAIL_TOPIC).publishMessage({
           json: {
-            from: from,
+            from,
             to: recipientAddress,
-            subject: subject,
-            html: html,
-            text: text,
+            subject,
+            html,
+            text,
           },
         })
       }
-
-      res.send('ok')
     } catch (e) {
       console.log(e)
       res.send(e)