From 3735b8aaa1f48ea3803e31e18f1e40d2fd091b26 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <gitlab.shinyamaeda@gmail.com>
Date: Fri, 17 Mar 2017 18:27:11 +0900
Subject: [PATCH] Allow only indexed columns in #order_and_sort. Remove present
(Because unnecessary) from condition. Added spec just in case.
---
app/finders/pipelines_finder.rb | 4 ++--
spec/finders/pipelines_finder_spec.rb | 10 +++++++++-
2 files changed, 11 insertions(+), 3 deletions(-)
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
--
GitLab