Removed getCreatedEvents and added verification trigger test (#15832)

refs https://github.com/TryGhost/Team/issues/2266

This removes the deprecated `getCreatedEvents` method in the event
repository and adds tests to the verification trigger to see if we don't
break anything.

Changes extracted from https://github.com/TryGhost/Ghost/pull/15831

ghost/core/core/server/services/members/service.js

@@ -74,6 +74,36 @@ const processImport = async (options) => {
     return result;
 };
 
+const updateVerificationTrigger = () => {
+    verificationTrigger = new VerificationTrigger({
+        apiTriggerThreshold: _.get(config.get('hostSettings'), 'emailVerification.apiThreshold'),
+        adminTriggerThreshold: _.get(config.get('hostSettings'), 'emailVerification.adminThreshold'),
+        importTriggerThreshold: _.get(config.get('hostSettings'), 'emailVerification.importThreshold'),
+        isVerified: () => config.get('hostSettings:emailVerification:verified') === true,
+        isVerificationRequired: () => settingsCache.get('email_verification_required') === true,
+        sendVerificationEmail: async ({subject, message, amountTriggered}) => {
+            const escalationAddress = config.get('hostSettings:emailVerification:escalationAddress');
+            const fromAddress = config.get('user_email');
+
+            if (escalationAddress) {
+                await ghostMailer.send({
+                    subject,
+                    html: tpl(message, {
+                        amountTriggered: amountTriggered,
+                        siteUrl: urlUtils.getSiteUrl()
+                    }),
+                    forceTextContent: true,
+                    from: fromAddress,
+                    to: escalationAddress
+                });
+            }
+        },
+        membersStats,
+        Settings: models.Settings,
+        eventRepository: membersApi.events
+    });
+};
+
 module.exports = {
     async init() {
         const stripeService = require('../stripe');
@@ -110,33 +140,7 @@ module.exports = {
             getMembersApi: () => module.exports.api
         });
 
-        verificationTrigger = new VerificationTrigger({
-            apiTriggerThreshold: _.get(config.get('hostSettings'), 'emailVerification.apiThreshold'),
-            adminTriggerThreshold: _.get(config.get('hostSettings'), 'emailVerification.adminThreshold'),
-            importTriggerThreshold: _.get(config.get('hostSettings'), 'emailVerification.importThreshold'),
-            isVerified: () => config.get('hostSettings:emailVerification:verified') === true,
-            isVerificationRequired: () => settingsCache.get('email_verification_required') === true,
-            sendVerificationEmail: ({subject, message, amountTriggered}) => {
-                const escalationAddress = config.get('hostSettings:emailVerification:escalationAddress');
-                const fromAddress = config.get('user_email');
-
-                if (escalationAddress) {
-                    ghostMailer.send({
-                        subject,
-                        html: tpl(message, {
-                            amountTriggered: amountTriggered,
-                            siteUrl: urlUtils.getSiteUrl()
-                        }),
-                        forceTextContent: true,
-                        from: fromAddress,
-                        to: escalationAddress
-                    });
-                }
-            },
-            membersStats,
-            Settings: models.Settings,
-            eventRepository: membersApi.events
-        });
+        updateVerificationTrigger();
 
         (async () => {
             try {
@@ -185,7 +189,10 @@ module.exports = {
     processImport: processImport,
 
     stats: membersStats,
-    export: require('./exporter/query')
+    export: require('./exporter/query'),
+
+    // Only for tests
+    _updateVerificationTrigger: updateVerificationTrigger
 };
 
 module.exports.middleware = require('./middleware');

ghost/core/test/regression/api/admin/members-importer.test.js

@@ -5,8 +5,12 @@ const supertest = require('supertest');
 const testUtils = require('../../../utils');
 const localUtils = require('./utils');
 const config = require('../../../../core/shared/config');
+const configUtils = require('../../../utils/configUtils');
+const settingsCache = require('../../../../core/shared/settings-cache');
 
 const {mockManager} = require('../../../utils/e2e-framework');
+const assert = require('assert');
+const {_updateVerificationTrigger} = require('../../../../core/server/services/members');
 
 let request;
 
@@ -237,4 +241,65 @@ describe('Members Importer API', function () {
                 jsonResponse.meta.import_label.slug.should.match(/^import-/);
             });
     });
+    
+    it('Can import members with host emailVerification limits', async function () {
+        // If this test fails, check if the total members that have been created with fixtures has increased a lot, and if required, increase the amount of imported members
+        configUtils.set('hostSettings:emailVerification', {
+            apiThreshold: 2,
+            adminThreshold: 2,
+            importThreshold: 1, // note: this one isn't really used because (totalMembers - members_created_in_last_30_days) is larger and used instead
+            verified: false,
+            escalationAddress: 'test@example.com'
+        });
+        _updateVerificationTrigger();
+
+        const res = await request
+            .post(localUtils.API.getApiQuery(`members/upload/`))
+            .field('labels', ['new-global-label'])
+            .attach('membersfile', path.join(__dirname, '/../../../utils/fixtures/csv/valid-members-import-large.csv'))
+            .set('Origin', config.get('url'))
+            .expect('Content-Type', /json/)
+            .expect('Cache-Control', testUtils.cacheRules.private)
+            .expect(201);
+        should.not.exist(res.headers['x-cache-invalidate']);
+        const jsonResponse = res.body;
+
+        should.exist(jsonResponse);
+        should.exist(jsonResponse.meta);
+        should.exist(jsonResponse.meta.stats);
+
+        jsonResponse.meta.stats.imported.should.equal(10);
+        jsonResponse.meta.stats.invalid.length.should.equal(0);
+
+        assert(!!settingsCache.get('email_verification_required'), 'Email verification should now be required');
+
+        mockManager.assert.sentEmail({
+            subject: 'Email needs verification'
+        });
+    });
+
+    it('Can still import members once email verification is required but does not send email', async function () {
+        const res = await request
+            .post(localUtils.API.getApiQuery(`members/upload/`))
+            .field('labels', ['new-global-label'])
+            .attach('membersfile', path.join(__dirname, '/../../../utils/fixtures/csv/valid-members-import-large.csv'))
+            .set('Origin', config.get('url'))
+            .expect('Content-Type', /json/)
+            .expect('Cache-Control', testUtils.cacheRules.private)
+            .expect(201);
+        should.not.exist(res.headers['x-cache-invalidate']);
+        const jsonResponse = res.body;
+
+        should.exist(jsonResponse);
+        should.exist(jsonResponse.meta);
+        should.exist(jsonResponse.meta.stats);
+
+        jsonResponse.meta.stats.imported.should.equal(10);
+        jsonResponse.meta.stats.invalid.length.should.equal(0);
+
+        assert(!!settingsCache.get('email_verification_required'), 'Email verification should now be required');
+
+        // Don't send another email
+        mockManager.assert.sentEmailCount(0);
+    });
 });

ghost/core/test/utils/fixtures/csv/valid-members-import-large.csv

@@ -0,0 +1,11 @@
+email,name,note,subscribed_to_emails,labels
+test1@example.com,joe,,true,,
+test2@example.com,test,"test note",false,"test-label"
+test3@example.com,test,"test note",false,"test-label"
+test4@example.com,test,"test note",false,"test-label"
+test5@example.com,test,"test note",false,"test-label"
+test6@example.com,test,"test note",false,"test-label"
+test7@example.com,test,"test note",false,"test-label"
+test8@example.com,test,"test note",false,"test-label"
+test9@example.com,test,"test note",false,"test-label"
+test10@example.com,test,"test note",false,"test-label"

ghost/members-api/lib/repositories/event.js

@@ -294,13 +294,6 @@ module.exports = class EventRepository {
         };
     }
 
-    /**
-     * @deprecated Do not use
-     */
-    async getCreatedEvents(options = {}, filter) {
-        return await this.getSignupEvents(options, filter);
-    }
-
     async getSignupEvents(options = {}, filter) {
         options = {
             ...options,

ghost/verification-trigger/lib/verification-trigger.js

@@ -19,7 +19,7 @@ class VerificationTrigger {
      * @param {number} deps.importTriggerThreshold Threshold for triggering Import sourced verifications
      * @param {() => boolean} deps.isVerified Check Ghost config to see if we are already verified
      * @param {() => boolean} deps.isVerificationRequired Check Ghost settings to see whether verification has been requested
-     * @param {(content: {subject: string, message: string, amountTriggered: number}) => void} deps.sendVerificationEmail Sends an email to the escalation address to confirm that customer needs to be verified
+     * @param {(content: {subject: string, message: string, amountTriggered: number}) => Promise<void>} deps.sendVerificationEmail Sends an email to the escalation address to confirm that customer needs to be verified
      * @param {any} deps.membersStats MemberStats service
      * @param {any} deps.Settings Ghost Settings model
      * @param {any} deps.eventRepository For querying events
@@ -66,7 +66,7 @@ class VerificationTrigger {
         if (['api', 'admin'].includes(source) && isFinite(sourceThreshold)) {
             const createdAt = new Date();
             createdAt.setDate(createdAt.getDate() - 30);
-            const events = await this._eventRepository.getCreatedEvents({}, {
+            const events = await this._eventRepository.getSignupEvents({}, {
                 source: source,
                 created_at: {
                     $gt: createdAt.toISOString().replace('T', ' ').substring(0, 19)
@@ -101,7 +101,7 @@ class VerificationTrigger {
 
         const createdAt = new Date();
         createdAt.setDate(createdAt.getDate() - 30);
-        const events = await this._eventRepository.getCreatedEvents({}, {
+        const events = await this._eventRepository.getSignupEvents({}, {
             source: 'import',
             created_at: {
                 $gt: createdAt.toISOString().replace('T', ' ').substring(0, 19)
@@ -157,7 +157,7 @@ class VerificationTrigger {
                     verificationMessage = messages.emailVerificationEmailMessageAdmin;
                 }
 
-                this._sendVerificationEmail({
+                await this._sendVerificationEmail({
                     message: verificationMessage,
                     subject: messages.emailVerificationEmailSubject,
                     amountTriggered: amount

ghost/verification-trigger/test/verification-trigger.test.js

@@ -175,7 +175,7 @@ describe('Email verification flow', function () {
             isVerificationRequired: () => false,
             sendVerificationEmail: emailStub,
             eventRepository: {
-                getCreatedEvents: eventStub
+                getSignupEvents: eventStub
             }
         });
 
@@ -215,7 +215,7 @@ describe('Email verification flow', function () {
             isVerificationRequired: () => false,
             sendVerificationEmail: emailStub,
             eventRepository: {
-                getCreatedEvents: eventStub
+                getSignupEvents: eventStub
             }
         });
 
@@ -259,7 +259,7 @@ describe('Email verification flow', function () {
             isVerificationRequired: () => false,
             sendVerificationEmail: emailStub,
             eventRepository: {
-                getCreatedEvents: eventStub
+                getSignupEvents: eventStub
             }
         });
 
@@ -308,7 +308,7 @@ describe('Email verification flow', function () {
             isVerificationRequired: () => false,
             sendVerificationEmail: emailStub,
             eventRepository: {
-                getCreatedEvents: eventStub
+                getSignupEvents: eventStub
             }
         });
 
@@ -356,7 +356,7 @@ describe('Email verification flow', function () {
             isVerificationRequired: () => false,
             sendVerificationEmail: emailStub,
             eventRepository: {
-                getCreatedEvents: eventStub
+                getSignupEvents: eventStub
             }
         });