From a9b011ad4b74a8fbda8a29ef26796503e1ef6e36 Mon Sep 17 00:00:00 2001 From: Daniel Lockyer Date: Tue, 26 Jul 2022 11:35:48 +0200 Subject: [PATCH] Avoided loading newsletter subscription events if threshold is Infinity - if the threshold is Infinity, we shouldn't be loading the newsletter subscription events because we are saying there is no threshold - the code has a quick path to avoid comparing the values, but it still loads the events upfront - this commit moves the quick path up to return earlier - this has the nice side-effect of producing 100% coverage on this package --- .../lib/verification-trigger.js | 10 +++--- .../test/verification-trigger.test.js | 36 +++++++++++++++++++ 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/ghost/verification-trigger/lib/verification-trigger.js b/ghost/verification-trigger/lib/verification-trigger.js index a812087a50..19bfdd4bb5 100644 --- a/ghost/verification-trigger/lib/verification-trigger.js +++ b/ghost/verification-trigger/lib/verification-trigger.js @@ -69,6 +69,11 @@ class VerificationTrigger { } async testImportThreshold() { + if (!isFinite(this._configThreshold)) { + // Infinite threshold, quick path + return; + } + const createdAt = new Date(); createdAt.setDate(createdAt.getDate() - 30); const events = await this._eventRepository.getNewsletterSubscriptionEvents({}, { @@ -76,11 +81,6 @@ class VerificationTrigger { 'data.created_at': `data.created_at:>'${createdAt.toISOString().replace('T', ' ').substring(0, 19)}'` }); - if (!isFinite(this._configThreshold)) { - // Inifinte threshold, quick path - return; - } - const membersTotal = await this._membersStats.getTotalMembers(); // Import threshold is either the total number of members (discounting any created by imports in diff --git a/ghost/verification-trigger/test/verification-trigger.test.js b/ghost/verification-trigger/test/verification-trigger.test.js index 2d9bda263c..45bde4bd32 100644 --- a/ghost/verification-trigger/test/verification-trigger.test.js +++ b/ghost/verification-trigger/test/verification-trigger.test.js @@ -233,4 +233,40 @@ describe('Email verification flow', function () { amountImported: 10 }); }); + + it('Does not fetch events and trigger when threshold is Infinity', async function () { + const emailStub = sinon.stub().resolves(null); + const settingsStub = sinon.stub().resolves(null); + const eventStub = sinon.stub().resolves({ + meta: { + pagination: { + total: 10 + } + } + }); + + const trigger = new VerificationTrigger({ + configThreshold: Infinity, + Settings: { + edit: settingsStub + }, + membersStats: { + getTotalMembers: () => 15 + }, + isVerified: () => false, + isVerificationRequired: () => false, + sendVerificationEmail: emailStub, + eventRepository: { + getNewsletterSubscriptionEvents: eventStub + } + }); + + await trigger.testImportThreshold(); + + // We shouldn't be fetching the events if the threshold is Infinity + eventStub.callCount.should.eql(0); + + // We shouldn't be sending emails if the threshold is infinity + emailStub.callCount.should.eql(0); + }); });