Merge pull request #12026 from edx/christina/verified-track-cohort

Automatic cohorting of verified track learners

lms/djangoapps/verified_track_content/models.py

@@ -3,8 +3,64 @@ Models for verified track selections.
 """
 from django.db import models
 from django.utils.translation import ugettext_lazy
+from django.dispatch import receiver
+from django.db.models.signals import post_save, pre_save
 
 from xmodule_django.models import CourseKeyField
+from student.models import CourseEnrollment
+from courseware.courses import get_course_by_id
+
+from verified_track_content.tasks import sync_cohort_with_mode, VERIFIED_COHORT_NAME
+from openedx.core.djangoapps.course_groups.cohorts import (
+    get_course_cohorts, CourseCohort, is_course_cohorted
+)
+
+import logging
+
+log = logging.getLogger(__name__)
+
+
+@receiver(post_save, sender=CourseEnrollment)
+def move_to_verified_cohort(sender, instance, **kwargs):  # pylint: disable=unused-argument
+    """
+    If the learner has changed modes, update assigned cohort iff the course is using
+    the Automatic Verified Track Cohorting MVP feature.
+    """
+    course_key = instance.course_id
+    verified_cohort_enabled = VerifiedTrackCohortedCourse.is_verified_track_cohort_enabled(course_key)
+
+    if verified_cohort_enabled and (instance.mode != instance._old_mode):  # pylint: disable=protected-access
+        if not is_course_cohorted(course_key):
+            log.error("Automatic verified cohorting enabled for course '%s', but course is not cohorted", course_key)
+        else:
+            existing_cohorts = get_course_cohorts(get_course_by_id(course_key), CourseCohort.MANUAL)
+            if any(cohort.name == VERIFIED_COHORT_NAME for cohort in existing_cohorts):
+                args = {'course_id': unicode(course_key), 'user_id': instance.user.id}
+                # Do the update with a 3-second delay in hopes that the CourseEnrollment transaction has been
+                # completed before the celery task runs. We want a reasonably short delay in case the learner
+                # immediately goes to the courseware.
+                sync_cohort_with_mode.apply_async(kwargs=args, countdown=3)
+
+                # In case the transaction actually was not committed before the celery task runs,
+                # run it again after 5 minutes. If the first completed successfully, this task will be a no-op.
+                sync_cohort_with_mode.apply_async(kwargs=args, countdown=300)
+            else:
+                log.error(
+                    "Automatic verified cohorting enabled for course '%s', but course does not have a verified cohort",
+                    course_key
+                )
+
+
+@receiver(pre_save, sender=CourseEnrollment)
+def pre_save_callback(sender, instance, **kwargs):  # pylint: disable=unused-argument
+    """
+    Extend to store previous mode.
+    """
+    try:
+        old_instance = sender.objects.get(pk=instance.pk)
+        instance._old_mode = old_instance.mode  # pylint: disable=protected-access
+    except CourseEnrollment.DoesNotExist:
+        instance._old_mode = None  # pylint: disable=protected-access
 
 
 class VerifiedTrackCohortedCourse(models.Model):

lms/djangoapps/verified_track_content/tasks.py

+"""
+Celery task for Automatic Verifed Track Cohorting MVP feature.
+"""
+from django.contrib.auth.models import User
+
+from celery.task import task
+from celery.utils.log import get_task_logger
+
+from opaque_keys.edx.keys import CourseKey
+from student.models import CourseEnrollment, CourseMode
+from openedx.core.djangoapps.course_groups.cohorts import (
+    get_cohort_by_name, get_cohort, add_user_to_cohort, DEFAULT_COHORT_NAME
+)
+
+VERIFIED_COHORT_NAME = "verified"
+LOGGER = get_task_logger(__name__)
+
+
+@task()
+def sync_cohort_with_mode(course_id, user_id):
+    """
+    If the learner's mode does not match their assigned cohort, move the learner into the correct cohort.
+    It is assumed that this task is only initiated for courses that are using the
+    Automatic Verified Track Cohorting MVP feature. It is also assumed that before
+    initiating this task, verification has been done to ensure that the course is
+    cohorted and has an appropriately named "verified" cohort.
+    """
+    course_key = CourseKey.from_string(course_id)
+    user = User.objects.get(id=user_id)
+    enrollment = CourseEnrollment.get_enrollment(user, course_key)
+    # Note that this will enroll the user in the default cohort on initial enrollment.
+    # That's good because it will force creation of the default cohort if necessary.
+    current_cohort = get_cohort(user, course_key)
+    verified_cohort = get_cohort_by_name(course_key, VERIFIED_COHORT_NAME)
+
+    if enrollment.mode == CourseMode.VERIFIED and (current_cohort.id != verified_cohort.id):
+        LOGGER.info(
+            "MOVING_TO_VERIFIED: Moving user '%s' to the verified cohort for course '%s'", user.username, course_id
+        )
+        add_user_to_cohort(verified_cohort, user.username)
+    elif enrollment.mode != CourseMode.VERIFIED and current_cohort.id == verified_cohort.id:
+        LOGGER.info(
+            "MOVING_TO_DEFAULT: Moving user '%s' to the default cohort for course '%s'", user.username, course_id
+        )
+        default_cohort = get_cohort_by_name(course_key, DEFAULT_COHORT_NAME)
+        add_user_to_cohort(default_cohort, user.username)
+    else:
+        LOGGER.info(
+            "NO_ACTION_NECESSARY: No action necessary for user '%s' in course '%s' and enrollment mode '%s'",
+            user.username, course_id, enrollment.mode
+        )

lms/djangoapps/verified_track_content/tests/test_models.py

@@ -2,10 +2,21 @@
 Tests for Verified Track Cohorting models
 """
 from django.test import TestCase
+import mock
+from mock import patch
 
 from opaque_keys.edx.keys import CourseKey
 
+from openedx.core.djangoapps.course_groups.cohorts import get_cohort
+from student.models import CourseMode
+from student.tests.factories import UserFactory, CourseEnrollmentFactory
+from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
 from verified_track_content.models import VerifiedTrackCohortedCourse
+from verified_track_content.tasks import sync_cohort_with_mode, VERIFIED_COHORT_NAME
+from openedx.core.djangoapps.course_groups.cohorts import (
+    set_course_cohort_settings, add_cohort, CourseCohort, DEFAULT_COHORT_NAME
+)
 
 
 class TestVerifiedTrackCohortedCourse(TestCase):
@@ -35,3 +46,140 @@ class TestVerifiedTrackCohortedCourse(TestCase):
         config = VerifiedTrackCohortedCourse.objects.create(course_key=course_key, enabled=True)
         config.save()
         self.assertEqual(unicode(config), "Course: {}, enabled: True".format(self.SAMPLE_COURSE))
+
+
+class TestMoveToVerified(SharedModuleStoreTestCase):
+    """ Tests for the post-save listener. """
+
+    @classmethod
+    def setUpClass(cls):
+        super(TestMoveToVerified, cls).setUpClass()
+        cls.course = CourseFactory.create()
+
+    def setUp(self):
+        self.user = UserFactory()
+        # Spy on number of calls to celery task.
+        celery_task_patcher = patch.object(
+            sync_cohort_with_mode, 'apply_async',
+            mock.Mock(wraps=sync_cohort_with_mode.apply_async)
+        )
+        self.mocked_celery_task = celery_task_patcher.start()
+        self.addCleanup(celery_task_patcher.stop)
+
+    def _enable_cohorting(self):
+        set_course_cohort_settings(self.course.id, is_cohorted=True)
+
+    def _create_verified_cohort(self):
+        add_cohort(self.course.id, VERIFIED_COHORT_NAME, CourseCohort.MANUAL)
+
+    def _enable_verified_track_cohorting(self):
+        """ Enable verified track cohorting for the default course. """
+        config = VerifiedTrackCohortedCourse.objects.create(course_key=self.course.id, enabled=True)
+        config.save()
+
+    def _enroll_in_course(self):
+        self.enrollment = CourseEnrollmentFactory(course_id=self.course.id, user=self.user)
+
+    def _upgrade_to_verified(self):
+        """ Upgrade the default enrollment to verified. """
+        self.enrollment.update_enrollment(mode=CourseMode.VERIFIED)
+
+    def _verify_no_automatic_cohorting(self):
+        self._enroll_in_course()
+        self.assertIsNone(get_cohort(self.user, self.course.id, assign=False))
+        self._upgrade_to_verified()
+        self.assertIsNone(get_cohort(self.user, self.course.id, assign=False))
+        self.assertEqual(0, self.mocked_celery_task.call_count)
+
+    def _unenroll(self):
+        self.enrollment.unenroll(self.user, self.course.id)
+
+    def _reenroll(self):
+        self.enrollment.activate()
+        self.enrollment.change_mode(CourseMode.AUDIT)
+
+    @mock.patch('verified_track_content.models.log.error')
+    def test_automatic_cohorting_disabled(self, error_logger):
+        """
+        If the VerifiedTrackCohortedCourse feature is disabled for a course, enrollment mode changes do not move
+        learners into a cohort.
+        """
+        # Enable cohorting and create a verified cohort.
+        self._enable_cohorting()
+        self._create_verified_cohort()
+        # But do not enable the verified track cohorting feature.
+        self.assertFalse(VerifiedTrackCohortedCourse.is_verified_track_cohort_enabled(self.course.id))
+        self._verify_no_automatic_cohorting()
+        # No logging occurs if feature is disabled for course.
+        self.assertFalse(error_logger.called)
+
+    @mock.patch('verified_track_content.models.log.error')
+    def test_cohorting_enabled_course_not_cohorted(self, error_logger):
+        """
+        If the VerifiedTrackCohortedCourse feature is enabled for a course, but the course is not cohorted,
+        an error is logged and enrollment mode changes do not move learners into a cohort.
+        """
+        # Enable verified track cohorting feature, but course has not been marked as cohorting.
+        self._enable_verified_track_cohorting()
+        self.assertTrue(VerifiedTrackCohortedCourse.is_verified_track_cohort_enabled(self.course.id))
+        self._verify_no_automatic_cohorting()
+        self.assertTrue(error_logger.called)
+        self.assertIn("course is not cohorted", error_logger.call_args[0][0])
+
+    @mock.patch('verified_track_content.models.log.error')
+    def test_cohorting_enabled_missing_verified_cohort(self, error_logger):
+        """
+        If the VerifiedTrackCohortedCourse feature is enabled for a course and the course is cohorted,
+        but the course does not have a verified cohort, an error is logged and enrollment mode changes do not
+        move learners into a cohort.
+        """
+        # Enable cohorting, but do not create the verified cohort.
+        self._enable_cohorting()
+        # Enable verified track cohorting feature
+        self._enable_verified_track_cohorting()
+        self.assertTrue(VerifiedTrackCohortedCourse.is_verified_track_cohort_enabled(self.course.id))
+        self._verify_no_automatic_cohorting()
+        self.assertTrue(error_logger.called)
+        self.assertIn("course does not have a verified cohort", error_logger.call_args[0][0])
+
+    def test_automatic_cohorting_enabled(self):
+        """
+        If the VerifiedTrackCohortedCourse feature is enabled for a course (with course cohorting enabled
+        with an existing verified cohort), enrollment in the verified track automatically moves learners
+        into the verified cohort.
+        """
+        # Enable cohorting and create a verified cohort.
+        self._enable_cohorting()
+        self._create_verified_cohort()
+        # Enable verified track cohorting feature
+        self._enable_verified_track_cohorting()
+        self.assertTrue(VerifiedTrackCohortedCourse.is_verified_track_cohort_enabled(self.course.id))
+        self._enroll_in_course()
+
+        self.assertEqual(2, self.mocked_celery_task.call_count)
+        self.assertEqual(DEFAULT_COHORT_NAME, get_cohort(self.user, self.course.id, assign=False).name)
+
+        self._upgrade_to_verified()
+        self.assertEqual(4, self.mocked_celery_task.call_count)
+        self.assertEqual(VERIFIED_COHORT_NAME, get_cohort(self.user, self.course.id, assign=False).name)
+
+    def test_unenrolled(self):
+        """
+        Test that un-enrolling and re-enrolling works correctly. This is important because usually
+        learners maintain their previously assigned cohort on re-enrollment.
+        """
+        # Enable verified track cohorting feature and enroll in the verified track
+        self._enable_cohorting()
+        self._create_verified_cohort()
+        self._enable_verified_track_cohorting()
+        self._enroll_in_course()
+        self._upgrade_to_verified()
+        self.assertEqual(VERIFIED_COHORT_NAME, get_cohort(self.user, self.course.id, assign=False).name)
+
+        # Un-enroll from the course and then re-enroll
+        self._unenroll()
+        self.assertEqual(VERIFIED_COHORT_NAME, get_cohort(self.user, self.course.id, assign=False).name)
+        self._reenroll()
+        self.assertEqual(DEFAULT_COHORT_NAME, get_cohort(self.user, self.course.id, assign=False).name)
+        self._upgrade_to_verified()
+        self.assertEqual(VERIFIED_COHORT_NAME, get_cohort(self.user, self.course.id, assign=False).name)