Merge pull request #1634 from edx/flowerhack/feature/refactor-to-track-enrollment

Flowerhack/feature/refactor to track enrollment

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:
+        # if is_active is None, then the call to update_enrollment didn't specify
-            enrollment.is_active = is_active
+        # 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:
+        # if mode is None, the call to update_enrollment didn't specify a new 
-            enrollment.mode = mode
+        # 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()
+            self.save()
-
+        if activation_changed:
-        if created:
+            if self.is_active:
-            if is_active:
+                self.emit_event(EVENT_NAME_ENROLLMENT_ACTIVATED)
-                enrollment.emit_event(EVENT_NAME_ENROLLMENT_ACTIVATED)
+            else:
-        else:
+                unenroll_done.send(sender=None, course_enrollment=self)
-            if activation_changed:
+                self.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED)
-                if is_active:
-                    enrollment.emit_event(EVENT_NAME_ENROLLMENT_ACTIVATED)
-                else:
-                    enrollment.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED)
-
-        return enrollment
 
     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.update_enrollment(is_active=False)
-                record.is_active = False
-                record.save()
-
-                record.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED)
-                unenroll_done.send(sender=cls, course_enrollment=record)
 
         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.update_enrollment(is_active=True)
-            self.is_active = True
-            self.save()
-            self.emit_event(EVENT_NAME_ENROLLMENT_ACTIVATED)
 
     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.update_enrollment(is_active=False)
-            self.is_active = False
+
-            self.save()
+    def change_mode(self, mode):
-            self.emit_event(EVENT_NAME_ENROLLMENT_DEACTIVATED)
+        """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.

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()
 

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.get_or_create_enrollment(order.user, course_id)
-            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)
 
         # 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.change_mode(self.mode)
-        self.course_enrollment.save()
         self.course_enrollment.activate()
 
     @property