mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-02-24 23:48:13 -05:00
🐛 Fixed not reactivating email analytics jobs
no issue When a site doesn't have any emails on boot, it doesn't schedule the email analytics job. With this change, the new email flow will also restart that job after an email has been created.
This commit is contained in:
parent
6631071dd3
commit
c798f383f9
4 changed files with 41 additions and 6 deletions
|
@ -7,7 +7,7 @@ const jobsService = require('../../jobs');
|
||||||
let hasScheduled = false;
|
let hasScheduled = false;
|
||||||
|
|
||||||
module.exports = {
|
module.exports = {
|
||||||
async scheduleRecurringJobs() {
|
async scheduleRecurringJobs(skipEmailCheck = false) {
|
||||||
if (
|
if (
|
||||||
!hasScheduled &&
|
!hasScheduled &&
|
||||||
config.get('emailAnalytics') &&
|
config.get('emailAnalytics') &&
|
||||||
|
@ -17,10 +17,10 @@ module.exports = {
|
||||||
// Don't register email analytics job if we have no emails,
|
// Don't register email analytics job if we have no emails,
|
||||||
// processor usage from many sites spinning up threads can be high.
|
// processor usage from many sites spinning up threads can be high.
|
||||||
// Mega service will re-run this scheduling task when an email is sent
|
// Mega service will re-run this scheduling task when an email is sent
|
||||||
const emailCount = await models.Email
|
const emailCount = skipEmailCheck ? 1 : (await models.Email
|
||||||
.where('created_at', '>', moment.utc().subtract(30, 'days').toDate())
|
.where('created_at', '>', moment.utc().subtract(30, 'days').toDate())
|
||||||
.where('status', '<>', 'failed')
|
.where('status', '<>', 'failed')
|
||||||
.count();
|
.count());
|
||||||
|
|
||||||
if (emailCount > 0) {
|
if (emailCount > 0) {
|
||||||
// use a random seconds value to avoid spikes to external APIs on the minute
|
// use a random seconds value to avoid spikes to external APIs on the minute
|
||||||
|
|
|
@ -35,6 +35,7 @@ class EmailServiceWrapper {
|
||||||
const linkTracking = require('../link-tracking');
|
const linkTracking = require('../link-tracking');
|
||||||
const audienceFeedback = require('../audience-feedback');
|
const audienceFeedback = require('../audience-feedback');
|
||||||
const storageUtils = require('../../adapters/storage/utils');
|
const storageUtils = require('../../adapters/storage/utils');
|
||||||
|
const emailAnalyticsJobs = require('../email-analytics/jobs');
|
||||||
|
|
||||||
// capture errors from mailgun client and log them in sentry
|
// capture errors from mailgun client and log them in sentry
|
||||||
const errorHandler = (error) => {
|
const errorHandler = (error) => {
|
||||||
|
@ -103,7 +104,8 @@ class EmailServiceWrapper {
|
||||||
emailSegmenter,
|
emailSegmenter,
|
||||||
limitService,
|
limitService,
|
||||||
membersRepository,
|
membersRepository,
|
||||||
verificationTrigger: membersService.verificationTrigger
|
verificationTrigger: membersService.verificationTrigger,
|
||||||
|
emailAnalyticsJobs
|
||||||
});
|
});
|
||||||
|
|
||||||
this.controller = new EmailController(this.service, {
|
this.controller = new EmailController(this.service, {
|
||||||
|
|
|
@ -13,6 +13,7 @@ const tpl = require('@tryghost/tpl');
|
||||||
const EmailRenderer = require('./email-renderer');
|
const EmailRenderer = require('./email-renderer');
|
||||||
const EmailSegmenter = require('./email-segmenter');
|
const EmailSegmenter = require('./email-segmenter');
|
||||||
const SendingService = require('./sending-service');
|
const SendingService = require('./sending-service');
|
||||||
|
const logging = require('@tryghost/logging');
|
||||||
|
|
||||||
const messages = {
|
const messages = {
|
||||||
archivedNewsletterError: 'Cannot send email to archived newsletters',
|
archivedNewsletterError: 'Cannot send email to archived newsletters',
|
||||||
|
@ -30,6 +31,7 @@ class EmailService {
|
||||||
#limitService;
|
#limitService;
|
||||||
#membersRepository;
|
#membersRepository;
|
||||||
#verificationTrigger;
|
#verificationTrigger;
|
||||||
|
#emailAnalyticsJobs;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
*
|
*
|
||||||
|
@ -44,6 +46,7 @@ class EmailService {
|
||||||
* @param {LimitService} dependencies.limitService
|
* @param {LimitService} dependencies.limitService
|
||||||
* @param {object} dependencies.membersRepository
|
* @param {object} dependencies.membersRepository
|
||||||
* @param {VerificationTrigger} dependencies.verificationTrigger
|
* @param {VerificationTrigger} dependencies.verificationTrigger
|
||||||
|
* @param {object} dependencies.emailAnalyticsJobs
|
||||||
*/
|
*/
|
||||||
constructor({
|
constructor({
|
||||||
batchSendingService,
|
batchSendingService,
|
||||||
|
@ -54,7 +57,8 @@ class EmailService {
|
||||||
emailSegmenter,
|
emailSegmenter,
|
||||||
limitService,
|
limitService,
|
||||||
membersRepository,
|
membersRepository,
|
||||||
verificationTrigger
|
verificationTrigger,
|
||||||
|
emailAnalyticsJobs
|
||||||
}) {
|
}) {
|
||||||
this.#batchSendingService = batchSendingService;
|
this.#batchSendingService = batchSendingService;
|
||||||
this.#models = models;
|
this.#models = models;
|
||||||
|
@ -65,6 +69,7 @@ class EmailService {
|
||||||
this.#membersRepository = membersRepository;
|
this.#membersRepository = membersRepository;
|
||||||
this.#sendingService = sendingService;
|
this.#sendingService = sendingService;
|
||||||
this.#verificationTrigger = verificationTrigger;
|
this.#verificationTrigger = verificationTrigger;
|
||||||
|
this.#emailAnalyticsJobs = emailAnalyticsJobs;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -141,6 +146,13 @@ class EmailService {
|
||||||
}, {patch: true});
|
}, {patch: true});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// make sure recurring background analytics jobs are running once we have emails
|
||||||
|
try {
|
||||||
|
await this.#emailAnalyticsJobs.scheduleRecurringJobs(true);
|
||||||
|
} catch (e) {
|
||||||
|
logging.error(e);
|
||||||
|
}
|
||||||
|
|
||||||
return email;
|
return email;
|
||||||
}
|
}
|
||||||
async retryEmail(email) {
|
async retryEmail(email) {
|
||||||
|
|
|
@ -10,6 +10,7 @@ describe('Email Service', function () {
|
||||||
let membersRepository;
|
let membersRepository;
|
||||||
let emailRenderer;
|
let emailRenderer;
|
||||||
let sendingService;
|
let sendingService;
|
||||||
|
let scheduleRecurringJobs;
|
||||||
|
|
||||||
beforeEach(function () {
|
beforeEach(function () {
|
||||||
memberCount = 123;
|
memberCount = 123;
|
||||||
|
@ -19,6 +20,7 @@ describe('Email Service', function () {
|
||||||
};
|
};
|
||||||
verificicationRequired = false;
|
verificicationRequired = false;
|
||||||
scheduleEmail = sinon.stub().returns();
|
scheduleEmail = sinon.stub().returns();
|
||||||
|
scheduleRecurringJobs = sinon.stub().resolves();
|
||||||
settings = {};
|
settings = {};
|
||||||
settingsCache = {
|
settingsCache = {
|
||||||
get(key) {
|
get(key) {
|
||||||
|
@ -85,7 +87,10 @@ describe('Email Service', function () {
|
||||||
settingsCache,
|
settingsCache,
|
||||||
emailRenderer,
|
emailRenderer,
|
||||||
membersRepository,
|
membersRepository,
|
||||||
sendingService
|
sendingService,
|
||||||
|
emailAnalyticsJobs: {
|
||||||
|
scheduleRecurringJobs
|
||||||
|
}
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
@ -155,6 +160,22 @@ describe('Email Service', function () {
|
||||||
assert.strictEqual(email.get('status'), 'pending');
|
assert.strictEqual(email.get('status'), 'pending');
|
||||||
assert.strictEqual(email.get('source'), post.get('mobiledoc'));
|
assert.strictEqual(email.get('source'), post.get('mobiledoc'));
|
||||||
assert.strictEqual(email.get('source_type'), 'mobiledoc');
|
assert.strictEqual(email.get('source_type'), 'mobiledoc');
|
||||||
|
sinon.assert.calledOnce(scheduleRecurringJobs);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('Ignores analytics job scheduling errors', async function () {
|
||||||
|
const post = createModel({
|
||||||
|
id: '123',
|
||||||
|
newsletter: createModel({
|
||||||
|
status: 'active',
|
||||||
|
feedback_enabled: true
|
||||||
|
}),
|
||||||
|
mobiledoc: 'Mobiledoc'
|
||||||
|
});
|
||||||
|
|
||||||
|
scheduleRecurringJobs.rejects(new Error('Test error'));
|
||||||
|
await service.createEmail(post);
|
||||||
|
sinon.assert.calledOnce(scheduleRecurringJobs);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('Creates and schedules an email with lexical', async function () {
|
it('Creates and schedules an email with lexical', async function () {
|
||||||
|
|
Loading…
Add table
Reference in a new issue