Skip to content

Commit 19d42f7

Browse files
committed
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`.
1 parent 50f59f0 commit 19d42f7

6 files changed

Lines changed: 279 additions & 1 deletion

File tree

src/ol_openedx_canvas_integration/ol_openedx_canvas_integration/client.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ def get_canvas_assignments(self):
143143
assignment.get("integration_id"): {
144144
"id": assignment["id"],
145145
"is_published": assignment.get("published", False),
146+
"due_at": assignment.get("due_at"),
146147
}
147148
for assignment in assignments
148149
if assignment.get("integration_id") is not None

src/ol_openedx_canvas_integration/ol_openedx_canvas_integration/tasks.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22

33
import hashlib
44
import logging
5+
from datetime import UTC, datetime
56
from functools import partial
67

78
import requests
89
from celery import shared_task
910
from django.contrib.auth import get_user_model
11+
from django.utils.dateparse import parse_datetime
1012
from lms.djangoapps.courseware.courses import get_course_by_id
1113
from lms.djangoapps.grades.models import PersistentSubsectionGrade
1214
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):
8486

8587
@shared_task
8688
def sync_user_grade_with_canvas(grade_id):
89+
"""
90+
Call the Canvas API and update the user's grade.
91+
"""
92+
_sync_user_grade_with_canvas(grade_id)
93+
94+
95+
def _sync_user_grade_with_canvas(grade_id):
8796
"""
8897
Call the Canvas API and update the user's grade.
8998
"""
@@ -109,6 +118,15 @@ def sync_user_grade_with_canvas(grade_id):
109118
grade_instance.usage_key,
110119
)
111120
return
121+
due_date = existing_assignments_map[str(grade_instance.usage_key)]["due_at"]
122+
if due_date:
123+
due_date = parse_datetime(due_date)
124+
if due_date and due_date < datetime.now(tz=UTC):
125+
TASK_LOG.warning(
126+
"The assignment %s is past its due date. Skipping grade sync.",
127+
grade_instance.usage_key,
128+
)
129+
return
112130

113131
canvas_assignment_id = existing_assignments_map[str(grade_instance.usage_key)]["id"]
114132
openedx_user = USER_MODEL.objects.get(id=grade_instance.user_id)

src/ol_openedx_canvas_integration/pyproject.toml

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[project]
22
name = "ol-openedx-canvas-integration"
3-
version = "0.6.0"
3+
version = "0.7.1"
44
description = "An Open edX plugin to add canvas integration support"
55
authors = [
66
{name = "MIT Office of Digital Learning"}
@@ -48,3 +48,12 @@ include = [
4848
exclude = [
4949
"ol_openedx_canvas_integration/**/test_*"
5050
]
51+
52+
[tool.pytest.ini_options]
53+
addopts = ["--no-migrations", "--reuse-db"]
54+
filterwarnings = [
55+
"default",
56+
"ignore:'cgi' is deprecated and slated for removal in Python 3.13:DeprecationWarning",
57+
"ignore:.*pkg_resources.*:DeprecationWarning",
58+
"ignore:.*Python 3.13.*:DeprecationWarning",
59+
]

src/ol_openedx_canvas_integration/tests/__init__.py

Whitespace-only changes.
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
"""
2+
Pytest configuration for ol_openedx_canvas_integration tests.
3+
"""
4+
5+
from django.conf import settings
6+
7+
8+
def pytest_configure():
9+
"""Pytest hook that runs after command line options have been parsed"""
10+
11+
# Add additional Django settings needed for the plugin tests
12+
if not hasattr(settings, "BULK_EMAIL_DEFAULT_RETRY_DELAY"):
13+
settings.BULK_EMAIL_DEFAULT_RETRY_DELAY = 10
14+
if not hasattr(settings, "BULK_EMAIL_MAX_RETRIES"):
15+
settings.BULK_EMAIL_MAX_RETRIES = 5
Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
"""Tests for Canvas integration tasks"""
2+
3+
from datetime import UTC, datetime, timedelta
4+
from unittest.mock import MagicMock, patch
5+
6+
import requests
7+
from django.contrib.auth import get_user_model
8+
from django.test import TestCase, override_settings
9+
from lms.djangoapps.grades.models import PersistentSubsectionGrade
10+
from ol_openedx_canvas_integration.tasks import _sync_user_grade_with_canvas
11+
from opaque_keys.edx.keys import CourseKey, UsageKey
12+
from openedx.core.djangolib.testing.utils import skip_unless_lms
13+
14+
USER_MODEL = get_user_model()
15+
16+
17+
@override_settings(BULK_EMAIL_DEFAULT_RETRY_DELAY=10, BULK_EMAIL_MAX_RETRIES=5)
18+
@patch("ol_openedx_canvas_integration.tasks.submit_task", MagicMock(return_value=None))
19+
@patch(
20+
"ol_openedx_canvas_integration.tasks.get_course_by_id",
21+
MagicMock(return_value=MagicMock()),
22+
)
23+
@skip_unless_lms
24+
class TestSyncUserGradeWithCanvas(TestCase):
25+
"""Tests for _sync_user_grade_with_canvas task"""
26+
27+
def setUp(self):
28+
"""Setup test data"""
29+
self.grade_id = 123
30+
self.course_id = CourseKey.from_string("course-v1:org+course+run")
31+
self.usage_key = UsageKey.from_string(
32+
"block-v1:org+course+run+type@sequential+block@subsection"
33+
)
34+
self.canvas_course_id = "canvas-123"
35+
self.email = "student@example.com"
36+
self.canvas_user_id = 456
37+
self.canvas_assignment_id = 789
38+
self.user = USER_MODEL.objects.create_user(
39+
username="student",
40+
email="student@example.com",
41+
password="password", # pragma: allowlist secret # noqa: S106
42+
)
43+
self.grade_instance = PersistentSubsectionGrade.update_or_create_grade(
44+
user_id=self.user.id,
45+
id=self.grade_id,
46+
course_id=self.course_id,
47+
usage_key=self.usage_key,
48+
earned_all=6.0,
49+
possible_all=12.0,
50+
earned_graded=6.0,
51+
possible_graded=8.0,
52+
visible_blocks=[],
53+
first_attempted=datetime.now(tz=UTC),
54+
)
55+
# Mock Course
56+
self.course = MagicMock()
57+
58+
@patch("ol_openedx_canvas_integration.tasks.CanvasClient")
59+
@patch("ol_openedx_canvas_integration.tasks.get_subsection_user_grades")
60+
@patch("ol_openedx_canvas_integration.tasks.TASK_LOG")
61+
@patch(
62+
"ol_openedx_canvas_integration.tasks.get_canvas_course_id",
63+
MagicMock(return_value="canvas-123"),
64+
)
65+
def test_sync_success(
66+
self,
67+
mock_task_log,
68+
mock_get_grades,
69+
mock_client_class,
70+
):
71+
"""Test successful grade sync to Canvas"""
72+
mock_client = mock_client_class.return_value
73+
mock_client.get_canvas_assignments.return_value = {
74+
str(self.usage_key): {
75+
"id": self.canvas_assignment_id,
76+
"due_at": "",
77+
}
78+
}
79+
mock_client.get_student_id_by_email.return_value = self.canvas_user_id
80+
mock_client.update_assignment_grades.return_value = MagicMock(
81+
status_code=requests.codes.ok
82+
)
83+
84+
grade_obj = MagicMock()
85+
grade_obj.percent_graded = 0.85
86+
mock_get_grades.return_value = {self.usage_key: {self.user: grade_obj}}
87+
88+
_sync_user_grade_with_canvas(self.grade_id)
89+
90+
mock_client.update_assignment_grades.assert_called_once()
91+
92+
mock_task_log.error.assert_not_called()
93+
94+
@patch("ol_openedx_canvas_integration.tasks.CanvasClient")
95+
@patch(
96+
"ol_openedx_canvas_integration.tasks.get_canvas_course_id",
97+
MagicMock(return_value=None),
98+
)
99+
def test_no_canvas_id(
100+
self,
101+
mock_client_class,
102+
):
103+
104+
_sync_user_grade_with_canvas(self.grade_id)
105+
106+
mock_client_class.assert_not_called()
107+
108+
@patch("ol_openedx_canvas_integration.tasks.CanvasClient")
109+
@patch(
110+
"ol_openedx_canvas_integration.tasks.get_canvas_course_id",
111+
MagicMock(return_value="canvas-123"),
112+
)
113+
def test_assignment_not_synced(
114+
self,
115+
mock_client_class,
116+
):
117+
mock_client = mock_client_class.return_value
118+
mock_client.get_canvas_assignments.return_value = {
119+
"dummy-key": {
120+
"id": self.canvas_assignment_id,
121+
"due_at": "",
122+
}
123+
}
124+
125+
_sync_user_grade_with_canvas(self.grade_id)
126+
127+
mock_client.update_assignment_grades.assert_not_called()
128+
129+
@patch("ol_openedx_canvas_integration.tasks.CanvasClient")
130+
@patch(
131+
"ol_openedx_canvas_integration.tasks.get_canvas_course_id",
132+
MagicMock(return_value="canvas-123"),
133+
)
134+
def test_assignment_past_due_date(
135+
self,
136+
mock_client_class,
137+
):
138+
mock_client = mock_client_class.return_value
139+
mock_client.get_canvas_assignments.return_value = {
140+
"dummy-key": {
141+
"id": self.canvas_assignment_id,
142+
"due_at": str(datetime.now(tz=UTC) - timedelta(days=1)),
143+
}
144+
}
145+
146+
_sync_user_grade_with_canvas(self.grade_id)
147+
148+
mock_client.update_assignment_grades.assert_not_called()
149+
150+
@patch("ol_openedx_canvas_integration.tasks.CanvasClient")
151+
@patch(
152+
"ol_openedx_canvas_integration.tasks.get_canvas_course_id",
153+
MagicMock(return_value="canvas-123"),
154+
)
155+
def test_no_canvas_user_id(
156+
self,
157+
mock_client_class,
158+
):
159+
mock_client = mock_client_class.return_value
160+
mock_client.get_canvas_assignments.return_value = {
161+
str(self.usage_key): {
162+
"id": self.canvas_assignment_id,
163+
"due_at": "",
164+
}
165+
}
166+
mock_client.get_student_id_by_email.return_value = None
167+
168+
_sync_user_grade_with_canvas(self.grade_id)
169+
170+
mock_client.update_assignment_grades.assert_not_called()
171+
172+
@patch("ol_openedx_canvas_integration.tasks.CanvasClient")
173+
@patch("ol_openedx_canvas_integration.tasks.get_subsection_user_grades")
174+
@patch(
175+
"ol_openedx_canvas_integration.tasks.get_canvas_course_id",
176+
MagicMock(return_value="canvas-123"),
177+
)
178+
def test_sync_key_error(
179+
self,
180+
mock_get_grades,
181+
mock_client_class,
182+
):
183+
"""Test successful grade sync to Canvas"""
184+
mock_client = mock_client_class.return_value
185+
mock_client.get_canvas_assignments.return_value = {
186+
str(self.usage_key): {
187+
"id": self.canvas_assignment_id,
188+
"due_at": "",
189+
}
190+
}
191+
mock_client.get_student_id_by_email.return_value = self.canvas_user_id
192+
mock_client.update_assignment_grades.return_value = MagicMock(
193+
status_code=requests.codes.ok
194+
)
195+
196+
grade_obj = MagicMock()
197+
grade_obj.percent_graded = 0.85
198+
mock_get_grades.return_value = {"dummy-key": {self.user: grade_obj}}
199+
200+
_sync_user_grade_with_canvas(self.grade_id)
201+
202+
mock_client.update_assignment_grades.assert_not_called()
203+
204+
@patch("ol_openedx_canvas_integration.tasks.CanvasClient")
205+
@patch("ol_openedx_canvas_integration.tasks.get_subsection_user_grades")
206+
@patch("ol_openedx_canvas_integration.tasks.TASK_LOG")
207+
@patch(
208+
"ol_openedx_canvas_integration.tasks.get_canvas_course_id",
209+
MagicMock(return_value="canvas-123"),
210+
)
211+
def test_sync_fail_code(
212+
self,
213+
mock_task_log,
214+
mock_get_grades,
215+
mock_client_class,
216+
):
217+
mock_client = mock_client_class.return_value
218+
mock_client.get_canvas_assignments.return_value = {
219+
str(self.usage_key): {
220+
"id": self.canvas_assignment_id,
221+
"due_at": "",
222+
}
223+
}
224+
mock_client.get_student_id_by_email.return_value = self.canvas_user_id
225+
mock_client.update_assignment_grades.return_value = MagicMock(status_code=502)
226+
227+
grade_obj = MagicMock()
228+
grade_obj.percent_graded = 0.85
229+
mock_get_grades.return_value = {self.usage_key: {self.user: grade_obj}}
230+
231+
_sync_user_grade_with_canvas(self.grade_id)
232+
233+
mock_client.update_assignment_grades.assert_called_once()
234+
235+
mock_task_log.error.assert_called_once()

0 commit comments

Comments
 (0)