feat: improve Canvas grade sync and add test coverage

- Added validation to skip grade sync for assignments past their due date.
- Added missing test suite for Canvas grade sync tasks under `tests/`.
- Minor code cleanup in `api.py` and `client.py`.

Author: xitij2000

src/ol_openedx_canvas_integration/ol_openedx_canvas_integration/client.py

@@ -143,6 +143,7 @@ def get_canvas_assignments(self):
             assignment.get("integration_id"): {
                 "id": assignment["id"],
                 "is_published": assignment.get("published", False),
+                "due_at": assignment.get("due_at"),
             }
             for assignment in assignments
             if assignment.get("integration_id") is not None

src/ol_openedx_canvas_integration/ol_openedx_canvas_integration/tasks.py

@@ -2,11 +2,13 @@
 
 import hashlib
 import logging
+from datetime import UTC, datetime
 from functools import partial
 
 import requests
 from celery import shared_task
 from django.contrib.auth import get_user_model
+from django.utils.dateparse import parse_datetime
 from lms.djangoapps.courseware.courses import get_course_by_id
 from lms.djangoapps.grades.models import PersistentSubsectionGrade
 from lms.djangoapps.instructor_task.api_helper import submit_task
@@ -84,6 +86,13 @@ def push_edx_grades_to_canvas_task(entry_id, xmodule_instance_args):
 
 @shared_task
 def sync_user_grade_with_canvas(grade_id):
+    """
+    Call the Canvas API and update the user's grade.
+    """
+    _sync_user_grade_with_canvas(grade_id)
+
+
+def _sync_user_grade_with_canvas(grade_id):
     """
     Call the Canvas API and update the user's grade.
     """
@@ -109,6 +118,15 @@ def sync_user_grade_with_canvas(grade_id):
             grade_instance.usage_key,
         )
         return
+    due_date = existing_assignments_map[str(grade_instance.usage_key)]["due_at"]
+    if due_date:
+        due_date = parse_datetime(due_date)
+    if due_date and due_date < datetime.now(tz=UTC):
+        TASK_LOG.warning(
+            "The assignment %s is past its due date. Skipping grade sync.",
+            grade_instance.usage_key,
+        )
+        return
 
     canvas_assignment_id = existing_assignments_map[str(grade_instance.usage_key)]["id"]
     openedx_user = USER_MODEL.objects.get(id=grade_instance.user_id)

src/ol_openedx_canvas_integration/pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "ol-openedx-canvas-integration"
-version = "0.6.0"
+version = "0.7.1"
 description = "An Open edX plugin to add canvas integration support"
 authors = [
   {name = "MIT Office of Digital Learning"}
@@ -48,3 +48,12 @@ include = [
 exclude = [
   "ol_openedx_canvas_integration/**/test_*"
 ]
+
+[tool.pytest.ini_options]
+addopts = ["--no-migrations", "--reuse-db"]
+filterwarnings = [
+  "default",
+  "ignore:'cgi' is deprecated and slated for removal in Python 3.13:DeprecationWarning",
+  "ignore:.*pkg_resources.*:DeprecationWarning",
+  "ignore:.*Python 3.13.*:DeprecationWarning",
+]

src/ol_openedx_canvas_integration/tests/__init__.py

@@ -0,0 +1,15 @@
+"""
+Pytest configuration for ol_openedx_canvas_integration tests.
+"""
+
+from django.conf import settings
+
+
+def pytest_configure():
+    """Pytest hook that runs after command line options have been parsed"""
+
+    # Add additional Django settings needed for the plugin tests
+    if not hasattr(settings, "BULK_EMAIL_DEFAULT_RETRY_DELAY"):
+        settings.BULK_EMAIL_DEFAULT_RETRY_DELAY = 10
+    if not hasattr(settings, "BULK_EMAIL_MAX_RETRIES"):
+        settings.BULK_EMAIL_MAX_RETRIES = 5

src/ol_openedx_canvas_integration/tests/conftest.py

@@ -0,0 +1,245 @@
+"""Tests for Canvas integration tasks"""
+
+from datetime import UTC, datetime, timedelta
+from unittest.mock import MagicMock, patch
+
+import requests
+from django.contrib.auth import get_user_model
+from django.test import TestCase, override_settings
+from lms.djangoapps.grades.models import PersistentSubsectionGrade
+from ol_openedx_canvas_integration.tasks import _sync_user_grade_with_canvas
+from opaque_keys.edx.keys import CourseKey, UsageKey
+from openedx.core.djangolib.testing.utils import skip_unless_lms
+
+USER_MODEL = get_user_model()
+
+
+@override_settings(BULK_EMAIL_DEFAULT_RETRY_DELAY=10, BULK_EMAIL_MAX_RETRIES=5)
+@patch("ol_openedx_canvas_integration.tasks.submit_task", MagicMock(return_value=None))
+@patch(
+    "ol_openedx_canvas_integration.tasks.get_course_by_id",
+    MagicMock(return_value=MagicMock()),
+)
+@skip_unless_lms
+class TestSyncUserGradeWithCanvas(TestCase):
+    """Tests for _sync_user_grade_with_canvas task"""
+
+    def setUp(self):
+        """Setup test data"""
+        self.grade_id = 123
+        self.course_id = CourseKey.from_string("course-v1:org+course+run")
+        self.usage_key = UsageKey.from_string(
+            "block-v1:org+course+run+type@sequential+block@subsection"
+        )
+        self.canvas_course_id = "canvas-123"
+        self.email = "student@example.com"
+        self.canvas_user_id = 456
+        self.canvas_assignment_id = 789
+        self.user = USER_MODEL.objects.create_user(
+            username="student",
+            email="student@example.com",
+            password="password",  # pragma: allowlist secret # noqa: S106
+        )
+        self.grade_instance = PersistentSubsectionGrade.update_or_create_grade(
+            user_id=self.user.id,
+            id=self.grade_id,
+            course_id=self.course_id,
+            usage_key=self.usage_key,
+            earned_all=6.0,
+            possible_all=12.0,
+            earned_graded=6.0,
+            possible_graded=8.0,
+            visible_blocks=[],
+            first_attempted=datetime.now(tz=UTC),
+        )
+        # Mock Course
+        self.course = MagicMock()
+
+    @patch("ol_openedx_canvas_integration.tasks.CanvasClient")
+    @patch("ol_openedx_canvas_integration.tasks.get_subsection_user_grades")
+    @patch("ol_openedx_canvas_integration.tasks.TASK_LOG")
+    @patch(
+        "ol_openedx_canvas_integration.tasks.get_canvas_course_id",
+        MagicMock(return_value="canvas-123"),
+    )
+    def test_sync_success(
+        self,
+        mock_task_log,
+        mock_get_grades,
+        mock_client_class,
+    ):
+        """Test successful grade sync to Canvas"""
+        mock_client = mock_client_class.return_value
+        # The graded assignment is linked to canvas
+        mock_client.get_canvas_assignments.return_value = {
+            str(self.usage_key): {
+                "id": self.canvas_assignment_id,
+                "due_at": "",
+            }
+        }
+        # The user is linked to a Canvas user
+        mock_client.get_student_id_by_email.return_value = self.canvas_user_id
+        mock_client.update_assignment_grades.return_value = MagicMock(
+            status_code=requests.codes.ok
+        )
+
+        grade_obj = MagicMock()
+        grade_obj.percent_graded = 0.85
+        mock_get_grades.return_value = {self.usage_key: {self.user: grade_obj}}
+
+        _sync_user_grade_with_canvas(self.grade_id)
+
+        mock_client.update_assignment_grades.assert_called_once()
+
+        mock_task_log.error.assert_not_called()
+
+    @patch("ol_openedx_canvas_integration.tasks.CanvasClient")
+    @patch(
+        "ol_openedx_canvas_integration.tasks.get_canvas_course_id",
+        MagicMock(return_value=None),
+    )
+    def test_no_canvas_id(
+        self,
+        mock_client_class,
+    ):
+        """Test that grades are not synced if course has no linked canvas id"""
+        _sync_user_grade_with_canvas(self.grade_id)
+
+        mock_client_class.assert_not_called()
+
+    @patch("ol_openedx_canvas_integration.tasks.CanvasClient")
+    @patch(
+        "ol_openedx_canvas_integration.tasks.get_canvas_course_id",
+        MagicMock(return_value="canvas-123"),
+    )
+    def test_assignment_not_synced_to_canvas(
+        self,
+        mock_client_class,
+    ):
+        """Test that assignments that are not synced to Canvas are not updated."""
+        mock_client = mock_client_class.return_value
+        # There are assignment synced to Canvas, but not the one being graded
+        mock_client.get_canvas_assignments.return_value = {
+            "dummy-key": {
+                "id": self.canvas_assignment_id,
+                "due_at": "",
+            }
+        }
+
+        _sync_user_grade_with_canvas(self.grade_id)
+
+        mock_client.update_assignment_grades.assert_not_called()
+
+    @patch("ol_openedx_canvas_integration.tasks.CanvasClient")
+    @patch(
+        "ol_openedx_canvas_integration.tasks.get_canvas_course_id",
+        MagicMock(return_value="canvas-123"),
+    )
+    def test_assignment_past_due_date(
+        self,
+        mock_client_class,
+    ):
+        """Test that grades are not synced for assignments past due date"""
+        mock_client = mock_client_class.return_value
+        # Assignment is synced to Canvas, but past due date
+        mock_client.get_canvas_assignments.return_value = {
+            str(self.usage_key): {
+                "id": self.canvas_assignment_id,
+                "due_at": str(datetime.now(tz=UTC) - timedelta(days=1)),
+            }
+        }
+
+        _sync_user_grade_with_canvas(self.grade_id)
+
+        mock_client.update_assignment_grades.assert_not_called()
+
+    @patch("ol_openedx_canvas_integration.tasks.CanvasClient")
+    @patch(
+        "ol_openedx_canvas_integration.tasks.get_canvas_course_id",
+        MagicMock(return_value="canvas-123"),
+    )
+    def test_no_canvas_user_id(
+        self,
+        mock_client_class,
+    ):
+        """Test that grades are not synced if user is not linked to a Canvas user"""
+        mock_client = mock_client_class.return_value
+        # The assignment is linked to Canvas ...
+        mock_client.get_canvas_assignments.return_value = {
+            str(self.usage_key): {
+                "id": self.canvas_assignment_id,
+                "due_at": "",
+            }
+        }
+        # ... but the user is not linked to a Canvas user
+        mock_client.get_student_id_by_email.return_value = None
+
+        _sync_user_grade_with_canvas(self.grade_id)
+
+        mock_client.update_assignment_grades.assert_not_called()
+
+    @patch("ol_openedx_canvas_integration.tasks.CanvasClient")
+    @patch("ol_openedx_canvas_integration.tasks.get_subsection_user_grades")
+    @patch(
+        "ol_openedx_canvas_integration.tasks.get_canvas_course_id",
+        MagicMock(return_value="canvas-123"),
+    )
+    def test_sync_key_error(
+        self,
+        mock_get_grades,
+        mock_client_class,
+    ):
+        """Test that grades are not synced if the usage key is not found in the grades dict"""
+        mock_client = mock_client_class.return_value
+        mock_client.get_canvas_assignments.return_value = {
+            str(self.usage_key): {
+                "id": self.canvas_assignment_id,
+                "due_at": "",
+            }
+        }
+        mock_client.get_student_id_by_email.return_value = self.canvas_user_id
+        mock_client.update_assignment_grades.return_value = MagicMock(
+            status_code=requests.codes.ok
+        )
+
+        grade_obj = MagicMock()
+        grade_obj.percent_graded = 0.85
+        mock_get_grades.return_value = {"dummy-key": {self.user: grade_obj}}
+
+        _sync_user_grade_with_canvas(self.grade_id)
+
+        mock_client.update_assignment_grades.assert_not_called()
+
+    @patch("ol_openedx_canvas_integration.tasks.CanvasClient")
+    @patch("ol_openedx_canvas_integration.tasks.get_subsection_user_grades")
+    @patch("ol_openedx_canvas_integration.tasks.TASK_LOG")
+    @patch(
+        "ol_openedx_canvas_integration.tasks.get_canvas_course_id",
+        MagicMock(return_value="canvas-123"),
+    )
+    def test_sync_fail_code(
+        self,
+        mock_task_log,
+        mock_get_grades,
+        mock_client_class,
+    ):
+        """Test that grades are not synced if the Canvas API returns an error"""
+        mock_client = mock_client_class.return_value
+        mock_client.get_canvas_assignments.return_value = {
+            str(self.usage_key): {
+                "id": self.canvas_assignment_id,
+                "due_at": "",
+            }
+        }
+        mock_client.get_student_id_by_email.return_value = self.canvas_user_id
+        mock_client.update_assignment_grades.return_value = MagicMock(status_code=502)
+
+        grade_obj = MagicMock()
+        grade_obj.percent_graded = 0.85
+        mock_get_grades.return_value = {self.usage_key: {self.user: grade_obj}}
+
+        _sync_user_grade_with_canvas(self.grade_id)
+
+        mock_client.update_assignment_grades.assert_called_once()
+
+        mock_task_log.error.assert_called_once()