Refactored api version compatibility service API

refs https://github.com/TryGhost/Toolbox/issues/292

- The service used to take in a whole bunch of functions as parameters and did expect the client to know about the "version-notifications-data-service" which is not necessary and make the constructor API a notch complicated
- Putting in the data service initialization internally allows for the client to pass in less parameters and know less abou the internal working of the service - way easier to use!

ghost/api-version-compatibility-service/lib/api-version-compatibility-service.js

@@ -1,22 +1,24 @@
 const path = require('path');
+const VersionNotificationsDataService = require('@tryghost/version-notifications-data-service');
 const EmailContentGenerator = require('@tryghost/email-content-generator');
 
 class APIVersionCompatibilityService {
     /**
      *
      * @param {Object} options
+     * @param {Object} options.UserModel - ghost user model
+     * @param {Object} options.settingsService - ghost settings service
      * @param {(Object: {subject: String, to: String, text: String, html: String}) => Promise<any>} options.sendEmail - email sending function
-     * @param {() => Promise<any>} options.fetchEmailsToNotify - email address to receive notifications
-     * @param {(acceptVersion: String) => Promise<any>} options.fetchHandled - retrives already handled compatibility notifications
-     * @param {(acceptVersion: String) => Promise<any>} options.saveHandled - persists already handled compatibility notifications
      * @param {Function} options.getSiteUrl
      * @param {Function} options.getSiteTitle
     */
-    constructor({sendEmail, fetchEmailsToNotify, fetchHandled, saveHandled, getSiteUrl, getSiteTitle}) {
+    constructor({UserModel, settingsService, sendEmail, getSiteUrl, getSiteTitle}) {
         this.sendEmail = sendEmail;
-        this.fetchEmailsToNotify = fetchEmailsToNotify;
+
-        this.fetchHandled = fetchHandled;
+        this.versionNotificationsDataService = new VersionNotificationsDataService({
-        this.saveHandled = saveHandled;
+            UserModel,
+            settingsService
+        });
 
         this.emailContentGenerator = new EmailContentGenerator({
             getSiteUrl,
@@ -34,9 +36,9 @@ class APIVersionCompatibilityService {
      * @param {string} [options.userAgent] - client's user-agent header value
      */
     async handleMismatch({acceptVersion, contentVersion, requestURL, userAgent = ''}) {
-        if (!await this.fetchHandled(acceptVersion)) {
+        if (!await this.versionNotificationsDataService.fetchNotification(acceptVersion)) {
             const trimmedUseAgent = userAgent.split('/')[0];
-            const emails = await this.fetchEmailsToNotify();
+            const emails = await this.versionNotificationsDataService.getNotificationEmails();
 
             for (const email of emails) {
                 const template = (trimmedUseAgent === 'Zapier')
@@ -66,7 +68,7 @@ class APIVersionCompatibilityService {
                 });
             }
 
-            await this.saveHandled(acceptVersion);
+            await this.versionNotificationsDataService.saveNotification(acceptVersion);
         }
     }
 }

ghost/api-version-compatibility-service/test/api-version-compatibility-service.test.js

@@ -5,6 +5,40 @@ const APIVersionCompatibilityService = require('../index');
 describe('APIVersionCompatibilityService', function () {
     const getSiteUrl = () => 'https://amazeballsghostsite.com';
     const getSiteTitle = () => 'Tahini and chickpeas';
+    let UserModel;
+    let settingsService;
+
+    beforeEach(function () {
+        UserModel = {
+            findAll: sinon
+                .stub()
+                .withArgs({
+                    withRelated: ['roles'],
+                    filter: 'status:active'
+                }, {
+                    internal: true
+                })
+                .resolves({
+                    toJSON: () => [{
+                        email: 'simon@example.com',
+                        roles: [{
+                            name: 'Administrator'
+                        }]
+                    }]
+                })
+        };
+        settingsService = {
+            read: sinon.stub().resolves({
+                version_notifications: {
+                    value: JSON.stringify([
+                        'v3.4',
+                        'v4.1'
+                    ])
+                }
+            }),
+            edit: sinon.stub().resolves()
+        };
+    });
 
     afterEach(function () {
         sinon.reset();
@@ -12,14 +46,10 @@ describe('APIVersionCompatibilityService', function () {
 
     it('Sends an email to the instance owners when fresh accept-version header mismatch detected', async function () {
         const sendEmail = sinon.spy();
-        const fetchHandled = sinon.spy();
-        const saveHandled = sinon.spy();
-
         const compatibilityService = new APIVersionCompatibilityService({
+            UserModel,
+            settingsService,
             sendEmail,
-            fetchEmailsToNotify: async () => ['test_env@example.com'],
-            fetchHandled,
-            saveHandled,
             getSiteUrl,
             getSiteTitle
         });
@@ -32,7 +62,7 @@ describe('APIVersionCompatibilityService', function () {
         });
 
         assert.equal(sendEmail.called, true);
-        assert.equal(sendEmail.args[0][0].to, 'test_env@example.com');
+        assert.equal(sendEmail.args[0][0].to, 'simon@example.com');
         assert.equal(sendEmail.args[0][0].subject, `Attention required: Your Elaborate Fox integration has failed`);
 
         assert.match(sendEmail.args[0][0].html, /Ghost has noticed that your <strong style="font-weight: 600;">Elaborate Fox<\/strong> is no longer working as expected\./);
@@ -41,7 +71,7 @@ describe('APIVersionCompatibilityService', function () {
         assert.match(sendEmail.args[0][0].html, /Failed request URL:<\/strong>&nbsp; https:\/\/amazeballsghostsite.com\/ghost\/api\/admin\/posts\/dew023d9203se4/);
 
         assert.match(sendEmail.args[0][0].html, /This email was sent from <a href="https:\/\/amazeballsghostsite.com"/);
-        assert.match(sendEmail.args[0][0].html, /to <a href="mailto:test_env@example.com"/);
+        assert.match(sendEmail.args[0][0].html, /to <a href="mailto:simon@example.com"/);
 
         assert.match(sendEmail.args[0][0].text, /Ghost has noticed that your Elaborate Fox is no longer working as expected\./);
         assert.match(sendEmail.args[0][0].text, /Elaborate Fox integration expected Ghost version:v4.5/);
@@ -50,22 +80,37 @@ describe('APIVersionCompatibilityService', function () {
         assert.match(sendEmail.args[0][0].text, /https:\/\/amazeballsghostsite.com\/ghost\/api\/admin\/posts\/dew023d9203se4/);
 
         assert.match(sendEmail.args[0][0].text, /This email was sent from https:\/\/amazeballsghostsite.com/);
-        assert.match(sendEmail.args[0][0].text, /to test_env@example.com/);
+        assert.match(sendEmail.args[0][0].text, /to simon@example.com/);
     });
 
     it('Does NOT send an email to the instance owner when previously handled accept-version header mismatch is detected', async function () {
         const sendEmail = sinon.spy();
-        const fetchHandled = sinon.stub()
+        settingsService = {
-            .onFirstCall().resolves(null)
+            read: sinon.stub()
-            .onSecondCall().resolves({});
+                .onFirstCall().resolves({
-
+                    version_notifications: {
-        const saveHandled = sinon.stub().resolves({});
+                        value: JSON.stringify([])
+                    }
+                })
+                .onSecondCall().resolves({
+                    version_notifications: {
+                        value: JSON.stringify([])
+                    }
+                })
+                .onThirdCall().resolves({
+                    version_notifications: {
+                        value: JSON.stringify([
+                            'v4.5'
+                        ])
+                    }
+                }),
+            edit: sinon.stub().resolves()
+        };
 
         const compatibilityService = new APIVersionCompatibilityService({
             sendEmail,
-            fetchEmailsToNotify: async () => ['test_env@example.com'],
+            UserModel,
-            fetchHandled,
+            settingsService,
-            saveHandled,
             getSiteUrl,
             getSiteTitle
         });
@@ -78,7 +123,7 @@ describe('APIVersionCompatibilityService', function () {
         });
 
         assert.equal(sendEmail.called, true);
-        assert.equal(sendEmail.args[0][0].to, 'test_env@example.com');
+        assert.equal(sendEmail.args[0][0].to, 'simon@example.com');
         assert.equal(sendEmail.args[0][0].subject, `Attention required: Your Elaborate Fox integration has failed`);
 
         assert.match(sendEmail.args[0][0].html, /Ghost has noticed that your <strong style="font-weight: 600;">Elaborate Fox<\/strong> is no longer working as expected\./);
@@ -87,7 +132,7 @@ describe('APIVersionCompatibilityService', function () {
         assert.match(sendEmail.args[0][0].html, /Failed request URL:<\/strong>&nbsp; https:\/\/amazeballsghostsite.com\/ghost\/api\/admin\/posts\/dew023d9203se4/);
 
         assert.match(sendEmail.args[0][0].html, /This email was sent from <a href="https:\/\/amazeballsghostsite.com"/);
-        assert.match(sendEmail.args[0][0].html, /to <a href="mailto:test_env@example.com"/);
+        assert.match(sendEmail.args[0][0].html, /to <a href="mailto:simon@example.com"/);
 
         assert.match(sendEmail.args[0][0].text, /Ghost has noticed that your Elaborate Fox is no longer working as expected\./);
         assert.match(sendEmail.args[0][0].text, /Elaborate Fox integration expected Ghost version:v4.5/);
@@ -96,7 +141,7 @@ describe('APIVersionCompatibilityService', function () {
         assert.match(sendEmail.args[0][0].text, /https:\/\/amazeballsghostsite.com\/ghost\/api\/admin\/posts\/dew023d9203se4/);
 
         assert.match(sendEmail.args[0][0].text, /This email was sent from https:\/\/amazeballsghostsite.com/);
-        assert.match(sendEmail.args[0][0].text, /to test_env@example.com/);
+        assert.match(sendEmail.args[0][0].text, /to simon@example.com/);
 
         await compatibilityService.handleMismatch({
             acceptVersion: 'v4.5',
@@ -105,22 +150,68 @@ describe('APIVersionCompatibilityService', function () {
             requestURL: 'does not matter'
         });
 
+        assert.equal(sendEmail.calledOnce, true);
         assert.equal(sendEmail.calledTwice, false);
     });
 
     it('Does send multiple emails to the instance owners when previously unhandled accept-version header mismatch is detected', async function () {
         const sendEmail = sinon.spy();
-        const fetchHandled = sinon.stub()
+        UserModel = {
-            .onFirstCall().resolves(null)
+            findAll: sinon
-            .onSecondCall().resolves(null);
+                .stub()
-
+                .withArgs({
-        const saveHandled = sinon.stub().resolves({});
+                    withRelated: ['roles'],
+                    filter: 'status:active'
+                }, {
+                    internal: true
+                })
+                .resolves({
+                    toJSON: () => [{
+                        email: 'simon@example.com',
+                        roles: [{
+                            name: 'Administrator'
+                        }]
+                    }, {
+                        email: 'sam@example.com',
+                        roles: [{
+                            name: 'Owner'
+                        }]
+                    }]
+                })
+        };
+        settingsService = {
+            read: sinon.stub()
+                .onCall(0).resolves({
+                    version_notifications: {
+                        value: JSON.stringify([])
+                    }
+                })
+                .onCall(1).resolves({
+                    version_notifications: {
+                        value: JSON.stringify([])
+                    }
+                })
+                .onCall(2).resolves({
+                    version_notifications: {
+                        value: JSON.stringify([
+                            'v4.5'
+                        ])
+                    }
+                })
+                .onCall(3).resolves({
+                    version_notifications: {
+                        value: JSON.stringify([
+                            'v4.5'
+                        ])
+                    }
+                }),
+            edit: sinon.stub().resolves()
+        };
 
         const compatibilityService = new APIVersionCompatibilityService({
             sendEmail,
-            fetchEmailsToNotify: async () => ['test_env@example.com', 'test_env2@example.com'],
+            UserModel,
-            fetchHandled,
+            settingsService,
-            saveHandled,
             getSiteUrl,
             getSiteTitle
         });
@@ -133,7 +224,7 @@ describe('APIVersionCompatibilityService', function () {
         });
 
         assert.equal(sendEmail.calledTwice, true);
-        assert.equal(sendEmail.args[0][0].to, 'test_env@example.com');
+        assert.equal(sendEmail.args[0][0].to, 'simon@example.com');
         assert.equal(sendEmail.args[0][0].subject, `Attention required: Your Elaborate Fox integration has failed`);
 
         assert.match(sendEmail.args[0][0].html, /Ghost has noticed that your <strong style="font-weight: 600;">Elaborate Fox<\/strong> is no longer working as expected\./);
@@ -142,7 +233,7 @@ describe('APIVersionCompatibilityService', function () {
         assert.match(sendEmail.args[0][0].html, /Failed request URL:<\/strong>&nbsp; https:\/\/amazeballsghostsite.com\/ghost\/api\/admin\/posts\/dew023d9203se4/);
 
         assert.match(sendEmail.args[0][0].html, /This email was sent from <a href="https:\/\/amazeballsghostsite.com"/);
-        assert.match(sendEmail.args[0][0].html, /to <a href="mailto:test_env@example.com"/);
+        assert.match(sendEmail.args[0][0].html, /to <a href="mailto:simon@example.com"/);
 
         assert.match(sendEmail.args[0][0].text, /Ghost has noticed that your Elaborate Fox is no longer working as expected\./);
         assert.match(sendEmail.args[0][0].text, /Elaborate Fox integration expected Ghost version:v4.5/);
@@ -151,10 +242,10 @@ describe('APIVersionCompatibilityService', function () {
         assert.match(sendEmail.args[0][0].text, /https:\/\/amazeballsghostsite.com\/ghost\/api\/admin\/posts\/dew023d9203se4/);
 
         assert.match(sendEmail.args[0][0].text, /This email was sent from https:\/\/amazeballsghostsite.com/);
-        assert.match(sendEmail.args[0][0].text, /to test_env@example.com/);
+        assert.match(sendEmail.args[0][0].text, /to simon@example.com/);
 
         assert.equal(sendEmail.calledTwice, true);
-        assert.equal(sendEmail.args[1][0].to, 'test_env2@example.com');
+        assert.equal(sendEmail.args[1][0].to, 'sam@example.com');
         assert.equal(sendEmail.args[1][0].subject, `Attention required: Your Elaborate Fox integration has failed`);
 
         assert.match(sendEmail.args[1][0].html, /Ghost has noticed that your <strong style="font-weight: 600;">Elaborate Fox<\/strong> is no longer working as expected\./);
@@ -163,7 +254,7 @@ describe('APIVersionCompatibilityService', function () {
         assert.match(sendEmail.args[1][0].html, /Failed request URL:<\/strong>&nbsp; https:\/\/amazeballsghostsite.com\/ghost\/api\/admin\/posts\/dew023d9203se4/);
 
         assert.match(sendEmail.args[1][0].html, /This email was sent from <a href="https:\/\/amazeballsghostsite.com"/);
-        assert.match(sendEmail.args[1][0].html, /to <a href="mailto:test_env2@example.com"/);
+        assert.match(sendEmail.args[1][0].html, /to <a href="mailto:sam@example.com"/);
 
         assert.match(sendEmail.args[1][0].text, /Ghost has noticed that your Elaborate Fox is no longer working as expected\./);
         assert.match(sendEmail.args[1][0].text, /Elaborate Fox integration expected Ghost version:v4.5/);
@@ -172,7 +263,7 @@ describe('APIVersionCompatibilityService', function () {
         assert.match(sendEmail.args[1][0].text, /https:\/\/amazeballsghostsite.com\/ghost\/api\/admin\/posts\/dew023d9203se4/);
 
         assert.match(sendEmail.args[1][0].text, /This email was sent from https:\/\/amazeballsghostsite.com/);
-        assert.match(sendEmail.args[1][0].text, /to test_env2@example.com/);
+        assert.match(sendEmail.args[1][0].text, /to sam@example.com/);
 
         await compatibilityService.handleMismatch({
             acceptVersion: 'v4.8',
@@ -182,7 +273,7 @@ describe('APIVersionCompatibilityService', function () {
         });
 
         assert.equal(sendEmail.callCount, 4);
-        assert.equal(sendEmail.args[2][0].to, 'test_env@example.com');
+        assert.equal(sendEmail.args[2][0].to, 'simon@example.com');
 
         assert.match(sendEmail.args[2][0].html, /Ghost has noticed that your <strong style="font-weight: 600;">Elaborate Fox<\/strong> is no longer working as expected\./);
         assert.match(sendEmail.args[2][0].html, /Elaborate Fox integration expected Ghost version:<\/strong>&nbsp; v4.8/);
@@ -198,14 +289,11 @@ describe('APIVersionCompatibilityService', function () {
 
     it('Trims down the name of the integration when a lot of meta information is present in user-agent header', async function (){
         const sendEmail = sinon.spy();
-        const fetchHandled = sinon.spy();
-        const saveHandled = sinon.spy();
 
         const compatibilityService = new APIVersionCompatibilityService({
             sendEmail,
-            fetchEmailsToNotify: async () => ['test_env@example.com'],
+            UserModel,
-            fetchHandled,
+            settingsService,
-            saveHandled,
             getSiteUrl,
             getSiteTitle
         });
@@ -218,7 +306,7 @@ describe('APIVersionCompatibilityService', function () {
         });
 
         assert.equal(sendEmail.called, true);
-        assert.equal(sendEmail.args[0][0].to, 'test_env@example.com');
+        assert.equal(sendEmail.args[0][0].to, 'simon@example.com');
         assert.equal(sendEmail.args[0][0].subject, `Attention required: Your Fancy Pants integration has failed`);
 
         assert.match(sendEmail.args[0][0].html, /Ghost has noticed that your <strong style="font-weight: 600;">Fancy Pants<\/strong> is no longer working as expected\./);
@@ -227,7 +315,7 @@ describe('APIVersionCompatibilityService', function () {
         assert.match(sendEmail.args[0][0].html, /Failed request URL:<\/strong>&nbsp; https:\/\/amazeballsghostsite.com\/ghost\/api\/admin\/posts\/dew023d9203se4/);
 
         assert.match(sendEmail.args[0][0].html, /This email was sent from <a href="https:\/\/amazeballsghostsite.com"/);
-        assert.match(sendEmail.args[0][0].html, /to <a href="mailto:test_env@example.com"/);
+        assert.match(sendEmail.args[0][0].html, /to <a href="mailto:simon@example.com"/);
 
         assert.match(sendEmail.args[0][0].text, /Ghost has noticed that your Fancy Pants is no longer working as expected\./);
         assert.match(sendEmail.args[0][0].text, /Fancy Pants integration expected Ghost version:v4.5/);
@@ -236,19 +324,16 @@ describe('APIVersionCompatibilityService', function () {
         assert.match(sendEmail.args[0][0].text, /https:\/\/amazeballsghostsite.com\/ghost\/api\/admin\/posts\/dew023d9203se4/);
 
         assert.match(sendEmail.args[0][0].text, /This email was sent from https:\/\/amazeballsghostsite.com/);
-        assert.match(sendEmail.args[0][0].text, /to test_env@example.com/);
+        assert.match(sendEmail.args[0][0].text, /to simon@example.com/);
     });
 
     it('Sends Zapier-specific email when userAgent is a Zapier client', async function (){
         const sendEmail = sinon.spy();
-        const fetchHandled = sinon.spy();
-        const saveHandled = sinon.spy();
 
         const compatibilityService = new APIVersionCompatibilityService({
             sendEmail,
-            fetchEmailsToNotify: async () => ['test_env@example.com'],
+            UserModel,
-            fetchHandled,
+            settingsService,
-            saveHandled,
             getSiteUrl,
             getSiteTitle
         });
@@ -261,19 +346,19 @@ describe('APIVersionCompatibilityService', function () {
         });
 
         assert.equal(sendEmail.called, true);
-        assert.equal(sendEmail.args[0][0].to, 'test_env@example.com');
+        assert.equal(sendEmail.args[0][0].to, 'simon@example.com');
         assert.equal(sendEmail.args[0][0].subject, `Attention required: One of your Zaps has failed`);
 
         assert.match(sendEmail.args[0][0].html, /Ghost has noticed that one of the Zaps in your Zapier integration has <span style="font-weight: 600;">stopped working<\/span>\./);
         assert.match(sendEmail.args[0][0].html, /To get this resolved as quickly as possible, please log in to your Zapier account to view any failing Zaps and recreate them using the most recent Ghost-supported versions. Zap errors can be found <a href="https:\/\/zapier.com\/app\/history\/usage" style="color: #738A94;">here<\/a> in your “Zap history”\./);
 
         assert.match(sendEmail.args[0][0].html, /This email was sent from <a href="https:\/\/amazeballsghostsite.com"/);
-        assert.match(sendEmail.args[0][0].html, /to <a href="mailto:test_env@example.com"/);
+        assert.match(sendEmail.args[0][0].html, /to <a href="mailto:simon@example.com"/);
 
         assert.match(sendEmail.args[0][0].text, /Ghost has noticed that one of the Zaps in your Zapier integration has stopped/);
         assert.match(sendEmail.args[0][0].text, /To get this resolved as quickly as possible, please log in to your Zapier/);
 
         assert.match(sendEmail.args[0][0].text, /This email was sent from https:\/\/amazeballsghostsite.com/);
-        assert.match(sendEmail.args[0][0].text, /to test_env@example.com/);
+        assert.match(sendEmail.args[0][0].text, /to simon@example.com/);
     });
 });