From 44f189b56acadaf4b2cb873636bbd5a18c1389b7 Mon Sep 17 00:00:00 2001
From: Simon Backx <simon@ghost.org>
Date: Mon, 21 Nov 2022 10:29:53 +0100
Subject: [PATCH] Added email service package (#15849)

fixes https://github.com/TryGhost/Team/issues/2282

Added a new email service package that is used when the email stability
flag is enabled. Currently not yet implemented so will throw an error
for all entry points (if flag enabled).

Removed usage of `labs.isSet.bind` across the code, because that breaks
the stubbing of labs by `mockManager.mockLabsEnabled` and
`mockManager.mockLabsDisabled`. `flag => labs.isSet(flag)` should be
used instead.

All email depending tests now disable the `emailStability` feature flag
to keep the tests passing + make sure we still run all the tests for the
old flow while the email stability package is being built.
---
 ghost/core/core/boot.js                       |  2 +
 .../server/api/endpoints/email-previews.js    | 12 +++-
 .../server/services/email-service/index.js    | 16 +++++
 ghost/core/core/server/services/mega/mega.js  |  5 ++
 .../core/server/services/members/service.js   |  2 +-
 .../server/services/posts/posts-service.js    | 25 +++++--
 ghost/core/package.json                       |  1 +
 .../test/e2e-api/admin/email-previews.test.js | 10 ++-
 .../test/e2e-api/admin/posts-legacy.test.js   |  5 ++
 .../test/e2e-server/click-tracking.test.js    |  1 +
 .../test/integration/services/mega.test.js    | 16 ++---
 .../test/regression/api/admin/posts.test.js   |  8 +++
 ghost/email-service/.eslintrc.js              |  6 ++
 ghost/email-service/README.md                 | 23 +++++++
 ghost/email-service/index.js                  |  4 ++
 ghost/email-service/lib/email-controller.js   | 65 +++++++++++++++++++
 ghost/email-service/lib/email-service.js      | 28 ++++++++
 ghost/email-service/package.json              | 30 +++++++++
 ghost/email-service/test/.eslintrc.js         |  6 ++
 ghost/email-service/test/hello.test.js        | 10 +++
 ghost/email-service/test/utils/assertions.js  | 11 ++++
 ghost/email-service/test/utils/index.js       | 11 ++++
 ghost/email-service/test/utils/overrides.js   | 10 +++
 23 files changed, 289 insertions(+), 18 deletions(-)
 create mode 100644 ghost/core/core/server/services/email-service/index.js
 create mode 100644 ghost/email-service/.eslintrc.js
 create mode 100644 ghost/email-service/README.md
 create mode 100644 ghost/email-service/index.js
 create mode 100644 ghost/email-service/lib/email-controller.js
 create mode 100644 ghost/email-service/lib/email-service.js
 create mode 100644 ghost/email-service/package.json
 create mode 100644 ghost/email-service/test/.eslintrc.js
 create mode 100644 ghost/email-service/test/hello.test.js
 create mode 100644 ghost/email-service/test/utils/assertions.js
 create mode 100644 ghost/email-service/test/utils/index.js
 create mode 100644 ghost/email-service/test/utils/overrides.js

diff --git a/ghost/core/core/boot.js b/ghost/core/core/boot.js
index 7cc4ac9d88..545faa8596 100644
--- a/ghost/core/core/boot.js
+++ b/ghost/core/core/boot.js
@@ -291,6 +291,7 @@ async function initServices({config}) {
     const linkTracking = require('./server/services/link-tracking');
     const audienceFeedback = require('./server/services/audience-feedback');
     const emailSuppressionList = require('./server/services/email-suppression-list');
+    const emailService = require('./server/services/email-service');
 
     const urlUtils = require('./shared/url-utils');
 
@@ -311,6 +312,7 @@ async function initServices({config}) {
         permissions.init(),
         xmlrpc.listen(),
         slack.listen(),
+        emailService.init(),
         mega.listen(),
         webhooks.listen(),
         appService.init(),
diff --git a/ghost/core/core/server/api/endpoints/email-previews.js b/ghost/core/core/server/api/endpoints/email-previews.js
index edb317442c..39767e28d1 100644
--- a/ghost/core/core/server/api/endpoints/email-previews.js
+++ b/ghost/core/core/server/api/endpoints/email-previews.js
@@ -2,7 +2,8 @@ const models = require('../../models');
 const tpl = require('@tryghost/tpl');
 const errors = require('@tryghost/errors');
 const mega = require('../../services/mega');
-
+const emailService = require('../../services/email-service');
+const labs = require('../../../shared/labs');
 const messages = {
     postNotFound: 'Post not found.'
 };
@@ -29,6 +30,10 @@ module.exports = {
         ],
         permissions: true,
         async query(frame) {
+            if (labs.isSet('emailStability')) {
+                return await emailService.controller.previewEmail(frame);
+            }
+
             const options = Object.assign(frame.options, {formats: 'html,plaintext', withRelated: ['authors', 'posts_meta']});
             const data = Object.assign(frame.data, {status: 'all'});
 
@@ -61,6 +66,10 @@ module.exports = {
         },
         permissions: true,
         async query(frame) {
+            if (labs.isSet('emailStability')) {
+                return await emailService.controller.sendTestEmail(frame);
+            }
+
             const options = Object.assign(frame.options, {status: 'all'});
             let model = await models.Post.findOne(options, {withRelated: ['authors']});
 
@@ -69,7 +78,6 @@ module.exports = {
                     message: tpl(messages.postNotFound)
                 });
             }
-
             const {emails = [], memberSegment} = frame.data;
             return await mega.mega.sendTestEmail(model, emails, memberSegment);
         }
diff --git a/ghost/core/core/server/services/email-service/index.js b/ghost/core/core/server/services/email-service/index.js
new file mode 100644
index 0000000000..6d98fa66db
--- /dev/null
+++ b/ghost/core/core/server/services/email-service/index.js
@@ -0,0 +1,16 @@
+class EmailServiceWrapper {
+    init() {
+        const {EmailService, EmailController} = require('@tryghost/email-service');
+        const {Post, Newsletter} = require('../../models');
+
+        this.service = new EmailService({});
+        this.controller = new EmailController(this.service, {
+            models: {
+                Post,
+                Newsletter
+            }
+        });
+    }
+}
+
+module.exports = new EmailServiceWrapper();
diff --git a/ghost/core/core/server/services/mega/mega.js b/ghost/core/core/server/services/mega/mega.js
index 8caac3ae08..e6add349fd 100644
--- a/ghost/core/core/server/services/mega/mega.js
+++ b/ghost/core/core/server/services/mega/mega.js
@@ -15,6 +15,7 @@ const db = require('../../data/db');
 const models = require('../../models');
 const postEmailSerializer = require('./post-email-serializer');
 const {getSegmentsFromHtml} = require('./segment-parser');
+const labs = require('../../../shared/labs');
 
 // Used to listen to email.added and email.edited model events originally, I think to offload this - ideally would just use jobs now if possible
 const events = require('../../lib/common/events');
@@ -265,6 +266,10 @@ const retryFailedEmail = async (emailModel) => {
 };
 
 async function pendingEmailHandler(emailModel, options) {
+    if (labs.isSet('emailStability')) {
+        return;
+    }
+
     // CASE: do not send email if we import a database
     // TODO: refactor post.published events to never fire on importing
     if (options && options.importing) {
diff --git a/ghost/core/core/server/services/members/service.js b/ghost/core/core/server/services/members/service.js
index c1a9ec613c..b6e23c3ab2 100644
--- a/ghost/core/core/server/services/members/service.js
+++ b/ghost/core/core/server/services/members/service.js
@@ -56,7 +56,7 @@ const membersImporter = new MembersCSVImporter({
         return tiersService.api.readDefaultTier();
     },
     sendEmail: ghostMailer.send.bind(ghostMailer),
-    isSet: labsService.isSet.bind(labsService),
+    isSet: flag => labsService.isSet(flag),
     addJob: jobsService.addJob.bind(jobsService),
     knex: db.knex,
     urlFor: urlUtils.urlFor.bind(urlUtils),
diff --git a/ghost/core/core/server/services/posts/posts-service.js b/ghost/core/core/server/services/posts/posts-service.js
index a75241b6dd..d59500a041 100644
--- a/ghost/core/core/server/services/posts/posts-service.js
+++ b/ghost/core/core/server/services/posts/posts-service.js
@@ -8,12 +8,13 @@ const messages = {
 };
 
 class PostsService {
-    constructor({mega, urlUtils, models, isSet, stats}) {
+    constructor({mega, urlUtils, models, isSet, stats, emailService}) {
         this.mega = mega;
         this.urlUtils = urlUtils;
         this.models = models;
         this.isSet = isSet;
         this.stats = stats;
+        this.emailService = emailService;
     }
 
     async editPost(frame) {
@@ -41,12 +42,22 @@ class PostsService {
 
             if (sendEmail) {
                 let postEmail = model.relations.email;
+                let email;
 
                 if (!postEmail) {
-                    const email = await this.mega.addEmail(model, frame.options);
-                    model.set('email', email);
+                    if (this.isSet('emailStability')) {
+                        email = await this.emailService.createEmail(model);
+                    } else {
+                        email = await this.mega.addEmail(model, frame.options);
+                    }
                 } else if (postEmail && postEmail.get('status') === 'failed') {
-                    const email = await this.mega.retryFailedEmail(postEmail);
+                    if (this.isSet('emailStability')) {
+                        email = await this.emailService.retryEmail(postEmail);
+                    } else {
+                        email = await this.mega.retryFailedEmail(postEmail);
+                    }
+                }
+                if (email) {
                     model.set('email', email);
                 }
             }
@@ -123,6 +134,7 @@ const getPostServiceInstance = () => {
     const labs = require('../../../shared/labs');
     const models = require('../../models');
     const PostStats = require('./stats/post-stats');
+    const emailService = require('../email-service');
 
     const postStats = new PostStats();
 
@@ -130,8 +142,9 @@ const getPostServiceInstance = () => {
         mega: mega,
         urlUtils: urlUtils,
         models: models,
-        isSet: labs.isSet.bind(labs),
-        stats: postStats
+        isSet: flag => labs.isSet(flag), // don't use bind, that breaks test subbing of labs
+        stats: postStats,
+        emailService: emailService.service
     });
 };
 
diff --git a/ghost/core/package.json b/ghost/core/package.json
index 1e4d1b129d..cb1ad20167 100644
--- a/ghost/core/package.json
+++ b/ghost/core/package.json
@@ -132,6 +132,7 @@
     "@tryghost/verification-trigger": "0.0.0",
     "@tryghost/version": "0.1.16",
     "@tryghost/zip": "1.1.29",
+    "@tryghost/email-service": "0.0.0",
     "amperize": "0.6.1",
     "analytics-node": "6.2.0",
     "bluebird": "3.7.2",
diff --git a/ghost/core/test/e2e-api/admin/email-previews.test.js b/ghost/core/test/e2e-api/admin/email-previews.test.js
index 625d1c4c23..262a850d09 100644
--- a/ghost/core/test/e2e-api/admin/email-previews.test.js
+++ b/ghost/core/test/e2e-api/admin/email-previews.test.js
@@ -1,4 +1,4 @@
-const {agentProvider, fixtureManager, matchers} = require('../../utils/e2e-framework');
+const {agentProvider, fixtureManager, matchers, mockManager} = require('../../utils/e2e-framework');
 const {anyEtag, anyErrorId} = matchers;
 const assert = require('assert');
 
@@ -10,6 +10,14 @@ const models = require('../../../core/server/models/index');
 describe('Email Preview API', function () {
     let agent;
 
+    beforeEach(function () {
+        mockManager.mockLabsDisabled('emailStability');
+    });
+
+    afterEach(function () {
+        mockManager.restore();
+    });
+
     before(async function () {
         agent = await agentProvider.getAdminAPIAgent();
         await fixtureManager.init('users', 'newsletters', 'posts');
diff --git a/ghost/core/test/e2e-api/admin/posts-legacy.test.js b/ghost/core/test/e2e-api/admin/posts-legacy.test.js
index 345c7c50fe..00965fadb7 100644
--- a/ghost/core/test/e2e-api/admin/posts-legacy.test.js
+++ b/ghost/core/test/e2e-api/admin/posts-legacy.test.js
@@ -30,7 +30,12 @@ describe('Posts API', function () {
         await models.Post.edit({newsletter_id: newsletterId}, {id: postId});
     });
 
+    beforeEach(function () {
+        mockManager.mockLabsDisabled('emailStability');
+    });
+
     afterEach(function () {
+        mockManager.restore();
         nock.cleanAll();
     });
 
diff --git a/ghost/core/test/e2e-server/click-tracking.test.js b/ghost/core/test/e2e-server/click-tracking.test.js
index d0e33d7951..45fc689ff3 100644
--- a/ghost/core/test/e2e-server/click-tracking.test.js
+++ b/ghost/core/test/e2e-server/click-tracking.test.js
@@ -15,6 +15,7 @@ describe('Click Tracking', function () {
     });
 
     beforeEach(function () {
+        mockManager.mockLabsDisabled('emailStability');
         mockManager.mockMail();
     });
 
diff --git a/ghost/core/test/integration/services/mega.test.js b/ghost/core/test/integration/services/mega.test.js
index fd186f8e28..79a693e21c 100644
--- a/ghost/core/test/integration/services/mega.test.js
+++ b/ghost/core/test/integration/services/mega.test.js
@@ -54,6 +54,14 @@ describe('MEGA', function () {
     let _mailgunClient;
     let frontendAgent;
 
+    beforeEach(function () {
+        mockManager.mockLabsDisabled('emailStability');
+    });
+
+    afterEach(function () {
+        mockManager.restore();
+    });
+
     describe('sendEmailJob', function () {
         before(async function () {
             agent = await agentProvider.getAdminAPIAgent();
@@ -63,10 +71,6 @@ describe('MEGA', function () {
             _mailgunClient = require('../../../core/server/services/bulk-email')._mailgunClient;
         });
 
-        afterEach(function () {
-            mockManager.restore();
-        });
-
         it('Can send a scheduled post email', async function () {
             sinon.stub(_mailgunClient, 'getInstance').returns({});
             sinon.stub(_mailgunClient, 'send').callsFake(async () => {
@@ -144,10 +148,6 @@ describe('MEGA', function () {
             _mailgunClient = require('../../../core/server/services/bulk-email')._mailgunClient;
         });
 
-        afterEach(function () {
-            mockManager.restore();
-        });
-
         it('Tracks all the links in an email', async function () {
             const linkRedirectService = require('../../../core/server/services/link-redirection');
             const linkRedirectRepository = linkRedirectService.linkRedirectRepository;
diff --git a/ghost/core/test/regression/api/admin/posts.test.js b/ghost/core/test/regression/api/admin/posts.test.js
index 8e04005e0f..26323abbe5 100644
--- a/ghost/core/test/regression/api/admin/posts.test.js
+++ b/ghost/core/test/regression/api/admin/posts.test.js
@@ -26,6 +26,14 @@ describe('Posts API', function () {
         await localUtils.doAuth(request, 'users:extra', 'posts', 'emails', 'newsletters', 'members:newsletters');
     });
 
+    beforeEach(function () {
+        mockManager.mockLabsDisabled('emailStability');
+    });
+
+    afterEach(function () {
+        mockManager.restore();
+    });
+
     describe('Browse', function () {
         it('fields & formats combined', function (done) {
             request.get(localUtils.API.getApiQuery('posts/?formats=mobiledoc,html&fields=id,title'))
diff --git a/ghost/email-service/.eslintrc.js b/ghost/email-service/.eslintrc.js
new file mode 100644
index 0000000000..c9c1bcb522
--- /dev/null
+++ b/ghost/email-service/.eslintrc.js
@@ -0,0 +1,6 @@
+module.exports = {
+    plugins: ['ghost'],
+    extends: [
+        'plugin:ghost/node'
+    ]
+};
diff --git a/ghost/email-service/README.md b/ghost/email-service/README.md
new file mode 100644
index 0000000000..3bda4382db
--- /dev/null
+++ b/ghost/email-service/README.md
@@ -0,0 +1,23 @@
+# Email Service
+
+Manages how posts are sent via email
+
+
+## Usage
+
+
+## Develop
+
+This is a monorepo package.
+
+Follow the instructions for the top-level repo.
+1. `git clone` this repo & `cd` into it as usual
+2. Run `yarn` to install top-level dependencies.
+
+
+
+## Test
+
+- `yarn lint` run just eslint
+- `yarn test` run lint and tests
+
diff --git a/ghost/email-service/index.js b/ghost/email-service/index.js
new file mode 100644
index 0000000000..b9e9ceee03
--- /dev/null
+++ b/ghost/email-service/index.js
@@ -0,0 +1,4 @@
+module.exports = {
+    EmailService: require('./lib/email-service'),
+    EmailController: require('./lib/email-controller')
+};
diff --git a/ghost/email-service/lib/email-controller.js b/ghost/email-service/lib/email-controller.js
new file mode 100644
index 0000000000..a06c7e2e3a
--- /dev/null
+++ b/ghost/email-service/lib/email-controller.js
@@ -0,0 +1,65 @@
+const errors = require('@tryghost/errors');
+const tpl = require('@tryghost/tpl');
+
+const messages = {
+    postNotFound: 'Post not found.',
+    noEmailsProvided: 'No emails provided.'
+};
+
+class EmailController {
+    service;
+    models;
+
+    /**
+     * 
+     * @param {EmailService} service 
+     * @param {{models: {Post: any, Newsletter: any}}} dependencies
+     */
+    constructor(service, {models}) {
+        this.service = service;
+        this.models = models;
+    }
+
+    async _getFrameData(frame) {
+        const post = await this.models.Post.findOne({...frame.data, status: 'all'}, {...frame.options});
+
+        if (!post) {
+            throw new errors.NotFoundError({
+                message: tpl(messages.postNotFound)
+            });
+        }
+
+        let newsletter;
+        if (frame.options.newsletter) {
+            newsletter = await this.models.Newsletter.findOne({slug: frame.options.newsletter});
+        } else {
+            newsletter = (await post.getLazyRelation('newsletter')) ?? (await this.models.Newsletter.getDefaultNewsletter());
+        }
+        return {
+            post,
+            newsletter,
+            segment: frame.options.memberSegment ?? frame.data.memberSegment ?? null
+        };
+    }
+
+    async previewEmail(frame) {
+        const {post, newsletter, segment} = await this._getFrameData(frame);
+        return await this.service.previewEmail(post, newsletter, segment);
+    }
+
+    async sendTestEmail(frame) {
+        const {post, newsletter, segment} = await this._getFrameData(frame);
+
+        const emails = frame.data.emails ?? [];
+
+        if (emails.length === 0) {
+            throw new errors.ValidationError({
+                message: tpl(messages.noEmailsProvided)
+            });
+        }
+
+        return await this.service.sendTestEmail(post, newsletter, segment, emails);
+    }
+}
+
+module.exports = EmailController;
diff --git a/ghost/email-service/lib/email-service.js b/ghost/email-service/lib/email-service.js
new file mode 100644
index 0000000000..6a4dbed6f6
--- /dev/null
+++ b/ghost/email-service/lib/email-service.js
@@ -0,0 +1,28 @@
+/* eslint-disable no-unused-vars */
+
+class EmailService {
+    constructor(dependencies) {
+        // ...
+    }
+
+    async createEmail(post) {
+        // eslint-disable-next-line no-restricted-syntax
+        throw new Error('Not implemented');
+    }
+    async retryEmail(email) {
+        // eslint-disable-next-line no-restricted-syntax
+        throw new Error('Not implemented');
+    }
+
+    async previewEmail(post, newsletter, segment) {
+        // eslint-disable-next-line no-restricted-syntax
+        throw new Error('Previewing an email has not been implemented yet. Turn off the email stability flag is you need this functionality.');
+    }
+
+    async sendTestEmail(post, newsletter, segment, emails) {
+        // eslint-disable-next-line no-restricted-syntax
+        throw new Error('Sending a test email has not been implemented yet. Turn off the email stability flag is you need this functionality.');
+    }
+}
+
+module.exports = EmailService;
diff --git a/ghost/email-service/package.json b/ghost/email-service/package.json
new file mode 100644
index 0000000000..204862f7ef
--- /dev/null
+++ b/ghost/email-service/package.json
@@ -0,0 +1,30 @@
+{
+  "name": "@tryghost/email-service",
+  "version": "0.0.0",
+  "repository": "https://github.com/TryGhost/Ghost/tree/main/packages/email-service",
+  "author": "Ghost Foundation",
+  "private": true,
+  "main": "index.js",
+  "scripts": {
+    "dev": "echo \"Implement me!\"",
+    "test:unit": "NODE_ENV=testing c8 --all --reporter text --reporter cobertura mocha './test/**/*.test.js'",
+    "test": "yarn test:unit",
+    "lint:code": "eslint *.js lib/ --ext .js --cache",
+    "lint": "yarn lint:code && yarn lint:test",
+    "lint:test": "eslint -c test/.eslintrc.js test/ --ext .js --cache"
+  },
+  "files": [
+    "index.js",
+    "lib"
+  ],
+  "devDependencies": {
+    "c8": "7.12.0",
+    "mocha": "10.1.0",
+    "should": "13.2.3",
+    "sinon": "14.0.2"
+  },
+  "dependencies": {
+    "@tryghost/errors": "1.2.18",
+    "@tryghost/tpl": "0.1.19"
+  }
+}
diff --git a/ghost/email-service/test/.eslintrc.js b/ghost/email-service/test/.eslintrc.js
new file mode 100644
index 0000000000..829b601eb0
--- /dev/null
+++ b/ghost/email-service/test/.eslintrc.js
@@ -0,0 +1,6 @@
+module.exports = {
+    plugins: ['ghost'],
+    extends: [
+        'plugin:ghost/test'
+    ]
+};
diff --git a/ghost/email-service/test/hello.test.js b/ghost/email-service/test/hello.test.js
new file mode 100644
index 0000000000..85d69d1e08
--- /dev/null
+++ b/ghost/email-service/test/hello.test.js
@@ -0,0 +1,10 @@
+// Switch these lines once there are useful utils
+// const testUtils = require('./utils');
+require('./utils');
+
+describe('Hello world', function () {
+    it('Runs a test', function () {
+        // TODO: Write me!
+        'hello'.should.eql('hello');
+    });
+});
diff --git a/ghost/email-service/test/utils/assertions.js b/ghost/email-service/test/utils/assertions.js
new file mode 100644
index 0000000000..7364ee8aa1
--- /dev/null
+++ b/ghost/email-service/test/utils/assertions.js
@@ -0,0 +1,11 @@
+/**
+ * Custom Should Assertions
+ *
+ * Add any custom assertions to this file.
+ */
+
+// Example Assertion
+// should.Assertion.add('ExampleAssertion', function () {
+//     this.params = {operator: 'to be a valid Example Assertion'};
+//     this.obj.should.be.an.Object;
+// });
diff --git a/ghost/email-service/test/utils/index.js b/ghost/email-service/test/utils/index.js
new file mode 100644
index 0000000000..0d67d86ff8
--- /dev/null
+++ b/ghost/email-service/test/utils/index.js
@@ -0,0 +1,11 @@
+/**
+ * Test Utilities
+ *
+ * Shared utils for writing tests
+ */
+
+// Require overrides - these add globals for tests
+require('./overrides');
+
+// Require assertions - adds custom should assertions
+require('./assertions');
diff --git a/ghost/email-service/test/utils/overrides.js b/ghost/email-service/test/utils/overrides.js
new file mode 100644
index 0000000000..90203424ee
--- /dev/null
+++ b/ghost/email-service/test/utils/overrides.js
@@ -0,0 +1,10 @@
+// This file is required before any test is run
+
+// Taken from the should wiki, this is how to make should global
+// Should is a global in our eslint test config
+global.should = require('should').noConflict();
+should.extend();
+
+// Sinon is a simple case
+// Sinon is a global in our eslint test config
+global.sinon = require('sinon');