Merge branch 'contacted_at' into 'master'

Guarantee visually correct order of runners on the "Admin Area > Runners" page

Closes #49791

See merge request gitlab-org/gitlab-ce!21627

5 files changed, 62 insertions, 2 deletions

diff --git a/app/helpers/runners_helper.rb b/app/helpers/runners_helper.rb
index cb21f922401..0d880c38a7b 100644
--- a/app/helpers/runners_helper.rb
+++ b/app/helpers/runners_helper.rb
@@ -28,4 +28,14 @@ module RunnersHelper
       display_name + id
     end
   end
+
+  # Due to inability of performing sorting of runners by cached "contacted_at" values we have to show uncached values if sorting by "contacted_asc" is requested.
+  # Please refer to the following issue for more details: https://gitlab.com/gitlab-org/gitlab-ce/issues/55920
+  def runner_contacted_at(runner)
+    if params[:sort] == 'contacted_asc'
+      runner.uncached_contacted_at
+    else
+      runner.contacted_at
+    end
+  end
 end
diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 8249199e76f..5aae31de6e2 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -256,6 +256,10 @@ module Ci
       end
     end
 
+    def uncached_contacted_at
+      read_attribute(:contacted_at)
+    end
+
     private
 
     def cleanup_runner_queue
diff --git a/app/views/admin/runners/_runner.html.haml b/app/views/admin/runners/_runner.html.haml
index e4fc2985087..829d2c8949f 100644
--- a/app/views/admin/runners/_runner.html.haml
+++ b/app/views/admin/runners/_runner.html.haml
@@ -56,8 +56,9 @@
   .table-section.section-10
     .table-mobile-header{ role: 'rowheader' }= _('Last contact')
     .table-mobile-content
-      - if runner.contacted_at
-        = time_ago_with_tooltip runner.contacted_at
+      - contacted_at = runner_contacted_at(runner)
+      - if contacted_at
+        = time_ago_with_tooltip contacted_at
       - else
         = _('Never')
 
diff --git a/spec/helpers/runners_helper_spec.rb b/spec/helpers/runners_helper_spec.rb
index a4a483e68a8..bf00841fcb6 100644
--- a/spec/helpers/runners_helper_spec.rb
+++ b/spec/helpers/runners_helper_spec.rb
@@ -15,4 +15,40 @@ describe RunnersHelper do
     runner = FactoryBot.build(:ci_runner, contacted_at: 1.second.ago, active: true)
     expect(runner_status_icon(runner)).to include("Runner is online")
   end
+
+  describe '#runner_contacted_at' do
+    let(:contacted_at_stored) { 1.hour.ago.change(usec: 0) }
+    let(:contacted_at_cached) { 1.second.ago.change(usec: 0) }
+    let(:runner) { create(:ci_runner, contacted_at: contacted_at_stored) }
+
+    before do
+      runner.cache_attributes(contacted_at: contacted_at_cached)
+    end
+
+    context 'without sorting' do
+      it 'returns cached value' do
+        expect(runner_contacted_at(runner)).to eq(contacted_at_cached)
+      end
+    end
+
+    context 'with sorting set to created_date' do
+      before do
+        controller.params[:sort] = 'created_date'
+      end
+
+      it 'returns cached value' do
+        expect(runner_contacted_at(runner)).to eq(contacted_at_cached)
+      end
+    end
+
+    context 'with sorting set to contacted_asc' do
+      before do
+        controller.params[:sort] = 'contacted_asc'
+      end
+
+      it 'returns stored value' do
+        expect(runner_contacted_at(runner)).to eq(contacted_at_stored)
+      end
+    end
+  end
 end
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index ad79f8d4ce0..eb2daed7f32 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -817,4 +817,13 @@ describe Ci::Runner do
       expect(runners).to eq([runner2, runner1])
     end
   end
+
+  describe '#uncached_contacted_at' do
+    let(:contacted_at_stored) { 1.hour.ago.change(usec: 0) }
+    let(:runner) { create(:ci_runner, contacted_at: contacted_at_stored) }
+
+    subject { runner.uncached_contacted_at }
+
+    it { is_expected.to eq(contacted_at_stored) }
+  end
 end