From b5d4127d4ac4bbdc11d4bf3ee0dc4fc701b03a31 Mon Sep 17 00:00:00 2001 From: Joshua Liao Date: Mon, 30 Oct 2023 20:27:35 -0700 Subject: [PATCH 1/4] fix for coord adding nonexistent user --- csm_web/scheduler/models.py | 1 + csm_web/scheduler/views/section.py | 69 +++++++++++++++--------------- 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/csm_web/scheduler/models.py b/csm_web/scheduler/models.py index 9e367ab6..c1779424 100644 --- a/csm_web/scheduler/models.py +++ b/csm_web/scheduler/models.py @@ -60,6 +60,7 @@ def can_enroll_in_course(self, course, bypass_enrollment_time=False): is_associated = ( self.student_set.filter(active=True, section__mentor__course=course).count() or self.mentor_set.filter(section__mentor__course=course).count() + or self.id not in course.coordinator_set.values_list("user", flat=True) ) if bypass_enrollment_time: return not is_associated diff --git a/csm_web/scheduler/views/section.py b/csm_web/scheduler/views/section.py index 1a063140..4b02b9c9 100644 --- a/csm_web/scheduler/views/section.py +++ b/csm_web/scheduler/views/section.py @@ -374,42 +374,43 @@ class RestrictedAction: status=status.HTTP_500_INTERNAL_SERVER_ERROR, ) if student_queryset.count() == 0: - # check if the user can actually enroll in the section - student_user, _ = User.objects.get_or_create( - username=email.split("@")[0], email=email - ) - if ( - student_user.id not in course_coords - and student_user.can_enroll_in_course( - section.mentor.course, bypass_enrollment_time=True - ) - ): - # student does not exist yet; we can always create it - db_actions.append(("create", email)) - curstatus["status"] = Status.OK - else: - # user can't enroll; give details on the reason why - curstatus["status"] = Status.CONFLICT - if not student_user.is_whitelisted_for(section.mentor.course): - if ( - email_obj.get("restricted_action") - == RestrictedAction.WHITELIST - ): - db_actions.append(("create", email)) - curstatus["status"] = Status.OK - else: - any_invalid = True - curstatus["status"] = Status.RESTRICTED + # There are no students in the course with this email. + # Check if student exists. + try: + student_user = User.objects.get(email=email) + # Check if the student is associated with the course. + if student_user.id in course_coords: + curstatus["status"] = Status.CONFLICT + curstatus["detail"] = {"reason": "coordinator"} + elif student_user.mentor_set.filter( + course=section.mentor.course + ).exists(): + curstatus["status"] = Status.CONFLICT + curstatus["detail"] = {"reason": "mentor"} + # Check if the course is available. + elif ( + not student_user.is_whitelisted_for(section.mentor.course) + and not email_obj.get("restricted_action") + == RestrictedAction.WHITELIST + ): + any_invalid = True + curstatus["status"] = Status.RESTRICTED + # Everything is okay: enroll student. + else: + db_actions.append(("enroll", student)) + curstatus["status"] = Status.OK + except User.DoesNotExist: + # If the course is not restricted, or if they are allowed to whitelist, allow. + if ( + not section.mentor.course.is_restricted + or email_obj.get("restricted_action") + == RestrictedAction.WHITELIST + ): + db_actions.append(("create", email)) + curstatus["status"] = Status.OK else: any_invalid = True - reason = "other" - if student_user.id in course_coords: - reason = "coordinator" - elif student_user.mentor_set.filter( - course=section.mentor.course - ).exists(): - reason = "mentor" - curstatus["detail"] = {"reason": reason} + curstatus["status"] = Status.RESTRICTED else: # student_queryset.count() == 1 student = student_queryset.get() From 83ba04dc40d5cb57ee64ca3691f077ee66d6fcc2 Mon Sep 17 00:00:00 2001 From: Joshua Liao Date: Mon, 6 Nov 2023 19:26:09 -0800 Subject: [PATCH 2/4] bool bugfix in association --- csm_web/scheduler/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/csm_web/scheduler/models.py b/csm_web/scheduler/models.py index c1779424..8722694a 100644 --- a/csm_web/scheduler/models.py +++ b/csm_web/scheduler/models.py @@ -60,7 +60,7 @@ def can_enroll_in_course(self, course, bypass_enrollment_time=False): is_associated = ( self.student_set.filter(active=True, section__mentor__course=course).count() or self.mentor_set.filter(section__mentor__course=course).count() - or self.id not in course.coordinator_set.values_list("user", flat=True) + or self.id in course.coordinator_set.values_list("user", flat=True) ) if bypass_enrollment_time: return not is_associated From d9a2dad4addc3b79254ece668ee029a28ba142de Mon Sep 17 00:00:00 2001 From: Joshua Liao Date: Mon, 27 Nov 2023 20:10:31 -0800 Subject: [PATCH 3/4] user vs student fix --- csm_web/scheduler/models.py | 13 ++-- csm_web/scheduler/views/section.py | 101 ++++++++++++++--------------- 2 files changed, 51 insertions(+), 63 deletions(-) diff --git a/csm_web/scheduler/models.py b/csm_web/scheduler/models.py index 8722694a..cca8cb44 100644 --- a/csm_web/scheduler/models.py +++ b/csm_web/scheduler/models.py @@ -60,7 +60,6 @@ def can_enroll_in_course(self, course, bypass_enrollment_time=False): is_associated = ( self.student_set.filter(active=True, section__mentor__course=course).count() or self.mentor_set.filter(section__mentor__course=course).count() - or self.id in course.coordinator_set.values_list("user", flat=True) ) if bypass_enrollment_time: return not is_associated @@ -261,19 +260,15 @@ def save(self, *args, **kwargs): ): if settings.DJANGO_ENV != settings.DEVELOPMENT: logger.info( - ( - " SO automatically created for student" - " %s in course %s for date %s" - ), + " SO automatically created for student" + " %s in course %s for date %s", self.user.email, course.name, now.date(), ) logger.info( - ( - " Attendance automatically created for student" - " %s in course %s for date %s" - ), + " Attendance automatically created for student" + " %s in course %s for date %s", self.user.email, course.name, now.date(), diff --git a/csm_web/scheduler/views/section.py b/csm_web/scheduler/views/section.py index 4b02b9c9..3117e86a 100644 --- a/csm_web/scheduler/views/section.py +++ b/csm_web/scheduler/views/section.py @@ -356,10 +356,8 @@ class RestrictedAction: if student_queryset.count() > 1: # something bad happened, return immediately with error logger.error( - ( - " Multiple student objects exist in the" - " database (Students %s)!" - ), + " Multiple student objects exist in the" + " database (Students %s)!", student_queryset.all(), ) return Response( @@ -377,40 +375,47 @@ class RestrictedAction: # There are no students in the course with this email. # Check if student exists. try: - student_user = User.objects.get(email=email) + user = User.objects.get(email=email) # Check if the student is associated with the course. - if student_user.id in course_coords: - curstatus["status"] = Status.CONFLICT - curstatus["detail"] = {"reason": "coordinator"} - elif student_user.mentor_set.filter( - course=section.mentor.course - ).exists(): - curstatus["status"] = Status.CONFLICT - curstatus["detail"] = {"reason": "mentor"} - # Check if the course is available. - elif ( - not student_user.is_whitelisted_for(section.mentor.course) - and not email_obj.get("restricted_action") - == RestrictedAction.WHITELIST + if ( + user.id not in course_coords + and user.can_enroll_in_course( + section.mentor.course, bypass_enrollment_time=True + ) + or ( + not user.is_whitelisted_for(section.mentor.course) + and email_obj.get("restricted_action") + == RestrictedAction.WHITELIST + ) ): - any_invalid = True - curstatus["status"] = Status.RESTRICTED - # Everything is okay: enroll student. - else: - db_actions.append(("enroll", student)) + db_actions.append("create", email) curstatus["status"] = Status.OK + else: + any_invalid = True + reason = "other" + if not user.is_whitelisted_for(section.mentor.course): + curstatus["status"] = Status.RESTRICTED + reason = "restricted" + elif user.id in course_coords: + curstatus["status"] = Status.CONFLICT + reason = "coordinator" + elif user.mentor_set.filter( + course=section.mentor.course + ).exists(): + reason = "mentor" + curstatus["detail"] = {"reason": reason} except User.DoesNotExist: - # If the course is not restricted, or if they are allowed to whitelist, allow. - if ( - not section.mentor.course.is_restricted - or email_obj.get("restricted_action") - == RestrictedAction.WHITELIST + # Create user. If they would be allowed to enroll, also create student. + User.objects.create(username=email.split("@")[0], email=email) + if user.can_enroll_in_course( + section.mentor.course, bypass_enrollment_time=True ): - db_actions.append(("create", email)) + db_actions.append("create", email) curstatus["status"] = Status.OK else: any_invalid = True curstatus["status"] = Status.RESTRICTED + curstatus["detail"] = {"reason": "new users restricted"} else: # student_queryset.count() == 1 student = student_queryset.get() @@ -537,10 +542,8 @@ class RestrictedAction: ) student.save() logger.info( - ( - " User %s swapped into Section %s from" - " Section %s" - ), + " User %s swapped into Section %s from" + " Section %s", log_str(student.user), log_str(section), log_str(old_section), @@ -565,26 +568,20 @@ def _student_add(self, request, section): """ if not request.user.can_enroll_in_course(section.mentor.course): logger.warning( - ( - " User %s was unable to enroll in Section %s" - " because they are already involved in this course" - ), + " User %s was unable to enroll in Section %s" + " because they are already involved in this course", log_str(request.user), log_str(section), ) raise PermissionDenied( - ( - "You are already either mentoring for this course or enrolled in a" - " section, or the course is closed for enrollment" - ), + "You are already either mentoring for this course or enrolled in a" + " section, or the course is closed for enrollment", status.HTTP_422_UNPROCESSABLE_ENTITY, ) if section.current_student_count >= section.capacity: logger.warning( - ( - " User %s was unable to enroll in Section %s" - " because it was full" - ), + " User %s was unable to enroll in Section %s" + " because it was full", log_str(request.user), log_str(section), ) @@ -597,18 +594,14 @@ def _student_add(self, request, section): ) if student_queryset.count() > 1: logger.error( - ( - " Multiple student objects exist in the" - " database (Students %s)!" - ), + " Multiple student objects exist in the" + " database (Students %s)!", student_queryset.all(), ) return PermissionDenied( - ( - "An internal error occurred; email mentors@berkeley.edu" - " immediately. (Duplicate students exist in the database (Students" - f" {student_queryset.all()}))" - ), + "An internal error occurred; email mentors@berkeley.edu" + " immediately. (Duplicate students exist in the database (Students" + f" {student_queryset.all()}))", code=status.HTTP_500_INTERNAL_SERVER_ERROR, ) if student_queryset.count() == 1: From b8c57213209b39a032d239cd2a494cf2a6488836 Mon Sep 17 00:00:00 2001 From: Joshua Liao Date: Mon, 27 Nov 2023 20:26:09 -0800 Subject: [PATCH 4/4] fix status message --- csm_web/scheduler/views/section.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/csm_web/scheduler/views/section.py b/csm_web/scheduler/views/section.py index 3117e86a..ceecce1a 100644 --- a/csm_web/scheduler/views/section.py +++ b/csm_web/scheduler/views/section.py @@ -373,7 +373,7 @@ class RestrictedAction: ) if student_queryset.count() == 0: # There are no students in the course with this email. - # Check if student exists. + # Check if user exists. try: user = User.objects.get(email=email) # Check if the student is associated with the course. @@ -392,18 +392,17 @@ class RestrictedAction: curstatus["status"] = Status.OK else: any_invalid = True - reason = "other" + curstatus["status"] = Status.CONFLICT if not user.is_whitelisted_for(section.mentor.course): curstatus["status"] = Status.RESTRICTED - reason = "restricted" elif user.id in course_coords: - curstatus["status"] = Status.CONFLICT - reason = "coordinator" + curstatus["detail"] = {"reason": "coordinator"} elif user.mentor_set.filter( course=section.mentor.course ).exists(): - reason = "mentor" - curstatus["detail"] = {"reason": reason} + curstatus["detail"] = {"reason": "mentor"} + else: + curstatus["detail"] = {"reason": "other"} except User.DoesNotExist: # Create user. If they would be allowed to enroll, also create student. User.objects.create(username=email.split("@")[0], email=email)