Refactorize jobs finding logic

Author: tmaczukin

app/finders/runner_jobs_finder.rb

@@ -0,0 +1,22 @@
+class RunnerJobsFinder
+  attr_reader :runner, :params
+
+  def initialize(runner, params = {})
+    @runner = runner
+    @params = params
+  end
+
+  def execute
+    items = @runner.builds
+    items = by_status(items)
+    items
+  end
+
+  private
+
+  def by_status(items)
+    return items unless HasStatus::AVAILABLE_STATUSES.include?(params[:status])
+
+    items.where(status: params[:status])
+  end
+end

lib/api/runners.rb

@@ -90,18 +90,14 @@ class Runners < Grape::API
       end
       params do
         requires :id, type: Integer, desc: 'The ID of the runner'
-        optional :status, type: String, desc: 'Status of job'
+        optional :status, type: String, desc: 'Status of the job', values: Ci::Build::AVAILABLE_STATUSES
         use :pagination
       end
       get  ':id/jobs' do
         runner = get_runner(params[:id])
         authenticate_list_runners_jobs!(runner)
 
-        jobs = runner.builds
-        if params[:status]
-          not_found!('Status') unless Ci::Build::AVAILABLE_STATUSES.include?(params[:status])
-          jobs = jobs.where(status: params[:status].to_sym)
-        end
+        jobs = RunnerJobsFinder.new(runner, params).execute
 
         present paginate(jobs), with: Entities::JobBasicWithProject
       end

spec/finders/runner_jobs_finder_spec.rb

@@ -0,0 +1,39 @@
+require 'spec_helper'
+
+describe RunnerJobsFinder do
+  let(:project) { create(:project) }
+  let(:runner) { create(:ci_runner, :shared) }
+
+  subject { described_class.new(runner, params).execute }
+
+  describe '#execute' do
+    context 'when params is empty' do
+      let(:params) { {} }
+      let!(:job) { create(:ci_build, runner: runner, project: project) }
+      let!(:job1) { create(:ci_build, project: project) }
+
+      it 'returns all jobs assigned to Runner' do
+        is_expected.to match_array(job)
+        is_expected.not_to match_array(job1)
+      end
+    end
+
+    context 'when params contains status' do
+      HasStatus::AVAILABLE_STATUSES.each do |target_status|
+        context "when status is #{target_status}" do
+          let(:params) { { status: target_status } }
+          let!(:job) { create(:ci_build, runner: runner, project: project, status: target_status) }
+
+          before do
+            exception_status = HasStatus::AVAILABLE_STATUSES - [target_status]
+            create(:ci_build, runner: runner, project: project, status: exception_status.first)
+          end
+
+          it 'returns matched job' do
+            is_expected.to eq([job])
+          end
+        end
+      end
+    end
+  end
+end

spec/requests/api/runners_spec.rb

@@ -401,10 +401,10 @@ def update_runner(id, user, args)
         end
 
         context 'when invalid status is provided' do
-          it 'return 404' do
+          it 'return 400' do
             get api("/runners/#{specific_runner.id}/jobs?status=non-existing", admin)
 
-            expect(response).to have_gitlab_http_status(404)
+            expect(response).to have_gitlab_http_status(400)
           end
         end
       end
@@ -454,10 +454,10 @@ def update_runner(id, user, args)
         end
 
         context 'when invalid status is provided' do
-          it 'return 404' do
+          it 'return 400' do
             get api("/runners/#{specific_runner.id}/jobs?status=non-existing", user)
 
-            expect(response).to have_gitlab_http_status(404)
+            expect(response).to have_gitlab_http_status(400)
           end
         end
       end