Merge branch 'fix-rubocop-master-failures' into 'master'

Cleaned up CI runner administration code

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

4 files changed, 30 insertions, 4 deletions

diff --git a/app/controllers/admin/runners_controller.rb b/app/controllers/admin/runners_controller.rb
index 2ac14ecd79c..911603bac17 100644
--- a/app/controllers/admin/runners_controller.rb
+++ b/app/controllers/admin/runners_controller.rb
@@ -1,14 +1,12 @@
 class Admin::RunnersController < Admin::ApplicationController
   before_action :runner, except: :index
 
-  # rubocop: disable CodeReuse/ActiveRecord
   def index
     finder = Admin::RunnersFinder.new(params: params)
     @runners = finder.execute
     @active_runners_count = Ci::Runner.online.count
     @sort = finder.sort_key
   end
-  # rubocop: enable CodeReuse/ActiveRecord
 
   def show
     assign_builds_and_projects
diff --git a/app/finders/admin/runners_finder.rb b/app/finders/admin/runners_finder.rb
index 7adee486e33..3c2d7ee7d76 100644
--- a/app/finders/admin/runners_finder.rb
+++ b/app/finders/admin/runners_finder.rb
@@ -43,8 +43,7 @@ class Admin::RunnersFinder < UnionFinder
   end
 
   def sort!
-    sort = sort_key == 'contacted_asc' ? { contacted_at: :asc } : { created_at: :desc }
-    @runners = @runners.order(sort)
+    @runners = @runners.order_by(sort_key)
   end
 
   def paginate!
diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 45fd15a6211..eabb41c29d7 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -72,6 +72,9 @@ module Ci
         .project_type
     end
 
+    scope :order_contacted_at_asc, -> { order(contacted_at: :asc) }
+    scope :order_created_at_desc, -> { order(created_at: :desc) }
+
     validate :tag_constraints
     validates :access_level, presence: true
     validates :runner_type, presence: true
@@ -124,6 +127,14 @@ module Ci
       ONLINE_CONTACT_TIMEOUT.ago
     end
 
+    def self.order_by(order)
+      if order == 'contacted_asc'
+        order_contacted_at_asc
+      else
+        order_created_at_desc
+      end
+    end
+
     def set_default_values
       self.token = SecureRandom.hex(15) if self.token.blank?
     end
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index f1d0ed15d29..b545e036aa1 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -797,4 +797,22 @@ describe Ci::Runner do
       expect { subject.destroy }.to change { described_class.count }.by(-1)
     end
   end
+
+  describe '.order_by' do
+    it 'supports ordering by the contact date' do
+      runner1 = create(:ci_runner, contacted_at: 1.year.ago)
+      runner2 = create(:ci_runner, contacted_at: 1.month.ago)
+      runners = described_class.order_by('contacted_asc')
+
+      expect(runners).to eq([runner1, runner2])
+    end
+
+    it 'supports ordering by the creation date' do
+      runner1 = create(:ci_runner, created_at: 1.year.ago)
+      runner2 = create(:ci_runner, created_at: 1.month.ago)
+      runners = described_class.order_by('created_asc')
+
+      expect(runners).to eq([runner2, runner1])
+    end
+  end
 end