fix version links on roadmap (#8936)

* adds an id to the version link to be anchorable
* adds an achor element to the link in the version list
* whitelists the possible filter parameters for the version list links

The solution has an insufficiency in that the browser unnecessarily reloads the page if the user had
filtered before and then clicks on a link. I find that acceptable.

app/helpers/versions_helper.rb

@@ -41,6 +41,8 @@ module VersionsHelper
   def link_to_version(version, html_options = {}, options = {})
     return '' unless version&.is_a?(Version)
 
+    html_options = html_options.merge(id: link_to_version_id(version))
+
     link_name = options[:before_text].to_s.html_safe + format_version_name(version, options[:project] || @project)
     link_to_if version.visible?,
                link_name,
@@ -48,6 +50,10 @@ module VersionsHelper
                html_options
   end
 
+  def link_to_version_id(version)
+    ERB::Util.url_encode("version-#{version.name}")
+  end
+
   def format_version_name(version, project = @project)
     h(version.to_s_for_project(project))
   end

app/views/versions/_roadmap_filter.html.erb

@@ -21,7 +21,6 @@
 
     <% if @project.descendants.active.any? %>
       <div class="form--space"></div>
-      <%= hidden_field_tag 'with_subprojects', 0 %>
       <div class="form--field -trailing-label -no-margin">
         <%= styled_label_tag "with-subprojects", t(:label_subproject_plural) %>
 

app/views/versions/_roadmap_version_links.html.erb

@@ -1,5 +1,8 @@
 <h3><%= t(:label_version_plural) %></h3>
 
 <% @versions.each do |version| %>
-  <%= link_to format_version_name(version), "#{project_roadmap_url}##{version.name}" %><br />
+  <%= link_to format_version_name(version),
+              project_roadmap_path({ anchor: link_to_version_id(version) }.merge(params.permit(:completed,
+                                                                                               :with_subprojects,
+                                                                                               type_ids: []).to_h)) %><br />
 <% end %>

app/views/versions/index.html.erb

@@ -45,7 +45,7 @@ See docs/COPYRIGHT.rdoc for more details.
   <div id="roadmap">
     <% @versions.each do |version| %>
       <h3 class="version icon-context icon-modules">
-        <%= link_to_version version, name: h(version.name) %>
+        <%= link_to_version(version, name: h(version.name), id: "version-#{version.name}") %>
       </h3>
       <%= render partial: 'versions/overview', locals: {version: version} %>
       <%= render(partial: "wiki/content", locals: {content: version.wiki_page.content}) if version.wiki_page %>

spec/helpers/versions_helper_spec.rb

@@ -59,7 +59,8 @@ describe VersionsHelper, type: :helper do
     context 'a version' do
       context 'with being allowed to see the version' do
         it 'does not create a link, without permission' do
-          expect(link_to_version(version)).to eq("#{test_project.name} - #{version.name}")
+          expect(link_to_version(version))
+            .to eq("#{test_project.name} - #{version.name}")
         end
       end
 
@@ -71,21 +72,22 @@ describe VersionsHelper, type: :helper do
         end
 
         it 'generates a link' do
-          expect(link_to_version(version)).to eq("<a href=\"/versions/#{version.id}\">#{test_project.name} - #{version.name}</a>")
+          expect(link_to_version(version))
+            .to be_html_eql("<a href=\"/versions/#{version.id}\" id=\"version-#{ERB::Util.url_encode(version.name)}\">#{test_project.name} - #{version.name}</a>")
         end
 
         it 'generates a link within a project' do
           @project = test_project
-          expect(link_to_version(version)).to eq("<a href=\"/versions/#{version.id}\">#{version.name}</a>")
+          expect(link_to_version(version))
+            .to be_html_eql("<a href=\"/versions/#{version.id}\" id=\"version-#{ERB::Util.url_encode(version.name)}\">#{version.name}</a>")
         end
       end
     end
 
-    describe 'an invalid version' do
-      let(:version) { Object }
-
-      it 'does not generate a link' do
-        expect(link_to_version(Object)).to be_empty
+    describe '#link_to_version_id' do
+      it 'generates an escaped id' do
+        expect(link_to_version_id(version))
+          .to eql("version-#{ERB::Util.url_encode(version.name)}")
       end
     end
   end