Check for updates: edge cases when the message on the dashboard doesn't make sense.

app/assets/javascripts/admin/models/version_check.js

@@ -7,6 +7,21 @@
   @module Discourse
 **/
 Discourse.VersionCheck = Discourse.Model.extend({
+
+  noCheckPerformed: function() {
+    return this.get('updated_at') === null;
+  }.property('updated_at'),
+
+  dataIsOld: function() {
+    return moment().diff(moment(this.get('updated_at')), 'hours') >= 48;
+  }.property('updated_at'),
+
+  staleData: function() {
+    return ( this.get('dataIsOld') ||
+             (this.get('installed_version') !== this.get('latest_version') && this.get('missing_versions_count') === 0) ||
+             (this.get('installed_version') === this.get('latest_version') && this.get('missing_versions_count') !== 0) );
+  }.property('dataIsOld', 'missing_versions_count', 'installed_version', 'latest_version'),
+
   upToDate: function() {
     return this.get('missing_versions_count') === 0 || this.get('missing_versions_count') === null;
   }.property('missing_versions_count'),

app/assets/javascripts/admin/templates/dashboard.js.handlebars

@@ -49,29 +49,50 @@
           <tbody>
             <td class="title">{{i18n admin.dashboard.version}}</td>
             <td class="version-number"><a {{bindAttr href="versionCheck.gitLink"}} target="_blank">{{ versionCheck.installed_version }}</a></td>
-            <td class="version-number">{{ versionCheck.latest_version }}</td>
+
-            <td class="face">
+            {{#if versionCheck.noCheckPerformed}}
-              {{#if versionCheck.upToDate }}
+              <td class="version-number">&nbsp;</td>
-                <span class='icon update-to-date'>☻</span>
+              <td class="face">
+                <span class="icon critical-updates-available">☹</span>
+              </td>
+              <td class="version-notes">
+                <span class="normal-note">{{i18n admin.dashboard.no_check_performed}}</span>
+              </td>
+            {{else}}
+              {{#if versionCheck.staleData}}
+                <td class="version-number">&nbsp;</td>
+                <td class="face">
+                  <span class="icon critical-updates-available">☹</span>
+                </td>
+                <td class="version-notes">
+                  <span class="normal-note">{{i18n admin.dashboard.stale_data}}</span>
+                </td>
               {{else}}
-                <span {{bindAttr class=":icon versionCheck.critical_updates:critical-updates-available:updates-available"}}>
+                <td class="version-number">{{ versionCheck.latest_version }}</td>
-                  {{#if versionCheck.behindByOneVersion}}
+                <td class="face">
-                    ☺
+                  {{#if versionCheck.upToDate }}
+                    <span class='icon update-to-date'>☻</span>
                   {{else}}
-                    ☹
+                    <span {{bindAttr class=":icon versionCheck.critical_updates:critical-updates-available:updates-available"}}>
+                      {{#if versionCheck.behindByOneVersion}}
+                        ☺
+                      {{else}}
+                        ☹
+                      {{/if}}
+                    </span>
                   {{/if}}
-                </span>
+                </td>
+                <td class="version-notes">
+                  {{#if versionCheck.upToDate }}
+                    {{i18n admin.dashboard.up_to_date}}
+                  {{else}}
+                    <span class="critical-note">{{i18n admin.dashboard.critical_available}}</span>
+                    <span class="normal-note">{{i18n admin.dashboard.updates_available}}</span>
+                    {{i18n admin.dashboard.please_upgrade}}
+                  {{/if}}
+                </td>
               {{/if}}
-            </td>
+            {{/if}}
-            <td class="version-notes">
-              {{#if versionCheck.upToDate }}
-                {{i18n admin.dashboard.up_to_date}}
-              {{else}}
-                <span class="critical-note">{{i18n admin.dashboard.critical_available}}</span>
-                <span class="normal-note">{{i18n admin.dashboard.updates_available}}</span>
-                {{i18n admin.dashboard.please_upgrade}}
-              {{/if}}
-            </td>
           </tbody>
         {{/unless}}
       </table>

app/models/discourse_version_check.rb

@@ -5,7 +5,7 @@ class DiscourseVersionCheck
   include ActiveAttr::MassAssignment
   include ActiveModel::Serialization
 
-  attr_accessor :latest_version, :critical_updates, :installed_version, :installed_sha, :missing_versions_count
+  attr_accessor :latest_version, :critical_updates, :installed_version, :installed_sha, :missing_versions_count, :updated_at
 
   def active_model_serializer
     DiscourseVersionCheckSerializer

app/serializers/discourse_version_check_serializer.rb

@@ -1,5 +1,5 @@
 class DiscourseVersionCheckSerializer < ApplicationSerializer
-  attributes :latest_version, :critical_updates, :installed_version, :installed_sha, :missing_versions_count
+  attributes :latest_version, :critical_updates, :installed_version, :installed_sha, :missing_versions_count, :updated_at
 
   self.root = false
 end

config/locales/client.en.yml

@@ -1000,6 +1000,8 @@ en:
         critical_available: "A critical update is available."
         updates_available: "Updates are available."
         please_upgrade: "Please upgrade!"
+        no_check_performed: "A check for updates have not been performed. Ensure clockwork and sidekiq are running."
+        stale_data: "A check for updates has not been performed lately. Ensure clockwork and sidekiq are running."
         installed_version: "Installed"
         latest_version: "Latest"
         problems_found: "Some problems have been found with your installation of Discourse:"

lib/discourse_updates.rb

@@ -3,14 +3,30 @@ module DiscourseUpdates
   class << self
 
     def check_version
-      DiscourseVersionCheck.new(
+      version_info = if updated_at.nil?
-        latest_version: latest_version || Discourse::VERSION::STRING,
+        DiscourseVersionCheck.new(
-        critical_updates: critical_updates_available?,
+          installed_version: Discourse::VERSION::STRING,
-        installed_version: Discourse::VERSION::STRING,
+          installed_sha: (Discourse.git_version == 'unknown' ? nil : Discourse.git_version),
-        installed_sha: (Discourse.git_version == 'unknown' ? nil : Discourse.git_version),
+          updated_at: nil
-        missing_versions_count: missing_versions_count || nil
+        )
-        # TODO: more info, like links and release messages
+      else
-      )
+        DiscourseVersionCheck.new(
+          latest_version: latest_version,
+          critical_updates: critical_updates_available?,
+          installed_version: Discourse::VERSION::STRING,
+          installed_sha: (Discourse.git_version == 'unknown' ? nil : Discourse.git_version),
+          missing_versions_count: missing_versions_count,
+          updated_at: updated_at
+        )
+      end
+
+      if version_info.updated_at.nil? or
+          (version_info.missing_versions_count == 0 and version_info.latest_version != version_info.installed_version)
+        # Version check data is out of date.
+        Jobs.enqueue(:version_check, all_sites: true)
+      end
+
+      version_info
     end
 
     def latest_version
@@ -25,6 +41,15 @@ module DiscourseUpdates
       ($redis.get(critical_updates_available_key) || false) == 'true'
     end
 
+    def updated_at
+      t = $redis.get(updated_at_key)
+      t ? Time.zone.parse(t) : nil
+    end
+
+    def updated_at=(time_with_zone)
+      $redis.set updated_at_key, time_with_zone.as_json
+    end
+
     ['latest_version', 'missing_versions_count', 'critical_updates_available'].each do |name|
       eval "define_method :#{name}= do |arg|
         $redis.set #{name}_key, arg
@@ -45,5 +70,9 @@ module DiscourseUpdates
       def missing_versions_count_key
         'missing_versions_count'
       end
+
+      def updated_at_key
+        'last_version_check_at'
+      end
   end
 end

lib/jobs/version_check.rb

@@ -5,12 +5,13 @@ module Jobs
   class VersionCheck < Jobs::Base
 
     def execute(args)
-      if SiteSetting.version_checks
+      if SiteSetting.version_checks and (DiscourseUpdates.updated_at.nil? or DiscourseUpdates.updated_at < 1.minute.ago)
         begin
           json = DiscourseHub.discourse_version_check
           DiscourseUpdates.latest_version = json['latestVersion']
           DiscourseUpdates.critical_updates_available = json['criticalUpdates']
           DiscourseUpdates.missing_versions_count = json['missingVersionsCount']
+          DiscourseUpdates.updated_at = Time.zone.now
         rescue => e
           raise e unless Rails.env == 'development' # Fail version check silently in development mode
         end

spec/components/discourse_updates_spec.rb

@@ -0,0 +1,84 @@
+require 'spec_helper'
+require_dependency 'discourse_updates'
+
+describe DiscourseUpdates do
+
+  def stub_data(latest, missing, critical, updated_at)
+    DiscourseUpdates.stubs(:latest_version).returns(latest)
+    DiscourseUpdates.stubs(:missing_versions_count).returns(missing)
+    DiscourseUpdates.stubs(:critical_updates_available?).returns(critical)
+    DiscourseUpdates.stubs(:updated_at).returns(updated_at)
+  end
+
+  before do
+    Jobs::VersionCheck.any_instance.stubs(:execute).returns(true)
+  end
+
+  subject { DiscourseUpdates.check_version.as_json }
+
+  context 'a good version check request happened recently' do
+    context 'and server is up-to-date' do
+      before { stub_data(Discourse::VERSION::STRING, 0, false, 12.hours.ago) }
+
+      it 'returns all the version fields' do
+        subject['latest_version'].should == Discourse::VERSION::STRING
+        subject['missing_versions_count'].should == 0
+        subject['critical_updates'].should == false
+        subject['installed_version'].should == Discourse::VERSION::STRING
+      end
+
+      it 'returns the timestamp of the last version check' do
+        subject['updated_at'].should be_within_one_second_of(12.hours.ago)
+      end
+    end
+
+    context 'and server is not up-to-date' do
+      before { stub_data('0.9.0', 2, false, 12.hours.ago) }
+
+      it 'returns all the version fields' do
+        subject['latest_version'].should == '0.9.0'
+        subject['missing_versions_count'].should == 2
+        subject['critical_updates'].should == false
+        subject['installed_version'].should == Discourse::VERSION::STRING
+      end
+
+      it 'returns the timestamp of the last version check' do
+        subject['updated_at'].should be_within_one_second_of(12.hours.ago)
+      end
+    end
+  end
+
+  context 'a version check has never been performed' do
+    before { stub_data(nil, nil, false, nil) }
+
+    it 'returns the installed version' do
+      subject['installed_version'].should == Discourse::VERSION::STRING
+    end
+
+    it 'indicates that version check has not been performed' do
+      subject.should have_key('updated_at')
+      subject['updated_at'].should == nil
+    end
+
+    it 'does not return latest version info' do
+      subject.should_not have_key('latest_version')
+      subject.should_not have_key('missing_versions_count')
+      subject.should_not have_key('critical_updates')
+    end
+
+    it 'queues a version check' do
+      Jobs.expects(:enqueue).with(:version_check, anything)
+      subject
+    end
+  end
+
+  context 'installed version is newer' do
+    before { stub_data('0.9.3', 0, false, 28.hours.ago) }
+
+    it 'queues a version check' do
+      Jobs.expects(:enqueue).with(:version_check, anything)
+      subject
+    end
+  end
+
+end

spec/controllers/admin/dashboard_controller_spec.rb

@@ -6,6 +6,7 @@ describe Admin::DashboardController do
     #NOTE: Rails.cache should be blanked between tests, at the moment we can share state with it
     # that is seriously bust on quite a few levels
     Rails.cache.delete("admin-dashboard-data-#{Discourse::VERSION::STRING}")
+    Jobs::VersionCheck.any_instance.stubs(:execute).returns(true)
   end
 
   it "is a subclass of AdminController" do

spec/controllers/admin/versions_controller_spec.rb

@@ -4,6 +4,7 @@ require_dependency 'version'
 describe Admin::VersionsController do
 
   before do
+    DiscourseUpdates.stubs(:updated_at).returns(2.hours.ago)
     DiscourseUpdates.stubs(:latest_version).returns('1.2.33')
     DiscourseUpdates.stubs(:critical_updates_available?).returns(false)
   end

test/javascripts/models/version_check_test.js

@@ -0,0 +1,24 @@
+module("Discourse.VersionCheck");
+
+test('dataIsOld', function() {
+  var dataIsOld = function(args, expected, message) {
+    equal(Discourse.VersionCheck.create(args).get('dataIsOld'), expected, message);
+  };
+
+  dataIsOld({updated_at: moment().subtract('hours', 2).toJSON()},  false, '2 hours ago');
+  dataIsOld({updated_at: moment().subtract('hours', 49).toJSON()}, true,  '49 hours ago');
+});
+
+test('staleData', function() {
+  var updatedAt = function(hoursAgo) {
+    return moment().subtract('hours', hoursAgo).toJSON();
+  };
+  var staleData = function(args, expected, message) {
+    equal(Discourse.VersionCheck.create(args).get('staleData'), expected, message);
+  };
+
+  staleData({missing_versions_count: 0, installed_version: '0.9.3', latest_version: '0.9.3', updated_at: updatedAt(2)}, false, 'up to date');
+  staleData({missing_versions_count: 0, installed_version: '0.9.4', latest_version: '0.9.3', updated_at: updatedAt(2)}, true,  'installed and latest do not match, but missing_versions_count is 0');
+  staleData({missing_versions_count: 1, installed_version: '0.9.3', latest_version: '0.9.3', updated_at: updatedAt(2)}, true,  'installed and latest match, but missing_versions_count is not 0');
+  staleData({missing_versions_count: 0, installed_version: '0.9.3', latest_version: '0.9.3', updated_at: updatedAt(50)}, true, 'old version check data');
+});