Skip to content

Commit

Permalink
Only update assignment attributes in update_assignment
Browse files Browse the repository at this point in the history
update_assignment takes care of the case where speedgrader sends us
bogus data and ignores it.

Without this change we were updating lti_v13_resource_link_id and
lis_outcome_url on speedgrader launches to incorrect values.
  • Loading branch information
marcospri committed Oct 2, 2024
1 parent 057cbc6 commit afb5c3a
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 26 deletions.
20 changes: 10 additions & 10 deletions lms/services/assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,16 @@ def update_assignment( # noqa: PLR0913, PLR0917
assignment.is_gradable = self._misc_plugin.is_assignment_gradable(
request.lti_params
)
# Set the value for the v13 id for this assignment.
assignment.lti_v13_resource_link_id = request.lti_params.v13.get(
"https://purl.imsglobal.org/spec/lti/claim/resource_link", {}
).get("id")

# Keep record of the grading service URL relevant for this assignment if available
assignment.lis_outcome_service_url = request.lti_params.get(
"lis_outcome_service_url"
)

assignment.course_id = course.id
self._update_auto_grading_config(assignment, auto_grading_config)

Expand Down Expand Up @@ -164,16 +174,6 @@ def get_assignment_for_launch(self, request, course: Course) -> Assignment | Non
)
)

# Set the value for the v13 id for this assignment.
assignment.lti_v13_resource_link_id = request.lti_params.v13.get(
"https://purl.imsglobal.org/spec/lti/claim/resource_link", {}
).get("id")

# Keep record of the grading service URL relevant for this assignment if available
assignment.lis_outcome_service_url = request.lti_params.get(
"lis_outcome_service_url"
)

# Always update the assignment configuration
# It often will be the same one while launching the assignment again but
# it might for example be an updated deep linked URL or similar.
Expand Down
8 changes: 0 additions & 8 deletions tests/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,6 @@ def pyramid_request(db_session, application_instance, lti_v11_params):
return pyramid_request


@pytest.fixture
def lti_v13_pyramid_request(pyramid_request, lti_v13_params, lti_v11_params):
pyramid_request.lti_jwt = "JWT"
pyramid_request.lti_params = LTIParams(v11=lti_v11_params, v13=lti_v13_params)

return pyramid_request


@pytest.fixture
def product(pyramid_request):
return pyramid_request.product
Expand Down
19 changes: 11 additions & 8 deletions tests/unit/lms/services/assignment_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ def test_create_assignment(self, svc, db_session):
"resource_link_title, title",
[("", None), (" ", None), (" title ", "title")],
)
@pytest.mark.parametrize("v13_resource_link_id", ["V13_RESOURCE_LINK_ID", None])
def test_update_assignment(
self,
svc,
Expand All @@ -45,8 +46,14 @@ def test_update_assignment(
resource_link_title,
title,
course,
v13_resource_link_id,
):
pyramid_request.lti_params["resource_link_title"] = resource_link_title
pyramid_request.lti_params.v13 = {
"https://purl.imsglobal.org/spec/lti/claim/resource_link": {
"id": v13_resource_link_id
}
}
misc_plugin.is_speed_grader_launch.return_value = is_speed_grader

assignment = svc.update_assignment(
Expand All @@ -60,11 +67,15 @@ def test_update_assignment(
if is_speed_grader:
assert assignment.extra == {}
assert assignment.document_url != sentinel.document_url
assert not assignment.lis_outcome_service_url
assert not assignment.lti_v13_resource_link_id
else:
assert assignment.document_url == sentinel.document_url
assert assignment.extra["group_set_id"] == sentinel.group_set_id
assert assignment.title == title
assert assignment.course_id == course.id
assert assignment.lis_outcome_service_url == "GRADING URL"
assert assignment.lti_v13_resource_link_id == v13_resource_link_id

@pytest.mark.parametrize("with_existing", [True, False])
def test_update_assignment_with_auto_grading_config(
Expand Down Expand Up @@ -193,14 +204,6 @@ def test_get_assignment_for_launch_existing(
)
assert assignment.is_gradable == misc_plugin.is_assignment_gradable.return_value
assert assignment.course_id == course.id
assert assignment.lis_outcome_service_url == "GRADING URL"

def test_get_assignment_for_launch_set_v13_context_id(
self, lti_v13_pyramid_request, svc, course
):
assignment = svc.get_assignment_for_launch(lti_v13_pyramid_request, course)

assert assignment.lti_v13_resource_link_id == "RESOURCE_LINK_ID"

def test_get_assignment_returns_None_with_when_no_document(
self, pyramid_request, svc, misc_plugin, course
Expand Down

0 comments on commit afb5c3a

Please sign in to comment.