diff --git a/app/main/forms.py b/app/main/forms.py index 825c39158..6e4fe16f6 100644 --- a/app/main/forms.py +++ b/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 diff --git a/app/utils.py b/app/utils.py index 39ec1f771..526a4a493 100644 --- a/app/utils.py +++ b/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) diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py index a53723f00..776143fa6 100644 --- a/tests/app/test_utils.py +++ b/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