Skip to content

Commit

Permalink
Capture "not in course" errors while sending Canvas submission
Browse files Browse the repository at this point in the history
These occur while setting a participation date limit in the course.

While this commit doesn't really solve the issue for students it:

- Generates an error message that at least mentions "submissions". The
  previous error was a generic one generated by an unhandled exceptions.

- Records an event in the event table pointing to the student /
  assignment / course
  • Loading branch information
marcospri committed May 22, 2024
1 parent 56e6f94 commit 0997cd8
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 25 deletions.
1 change: 1 addition & 0 deletions lms/error_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ class ErrorCode(str, Enum):
"vitalsource_student_pay_license_launch_instructor"
)
REUSED_CONSUMER_KEY = "reused_consumer_key"
CANVAS_SUBMISSION_COURSE_NOT_AVAILABLE = "canvas_submission_course_not_available"
40 changes: 30 additions & 10 deletions lms/views/api/grading.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import datetime
import logging
import re
from datetime import timezone

from pyramid.view import view_config, view_defaults

from lms.error_code import ErrorCode
from lms.events import LTIEvent
from lms.security import Permissions
from lms.services import LTIGradingService
from lms.services.exceptions import ExternalRequestError, SerializableError
from lms.validation import (
APIReadResultSchema,
APIRecordResultSchema,
Expand Down Expand Up @@ -101,18 +104,35 @@ def record_canvas_speedgrader_submission(self):
# theory require a grade).

lis_result_sourcedid = self.parsed_params["lis_result_sourcedid"]

# If we already have a score, then we've already recorded this info
if self.lti_grading_service.read_result(lis_result_sourcedid).score:
LOG.debug(
"Grade already present, not recording submission. User ID: %s",
self.request.user.id,
try:
# If we already have a score, then we've already recorded this info
if self.lti_grading_service.read_result(lis_result_sourcedid).score:
LOG.debug(
"Grade already present, not recording submission. User ID: %s",
self.request.user.id,
)
return None

self.lti_grading_service.record_result(
lis_result_sourcedid, pre_record_hook=CanvasPreRecordHook(self.request)
)
return None

self.lti_grading_service.record_result(
lis_result_sourcedid, pre_record_hook=CanvasPreRecordHook(self.request)
)
except ExternalRequestError as err:
# This is Canvas only, we do the error handling here instead of the view to avoid
# conflicting different, more general use cases.
# This is what we get with the Course Participation end date has passed.
if re.search(
# LTI1.1 and LTI 1.3 strings respectively.
# The LTI1.3 can happen in both reading and record operations.
r"Course not available for student|This course has concluded",
getattr(getattr(err, "response", None), "text", ""),
):
raise SerializableError(
error_code=ErrorCode.CANVAS_SUBMISSION_COURSE_NOT_AVAILABLE
) from err

raise err

self.request.registry.notify(
LTIEvent.from_request(request=self.request, type_=LTIEvent.Type.SUBMISSION)
)
Expand Down
63 changes: 48 additions & 15 deletions tests/unit/lms/views/api/grading_test.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import datetime
from datetime import timezone
from unittest.mock import patch, sentinel
from unittest.mock import Mock, patch, sentinel

import pytest
from h_matchers import Any

from lms.services.exceptions import ExternalRequestError, SerializableError
from lms.services.lti_grading.interface import GradingResult
from lms.views.api.grading import CanvasPreRecordHook, GradingViews

Expand All @@ -14,12 +15,21 @@
class TestRecordCanvasSpeedgraderSubmission:
GRADING_ID = "lis_result_sourcedid"

def test_it_passes_correct_params_to_read_current_score(
self, pyramid_request, lti_grading_service
):
def test_it(self, pyramid_request, lti_grading_service, LTIEvent):
lti_grading_service.read_result.return_value = GradingResult(
score=None, comment=None
)

GradingViews(pyramid_request).record_canvas_speedgrader_submission()

lti_grading_service.read_result.assert_called_once_with(self.GRADING_ID)
lti_grading_service.record_result.assert_called_once_with(
self.GRADING_ID,
pre_record_hook=Any.instance_of(CanvasPreRecordHook),
)
LTIEvent.from_request.assert_called_once_with(
request=pyramid_request, type_=LTIEvent.Type.SUBMISSION
)

def test_it_does_not_record_result_if_score_already_exists(
self, pyramid_request, lti_grading_service
Expand All @@ -32,24 +42,47 @@ def test_it_does_not_record_result_if_score_already_exists(

lti_grading_service.record_result.assert_not_called()

def test_it_passes_the_callback_if_there_is_no_score(
self, pyramid_request, lti_grading_service, LTIEvent
@pytest.mark.parametrize(
"error_message",
["Course not available for students", "This course has concluded"],
)
def test_it_course_has_concluded_reading(
self, pyramid_request, lti_grading_service, error_message
):
lti_grading_service.read_result.side_effect = ExternalRequestError(
response=Mock(text=error_message)
)

with pytest.raises(SerializableError):
GradingViews(pyramid_request).record_canvas_speedgrader_submission()

@pytest.mark.parametrize(
"error_message",
["Course not available for students", "This course has concluded"],
)
def test_it_course_has_concluded_recording(
self, pyramid_request, lti_grading_service, error_message
):
lti_grading_service.read_result.return_value = GradingResult(
score=None, comment=None
)
lti_grading_service.record_result.side_effect = ExternalRequestError(
response=Mock(text=error_message)
)

GradingViews(pyramid_request).record_canvas_speedgrader_submission()
with pytest.raises(SerializableError):
GradingViews(pyramid_request).record_canvas_speedgrader_submission()

lti_grading_service.record_result.assert_called_once_with(
self.GRADING_ID,
pre_record_hook=Any.instance_of(CanvasPreRecordHook),
# lti_launch_url=expected_launch_url,
# submitted_at=datetime.datetime(2001, 1, 1, tzinfo=timezone.utc),
)
LTIEvent.from_request.assert_called_once_with(
request=pyramid_request, type_=LTIEvent.Type.SUBMISSION
def test_it_raises_unhandled_external_request_error(
self, pyramid_request, lti_grading_service
):
lti_grading_service.read_result.return_value = GradingResult(
score=None, comment=None
)
lti_grading_service.record_result.side_effect = ExternalRequestError()

with pytest.raises(ExternalRequestError):
GradingViews(pyramid_request).record_canvas_speedgrader_submission()

@pytest.fixture
def pyramid_request(self, pyramid_request):
Expand Down

0 comments on commit 0997cd8

Please sign in to comment.