0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-03 23:00:14 -05:00

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)
This commit is contained in:
Naz 2021-06-02 14:31:07 +04:00
parent 2e7d0a4e26
commit bd51dd09db
3 changed files with 12 additions and 13 deletions

View file

@ -38,13 +38,14 @@ class UpdateCheckService {
* @param {string[]} [options.config.notificationGroups] - example values ["migration", "something"] * @param {string[]} [options.config.notificationGroups] - example values ["migration", "something"]
* @param {string} options.config.siteUrl - Ghost instance URL * @param {string} options.config.siteUrl - Ghost instance URL
* @param {boolean} [options.config.forceUpdate] * @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.api = api;
this.config = config; this.config = config;
this.i18n = i18n; this.i18n = i18n;
this.logging = logging; this.logging = logging;
this.urlUtils = urlUtils; this.urlFor = urlFor;
this.request = request; this.request = request;
this.ghostVersion = ghostVersion; this.ghostVersion = ghostVersion;
this.ghostMailer = ghostMailer; this.ghostMailer = ghostMailer;
@ -105,7 +106,7 @@ class UpdateCheckService {
const users = await this.api.users.browse(internal); const users = await this.api.users.browse(internal);
const npm = await Promise.promisify(exec)('npm -v'); 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 parsedBlogUrl = url.parse(blogUrl);
const blogId = parsedBlogUrl.hostname + parsedBlogUrl.pathname.replace(/\//, '') + hash.value; const blogId = parsedBlogUrl.hostname + parsedBlogUrl.pathname.replace(/\//, '') + hash.value;

View file

@ -48,7 +48,7 @@ module.exports = () => {
}, },
i18n, i18n,
logging, logging,
urlUtils, urlFor: urlUtils.urlFor,
request, request,
ghostVersion, ghostVersion,
ghostMailer ghostMailer

View file

@ -2,7 +2,6 @@ const should = require('should');
const sinon = require('sinon'); const sinon = require('sinon');
const moment = require('moment'); const moment = require('moment');
const uuid = require('uuid'); const uuid = require('uuid');
const urlUtils = require('../../utils/urlUtils');
const packageInfo = require('../../../package.json'); const packageInfo = require('../../../package.json');
const ghostVersion = require('../../../core/server/lib/ghost-version'); const ghostVersion = require('../../../core/server/lib/ghost-version');
@ -14,7 +13,7 @@ describe('Update Check', function () {
let i18nStub; let i18nStub;
let loggingStub; let loggingStub;
let requestStub; let requestStub;
let urlUtilsStub; let urlForStub;
beforeEach(function () { beforeEach(function () {
settingsStub = sinon.stub().resolves({ settingsStub = sinon.stub().resolves({
@ -40,12 +39,11 @@ describe('Update Check', function () {
}; };
requestStub = sinon.stub(); requestStub = sinon.stub();
urlUtilsStub = urlUtils.stubUrlUtilsFromConfig(); urlForStub = sinon.stub().returns('https://localhost:2368/');
}); });
afterEach(function () { afterEach(function () {
sinon.restore(); sinon.restore();
urlUtils.restore();
}); });
describe('UpdateCheck execution', function () { describe('UpdateCheck execution', function () {
@ -69,7 +67,7 @@ describe('Update Check', function () {
}, },
i18n: i18nStub, i18n: i18nStub,
logging: loggingStub, logging: loggingStub,
urlUtils: urlUtilsStub, urlFor: urlForStub,
request: requestStub, request: requestStub,
ghostVersion, ghostVersion,
ghostMailer: { ghostMailer: {
@ -133,7 +131,7 @@ describe('Update Check', function () {
}, },
i18n: i18nStub, i18n: i18nStub,
logging: loggingStub, logging: loggingStub,
urlUtils: urlUtilsStub, urlFor: urlForStub,
request: requestStub, request: requestStub,
ghostVersion, ghostVersion,
ghostMailer: { ghostMailer: {
@ -183,7 +181,7 @@ describe('Update Check', function () {
}, },
i18n: i18nStub, i18n: i18nStub,
logging: loggingStub, logging: loggingStub,
urlUtils: urlUtilsStub, urlFor: urlForStub,
request: requestStub, request: requestStub,
ghostVersion, ghostVersion,
ghostMailer: { ghostMailer: {
@ -254,7 +252,7 @@ describe('Update Check', function () {
config: {}, config: {},
i18n: i18nStub, i18n: i18nStub,
logging: loggingStub, logging: loggingStub,
urlUtils: urlUtilsStub, urlFor: urlForStub,
request: sinon.stub().resolves({ request: sinon.stub().resolves({
body: { body: {
notifications: [notification] notifications: [notification]
@ -325,7 +323,7 @@ describe('Update Check', function () {
}, },
i18n: i18nStub, i18n: i18nStub,
logging: loggingStub, logging: loggingStub,
urlUtils: urlUtilsStub, urlFor: urlForStub,
request: sinon.stub().resolves({ request: sinon.stub().resolves({
body: [notification] body: [notification]
}), }),