Explicitly list of GroupInfo fields being updated.

Remove the last usage of BASE.update_from_dict and delete the method.

lms/db/__init__.py

@@ -28,34 +28,6 @@ def columns(cls):
             if isinstance(property, ColumnProperty)
         ]
 
-    def update_from_dict(self, data, skip_keys=None):
-        """
-        Update this model from the provided dict.
-
-        Any keys listed in ``skip_keys`` will *not* be updated, even if they are in ``data``.
-
-        :param data: The data to update
-        :param skip_keys: A set of keys to exclude from being updated (default: {"id"})
-        :type skip_keys: set[str]
-
-        :raise TypeError: if skip_keys isn't a set
-        """
-
-        if skip_keys is None:
-            skip_keys = {"id"}
-
-        if not isinstance(skip_keys, set):
-            raise TypeError(
-                f"Expected a set of keys to skip but found '{type(skip_keys)}'"
-            )
-
-        columns = set(self.columns())
-        columns -= skip_keys
-
-        for key in columns:
-            if key in data:
-                setattr(self, key, data[key])
-
     def __repr__(self):
         kwargs = ", ".join(
             f"{column}={repr(getattr(self, column))}" for column in self.columns()

lms/services/group_info.py

@@ -46,9 +46,25 @@ def upsert_group_info(self, grouping: Grouping, params: dict):
         group_info.application_instance = grouping.application_instance
 
         group_info.type = self._GROUPING_TYPES[grouping.type]
-        group_info.update_from_dict(
-            params, skip_keys={"authority_provided_id", "id", "info"}
-        )
+
+        for field in [
+            # Most (all) of these are duplicated elsewhere, we'll keep updating for now
+            # because external analytics query rely on this table.
+            "context_id",
+            "context_title",
+            "context_label",
+            "tool_consumer_info_product_family_code",
+            "tool_consumer_info_version",
+            "tool_consumer_instance_name",
+            "tool_consumer_instance_description",
+            "tool_consumer_instance_url",
+            "tool_consumer_instance_contact_email",
+            "tool_consumer_instance_guid",
+            "custom_canvas_api_domain",
+            "custom_canvas_course_id",
+        ]:
+            if field in params:
+                setattr(group_info, field, params[field])
 
         if self._lti_user.is_instructor:
             group_info.upsert_instructor(

tests/unit/lms/db/__init___test.py

@@ -1,4 +1,3 @@
-import pytest
 import sqlalchemy as sa
 
 from lms.db import BASE
@@ -25,31 +24,6 @@ def test_we_can_get_columns(self):
             "id",
         ]
 
-    def test_we_can_update_from_dict(self, model):
-        model.update_from_dict(
-            {
-                "id": 4321,
-                "column": "new_value",
-                "relationship": "something",
-                "missing": "Another value",
-            }
-        )
-
-        assert model.id == 1234
-        assert model.column == "new_value"
-
-    def test_we_can_specify_keys_to_skip(self, model):
-        model.update_from_dict(
-            {"id": 4321, "column": "new_value"}, skip_keys={"column"}
-        )
-
-        assert model.id == 4321
-        assert model.column == "original_value"
-
-    def test_we_fail_to_update_when_skip_keys_is_not_a_set(self):
-        with pytest.raises(TypeError):
-            ModelClass().update_from_dict({}, skip_keys=["a"])
-
     def test_repr(self):
         model = ModelClass(_aliased_column=77, id=23, column=46)
 
@@ -65,7 +39,3 @@ def test_repr_is_valid_python(self):
             "column",
         ):
             assert getattr(deserialized_model, attr) == getattr(model, attr)
-
-    @pytest.fixture
-    def model(self):
-        return ModelClass(id=1234, column="original_value")