Skip to content

Commit

Permalink
Protect against buggy participants reports from Aalto SISU API
Browse files Browse the repository at this point in the history
Fixes #1180
  • Loading branch information
ihalaij1 authored and markkuriekkinen committed May 17, 2023
1 parent 8ab057c commit fd87748
Showing 1 changed file with 22 additions and 15 deletions.
37 changes: 22 additions & 15 deletions course/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from django.utils.text import format_lazy
from django.utils.translation import gettext_lazy as _
from django_colortag.models import ColorTag
from requests.exceptions import HTTPError

from apps.models import BaseTab, BasePlugin
from authorization.models import JWTAccessible
Expand Down Expand Up @@ -935,9 +936,11 @@ def enroll_from_sis(self) -> Tuple[int, int]:
delcount = 0
try:
participants = sis.get_participants(self.sis_id)
except HTTPError as exc:
logger.exception("%s: Error %d when getting participants from SIS.", self, exc.response.status_code)
return -1, -1
except Exception:
# pylint: disable-next=logging-fstring-interpolation
logger.exception(f"{self}: Error in getting participants from SIS.")
logger.exception("%s: Error in getting participants from SIS.", self)
return -1, -1

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

# Remove SIS-enrolled students who are not anymore in SIS participants,
# for example, because they have first enrolled in SIS, but then
# unenrolled themselves.
students = self.all_students.filter(enrollment__from_sis=True)
to_remove = students.exclude(student_id__in=participants)
qs = Enrollment.objects.filter(user_profile__in=to_remove, course_instance=self)
qs.update(status=Enrollment.ENROLLMENT_STATUS.REMOVED)
for e in qs:
invalidate_content(Enrollment, e)
delcount += 1

# pylint: disable-next=logging-fstring-interpolation
logger.info(f"{self}: enrolled {addcount}, removed {delcount} students based on SIS")
# Ignore empty participants list caused by a rare SIS API gateway malfunction
if participants:
# Remove SIS-enrolled students who are not anymore in SIS participants,
# for example, because they have first enrolled in SIS, but then
# unenrolled themselves.
students = self.all_students.filter(enrollment__from_sis=True)
to_remove = students.exclude(student_id__in=participants)
qs = Enrollment.objects.filter(user_profile__in=to_remove, course_instance=self)
qs.update(status=Enrollment.ENROLLMENT_STATUS.REMOVED)
for e in qs:
invalidate_content(Enrollment, e)
delcount += 1
else:
logger.warning("%s: Received an empty participants list from SIS.", self)
return 0, 0

logger.info("%s: enrolled %d, removed %d students based on SIS", self, addcount, delcount)
return addcount, delcount

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

0 comments on commit fd87748

Please sign in to comment.