diff --git a/edit_course/operations/configure.py b/edit_course/operations/configure.py index 730d3c8e7..28ecbd0dc 100644 --- a/edit_course/operations/configure.py +++ b/edit_course/operations/configure.py @@ -1,8 +1,9 @@ import copy from dataclasses import dataclass from datetime import datetime, timedelta +from functools import partial import json -from typing import Any, cast, Dict, List, Optional, Set, Tuple, Type, Union +from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Type from aplus_auth.payload import Permission, Permissions from aplus_auth.requests import get as aplus_get @@ -111,11 +112,15 @@ def remove_newlines(value): return value.replace('\r\n', ' ').replace('\n', ' ').replace('\r', ' ') +ChangesGetter = Callable[[dict, dict], Optional[dict]] + + def get_config_changes( old: Dict[str, Any], new: Dict[str, Any], *, - keep: Union[None, List[str], List[List[str]]] = None, + recurser: Optional[ChangesGetter] = None, + keep: Optional[List[str]] = None, keep_unchanged: bool = False, ) -> Optional[Dict]: """Gets changes between configs. @@ -124,11 +129,8 @@ def get_config_changes( :param new: new config Keyword-only parameters: - :param keep: fields in to add output as-is (unless no changes were detected, - see the param). May be a list of fields or a list of lists of fields - where first list is used to determine fields to keep now and later lists are - recursively used for nested dicts (second list is used for dicts nested in the - current dict, third for those nested in the nested dicts, etc.). + :param keep: fields in to add to output as-is (unless no changes were detected, + see the param). :param keep_unchanged: whether to add fields in to output even when they have no changes. Fields in are normally not added to output if there were no changes in the dict itself. E.g. comparison of {"field": "value"} to @@ -137,32 +139,27 @@ def get_config_changes( {"field": "value"}. Returns None if == (and is False). Otherwise: - Recursively finds the differences in dicts and . If the values for - a key differ, the value in is added to output. If the values are the same, - the key is removed from the output. + Finds the differences in dicts and where is used to find differences + between dict values. If the values for a key differ, the value in is added to output. + If the values are the same, the key is removed from the output. """ if keep is None: keep = [] - if len(keep) > 0 and isinstance(keep[0], list): - keep_now, *keep_later = cast(List[List[str]], keep) - else: - keep_now, keep_later = cast(List[str], keep), [] - if not keep_unchanged and old == new: return None # Keys to keep in output whether changed or not. This includes the ones - # in , and the keys that are in but not in - kept = new.keys() - (old.keys() - keep_now) + # in , and the keys that are in but not in + kept = new.keys() - (old.keys() - keep) # Keys that need to be compared: those that are in both dicts, - # and are not in - commonkeys_not_in_keep = old.keys() & new.keys() - keep_now + # and are not in + commonkeys_not_in_keep = old.keys() & new.keys() - keep diff = {key: new[key] for key in kept} for key in commonkeys_not_in_keep: - if isinstance(old[key], dict) and isinstance(new[key], dict): - new_val = get_config_changes(old[key], new[key], keep=keep_later, keep_unchanged=keep_unchanged) + if recurser is not None and isinstance(old[key], dict) and isinstance(new[key], dict): + new_val = recurser(old[key], new[key]) # Do not include the attribute if there were no changes if new_val is not None: diff[key] = new_val @@ -172,6 +169,17 @@ def get_config_changes( return diff +# Gets changes between two {lobj key: lobj config} -dicts +get_lobject_changes: ChangesGetter = partial( + get_config_changes, + recurser=partial( + # We don't want to recurse any further to leave each field in the lobj intact. + get_config_changes, + keep=["children", "category", "target_category", "target_url", "max_submissions", "lti"] + ) +) + + def lobject_class(config: dict) -> Type[LearningObject]: if "lti1p3" in config: return LTI1p3Exercise @@ -539,11 +547,7 @@ def diff(old: Optional["ConfigParts"], new: "ConfigParts") -> "ConfigParts": if modules_config is None: modules_config = {} - module_lobjects = get_config_changes( - old.module_lobjects, - new.module_lobjects, - keep=[[], [], ["children", "category", "target_category", "target_url", "max_submissions", "lti"]], - ) + module_lobjects = get_config_changes(old.module_lobjects, new.module_lobjects, recurser=get_lobject_changes) if module_lobjects is None: module_lobjects = {} diff --git a/edit_course/operations/tests.py b/edit_course/operations/tests.py index 7d5d7bb5b..e67479e96 100644 --- a/edit_course/operations/tests.py +++ b/edit_course/operations/tests.py @@ -232,6 +232,36 @@ def test_changing_type(self): 0, ) + def test_field_defined_by_dict(self): + exercise_config = self.get_exercise_config("BaseExercise", 0, "test_BaseExercise") + + exercise_config["exercise_info"] = { + "test": "test", + "test2": "test2", + } + self.insert_exercise(self.config, exercise_config, 0) + self.configure_and_test() + + exercise_config["exercise_info"] = { + "test": "test", + "test2": "test2", + "test3": "test3", + } + self.insert_exercise(self.config, exercise_config, 0) + self.configure_and_test() + + exercise_config["exercise_info"] = { + "test": "test", + } + self.insert_exercise(self.config, exercise_config, 0) + self.configure_and_test() + + exercise_config["exercise_info"] = { + "test5": "test5", + } + self.insert_exercise(self.config, exercise_config, 0) + self.configure_and_test() + def test_categories(self): self.set_category(self.config, "cat3", category_configs["cat3"])