summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYorick Peterse <yorickpeterse@gmail.com>2018-09-14 15:05:46 +0200
committerYorick Peterse <yorickpeterse@gmail.com>2018-09-14 15:05:46 +0200
commitf0e7b5e7a321383aa847790d36b1328a70c2fe2c (patch)
treea0cabd59ae9d81f5401235ede383dbc39b2fba78
parent197beefc414c34ba9af89b91630f5d3137bf06de (diff)
downloadgitlab-ce-f0e7b5e7a321383aa847790d36b1328a70c2fe2c.tar.gz
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.
-rw-r--r--app/controllers/admin/runners_controller.rb2
-rw-r--r--app/finders/admin/runners_finder.rb3
-rw-r--r--app/models/ci/runner.rb11
-rw-r--r--spec/models/ci/runner_spec.rb18
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