Update service to use the new ext_lti_assignment_id

- Added .exists() .get_for_canvas_launch() .merge_canvas_assignments()
- Get doesn't return None but raises NoResultFound

lms/services/assignment.py

@@ -1,5 +1,7 @@
 from functools import lru_cache
 
+from sqlalchemy.orm.exc import MultipleResultsFound, NoResultFound
+
 from lms.models import Assignment
 
 
@@ -10,54 +12,199 @@ def __init__(self, db):
         self._db = db
 
     @lru_cache(maxsize=128)
-    def get(self, tool_consumer_instance_guid, resource_link_id):
+    def get(
+        self,
+        tool_consumer_instance_guid,
+        resource_link_id=None,
+        ext_lti_assignment_id=None,
+    ):
+        """Get an assignment using using resource_link_id, ext_lti_assignment_id or both."""
+
+        if not any([resource_link_id, ext_lti_assignment_id]):
+            raise ValueError(
+                "Can't get an assignment without neither resource_link_id or ext_lti_assignment_id"
+            )
+
+        if all([resource_link_id, ext_lti_assignment_id]):
+            # When launching an assignment in Canvas there are both resource_link_id and
+            # ext_lti_assignment_id launch params.
+            assignments = self.get_for_canvas_launch(
+                tool_consumer_instance_guid, resource_link_id, ext_lti_assignment_id
+            )
+            if len(assignments) > 1:
+                raise MultipleResultsFound(
+                    "Multiple assignments found. Should merge_canvas_assignments have been called"
+                )
+            return assignments[0]
+
+        if not resource_link_id:
+            # When creating or editing an assignment Canvas launches us with an
+            # ext_lti_assignment_id but no resource_link_id.
+            return self._get_for_canvas_assignment_config(
+                tool_consumer_instance_guid, ext_lti_assignment_id
+            )
+
+        # Non-Canvas assignment launches always have a resource_link_id and never
+        # have an ext_lti_assignment_id.
+        return self._get_by_resource_link_id(
+            tool_consumer_instance_guid, resource_link_id
+        )
+
+    @lru_cache(maxsize=128)
+    def get_for_canvas_launch(
+        self,
+        tool_consumer_instance_guid,
+        resource_link_id,
+        ext_lti_assignment_id,
+    ):
+        """Get a canvas assignment by both resource_link_id and ext_lti_assignment_id."""
+        assert resource_link_id and ext_lti_assignment_id
+
+        assignments = (
+            self._db.query(Assignment)
+            .filter(
+                Assignment.tool_consumer_instance_guid == tool_consumer_instance_guid,
+                (
+                    (
+                        (Assignment.resource_link_id == resource_link_id)
+                        & (Assignment.ext_lti_assignment_id == ext_lti_assignment_id)
+                    )
+                    | (
+                        (Assignment.resource_link_id == resource_link_id)
+                        & (Assignment.ext_lti_assignment_id.is_(None))
+                    )
+                    | (
+                        (Assignment.resource_link_id.is_(None))
+                        & (Assignment.ext_lti_assignment_id == ext_lti_assignment_id)
+                    )
+                ),
+            )
+            .order_by(Assignment.resource_link_id.asc())
+            .all()
+        )
+
+        if not assignments:
+            raise NoResultFound()
+
+        return assignments
+
+    def exists(
+        self,
+        tool_consumer_instance_guid,
+        resource_link_id=None,
+        ext_lti_assignment_id=None,
+    ) -> bool:
+        try:
+            self.get(
+                tool_consumer_instance_guid, resource_link_id, ext_lti_assignment_id
+            )
+        except MultipleResultsFound:
+            # Merge needed but it exists
+            return True
+        except (NoResultFound, ValueError):
+            return False
+        else:
+            return True
+
+    def set_document_url(  # pylint:disable=too-many-arguments
+        self,
+        document_url,
+        tool_consumer_instance_guid,
+        resource_link_id=None,
+        ext_lti_assignment_id=None,
+        extra=None,
+    ):
+        """
+        Save the given document_url in an existing assignment or create new one.
+
+        Set the document_url for the assignment that matches
+        tool_consumer_instance_guid and resource_link_id/ext_lti_assignment_id
+        or create a new one if none exist on the DB.
+
+        Any existing document URL for this assignment will be overwritten.
+        """
+        try:
+            assignment = self.get(
+                tool_consumer_instance_guid, resource_link_id, ext_lti_assignment_id
+            )
+        except NoResultFound:
+            assignment = Assignment(
+                tool_consumer_instance_guid=tool_consumer_instance_guid,
+                document_url=document_url,
+                resource_link_id=resource_link_id,
+                ext_lti_assignment_id=ext_lti_assignment_id,
+            )
+            self._db.add(assignment)
+
+        assignment.document_url = document_url
+        assignment.extra = self._update_extra(assignment.extra, extra)
+
+        self._clear_cache()
+        return assignment
+
+    def _get_by_resource_link_id(self, tool_consumer_instance_guid, resource_link_id):
         return (
             self._db.query(Assignment)
             .filter_by(
                 tool_consumer_instance_guid=tool_consumer_instance_guid,
                 resource_link_id=resource_link_id,
             )
-            .one_or_none()
+            .one()
         )
 
-    def get_document_url(self, tool_consumer_instance_guid, resource_link_id):
-        """
-        Return the matching document URL or None.
+    def _get_for_canvas_assignment_config(
+        self, tool_consumer_instance_guid, ext_lti_assignment_id
+    ):
+        return (
+            self._db.query(Assignment)
+            .filter_by(
+                tool_consumer_instance_guid=tool_consumer_instance_guid,
+                ext_lti_assignment_id=ext_lti_assignment_id,
+            )
+            .one()
+        )
 
-        Return the document URL for the assignment with the given
-        tool_consumer_instance_guid and resource_link_id, or None.
-        """
-        assignment = self.get(tool_consumer_instance_guid, resource_link_id)
+    @staticmethod
+    def _update_extra(old_extra, new_extra):
+        new_extra = new_extra if new_extra else {}
+        if not old_extra:
+            return new_extra
 
-        return assignment.document_url if assignment else None
+        old_extra = dict(old_extra)
+        if old_canvas_file_mappings := old_extra.get("canvas_file_mappings"):
+            new_extra["canvas_file_mappings"] = old_canvas_file_mappings
 
-    def set_document_url(
-        self, tool_consumer_instance_guid, resource_link_id, document_url
-    ):
+        return new_extra
+
+    def merge_canvas_assignments(self, old_assignment, new_assignment):
         """
-        Save the given document_url.
+        Merge two Canvas assignments into one and return the merged assignment.
 
-        Set the document_url for the assignment that matches
-        tool_consumer_instance_guid and resource_link_id. Any existing document
-        URL for this assignment will be overwritten.
+        Merge old_assignment into new_assignment, delete old_assignment, and return the updated
+        new_assignment.
         """
-        assignment = self.get(tool_consumer_instance_guid, resource_link_id)
+        new_extra = self._update_extra(old_assignment.extra, new_assignment.extra)
 
-        if assignment:
-            assignment.document_url = document_url
-        else:
-            self._db.add(
-                Assignment(
-                    document_url=document_url,
-                    resource_link_id=resource_link_id,
-                    tool_consumer_instance_guid=tool_consumer_instance_guid,
-                )
-            )
+        self._db.delete(old_assignment)
+        # Flushing early so the `resource_link_id` constraints doesn't
+        # conflict between the deleted record and new_assignment .
+        self._db.flush()
+
+        new_assignment.extra = new_extra
+        new_assignment.resource_link_id = old_assignment.resource_link_id
+
+        self._clear_cache()
+        return new_assignment
 
-        # Clear the cache (@lru_cache) on self.get because we've changed the
-        # contents of the DB. (Python's @lru_cache doesn't have a way to remove
-        # just one key from the cache, you have to clear the entire cache.)
+    def _clear_cache(self):
+        """
+        Clear the cache (@lru_cache) because we've changed the contents of the DB.
+
+        Python's @lru_cache doesn't have a way to remove
+        just one key from the cache, you have to clear the entire cache.)
+        """
         self.get.cache_clear()
+        self.get_for_canvas_launch.cache_clear()
 
 
 def factory(_context, request):

lms/views/basic_lti_launch.py

@@ -113,12 +113,14 @@ def canvas_file_basic_lti_launch(self):
         # module item configuration. As a result of this we can rely on this
         # being around in future code.
         self.assignment_service.set_document_url(
-            self.request.params["tool_consumer_instance_guid"],
-            resource_link_id,
             # This URL is mostly for show. We just want to ensure that a module
             # configuration exists. If we're going to do that we might as well
             # make sure this URL is meaningful.
             document_url=f"canvas://file/course/{course_id}/file_id/{file_id}",
+            tool_consumer_instance_guid=self.request.params[
+                "tool_consumer_instance_guid"
+            ],
+            resource_link_id=resource_link_id,
         )
 
         self.context.js_config.add_canvas_file_id(course_id, resource_link_id, file_id)
@@ -162,9 +164,9 @@ def db_configured_basic_lti_launch(self):
         # here we can safely assume that the document_url exists.
         tool_consumer_instance_guid = self.request.params["tool_consumer_instance_guid"]
         resource_link_id = self.request.params["resource_link_id"]
-        document_url = self.assignment_service.get_document_url(
+        document_url = self.assignment_service.get(
             tool_consumer_instance_guid, resource_link_id
-        )
+        ).document_url
         return self.basic_lti_launch(document_url)
 
     @view_config(blackboard_copied=True)
@@ -206,12 +208,12 @@ def course_copied_basic_lti_launch(self, original_resource_link_id):
         tool_consumer_instance_guid = self.request.params["tool_consumer_instance_guid"]
         resource_link_id = self.request.params["resource_link_id"]
 
-        document_url = self.assignment_service.get_document_url(
+        document_url = self.assignment_service.get(
             tool_consumer_instance_guid, original_resource_link_id
-        )
+        ).document_url
 
         self.assignment_service.set_document_url(
-            tool_consumer_instance_guid, resource_link_id, document_url
+            document_url, tool_consumer_instance_guid, resource_link_id
         )
 
         return self.basic_lti_launch(document_url)
@@ -309,9 +311,9 @@ def configure_assignment(self):
         document_url = self.request.parsed_params["document_url"]
 
         self.assignment_service.set_document_url(
+            document_url,
             self.request.parsed_params["tool_consumer_instance_guid"],
             self.request.parsed_params["resource_link_id"],
-            document_url,
         )
 
         self.context.js_config.add_document_url(document_url)

lms/views/predicates/_lti_launch.py

@@ -35,14 +35,11 @@ def __call__(self, context, request):
         resource_link_id = request.params.get("resource_link_id")
         tool_consumer_instance_guid = request.params.get("tool_consumer_instance_guid")
 
-        has_document_url = bool(
-            assignment_svc.get_document_url(
-                tool_consumer_instance_guid, resource_link_id
-            )
+        return (
+            assignment_svc.exists(tool_consumer_instance_guid, resource_link_id)
+            == self.value
         )
 
-        return has_document_url == self.value
-
 
 class _CourseCopied(Base, ABC):
     """
@@ -111,10 +108,9 @@ def __call__(self, context, request):
                 # Look for the document URL of the previous assignment that
                 # this one was copied from.
                 assignment_service = request.find_service(name="assignment")
-                previous_document_url = assignment_service.get_document_url(
+                is_newly_copied = assignment_service.exists(
                     tool_consumer_instance_guid, original_resource_link_id
                 )
-                is_newly_copied = bool(previous_document_url)
 
         return is_newly_copied == self.value