From fd302061f915f535b2dd419d5a76efb76ab534be Mon Sep 17 00:00:00 2001
From: Shinya Maeda <gitlab.shinyamaeda@gmail.com>
Date: Wed, 1 Mar 2017 15:58:06 +0900
Subject: [PATCH] Fix rubocop offences and rspec failures

---
 app/finders/pipelines_finder.rb       |  4 +--
 lib/api/pipelines.rb                  |  6 ++--
 spec/finders/pipelines_finder_spec.rb | 45 +++++++++++++--------------
 3 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/app/finders/pipelines_finder.rb b/app/finders/pipelines_finder.rb
index ff16d7305ad..4a44a469a48 100644
--- a/app/finders/pipelines_finder.rb
+++ b/app/finders/pipelines_finder.rb
@@ -101,8 +101,8 @@ class PipelinesFinder
 
   def order_and_sort(items)
     if params[:order_by].present? && params[:sort].present? && 
-       items.column_names.include?(params[:order_by]) && 
-       (params[:sort].downcase == 'asc' || params[:sort].downcase == 'desc')
+        items.column_names.include?(params[:order_by]) && 
+        (params[:sort].casecmp('ASC') || params[:sort].casecmp('DESC'))
       items.order("#{params[:order_by]} #{params[:sort]}")
     else
       items.order(id: :desc)
diff --git a/lib/api/pipelines.rb b/lib/api/pipelines.rb
index ecd6b64cfa7..70837d91c7a 100644
--- a/lib/api/pipelines.rb
+++ b/lib/api/pipelines.rb
@@ -16,14 +16,14 @@ module API
         use :pagination
         optional :scope,    type: String, values: %w(running pending finished branches tags),
                             desc: 'The scope of pipelines'
-        optional :status,   type: String, values: ['running', 'pending', 'success', 'failed', 'canceled', 'skipped'],
+        optional :status,   type: String, values: %w(running pending success failed canceled skipped),
                             desc: 'The status of pipelines'
         optional :ref,      type: String, desc: 'The ref of pipelines'
         optional :yaml_errors, type: Boolean, desc: 'If true, Returns only yaml error pipelines'
         optional :username, type: String, desc: 'The name of user who triggered pipelines'
-        optional :order_by, type: String, values: ['id', 'status', 'ref', 'username', 'started_at', 'finished_at', 'created_at', 'updated_at'], default: 'id',
+        optional :order_by, type: String, values: %w(id status ref username started_at finished_at created_at updated_at), default: 'id',
                             desc: 'The order_by which is combined with a sort'
-        optional :sort,     type: String, values: ['asc', 'desc'], default: 'desc',
+        optional :sort,     type: String, values: %w(asc desc), default: 'desc',
                             desc: 'The sort method which is combined with an order_by'
       end
       get ':id/pipelines' do
diff --git a/spec/finders/pipelines_finder_spec.rb b/spec/finders/pipelines_finder_spec.rb
index 3bb828000d4..336901d0264 100644
--- a/spec/finders/pipelines_finder_spec.rb
+++ b/spec/finders/pipelines_finder_spec.rb
@@ -34,16 +34,15 @@ describe PipelinesFinder do
       end
 
       it 'orders in descending order on ID' do
-        expected_ids = [
-          tag_pipeline.id, 
-          created_pipeline.id, 
-          pending_pipeline.id,
-          running_pipeline.id,
-          success_pipeline.id,
-          failed_pipeline.id,
-          canceled_pipeline.id,
-          skipped_pipeline.id,
-          yaml_errors_pipeline.id].sort.reverse
+        expected_ids = [tag_pipeline.id, 
+                        created_pipeline.id, 
+                        pending_pipeline.id,
+                        running_pipeline.id,
+                        success_pipeline.id,
+                        failed_pipeline.id,
+                        canceled_pipeline.id,
+                        skipped_pipeline.id,
+                        yaml_errors_pipeline.id].sort.reverse
         expect(subject.map(&:id)).to eq expected_ids
       end
     end
@@ -214,21 +213,21 @@ describe PipelinesFinder do
     end
 
     context 'when a order_by and sort are passed' do
-      context 'when order by started_at asc' do
+      context 'when order by created_at asc' do
         let(:params) { { order_by: 'created_at', sort: 'asc' } }
 
-        it 'sorts by started_at asc' do
-          expected_created_at = [
-            tag_pipeline.created_at, 
-            created_pipeline.created_at, 
-            pending_pipeline.created_at,
-            running_pipeline.created_at,
-            success_pipeline.created_at,
-            failed_pipeline.created_at,
-            canceled_pipeline.created_at,
-            skipped_pipeline.created_at,
-            yaml_errors_pipeline.created_at].sort
-          expect(subject.map(&:created_at)).to eq expected_created_at
+        it 'sorts by created_at asc' do
+          expect(subject.first).to eq(tag_pipeline)
+          expect(subject.last).to eq(yaml_errors_pipeline)
+        end
+      end
+
+      context 'when order by created_at desc' do
+        let(:params) { { order_by: 'created_at', sort: 'desc' } }
+
+        it 'sorts by created_at desc' do
+          expect(subject.first).to eq(yaml_errors_pipeline)
+          expect(subject.last).to eq(tag_pipeline)
         end
       end
     end