From ede59a4386e24ad1bb7ecbbec176f60ef5e0cf75 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Mon, 4 Nov 2013 12:51:01 -0500 Subject: [PATCH] FIX: issue 1538. After upgrading and before a new version check request has been made, dashboard might still say that an update is available. --- .../javascripts/admin/models/version_check.js | 2 +- .../admin/templates/dashboard.js.handlebars | 16 +- .../stylesheets/common/admin/admin_base.scss | 2 +- app/jobs/scheduled/version_check.rb | 1 + app/models/discourse_version_check.rb | 2 +- config/locales/client.en.yml | 1 + lib/discourse_updates.rb | 27 ++- spec/components/discourse_updates_spec.rb | 154 +++++++++++------- .../admin/versions_controller_spec.rb | 1 + test/javascripts/models/version_check_test.js | 2 + 10 files changed, 133 insertions(+), 75 deletions(-) diff --git a/app/assets/javascripts/admin/models/version_check.js b/app/assets/javascripts/admin/models/version_check.js index 2b05fb87b3c..2add3b7b2f0 100644 --- a/app/assets/javascripts/admin/models/version_check.js +++ b/app/assets/javascripts/admin/models/version_check.js @@ -13,7 +13,7 @@ Discourse.VersionCheck = Discourse.Model.extend({ }.property('updated_at'), dataIsOld: function() { - return moment().diff(moment(this.get('updated_at')), 'hours') >= 48; + return this.get('version_check_pending') || moment().diff(moment(this.get('updated_at')), 'hours') >= 48; }.property('updated_at'), staleData: function() { diff --git a/app/assets/javascripts/admin/templates/dashboard.js.handlebars b/app/assets/javascripts/admin/templates/dashboard.js.handlebars index 621d355ee61..fdc9fdd5f16 100644 --- a/app/assets/javascripts/admin/templates/dashboard.js.handlebars +++ b/app/assets/javascripts/admin/templates/dashboard.js.handlebars @@ -62,16 +62,26 @@ {{#if versionCheck.staleData}}   - ☹ + {{#if versionCheck.version_check_pending}} + ☻ + {{else}} + ☹ + {{/if}} - {{i18n admin.dashboard.stale_data}} + + {{#if versionCheck.version_check_pending}} + {{i18n admin.dashboard.version_check_pending}} + {{else}} + {{i18n admin.dashboard.stale_data}} + {{/if}} + {{else}} {{ versionCheck.latest_version }} {{#if versionCheck.upToDate }} - ☻ + ☻ {{else}} {{#if versionCheck.behindByOneVersion}} diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss index 22696753c0d..12fed2b0231 100644 --- a/app/assets/stylesheets/common/admin/admin_base.scss +++ b/app/assets/stylesheets/common/admin/admin_base.scss @@ -382,7 +382,7 @@ table { font-size: 26px; } - .update-to-date { + .up-to-date { color: green; } .updates-available { diff --git a/app/jobs/scheduled/version_check.rb b/app/jobs/scheduled/version_check.rb index 91528bf7c54..a50e7c2fef3 100644 --- a/app/jobs/scheduled/version_check.rb +++ b/app/jobs/scheduled/version_check.rb @@ -11,6 +11,7 @@ module Jobs should_send_email = (SiteSetting.new_version_emails and DiscourseUpdates.missing_versions_count and DiscourseUpdates.missing_versions_count == 0) json = DiscourseHub.discourse_version_check + DiscourseUpdates.last_installed_version = Discourse::VERSION::STRING DiscourseUpdates.latest_version = json['latestVersion'] DiscourseUpdates.critical_updates_available = json['criticalUpdates'] DiscourseUpdates.missing_versions_count = json['missingVersionsCount'] diff --git a/app/models/discourse_version_check.rb b/app/models/discourse_version_check.rb index 00e21d04f02..01a1a9d0524 100644 --- a/app/models/discourse_version_check.rb +++ b/app/models/discourse_version_check.rb @@ -7,7 +7,7 @@ class DiscourseVersionCheck include ActiveModel::Serialization end - attr_accessor :latest_version, :critical_updates, :installed_version, :installed_sha, :missing_versions_count, :updated_at + attr_accessor :latest_version, :critical_updates, :installed_version, :installed_sha, :missing_versions_count, :updated_at, :version_check_pending unless rails4? def active_model_serializer diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index f51176145b0..619cf683100 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1090,6 +1090,7 @@ en: please_upgrade: "Please upgrade!" no_check_performed: "A check for updates has not been performed. Ensure sidekiq is running." stale_data: "A check for updates has not been performed lately. Ensure sidekiq is running." + version_check_pending: "Looks like you upgraded recently. Fantastic!" 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 918c6d1c84b..250e2f8975d 100644 --- a/lib/discourse_updates.rb +++ b/lib/discourse_updates.rb @@ -24,20 +24,27 @@ module DiscourseUpdates # Handle cases when version check data is old so we report something that makes sense - if (version_info.updated_at.nil? or - (version_info.missing_versions_count == 0 and version_info.latest_version != version_info.installed_version) or - (version_info.missing_versions_count != 0 and version_info.latest_version == version_info.installed_version)) + if (version_info.updated_at.nil? or # never performed a version check + last_installed_version != Discourse::VERSION::STRING or # upgraded since the last version check + (version_info.missing_versions_count == 0 and version_info.latest_version != version_info.installed_version) or # old data + (version_info.missing_versions_count != 0 and version_info.latest_version == version_info.installed_version)) # old data Jobs.enqueue(:version_check, all_sites: true) - end - - if !version_info.updated_at.nil? and version_info.latest_version == version_info.installed_version - version_info.missing_versions_count = 0 + version_info.version_check_pending = true + unless version_info.updated_at.nil? + version_info.missing_versions_count = 0 + version_info.critical_updates = false + end end end version_info end + # last_installed_version is the installed version at the time of the last version check + def last_installed_version + $redis.get last_installed_version_key + end + def latest_version $redis.get latest_version_key end @@ -59,7 +66,7 @@ module DiscourseUpdates $redis.set updated_at_key, time_with_zone.as_json end - ['latest_version', 'missing_versions_count', 'critical_updates_available'].each do |name| + ['last_installed_version', 'latest_version', 'missing_versions_count', 'critical_updates_available'].each do |name| eval "define_method :#{name}= do |arg| $redis.set #{name}_key, arg end" @@ -68,6 +75,10 @@ module DiscourseUpdates private + def last_installed_version_key + 'last_installed_version' + end + def latest_version_key 'discourse_latest_version' end diff --git a/spec/components/discourse_updates_spec.rb b/spec/components/discourse_updates_spec.rb index 7e4e96ce8de..f3d36e153e4 100644 --- a/spec/components/discourse_updates_spec.rb +++ b/spec/components/discourse_updates_spec.rb @@ -16,75 +16,103 @@ describe DiscourseUpdates do 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) } + context 'version check was done at the current installed version' do + before do + DiscourseUpdates.stubs(:last_installed_version).returns(Discourse::VERSION::STRING) + end - 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 + 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 'returns the timestamp of the last version check' do - subject['updated_at'].should be_within_one_second_of(12.hours.ago) + 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 'and server is not up-to-date' do - before { stub_data('0.9.0', 2, false, 12.hours.ago) } + # These cases should never happen anymore, but keep the specs to be sure + # they're handled in a sane way. + context 'old version check data' do + shared_examples "queue version check and report that version is ok" do + it 'queues a version check' do + Jobs.expects(:enqueue).with(:version_check, anything) + subject + end - 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 + it 'reports 0 missing versions' do + subject['missing_versions_count'].should == 0 + end + + it 'reports that a version check will be run soon' do + subject['version_check_pending'].should == true + end end - it 'returns the timestamp of the last version check' do - subject['updated_at'].should be_within_one_second_of(12.hours.ago) + context 'installed is latest' do + before { stub_data(Discourse::VERSION::STRING, 1, false, 8.hours.ago) } + include_examples "queue version check and report that version is ok" + end + + context 'installed does not match latest version, but missing_versions_count is 0' do + before { stub_data('0.10.10.123', 0, false, 8.hours.ago) } + include_examples "queue version check and report that version is ok" 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 + context 'version check was done at a different installed version' do + before do + DiscourseUpdates.stubs(:last_installed_version).returns('0.9.1') 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 - - context 'old version check data' do - context 'installed is latest' do - before { stub_data(Discourse::VERSION::STRING, 1, false, 8.hours.ago) } - + shared_examples "when last_installed_version is old" do it 'queues a version check' do Jobs.expects(:enqueue).with(:version_check, anything) subject @@ -93,16 +121,20 @@ describe DiscourseUpdates do it 'reports 0 missing versions' do subject['missing_versions_count'].should == 0 end - end - context 'installed is not latest' do - before { stub_data('0.9.1', 0, false, 8.hours.ago) } - - it 'queues a version check' do - Jobs.expects(:enqueue).with(:version_check, anything) - subject + it 'reports that a version check will be run soon' do + subject['version_check_pending'].should == true end end - end + context 'missing_versions_count is 0' do + before { stub_data('0.9.7', 0, false, 8.hours.ago) } + include_examples "when last_installed_version is old" + end + + context 'missing_versions_count is not 0' do + before { stub_data('0.9.7', 1, false, 8.hours.ago) } + include_examples "when last_installed_version is old" + end + end end diff --git a/spec/controllers/admin/versions_controller_spec.rb b/spec/controllers/admin/versions_controller_spec.rb index 13e0cad7b06..364c5503a0b 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 + Jobs::VersionCheck.any_instance.stubs(:execute).returns(true) DiscourseUpdates.stubs(:updated_at).returns(2.hours.ago) DiscourseUpdates.stubs(:latest_version).returns('1.2.33') DiscourseUpdates.stubs(:critical_updates_available?).returns(false) diff --git a/test/javascripts/models/version_check_test.js b/test/javascripts/models/version_check_test.js index 4b6d2b57eaa..b03102a53af 100644 --- a/test/javascripts/models/version_check_test.js +++ b/test/javascripts/models/version_check_test.js @@ -7,6 +7,7 @@ test('dataIsOld', function() { dataIsOld({updated_at: moment().subtract('hours', 2).toJSON()}, false, '2 hours ago'); dataIsOld({updated_at: moment().subtract('hours', 49).toJSON()}, true, '49 hours ago'); + dataIsOld({updated_at: moment().subtract('hours', 2).toJSON(), version_check_pending: true}, true, 'version check pending'); }); test('staleData', function() { @@ -21,4 +22,5 @@ test('staleData', function() { 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'); + staleData({version_check_pending: true, missing_versions_count: 0, installed_version: '0.9.4', latest_version: '0.9.3', updated_at: updatedAt(2)}, true, 'version was upgraded, but no version check has been done since the upgrade'); });