Cleaned up CI runner administration code

In https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/19625 some
changes were introduced that do not meet our abstraction reuse rules.
This commit cleans up some of these changes so the requirements are met.

Most notably, sorting of the runners in Admin::RunnersFinder has been
delegated to Ci::Runner.order_by, similar to how we order data in
models that include the Sortable module. If we need more sort orders in
the future we can include Sortable and have Ci::Runner.order_by call
`super` to delegate to Sortable.order_by.

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