diff --git a/lms/djangoapps/verified_track_content/models.py b/lms/djangoapps/verified_track_content/models.py index a6c54b2105..ea09a8a543 100644 --- a/lms/djangoapps/verified_track_content/models.py +++ b/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): diff --git a/lms/djangoapps/verified_track_content/tasks.py b/lms/djangoapps/verified_track_content/tasks.py new file mode 100644 index 0000000000..63dffe72e3 --- /dev/null +++ b/lms/djangoapps/verified_track_content/tasks.py @@ -0,0 +1,51 @@ +""" +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 + ) diff --git a/lms/djangoapps/verified_track_content/tests/test_models.py b/lms/djangoapps/verified_track_content/tests/test_models.py index 37479f85c9..f8c38b80e7 100644 --- a/lms/djangoapps/verified_track_content/tests/test_models.py +++ b/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)