Include assets, tags and template in diff between revisions

5 files changed, 114 insertions, 13 deletions

diff --git a/app/models/page.rb b/app/models/page.rb
index ea04cd6..740d42e 100644
--- a/app/models/page.rb
+++ b/app/models/page.rb
@@ -176,19 +176,26 @@ class Page < ApplicationRecord
   end
 
   def diff_against other, view: :inline
-    if view == :side_by_side
-      {
-        title:    HtmlWordDiff.side_by_side(other.title.to_s,    title.to_s),
-        abstract: HtmlWordDiff.side_by_side(other.abstract.to_s, abstract.to_s),
-        body:     HtmlWordDiff.side_by_side(other.body.to_s,     body.to_s)
-      }
-    else
-      {
-        title:    HtmlWordDiff.inline(other.title.to_s,    title.to_s),
-        abstract: HtmlWordDiff.inline(other.abstract.to_s, abstract.to_s),
-        body:     HtmlWordDiff.inline(other.body.to_s,     body.to_s)
-      }
-    end
+    text_diffs =
+      if view == :side_by_side
+        {
+          title:    HtmlWordDiff.side_by_side(other.title.to_s,    title.to_s),
+          abstract: HtmlWordDiff.side_by_side(other.abstract.to_s, abstract.to_s),
+          body:     HtmlWordDiff.side_by_side(other.body.to_s,     body.to_s)
+        }
+      else
+        {
+          title:    HtmlWordDiff.inline(other.title.to_s,    title.to_s),
+          abstract: HtmlWordDiff.inline(other.abstract.to_s, abstract.to_s),
+          body:     HtmlWordDiff.inline(other.body.to_s,     body.to_s)
+        }
+      end
+
+    text_diffs.merge(
+      tags:          { added: tag_list.to_a - other.tag_list.to_a, removed: other.tag_list.to_a - tag_list.to_a },
+      template_name: { from: other.template_name, to: template_name, changed: template_name != other.template_name },
+      assets:        { added: assets.to_a - other.assets.to_a, removed: other.assets.to_a - assets.to_a }
+    )
   end
 
   def public?
diff --git a/app/views/revisions/diff.html.erb b/app/views/revisions/diff.html.erb
index d70503c..b9ce6bd 100644
--- a/app/views/revisions/diff.html.erb
+++ b/app/views/revisions/diff.html.erb
@@ -85,4 +85,31 @@
     <h3>Body</h3>
     <%= raw @diff[:body] %>
   <% end %>
+
+  <h3>Tags</h3>
+  <% if @diff[:tags][:added].empty? && @diff[:tags][:removed].empty? %>
+    <p class="diff_unchanged">No change.</p>
+  <% else %>
+    <ul class="diff_set_list">
+      <% @diff[:tags][:added].each do |tag| %><li><ins><%= tag %></ins></li><% end %>
+      <% @diff[:tags][:removed].each do |tag| %><li><del><%= tag %></del></li><% end %>
+    </ul>
+  <% end %>
+
+  <h3>Template</h3>
+  <% if @diff[:template_name][:changed] %>
+    <p><del><%= @diff[:template_name][:from] || '(none)' %></del> <ins><%= @diff[:template_name][:to] || '(none)' %></ins></p>
+  <% else %>
+    <p class="diff_unchanged">No change.</p>
+  <% end %>
+
+  <h3>Assets</h3>
+  <% if @diff[:assets][:added].empty? && @diff[:assets][:removed].empty? %>
+    <p class="diff_unchanged">No change.</p>
+  <% else %>
+    <ul class="diff_set_list">
+      <% @diff[:assets][:added].each do |asset| %><li><ins><%= asset.upload_file_name %></ins></li><% end %>
+      <% @diff[:assets][:removed].each do |asset| %><li><del><%= asset.upload_file_name %></del></li><% end %>
+    </ul>
+  <% end %>
 </div>
diff --git a/public/stylesheets/admin.css b/public/stylesheets/admin.css
index e04499d..7b39c72 100644
--- a/public/stylesheets/admin.css
+++ b/public/stylesheets/admin.css
@@ -549,6 +549,17 @@ table.revisions_table tr:hover {
   cursor: help;
 }
 
+.diff_unchanged {
+  color: #969696;
+  font-style: italic;
+}
+
+.diff_set_list {
+  list-style: none;
+  padding-left: 0;
+  margin: 0;
+}
+
 table.user_table td.user_login {
   padding-right: 30px;
 }
diff --git a/test/controllers/revisions_controller_test.rb b/test/controllers/revisions_controller_test.rb
index bf92c5b..53927b4 100644
--- a/test/controllers/revisions_controller_test.rb
+++ b/test/controllers/revisions_controller_test.rb
@@ -148,4 +148,17 @@ class RevisionsControllerTest < ActionController::TestCase
     assert_response :success
     assert_select "a", "Side by side"
   end
+
+  test "diffing two revisions also shows tag, template, and asset changes" do
+    login_as :quentin
+    @node.find_or_create_draft(@user)
+    @node.draft.tag_list = "update"
+    @node.draft.save!
+
+    post(:diff, params: { :node_id => @node.id, :start_revision => @node.pages.first.revision, :end_revision => @node.pages.last.revision })
+    assert_response :success
+    assert_select "h3", "Tags"
+    assert_select "h3", "Template"
+    assert_select "h3", "Assets"
+  end
 end
diff --git a/test/models/page_test.rb b/test/models/page_test.rb
index 3868a53..bcddc89 100644
--- a/test/models/page_test.rb
+++ b/test/models/page_test.rb
@@ -228,4 +228,47 @@ class PageTest < ActiveSupport::TestCase
     assert_equal 2, fragment.css('ins.diff_structural').length
     assert_match "der Zugang erfolgt über den Hinterhof.", fragment.text
   end
+
+  test "diff_against reports tag and template changes" do
+    n = Node.root.children.create! :slug => "field_diff_test"
+    d = n.find_or_create_draft @user1
+    d.tag_list = "update"
+    d.template_name = "standard_template"
+    d.save!
+    n.publish_draft!
+
+    d2 = n.find_or_create_draft @user1
+    d2.tag_list = "update, pressemitteilung"
+    d2.template_name = "title_only"
+    d2.save!
+
+    diff = d2.diff_against(n.head)
+
+    assert_equal ["pressemitteilung"], diff[:tags][:added]
+    assert_equal [], diff[:tags][:removed]
+    assert diff[:template_name][:changed]
+    assert_equal "standard_template", diff[:template_name][:from]
+    assert_equal "title_only", diff[:template_name][:to]
+  end
+
+  test "diff_against reports added and removed assets by filename" do
+    n = Node.root.children.create! :slug => "asset_diff_test"
+    d = n.find_or_create_draft @user1
+    d.save!
+    n.publish_draft!
+
+    kept_asset    = Asset.create!(:upload_file_name => "kept.png", :upload_content_type => "image/png", :upload_file_size => 1)
+    removed_asset = Asset.create!(:upload_file_name => "removed.pdf", :upload_content_type => "application/pdf", :upload_file_size => 1)
+    n.head.update_assets([kept_asset.id, removed_asset.id])
+
+    d2 = n.find_or_create_draft @user1
+    added_asset = Asset.create!(:upload_file_name => "added.png", :upload_content_type => "image/png", :upload_file_size => 1)
+    d2.update_assets([kept_asset.id, added_asset.id])
+    d2.save!
+
+    diff = d2.diff_against(n.head)
+
+    assert_equal [added_asset], diff[:assets][:added]
+    assert_equal [removed_asset], diff[:assets][:removed]
+  end
 end