From 52fe571eb27cb68151a5c7f9dc338ff7d5def8a4 Mon Sep 17 00:00:00 2001
From: Adrian Turjak <adriant@catalyst.net.nz>
Date: Fri, 26 Apr 2019 16:12:52 +1200
Subject: [PATCH] Fix streamfield structblock compare append usage

Append was being called with 2 params, which clearly should
have been a tuple.

Fixes: https://github.com/wagtail/wagtail/issues/5261
---
 CHANGELOG.txt                       |  6 +++
 docs/releases/index.rst             |  1 +
 wagtail/admin/compare.py            |  4 +-
 wagtail/admin/tests/test_compare.py | 57 +++++++++++++++++++++++++++++
 4 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.txt b/CHANGELOG.txt
index bbb53f694..402885222 100644
--- a/CHANGELOG.txt
+++ b/CHANGELOG.txt
@@ -16,6 +16,12 @@ Changelog
  * Fix: Explorer menu no longer shows sibling pages for which the user does not have access (Mike Hearn)
 
 
+2.5.1 (xx.xx.xxxx)
+~~~~~~~~~~~~~~~~~~
+
+ * Fix: Prevent crash when comparing StructBlocks in revision history (Adrian Turjak, Matt Westcott)
+
+
 2.5 (24.04.2019)
 ~~~~~~~~~~~~~~~~
 
diff --git a/docs/releases/index.rst b/docs/releases/index.rst
index 33558ca16..5d812e254 100644
--- a/docs/releases/index.rst
+++ b/docs/releases/index.rst
@@ -6,6 +6,7 @@ Release notes
 
    upgrading
    2.6
+   2.5.1
    2.5
    2.4
    2.3
diff --git a/wagtail/admin/compare.py b/wagtail/admin/compare.py
index ed0dc7f58..d5b41421f 100644
--- a/wagtail/admin/compare.py
+++ b/wagtail/admin/compare.py
@@ -115,7 +115,7 @@ class StructBlockComparison(BlockComparison):
             label = self.block.child_blocks[name].label
             comparison_class = get_comparison_class_for_block(block)
 
-            htmlvalues.append(label, comparison_class(block, True, True, val[name], val[name]).htmlvalue(val[name]))
+            htmlvalues.append((label, comparison_class(block, True, True, val[name], val[name]).htmlvalue(val[name])))
 
         return format_html('<dl>\n{}\n</dl>', format_html_join(
             '\n', '    <dt>{}</dt>\n    <dd>{}</dd>', htmlvalues))
@@ -126,7 +126,7 @@ class StructBlockComparison(BlockComparison):
             label = self.block.child_blocks[name].label
             comparison_class = get_comparison_class_for_block(block)
 
-            htmldiffs.append(label, comparison_class(block, self.exists_a, self.exists_b, self.val_a[name], self.val_b[name]).htmldiff())
+            htmldiffs.append((label, comparison_class(block, self.exists_a, self.exists_b, self.val_a[name], self.val_b[name]).htmldiff()))
 
         return format_html('<dl>\n{}\n</dl>', format_html_join(
             '\n', '    <dt>{}</dt>\n    <dd>{}</dd>', htmldiffs))
diff --git a/wagtail/admin/tests/test_compare.py b/wagtail/admin/tests/test_compare.py
index f60d8a97d..568c5717e 100644
--- a/wagtail/admin/tests/test_compare.py
+++ b/wagtail/admin/tests/test_compare.py
@@ -243,6 +243,63 @@ class TestStreamFieldComparison(TestCase):
         self.assertEqual(comparison.htmldiff(), '<div class="comparison__child-object"><span class="deletion">Original content</span><span class="addition">doSomethingBad();</span></div>')
         self.assertIsInstance(comparison.htmldiff(), SafeText)
 
+    def test_compare_structblock(self):
+        field = StreamPage._meta.get_field('body')
+
+        comparison = self.comparison_class(
+            field,
+            StreamPage(body=StreamValue(field.stream_block, [
+                ('product', {'name': 'a packet of rolos', 'price': '75p'}, '1'),
+            ])),
+            StreamPage(body=StreamValue(field.stream_block, [
+                ('product', {'name': 'a packet of rolos', 'price': '85p'}, '1'),
+            ])),
+        )
+
+        expected = """
+            <div class="comparison__child-object"><dl>
+                <dt>Name</dt>
+                <dd>a packet of rolos</dd>
+                <dt>Price</dt>
+                <dd><span class="deletion">75p</span><span class="addition">85p</span></dd>
+            </dl></div>
+        """
+        self.assertHTMLEqual(comparison.htmldiff(), expected)
+        self.assertIsInstance(comparison.htmldiff(), SafeText)
+        self.assertTrue(comparison.has_changed())
+
+    def test_compare_imagechooserblock(self):
+        image_model = get_image_model()
+        test_image_1 = image_model.objects.create(
+            title="Test image 1",
+            file=get_test_image_file(),
+        )
+        test_image_2 = image_model.objects.create(
+            title="Test image 2",
+            file=get_test_image_file(),
+        )
+
+        field = StreamPage._meta.get_field('body')
+
+        comparison = self.comparison_class(
+            field,
+            StreamPage(body=StreamValue(field.stream_block, [
+                ('image', test_image_1, '1'),
+            ])),
+            StreamPage(body=StreamValue(field.stream_block, [
+                ('image', test_image_2, '1'),
+            ])),
+        )
+
+        result = comparison.htmldiff()
+        self.assertIn('<div class="preview-image deletion">', result)
+        self.assertIn('alt="Test image 1"', result)
+        self.assertIn('<div class="preview-image addition">', result)
+        self.assertIn('alt="Test image 2"', result)
+
+        self.assertIsInstance(result, SafeText)
+        self.assertTrue(comparison.has_changed())
+
 
 class TestChoiceFieldComparison(TestCase):
     comparison_class = compare.ChoiceFieldComparison