Guard against Canvas API response with None students

We account for the API returning no "students" key in the response but Canvas can return this key with a value of None.
hypothesis · Feb 6, 2025 · 1d75712 · 1d75712
1 parent 4033c6b
commit 1d75712
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 14 deletions.
diff --git a/lms/services/roster.py b/lms/services/roster.py
@@ -461,7 +461,7 @@ def fetch_canvas_sections_roster(self, lms_course: LMSCourse) -> None:
 
             # We use a set here to avoid any pontential duplicates in the API response.
             student_ids = {
-                str(student["id"]) for student in api_section.get("students", [])
+                str(student["id"]) for student in api_section.get("students") or []
             }
             for student_id in student_ids:
                 db_student = db_course_users_by_lms_api_id.get(student_id)

diff --git a/tests/unit/lms/services/roster_test.py b/tests/unit/lms/services/roster_test.py
@@ -534,12 +534,13 @@ def test_fetch_canvas_sections_roster_with_no_api_sections(
 
         assert "No sections found on the API" in caplog.records[0].message
 
-    @pytest.mark.usefixtures("instructor_in_course", "canvas_section")
+    @pytest.mark.usefixtures("instructor_in_course")
+    @pytest.mark.parametrize("students", [None, []])
     def test_fetch_canvas_sections_roster_with_nothing_to_insert(
-        self, svc, lms_course, caplog, canvas_api_client
+        self, svc, lms_course, caplog, canvas_api_client, students, canvas_section
     ):
         canvas_api_client.course_sections.return_value = [
-            {"id": 10, "name": "Section 1", "students": []}
+            {"id": canvas_section.id, "name": "Section 1", "students": students}
         ]
 
         svc.fetch_canvas_sections_roster(lms_course)
@@ -559,8 +560,6 @@ def test_fetch_canvas_sections_roster(
         lti_v13_application_instance,
         instructor_in_course,
     ):
-        db_session.flush()
-
         canvas_api_client.course_sections.return_value = [
             {
                 "id": int(canvas_section.lms_id),
@@ -607,8 +606,6 @@ def test_fetch_canvas_sections_roster_needing_refresh(
         canvas_section,
         db_session,
     ):
-        db_session.flush()
-
         canvas_api_client.course_sections.side_effect = [
             OAuth2TokenError(refreshable=True),
             [
@@ -637,8 +634,6 @@ def test_fetch_canvas_sections_roster_needing_refresh(
     def test_fetch_canvas_sections_roster_failed_refresh(
         self, svc, lms_course, canvas_api_client, db_session, caplog
     ):
-        db_session.flush()
-
         canvas_api_client.course_sections.side_effect = OAuth2TokenError(
             refreshable=True
         )
@@ -652,8 +647,6 @@ def test_fetch_canvas_sections_roster_failed_refresh(
     def test_fetch_canvas_sections_roster_with_invalid_token(
         self, svc, lms_course, canvas_api_client, db_session, caplog
     ):
-        db_session.flush()
-
         canvas_api_client.course_sections.side_effect = OAuth2TokenError(
             refreshable=False
         )
@@ -696,10 +689,12 @@ def student_in_course(self, lms_course):
         return student
 
     @pytest.fixture
-    def canvas_section(self, lms_course):
-        return factories.LMSSegment(
+    def canvas_section(self, lms_course, db_session):
+        section = factories.LMSSegment(
             type="canvas_section", lms_course=lms_course, lms_id="1"
         )
+        db_session.flush()
+        return section
 
     @pytest.fixture
     def caplog(self, caplog):