Skip to content

Commit c0190b6

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 8309878 commit c0190b6

6 files changed

Lines changed: 245 additions & 6 deletions

File tree

src/ol_openedx_canvas_integration/ol_openedx_canvas_integration/api.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -265,9 +265,7 @@ def push_edx_grades_to_canvas(course):
265265
# Send requests to update grades in each relevant course
266266
assignment_grades_updated = {
267267
subsection_block: client.update_assignment_grades(
268-
canvas_assignment_id=existing_assignment_dict[
269-
str(subsection_block.location)
270-
]["id"],
268+
canvas_assignment_id=existing_assignment_dict[str(subsection_block.location)]["id"],
271269
payload=grade_request_payload,
272270
)
273271
for subsection_block, grade_request_payload in grade_update_payloads.items()

src/ol_openedx_canvas_integration/ol_openedx_canvas_integration/client.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,4 +297,4 @@ def update_grade_payload_kv(user_id, grade_percent):
297297
Returns:
298298
(tuple): A key/value pair that will be used in the body of a bulk grade update request
299299
""" # noqa: D401, E501
300-
return (f"grade_data[{user_id}][posted_grade]", f"{grade_percent * 100}%")
300+
return f"grade_data[{user_id}][posted_grade]", f"{grade_percent * 100}%"

src/ol_openedx_canvas_integration/ol_openedx_canvas_integration/tasks.py

Lines changed: 16 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 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,12 @@ 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+
def _sync_user_grade_with_canvas(grade_id):
8795
"""
8896
Call the Canvas API and update the user's grade.
8997
"""
@@ -110,6 +118,14 @@ def sync_user_grade_with_canvas(grade_id):
110118
)
111119
return
112120

121+
due_date = parse_datetime(existing_assignments_map[str(grade_instance.usage_key)]["due_at"])
122+
if due_date and due_date < datetime.now():
123+
TASK_LOG.warning(
124+
"The assignment %s is past its due date. Skipping grade sync.",
125+
grade_instance.usage_key,
126+
)
127+
return
128+
113129
canvas_assignment_id = existing_assignments_map[str(grade_instance.usage_key)]["id"]
114130
openedx_user = USER_MODEL.objects.get(id=grade_instance.user_id)
115131
canvas_user_id = client.get_student_id_by_email(openedx_user.email)

src/ol_openedx_canvas_integration/pyproject.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ exclude = [
4949
"ol_openedx_canvas_integration/**/test_*"
5050
]
5151

52-
[tool.pytest]
53-
addopts = ["--nomigrations", "--reuse-db"]
52+
[tool.pytest.ini_options]
53+
addopts = ["--no-migrations", "--reuse-db"]
5454
filterwarnings = [
5555
"default",
5656
"ignore:'cgi' is deprecated and slated for removal in Python 3.13:DeprecationWarning",
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
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
16+
17+
Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
"""Tests for Canvas integration tasks"""
2+
3+
from datetime import 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 override_settings, TestCase
9+
from opaque_keys.edx.keys import UsageKey, CourseKey
10+
11+
from ol_openedx_canvas_integration.tasks import _sync_user_grade_with_canvas
12+
from lms.djangoapps.grades.models import PersistentSubsectionGrade
13+
from openedx.core.djangolib.testing.utils import skip_unless_lms
14+
USER_MODEL = get_user_model()
15+
16+
@override_settings(BULK_EMAIL_DEFAULT_RETRY_DELAY=10, BULK_EMAIL_MAX_RETRIES=5)
17+
@patch('ol_openedx_canvas_integration.tasks.submit_task', MagicMock(return_value=None))
18+
@patch("ol_openedx_canvas_integration.tasks.get_course_by_id", MagicMock(return_value=MagicMock()))
19+
@skip_unless_lms
20+
class TestSyncUserGradeWithCanvas(TestCase):
21+
"""Tests for _sync_user_grade_with_canvas task"""
22+
23+
def setUp(self):
24+
"""Setup test data"""
25+
self.grade_id = 123
26+
self.course_id = CourseKey.from_string("course-v1:org+course+run")
27+
self.usage_key = UsageKey.from_string("block-v1:org+course+run+type@sequential+block@subsection")
28+
self.canvas_course_id = "canvas-123"
29+
self.email = "student@example.com"
30+
self.canvas_user_id = 456
31+
self.canvas_assignment_id = 789
32+
self.user = USER_MODEL.objects.create_user(username="student", email="student@example.com", password="password")
33+
self.grade_instance = PersistentSubsectionGrade.update_or_create_grade(
34+
user_id=self.user.id,
35+
id=self.grade_id,
36+
course_id=self.course_id,
37+
usage_key=self.usage_key,
38+
earned_all=6.0,
39+
possible_all=12.0,
40+
earned_graded=6.0,
41+
possible_graded=8.0,
42+
visible_blocks=[],
43+
first_attempted=datetime.now(),
44+
)
45+
# Mock Course
46+
self.course = MagicMock()
47+
48+
49+
@patch("ol_openedx_canvas_integration.tasks.CanvasClient")
50+
@patch("ol_openedx_canvas_integration.tasks.get_subsection_user_grades")
51+
@patch("ol_openedx_canvas_integration.tasks.TASK_LOG")
52+
@patch("ol_openedx_canvas_integration.tasks.get_canvas_course_id", MagicMock(return_value="canvas-123"))
53+
def test_sync_success(
54+
self,
55+
mock_task_log,
56+
mock_get_grades,
57+
mock_client_class,
58+
):
59+
"""Test successful grade sync to Canvas"""
60+
mock_client = mock_client_class.return_value
61+
mock_client.get_canvas_assignments.return_value = {
62+
str(self.usage_key): {
63+
"id": self.canvas_assignment_id,
64+
"due_at": "",
65+
}
66+
}
67+
mock_client.get_student_id_by_email.return_value = self.canvas_user_id
68+
mock_client.update_assignment_grades.return_value = MagicMock(
69+
status_code=requests.codes.ok
70+
)
71+
72+
grade_obj = MagicMock()
73+
grade_obj.percent_graded = 0.85
74+
mock_get_grades.return_value = {self.usage_key: {self.user: grade_obj}}
75+
76+
_sync_user_grade_with_canvas(self.grade_id)
77+
78+
mock_client.update_assignment_grades.assert_called_once()
79+
80+
mock_task_log.error.assert_not_called()
81+
82+
@patch("ol_openedx_canvas_integration.tasks.CanvasClient")
83+
@patch("ol_openedx_canvas_integration.tasks.get_canvas_course_id", MagicMock(return_value=None))
84+
def test_no_canvas_id(
85+
self,
86+
mock_client_class,
87+
):
88+
89+
_sync_user_grade_with_canvas(self.grade_id)
90+
91+
mock_client_class.assert_not_called()
92+
93+
@patch("ol_openedx_canvas_integration.tasks.CanvasClient")
94+
@patch("ol_openedx_canvas_integration.tasks.get_canvas_course_id", MagicMock(return_value="canvas-123"))
95+
def test_assignment_not_synced(
96+
self,
97+
mock_client_class,
98+
):
99+
mock_client = mock_client_class.return_value
100+
mock_client.get_canvas_assignments.return_value = {
101+
"dummy-key": {
102+
"id": self.canvas_assignment_id,
103+
"due_at": "",
104+
}
105+
}
106+
107+
_sync_user_grade_with_canvas(self.grade_id)
108+
109+
mock_client.update_assignment_grades.assert_not_called()
110+
111+
@patch("ol_openedx_canvas_integration.tasks.CanvasClient")
112+
@patch("ol_openedx_canvas_integration.tasks.get_canvas_course_id", MagicMock(return_value="canvas-123"))
113+
def test_assignment_past_due_date(
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": str(datetime.now() - timedelta(days=1)),
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("ol_openedx_canvas_integration.tasks.get_canvas_course_id", MagicMock(return_value="canvas-123"))
131+
def test_no_canvas_user_id(
132+
self,
133+
mock_client_class,
134+
):
135+
mock_client = mock_client_class.return_value
136+
mock_client.get_canvas_assignments.return_value = {
137+
str(self.usage_key): {
138+
"id": self.canvas_assignment_id,
139+
"due_at": "",
140+
}
141+
}
142+
mock_client.get_student_id_by_email.return_value = None
143+
144+
_sync_user_grade_with_canvas(self.grade_id)
145+
146+
mock_client.update_assignment_grades.assert_not_called()
147+
148+
149+
@patch("ol_openedx_canvas_integration.tasks.CanvasClient")
150+
@patch("ol_openedx_canvas_integration.tasks.get_subsection_user_grades")
151+
@patch("ol_openedx_canvas_integration.tasks.get_canvas_course_id", MagicMock(return_value="canvas-123"))
152+
def test_sync_key_error(
153+
self,
154+
mock_get_grades,
155+
mock_client_class,
156+
):
157+
"""Test successful grade sync to Canvas"""
158+
mock_client = mock_client_class.return_value
159+
mock_client.get_canvas_assignments.return_value = {
160+
str(self.usage_key): {
161+
"id": self.canvas_assignment_id,
162+
"due_at": "",
163+
}
164+
}
165+
mock_client.get_student_id_by_email.return_value = self.canvas_user_id
166+
mock_client.update_assignment_grades.return_value = MagicMock(
167+
status_code=requests.codes.ok
168+
)
169+
170+
grade_obj = MagicMock()
171+
grade_obj.percent_graded = 0.85
172+
mock_get_grades.return_value = {"dummy-key": {self.user: grade_obj}}
173+
174+
_sync_user_grade_with_canvas(self.grade_id)
175+
176+
mock_client.update_assignment_grades.assert_not_called()
177+
178+
@patch("ol_openedx_canvas_integration.tasks.CanvasClient")
179+
@patch("ol_openedx_canvas_integration.tasks.get_subsection_user_grades")
180+
@patch("ol_openedx_canvas_integration.tasks.TASK_LOG")
181+
@patch("ol_openedx_canvas_integration.tasks.get_canvas_course_id", MagicMock(return_value="canvas-123"))
182+
def test_sync_fail_code(
183+
self,
184+
mock_task_log,
185+
mock_get_grades,
186+
mock_client_class,
187+
):
188+
mock_client = mock_client_class.return_value
189+
mock_client.get_canvas_assignments.return_value = {
190+
str(self.usage_key): {
191+
"id": self.canvas_assignment_id,
192+
"due_at": "",
193+
}
194+
}
195+
mock_client.get_student_id_by_email.return_value = self.canvas_user_id
196+
mock_client.update_assignment_grades.return_value = MagicMock(
197+
status_code=502
198+
)
199+
200+
grade_obj = MagicMock()
201+
grade_obj.percent_graded = 0.85
202+
mock_get_grades.return_value = {self.usage_key: {self.user: grade_obj}}
203+
204+
_sync_user_grade_with_canvas(self.grade_id)
205+
206+
mock_client.update_assignment_grades.assert_called_once()
207+
208+
mock_task_log.error.assert_called_once()

0 commit comments

Comments
 (0)