From 69ae2d31ee82c3be2e1d2f5a83b857d579195e20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 2 May 2022 18:07:37 +0200 Subject: [PATCH 1/5] Extract component to render the budget map We're going to make a change, and it's easier if we've already got a component with tests so we don't have to write system tests to check whether the map is rendered. --- .../budgets/budget_component.html.erb | 8 +----- app/components/budgets/budget_component.rb | 16 +----------- app/components/budgets/map_component.html.erb | 4 +++ app/components/budgets/map_component.rb | 26 +++++++++++++++++++ spec/components/budgets/map_component_spec.rb | 23 ++++++++++++++++ spec/system/budgets/budgets_spec.rb | 6 +---- 6 files changed, 56 insertions(+), 27 deletions(-) create mode 100644 app/components/budgets/map_component.html.erb create mode 100644 app/components/budgets/map_component.rb create mode 100644 spec/components/budgets/map_component_spec.rb diff --git a/app/components/budgets/budget_component.html.erb b/app/components/budgets/budget_component.html.erb index 232d5da75..d0855c9db 100644 --- a/app/components/budgets/budget_component.html.erb +++ b/app/components/budgets/budget_component.html.erb @@ -33,13 +33,7 @@ <% end %> <%= render Budgets::SupportsInfoComponent.new(budget) %> - - <% unless budget.informing? %> -
-

<%= t("budgets.index.map") %>

- <%= render_map(nil, "budgets", false, nil, coordinates) %> -
- <% end %> + <%= render Budgets::MapComponent.new(budget) %> diff --git a/app/components/budgets/budget_component.rb b/app/components/budgets/budget_component.rb index 83536cfd6..e7ed09c47 100644 --- a/app/components/budgets/budget_component.rb +++ b/app/components/budgets/budget_component.rb @@ -1,22 +1,8 @@ class Budgets::BudgetComponent < ApplicationComponent - delegate :wysiwyg, :auto_link_already_sanitized_html, :render_map, to: :helpers + delegate :wysiwyg, :auto_link_already_sanitized_html, to: :helpers attr_reader :budget def initialize(budget) @budget = budget end - - private - - def coordinates - return unless budget.present? - - if budget.publishing_prices_or_later? && budget.investments.selected.any? - investments = budget.investments.selected - else - investments = budget.investments - end - - MapLocation.where(investment_id: investments).map(&:json_data) - end end diff --git a/app/components/budgets/map_component.html.erb b/app/components/budgets/map_component.html.erb new file mode 100644 index 000000000..733ce56af --- /dev/null +++ b/app/components/budgets/map_component.html.erb @@ -0,0 +1,4 @@ +
+

<%= t("budgets.index.map") %>

+ <%= render_map(nil, "budgets", false, nil, coordinates) %> +
diff --git a/app/components/budgets/map_component.rb b/app/components/budgets/map_component.rb new file mode 100644 index 000000000..be83fcce4 --- /dev/null +++ b/app/components/budgets/map_component.rb @@ -0,0 +1,26 @@ +class Budgets::MapComponent < ApplicationComponent + delegate :render_map, to: :helpers + attr_reader :budget + + def initialize(budget) + @budget = budget + end + + def render? + !budget.informing? + end + + private + + def coordinates + return unless budget.present? + + if budget.publishing_prices_or_later? && budget.investments.selected.any? + investments = budget.investments.selected + else + investments = budget.investments + end + + MapLocation.where(investment_id: investments).map(&:json_data) + end +end diff --git a/spec/components/budgets/map_component_spec.rb b/spec/components/budgets/map_component_spec.rb new file mode 100644 index 000000000..662507bb1 --- /dev/null +++ b/spec/components/budgets/map_component_spec.rb @@ -0,0 +1,23 @@ +require "rails_helper" + +describe Budgets::MapComponent do + let(:budget) { build(:budget) } + + describe "#render?" do + it "is rendered after the informing phase" do + budget.phase = "accepting" + + render_inline Budgets::MapComponent.new(budget) + + expect(page.first("div.map")).to have_content "located geographically" + end + + it "is not rendered during the informing phase" do + budget.phase = "informing" + + render_inline Budgets::MapComponent.new(budget) + + expect(page).not_to be_rendered + end + end +end diff --git a/spec/system/budgets/budgets_spec.rb b/spec/system/budgets/budgets_spec.rb index b0a8ce7f9..0ba94c64c 100644 --- a/spec/system/budgets/budgets_spec.rb +++ b/spec/system/budgets/budgets_spec.rb @@ -100,8 +100,6 @@ describe "Budgets" do expect(page).not_to have_link("List of all investment projects") expect(page).not_to have_link("List of all unfeasible investment projects") expect(page).not_to have_link("List of all investment projects not selected for balloting") - - expect(page).not_to have_css("div.map") end end @@ -114,8 +112,6 @@ describe "Budgets" do within("#budget_info") do expect(page).not_to have_link heading.name expect(page).to have_content "#{heading.name}\n€1,000,000" - - expect(page).to have_css("div.map") end end @@ -342,7 +338,7 @@ describe "Budgets" do } end - allow_any_instance_of(Budgets::BudgetComponent).to receive(:coordinates).and_return(coordinates) + allow_any_instance_of(Budgets::MapComponent).to receive(:coordinates).and_return(coordinates) visit budgets_path From cf7fe89dddc14690db6b72a2766c213add59d129 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 2 May 2022 18:10:05 +0200 Subject: [PATCH 2/5] Revove obsolete references in budget component These helpers aren't used here since commit 090f1bcdd. --- app/components/budgets/budget_component.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/components/budgets/budget_component.rb b/app/components/budgets/budget_component.rb index e7ed09c47..b11ebce85 100644 --- a/app/components/budgets/budget_component.rb +++ b/app/components/budgets/budget_component.rb @@ -1,5 +1,4 @@ class Budgets::BudgetComponent < ApplicationComponent - delegate :wysiwyg, :auto_link_already_sanitized_html, to: :helpers attr_reader :budget def initialize(budget) From e8b33ae25bd2cde76904b521f7706600dd1bba35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 2 May 2022 18:16:34 +0200 Subject: [PATCH 3/5] Remove obsolete expectations in budget index test We forgot to do so in commit 04605d5d5. Before that commit, we were testing the links weren't displayed in the "informing" phase as opposed to the "finished" phase. After that commit, they weren't displayed anywhere since a field with the links generated by the `Budgets::Investments::FiltersComponent`. We've already got tests for these links. --- spec/system/budgets/budgets_spec.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/spec/system/budgets/budgets_spec.rb b/spec/system/budgets/budgets_spec.rb index 0ba94c64c..d8db309f8 100644 --- a/spec/system/budgets/budgets_spec.rb +++ b/spec/system/budgets/budgets_spec.rb @@ -96,10 +96,6 @@ describe "Budgets" do within("#budget_info") do expect(page).not_to have_link heading.name expect(page).to have_content "#{heading.name}\n€1,000,000" - - expect(page).not_to have_link("List of all investment projects") - expect(page).not_to have_link("List of all unfeasible investment projects") - expect(page).not_to have_link("List of all investment projects not selected for balloting") end end From 8befe55ba173ec00fe9761b4d65daa93f99d1a6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 2 May 2022 20:31:50 +0200 Subject: [PATCH 4/5] Remove obsolete feature_maps? method It isn't used since commit c34aa5412. --- app/models/concerns/mappable.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/models/concerns/mappable.rb b/app/models/concerns/mappable.rb index 4b2dbbb53..356a38231 100644 --- a/app/models/concerns/mappable.rb +++ b/app/models/concerns/mappable.rb @@ -4,9 +4,5 @@ module Mappable included do has_one :map_location, dependent: :destroy accepts_nested_attributes_for :map_location, allow_destroy: true, reject_if: :all_blank - - def feature_maps? - Setting["feature.map"].present? - end end end From d5174032349a81f161d8a494a345c783d9f1c9a2 Mon Sep 17 00:00:00 2001 From: decabeza Date: Tue, 12 Apr 2022 18:37:04 +0200 Subject: [PATCH 5/5] Show budgets map only if feature is enabled --- app/components/budgets/map_component.rb | 2 +- spec/components/budgets/map_component_spec.rb | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/app/components/budgets/map_component.rb b/app/components/budgets/map_component.rb index be83fcce4..1da16c137 100644 --- a/app/components/budgets/map_component.rb +++ b/app/components/budgets/map_component.rb @@ -7,7 +7,7 @@ class Budgets::MapComponent < ApplicationComponent end def render? - !budget.informing? + feature?(:map) && !budget.informing? end private diff --git a/spec/components/budgets/map_component_spec.rb b/spec/components/budgets/map_component_spec.rb index 662507bb1..7d9d3ffb8 100644 --- a/spec/components/budgets/map_component_spec.rb +++ b/spec/components/budgets/map_component_spec.rb @@ -4,7 +4,8 @@ describe Budgets::MapComponent do let(:budget) { build(:budget) } describe "#render?" do - it "is rendered after the informing phase" do + it "is rendered after the informing phase when the map feature is enabled" do + Setting["feature.map"] = true budget.phase = "accepting" render_inline Budgets::MapComponent.new(budget) @@ -13,11 +14,21 @@ describe Budgets::MapComponent do end it "is not rendered during the informing phase" do + Setting["feature.map"] = true budget.phase = "informing" render_inline Budgets::MapComponent.new(budget) expect(page).not_to be_rendered end + + it "is not rendered when the map feature is disabled" do + Setting["feature.map"] = false + budget.phase = "accepting" + + render_inline Budgets::MapComponent.new(budget) + + expect(page).not_to be_rendered + end end end