Implement sort param for bulk imports API

* Make sort descending by default

Changelog: added

app/finders/bulk_imports/entities_finder.rb

@@ -2,10 +2,10 @@
 
 module BulkImports
   class EntitiesFinder
-    def initialize(user:, bulk_import: nil, status: nil)
+    def initialize(user:, bulk_import: nil, params: {})
       @user = user
       @bulk_import = bulk_import
-      @status = status
+      @params = params
     end
 
     def execute
@@ -14,6 +14,7 @@ module BulkImports
         .by_user_id(user.id)
         .then(&method(:filter_by_bulk_import))
         .then(&method(:filter_by_status))
+        .then(&method(:sort))
     end
 
     private
@@ -23,13 +24,19 @@ module BulkImports
     def filter_by_bulk_import(entities)
       return entities unless bulk_import
 
-      entities.where(bulk_import_id: bulk_import.id) # rubocop: disable CodeReuse/ActiveRecord
+      entities.by_bulk_import_id(bulk_import.id)
     end
 
     def filter_by_status(entities)
-      return entities unless ::BulkImports::Entity.all_human_statuses.include?(status)
+      return entities unless ::BulkImports::Entity.all_human_statuses.include?(@params[:status])
 
-      entities.with_status(status)
+      entities.with_status(@params[:status])
+    end
+
+    def sort(entities)
+      return entities unless @params[:sort]
+
+      entities.order_by_created_at(@params[:sort])
     end
   end
 end

app/finders/bulk_imports/imports_finder.rb

@@ -2,13 +2,14 @@
 
 module BulkImports
   class ImportsFinder
-    def initialize(user:, status: nil)
+    def initialize(user:, params: {})
       @user = user
-      @status = status
+      @params = params
     end
 
     def execute
-      filter_by_status(user.bulk_imports)
+      imports = filter_by_status(user.bulk_imports)
+      sort(imports)
     end
 
     private
@@ -16,9 +17,15 @@ module BulkImports
     attr_reader :user, :status
 
     def filter_by_status(imports)
-      return imports unless BulkImport.all_human_statuses.include?(status)
+      return imports unless BulkImport.all_human_statuses.include?(@params[:status])
 
-      imports.with_status(status)
+      imports.with_status(@params[:status])
+    end
+
+    def sort(imports)
+      return imports unless @params[:sort]
+
+      imports.order_by_created_at(@params[:sort])
     end
   end
 end

app/models/bulk_import.rb

@@ -17,6 +17,7 @@ class BulkImport < ApplicationRecord
   enum source_type: { gitlab: 0 }
 
   scope :stale, -> { where('created_at < ?', 8.hours.ago).where(status: [0, 1]) }
+  scope :order_by_created_at, -> (direction) { order(created_at: direction) }
 
   state_machine :status, initial: :created do
     state :created, value: 0

app/models/bulk_imports/entity.rb

@@ -52,6 +52,8 @@ class BulkImports::Entity < ApplicationRecord
 
   scope :by_user_id, ->(user_id) { joins(:bulk_import).where(bulk_imports: { user_id: user_id }) }
   scope :stale, -> { where('created_at < ?', 8.hours.ago).where(status: [0, 1]) }
+  scope :by_bulk_import_id, ->(bulk_import_id) { where(bulk_import_id: bulk_import_id)}
+  scope :order_by_created_at, -> (direction) { order(created_at: direction) }
 
   state_machine :status, initial: :created do
     state :created, value: 0

doc/api/bulk_imports.md

@@ -59,9 +59,10 @@ GET /bulk_imports
 ```
 
 | Attribute  | Type    | Required | Description                                                                                 |
-|:-----------|:--------|:---------|:---------------------------------------|
+|:-----------|:--------|:---------|:--------------------------------------------------------------------------------------------|
 | `per_page` | integer | no       | Number of records to return per page.                                                       |
 | `page`     | integer | no       | Page to retrieve.                                                                           |
+| `sort`     | string  | no       | Return GitLab migration sorted in `asc` or `desc` order by creation date. Default is `desc` |
 | `status`   | string  | no       | Import status.                                                                              |
 
 The status can be one of the following:
@@ -101,9 +102,10 @@ GET /bulk_imports/entities
 ```
 
 | Attribute  | Type    | Required | Description                                                                                          |
-|:-----------|:--------|:---------|:---------------------------------------|
+|:-----------|:--------|:---------|:-----------------------------------------------------------------------------------------------------|
 | `per_page` | integer | no       | Number of records to return per page.                                                                |
 | `page`     | integer | no       | Page to retrieve.                                                                                    |
+| `sort`     | string  | no       | Return GitLab migration entities sorted in `asc` or `desc` order by creation date. Default is `desc` |
 | `status`   | string  | no       | Import status.                                                                                       |
 
 The status can be one of the following:
@@ -185,9 +187,10 @@ GET /bulk_imports/:id/entities
 ```
 
 | Attribute  | Type    | Required | Description                                                                                 |
-|:-----------|:--------|:---------|:---------------------------------------|
+|:-----------|:--------|:---------|:--------------------------------------------------------------------------------------------|
 | `per_page` | integer | no       | Number of records to return per page.                                                       |
 | `page`     | integer | no       | Page to retrieve.                                                                           |
+| `sort`     | string  | no       | Return GitLab migration sorted in `asc` or `desc` order by creation date. Default is `desc` |
 | `status`   | string  | no       | Import status.                                                                              |
 
 The status can be one of the following:

lib/api/bulk_imports.rb

@@ -10,7 +10,7 @@ module API
       def bulk_imports
         @bulk_imports ||= ::BulkImports::ImportsFinder.new(
           user: current_user,
-          status: params[:status]
+          params: params
         ).execute
       end
 
@@ -22,7 +22,7 @@ module API
         @bulk_import_entities ||= ::BulkImports::EntitiesFinder.new(
           user: current_user,
           bulk_import: bulk_import,
-          status: params[:status]
+          params: params
         ).execute
       end
 
@@ -70,6 +70,8 @@ module API
       end
       params do
         use :pagination
+        optional :sort, type: String, values: %w[asc desc], default: 'desc',
+        desc: 'Return GitLab Migrations sorted in created by `asc` or `desc` order.'
         optional :status, type: String, values: BulkImport.all_human_statuses,
           desc: 'Return GitLab Migrations with specified status'
       end
@@ -82,13 +84,15 @@ module API
       end
       params do
         use :pagination
+        optional :sort, type: String, values: %w[asc desc], default: 'desc',
+          desc: 'Return GitLab Migrations sorted in created by `asc` or `desc` order.'
         optional :status, type: String, values: ::BulkImports::Entity.all_human_statuses,
           desc: "Return all GitLab Migrations' entities with specified status"
       end
       get :entities do
         entities = ::BulkImports::EntitiesFinder.new(
           user: current_user,
-          status: params[:status]
+          params: params
         ).execute
 
         present paginate(entities), with: Entities::BulkImports::Entity

spec/finders/bulk_imports/entities_finder_spec.rb

@@ -51,7 +51,7 @@ RSpec.describe BulkImports::EntitiesFinder do
     end
 
     context 'when status is specified' do
-      subject { described_class.new(user: user, status: 'failed') }
+      subject { described_class.new(user: user, params: { status: 'failed' }) }
 
       it 'returns a list of import entities filtered by status' do
         expect(subject.execute)
@@ -61,7 +61,7 @@ RSpec.describe BulkImports::EntitiesFinder do
       end
 
       context 'when invalid status is specified' do
-        subject { described_class.new(user: user, status: 'invalid') }
+        subject { described_class.new(user: user, params: { status: 'invalid' }) }
 
         it 'does not filter entities by status' do
           expect(subject.execute)
@@ -74,11 +74,37 @@ RSpec.describe BulkImports::EntitiesFinder do
     end
 
     context 'when bulk import and status are specified' do
-      subject { described_class.new(user: user, bulk_import: user_import_2, status: 'finished') }
+      subject { described_class.new(user: user, bulk_import: user_import_2, params: { status: 'finished' }) }
 
       it 'returns matched import entities' do
         expect(subject.execute).to contain_exactly(finished_entity_2)
       end
     end
+
+    context 'when order is specifed' do
+      subject { described_class.new(user: user, params: { sort: order }) }
+
+      context 'when order is specified as asc' do
+        let(:order) { :asc }
+
+        it 'returns entities sorted ascending' do
+          expect(subject.execute).to eq([
+            started_entity_1, finished_entity_1, failed_entity_1,
+            started_entity_2, finished_entity_2, failed_entity_2
+          ])
+        end
+      end
+
+      context 'when order is specified as desc' do
+        let(:order) { :desc }
+
+        it 'returns entities sorted descending' do
+          expect(subject.execute).to eq([
+            failed_entity_2, finished_entity_2, started_entity_2,
+            failed_entity_1, finished_entity_1, started_entity_1
+          ])
+        end
+      end
+    end
   end
 end

spec/finders/bulk_imports/imports_finder_spec.rb

@@ -16,19 +16,39 @@ RSpec.describe BulkImports::ImportsFinder do
     end
 
     context 'when status is specified' do
-      subject { described_class.new(user: user, status: 'started') }
+      subject { described_class.new(user: user, params: { status: 'started' }) }
 
       it 'returns a list of import entities filtered by status' do
         expect(subject.execute).to contain_exactly(started_import)
       end
 
       context 'when invalid status is specified' do
-        subject { described_class.new(user: user, status: 'invalid') }
+        subject { described_class.new(user: user, params: { status: 'invalid' }) }
 
         it 'does not filter entities by status' do
           expect(subject.execute).to contain_exactly(started_import, finished_import)
         end
       end
     end
+
+    context 'when order is specifed' do
+      subject { described_class.new(user: user, params: { sort: order }) }
+
+      context 'when order is specified as asc' do
+        let(:order) { :asc }
+
+        it 'returns entities sorted ascending' do
+          expect(subject.execute).to eq([started_import, finished_import])
+        end
+      end
+
+      context 'when order is specified as desc' do
+        let(:order) { :desc }
+
+        it 'returns entities sorted descending' do
+          expect(subject.execute).to eq([finished_import, started_import])
+        end
+      end
+    end
   end
 end

spec/requests/api/bulk_imports_spec.rb

@@ -18,6 +18,29 @@ RSpec.describe API::BulkImports do
       expect(response).to have_gitlab_http_status(:ok)
       expect(json_response.pluck('id')).to contain_exactly(import_1.id, import_2.id)
     end
+
+    context 'sort parameter' do
+      it 'sorts by created_at descending by default' do
+        get api('/bulk_imports', user)
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response.pluck('id')).to eq([import_2.id, import_1.id])
+      end
+
+      it 'sorts by created_at descending when explicitly specified' do
+        get api('/bulk_imports', user), params: { sort: 'desc' }
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response.pluck('id')).to eq([import_2.id, import_1.id])
+      end
+
+      it 'sorts by created_at ascending when explicitly specified' do
+        get api('/bulk_imports', user), params: { sort: 'asc' }
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response.pluck('id')).to eq([import_1.id, import_2.id])
+      end
+    end
   end
 
   describe 'POST /bulk_imports' do