Fix display update notification

closes #1932

- added showUpdateNotification with version check
- added temp workaround for boolean values in database
- changed default value from false to null
- updated tests

core/server/data/default-settings.json

@@ -10,7 +10,7 @@
             "defaultValue": null
         },
         "displayUpdateNotification": {
-            "defaultValue": false
+            "defaultValue": null
         }
     },
     "blog": {

core/server/helpers/index.js

@@ -12,6 +12,7 @@ var downsize        = require('downsize'),
     filters         = require('../filters'),
     template        = require('./template'),
     schema          = require('../data/schema').checks,
+    updateCheck     = require('../update-check'),
 
     assetTemplate   = _.template('<%= source %>?v=<%= version %>'),
     scriptTemplate  = _.template('<script src="<%= source %>?v=<%= version %>"></script>'),
@@ -569,8 +570,8 @@ coreHelpers.updateNotification = function () {
         return when(output);
     }
 
-    return api.settings.read('displayUpdateNotification').then(function (display) {
-        if (display && display.value && display.value === 'true') {
+    return updateCheck.showUpdateNotification().then(function (result) {
+        if (result) {
             output = '<div class="notification-success">' +
                 'A new version of Ghost is available! Hot damn. ' +
                 '<a href="http://ghost.org/download">Upgrade now</a></div>';

core/server/update-check.js

@@ -31,10 +31,11 @@ var crypto   = require('crypto'),
     api      = require('./api'),
     config   = require('./config'),
     errors   = require('./errorHandling'),
+    packageInfo = require('../../package.json'),
 
     allowedCheckEnvironments = ['development', 'production'],
     checkEndpoint = 'updates.ghost.org',
-    currentVersion;
+    currentVersion = packageInfo.version;
 
 function updateCheckError(error) {
     errors.logError(
@@ -140,13 +141,12 @@ function updateCheckRequest() {
 // 1. Updates the time we can next make a check
 // 2. Checks if the version in the response is new, and updates the notification setting
 function updateCheckResponse(response) {
-    var ops = [],
-        displayUpdateNotification = currentVersion && semver.gt(response.version, currentVersion);
+    var ops = [];
 
     ops.push(api.settings.edit('nextUpdateCheck', response.next_check)
         .otherwise(errors.rejectError));
 
-    ops.push(api.settings.edit('displayUpdateNotification', displayUpdateNotification)
+    ops.push(api.settings.edit('displayUpdateNotification', response.version)
                 .otherwise(errors.rejectError));
 
     return when.settle(ops).then(function (descriptors) {
@@ -159,7 +159,7 @@ function updateCheckResponse(response) {
     });
 }
 
-function updateCheck(res) {
+function updateCheck() {
     var deferred = when.defer();
 
     // The check will not happen if:
@@ -175,8 +175,7 @@ function updateCheck(res) {
                 // It's not time to check yet
                 deferred.resolve();
             } else {
-                // We need to do a check, store the current version
-                currentVersion = res.locals.version;
+                // We need to do a check
                 return updateCheckRequest()
                     .then(updateCheckResponse)
                     .otherwise(updateCheckError);
@@ -188,4 +187,21 @@ function updateCheck(res) {
     return deferred.promise;
 }
 
+function showUpdateNotification() {
+    return api.settings.read('displayUpdateNotification').then(function (display) {
+        // Version 0.4 used boolean to indicate the need for an update. This special case is
+        // translated to the version string.
+        // TODO: remove in future version.
+        if (display.value === 'false' || display.value === 'true') {
+            display.value = '0.4.0';
+        }
+
+        if (display && display.value && currentVersion && semver.gt(display.value, currentVersion)) {
+            return when(true);
+        }
+        return when(false);
+    });
+}
+
 module.exports = updateCheck;
+module.exports.showUpdateNotification = showUpdateNotification;

core/test/unit/server_helpers_index_spec.js

@@ -8,7 +8,7 @@ var testUtils  = require('../utils'),
     rewire      = require('rewire'),
     api         = require('../../server/api'),
     hbs         = require('express-hbs'),
-
+    packageInfo = require('../../../package'),
 
     // Stuff we are testing
     handlebars  = hbs.handlebars,
@@ -939,14 +939,16 @@ describe('Core Helpers', function () {
         });
     });
     describe('updateNotification', function () {
-        it('outputs a correctly formatted notification when display is set to true', function (done) {
+        it('outputs a correctly formatted notification when db version is higher than package version', function (done) {
             var output = '<div class="notification-success">' +
                 'A new version of Ghost is available! Hot damn. ' +
                 '<a href="http://ghost.org/download">Upgrade now</a></div>';
 
             apiStub.restore();
             apiStub = sandbox.stub(api.settings, 'read', function () {
-                return when({value: 'true'});
+                var futureversion = packageInfo.version.split('.');
+                futureversion[futureversion.length-1] = parseInt(futureversion[futureversion.length-1], 10) + 1;
+                return when({value: futureversion.join('.')});
             });
 
             helpers.updateNotification.call({currentUser: {name: 'bob'}}).then(function (rendered) {
@@ -957,7 +959,12 @@ describe('Core Helpers', function () {
             }).then(null, done);
         });
 
-        it('does NOT output a correctly formatted notification when display is not set to true', function (done) {
+        it('does NOT output a correctly formatted notification when db version equals package version', function (done) {
+            apiStub.restore();
+            apiStub = sandbox.stub(api.settings, 'read', function () {
+                return when({value: packageInfo.version});
+            });
+
             helpers.updateNotification.call({currentUser: {name: 'bob'}}).then(function (rendered) {
                 should.exist(rendered);
                 rendered.should.equal('');
@@ -983,7 +990,9 @@ describe('Core Helpers', function () {
         it('does NOT output a notification if the user is not logged in', function (done) {
             apiStub.restore();
             apiStub = sandbox.stub(api.settings, 'read', function () {
-                return when({value: 'true'});
+                var futureversion = packageInfo.version.split('.');
+                futureversion[futureversion.length-1] = parseInt(futureversion[futureversion.length-1], 10) + 1;
+                return when({value: futureversion.join('.')});
             });
 
             helpers.updateNotification.call().then(function (rendered) {