From bd51dd09db98c8ae90cf0944ce18c7615641b472 Mon Sep 17 00:00:00 2001
From: Naz <nazargargol@gmail.com>
Date: Wed, 2 Jun 2021 14:31:07 +0400
Subject: [PATCH] Limited the API surface of the UpdateCheckService

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

- This is continuation of the previous commit. TLDR: Passing only the necessary API endpoint function makes it easier to reason about what dependencies the UpdateCheckService has to deal with
- Limited urlUtils to only one function as that's all the UpdateCheck uses. Next step will be removing the function completely as and passing a 'blogURL' as a config value (way better readability this way)
---
 core/server/update-check-service.js             |  7 ++++---
 core/server/update-check.js                     |  2 +-
 test/unit/services/update_check_service_spec.js | 16 +++++++---------
 3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/core/server/update-check-service.js b/core/server/update-check-service.js
index 9f62cb3a65..9e277a31ac 100644
--- a/core/server/update-check-service.js
+++ b/core/server/update-check-service.js
@@ -38,13 +38,14 @@ class UpdateCheckService {
      * @param {string[]} [options.config.notificationGroups] - example values ["migration", "something"]
      * @param {string} options.config.siteUrl - Ghost instance URL
      * @param {boolean} [options.config.forceUpdate]
+     * @param {Function} urlFor - function creating a URL for a certain context
     */
-    constructor({api, config, i18n, logging, urlUtils, request, ghostVersion, ghostMailer}) {
+    constructor({api, config, i18n, logging, urlFor, request, ghostVersion, ghostMailer}) {
         this.api = api;
         this.config = config;
         this.i18n = i18n;
         this.logging = logging;
-        this.urlUtils = urlUtils;
+        this.urlFor = urlFor;
         this.request = request;
         this.ghostVersion = ghostVersion;
         this.ghostMailer = ghostMailer;
@@ -105,7 +106,7 @@ class UpdateCheckService {
             const users = await this.api.users.browse(internal);
             const npm = await Promise.promisify(exec)('npm -v');
 
-            const blogUrl = this.urlUtils.urlFor('home', true);
+            const blogUrl = this.urlFor('home', true);
             const parsedBlogUrl = url.parse(blogUrl);
             const blogId = parsedBlogUrl.hostname + parsedBlogUrl.pathname.replace(/\//, '') + hash.value;
 
diff --git a/core/server/update-check.js b/core/server/update-check.js
index 04076c0f13..48f41203dd 100644
--- a/core/server/update-check.js
+++ b/core/server/update-check.js
@@ -48,7 +48,7 @@ module.exports = () => {
             },
             i18n,
             logging,
-            urlUtils,
+            urlFor: urlUtils.urlFor,
             request,
             ghostVersion,
             ghostMailer
diff --git a/test/unit/services/update_check_service_spec.js b/test/unit/services/update_check_service_spec.js
index 81b55f6b7f..3bdc825609 100644
--- a/test/unit/services/update_check_service_spec.js
+++ b/test/unit/services/update_check_service_spec.js
@@ -2,7 +2,6 @@ const should = require('should');
 const sinon = require('sinon');
 const moment = require('moment');
 const uuid = require('uuid');
-const urlUtils = require('../../utils/urlUtils');
 const packageInfo = require('../../../package.json');
 const ghostVersion = require('../../../core/server/lib/ghost-version');
 
@@ -14,7 +13,7 @@ describe('Update Check', function () {
     let i18nStub;
     let loggingStub;
     let requestStub;
-    let urlUtilsStub;
+    let urlForStub;
 
     beforeEach(function () {
         settingsStub = sinon.stub().resolves({
@@ -40,12 +39,11 @@ describe('Update Check', function () {
         };
 
         requestStub = sinon.stub();
-        urlUtilsStub = urlUtils.stubUrlUtilsFromConfig();
+        urlForStub = sinon.stub().returns('https://localhost:2368/');
     });
 
     afterEach(function () {
         sinon.restore();
-        urlUtils.restore();
     });
 
     describe('UpdateCheck execution', function () {
@@ -69,7 +67,7 @@ describe('Update Check', function () {
                 },
                 i18n: i18nStub,
                 logging: loggingStub,
-                urlUtils: urlUtilsStub,
+                urlFor: urlForStub,
                 request: requestStub,
                 ghostVersion,
                 ghostMailer: {
@@ -133,7 +131,7 @@ describe('Update Check', function () {
                 },
                 i18n: i18nStub,
                 logging: loggingStub,
-                urlUtils: urlUtilsStub,
+                urlFor: urlForStub,
                 request: requestStub,
                 ghostVersion,
                 ghostMailer: {
@@ -183,7 +181,7 @@ describe('Update Check', function () {
                 },
                 i18n: i18nStub,
                 logging: loggingStub,
-                urlUtils: urlUtilsStub,
+                urlFor: urlForStub,
                 request: requestStub,
                 ghostVersion,
                 ghostMailer: {
@@ -254,7 +252,7 @@ describe('Update Check', function () {
                 config: {},
                 i18n: i18nStub,
                 logging: loggingStub,
-                urlUtils: urlUtilsStub,
+                urlFor: urlForStub,
                 request: sinon.stub().resolves({
                     body: {
                         notifications: [notification]
@@ -325,7 +323,7 @@ describe('Update Check', function () {
                 },
                 i18n: i18nStub,
                 logging: loggingStub,
-                urlUtils: urlUtilsStub,
+                urlFor: urlForStub,
                 request: sinon.stub().resolves({
                     body: [notification]
                 }),