Skip to content

Commit

Permalink
Trust information from the launch instead of the DB for is_gradable
Browse files Browse the repository at this point in the history
We use assignment.is_gradable for record keeping but ultimately the
source of truth should be the current launch. If we don't have the
necessary parameters to call the grading APIs, it's not a gradable launch.

The discrepancy of both method for `is_gradable` comes (as far as we know) from SpeedGrader launches
which are always not gradable (!?) and confuse the course and assignment
parameters.

After 387ec0a we stopped recording that
bogus data in the DB. As a result of that assignment are correctly
marked as gradable in the DB but launches by students in the "submission
details" view (which used SpeedGrader launches) fail because they are in
fact, not gradable launches.
  • Loading branch information
marcospri committed May 15, 2024
1 parent 3548f95 commit 7297fdb
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 4 deletions.
2 changes: 1 addition & 1 deletion lms/product/canvas/_plugin/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def post_launch_assignment_hook(self, request, js_config, assignment):
# `lis_result_sourcedid` associates a specific user with an
# assignment.
if (
assignment.is_gradable
self.is_assignment_gradable(lti_params)
and lti_user.is_learner
and lti_params.get("lis_result_sourcedid")
):
Expand Down
21 changes: 18 additions & 3 deletions tests/unit/lms/product/canvas/_plugin/misc_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from unittest.mock import create_autospec, sentinel
from unittest.mock import create_autospec, patch, sentinel

import pytest

Expand All @@ -25,16 +25,18 @@ def test_post_launch_assignment_hook(
is_learner,
grading_id,
focused_user,
is_assignment_gradable,
):
assignment = factories.Assignment(is_gradable=is_gradable)
assignment = factories.Assignment()
pyramid_request.lti_params["lis_result_sourcedid"] = grading_id
pyramid_request.params["focused_user"] = focused_user
is_assignment_gradable.return_value = is_gradable
if is_learner:
request.getfixturevalue("user_is_learner")

plugin.post_launch_assignment_hook(pyramid_request, js_config, assignment)

if assignment.is_gradable and is_learner and grading_id:
if is_gradable and is_learner and grading_id:
js_config.add_canvas_speedgrader_settings.assert_called_once_with(
assignment.document_url
)
Expand Down Expand Up @@ -179,6 +181,14 @@ def test_get_deep_linked_assignment_configuration(
else:
assert "url" not in result

@pytest.mark.parametrize(
"get,expected", [({}, False), ({"learner_canvas_user_id": "ID"}, True)]
)
def test_is_speed_grader_launch(self, get, expected, plugin, pyramid_request):
pyramid_request.GET = get

assert plugin.is_speed_grader_launch(pyramid_request) == expected

def test_factory(self, pyramid_request):
plugin = CanvasMiscPlugin.factory(sentinel.context, pyramid_request)
assert isinstance(plugin, CanvasMiscPlugin)
Expand All @@ -194,3 +204,8 @@ def js_config(self):
@pytest.fixture
def VSBookLocation(self, patch):
return patch("lms.product.canvas._plugin.misc.VSBookLocation")

@pytest.fixture
def is_assignment_gradable(self, plugin):
with patch.object(plugin, "is_assignment_gradable") as is_assignment_gradable:
yield is_assignment_gradable

0 comments on commit 7297fdb

Please sign in to comment.