diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index b4a78d64d2..d7a918b569 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -714,7 +714,7 @@ class CourseEnrollment(models.Model): ).format(self.user, self.course_id, self.created, self.is_active) @classmethod - def create_enrollment(cls, user, course_id, mode="honor", is_active=False): + def get_or_create_enrollment(cls, user, course_id): """ Create an enrollment for a user in a class. By default *this enrollment is not active*. This is useful for when an enrollment needs to go @@ -729,16 +729,6 @@ class CourseEnrollment(models.Model): `course_id` is our usual course_id string (e.g. "edX/Test101/2013_Fall) - `mode` is a string specifying what kind of enrollment this is. The - default is "honor", meaning honor certificate. Future options - may include "audit", "verified_id", etc. Please don't use it - until we have these mapped out. - - `is_active` is a boolean. If the CourseEnrollment object has - `is_active=False`, then calling - `CourseEnrollment.is_enrolled()` for that user/course_id - will return False. - It is expected that this method is called from a method which has already verified the user authentication and access. """ @@ -754,30 +744,45 @@ class CourseEnrollment(models.Model): course_id=course_id, ) + # If we *did* just create a new enrollment, set some defaults + if created: + enrollment.mode = "honor" + enrollment.is_active = False + enrollment.save() + + return enrollment + + def update_enrollment(self, mode=None, is_active=None): + """ + Updates an enrollment for a user in a class. This includes options + like changing the mode, toggling is_active True/False, etc. + + Also emits relevant events for analytics purposes. + + This saves immediately. + """ activation_changed = False - if enrollment.is_active != is_active: - enrollment.is_active = is_active + # if is_active is None, then the call to update_enrollment didn't specify + # any value, so just leave is_active as it is + if self.is_active != is_active and is_active is not None: + self.is_active = is_active activation_changed = True mode_changed = False - if enrollment.mode != mode: - enrollment.mode = mode + # if mode is None, the call to update_enrollment didn't specify a new + # mode, so leave as-is + if self.mode != mode and mode is not None: + self.mode = mode mode_changed = True if activation_changed or mode_changed: - enrollment.save() - - if created: - if is_active: - enrollment.emit_event(EVENT_NAME_ENROLLMENT_ACTIVATED) - else: - if activation_changed: - if is_active: - enrollment.emit_event(EVENT_NAME_ENROLLMENT_ACTIVATED) - else: - enrollment.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED) - - return enrollment + self.save() + if activation_changed: + if self.is_active: + self.emit_event(EVENT_NAME_ENROLLMENT_ACTIVATED) + else: + unenroll_done.send(sender=None, course_enrollment=self) + self.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED) def emit_event(self, event_name): """ @@ -819,7 +824,9 @@ class CourseEnrollment(models.Model): It is expected that this method is called from a method which has already verified the user authentication and access. """ - return cls.create_enrollment(user, course_id, mode, is_active=True) + enrollment = cls.get_or_create_enrollment(user, course_id) + enrollment.update_enrollment(is_active=True) + return enrollment @classmethod def enroll_by_email(cls, email, course_id, mode="honor", ignore_errors=True): @@ -873,12 +880,7 @@ class CourseEnrollment(models.Model): """ try: record = CourseEnrollment.objects.get(user=user, course_id=course_id) - if record.is_active: - record.is_active = False - record.save() - - record.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED) - unenroll_done.send(sender=cls, course_enrollment=record) + record.update_enrollment(is_active=False) except cls.DoesNotExist: err_msg = u"Tried to unenroll student {} from {} but they were not enrolled" @@ -967,19 +969,17 @@ class CourseEnrollment(models.Model): def activate(self): """Makes this `CourseEnrollment` record active. Saves immediately.""" - if not self.is_active: - self.is_active = True - self.save() - self.emit_event(EVENT_NAME_ENROLLMENT_ACTIVATED) + self.update_enrollment(is_active=True) def deactivate(self): """Makes this `CourseEnrollment` record inactive. Saves immediately. An inactive record means that the student is not enrolled in this course. """ - if self.is_active: - self.is_active = False - self.save() - self.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED) + self.update_enrollment(is_active=False) + + def change_mode(self, mode): + """Changes this `CourseEnrollment` record's mode to `mode`. Saves immediately.""" + self.update_enrollment(mode=mode) def refundable(self): """ @@ -993,7 +993,6 @@ class CourseEnrollment(models.Model): return True - class CourseEnrollmentAllowed(models.Model): """ Table of users (specified by email address strings) who are allowed to enroll in a specified course. diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 06d61c0425..1e7ee4baa8 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -443,7 +443,7 @@ class EnrollInCourseTest(TestCase): # Creating an enrollment doesn't actually enroll a student # (calling CourseEnrollment.enroll() would have) - enrollment = CourseEnrollment.create_enrollment(user, course_id) + enrollment = CourseEnrollment.get_or_create_enrollment(user, course_id) self.assertFalse(CourseEnrollment.is_enrolled(user, course_id)) self.assert_no_events_were_emitted() diff --git a/lms/djangoapps/shoppingcart/models.py b/lms/djangoapps/shoppingcart/models.py index 03be80861a..8d3ecfa32f 100644 --- a/lms/djangoapps/shoppingcart/models.py +++ b/lms/djangoapps/shoppingcart/models.py @@ -400,7 +400,7 @@ class CertificateItem(OrderItem): course_enrollment = models.ForeignKey(CourseEnrollment) mode = models.SlugField() - @receiver(unenroll_done, sender=CourseEnrollment) + @receiver(unenroll_done) def refund_cert_callback(sender, course_enrollment=None, **kwargs): """ When a CourseEnrollment object calls its unenroll method, this function checks to see if that unenrollment @@ -464,10 +464,7 @@ class CertificateItem(OrderItem): """ super(CertificateItem, cls).add_to_order(order, course_id, cost, currency=currency) - try: - course_enrollment = CourseEnrollment.objects.get(user=order.user, course_id=course_id) - except ObjectDoesNotExist: - course_enrollment = CourseEnrollment.create_enrollment(order.user, course_id, mode=mode) + course_enrollment = CourseEnrollment.get_or_create_enrollment(order.user, course_id) # do some validation on the enrollment mode valid_modes = CourseMode.modes_for_course_dict(course_id) @@ -506,8 +503,7 @@ class CertificateItem(OrderItem): "Could not submit verification attempt for enrollment {}".format(self.course_enrollment) ) - self.course_enrollment.mode = self.mode - self.course_enrollment.save() + self.course_enrollment.change_mode(self.mode) self.course_enrollment.activate() @property