add a tag diff to the /history endpoint

Author: stillhart

app/helpers/tag_diff_helper.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module TagDiffHelper
+  def tag_diff(current_tags, previous_tags)
+    current_tags ||= {}
+    previous_tags ||= {}
+
+    (current_tags.keys | previous_tags.keys).sort.filter_map do |key|
+      if current_tags.key?(key) && previous_tags.key?(key)
+        { :type => "modified", :key => key, :old_value => previous_tags[key], :new_value => current_tags[key] } if current_tags[key] != previous_tags[key]
+      elsif current_tags.key?(key)
+        { :type => "added", :key => key, :old_value => nil, :new_value => current_tags[key] }
+      else
+        { :type => "removed", :key => key, :old_value => previous_tags[key], :new_value => nil }
+      end
+    end
+  end
+end

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? %>
+    <details class="mb-3 border-bottom border-secondary-subtle pb-3">
+      <summary class="text-secondary"><%= t("browse.tag_diff.summary", :old_version => old_element.version, :new_version => new_element.version) %></summary>
+      <div class="border border-secondary-subtle rounded overflow-hidden mt-2">
+        <table class="table mb-0 browse-tag-list align-middle table-sm">
+          <tbody>
+            <% diffs.each do |diff| %>
+              <% case diff[:type] %>
+              <% when "added" %>
+                <tr class="table-secondary">
+                  <th scope="row" class="py-1 border-secondary-subtle fw-normal table-success" dir="auto"><span class="visually-hidden"><%= t("browse.tag_diff.type.added") %>:</span><%= format_key(diff[:key]) %> </th>
+                  <td class="py-1 border-secondary-subtle border-start table-success" dir="auto"><%= format_value(diff[:key], diff[:new_value]) %></td>
+                </tr>
+              <% when "removed" %>
+                <tr class="table-secondary">
+                  <th scope="row" class="py-1 border-secondary-subtle fw-normal table-danger" dir="auto"><span class="visually-hidden"><%= t("browse.tag_diff.type.removed") %>:</span><%= format_key(diff[:key]) %> </th>
+                  <td class="py-1 border-secondary-subtle border-start table-danger" dir="auto"><%= format_value(diff[:key], diff[:old_value]) %></td>
+                </tr>
+              <% when "modified" %>
+                <tr class="table-secondary">
+                  <th scope="row" rowspan="2" class="py-1 border-secondary-subtle fw-normal" dir="auto"><span class="visually-hidden"><%= t("browse.tag_diff.type.modified") %>:</span><%= format_key(diff[:key]) %> </th>
+                  <td class="py-1 border-secondary-subtle border-start table-success text-secondary" dir="auto"><span class="visually-hidden"><%= t("browse.tag_diff.status.new") %>:</span><%= format_value(diff[:key], diff[:new_value]) %></td>
+                </tr>
+                <tr class="table-secondary">
+                  <td class="py-1 border-secondary-subtle border-start table-danger text-secondary" dir="auto"><span class="visually-hidden"><%= t("browse.tag_diff.status.old") %>:</span><%= format_value(diff[:key], diff[:old_value]) %></td>
+                </tr>
+              <% end %>
+            <% end %>
+          </tbody>
+        </table>
+      </div>
+    </details>
+  <% end %>
+<% end %>

app/views/old_elements/index.html.erb

@@ -14,7 +14,11 @@
 <% end %>
 
 <div id="element_versions_list">
-  <%= 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 %>
 </div>
 
 <% if @old_features.older_items_cursor %>

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"

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

test/system/element_history_test.rb

@@ -339,4 +339,50 @@ def check_element_history_pages(get_path)
       assert_no_link "Older Versions"
     end
   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 v1 to v2"
+
+      find("details summary", :text => "Tag changes from version v1 to v2").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
 end