Deep merge jsonlike objects

Also remove duplicate checkboxes code

app/main/forms.py

@@ -58,7 +58,7 @@ from app.models.roles_and_permissions import (
     permissions,
     roles,
 )
-from app.utils import guess_name_from_email_address
+from app.utils import guess_name_from_email_address, merge_jsonlike
 
 
 def get_time_value_and_label(future_time):
@@ -241,12 +241,9 @@ def govuk_field_widget(self, field, type=None, param_extensions=None, **kwargs):
         params["type"] = type
 
     # extend default params with any sent in
-    if self.param_extensions:
-        params.update(self.param_extensions)
-
-    # add any sent in though use in templates
-    if param_extensions:
-        params.update(param_extensions)
+    merge_jsonlike(params, self.param_extensions)
+    # add any sent in through use in templates
+    merge_jsonlike(params, param_extensions)
 
     return Markup(
         render_template('vendor/govuk-frontend/components/input/template.njk', params=params))
@@ -290,8 +287,8 @@ class GovukEmailField(EmailField):
     def widget(self, field, param_extensions=None, **kwargs):
 
         params = {"attributes": {"spellcheck": "false"}}  # email addresses don't need to be spellchecked
-        if param_extensions:
-            params.update(param_extensions)
+        merge_jsonlike(params, param_extensions)
+
         return govuk_field_widget(self, field, type="email", param_extensions=params, **kwargs)
 
 
@@ -307,8 +304,8 @@ class GovukSearchField(SearchField):
     def widget(self, field, param_extensions=None, **kwargs):
 
         params = {"classes": "govuk-!-width-full"}  # email addresses don't need to be spellchecked
-        if param_extensions:
-            params.update(param_extensions)
+        merge_jsonlike(params, param_extensions)
+
         return govuk_field_widget(self, field, type="search", param_extensions=params, **kwargs)
 
 
@@ -349,9 +346,8 @@ class SMSCode(GovukTextInputField):
     def __call__(self, **kwargs):
         params = {"attributes": {"pattern": "[0-9]*"}}
         if "param_extensions" in kwargs:
-            kwargs["param_extensions"].update(params)
-        else:
-            kwargs["param_extensions"] = params
+            merge_jsonlike(kwargs["param_extensions"], params)
+
         return super().__call__(type='tel', **kwargs)
 
     def process_formdata(self, valuelist):
@@ -703,7 +699,7 @@ class govukCheckboxesMixin:
             del extensions['items']
 
         # merge dicts
-        params.update(extensions)
+        merge_jsonlike(params, extensions)
 
         # merge items
         if items:
@@ -789,29 +785,6 @@ class govukCheckboxesField(govukCheckboxesMixin, SelectMultipleField):
     def get_items_from_options(self, field):
         return [self.get_item_from_option(option) for option in field]
 
-    def extend_params(self, params, extensions):
-        items = None
-        param_items = len(params['items']) if 'items' in params else 0
-
-        # split items off from params to make it a pure dict
-        if 'items' in extensions:
-            items = extensions['items']
-            del extensions['items']
-
-        # merge dicts
-        params.update(extensions)
-
-        # merge items
-        if items:
-            if 'items' not in params:
-                params['items'] = items
-            else:
-                for idx, _item in enumerate(items):
-                    if idx >= param_items:
-                        params['items'].append(items[idx])
-                    else:
-                        params['items'][idx].update(items[idx])
-
     # self.__call__ renders the HTML for the field by:
     # 1. delegating to self.meta.render_field which
     # 2. calls field.widget

app/utils.py

@@ -822,3 +822,34 @@ def hide_from_search_engines(f):
         response.headers['X-Robots-Tag'] = 'noindex'
         return response
     return decorated_function
+
+
+# Function to merge two dict or lists with a JSON-like structure into one.
+# JSON-like means they can contain all types JSON can: all the main primitives
+# plus nested lists or dictionaries.
+# Merge is additive. New values overwrite old and collections are added to.
+def merge_jsonlike(source, destination):
+    def merge_items(source_item, destination_item):
+        if isinstance(source_item, dict) and isinstance(destination_item, dict):
+            merge_dicts(source_item, destination_item)
+        elif isinstance(source_item, list) and isinstance(destination_item, list):
+            merge_lists(source_item, destination_item)
+        else:  # primitive value
+            return False
+        return True
+
+    def merge_lists(source, destination):
+        for item in destination:
+            if item not in source:
+                source.append(item)
+
+    def merge_dicts(source, destination):
+        for key, value in destination.items():
+            if key in source:
+                # assign destination value if can't be merged into source
+                if merge_items(source[key], value) is False:
+                    source[key] = value
+            else:
+                source[key] = value
+
+    merge_items(source, destination)

tests/app/test_utils.py

@@ -21,6 +21,7 @@ from app.utils import (
     get_logo_cdn_domain,
     get_sample_template,
     is_less_than_90_days_ago,
+    merge_jsonlike,
     printing_today_or_tomorrow,
 )
 from tests.conftest import fake_uuid
@@ -596,3 +597,26 @@ def test_is_less_than_90_days_ago(date_from_db, expected_result):
 def test_get_sample_template_returns_template(template_type):
     template = get_sample_template(template_type)
     assert isinstance(template, Template)
+
+
+@pytest.mark.parametrize("source_object, destination_object, expected_result", [
+    # simple dicts:
+    ({"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:
+    ({"a": "b"}, {"a": "c"}, {"a": "c"}),
+    # 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"]),
+    # 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"}]),
+    # merge a dict with a null object returns that dict (does not work the other way round)
+    ({"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)
+    assert source_object == expected_result