From 23d391e3fc3c348a718eb7c611285b936631f170 Mon Sep 17 00:00:00 2001
From: Tom Byers <tombaromba@gmail.com>
Date: Thu, 21 Jan 2021 16:49:12 +0000
Subject: [PATCH] Change how merge_jsonlike treats lists

Current behaviour is to check item-against-item
and merge based on whether items match, irrelevant
of position. This doesn't produce the results we
need for our usecases (merging data to send to
GOVUK Frontend components).

We actually want:
- items to be compared based on their position
- new primitive items at the same position to
  overwrite existing ones
- dicts or lists at the same position to be merged

For example,

Starting with this list:

[{"name": "option-1", "value": "1"}]

Merging in this list:

[{"hint": {"text": "Choose one option"}}]

You currently get this:

[
  {"name": "option-1", "value": "1"},
  {"hint": {"text": "Choose one option"}}
]

We want to get this:

[
  {
    "name": "option-1", "value": "1",
    "hint": {"text": "Choose one option"}
  }
]
---
 app/utils.py            |  9 +++++++--
 tests/app/test_utils.py | 14 ++++++++------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/app/utils.py b/app/utils.py
index 41cdc29d1..75f301b72 100644
--- a/app/utils.py
+++ b/app/utils.py
@@ -651,8 +651,13 @@ def merge_jsonlike(source, destination):
         return True
 
     def merge_lists(source, destination):
-        for item in destination:
-            if item not in source:
+        last_dest_idx = len(destination) - 1
+        for idx, item in enumerate(destination):
+            if idx <= last_dest_idx:
+                # assign destination value if can't be merged into source
+                if merge_items(source[idx], destination[idx]) is False:
+                    source[idx] = destination[idx]
+            else:
                 source.append(item)
 
     def merge_dicts(source, destination):
diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py
index 4246b5037..b869e9a6a 100644
--- a/tests/app/test_utils.py
+++ b/tests/app/test_utils.py
@@ -614,18 +614,20 @@ def test_get_sample_template_returns_template(template_type):
     ({"a": "b"}, {"c": "d"}, {"a": "b", "c": "d"}),
     # dicts with nested dict, both under same key, additive behaviour:
     ({"a": {"b": "c"}}, {"a": {"e": "f"}}, {"a": {"b": "c", "e": "f"}}),
-    # same key in both dicts, value is a string, destination supersedes source:
+    # same key in both dicts, value is a string, destination supercedes source:
     ({"a": "b"}, {"a": "c"}, {"a": "c"}),
+    # lists with same length but different items, destination supercedes source:
+    (["b", "c", "d"], ["b", "e", "f"], ["b", "e", "f"]),
     # lists in dicts behave as top level lists
-    ({"a": ["b", "c", "d"]}, {"a": ["b", "e", "f"]}, {"a": ["b",  "c", "d", "e", "f"]}),
-    # lists with same string in both result in a list of unique values
-    (["a", "b", "c", "d"], ["d", "e", "f"], ["a", "b", "c", "d", "e", "f"]),
+    ({"a": ["b", "c", "d"]}, {"a": ["b", "e", "f"]}, {"a": ["b", "e", "f"]}),
+    # lists with same string in both, at different positions, result in duplicates keeping their positions
+    (["a", "b", "c", "d"], ["d", "e", "f"], ["d", "e", "f", "d"]),
     # lists with same dict in both result in a list with one instance of that dict
     ([{"b": "c"}], [{"b": "c"}], [{"b": "c"}]),
     # if dicts in lists have different values, they are not merged
-    ([{"b": "c"}], [{"b": "e"}], [{"b": "c"}, {"b": "e"}]),
+    ([{"b": "c"}], [{"b": "e"}], [{"b": "e"}]),
     # merge a dict with a null object returns that dict (does not work the other way round)
-    ({"a": {"b": "c"}}, None, {"a": {"b": "c"}}),
+    ({"a": {"b": "c"}}, None, {"a": {"b": "c"}})
 ])
 def test_merge_jsonlike_merges_jsonlike_objects_correctly(source_object, destination_object, expected_result):
     merge_jsonlike(source_object, destination_object)