From 03ef20bfdeb89d194dea0364fd5a07412cec5a96 Mon Sep 17 00:00:00 2001
From: kikito <kikito@gmail.com>
Date: Sat, 24 Dec 2016 14:42:46 +0100
Subject: [PATCH] Merges orders_helper functionality inside has_orders concern

Both these objects where using/setting a variable called valid_orders.
As a results, some specs where becoming flacky
(features/budgets/investments_spec.rb, balloting phase). By putting
them together, there is no `valid_orders` variable any more (everything
is @valid_orders) and the flackyness is gone.
---
 app/controllers/concerns/has_orders.rb       |  4 ++--
 app/helpers/orders_helper.rb                 |  7 -------
 app/views/shared/_order_links.html.erb       |  2 +-
 app/views/shared/_order_selector.html.erb    |  2 +-
 spec/controllers/concerns/has_orders_spec.rb | 13 +++++++++----
 spec/helpers/orders_helper_spec.rb           | 18 ------------------
 6 files changed, 13 insertions(+), 33 deletions(-)
 delete mode 100644 app/helpers/orders_helper.rb
 delete mode 100644 spec/helpers/orders_helper_spec.rb

diff --git a/app/controllers/concerns/has_orders.rb b/app/controllers/concerns/has_orders.rb
index b17781da2..7adadc1d3 100644
--- a/app/controllers/concerns/has_orders.rb
+++ b/app/controllers/concerns/has_orders.rb
@@ -4,8 +4,8 @@ module HasOrders
   class_methods do
     def has_orders(valid_orders, *args)
       before_action(*args) do |c|
-        valid_orders = valid_orders.call(c) if valid_orders.respond_to?(:call)
-        @valid_orders = valid_orders
+        @valid_orders = valid_orders.respond_to?(:call) ? valid_orders.call(c) : valid_orders
+        @valid_orders.delete('relevance') unless params[:search].present?
         @current_order = @valid_orders.include?(params[:order]) ? params[:order] : @valid_orders.first
       end
     end
diff --git a/app/helpers/orders_helper.rb b/app/helpers/orders_helper.rb
deleted file mode 100644
index 08d5588ca..000000000
--- a/app/helpers/orders_helper.rb
+++ /dev/null
@@ -1,7 +0,0 @@
-module OrdersHelper
-
-  def valid_orders
-    @valid_orders.reject { |order| order =='relevance' && params[:search].blank? }
-  end
-
-end
\ No newline at end of file
diff --git a/app/views/shared/_order_links.html.erb b/app/views/shared/_order_links.html.erb
index 1032939d9..622f253ec 100644
--- a/app/views/shared/_order_links.html.erb
+++ b/app/views/shared/_order_links.html.erb
@@ -1,5 +1,5 @@
 <div class="submenu">
-  <% valid_orders.each do |order| %>
+  <% @valid_orders.each do |order| %>
     <%= link_to current_path_with_query_params(order: order, page: 1), class: order == @current_order ? 'active' : '' do %>
       <%= content_tag(order == @current_order ? :h2 : :span) do %>
         <%= t("#{i18n_namespace}.orders.#{order}") %>
diff --git a/app/views/shared/_order_selector.html.erb b/app/views/shared/_order_selector.html.erb
index cd3e2cbd5..d747f6f52 100644
--- a/app/views/shared/_order_selector.html.erb
+++ b/app/views/shared/_order_selector.html.erb
@@ -4,7 +4,7 @@
           data-order="<%= @current_order %>"
           name="order-selector"
           id="order-selector-participation">
-     <% valid_orders.each do |order| %>
+     <% @valid_orders.each do |order| %>
        <option <%= 'selected' if order == @current_order %>
           value='<%= current_path_with_query_params(order: order, page: 1) %>'>
          <%= t("#{i18n_namespace}.orders.#{order}") %>
diff --git a/spec/controllers/concerns/has_orders_spec.rb b/spec/controllers/concerns/has_orders_spec.rb
index e7408c976..493ffcc3c 100644
--- a/spec/controllers/concerns/has_orders_spec.rb
+++ b/spec/controllers/concerns/has_orders_spec.rb
@@ -1,13 +1,13 @@
 require 'rails_helper'
 
-xdescribe 'HasOrders' do
+describe 'HasOrders' do
 
   class FakeController < ActionController::Base; end
 
   controller(FakeController) do
     include HasOrders
-    has_orders ['created_at', 'votes_count', 'flags_count'], only: :index
-    has_orders -> { ['votes_count', 'flags_count'] }, only: :new
+    has_orders ['created_at', 'votes_count', 'flags_count', 'relevance'], only: :index
+    has_orders -> (c) { ['votes_count', 'flags_count'] }, only: :new
 
     def index
       render text: "#{@current_order} (#{@valid_orders.join(' ')})"
@@ -18,7 +18,7 @@ xdescribe 'HasOrders' do
     end
   end
 
-  it "has the valid orders set up" do
+  it "displays all the orders except relevance when not searching" do
     get :index
     expect(response.body).to eq('created_at (created_at votes_count flags_count)')
   end
@@ -28,6 +28,11 @@ xdescribe 'HasOrders' do
     expect(response.body).to eq('votes_count (votes_count flags_count)')
   end
 
+  it "displays relevance when searching" do
+    get :index, search: 'ipsum'
+    expect(response.body).to eq('created_at (created_at votes_count flags_count relevance)')
+  end
+
   describe "the current order" do
     it "defaults to the first one on the list" do
       get :index
diff --git a/spec/helpers/orders_helper_spec.rb b/spec/helpers/orders_helper_spec.rb
deleted file mode 100644
index 39475f7b2..000000000
--- a/spec/helpers/orders_helper_spec.rb
+++ /dev/null
@@ -1,18 +0,0 @@
-require 'rails_helper'
-
-describe OrdersHelper do
-
-  describe '#valid_orders' do
-    it 'displays relevance when searching' do
-      params[:search] = 'ipsum'
-      assign(:valid_orders, %w(created_at random relevance))
-      expect(helper.valid_orders).to eq %w(created_at random relevance)
-    end
-
-    it 'does not display relevance when not searching' do
-      assign(:valid_orders, %w(created_at random relevance))
-      expect(helper.valid_orders).to eq %w(created_at random)
-    end
-  end
-
-end
\ No newline at end of file