diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 7bb5b126a..0cb2a5b44 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -102,7 +102,7 @@ class CommentsController < ApplicationController return if current_user.administrator? || current_user.moderator? if @commentable.respond_to?(:comments_closed?) && @commentable.comments_closed? - redirect_to polymorphic_hierarchy_path(@commentable), alert: t("comments.comments_closed") + redirect_to polymorphic_path(@commentable), alert: t("comments.comments_closed") end end diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index f01856408..8d5e2fe4f 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -39,7 +39,7 @@ class NotificationsController < ApplicationController if notification.linkable_resource.is_a?(AdminNotification) notification.linkable_resource.link || notifications_path else - polymorphic_hierarchy_path(notification.linkable_resource) + polymorphic_path(notification.linkable_resource) end end end diff --git a/app/helpers/comments_helper.rb b/app/helpers/comments_helper.rb index 0516aee07..bb9015ea1 100644 --- a/app/helpers/comments_helper.rb +++ b/app/helpers/comments_helper.rb @@ -40,7 +40,7 @@ module CommentsHelper end def commentable_path(comment) - polymorphic_hierarchy_path(comment.commentable) + polymorphic_path(comment.commentable) end def user_level_class(comment) diff --git a/config/initializers/routes_hierarchy.rb b/config/initializers/routes_hierarchy.rb index ae86c07a5..8b9a07acb 100644 --- a/config/initializers/routes_hierarchy.rb +++ b/config/initializers/routes_hierarchy.rb @@ -1,69 +1,20 @@ # This module is expanded in order to make it easier to use polymorphic -# routes with nested resources. -# HACK: is there a way to avoid monkey-patching here? Using helpers is -# a similar use of a global namespace too... +# routes with nested resources in the admin namespace module ActionDispatch::Routing::UrlFor def resource_hierarchy_for(resource) - case resource.class.name - when "Budget::Investment", "Budget::Phase", "Budget::Group" - [resource.budget, resource] - when "Budget::Heading" - [resource.group.budget, resource.group, resource] - when "Milestone" - [*resource_hierarchy_for(resource.milestoneable), resource] - when "ProgressBar" - [*resource_hierarchy_for(resource.progressable), resource] - when "Audit" - [*resource_hierarchy_for(resource.associated || resource.auditable), resource] - when "Legislation::Annotation" - [resource.draft_version.process, resource.draft_version, resource] - when "Legislation::Proposal", "Legislation::Question", "Legislation::DraftVersion" - [resource.process, resource] - when "Topic" - [resource.community, resource] + if polymorphic_mapping(resource) + resolve = polymorphic_mapping(resource).send(:eval_block, self, resource, {}) + + if resolve.last.is_a?(Hash) + [resolve.first, *resolve.last.values] + else + resolve + end else resource end end - def polymorphic_hierarchy_path(resource) - # Unfortunately, we can't use polymorphic routes because there - # are cases where polymorphic_path doesn't get the named routes properly. - # Example: - # - # polymorphic_path([legislation_proposal.process, legislation_proposal]) - # - # That line tries to find legislation_process_legislation_proposal_path - # while the correct route would be legislation_process_proposal_path - # - # We probably need to define routes differently in order to be able to use - # polymorphic_path which might be possible with Rails 5.1 `direct` and - # `resolve` methods. - - resources = resource_hierarchy_for(resource) - - case resource.class.name - when "Budget::Investment" - # polymorphic_path would return budget_budget_investment_path - budget_investment_path(*resources) - when "Legislation::Annotation" - # polymorphic_path would return: - # "legislation_process_legislation_draft_version_legislation_annotation_path" - legislation_process_draft_version_annotation_path(*resources) - when "Legislation::Proposal" - # polymorphic_path would return legislation_process_legislation_proposal_path - legislation_process_proposal_path(*resources) - when "Legislation::Question" - # polymorphic_path would return legislation_process_legislation_question_path - legislation_process_question_path(*resources) - when "Poll::Question" - # polymorphic_path would return poll_question_path - question_path(*resources) - else - polymorphic_path(resources) - end - end - def admin_polymorphic_path(resource, options = {}) polymorphic_path([:admin, *resource_hierarchy_for(resource)], options) end diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 7bfc22484..f8f1c8f13 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -253,3 +253,15 @@ namespace :admin do resources :imports, only: [:new, :create, :show] end end + +resolve "Milestone" do |milestone| + [*resource_hierarchy_for(milestone.milestoneable), milestone] +end + +resolve "ProgressBar" do |progress_bar| + [*resource_hierarchy_for(progress_bar.progressable), progress_bar] +end + +resolve "Audit" do |audit| + [*resource_hierarchy_for(audit.associated || audit.auditable), audit] +end diff --git a/config/routes/budget.rb b/config/routes/budget.rb index f0742a99f..bed9b7324 100644 --- a/config/routes/budget.rb +++ b/config/routes/budget.rb @@ -19,5 +19,9 @@ resources :budgets, only: [:show, :index] do resource :executions, only: :show, controller: "budgets/executions" end +resolve "Budget::Investment" do |investment, options| + [investment.budget, :investment, options.merge(id: investment)] +end + get "investments/:id/json_data", action: :json_data, controller: "budgets/investments" get "/budgets/:budget_id/investments/:id/json_data", action: :json_data, controller: "budgets/investments" diff --git a/config/routes/community.rb b/config/routes/community.rb index 9e010ffb7..ca41ee2d1 100644 --- a/config/routes/community.rb +++ b/config/routes/community.rb @@ -1,3 +1,5 @@ resources :communities, only: [:show] do resources :topics end + +resolve("Topic") { |topic, options| [topic.community, topic, options] } diff --git a/config/routes/legislation.rb b/config/routes/legislation.rb index d5054e2d4..2458a2bae 100644 --- a/config/routes/legislation.rb +++ b/config/routes/legislation.rb @@ -36,3 +36,16 @@ namespace :legislation do end end end + +resolve "Legislation::Proposal" do |proposal, options| + [proposal.process, :proposal, options.merge(id: proposal)] +end + +resolve "Legislation::Question" do |question, options| + [question.process, :question, options.merge(id: question)] +end + +resolve "Legislation::Annotation" do |annotation, options| + [annotation.draft_version.process, :draft_version, :annotation, + options.merge(draft_version_id: annotation.draft_version, id: annotation)] +end diff --git a/config/routes/poll.rb b/config/routes/poll.rb index 96c43f856..5b3c03c7a 100644 --- a/config/routes/poll.rb +++ b/config/routes/poll.rb @@ -8,3 +8,7 @@ resources :polls, only: [:show, :index] do post :answer, on: :member end end + +resolve "Poll::Question" do |question, options| + [:question, options.merge(id: question)] +end diff --git a/spec/routing/polymorphic_routes_spec.rb b/spec/routing/polymorphic_routes_spec.rb new file mode 100644 index 000000000..abcda9bb3 --- /dev/null +++ b/spec/routing/polymorphic_routes_spec.rb @@ -0,0 +1,114 @@ +require "rails_helper" + +describe "Polymorphic routes" do + describe "polymorphic_path" do + it "routes investments" do + budget = create(:budget) + investment = create(:budget_investment, budget: budget) + + expect(polymorphic_path(investment)).to eq budget_investment_path(budget, investment) + end + + it "routes legislation proposals" do + process = create(:legislation_process) + proposal = create(:legislation_proposal, process: process) + + expect(polymorphic_path(proposal)).to eq legislation_process_proposal_path(process, proposal) + end + + it "routes legislation questions" do + process = create(:legislation_process) + question = create(:legislation_question, process: process) + + expect(polymorphic_path(question)).to eq legislation_process_question_path(process, question) + end + + it "routes legislation annotations" do + process = create(:legislation_process) + draft_version = create(:legislation_draft_version, process: process) + annotation = create(:legislation_annotation, draft_version: draft_version) + + expect(polymorphic_path(annotation)).to eq( + legislation_process_draft_version_annotation_path(process, draft_version, annotation) + ) + end + + it "routes poll questions" do + question = create(:poll_question) + + expect(polymorphic_path(question)).to eq question_path(question) + end + + it "routes topics" do + community = create(:proposal).community + topic = create(:topic, community: community) + + expect(polymorphic_path(topic)).to eq community_topic_path(community, topic) + end + end + + describe "admin_polymorphic_path" do + include ActionDispatch::Routing::UrlFor + + it "routes milestones for resources with no hierarchy" do + proposal = create(:proposal) + milestone = create(:milestone, milestoneable: proposal) + + expect(admin_polymorphic_path(milestone)).to eq( + admin_proposal_milestone_path(proposal, milestone) + ) + end + + it "routes milestones for resources with hierarchy" do + budget = create(:budget) + investment = create(:budget_investment, budget: budget) + milestone = create(:milestone, milestoneable: investment) + + expect(admin_polymorphic_path(milestone)).to eq( + admin_budget_budget_investment_milestone_path(budget, investment, milestone) + ) + end + + it "routes progress bars for resources with no hierarchy" do + proposal = create(:proposal) + progress_bar = create(:progress_bar, progressable: proposal) + + expect(admin_polymorphic_path(progress_bar)).to eq( + admin_proposal_progress_bar_path(proposal, progress_bar) + ) + end + + it "routes progress_bars for resources with hierarchy" do + budget = create(:budget) + investment = create(:budget_investment, budget: budget) + progress_bar = create(:progress_bar, progressable: investment) + + expect(admin_polymorphic_path(progress_bar)).to eq( + admin_budget_budget_investment_progress_bar_path(budget, investment, progress_bar) + ) + end + + it "routes audits" do + budget = create(:budget) + investment = create(:budget_investment, budget: budget) + audit = investment.audits.create! + + expect(admin_polymorphic_path(audit)).to eq( + admin_budget_budget_investment_audit_path(budget, investment, audit) + ) + end + + it "supports routes for actions like edit" do + proposal = create(:proposal) + milestone = create(:milestone, milestoneable: proposal) + + expect(admin_polymorphic_path(milestone, action: :edit)).to eq( + edit_admin_proposal_milestone_path(proposal, milestone) + ) + end + end +end + +def polymorphic_path(record, options = {}) + super(record, options.merge(only_path: true)) +end diff --git a/spec/support/common_actions/notifications.rb b/spec/support/common_actions/notifications.rb index 569010555..4db1cc833 100644 --- a/spec/support/common_actions/notifications.rb +++ b/spec/support/common_actions/notifications.rb @@ -50,7 +50,7 @@ module Notifications end def path_for(resource) - polymorphic_hierarchy_path(resource) + polymorphic_path(resource) end def error_message(resource_model = nil) diff --git a/spec/system/admin/translatable_spec.rb b/spec/system/admin/translatable_spec.rb index 4ef2a299b..3fdf1ae0a 100644 --- a/spec/system/admin/translatable_spec.rb +++ b/spec/system/admin/translatable_spec.rb @@ -17,7 +17,7 @@ describe "Admin edit translatable records" do context "Add a translation", :js do context "Input fields" do let(:translatable) { create(:budget_heading) } - let(:path) { edit_admin_budget_group_heading_path(*resource_hierarchy_for(translatable)) } + let(:path) { edit_admin_budget_group_heading_path(translatable.budget, translatable.group, translatable) } scenario "Maintains existing translations" do visit path @@ -356,7 +356,7 @@ describe "Admin edit translatable records" do let(:translatable) { create(:milestone) } scenario "Shows an error message" do - visit edit_admin_budget_budget_investment_milestone_path(*resource_hierarchy_for(translatable)) + visit admin_polymorphic_path(translatable, action: :edit) click_link "Remove language" click_link "Remove language" diff --git a/spec/system/xss_spec.rb b/spec/system/xss_spec.rb index 172044797..894eec1fb 100644 --- a/spec/system/xss_spec.rb +++ b/spec/system/xss_spec.rb @@ -116,7 +116,7 @@ describe "Cross-Site Scripting protection", :js do annotation = create(:legislation_annotation) annotation.update_column(:context, attack_code) - visit polymorphic_hierarchy_path(annotation) + visit polymorphic_path(annotation) expect(page.text).not_to be_empty end