From 3c380628023cc12a8e9aefedf602c39076d900b0 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Wed, 3 Jul 2013 11:06:07 -0400 Subject: [PATCH] Check for updates: edge cases when the message on the dashboard doesn't make sense. --- .../javascripts/admin/models/version_check.js | 15 ++++ .../admin/templates/dashboard.js.handlebars | 59 ++++++++----- app/models/discourse_version_check.rb | 2 +- .../discourse_version_check_serializer.rb | 2 +- config/locales/client.en.yml | 2 + lib/discourse_updates.rb | 45 ++++++++-- lib/jobs/version_check.rb | 3 +- spec/components/discourse_updates_spec.rb | 84 +++++++++++++++++++ .../admin/dashboard_controller_spec.rb | 1 + .../admin/versions_controller_spec.rb | 1 + test/javascripts/models/version_check_test.js | 24 ++++++ 11 files changed, 208 insertions(+), 30 deletions(-) create mode 100644 spec/components/discourse_updates_spec.rb create mode 100644 test/javascripts/models/version_check_test.js diff --git a/app/assets/javascripts/admin/models/version_check.js b/app/assets/javascripts/admin/models/version_check.js index 5dd25cab176..2b05fb87b3c 100644 --- a/app/assets/javascripts/admin/models/version_check.js +++ b/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'), diff --git a/app/assets/javascripts/admin/templates/dashboard.js.handlebars b/app/assets/javascripts/admin/templates/dashboard.js.handlebars index 4454abff599..d58362cc607 100644 --- a/app/assets/javascripts/admin/templates/dashboard.js.handlebars +++ b/app/assets/javascripts/admin/templates/dashboard.js.handlebars @@ -49,29 +49,50 @@ {{i18n admin.dashboard.version}} {{ versionCheck.installed_version }} - {{ versionCheck.latest_version }} - - {{#if versionCheck.upToDate }} - ☻ + + {{#if versionCheck.noCheckPerformed}} +   + + ☹ + + + {{i18n admin.dashboard.no_check_performed}} + + {{else}} + {{#if versionCheck.staleData}} +   + + ☹ + + + {{i18n admin.dashboard.stale_data}} + {{else}} - - {{#if versionCheck.behindByOneVersion}} - ☺ + {{ versionCheck.latest_version }} + + {{#if versionCheck.upToDate }} + ☻ {{else}} - ☹ + + {{#if versionCheck.behindByOneVersion}} + ☺ + {{else}} + ☹ + {{/if}} + {{/if}} - + + + {{#if versionCheck.upToDate }} + {{i18n admin.dashboard.up_to_date}} + {{else}} + {{i18n admin.dashboard.critical_available}} + {{i18n admin.dashboard.updates_available}} + {{i18n admin.dashboard.please_upgrade}} + {{/if}} + {{/if}} - - - {{#if versionCheck.upToDate }} - {{i18n admin.dashboard.up_to_date}} - {{else}} - {{i18n admin.dashboard.critical_available}} - {{i18n admin.dashboard.updates_available}} - {{i18n admin.dashboard.please_upgrade}} - {{/if}} - + {{/if}} {{/unless}} diff --git a/app/models/discourse_version_check.rb b/app/models/discourse_version_check.rb index 803e7cee3d6..657f01ddc1d 100644 --- a/app/models/discourse_version_check.rb +++ b/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 diff --git a/app/serializers/discourse_version_check_serializer.rb b/app/serializers/discourse_version_check_serializer.rb index 91adad35397..73cf1e6965c 100644 --- a/app/serializers/discourse_version_check_serializer.rb +++ b/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 \ No newline at end of file diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index e991aeec8b5..fdc9eb154f1 100644 --- a/config/locales/client.en.yml +++ b/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:" diff --git a/lib/discourse_updates.rb b/lib/discourse_updates.rb index ea902d5078e..200aed138e7 100644 --- a/lib/discourse_updates.rb +++ b/lib/discourse_updates.rb @@ -3,14 +3,30 @@ module DiscourseUpdates class << self def check_version - DiscourseVersionCheck.new( - latest_version: latest_version || Discourse::VERSION::STRING, - 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 || nil - # TODO: more info, like links and release messages - ) + version_info = if updated_at.nil? + DiscourseVersionCheck.new( + installed_version: Discourse::VERSION::STRING, + installed_sha: (Discourse.git_version == 'unknown' ? nil : Discourse.git_version), + updated_at: nil + ) + 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 \ No newline at end of file diff --git a/lib/jobs/version_check.rb b/lib/jobs/version_check.rb index b952f588f1b..61632f61f1f 100644 --- a/lib/jobs/version_check.rb +++ b/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 diff --git a/spec/components/discourse_updates_spec.rb b/spec/components/discourse_updates_spec.rb new file mode 100644 index 00000000000..203a895427f --- /dev/null +++ b/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 diff --git a/spec/controllers/admin/dashboard_controller_spec.rb b/spec/controllers/admin/dashboard_controller_spec.rb index f46054d4d40..6cf637c3d4d 100644 --- a/spec/controllers/admin/dashboard_controller_spec.rb +++ b/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 diff --git a/spec/controllers/admin/versions_controller_spec.rb b/spec/controllers/admin/versions_controller_spec.rb index a8eea59741a..13e0cad7b06 100644 --- a/spec/controllers/admin/versions_controller_spec.rb +++ b/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 diff --git a/test/javascripts/models/version_check_test.js b/test/javascripts/models/version_check_test.js new file mode 100644 index 00000000000..4b6d2b57eaa --- /dev/null +++ b/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'); +});