From 666baf8d50b15540d7ebeb39306a3748b6f8cb72 Mon Sep 17 00:00:00 2001
From: Naz <nazargargol@gmail.com>
Date: Wed, 2 Jun 2021 15:18:32 +0400
Subject: [PATCH] Removed GhostMailer parameter from UpdateCheckService

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

- This is continuation of the previous commit. TLDR: Passing only the necessary parameter data makes it easier to reason about what dependencies the UpdateCheckService has to deal with
- Instead of passing in a whole GhostMailer instance passing only an email sending function, which again - makes things way more manageable to reason about
- The end of refactor, next will be a move of the UpdateCheckService into a separate module in tryghost/core
---
 core/server/update-check-service.js           |  7 +++--
 core/server/update-check.js                   |  2 +-
 .../services/update_check_service_spec.js     | 31 ++++++-------------
 3 files changed, 15 insertions(+), 25 deletions(-)

diff --git a/core/server/update-check-service.js b/core/server/update-check-service.js
index 877cb1e8b4..cf0fa10f91 100644
--- a/core/server/update-check-service.js
+++ b/core/server/update-check-service.js
@@ -40,14 +40,15 @@ class UpdateCheckService {
      * @param {boolean} [options.config.forceUpdate]
      * @param {string} options.config.ghostVersion - Ghost instance version
      * @param {Function} options.request - a HTTP request proxy function
+     * @param {Function} options.sendEmail - function handling sending an email
     */
-    constructor({api, config, i18n, logging, request, ghostMailer}) {
+    constructor({api, config, i18n, logging, request, sendEmail}) {
         this.api = api;
         this.config = config;
         this.i18n = i18n;
         this.logging = logging;
         this.request = request;
-        this.ghostMailer = ghostMailer;
+        this.sendEmail = sendEmail;
     }
 
     nextCheckTimestamp() {
@@ -311,7 +312,7 @@ class UpdateCheckService {
             if (toAdd.type === 'alert') {
                 for (const email of adminEmails) {
                     try {
-                        this.ghostMailer.send({
+                        this.sendEmail({
                             to: email,
                             subject: `Action required: Critical alert from Ghost instance ${siteUrl}`,
                             html: toAdd.message,
diff --git a/core/server/update-check.js b/core/server/update-check.js
index f41b250930..991600f3bb 100644
--- a/core/server/update-check.js
+++ b/core/server/update-check.js
@@ -50,7 +50,7 @@ module.exports = () => {
             i18n,
             logging,
             request,
-            ghostMailer
+            sendEmail: ghostMailer.send
         });
     }
 
diff --git a/test/unit/services/update_check_service_spec.js b/test/unit/services/update_check_service_spec.js
index a8bc88d19d..c816bcb5c1 100644
--- a/test/unit/services/update_check_service_spec.js
+++ b/test/unit/services/update_check_service_spec.js
@@ -65,10 +65,7 @@ describe('Update Check', function () {
                 },
                 i18n: i18nStub,
                 logging: loggingStub,
-                request: requestStub,
-                ghostMailer: {
-                    send: sinon.stub().resolves()
-                }
+                request: requestStub
             });
 
             await updateCheckService.check();
@@ -129,10 +126,7 @@ describe('Update Check', function () {
                 },
                 i18n: i18nStub,
                 logging: loggingStub,
-                request: requestStub,
-                ghostMailer: {
-                    send: sinon.stub().resolves()
-                }
+                request: requestStub
             });
 
             await updateCheckService.check();
@@ -254,10 +248,7 @@ describe('Update Check', function () {
                     body: {
                         notifications: [notification]
                     }
-                }),
-                ghostMailer: {
-                    send: sinon.stub().resolves()
-                }
+                })
             });
 
             await updateCheckService.check();
@@ -287,9 +278,7 @@ describe('Update Check', function () {
             };
 
             const notificationsAPIAddStub = sinon.stub().resolves();
-            const mailServiceStub = {
-                send: sinon.stub().resolves()
-            };
+            const sendEmailStub = sinon.stub().resolves();
 
             const updateCheckService = new UpdateCheckService({
                 api: {
@@ -322,16 +311,16 @@ describe('Update Check', function () {
                 request: sinon.stub().resolves({
                     body: [notification]
                 }),
-                ghostMailer: mailServiceStub
+                sendEmail: sendEmailStub
             });
 
             await updateCheckService.check();
 
-            mailServiceStub.send.called.should.be.true();
-            mailServiceStub.send.args[0][0].to.should.equal('jbloggs@example.com');
-            mailServiceStub.send.args[0][0].subject.should.equal('Action required: Critical alert from Ghost instance http://127.0.0.1:2369');
-            mailServiceStub.send.args[0][0].html.should.equal('<p>Critical message. Upgrade your site!</p>');
-            mailServiceStub.send.args[0][0].forceTextContent.should.equal(true);
+            sendEmailStub.called.should.be.true();
+            sendEmailStub.args[0][0].to.should.equal('jbloggs@example.com');
+            sendEmailStub.args[0][0].subject.should.equal('Action required: Critical alert from Ghost instance http://127.0.0.1:2369');
+            sendEmailStub.args[0][0].html.should.equal('<p>Critical message. Upgrade your site!</p>');
+            sendEmailStub.args[0][0].forceTextContent.should.equal(true);
 
             notificationsAPIAddStub.calledOnce.should.equal(true);
             notificationsAPIAddStub.args[0][0].notifications.length.should.equal(1);