From 7c522a4e024a8763125251f6df6bbcbcc0f0f1a1 Mon Sep 17 00:00:00 2001 From: Anton Khorev Date: Mon, 19 Feb 2024 04:34:41 +0300 Subject: [PATCH 1/1] Link to current and old element versions from changeset pages --- app/helpers/browse_helper.rb | 11 ++-- .../browse/_containing_relation.html.erb | 2 +- app/views/browse/_relation_member.html.erb | 2 +- app/views/browse/changeset.html.erb | 12 +++-- app/views/browse/feature.html.erb | 4 +- app/views/browse/history.html.erb | 4 +- app/views/old_nodes/show.html.erb | 4 +- app/views/old_relations/show.html.erb | 4 +- app/views/old_ways/show.html.erb | 4 +- config/locales/en.yml | 4 +- test/controllers/browse_controller_test.rb | 9 ++++ test/helpers/browse_helper_test.rb | 50 ++++++++----------- 12 files changed, 60 insertions(+), 50 deletions(-) diff --git a/app/helpers/browse_helper.rb b/app/helpers/browse_helper.rb index 3c9d4d092..7aa6e4754 100644 --- 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 06220a97d..2547ff7aa 100644 --- a/app/views/browse/_containing_relation.html.erb +++ b/app/views/browse/_containing_relation.html.erb @@ -1,4 +1,4 @@ -
  • <%= linked_name = link_to printable_name(containing_relation.relation), :action => "relation", :id => containing_relation.relation.id.to_s +
  • <%= 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 56b8f154f..16842f569 100644 --- 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 d2ccff44d..501712717 100644 --- a/app/views/browse/changeset.html.erb +++ b/app/views/browse/changeset.html.erb @@ -94,7 +94,9 @@ @@ -105,7 +107,9 @@ @@ -116,7 +120,9 @@ diff --git a/app/views/browse/feature.html.erb b/app/views/browse/feature.html.erb index 37be312b1..cd333d3bf 100644 --- 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 fd1f5ffc5..bf11e8f1e 100644 --- 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 9220b2cc2..90be6b7c9 100644 --- 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 b049a4cf0..42e80e651 100644 --- 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 d128cd2ad..b16c6b291 100644 --- 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 d66482639..633e8b1ed 100644 --- 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 7df246c57..1023d76ae 100644 --- 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 06d5f3d53..4cf0ab69a 100644 --- 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 "Test Node (#{node.id})", printable_name(node) - assert_dom_equal "Test Node (#{node.id})", printable_name(node_v2) - assert_dom_equal node.id.to_s, printable_name(node_v1) - assert_dom_equal "Test Node (#{node.id}, v2)", printable_name(node_v2, :version => true) - assert_dom_equal "#{node.id}, v1", printable_name(node_v1, :version => true) - assert_dom_equal "3.1415926 (#{node_with_ref_without_name.id})", printable_name(node_with_ref_without_name) + assert_dom_equal deleted_node.id.to_s, printable_element_name(deleted_node) + assert_dom_equal "Test Node (#{node.id})", printable_element_name(node) + assert_dom_equal "Test Node (#{node.id})", printable_element_name(node_v2) + assert_dom_equal node.id.to_s, printable_element_name(node_v1) + assert_dom_equal "3.1415926 (#{node_with_ref_without_name.id})", 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 "Nó teste (#{node.id})", printable_name(node) - assert_dom_equal "Nó teste (#{node.id})", printable_name(node_v2) - assert_dom_equal node.id.to_s, printable_name(node_v1) - assert_dom_equal "Nó teste (#{node.id}, v2)", printable_name(node_v2, :version => true) - assert_dom_equal "#{node.id}, v1", printable_name(node_v1, :version => true) - assert_dom_equal "3.1415926 (#{node_with_ref_without_name.id})", printable_name(node_with_ref_without_name) + assert_dom_equal deleted_node.id.to_s, printable_element_name(deleted_node) + assert_dom_equal "Nó teste (#{node.id})", printable_element_name(node) + assert_dom_equal "Nó teste (#{node.id})", printable_element_name(node_v2) + assert_dom_equal node.id.to_s, printable_element_name(node_v1) + assert_dom_equal "3.1415926 (#{node_with_ref_without_name.id})", 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 "Nó teste (#{node.id})", printable_name(node) - assert_dom_equal "Nó teste (#{node.id})", printable_name(node_v2) - assert_dom_equal node.id.to_s, printable_name(node_v1) - assert_dom_equal "Nó teste (#{node.id}, v2)", printable_name(node_v2, :version => true) - assert_dom_equal "#{node.id}, v1", printable_name(node_v1, :version => true) - assert_dom_equal "3.1415926 (#{node_with_ref_without_name.id})", printable_name(node_with_ref_without_name) + assert_dom_equal deleted_node.id.to_s, printable_element_name(deleted_node) + assert_dom_equal "Nó teste (#{node.id})", printable_element_name(node) + assert_dom_equal "Nó teste (#{node.id})", printable_element_name(node_v2) + assert_dom_equal node.id.to_s, printable_element_name(node_v1) + assert_dom_equal "3.1415926 (#{node_with_ref_without_name.id})", 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 "Test Node (#{node.id})", printable_name(node) - assert_dom_equal "Test Node (#{node.id})", printable_name(node_v2) - assert_dom_equal node.id.to_s, printable_name(node_v1) - assert_dom_equal "Test Node (#{node.id}, v2)", printable_name(node_v2, :version => true) - assert_dom_equal "#{node.id}, v1", printable_name(node_v1, :version => true) - assert_dom_equal "3.1415926 (#{node_with_ref_without_name.id})", printable_name(node_with_ref_without_name) + assert_dom_equal deleted_node.id.to_s, printable_element_name(deleted_node) + assert_dom_equal "Test Node (#{node.id})", printable_element_name(node) + assert_dom_equal "Test Node (#{node.id})", printable_element_name(node_v2) + assert_dom_equal node.id.to_s, printable_element_name(node_v1) + assert_dom_equal "3.1415926 (#{node_with_ref_without_name.id})", printable_element_name(node_with_ref_without_name) end end -- 2.39.5