diff --git a/app/helpers/tag_diff_helper.rb b/app/helpers/tag_diff_helper.rb new file mode 100644 index 00000000000..9c32d6fed9a --- /dev/null +++ b/app/helpers/tag_diff_helper.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module TagDiffHelper + def tag_diff(new_tags, old_tags) + new_tags ||= {} + old_tags ||= {} + + (new_tags.keys | old_tags.keys).sort.filter_map do |key| + if new_tags.key?(key) && old_tags.key?(key) + { :type => "modified", :key => key, :old_value => old_tags[key], :new_value => new_tags[key] } if new_tags[key] != old_tags[key] + elsif new_tags.key?(key) + { :type => "added", :key => key, :old_value => nil, :new_value => new_tags[key] } + else + { :type => "removed", :key => key, :old_value => old_tags[key], :new_value => nil } + end + end + end +end diff --git a/app/views/browse/_tag_diff.html.erb b/app/views/browse/_tag_diff.html.erb new file mode 100644 index 00000000000..2f51f12169a --- /dev/null +++ b/app/views/browse/_tag_diff.html.erb @@ -0,0 +1,37 @@ +<%# locals: (new_element:, old_element:) %> +<% if old_element %> + <% diffs = tag_diff(new_element.tags, old_element.tags) %> + <% if diffs.any? %> +
+ <%= t("browse.tag_diff.summary", :old_version => old_element.version, :new_version => new_element.version) %> +
+ + + <% diffs.each do |diff| %> + <% case diff[:type] %> + <% when "added" %> + + + + + <% when "removed" %> + + + + + <% when "modified" %> + + + + + + + + <% end %> + <% end %> + +
<%= format_key(diff[:key]) %> <%= format_value(diff[:key], diff[:new_value]) %>
<%= format_key(diff[:key]) %> <%= format_value(diff[:key], diff[:old_value]) %>
<%= format_key(diff[:key]) %> <%= format_value(diff[:key], diff[:new_value]) %>
<%= format_value(diff[:key], diff[:old_value]) %>
+
+
+ <% end %> +<% end %> diff --git a/app/views/old_elements/index.html.erb b/app/views/old_elements/index.html.erb index 8f436b56854..13f01f3e775 100644 --- a/app/views/old_elements/index.html.erb +++ b/app/views/old_elements/index.html.erb @@ -14,7 +14,11 @@ <% end %>
- <%= render :partial => "browse/#{@type}", :collection => @old_features.items %> + <% @old_features.items.each_with_index do |item, index| %> + <%= render :partial => "browse/#{@type}", :locals => { @type.to_sym => item } %> + <% old_element = @old_features.items[index + 1] %> + <%= render "browse/tag_diff", :new_element => item, :old_element => old_element if old_element %> + <% end %>
<% if @old_features.older_items_cursor %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 779ae57c040..0ccf3715ce8 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -337,6 +337,15 @@ en: already_declared: "You have already declared that you consider your edits to be in the Public Domain." did_not_confirm: "You didn't confirm that you consider your edits to be in the Public Domain." browse: + tag_diff: + summary: "Tag changes from version #%{old_version} to #%{new_version}" + type: + added: "Added" + removed: "Removed" + modified: "Modified" + status: + new: "After" + old: "Before" deleted_ago_by_html: "Deleted %{time_ago} by %{user}" edited_ago_by_html: "Edited %{time_ago} by %{user}" version: "Version" diff --git a/test/helpers/tag_diff_helper_test.rb b/test/helpers/tag_diff_helper_test.rb new file mode 100644 index 00000000000..4931f3c1485 --- /dev/null +++ b/test/helpers/tag_diff_helper_test.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require "test_helper" + +class TagDiffHelperTest < ActionView::TestCase + def test_tag_diff + old_tags = { "highway" => "trunk", "maxspeed" => "50", "ref" => "A1" } + new_tags = { "highway" => "tertiary", "name" => "Main St", "ref" => "A1" } + + diffs = tag_diff(new_tags, old_tags) + + assert_equal 3, diffs.length + + assert_equal "highway", diffs[0][:key] + assert_equal "modified", diffs[0][:type] + assert_equal "trunk", diffs[0][:old_value] + assert_equal "tertiary", diffs[0][:new_value] + + assert_equal "maxspeed", diffs[1][:key] + assert_equal "removed", diffs[1][:type] + assert_equal "50", diffs[1][:old_value] + + assert_equal "name", diffs[2][:key] + assert_equal "added", diffs[2][:type] + assert_equal "Main St", diffs[2][:new_value] + + assert_nil(diffs.find { |d| d[:key] == "ref" }) + end +end diff --git a/test/system/element_history_test.rb b/test/system/element_history_test.rb index 88777f5bd27..b1c52005834 100644 --- a/test/system/element_history_test.rb +++ b/test/system/element_history_test.rb @@ -314,6 +314,52 @@ class ElementHistoryTest < ApplicationSystemTestCase check_element_history_pages(->(v) { old_relation_path(relation, v) }) end + test "shows tag diff in way history" do + way = create(:way, :with_history, :version => 2) + way_v1 = way.old_ways.find_by(:version => 1) + way_v2 = way.old_ways.find_by(:version => 2) + + # 1. No change + create(:old_way_tag, :old_way => way_v1, :k => "ref", :v => "A1") + create(:old_way_tag, :old_way => way_v2, :k => "ref", :v => "A1") + + # 2. Modification + create(:old_way_tag, :old_way => way_v1, :k => "highway", :v => "trunk") + create(:old_way_tag, :old_way => way_v2, :k => "highway", :v => "tertiary") + + # 3. Addition + create(:old_way_tag, :old_way => way_v2, :k => "maxspeed", :v => "50") + + # 4. Removal + create(:old_way_tag, :old_way => way_v1, :k => "operator", :v => "OldOp") + + visit way_history_path(way) + + within_sidebar do + assert_text "Tag changes from version #1 to #2" + + find("details summary", :text => "Tag changes from version #1 to #2").click + + within "details" do + # Modification + assert_css "th[rowspan='2']", :text => "highway" + assert_css "td.table-success", :text => "tertiary" + assert_css "td.table-danger", :text => "trunk" + + # Addition + assert_css "th.table-success", :text => "maxspeed" + assert_css "td.table-success", :text => "50" + + # Removal + assert_css "th.table-danger", :text => "operator" + assert_css "td.table-danger", :text => "OldOp" + + # No change (should NOT be here) + assert_no_text "ref" + end + end + end + private def check_element_history_pages(get_path)