REFACTOR: Avoid duplicated logic on server and client.

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;

app/assets/javascripts/admin/templates/version-checks.hbs

@@ -24,7 +24,7 @@
           <span class="normal-note">{{i18n 'admin.dashboard.no_check_performed'}}</span>
         </td>
       {{else}}
-        {{#if versionCheck.staleData}}
+        {{#if versionCheck.stale_data}}
           <td class="version-number">{{#if versionCheck.version_check_pending}}{{dash-if-empty versionCheck.installed_version}}{{/if}}</td>
           <td class="face">
             {{#if versionCheck.version_check_pending}}

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

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

lib/discourse_updates.rb

@@ -3,47 +3,52 @@ module DiscourseUpdates
   class << self
 
     def check_version
-      version_info = if updated_at.nil?
+      attrs = {
-        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
+        updated_at: updated_at,
-        )
+      }
-      else
+
-        DiscourseVersionCheck.new(
+      unless updated_at.nil?
+        attrs.merge!(
           latest_version: latest_version,
           critical_updates: critical_updates_available?,
-          installed_version: Discourse::VERSION::STRING,
+          missing_versions_count: missing_versions_count
-          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
         )
       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

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.latest_version).to eq(Discourse::VERSION::STRING)
-          expect(subject['missing_versions_count']).to eq(0)
+          expect(subject.missing_versions_count).to eq(0)
-          expect(subject['critical_updates']).to eq(false)
+          expect(subject.critical_updates).to eq(false)
-          expect(subject['installed_version']).to eq(Discourse::VERSION::STRING)
+          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.latest_version).to eq('0.9.0')
-          expect(subject['missing_versions_count']).to eq(2)
+          expect(subject.missing_versions_count).to eq(2)
-          expect(subject['critical_updates']).to eq(false)
+          expect(subject.critical_updates).to eq(false)
-          expect(subject['installed_version']).to eq(Discourse::VERSION::STRING)
+          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.latest_version).to eq(nil)
-        expect(subject).not_to have_key('missing_versions_count')
+        expect(subject.missing_versions_count).to eq(nil)
-        expect(subject).not_to have_key('critical_updates')
+        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