From e5e3fcdc1c257f7e3f0253d9002c75ec10c2afb5 Mon Sep 17 00:00:00 2001 From: JJ <34042537+jhan217@users.noreply.github.com> Date: Mon, 9 Nov 2020 10:41:58 -0500 Subject: [PATCH] [REV-1262] Skip enrolling already enrolled users on changes to the user (#25490) Skip auto-enrolling users if they are already enrolled in their auto-enroll enabled courses to prevent downgrading users from paid course modes to audit/free course modes when they activate their account. --- common/djangoapps/student/models.py | 15 ++- .../djangoapps/student/tests/test_models.py | 118 +++++++++++++++++- 2 files changed, 130 insertions(+), 3 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index b62ea89707..4a62a02c54 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -74,9 +74,9 @@ from openedx.core.djangoapps.signals.signals import USER_ACCOUNT_ACTIVATED from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.xmodule_django.models import NoneToEmptyManager from openedx.core.djangolib.model_mixins import DeletableByUserValue +from openedx.core.toggles import ENTRANCE_EXAMS from student.signals import ENROLL_STATUS_CHANGE, ENROLLMENT_TRACK_UPDATED, UNENROLL_DONE from track import contexts, segment -from openedx.core.toggles import ENTRANCE_EXAMS from util.model_utils import emit_field_changed_events, get_changed_fields_dict from util.query import use_read_replica_if_available @@ -740,7 +740,7 @@ def user_post_save_callback(sender, **kwargs): """ When a user is modified and either its `is_active` state or email address is changed, and the user is, in fact, active, then check to see if there - are any courses that it needs to be automatically enrolled in. + are any courses that it needs to be automatically enrolled in and enroll them if needed. Additionally, emit analytics events after saving the User. """ @@ -753,6 +753,17 @@ def user_post_save_callback(sender, **kwargs): ceas = CourseEnrollmentAllowed.for_user(user).filter(auto_enroll=True) for cea in ceas: + # skip enrolling already enrolled users + if CourseEnrollment.is_enrolled(user, cea.course_id): + # Link the CEA to the user if the CEA isn't already linked to the user + # (e.g. the user was invited to a course but hadn't activated the account yet) + # This is to prevent students from changing e-mails and + # enrolling many accounts through the same e-mail. + if not cea.user: + cea.user = user + cea.save() + continue + enrollment = CourseEnrollment.enroll(user, cea.course_id) manual_enrollment_audit = ManualEnrollmentAudit.get_manual_enrollment_by_email(user.email) diff --git a/common/djangoapps/student/tests/test_models.py b/common/djangoapps/student/tests/test_models.py index fd18cbba39..b61822a158 100644 --- a/common/djangoapps/student/tests/test_models.py +++ b/common/djangoapps/student/tests/test_models.py @@ -4,7 +4,7 @@ import hashlib import ddt import factory import pytz -from django.contrib.auth.models import AnonymousUser +from django.contrib.auth.models import AnonymousUser, User from django.core.cache import cache from django.db.models import signals from django.db.models.functions import Lower @@ -403,3 +403,119 @@ class TestAccountRecovery(TestCase): # Assert that there is no longer an AccountRecovery record for this user assert len(AccountRecovery.objects.filter(user_id=user.id)) == 0 + + +@ddt.ddt +class TestUserPostSaveCallback(SharedModuleStoreTestCase): + """ + Tests for the user post save callback. + These tests are to ensure that user activation auto-enrolls invited users into courses without + changing any existing course mode states. + """ + def setUp(self): + super(TestUserPostSaveCallback, self).setUp() + self.course = CourseFactory.create() + + @ddt.data(*(set(CourseMode.ALL_MODES) - set(CourseMode.AUDIT_MODES))) + def test_paid_user_not_downgraded_on_activation(self, mode): + """ + Make sure that students who are already enrolled + have paid do not get downgraded to audit mode + when their account is activated. + """ + # fixture + student = self._set_up_invited_student( + course=self.course, + active=False, + course_mode=mode + ) + + # trigger the post_save callback + student.is_active = True + student.save() + + # reload values from the database + make sure they are in the expected state + actual_course_enrollment = CourseEnrollment.objects.get(user=student, course_id=self.course.id) + actual_student = User.objects.get(email=student.email) + actual_cea = CourseEnrollmentAllowed.objects.get(email=student.email) + + self.assertEqual(actual_course_enrollment.mode, mode) + self.assertEqual(actual_student.is_active, True) + self.assertEqual(actual_cea.user, student) + + def test_not_enrolled_student_is_enrolled(self): + """ + Make sure that invited students who are not enrolled become enrolled when their account is activated. + They should be enrolled in the course in audit mode. + """ + # fixture + student = self._set_up_invited_student( + course=self.course, + active=False, + enrolled=False + ) + + # trigger the post_save callback + student.is_active = True + student.save() + + # reload values from the database + make sure they are in the expected state + actual_course_enrollment = CourseEnrollment.objects.get(user=student, course_id=self.course.id) + actual_student = User.objects.get(email=student.email) + actual_cea = CourseEnrollmentAllowed.objects.get(email=student.email) + + self.assertEqual(actual_course_enrollment.mode, u"audit") + self.assertEqual(actual_student.is_active, True) + self.assertEqual(actual_cea.user, student) + + def test_verified_student_not_downgraded_when_changing_email(self): + """ + Make sure that verified students do not get downgrade if they are active + changing their email. + """ + # fixture + student = self._set_up_invited_student( + course=self.course, + active=True, + course_mode=u'verified' + ) + old_email = student.email + + # trigger the post_save callback + student.email = "foobar" + old_email + student.save() + + # reload values from the database + make sure they are in the expected state + actual_course_enrollment = CourseEnrollment.objects.get(user=student, course_id=self.course.id) + actual_student = User.objects.get(email=student.email) + + self.assertEqual(actual_course_enrollment.mode, u"verified") + self.assertEqual(actual_student.is_active, True) + + def _set_up_invited_student(self, course, active=False, enrolled=True, course_mode=''): + """ + Helper function to create a user in the right state, invite them into the course, and update their + course mode if needed. + """ + email = 'robot@robot.org' + user = UserFactory( + username='somestudent', + first_name='Student', + last_name='Person', + email=email, + is_active=active + ) + + # invite the user to the course + cea = CourseEnrollmentAllowed(email=email, course_id=course.id, auto_enroll=True) + cea.save() + + if enrolled: + CourseEnrollment.enroll(user, course.id) + + if course_mode: + course_enrollment = CourseEnrollment.objects.get( + user=user, course_id=self.course.id + ) + course_enrollment.mode = course_mode + course_enrollment.save() + + return user