Fix exception when categories removed in new config

Configuring would raise a KeyError if the new configuration didn't
have a category that was previously configured. Also added a test that
removes and adds categories.

edit_course/operations/configure.py

@@ -406,7 +406,7 @@ class ConfigParts:
     - categories, modules, and module_lobjects only contain the changed fields
     and some supplemental fields that are required elsewhere. E.g. to identify
     the lobject class type.
-    - *_keys fields contain whatever the <new> config contained. I.e. they
+    - *_names and *_keys fields contain whatever the <new> config contained. I.e. they
     contain the set of all such keys. E.g. module_keys contains the keys of
     all modules in the <new> config. They can be used to determined whether
     a module, exercise, etc. still exists in the course config even if nothing
@@ -415,8 +415,10 @@ class ConfigParts:
     """
     config: Dict[Any, Any]
 
+    # category name -> (category key, category config)
     categories: Dict[str, Tuple[Any, Dict[Any, Any]]]
     category_names: Set[str]
+    # category name -> category key
     category_key_map: Dict[str, str]
 
     modules: Dict[str, Dict[Any, Any]]
@@ -676,9 +678,11 @@ def configure(instance: CourseInstance, new_config: dict) -> Tuple[bool, List[st
         new_category_names = cparts.categories.keys() - (c.name for c in old_categories)
         new_categories = [LearningObjectCategory(course_instance=instance, name=name) for name in new_category_names]
 
+        # category key -> LearningObjectCategory object
         category_map = {
             cparts.category_key_map[category.name]: category
             for category in list(old_categories) + new_categories
+            if category.name in cparts.category_names
         }
 
         # Configure learning object categories.

edit_course/operations/tests.py

@@ -2,13 +2,13 @@
 from datetime import timedelta
 import itertools
 import logging
-from typing import Any, List, Optional, Union, Tuple
+from typing import Any, Iterable, List, Optional, Union, Tuple
 
 from django.test import TestCase
 from django.utils import timezone
 
 from .configure import configure, parse_bool
-from course.models import Course, CourseInstance, CourseModule
+from course.models import Course, CourseInstance, CourseModule, LearningObjectCategory
 from exercise.models import LearningObject, CourseChapter, BaseExercise, LTIExercise
 from external_services.models import LTIService
 
@@ -63,6 +63,19 @@
     ],
 }
 
+category_configs = {
+    "cat1": {
+        "name": "category1",
+    },
+    "cat2": {
+        "name": "category2",
+    },
+    "cat3": {
+        "name": "cat3",
+        "accept_unofficial_submits": True,
+    },
+}
+
 class ConfigureTest(TestCase):
     course: Course
     instance: CourseInstance
@@ -116,6 +129,7 @@ def setUp(self):
             "lti": ("lti_service", self.get_lti_service),
             "target_url": None,
         }
+        self.category_field_map = {}
 
     def tearDown(self) -> None:
         self.instance.course_modules.all().delete()
@@ -218,6 +232,18 @@ def test_changing_type(self):
                 0,
             )
 
+    def test_categories(self):
+        self.set_category(self.config, "cat3", category_configs["cat3"])
+
+        self.configure_and_test()
+
+        self.remove_category(self.config, "cat2")
+
+        self.configure_and_test()
+
+        self.set_category(self.config, "cat1", category_configs["cat2"])
+
+        self.configure_and_test()
 
     def configure_and_test(self):
         success, errors = configure(self.instance, self.config)
@@ -227,6 +253,24 @@ def configure_and_test(self):
         self.assertTrue(not errors)
         self.check_instance_config(self.instance, self.config)
 
+
+    def check_categories_config(self, categories: Iterable[LearningObjectCategory], config: dict):
+        # Hidden categories are old, so skip them
+        categories = [
+            category
+            for category in categories
+            if category.status != LearningObjectCategory.STATUS.HIDDEN
+        ]
+        category_names = {category.name for category in categories}
+        name_to_category_config = {c["name"]: c for c in config.values()}
+        config_category_names = set(name_to_category_config.keys())
+
+        self.assertEqual(category_names, config_category_names)
+
+        for category in categories:
+            self.check_fields(self.category_field_map, category, name_to_category_config[category.name])
+
+
     def check_lobject_config(self, lobject: LearningObject, config: dict):
         # order should be set by check_instance_config
         self.assertTrue("order" in config)
@@ -278,6 +322,8 @@ def set_order(config, n = 0) -> int:
         modules = {c["key"]: c for c in config.get("modules", [])}
         self.assertEqual(set(modules.keys()), set(module.url for module in instance.course_modules.all()))
 
+        self.check_categories_config(instance.categories.all(), config["categories"])
+
         n = 0
         nn = 0
         for o in config.get("modules", []):
@@ -308,7 +354,6 @@ def check_fields(self, mapping: dict, obj: Any, config: dict) -> None:
 
             self.assertEqual(getattr(obj, key), value, f"attr: {key}")
 
-
     def get_module_config(self, index, key):
         ret = deepcopy(module_configs[index])
         values = {
@@ -327,7 +372,6 @@ def get_exercise_config(self, typ, index, key):
         ret.update(values)
         return ret
 
-
     def insert_module(
             self,
             config: dict,
@@ -411,6 +455,20 @@ def remove_exercise(
         else:
             del parent["children"][index]
 
+    def set_category(
+            self,
+            config: dict,
+            category_key: str,
+            category_config: dict,
+            ) -> None:
+        config["categories"][category_key] = category_config
+
+    def remove_category(
+            self,
+            config: dict,
+            category_key: str,
+            ) -> None:
+        del config["categories"][category_key]
 
     def get_category_by_key(self, category_key):
         return self.instance.categories.get(name=self.config["categories"][category_key]["name"])