From 8ecf7f45054e30f33d86448f63047b2c10a2e137 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Mon, 18 Feb 2019 15:07:35 +0100
Subject: [PATCH 1/2] Extract constant to define investments per page

That way it's easier to stub in tests. It makes it easier to customize
CONSUL to show a different number of investments per page as well.
---
 app/controllers/budgets/investments_controller.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb
index 73c127db4..381a4cea1 100644
--- a/app/controllers/budgets/investments_controller.rb
+++ b/app/controllers/budgets/investments_controller.rb
@@ -6,6 +6,8 @@ module Budgets
     include FlagActions
     include ImageAttributes
 
+    PER_PAGE = 10
+
     before_action :authenticate_user!, except: [:index, :show, :json_data]
 
     load_and_authorize_resource :budget, except: :json_data
@@ -37,7 +39,7 @@ module Budgets
     respond_to :html, :js
 
     def index
-      @investments = investments.page(params[:page]).per(10).for_render
+      @investments = investments.page(params[:page]).per(PER_PAGE).for_render
 
       @investment_ids = @investments.pluck(:id)
       @investments_map_coordinates = MapLocation.where(investment: investments).map(&:json_data)

From facfb807e1c38194738c6199d9298b2f222a36ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Mon, 18 Feb 2019 15:44:04 +0100
Subject: [PATCH 2/2] Show all investments in the map

We were showing only the ones being shown in the current page because
we were modifying `@investments` using a method which used
`@investments`, and we were calling that method twice.

There are many possible solutions: using a local variable to store the
result of the `investments` method, modifying `@investments` after
modifying `@investments_map_coordinates`, ... I've used the one which in
my humble opinion is a bit less fragile: not using `@investments` inside
the `investments` method. That way, the `investments` method will always
return the same result.

Note `stub_const("Budgets::InvestmentsController::PER_PAGE", 2)`
wouldn't work because `Budgets::InvestmentsController` isn't loaded when
that line is executed. So we need to load it. Instead of requiring the
file, using `#{Budgets::InvestmentsController}` seems to be an easier
solution.
---
 .../budgets/investments_controller.rb          |  8 ++++----
 spec/features/budgets/investments_spec.rb      | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb
index 381a4cea1..a48fa3188 100644
--- a/app/controllers/budgets/investments_controller.rb
+++ b/app/controllers/budgets/investments_controller.rb
@@ -169,11 +169,11 @@ module Budgets
 
       def investments
         if @current_order == 'random'
-          @investments.apply_filters_and_search(@budget, params, @current_filter)
-                      .send("sort_by_#{@current_order}", params[:random_seed])
+          @budget.investments.apply_filters_and_search(@budget, params, @current_filter)
+                             .send("sort_by_#{@current_order}", params[:random_seed])
         else
-          @investments.apply_filters_and_search(@budget, params, @current_filter)
-                      .send("sort_by_#{@current_order}")
+          @budget.investments.apply_filters_and_search(@budget, params, @current_filter)
+                             .send("sort_by_#{@current_order}")
         end
       end
 
diff --git a/spec/features/budgets/investments_spec.rb b/spec/features/budgets/investments_spec.rb
index a0d02bc37..e669579df 100644
--- a/spec/features/budgets/investments_spec.rb
+++ b/spec/features/budgets/investments_spec.rb
@@ -1874,6 +1874,24 @@ feature 'Budget Investments' do
         expect(page).to have_css(".map-icon", count: 0, visible: false)
       end
     end
+
+    scenario "Shows all investments and not only the ones on the current page", :js do
+      stub_const("#{Budgets::InvestmentsController}::PER_PAGE", 2)
+
+      3.times do
+        create(:map_location, investment: create(:budget_investment, heading: heading))
+      end
+
+      visit budget_investments_path(budget, heading_id: heading.id)
+
+      within("#budget-investments") do
+        expect(page).to have_css(".budget-investment", count: 2)
+      end
+
+      within(".map_location") do
+        expect(page).to have_css(".map-icon", count: 3, visible: false)
+      end
+    end
   end
 
 end