Skip to content

Commit 3faba28

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

File tree

1 file changed

+22
-15
lines changed

1 file changed

+22
-15
lines changed

course/models.py

+22-15
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
@@ -935,9 +936,11 @@ 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+
logger.exception("%s: Error %d when getting participants from SIS.", self, exc.response.status_code)
941+
return -1, -1
938942
except Exception:
939-
# pylint: disable-next=logging-fstring-interpolation
940-
logger.exception(f"{self}: Error in getting participants from SIS.")
943+
logger.exception("%s: Error in getting participants from SIS.", self)
941944
return -1, -1
942945

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

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")
960+
# Ignore empty participants list caused by a rare SIS API gateway malfunction
961+
if participants:
962+
# Remove SIS-enrolled students who are not anymore in SIS participants,
963+
# for example, because they have first enrolled in SIS, but then
964+
# unenrolled themselves.
965+
students = self.all_students.filter(enrollment__from_sis=True)
966+
to_remove = students.exclude(student_id__in=participants)
967+
qs = Enrollment.objects.filter(user_profile__in=to_remove, course_instance=self)
968+
qs.update(status=Enrollment.ENROLLMENT_STATUS.REMOVED)
969+
for e in qs:
970+
invalidate_content(Enrollment, e)
971+
delcount += 1
972+
else:
973+
logger.warning("%s: Received an empty participants list from SIS.", self)
974+
return 0, 0
975+
976+
logger.info("%s: enrolled %d, removed %d students based on SIS", self, addcount, delcount)
970977
return addcount, delcount
971978

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

0 commit comments

Comments
 (0)