Skip to content

Commit

Permalink
Fix assignment membership query to handle multiple rows per student/role
Browse files Browse the repository at this point in the history
We store one membership role per assignment, course and role. The
existing query used SQLA .one() while querying by just assignment and
student which would raise one more that one role existed.

Use .first instead and assert the existence of the membership object
below.

The existing assert on  lti_v11_lis_result_sourcedid is there for the
typechecker benefit but we also include that condition on the query
  • Loading branch information
marcospri committed Oct 15, 2024
1 parent 0e319d2 commit 8246f50
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 13 deletions.
8 changes: 4 additions & 4 deletions lms/services/lti_grading/_v11.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ def sync_grade(
select(LMSUserAssignmentMembership).where(
LMSUserAssignmentMembership.lms_user_id == lms_user.id,
LMSUserAssignmentMembership.assignment_id == assignment.id,
LMSUserAssignmentMembership.lti_v11_lis_result_sourcedid.is_not(None),
)
).one()
).first()
assert (
assignment_membership.lti_v11_lis_result_sourcedid
), "Trying to grade a student without lti_v11_lis_result_sourcedid"

assignment_membership and assignment_membership.lti_v11_lis_result_sourcedid
), "Trying to grade a student without a membership or membership without lti_v11_lis_result_sourcedid"
request = self._record_score_payload(
score=score,
user_grading_id=assignment_membership.lti_v11_lis_result_sourcedid,
Expand Down
29 changes: 20 additions & 9 deletions tests/unit/lms/services/lti_grading/_v11_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,23 +74,34 @@ def test_record_result(self, svc, score, http_service, score_payload):
def test_sync_grade(self, svc, http_service, application_instance, db_session):
lms_user = factories.LMSUser()
assignment = factories.Assignment()
lti_role = factories.LTIRole()
db_session.flush()
membership = factories.LMSUserAssignmentMembership(
lms_user_id=lms_user.id,
assignment_id=assignment.id,
lti_role_id=lti_role.id,
factories.LMSUserAssignmentMembership(
lms_user=lms_user,
assignment=assignment,
lti_role=factories.LTIRole(),
lti_v11_lis_result_sourcedid="LIS",
)
# Membership for another role
factories.LMSUserAssignmentMembership(
lms_user=lms_user,
assignment=assignment,
lti_role=factories.LTIRole(),
lti_v11_lis_result_sourcedid="LIS",
)
# Membership without lis_result_sourcedid
factories.LMSUserAssignmentMembership(
lms_user=lms_user,
assignment=assignment,
lti_role=factories.LTIRole(),
lti_v11_lis_result_sourcedid=None,
)
db_session.flush()

svc.sync_grade(application_instance, assignment, None, lms_user, 0.5)

assert self.sent_pox_body(http_service) == {
"replaceResultRequest": {
"resultRecord": {
"sourcedGUID": {
"sourcedId": membership.lti_v11_lis_result_sourcedid
},
"sourcedGUID": {"sourcedId": "LIS"},
"result": {"resultScore": {"language": "en", "textString": "0.5"}},
}
}
Expand Down

0 comments on commit 8246f50

Please sign in to comment.