diff --git a/core/server/api/v2/notifications.js b/core/server/api/v2/notifications.js index 96a54cd2c7..ea36b8a1d7 100644 --- a/core/server/api/v2/notifications.js +++ b/core/server/api/v2/notifications.js @@ -85,7 +85,7 @@ module.exports = { }; let notificationsToCheck = frame.data.notifications; - let addedNotifications = []; + let notificationsToAdd = []; const allNotifications = _private.fetchAllNotifications(); @@ -95,7 +95,7 @@ module.exports = { }); if (!isDuplicate) { - addedNotifications.push(Object.assign({}, defaults, notification, overrides)); + notificationsToAdd.push(Object.assign({}, defaults, notification, overrides)); } }); @@ -111,29 +111,30 @@ module.exports = { } // CASE: nothing to add, skip - if (!addedNotifications.length) { + if (!notificationsToAdd.length) { return Promise.resolve(); } - const addedReleaseNotifications = addedNotifications.filter((notification) => { + const releaseNotificationsToAdd = notificationsToAdd.filter((notification) => { return !notification.custom; }); - // CASE: only latest release notification - if (addedReleaseNotifications.length > 1) { - addedNotifications = addedNotifications.filter((notification) => { + // CASE: reorder notifications before save + if (releaseNotificationsToAdd.length > 1) { + notificationsToAdd = notificationsToAdd.filter((notification) => { return notification.custom; }); - addedNotifications.push(_.orderBy(addedReleaseNotifications, 'created_at', 'desc')[0]); + notificationsToAdd.push(_.orderBy(releaseNotificationsToAdd, 'created_at', 'desc')[0]); } return api.settings.edit({ settings: [{ key: 'notifications', - value: allNotifications.concat(addedNotifications) + // @NOTE: We always need to store all notifications! + value: allNotifications.concat(notificationsToAdd) }] }, internalContext).then(() => { - return addedNotifications; + return notificationsToAdd; }); } }, @@ -175,6 +176,7 @@ module.exports = { return Promise.resolve(); } + // @NOTE: We don't remove the notifications, because otherwise we will receive them again from the service. allNotifications[notificationToMarkAsSeenIndex].seen = true; return api.settings.edit({ @@ -195,6 +197,7 @@ module.exports = { const allNotifications = _private.fetchAllNotifications(); allNotifications.forEach((notification) => { + // @NOTE: We don't remove the notifications, because otherwise we will receive them again from the service. notification.seen = true; }); diff --git a/core/server/update-check.js b/core/server/update-check.js index 34ab30b8a8..2a7d7cfbbb 100644 --- a/core/server/update-check.js +++ b/core/server/update-check.js @@ -40,6 +40,18 @@ function nextCheckTimestamp() { return now + (24 * 3600); } +/** + * @description Centralised error handler for the update check unit. + * + * CASES: + * - the update check service returns an error + * - error during collecting blog stats + * + * We still need to ensure that we set the "next_update_check" to a new value, otherwise no more + * update checks will happen. + * + * @param err + */ function updateCheckError(err) { api.settings.edit({ settings: [{ @@ -54,9 +66,10 @@ function updateCheckError(err) { } /** - * If the custom message is intended for current version, create and store a custom notification. + * @description Create a Ghost notification and call the API controller. + * * @param {Object} notification - * @return {*|Promise} + * @return {Promise} */ function createCustomNotification(notification) { if (!notification) { @@ -65,6 +78,7 @@ function createCustomNotification(notification) { return Promise.each(notification.messages, function (message) { let toAdd = { + // @NOTE: the update check service returns "0" or "1" (https://github.com/TryGhost/UpdateCheck/issues/43) custom: !!notification.custom, createdAt: moment(notification.created_at).toDate(), status: message.status || 'alert', @@ -80,6 +94,10 @@ function createCustomNotification(notification) { }); } +/** + * @description Collect stats from your blog. + * @returns {Promise} + */ function updateCheckData() { let data = {}, mailConfig = config.get('mail'); @@ -121,8 +139,13 @@ function updateCheckData() { } /** - * With the privacy setting `useUpdateCheck` you can control if you want to expose data from your blog to the - * Update Check Service. Enabled or disabled, you will receive the latest notification available from the service. + * @description Perform request to update check service. + * + * With the privacy setting `useUpdateCheck` you can control if you want to expose data/stats from your blog to the + * service. Enabled or disabled, you will receive the latest notification available from the service. + * + * @see https://docs.ghost.org/concepts/config/#privacy + * @returns {Promise} */ function updateCheckRequest() { return updateCheckData() @@ -134,6 +157,7 @@ function updateCheckRequest() { checkEndpoint = config.get('updateCheck:url'), checkMethod = config.isPrivacyDisabled('useUpdateCheck') ? 'GET' : 'POST'; + // CASE: Expose stats and do a check-in if (checkMethod === 'POST') { reqObj.json = true; reqObj.body = reqData; @@ -161,6 +185,7 @@ function updateCheckRequest() { }; } + // CASE: service returns JSON error, deserialize into JS error if (err.response && err.response.body && typeof err.response.body === 'object') { err = common.errors.utils.deserialize(err.response.body); } @@ -171,24 +196,39 @@ function updateCheckRequest() { } /** - * Handles the response from the update check - * Does three things with the information received: - * 1. Updates the time we can next make a check - * 2. Create custom notifications is response from UpdateCheck as "messages" array which has the following structure: + * @description This function handles the response from the update check service. + * + * The helper does three things: + * + * 1. Updates the time in the settings table to know when we can execute the next update check request. + * 2. Iterates over the received notifications and filters them out based on your notification groups. + * 3. Calls a custom helper to generate a Ghost notification for the database. + * + * The structure of the response is: + * + * { + * id: 20, + * version: 'all4', + * messages: + * [{ + * id: 'f8ff6c80-aa61-11e7-a126-6119te37e2b8', + * version: '^2', + * content: 'Hallouuuu custom', + * top: true, + * dismissible: true, + * type: 'info' + * }], + * created_at: '2021-10-06T07:00:00.000Z', + * custom: 1, + * next_check: 1555608722 + * } * - * "messages": [{ - * "id": ed9dc38c-73e5-4d72-a741-22b11f6e151a, - * "version": "0.5.x", - * "content": "

Hey there! 0.6 is available, visit Ghost.org to grab your copy now", - * "dismissible": true | false, - * "top": true | false - * ]} * * Example for grouped custom notifications in config: * - * notificationGroups: ['migration', 'something'] + * "notificationGroups": ["migration", "something"] * - * 'all' is a reserved name for general custom notifications. + * The group 'all' is a reserved name for general custom notifications, which every self hosted blog can receive. * * @param {Object} response * @return {Promise} @@ -202,22 +242,33 @@ function updateCheckResponse(response) { return api.settings.edit({settings: [{key: 'next_update_check', value: response.next_check}]}, internal) .then(function () { - // CASE: Update Check Service returns multiple notifications. + /** + * @NOTE: + * + * When we refactored notifications in Ghost 1.20, the service did not support returning multiple messages. + * But we wanted to already add the support for future functionality. + * That's why this helper supports two ways: returning an array of messages or returning an object with + * a "notifications" key. The second one is probably the best, because we need to support "next_check" + * on the root level of the response. + */ if (_.isArray(response)) { notifications = response; } else if ((response.hasOwnProperty('notifications') && _.isArray(response.notifications))) { notifications = response.notifications; } else { + // CASE: default right now notifications = [response]; } // CASE: Hook into received notifications and decide whether you are allowed to receive custom group messages. if (notificationGroups.length) { notifications = notifications.filter(function (notification) { + // CASE: release notification, keep if (!notification.custom) { return true; } + // CASE: filter out messages based on your groups return _.includes(notificationGroups.map(function (groupIdentifier) { if (notification.version.match(new RegExp(groupIdentifier))) { return true; @@ -232,6 +283,14 @@ function updateCheckResponse(response) { }); } +/** + * @description Entry point to trigger the update check unit. + * + * Based on a settings value, we check if `next_update_check` is less than now to decide whether + * we should request the update check service (http://updates.ghost.org) or not. + * + * @returns {Promise} + */ function updateCheck() { // CASE: The check will not happen if your NODE_ENV is not in the allowed defined environments. if (_.indexOf(allowedCheckEnvironments, process.env.NODE_ENV) === -1) { @@ -240,9 +299,10 @@ function updateCheck() { return api.settings.read(_.extend({key: 'next_update_check'}, internal)) .then(function then(result) { - var nextUpdateCheck = result.settings[0]; + const nextUpdateCheck = result.settings[0]; // CASE: Next update check should happen now? + // @NOTE: You can skip this check by adding a config value. This is helpful for developing. if (!config.get('updateCheck:forceUpdate') && nextUpdateCheck && nextUpdateCheck.value && nextUpdateCheck.value > moment().unix()) { return Promise.resolve(); } diff --git a/core/server/web/admin/controller.js b/core/server/web/admin/controller.js index 3837d7b8e8..b05f1f4990 100644 --- a/core/server/web/admin/controller.js +++ b/core/server/web/admin/controller.js @@ -4,13 +4,18 @@ const config = require('../../config'); const updateCheck = require('../../update-check'); const common = require('../../lib/common'); -// Route: index -// Path: /ghost/ -// Method: GET +/** + * @description Admin controller to handle /ghost/ requests. + * + * Every request to the admin panel will re-trigger the update check service. + * + * @param req + * @param res + */ module.exports = function adminController(req, res) { debug('index called'); - // run in background, don't block the admin rendering + // CASE: trigger update check unit and let it run in background, don't block the admin rendering updateCheck() .catch((err) => { common.logging.error(err);