🐛 Fixed version update indicator on about page

refs https://github.com/TryGhost/Team/issues/754
closes https://github.com/TryGhost/Ghost/issues/13088
refs a7dec233ba

- The corrupted data recovery mechanism for notifications is needed to be able to fix the data stored in `settings` table under `notifications` key. There was no validation in place, which has caused some instances to store data in unreadable/writable state
- The recovery mechanism is in place to avoid adding migrations every time we spot a broken notifications data (will be fixed by validation soon).
- The notification data is also NOT critical but valuable for system functioning properly, that's the reason why the data "healing" happens in less secure  "fire-and-forget" way
- The referenced commit is where the "bigger" problem that was causing the data corruption was at. This change is a "cleanup" after what has happened there - storing Ghost error object in `value` for `notifications` key

core/server/services/notifications/index.js

@@ -2,5 +2,11 @@ const settingsCache = require('../settings/cache');
 const i18n = require('../../../shared/i18n');
 const ghostVersion = require('@tryghost/version');
 const Notifications = require('./notifications');
+const models = require('../../models');
 
-module.exports.notifications = new Notifications({settingsCache, i18n, ghostVersion});
+module.exports.notifications = new Notifications({
+    settingsCache,
+    i18n,
+    ghostVersion,
+    SettingsModel: models.Settings
+});

core/server/services/notifications/notifications.js

@@ -13,16 +13,32 @@ class Notifications {
      * @param {Object} options.i18n - i18n instance
      * @param {Object} options.ghostVersion
      * @param {String} options.ghostVersion.full - Ghost instance version in "full" format - major.minor.patch
+     * @param {Object} options.SettingsModel - Ghost's Setting model instance
      */
-    constructor({settingsCache, i18n, ghostVersion}) {
+    constructor({settingsCache, i18n, ghostVersion, SettingsModel}) {
         this.settingsCache = settingsCache;
         this.i18n = i18n;
         this.ghostVersion = ghostVersion;
+        this.SettingsModel = SettingsModel;
     }
 
+    /**
+     * @returns {Object[]} - all notifications
+     */
     fetchAllNotifications() {
         let allNotifications = this.settingsCache.get('notifications');
 
+        // @TODO: this check can be removed to improve read operation perf. It's here only because
+        //        reads are done often and this gives a possibility to self-heal any broken records.
+        //        The check can be removed/moved to write operations once we have the guardrails on that
+        //        level long enough and are confident there's no broken data in the DB (e.g few minors after Ghost v5?)
+        if (!this.areNotificationsValid(allNotifications)) {
+            // Not using "await" here and doing the "fire-and-forget" because the result is know beforehand
+            // We only care for the notifications to ge into "correct" state eventually and work properly with next request
+            this.dangerousDestroyAll();
+            return [];
+        }
+
         allNotifications.forEach((notification) => {
             notification.addedAt = moment(notification.addedAt).toDate();
         });
@@ -30,6 +46,20 @@ class Notifications {
         return allNotifications;
     }
 
+    /**
+     *
+     * @param {Object[]} notifications - objects to check if they have valid notifications array format
+     *
+     * @returns {boolean}
+     */
+    areNotificationsValid(notifications) {
+        if (!(_.isArray(notifications))) {
+            return false;
+        }
+
+        return true;
+    }
+
     wasSeen(notification, user) {
         if (notification.seenBy === undefined) {
             return notification.seen;
@@ -183,6 +213,22 @@ class Notifications {
 
         return allNotifications;
     }
+
+    /**
+     * Comparing to destroyAll method this one wipes out the notifications data!
+     * It is only to be used in a situation when notifications data has been corrupted and
+     * there's a need to self-heal. Wiping out notifications will fetch some of the notifications
+     * again and repopulate the array with correct data.
+     */
+    async dangerousDestroyAll() {
+        // Same default as defined in "default-settings.json"
+        const defaultValue = '[]';
+
+        return this.SettingsModel.edit([{
+            key: 'notifications',
+            value: defaultValue
+        }], {context: {internal: true}});
+    }
 }
 
 module.exports = Notifications;

test/unit/services/notifications/notifications_spec.js

@@ -153,4 +153,62 @@ describe('Notifications Service', function () {
         should.exist(notifications);
         notifications.length.should.equal(0);
     });
+
+    describe('Stored notifications data corruption recovery', function () {
+        it('should correct broken notifications data on browse', function() {
+            const settingsCache = {
+                get: sinon.fake.returns({
+                    message: 'this object should be an array!'
+                })
+            };
+            const settingsModelStub = sinon.stub().resolves();
+
+            const notificationSvc = new Notifications({
+                settingsCache,
+                ghostVersion: {
+                    full: '5.0.0'
+                },
+                SettingsModel: {
+                    edit: settingsModelStub
+                }
+            });
+
+            const notifications = notificationSvc.browse({user: owner});
+
+            should.exist(notifications);
+            notifications.length.should.equal(0);
+
+            settingsModelStub.called.should.equal(true);
+            settingsModelStub.args[0][0].should.eql([{
+                key: 'notifications',
+                value: '[]'
+            }]);
+        });
+
+        it('does not trigger correction when the data is in valid format', function() {
+            const settingsCache = {
+                get: sinon.fake.returns([{
+                    message: 'this works! 5.1.0'
+                }])
+            };
+            const settingsModelStub = sinon.stub().resolves();
+
+            const notificationSvc = new Notifications({
+                settingsCache,
+                ghostVersion: {
+                    full: '5.0.0'
+                },
+                SettingsModel: {
+                    edit: settingsModelStub
+                }
+            });
+
+            const notifications = notificationSvc.browse({user: owner});
+
+            should.exist(notifications);
+            notifications.length.should.equal(1);
+
+            settingsModelStub.called.should.equal(false);
+        });
+    });
 });