Skip to content

Commit c39c26b

Browse files
committed
Protect against buggy participants reports from Aalto SISU API
Fixes apluslms#1180
1 parent 8ab057c commit c39c26b

File tree

1 file changed

+24
-16
lines changed

1 file changed

+24
-16
lines changed

course/models.py

+24-16
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
from django.utils.text import format_lazy
2525
from django.utils.translation import gettext_lazy as _
2626
from django_colortag.models import ColorTag
27+
from requests.exceptions import HTTPError
2728

2829
from apps.models import BaseTab, BasePlugin
2930
from authorization.models import JWTAccessible
@@ -915,7 +916,7 @@ def enroll_student(self, user, from_sis=False, use_pending=False):
915916
return True
916917
return False
917918

918-
def enroll_from_sis(self) -> Tuple[int, int]:
919+
def enroll_from_sis(self) -> Tuple[int, int]: # pylint: disable=too-many-locals
919920
"""
920921
Enroll students based on the participants information in Student Info System.
921922
If student has removed herself in SIS, she will also be marked as removed in A+.
@@ -935,9 +936,12 @@ def enroll_from_sis(self) -> Tuple[int, int]:
935936
delcount = 0
936937
try:
937938
participants = sis.get_participants(self.sis_id)
939+
except HTTPError as exc:
940+
code = exc.response.status_code
941+
logger.exception("%s: Error %d when getting participants from SIS.", self, code)
942+
return -1, -1
938943
except Exception:
939-
# pylint: disable-next=logging-fstring-interpolation
940-
logger.exception(f"{self}: Error in getting participants from SIS.")
944+
logger.exception("%s: Error in getting participants from SIS.", self)
941945
return -1, -1
942946

943947
from exercise.models import LearningObject # pylint: disable=import-outside-toplevel
@@ -954,19 +958,23 @@ def enroll_from_sis(self) -> Tuple[int, int]:
954958
# yet logged in to A+, then the user profile does not exist yet.
955959
pass
956960

957-
# Remove SIS-enrolled students who are not anymore in SIS participants,
958-
# for example, because they have first enrolled in SIS, but then
959-
# unenrolled themselves.
960-
students = self.all_students.filter(enrollment__from_sis=True)
961-
to_remove = students.exclude(student_id__in=participants)
962-
qs = Enrollment.objects.filter(user_profile__in=to_remove, course_instance=self)
963-
qs.update(status=Enrollment.ENROLLMENT_STATUS.REMOVED)
964-
for e in qs:
965-
invalidate_content(Enrollment, e)
966-
delcount += 1
967-
968-
# pylint: disable-next=logging-fstring-interpolation
969-
logger.info(f"{self}: enrolled {addcount}, removed {delcount} students based on SIS")
961+
# Ignore empty participants list caused by a rare SIS API gateway malfunction
962+
if participants:
963+
# Remove SIS-enrolled students who are not anymore in SIS participants,
964+
# for example, because they have first enrolled in SIS, but then
965+
# unenrolled themselves.
966+
students = self.all_students.filter(enrollment__from_sis=True)
967+
to_remove = students.exclude(student_id__in=participants)
968+
qs = Enrollment.objects.filter(user_profile__in=to_remove, course_instance=self)
969+
qs.update(status=Enrollment.ENROLLMENT_STATUS.REMOVED)
970+
for e in qs:
971+
invalidate_content(Enrollment, e)
972+
delcount += 1
973+
else:
974+
logger.warning("%s: Received an empty participants list from SIS.", self)
975+
return 0, 0
976+
977+
logger.info("%s: enrolled %d, removed %d students based on SIS", self, addcount, delcount)
970978
return addcount, delcount
971979

972980
def set_users_with_role(self, users, role, remove_others_with_role=False):

0 commit comments

Comments
 (0)