diff --git a/app/finders/pipelines_finder.rb b/app/finders/pipelines_finder.rb index 5e50eb46c7e..6a92aedc873 100644 --- a/app/finders/pipelines_finder.rb +++ b/app/finders/pipelines_finder.rb @@ -108,12 +108,12 @@ class PipelinesFinder end def order_and_sort(items) - order_by = if params[:order_by].present? && items.column_names.include?(params[:order_by]) + order_by = if %w[id status ref user_id].include?(params[:order_by]) # Allow only indexed columns params[:order_by] else :id end - sort = if params[:sort].present? && params[:sort] =~ /\A(ASC|DESC)\z/i + sort = if params[:sort] =~ /\A(ASC|DESC)\z/i params[:sort] else :desc diff --git a/spec/finders/pipelines_finder_spec.rb b/spec/finders/pipelines_finder_spec.rb index 02422bb6ebc..02c91cbf465 100644 --- a/spec/finders/pipelines_finder_spec.rb +++ b/spec/finders/pipelines_finder_spec.rb @@ -208,7 +208,7 @@ describe PipelinesFinder do context 'when order_by and sort are valid' do let(:params) { { order_by: 'created_at', sort: 'asc' } } - it 'sorts pipelines' do + it 'sorts pipelines by default' do expect(subject).to eq(Ci::Pipeline.order(created_at: :asc)) end end @@ -228,6 +228,14 @@ describe PipelinesFinder do expect(subject).to eq(Ci::Pipeline.order(created_at: :desc)) end end + + context 'when both are nil' do + let(:params) { { order_by: nil, sort: nil } } + + it 'sorts pipelines by default' do + expect(subject).to eq(Ci::Pipeline.order(id: :desc)) + end + end end end end