Skip to content

Commit

Permalink
Bugfix, clearing LMSUser.lti_v13_user_id on LTI1.1 launches
Browse files Browse the repository at this point in the history
bulk_upsert will update the table columns in `update_elements` with the
value passed in updated.

In the case of lti_v13_user_id  that means that the value will be set on
LTI1.3  and removed (set to None) on LTI1.1 launches.

To fix this we change the value set from the raw value to
coalesce(value, LMSUser.lti_v13_user_id)
  • Loading branch information
marcospri committed Nov 28, 2024
1 parent ac8680d commit 970fda9
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 4 deletions.
13 changes: 11 additions & 2 deletions lms/services/upsert.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
15 changes: 13 additions & 2 deletions lms/services/user.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -89,7 +89,18 @@ def upsert_lms_user(self, user: User, lti_params: LTIParams) -> LMSUser:
}
],
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"'),
),
),
],
).one()
bulk_upsert(
self._db,
Expand Down

0 comments on commit 970fda9

Please sign in to comment.