Commit f0e7b5e7 authored by Yorick Peterse's avatar Yorick Peterse

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.
parent 197beefc
class Admin::RunnersController < Admin::ApplicationController class Admin::RunnersController < Admin::ApplicationController
before_action :runner, except: :index before_action :runner, except: :index
# rubocop: disable CodeReuse/ActiveRecord
def index def index
finder = Admin::RunnersFinder.new(params: params) finder = Admin::RunnersFinder.new(params: params)
@runners = finder.execute @runners = finder.execute
@active_runners_count = Ci::Runner.online.count @active_runners_count = Ci::Runner.online.count
@sort = finder.sort_key @sort = finder.sort_key
end end
# rubocop: enable CodeReuse/ActiveRecord
def show def show
assign_builds_and_projects assign_builds_and_projects
......
...@@ -43,8 +43,7 @@ class Admin::RunnersFinder < UnionFinder ...@@ -43,8 +43,7 @@ class Admin::RunnersFinder < UnionFinder
end end
def sort! def sort!
sort = sort_key == 'contacted_asc' ? { contacted_at: :asc } : { created_at: :desc } @runners = @runners.order_by(sort_key)
@runners = @runners.order(sort)
end end
def paginate! def paginate!
......
...@@ -72,6 +72,9 @@ module Ci ...@@ -72,6 +72,9 @@ module Ci
.project_type .project_type
end end
scope :order_contacted_at_asc, -> { order(contacted_at: :asc) }
scope :order_created_at_desc, -> { order(created_at: :desc) }
validate :tag_constraints validate :tag_constraints
validates :access_level, presence: true validates :access_level, presence: true
validates :runner_type, presence: true validates :runner_type, presence: true
...@@ -124,6 +127,14 @@ module Ci ...@@ -124,6 +127,14 @@ module Ci
ONLINE_CONTACT_TIMEOUT.ago ONLINE_CONTACT_TIMEOUT.ago
end 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 def set_default_values
self.token = SecureRandom.hex(15) if self.token.blank? self.token = SecureRandom.hex(15) if self.token.blank?
end end
......
...@@ -797,4 +797,22 @@ describe Ci::Runner do ...@@ -797,4 +797,22 @@ describe Ci::Runner do
expect { subject.destroy }.to change { described_class.count }.by(-1) expect { subject.destroy }.to change { described_class.count }.by(-1)
end end
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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment