Store LMS API id on LMSUser during launches

lms/migrations/versions/3466cfe2cbca_add_lmsuser_lms_api_user_id.py

@@ -0,0 +1,15 @@
+"""Add LMSUser.lms_api_user_id."""
+
+import sqlalchemy as sa
+from alembic import op
+
+revision = "3466cfe2cbca"
+down_revision = "50fb51c51314"
+
+
+def upgrade() -> None:
+    op.add_column("lms_user", sa.Column("lms_api_user_id", sa.String(), nullable=True))
+
+
+def downgrade() -> None:
+    op.drop_column("lms_user", "lms_api_user_id")

lms/models/lms_user.py

@@ -39,6 +39,11 @@ class LMSUser(CreatedUpdatedMixin, Base):
           In those cases lti_user_id will be the 1.1 value and we'll store here the 1.3 version.
     """
 
+    lms_api_user_id: Mapped[str | None] = mapped_column()
+    """
+    ID of the user in the propietary LMS API.
+    """
+
     h_userid: Mapped[str] = mapped_column(unique=True, index=True)
     """The userid value in H. This is calculated hashing tool_consumer_instance_guid and lti_user_id together."""
 

lms/services/upsert.py

@@ -10,7 +10,7 @@ def bulk_upsert(
     model_class,
     values: list[dict],
     index_elements: list[str],
-    update_columns: list[str],
+    update_columns: list[str | tuple],
 ):
     """
     Create or update the specified values in a table.
@@ -50,7 +50,16 @@ def bulk_upsert(
         # The columns to use to find matching rows.
         index_elements=index_elements,
         # The columns to update.
-        set_={element: getattr(base.excluded, element) for element in update_columns},
+        set_={
+            # For tuples include the two elements as the key and value of the dict
+            # For strings use value: excluded.value by default
+            (element[0] if isinstance(element, tuple) else element): (
+                element[1]
+                if isinstance(element, tuple)
+                else getattr(base.excluded, element)
+            )
+            for element in update_columns
+        },
     ).returning(*index_elements_columns)
 
     result = db.execute(stmt)

lms/services/user.py

@@ -1,6 +1,6 @@
 from functools import lru_cache
 
-from sqlalchemy import select
+from sqlalchemy import func, select, text
 from sqlalchemy.exc import NoResultFound
 from sqlalchemy.sql import Select
 
@@ -75,6 +75,9 @@ def upsert_user(self, lti_user: LTIUser) -> User:
     def upsert_lms_user(self, user: User, lti_params: LTIParams) -> LMSUser:
         """Upsert LMSUser based on a User object."""
         self._db.flush()  # Make sure User has hit the DB on the current transaction
+
+        # API ID, only Canvas for now
+        lms_api_user_id = lti_params.get("custom_canvas_user_id")
         lms_user = bulk_upsert(
             self._db,
             LMSUser,
@@ -86,10 +89,23 @@ def upsert_lms_user(self, user: User, lti_params: LTIParams) -> LMSUser:
                     "h_userid": user.h_userid,
                     "email": user.email,
                     "display_name": user.display_name,
+                    "lms_api_user_id": lms_api_user_id,
                 }
             ],
             index_elements=["h_userid"],
-            update_columns=["updated", "display_name", "email", "lti_v13_user_id"],
+            update_columns=[
+                "updated",
+                "display_name",
+                "email",
+                (
+                    "lti_v13_user_id",
+                    func.coalesce(
+                        text('"excluded"."lti_v13_user_id"'),
+                        text('"lms_user"."lti_v13_user_id"'),
+                    ),
+                ),
+                "lms_api_user_id",
+            ],
         ).one()
         bulk_upsert(
             self._db,

tests/unit/lms/services/user_test.py

@@ -65,6 +65,8 @@ def test_upsert_user_doesnt_save_email_for_existing_students(
 
     def test_upsert_lms_user(self, service, lti_user, pyramid_request, db_session):
         user = service.upsert_user(lti_user)
+        pyramid_request.lti_params["custom_canvas_user_id"] = "lms_api_user_id"
+
         lms_user = service.upsert_lms_user(user, pyramid_request.lti_params)
 
         lms_user = db_session.scalars(
@@ -75,6 +77,22 @@ def test_upsert_lms_user(self, service, lti_user, pyramid_request, db_session):
         assert lms_user.email == user.email
         assert lms_user.updated == user.updated
         assert lms_user.lti_v13_user_id == pyramid_request.lti_params.v13.get("sub")
+        assert lms_user.lms_api_user_id == "lms_api_user_id"
+
+    def test_upsert_lms_user_doesnt_clear_lti_v13_user_id(
+        self, service, lti_user, pyramid_request, db_session
+    ):
+        lms_user = factories.LMSUser(
+            lti_v13_user_id="EXISTING",
+            h_userid=lti_user.h_user.userid(authority="authority.example.com"),
+        )
+        db_session.commit()
+        pyramid_request.lti_params.v13["sub"] = None
+
+        user = service.upsert_user(lti_user)
+        lms_user = service.upsert_lms_user(user, pyramid_request.lti_params)
+
+        assert lms_user.lti_v13_user_id == "EXISTING"
 
     def test_get(self, user, service):
         db_user = service.get(user.application_instance, user.user_id)