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.
This commit is contained in:
Simon Backx 2022-11-21 10:29:53 +01:00 committed by GitHub
parent 96aa1c930c
commit 44f189b56a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
23 changed files with 289 additions and 18 deletions

View File

@ -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(),

View File

@ -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);
}

View File

@ -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();

View File

@ -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) {

View File

@ -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),

View File

@ -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
});
};

View File

@ -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",

View File

@ -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');

View File

@ -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();
});

View File

@ -15,6 +15,7 @@ describe('Click Tracking', function () {
});
beforeEach(function () {
mockManager.mockLabsDisabled('emailStability');
mockManager.mockMail();
});

View File

@ -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;

View File

@ -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'))

View File

@ -0,0 +1,6 @@
module.exports = {
plugins: ['ghost'],
extends: [
'plugin:ghost/node'
]
};

View File

@ -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

View File

@ -0,0 +1,4 @@
module.exports = {
EmailService: require('./lib/email-service'),
EmailController: require('./lib/email-controller')
};

View File

@ -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;

View File

@ -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;

View File

@ -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"
}
}

View File

@ -0,0 +1,6 @@
module.exports = {
plugins: ['ghost'],
extends: [
'plugin:ghost/test'
]
};

View File

@ -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');
});
});

View File

@ -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;
// });

View File

@ -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');

View File

@ -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');