From f28912728a39c9807fe3da4312d459494368079f Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Tue, 11 Apr 2023 22:39:47 +0800 Subject: [PATCH 1/2] creates or updates subscription only if their is a valid unsubscribe link --- .../api/src/services/save_newsletter_email.ts | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/packages/api/src/services/save_newsletter_email.ts b/packages/api/src/services/save_newsletter_email.ts index 81e604d2c..fdba98824 100644 --- a/packages/api/src/services/save_newsletter_email.ts +++ b/packages/api/src/services/save_newsletter_email.ts @@ -1,18 +1,18 @@ import { MulticastMessage } from 'firebase-admin/messaging' import { createPubSubClient } from '../datalayer/pubsub' +import { updatePage } from '../elastic/pages' +import { Page } from '../elastic/types' +import { NewsletterEmail } from '../entity/newsletter_email' import { UserDeviceToken } from '../entity/user_device_tokens' import { env } from '../env' import { ContentReader } from '../generated/graphql' import { analytics } from '../utils/analytics' -import { SaveContext, saveEmail, SaveEmailInput } from './save_email' -import { Page } from '../elastic/types' -import { addLabelToPage } from './labels' -import { saveSubscription } from './subscriptions' -import { NewsletterEmail } from '../entity/newsletter_email' -import { fetchFavicon } from '../utils/parser' -import { updatePage } from '../elastic/pages' 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' export interface NewsletterMessage { from: string @@ -75,16 +75,18 @@ export const saveNewsletterEmail = async ( } } - // creates or updates subscription - const subscriptionId = await saveSubscription({ - userId: newsletterEmail.user.id, - name: data.author, - newsletterEmail, - unsubscribeMailTo: data.unsubMailTo, - unsubscribeHttpUrl: data.unsubHttpUrl, - icon: page.siteIcon, - }) - console.log('subscription saved', subscriptionId) + // creates or updates subscription only if their is a valid unsubscribe link + if (data.unsubMailTo || data.unsubHttpUrl) { + const subscriptionId = await saveSubscription({ + userId: newsletterEmail.user.id, + name: data.author, + newsletterEmail, + unsubscribeMailTo: data.unsubMailTo, + unsubscribeHttpUrl: data.unsubHttpUrl, + icon: page.siteIcon, + }) + console.log('subscription saved', subscriptionId) + } // adds newsletters label to page const result = await addLabelToPage(saveCtx, page.id, { From f740d70608ac24181db4813f6c9a2811cefa2762 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Wed, 12 Apr 2023 10:46:45 +0800 Subject: [PATCH 2/2] Add test case --- .../services/save_newsletter_email.test.ts | 48 ++++++++++++++----- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/packages/api/test/services/save_newsletter_email.test.ts b/packages/api/test/services/save_newsletter_email.test.ts index e77f64e61..7859aed45 100644 --- a/packages/api/test/services/save_newsletter_email.test.ts +++ b/packages/api/test/services/save_newsletter_email.test.ts @@ -1,18 +1,18 @@ -import 'mocha' import { expect } from 'chai' import 'chai/register-should' -import { createTestUser, deleteTestUser } from '../db' -import { createNewsletterEmail } from '../../src/services/newsletters' -import { saveNewsletterEmail } from '../../src/services/save_newsletter_email' -import { User } from '../../src/entity/user' -import { NewsletterEmail } from '../../src/entity/newsletter_email' -import { SaveContext } from '../../src/services/save_email' +import 'mocha' +import nock from 'nock' import { createPubSubClient } from '../../src/datalayer/pubsub' import { getPageByParam } from '../../src/elastic/pages' -import nock from 'nock' -import { getRepository } from '../../src/entity/utils' -import { Subscription } from '../../src/entity/subscription' +import { NewsletterEmail } from '../../src/entity/newsletter_email' import { ReceivedEmail } from '../../src/entity/received_email' +import { Subscription } from '../../src/entity/subscription' +import { User } from '../../src/entity/user' +import { getRepository } from '../../src/entity/utils' +import { createNewsletterEmail } from '../../src/services/newsletters' +import { SaveContext } from '../../src/services/save_email' +import { saveNewsletterEmail } from '../../src/services/save_newsletter_email' +import { createTestUser, deleteTestUser } from '../db' describe('saveNewsletterEmail', () => { const fakeContent = 'fake content' @@ -62,6 +62,7 @@ describe('saveNewsletterEmail', () => { title, author, receivedEmailId: receivedEmail.id, + unsubHttpUrl: 'https://blog.omnivore.app/unsubscribe', }, newsletterEmail, ctx @@ -86,7 +87,7 @@ describe('saveNewsletterEmail', () => { expect(updatedReceivedEmail?.type).to.equal('article') }) - it('should adds a Newsletter label to that page', async () => { + it('adds a Newsletter label to that page', async () => { const url = 'https://blog.omnivore.app/new-fake-url' const newLabel = { name: 'Newsletter', @@ -110,4 +111,29 @@ describe('saveNewsletterEmail', () => { const page = await getPageByParam({ userId: user.id, url }) expect(page?.labels?.[0]).to.deep.include(newLabel) }) + + it('does not create a subscription if no unsubscribe header', async () => { + const url = 'https://blog.omnivore.app/no-unsubscribe' + nock('https://blog.omnivore.app').get('/no-unsubscribe').reply(404) + + await saveNewsletterEmail( + { + email: newsletterEmail.address, + content: `fake content 2`, + url, + title, + author, + from, + receivedEmailId: receivedEmail.id, + }, + newsletterEmail, + ctx + ) + + const subscriptions = await getRepository(Subscription).findBy({ + newsletterEmail: { id: newsletterEmail.id }, + name: from, + }) + expect(subscriptions).to.be.empty + }) })