Allow only indexed columns in #order_and_sort. Remove present (Because unnecessary) from condition. Added spec just in case.

This commit is contained in:
Shinya Maeda 2017-03-17 18:27:11 +09:00
parent f0e3076a32
commit 3735b8aaa1
2 changed files with 11 additions and 3 deletions

View file

@ -108,12 +108,12 @@ class PipelinesFinder
end end
def order_and_sort(items) 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] params[:order_by]
else else
:id :id
end end
sort = if params[:sort].present? && params[:sort] =~ /\A(ASC|DESC)\z/i sort = if params[:sort] =~ /\A(ASC|DESC)\z/i
params[:sort] params[:sort]
else else
:desc :desc

View file

@ -208,7 +208,7 @@ describe PipelinesFinder do
context 'when order_by and sort are valid' do context 'when order_by and sort are valid' do
let(:params) { { order_by: 'created_at', sort: 'asc' } } 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)) expect(subject).to eq(Ci::Pipeline.order(created_at: :asc))
end end
end end
@ -228,6 +228,14 @@ describe PipelinesFinder do
expect(subject).to eq(Ci::Pipeline.order(created_at: :desc)) expect(subject).to eq(Ci::Pipeline.order(created_at: :desc))
end end
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 end
end end