Adding order by to list runner jobs api.

6 files changed, 63 insertions, 35 deletions

diff --git a/app/finders/runner_jobs_finder.rb b/app/finders/runner_jobs_finder.rb
index f1ee1d38255..ef90817416a 100644
--- a/app/finders/runner_jobs_finder.rb
+++ b/app/finders/runner_jobs_finder.rb
@@ -3,7 +3,7 @@
 class RunnerJobsFinder
   attr_reader :runner, :params
 
-  ALLOWED_INDEXED_COLUMNS = %w[id created_at].freeze
+  ALLOWED_INDEXED_COLUMNS = %w[id].freeze
 
   def initialize(runner, params = {})
     @runner = runner
@@ -28,13 +28,10 @@ class RunnerJobsFinder
 
   # rubocop: disable CodeReuse/ActiveRecord
   def sort_items(items)
-    order_by = if ALLOWED_INDEXED_COLUMNS.include?(params[:order_by])
-                 params[:order_by]
-               else
-                 :id
-               end
+    return items unless ALLOWED_INDEXED_COLUMNS.include?(params[:order_by])
 
-    sort = if params[:sort] =~ /\A(ASC|DESC)\z/i
+    order_by = params[:order_by]
+    sort = if /\A(ASC|DESC)\z/i.match?(params[:sort])
              params[:sort]
            else
              :desc
diff --git a/changelogs/unreleased/51794-add-ordering-to-runner-jobs-api.yml b/changelogs/unreleased/51794-add-ordering-to-runner-jobs-api.yml
index 6af61d7b145..908a132688c 100644
--- a/changelogs/unreleased/51794-add-ordering-to-runner-jobs-api.yml
+++ b/changelogs/unreleased/51794-add-ordering-to-runner-jobs-api.yml
@@ -1,5 +1,5 @@
 ---
-title: 51794-add-order-by-to-list-runner-jobs-api
-merge_request:
+title: Add order_by and sort params to list runner jobs api
+merge_request: 29629
 author: Sujay Patel
 type: added
diff --git a/doc/api/runners.md b/doc/api/runners.md
index 425b9aeef1b..1318b9ca828 100644
--- a/doc/api/runners.md
+++ b/doc/api/runners.md
@@ -291,7 +291,7 @@ GET /runners/:id/jobs
 |-----------|---------|----------|---------------------|
 | `id`      | integer | yes      | The ID of a runner  |
 | `status`  | string  | no       | Status of the job; one of: `running`, `success`, `failed`, `canceled` |
-| `order_by`| string  | no       | Order jobs by `id` or `created_at` (default: id) |
+| `order_by`| string  | no       | Order jobs by `id`. |
 | `sort`    | string  | no       | Sort jobs in `asc` or `desc` order (default: `desc`) |
 
 ```
diff --git a/lib/api/runners.rb b/lib/api/runners.rb
index f3fea463e7f..c2d371b6867 100644
--- a/lib/api/runners.rb
+++ b/lib/api/runners.rb
@@ -115,6 +115,8 @@ module API
       params do
         requires :id, type: Integer, desc: 'The ID of the runner'
         optional :status, type: String, desc: 'Status of the job', values: Ci::Build::AVAILABLE_STATUSES
+        optional :order_by, type: String, desc: 'Order by `id` or not', values: RunnerJobsFinder::ALLOWED_INDEXED_COLUMNS
+        optional :sort, type: String, values: %w[asc desc], default: 'desc', desc: 'Sort by asc (ascending) or desc (descending)'
         use :pagination
       end
       get ':id/jobs' do
diff --git a/spec/finders/runner_jobs_finder_spec.rb b/spec/finders/runner_jobs_finder_spec.rb
index 9ed6f50ddfb..01f45a37ba8 100644
--- a/spec/finders/runner_jobs_finder_spec.rb
+++ b/spec/finders/runner_jobs_finder_spec.rb
@@ -37,38 +37,23 @@ describe RunnerJobsFinder do
     end
 
     context 'when order_by and sort are specified' do
-      context 'when order_by created_at' do
-        let(:params) { { order_by: 'created_at', sort: 'asc' } }
-        let!(:jobs) { Array.new(2) { create(:ci_build, runner: runner, project: project, user: create(:user)) } }
-
-        it 'sorts as created_at: :asc' do
-          is_expected.to match_array(jobs)
-        end
-
-        context 'when sort is invalid' do
-          let(:params) { { order_by: 'created_at', sort: 'invalid_sort' } }
-
-          it 'sorts as created_at: :desc' do
-            is_expected.to eq(jobs.sort_by { |p| -p.user.id })
-          end
-        end
-      end
-
-      context 'when order_by is invalid' do
-        let(:params) { { order_by: 'invalid_column', sort: 'asc' } }
-        let!(:jobs) { Array.new(2) { create(:ci_build, runner: runner, project: project, user: create(:user)) } }
+      context 'when order_by id and sort is asc' do
+        let(:params) { { order_by: 'id', sort: 'asc' } }
+        let!(:jobs) { create_list(:ci_build, 2, runner: runner, project: project, user: create(:user)) }
 
         it 'sorts as id: :asc' do
-          is_expected.to eq(jobs.sort_by { |p| p.id })
+          is_expected.to eq(jobs.sort_by(&:id))
         end
       end
+    end
 
-      context 'when both are nil' do
-        let(:params) { { order_by: nil, sort: nil } }
-        let!(:jobs) { Array.new(2) { create(:ci_build, runner: runner, project: project, user: create(:user)) } }
+    context 'when order_by is specified and sort is not specified' do
+      context 'when order_by id and sort is not specified' do
+        let(:params) { { order_by: 'id' } }
+        let!(:jobs) { create_list(:ci_build, 2, runner: runner, project: project, user: create(:user)) }
 
         it 'sorts as id: :desc' do
-          is_expected.to eq(jobs.sort_by { |p| -p.id })
+          is_expected.to eq(jobs.sort_by(&:id).reverse)
         end
       end
     end
diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb
index 5548e3fd01a..f5ce3a3570e 100644
--- a/spec/requests/api/runners_spec.rb
+++ b/spec/requests/api/runners_spec.rb
@@ -584,6 +584,34 @@ describe API::Runners do
           end
         end
 
+        context 'when valid order_by is provided' do
+          context 'when sort order is not specified' do
+            it 'return jobs in descending order' do
+              get api("/runners/#{project_runner.id}/jobs?order_by=id", admin)
+
+              expect(response).to have_gitlab_http_status(200)
+              expect(response).to include_pagination_headers
+
+              expect(json_response).to be_an(Array)
+              expect(json_response.length).to eq(2)
+              expect(json_response.first).to include('id' => job_5.id)
+            end
+          end
+
+          context 'when sort order is specified as asc' do
+            it 'return jobs sorted in ascending order' do
+              get api("/runners/#{project_runner.id}/jobs?order_by=id&sort=asc", admin)
+
+              expect(response).to have_gitlab_http_status(200)
+              expect(response).to include_pagination_headers
+
+              expect(json_response).to be_an(Array)
+              expect(json_response.length).to eq(2)
+              expect(json_response.first).to include('id' => job_4.id)
+            end
+          end
+        end
+
         context 'when invalid status is provided' do
           it 'return 400' do
             get api("/runners/#{project_runner.id}/jobs?status=non-existing", admin)
@@ -591,6 +619,22 @@ describe API::Runners do
             expect(response).to have_gitlab_http_status(400)
           end
         end
+
+        context 'when invalid order_by is provided' do
+          it 'return 400' do
+            get api("/runners/#{project_runner.id}/jobs?order_by=non-existing", admin)
+
+            expect(response).to have_gitlab_http_status(400)
+          end
+        end
+
+        context 'when invalid sort is provided' do
+          it 'return 400' do
+            get api("/runners/#{project_runner.id}/jobs?sort=non-existing", admin)
+
+            expect(response).to have_gitlab_http_status(400)
+          end
+        end
       end
 
       context "when runner doesn't exist" do