Use "resolve" for polymorphic hierarchy paths

In the past, we couldn't use `polymorphic_path` in many places. For
instance, `polymorphic_path(budget, investment)` would return
`budget_budget_investment_path`, while in our routes we had defined
`budget_investment_path`.

With the `resolve` method, introduced in Rails 5.1, we can use symbols
to define we want it to use `investment` instead of `budget_investment`.
It also works with nested resources, so now we can write
`polymorphic_path(investment)`.

This makes the code for `resource_hierarchy_for` almost impossible to
understand. I reached this result after having a look at the internals
of the `resolve` method in order to get its results and then remove the
symbols we include.

Note using this method will not make admin routes compatible with
`polymorphic_path`. Quoting from the Rails documentation:

> This custom behavior only applies to simple polymorphic URLs where a
> single model instance is passed and not more complicated forms, e.g:
> [example showing admin routes won't work]

Also note that now the `admin_polymorphic_path` method will not work for
every model due to inconsistencies in our admin routes. For instance, we
define `groups` and `budget_investments`; we should either use the
`budget_` prefix in all places or remove it everywhere. Right now the
code only works for items with the prefix; it isn't a big deal because
we never call it with an item without the prefix.

Finally, for unknown reasons some routing tests fail if we use
`polymorphic_path`, so we need to redefine that method in those tests
and force the `only_path: true` option.

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
 

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

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)

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.
+# routes with nested resources in the admin namespace
-# HACK: is there a way to avoid monkey-patching here? Using helpers is
-# a similar use of a global namespace too...
 module ActionDispatch::Routing::UrlFor
   def resource_hierarchy_for(resource)
-    case resource.class.name
+    if polymorphic_mapping(resource)
-    when "Budget::Investment", "Budget::Phase", "Budget::Group"
+      resolve = polymorphic_mapping(resource).send(:eval_block, self, resource, {})
-      [resource.budget, resource]
+
-    when "Budget::Heading"
+      if resolve.last.is_a?(Hash)
-      [resource.group.budget, resource.group, resource]
+        [resolve.first, *resolve.last.values]
-    when "Milestone"
+      else
-      [*resource_hierarchy_for(resource.milestoneable), resource]
+        resolve
-    when "ProgressBar"
+      end
-      [*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]
     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

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

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"

config/routes/community.rb

@@ -1,3 +1,5 @@
 resources :communities, only: [:show] do
   resources :topics
 end
+
+resolve("Topic") { |topic, options| [topic.community, topic, options] }

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

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

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

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)

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"

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