From f7642e076d8dd1e371aaeb9b9e22ca60a0a2dfec Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 21 Nov 2017 17:01:27 +0800 Subject: [PATCH] REFACTOR: Avoid duplicated logic on server and client. --- .../admin/models/version-check.js.es6 | 12 ----- .../admin/templates/version-checks.hbs | 2 +- app/models/discourse_version_check.rb | 11 ++++- .../discourse_version_check_serializer.rb | 7 ++- lib/discourse_updates.rb | 45 ++++++++++--------- spec/components/discourse_updates_spec.rb | 43 +++++++++--------- 6 files changed, 64 insertions(+), 56 deletions(-) diff --git a/app/assets/javascripts/admin/models/version-check.js.es6 b/app/assets/javascripts/admin/models/version-check.js.es6 index 75f0d9e04fd..ce1f420b49b 100644 --- a/app/assets/javascripts/admin/models/version-check.js.es6 +++ b/app/assets/javascripts/admin/models/version-check.js.es6 @@ -8,18 +8,6 @@ const VersionCheck = Discourse.Model.extend({ return updatedAt === null; }, - @computed('updated_at', 'version_check_pending') - dataIsOld(updatedAt, versionCheckPending) { - return versionCheckPending || moment().diff(moment(updatedAt), 'hours') >= 48; - }, - - @computed('dataIsOld', 'installed_version', 'latest_version', 'missing_versions_count') - staleData(dataIsOld, installedVersion, latestVersion, missingVersionsCount) { - return dataIsOld || - (installedVersion !== latestVersion && missingVersionsCount === 0) || - (installedVersion === latestVersion && missingVersionsCount !== 0); - }, - @computed('missing_versions_count') upToDate(missingVersionsCount) { return missingVersionsCount === 0 || missingVersionsCount === null; diff --git a/app/assets/javascripts/admin/templates/version-checks.hbs b/app/assets/javascripts/admin/templates/version-checks.hbs index 4f392650267..a8724b7e5a5 100644 --- a/app/assets/javascripts/admin/templates/version-checks.hbs +++ b/app/assets/javascripts/admin/templates/version-checks.hbs @@ -24,7 +24,7 @@ {{i18n 'admin.dashboard.no_check_performed'}} {{else}} - {{#if versionCheck.staleData}} + {{#if versionCheck.stale_data}} {{#if versionCheck.version_check_pending}}{{dash-if-empty versionCheck.installed_version}}{{/if}} {{#if versionCheck.version_check_pending}} diff --git a/app/models/discourse_version_check.rb b/app/models/discourse_version_check.rb index 91aabe84c6a..084c8dc6da6 100644 --- a/app/models/discourse_version_check.rb +++ b/app/models/discourse_version_check.rb @@ -1,5 +1,14 @@ class DiscourseVersionCheck include ActiveModel::Model - attr_accessor :latest_version, :critical_updates, :installed_version, :installed_sha, :installed_describe, :missing_versions_count, :git_branch, :updated_at, :version_check_pending + attr_accessor :latest_version, + :critical_updates, + :installed_version, + :installed_sha, + :installed_describe, + :missing_versions_count, + :git_branch, + :updated_at, + :version_check_pending, + :stale_data end diff --git a/app/serializers/discourse_version_check_serializer.rb b/app/serializers/discourse_version_check_serializer.rb index ef53a646154..e6f38e8698d 100644 --- a/app/serializers/discourse_version_check_serializer.rb +++ b/app/serializers/discourse_version_check_serializer.rb @@ -1,5 +1,10 @@ class DiscourseVersionCheckSerializer < ApplicationSerializer - attributes :latest_version, :critical_updates, :installed_version, :installed_sha, :missing_versions_count, :updated_at + attributes :latest_version, + :critical_updates, + :installed_version, + :installed_sha, + :missing_versions_count, + :updated_at self.root = false end diff --git a/lib/discourse_updates.rb b/lib/discourse_updates.rb index 8f9684dca1d..782a03ab0b1 100644 --- a/lib/discourse_updates.rb +++ b/lib/discourse_updates.rb @@ -3,47 +3,52 @@ module DiscourseUpdates class << self def check_version - version_info = if updated_at.nil? - DiscourseVersionCheck.new( - installed_version: Discourse::VERSION::STRING, - installed_sha: (Discourse.git_version == 'unknown' ? nil : Discourse.git_version), - installed_describe: Discourse.full_version, - git_branch: Discourse.git_branch, - updated_at: nil - ) - else - DiscourseVersionCheck.new( + attrs = { + installed_version: Discourse::VERSION::STRING, + installed_sha: (Discourse.git_version == 'unknown' ? nil : Discourse.git_version), + installed_describe: Discourse.full_version, + git_branch: Discourse.git_branch, + updated_at: updated_at, + } + + unless updated_at.nil? + attrs.merge!( latest_version: latest_version, critical_updates: critical_updates_available?, - installed_version: Discourse::VERSION::STRING, - installed_sha: (Discourse.git_version == 'unknown' ? nil : Discourse.git_version), - installed_describe: Discourse.full_version, - missing_versions_count: missing_versions_count, - git_branch: Discourse.git_branch, - updated_at: updated_at + missing_versions_count: missing_versions_count ) end + version_info = DiscourseVersionCheck.new(attrs) + # replace -commit_count with +commit_count if version_info.installed_describe =~ /-(\d+)-/ version_info.installed_describe = version_info.installed_describe.gsub(/-(\d+)-.*/, " +#{$1}") end if SiteSetting.version_checks? + is_stale_data = + (version_info.missing_versions_count == 0 && version_info.latest_version != version_info.installed_version) || + (version_info.missing_versions_count != 0 && version_info.latest_version == version_info.installed_version) # Handle cases when version check data is old so we report something that makes sense + if version_info.updated_at.nil? || # never performed a version check + last_installed_version != Discourse::VERSION::STRING || # upgraded since the last version check + is_stale_data - if (version_info.updated_at.nil? || # never performed a version check - last_installed_version != (Discourse::VERSION::STRING) || # upgraded since the last version check - (version_info.missing_versions_count == (0) && version_info.latest_version != (version_info.installed_version)) || # old data - (version_info.missing_versions_count != (0) && version_info.latest_version == (version_info.installed_version))) # old data Jobs.enqueue(:version_check, all_sites: true) 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 + + version_info.stale_data = + version_info.version_check_pending || + (updated_at && updated_at < 48.hours.ago) || + is_stale_data end version_info diff --git a/spec/components/discourse_updates_spec.rb b/spec/components/discourse_updates_spec.rb index 7f3f1234af1..aa4a335c20c 100644 --- a/spec/components/discourse_updates_spec.rb +++ b/spec/components/discourse_updates_spec.rb @@ -14,7 +14,7 @@ describe DiscourseUpdates do Jobs::VersionCheck.any_instance.stubs(:execute).returns(true) end - subject { DiscourseUpdates.check_version.instance_values } + subject { DiscourseUpdates.check_version } context 'version check was done at the current installed version' do before do @@ -26,14 +26,15 @@ describe DiscourseUpdates do before { stub_data(Discourse::VERSION::STRING, 0, false, 12.hours.ago) } it 'returns all the version fields' do - expect(subject['latest_version']).to eq(Discourse::VERSION::STRING) - expect(subject['missing_versions_count']).to eq(0) - expect(subject['critical_updates']).to eq(false) - expect(subject['installed_version']).to eq(Discourse::VERSION::STRING) + expect(subject.latest_version).to eq(Discourse::VERSION::STRING) + expect(subject.missing_versions_count).to eq(0) + expect(subject.critical_updates).to eq(false) + expect(subject.installed_version).to eq(Discourse::VERSION::STRING) + expect(subject.stale_data).to eq(false) end it 'returns the timestamp of the last version check' do - expect(subject['updated_at']).to be_within_one_second_of(12.hours.ago) + expect(subject.updated_at).to be_within_one_second_of(12.hours.ago) end end @@ -41,14 +42,14 @@ describe DiscourseUpdates do before { stub_data('0.9.0', 2, false, 12.hours.ago) } it 'returns all the version fields' do - expect(subject['latest_version']).to eq('0.9.0') - expect(subject['missing_versions_count']).to eq(2) - expect(subject['critical_updates']).to eq(false) - expect(subject['installed_version']).to eq(Discourse::VERSION::STRING) + expect(subject.latest_version).to eq('0.9.0') + expect(subject.missing_versions_count).to eq(2) + expect(subject.critical_updates).to eq(false) + expect(subject.installed_version).to eq(Discourse::VERSION::STRING) end it 'returns the timestamp of the last version check' do - expect(subject['updated_at']).to be_within_one_second_of(12.hours.ago) + expect(subject.updated_at).to be_within_one_second_of(12.hours.ago) end end end @@ -57,18 +58,18 @@ describe DiscourseUpdates do before { stub_data(nil, nil, false, nil) } it 'returns the installed version' do - expect(subject['installed_version']).to eq(Discourse::VERSION::STRING) + expect(subject.installed_version).to eq(Discourse::VERSION::STRING) end it 'indicates that version check has not been performed' do - expect(subject).to have_key('updated_at') - expect(subject['updated_at']).to eq(nil) + expect(subject.updated_at).to eq(nil) + expect(subject.stale_data).to eq(true) end it 'does not return latest version info' do - expect(subject).not_to have_key('latest_version') - expect(subject).not_to have_key('missing_versions_count') - expect(subject).not_to have_key('critical_updates') + expect(subject.latest_version).to eq(nil) + expect(subject.missing_versions_count).to eq(nil) + expect(subject.critical_updates).to eq(nil) end it 'queues a version check' do @@ -87,11 +88,11 @@ describe DiscourseUpdates do end it 'reports 0 missing versions' do - expect(subject['missing_versions_count']).to eq(0) + expect(subject.missing_versions_count).to eq(0) end it 'reports that a version check will be run soon' do - expect(subject['version_check_pending']).to eq(true) + expect(subject.version_check_pending).to eq(true) end end @@ -119,11 +120,11 @@ describe DiscourseUpdates do end it 'reports 0 missing versions' do - expect(subject['missing_versions_count']).to eq(0) + expect(subject.missing_versions_count).to eq(0) end it 'reports that a version check will be run soon' do - expect(subject['version_check_pending']).to eq(true) + expect(subject.version_check_pending).to eq(true) end end