From 450b157a5e2ef993b2f1373f01b4656df6a18deb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sen=C3=A9n=20Rodero=20Rodr=C3=ADguez?= Date: Sat, 30 Jan 2021 14:04:55 +0100 Subject: [PATCH 1/3] Fix tag links on legislation proposals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On commit 1a902a96 we removed this helper to make use of polymorphic routes but when it's called for Legislation::Proposal fails as the namespace does not match the model namespace. Now we recover the removed helper but only the parts that do not work with polymorphic_url helper. Co-Authored-By: Javi Martín --- app/helpers/tags_helper.rb | 10 ++++++++++ app/views/shared/_tags.html.erb | 2 +- spec/system/legislation/proposals_spec.rb | 23 +++++++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 app/helpers/tags_helper.rb diff --git a/app/helpers/tags_helper.rb b/app/helpers/tags_helper.rb new file mode 100644 index 000000000..464a155d0 --- /dev/null +++ b/app/helpers/tags_helper.rb @@ -0,0 +1,10 @@ +module TagsHelper + def taggables_path(taggable, tag_name) + case taggable.class.name + when "Legislation::Proposal" + legislation_process_proposals_path(taggable.process, search: tag_name) + else + polymorphic_path(taggable.class, search: tag_name) + end + end +end diff --git a/app/views/shared/_tags.html.erb b/app/views/shared/_tags.html.erb index 103455017..f4ac1773e 100644 --- a/app/views/shared/_tags.html.erb +++ b/app/views/shared/_tags.html.erb @@ -7,7 +7,7 @@ <% taggable.tag_list_with_limit(limit).each do |tag| %>
  • <%= link_to sanitize(tag.name), - polymorphic_path(taggable.class, search: tag.name) %>
    • + taggables_path(taggable, tag.name) %> <% end %> <% if taggable.tags_count_out_of_limit(limit) > 0 %> diff --git a/spec/system/legislation/proposals_spec.rb b/spec/system/legislation/proposals_spec.rb index db773cf69..22c83211d 100644 --- a/spec/system/legislation/proposals_spec.rb +++ b/spec/system/legislation/proposals_spec.rb @@ -210,4 +210,27 @@ describe "Legislation Proposals" do expect(page).to have_link(process.title) end end + + scenario "Shows proposal tags as proposals filter", :js do + create(:legislation_proposal, process: process, tag_list: "Culture", title: "Open concert") + create(:legislation_proposal, process: process, tag_list: "Sports", title: "Baseball field") + + visit legislation_process_proposals_path(process) + + expect(page).to have_content "Open concert" + expect(page).to have_content "Baseball field" + + click_link "Culture" + + expect(page).not_to have_content "Baseball field" + expect(page).to have_content "Open concert" + end + + scenario "Show proposal tags on show", :js do + proposal = create(:legislation_proposal, process: process, tag_list: "Culture") + + visit legislation_process_proposal_path(proposal.process, proposal) + + expect(page).to have_link("Culture") + end end From 046e2273c7b4deb3fa8f8a4956377a3bc1689a97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sen=C3=A9n=20Rodero=20Rodr=C3=ADguez?= Date: Sat, 30 Jan 2021 21:24:56 +0100 Subject: [PATCH 2/3] Add tests for SDG::ProcessEnabled model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We were testing it through other models, but unit tests help when changing the code of this class. Co-Authored-By: Javi Martín --- spec/models/sdg/process_enabled_spec.rb | 69 +++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 spec/models/sdg/process_enabled_spec.rb diff --git a/spec/models/sdg/process_enabled_spec.rb b/spec/models/sdg/process_enabled_spec.rb new file mode 100644 index 000000000..9463e9ba8 --- /dev/null +++ b/spec/models/sdg/process_enabled_spec.rb @@ -0,0 +1,69 @@ +require "rails_helper" + +describe SDG::ProcessEnabled do + describe "#enabled?" do + context "SDG feature and namespace are enabled" do + before do + Setting["feature.sdg"] = true + %w[debates proposals polls budgets legislation].each do |process_name| + Setting["sdg.process.#{process_name}"] = true + end + end + + it "returns true with relatable records" do + SDG::Related::RELATABLE_TYPES.each do |relatable_type| + process = SDG::ProcessEnabled.new(relatable_type.constantize.new) + + expect(process).to be_enabled + end + end + + it "returns true with relatable class names" do + SDG::Related::RELATABLE_TYPES.each do |relatable_type| + process = SDG::ProcessEnabled.new(relatable_type) + + expect(process).to be_enabled + end + end + + it "returns true with relatable controller paths" do + process = SDG::ProcessEnabled.new("budgets/investments") + + expect(process).to be_enabled + end + end + + context "SDG feature is disabled" do + before do + Setting["feature.sdg"] = false + Setting["sdg.process.debates"] = true + end + + it "returns false" do + expect(SDG::ProcessEnabled.new(build(:debate))).not_to be_enabled + expect(SDG::ProcessEnabled.new("Debate")).not_to be_enabled + expect(SDG::ProcessEnabled.new("debates")).not_to be_enabled + end + end + + context "Some SDG processes are disabled" do + before do + Setting["feature.sdg"] = true + Setting["sdg.process.debates"] = true + Setting["sdg.process.proposals"] = false + end + + it "returns false for disabled processes" do + expect(SDG::ProcessEnabled.new(build(:proposal))).not_to be_enabled + expect(SDG::ProcessEnabled.new("Proposal")).not_to be_enabled + expect(SDG::ProcessEnabled.new("proposals")).not_to be_enabled + end + + it "returns true for enabled processes" do + expect(SDG::ProcessEnabled.new(build(:debate))).to be_enabled + expect(SDG::ProcessEnabled.new("Debate")).to be_enabled + expect(SDG::ProcessEnabled.new("debates")).to be_enabled + end + end + end +end From fb611d91cadb9b728244b4461f7226e2c954de3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sen=C3=A9n=20Rodero=20Rodr=C3=ADguez?= Date: Sun, 31 Jan 2021 13:43:22 +0100 Subject: [PATCH 3/3] Fix legislation proposal crash with SDG enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now we check the given record or name is a relatable instance or class to avoid trying to render goals for records which don't have a goals association. Note for now we are ignoring the case where we pass a controller_path for an unsupported class (for example, `legislation/proposals` or `budgets/headings`) because we never use it. We might need to revisit this case in the future. Co-Authored-By: Javi Martín --- app/models/sdg/process_enabled.rb | 8 +++++++- spec/models/sdg/process_enabled_spec.rb | 6 ++++++ spec/system/legislation/proposals_spec.rb | 5 ++++- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/models/sdg/process_enabled.rb b/app/models/sdg/process_enabled.rb index 67a17902a..29b567bcf 100644 --- a/app/models/sdg/process_enabled.rb +++ b/app/models/sdg/process_enabled.rb @@ -7,7 +7,7 @@ class SDG::ProcessEnabled end def enabled? - feature?("sdg") && feature?(process_name) && setting["sdg.process.#{process_name}"] + feature?("sdg") && feature?(process_name) && setting["sdg.process.#{process_name}"] && relatable? end def name @@ -39,4 +39,10 @@ class SDG::ProcessEnabled def module_name name.split("::").first end + + def relatable? + return true if controller_path_name? + + (SDG::Related::RELATABLE_TYPES & [record_or_name.class.name, record_or_name]).any? + end end diff --git a/spec/models/sdg/process_enabled_spec.rb b/spec/models/sdg/process_enabled_spec.rb index 9463e9ba8..5094b1d42 100644 --- a/spec/models/sdg/process_enabled_spec.rb +++ b/spec/models/sdg/process_enabled_spec.rb @@ -31,6 +31,12 @@ describe SDG::ProcessEnabled do expect(process).to be_enabled end + + it "returns false when record or name are not a relatable type" do + expect(SDG::ProcessEnabled.new(build(:legislation_proposal))).not_to be_enabled + expect(SDG::ProcessEnabled.new("Legislation::Proposal")).not_to be_enabled + expect(SDG::ProcessEnabled.new("officing/booth")).not_to be_enabled + end end context "SDG feature is disabled" do diff --git a/spec/system/legislation/proposals_spec.rb b/spec/system/legislation/proposals_spec.rb index 22c83211d..50162f2ae 100644 --- a/spec/system/legislation/proposals_spec.rb +++ b/spec/system/legislation/proposals_spec.rb @@ -226,7 +226,10 @@ describe "Legislation Proposals" do expect(page).to have_content "Open concert" end - scenario "Show proposal tags on show", :js do + scenario "Show proposal tags on show when SDG is enabled", :js do + Setting["feature.sdg"] = true + Setting["sdg.process.legislation"] = true + proposal = create(:legislation_proposal, process: process, tag_list: "Culture") visit legislation_process_proposal_path(proposal.process, proposal)