Link to current and old element versions from changeset pages

diff --git a/app/helpers/browse_helper.rb b/app/helpers/browse_helper.rb
index 3c9d4d09239ca9eb9ee0f0bdeaa788e9686f5df9..7aa6e4754d9078344fde557e4f256b34ec2067e3 100644 (file)
--- a/app/helpers/browse_helper.rb
+++ b/app/helpers/browse_helper.rb
@@ -2,7 +2,7 @@ module BrowseHelper
   def element_single_current_link(type, object, url)
     link_to url, { :class => element_class(type, object), :title => element_title(object), :rel => (link_follow(object) if type == "node") } do
       element_strikethrough object do
-        printable_name object
+        printable_element_name object
       end
     end
   end
@@ -13,14 +13,13 @@ module BrowseHelper
     end
   end
 
-  def printable_name(object, version: false)
+  def printable_element_name(object)
     id = if object.id.is_a?(Array)
            object.id[0]
          else
            object.id
          end
-    name = t "printable_name.with_id", :id => id.to_s
-    name = t "printable_name.with_version", :id => name, :version => object.version.to_s if version
+    name = id.to_s
 
     # don't look at object tags if redacted, so as to avoid giving
     # away redacted version tag information.
@@ -41,6 +40,10 @@ module BrowseHelper
     name
   end
 
+  def printable_element_version(object)
+    t "printable_name.version", :version => object.version
+  end
+
   def element_strikethrough(object, &block)
     if object.redacted? || !object.visible?
       tag.s(&block)

diff --git a/app/views/browse/_containing_relation.html.erb b/app/views/browse/_containing_relation.html.erb
index 06220a97ddb9267b471e2aaea31a86273b0a5463..2547ff7aa6a7cbcae116c43300b68bfc3c30c045 100644 (file)
--- a/app/views/browse/_containing_relation.html.erb
+++ b/app/views/browse/_containing_relation.html.erb
@@ -1,4 +1,4 @@
-<li><%= linked_name = link_to printable_name(containing_relation.relation), :action => "relation", :id => containing_relation.relation.id.to_s
+<li><%= linked_name = link_to printable_element_name(containing_relation.relation), :action => "relation", :id => containing_relation.relation.id.to_s
         if containing_relation.member_role.blank?
           t ".entry_html", :relation_name => linked_name
         else

diff --git a/app/views/browse/_relation_member.html.erb b/app/views/browse/_relation_member.html.erb
index 56b8f154fd3e3b0431f56570359ca3350cb873b0..16842f56922c97d9bd03f25ee6aa0a3936ffb283 100644 (file)
--- a/app/views/browse/_relation_member.html.erb
+++ b/app/views/browse/_relation_member.html.erb
@@ -1,4 +1,4 @@
-<% linked_name = link_to printable_name(relation_member.member), { :controller => :browse, :action => relation_member.member_type.downcase, :id => relation_member.member_id.to_s }, { :rel => link_follow(relation_member.member) }
+<% linked_name = link_to printable_element_name(relation_member.member), { :controller => :browse, :action => relation_member.member_type.downcase, :id => relation_member.member_id.to_s }, { :rel => link_follow(relation_member.member) }
    type_str = t ".type.#{relation_member.member_type.downcase}" %>
 <%= element_list_item relation_member.member_type.downcase, relation_member.member do %>
   <%= if relation_member.member_role.blank?

diff --git a/app/views/browse/changeset.html.erb b/app/views/browse/changeset.html.erb
index d2ccff44d2222eed590414caa53e61ab17255b53..50171271763f3bd0b2bd125b078ef734716d6212 100644 (file)
--- a/app/views/browse/changeset.html.erb
+++ b/app/views/browse/changeset.html.erb
@@ -94,7 +94,9 @@
     <ul class="list-unstyled">
       <% @ways.each do |way| %>
         <%= element_list_item "way", way do %>
-          <%= link_to printable_name(way, :version => true), { :action => "way", :id => way.way_id.to_s } %>
+          <%= t "printable_name.current_and_old_links_html",
+                :current_link => link_to(printable_element_name(way), way_path(way.way_id)),
+                :old_link => link_to(printable_element_version(way), old_way_path(way.way_id, way.version)) %>
         <% end %>
       <% end %>
     </ul>
@@ -105,7 +107,9 @@
     <ul class="list-unstyled">
       <% @relations.each do |relation| %>
         <%= element_list_item "relation", relation do %>
-          <%= link_to printable_name(relation, :version => true), { :action => "relation", :id => relation.relation_id.to_s } %>
+          <%= t "printable_name.current_and_old_links_html",
+                :current_link => link_to(printable_element_name(relation), relation_path(relation.relation_id)),
+                :old_link => link_to(printable_element_version(relation), old_relation_path(relation.relation_id, relation.version)) %>
         <% end %>
       <% end %>
     </ul>
@@ -116,7 +120,9 @@
     <ul class="list-unstyled">
       <% @nodes.each do |node| %>
         <%= element_list_item "node", node do %>
-          <%= link_to printable_name(node, :version => true), { :action => "node", :id => node.node_id.to_s }, { :rel => link_follow(node) } %>
+          <%= t "printable_name.current_and_old_links_html",
+                :current_link => link_to(printable_element_name(node), node_path(node.node_id), { :rel => link_follow(node) }),
+                :old_link => link_to(printable_element_version(node), old_node_path(node.node_id, node.version), { :rel => link_follow(node) }) %>
         <% end %>
       <% end %>
     </ul>

diff --git a/app/views/browse/feature.html.erb b/app/views/browse/feature.html.erb
index 37be312b1e86b0d38a9a3489389456bc1367a85c..cd333d3bfd1dbbcd1e301158274b40dfb7de3844 100644 (file)
--- a/app/views/browse/feature.html.erb
+++ b/app/views/browse/feature.html.erb
@@ -1,6 +1,6 @@
-<% set_title(t("browse.#{@type}.title_html", :name => printable_name(@feature))) %>
+<% set_title(t("browse.#{@type}.title_html", :name => printable_element_name(@feature))) %>
 
-<%= render "sidebar_header", :title => t("browse.#{@type}.title_html", :name => printable_name(@feature)) %>
+<%= render "sidebar_header", :title => t("browse.#{@type}.title_html", :name => printable_element_name(@feature)) %>
 
 <%= render :partial => @type, :object => @feature %>
 

diff --git a/app/views/browse/history.html.erb b/app/views/browse/history.html.erb
index fd1f5ffc5e1781c0dea5d671fe112cb01049bd19..bf11e8f1e20dec49bc014934701d99a82328af88 100644 (file)
--- a/app/views/browse/history.html.erb
+++ b/app/views/browse/history.html.erb
@@ -1,6 +1,6 @@
-<% set_title(t("browse.#{@type}.history_title_html", :name => printable_name(@feature))) %>
+<% set_title(t("browse.#{@type}.history_title_html", :name => printable_element_name(@feature))) %>
 
-<%= render "sidebar_header", :title => t("browse.#{@type}.history_title_html", :name => printable_name(@feature)) %>
+<%= render "sidebar_header", :title => t("browse.#{@type}.history_title_html", :name => printable_element_name(@feature)) %>
 
 <%= render :partial => @type, :collection => @feature.send(:"old_#{@type}s").reverse %>
 

diff --git a/app/views/old_nodes/show.html.erb b/app/views/old_nodes/show.html.erb
index 9220b2cc20ec03397707509a8d12d74220af0023..90be6b7c9007ad17536b615fe3f55d664f5c5068 100644 (file)
--- a/app/views/old_nodes/show.html.erb
+++ b/app/views/old_nodes/show.html.erb
@@ -1,6 +1,6 @@
-<% set_title t("browse.node.title_html", :name => printable_name(@feature)) %>
+<% set_title t("browse.node.title_html", :name => printable_element_name(@feature)) %>
 
-<%= render "sidebar_header", :title => t("browse.node.title_html", :name => printable_name(@feature)) %>
+<%= render "sidebar_header", :title => t("browse.node.title_html", :name => printable_element_name(@feature)) %>
 
 <%= render :partial => "browse/node", :object => @feature %>
 

diff --git a/app/views/old_relations/show.html.erb b/app/views/old_relations/show.html.erb
index b049a4cf0aebde2c54b51f607655af5112519190..42e80e6512ac084fe1877e7034ede0a0af3c5cb6 100644 (file)
--- a/app/views/old_relations/show.html.erb
+++ b/app/views/old_relations/show.html.erb
@@ -1,6 +1,6 @@
-<% set_title t("browse.relation.title_html", :name => printable_name(@feature)) %>
+<% set_title t("browse.relation.title_html", :name => printable_element_name(@feature)) %>
 
-<%= render "sidebar_header", :title => t("browse.relation.title_html", :name => printable_name(@feature)) %>
+<%= render "sidebar_header", :title => t("browse.relation.title_html", :name => printable_element_name(@feature)) %>
 
 <%= render :partial => "browse/relation", :object => @feature %>
 

diff --git a/app/views/old_ways/show.html.erb b/app/views/old_ways/show.html.erb
index d128cd2ad3665abbfbbcf4b4acd5bb9f9df05e5a..b16c6b291cdce8ea1dd828b4c96ddc9c7eedcff6 100644 (file)
--- a/app/views/old_ways/show.html.erb
+++ b/app/views/old_ways/show.html.erb
@@ -1,6 +1,6 @@
-<% set_title t("browse.way.title_html", :name => printable_name(@feature)) %>
+<% set_title t("browse.way.title_html", :name => printable_element_name(@feature)) %>
 
-<%= render "sidebar_header", :title => t("browse.way.title_html", :name => printable_name(@feature)) %>
+<%= render "sidebar_header", :title => t("browse.way.title_html", :name => printable_element_name(@feature)) %>
 
 <%= render :partial => "browse/way", :object => @feature %>
 

diff --git a/config/locales/en.yml b/config/locales/en.yml
index d664826393916b840376a283ea6e157c9aa6bc95..633e8b1eda563be1c342343ad363c345e5192d9b 100644 (file)
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -204,9 +204,9 @@ en:
         one: "%{count} year ago"
         other: "%{count} years ago"
   printable_name:
-    with_id: "%{id}"
-    with_version: "%{id}, v%{version}"
+    version: "v%{version}"
     with_name_html: "%{name} (%{id})"
+    current_and_old_links_html: "%{current_link}, %{old_link}"
   editor:
     default: "Default (currently %{name})"
     id:

diff --git a/test/controllers/browse_controller_test.rb b/test/controllers/browse_controller_test.rb
index 7df246c5781c65e609737be9f151c9eff7a4e875..1023d76ae58713b521e6d1353c3dee11d630b213 100644 (file)
--- a/test/controllers/browse_controller_test.rb
+++ b/test/controllers/browse_controller_test.rb
@@ -156,6 +156,15 @@ class BrowseControllerTest < ActionDispatch::IntegrationTest
     assert_select "div.changeset-comments ul li", :count => 4
   end
 
+  def test_read_changeset_element_links
+    changeset = create(:changeset)
+    node = create(:node, :with_history, :changeset => changeset)
+
+    browse_check :changeset_path, changeset.id, "browse/changeset"
+    assert_dom "a[href='#{node_path node}']", :count => 1
+    assert_dom "a[href='#{old_node_path node, 1}']", :count => 1
+  end
+
   ##
   #  Methods to check redaction.
   #

diff --git a/test/helpers/browse_helper_test.rb b/test/helpers/browse_helper_test.rb
index 06d5f3d531c5e00b3fa12129f9b8e48b8c3e0ee1..4cf0ab69ad3aab53863c4c2872ffda8a45a03073 100644 (file)
--- a/test/helpers/browse_helper_test.rb
+++ b/test/helpers/browse_helper_test.rb
@@ -4,7 +4,7 @@ class BrowseHelperTest < ActionView::TestCase
   include ERB::Util
   include ApplicationHelper
 
-  def test_printable_name
+  def test_printable_element_name
     node = create(:node, :with_history, :version => 2)
     node_v1 = node.old_nodes.find_by(:version => 1)
     node_v2 = node.old_nodes.find_by(:version => 2)
@@ -19,42 +19,34 @@ class BrowseHelperTest < ActionView::TestCase
 
     deleted_node = create(:node, :deleted)
 
-    assert_dom_equal deleted_node.id.to_s, printable_name(deleted_node)
-    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node)
-    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node_v2)
-    assert_dom_equal node.id.to_s, printable_name(node_v1)
-    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}, v2</bdi>)", printable_name(node_v2, :version => true)
-    assert_dom_equal "#{node.id}, v1", printable_name(node_v1, :version => true)
-    assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>#{node_with_ref_without_name.id}</bdi>)", printable_name(node_with_ref_without_name)
+    assert_dom_equal deleted_node.id.to_s, printable_element_name(deleted_node)
+    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}</bdi>)", printable_element_name(node)
+    assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}</bdi>)", printable_element_name(node_v2)
+    assert_dom_equal node.id.to_s, printable_element_name(node_v1)
+    assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>#{node_with_ref_without_name.id}</bdi>)", printable_element_name(node_with_ref_without_name)
 
     I18n.with_locale "pt" do
-      assert_dom_equal deleted_node.id.to_s, printable_name(deleted_node)
-      assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node)
-      assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node_v2)
-      assert_dom_equal node.id.to_s, printable_name(node_v1)
-      assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}, v2</bdi>)", printable_name(node_v2, :version => true)
-      assert_dom_equal "#{node.id}, v1", printable_name(node_v1, :version => true)
-      assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>#{node_with_ref_without_name.id}</bdi>)", printable_name(node_with_ref_without_name)
+      assert_dom_equal deleted_node.id.to_s, printable_element_name(deleted_node)
+      assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}</bdi>)", printable_element_name(node)
+      assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}</bdi>)", printable_element_name(node_v2)
+      assert_dom_equal node.id.to_s, printable_element_name(node_v1)
+      assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>#{node_with_ref_without_name.id}</bdi>)", printable_element_name(node_with_ref_without_name)
     end
 
     I18n.with_locale "pt-BR" do
-      assert_dom_equal deleted_node.id.to_s, printable_name(deleted_node)
-      assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node)
-      assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node_v2)
-      assert_dom_equal node.id.to_s, printable_name(node_v1)
-      assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}, v2</bdi>)", printable_name(node_v2, :version => true)
-      assert_dom_equal "#{node.id}, v1", printable_name(node_v1, :version => true)
-      assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>#{node_with_ref_without_name.id}</bdi>)", printable_name(node_with_ref_without_name)
+      assert_dom_equal deleted_node.id.to_s, printable_element_name(deleted_node)
+      assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}</bdi>)", printable_element_name(node)
+      assert_dom_equal "<bdi>Nó teste</bdi> (<bdi>#{node.id}</bdi>)", printable_element_name(node_v2)
+      assert_dom_equal node.id.to_s, printable_element_name(node_v1)
+      assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>#{node_with_ref_without_name.id}</bdi>)", printable_element_name(node_with_ref_without_name)
     end
 
     I18n.with_locale "de" do
-      assert_dom_equal deleted_node.id.to_s, printable_name(deleted_node)
-      assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node)
-      assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}</bdi>)", printable_name(node_v2)
-      assert_dom_equal node.id.to_s, printable_name(node_v1)
-      assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}, v2</bdi>)", printable_name(node_v2, :version => true)
-      assert_dom_equal "#{node.id}, v1", printable_name(node_v1, :version => true)
-      assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>#{node_with_ref_without_name.id}</bdi>)", printable_name(node_with_ref_without_name)
+      assert_dom_equal deleted_node.id.to_s, printable_element_name(deleted_node)
+      assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}</bdi>)", printable_element_name(node)
+      assert_dom_equal "<bdi>Test Node</bdi> (<bdi>#{node.id}</bdi>)", printable_element_name(node_v2)
+      assert_dom_equal node.id.to_s, printable_element_name(node_v1)
+      assert_dom_equal "<bdi>3.1415926</bdi> (<bdi>#{node_with_ref_without_name.id}</bdi>)", printable_element_name(node_with_ref_without_name)
     end
   end