diff --git a/lms/djangoapps/verified_track_content/migrations/0002_verifiedtrackcohortedcourse_verified_cohort_name.py b/lms/djangoapps/verified_track_content/migrations/0002_verifiedtrackcohortedcourse_verified_cohort_name.py new file mode 100644 index 0000000000..2fe7c0ce8e --- /dev/null +++ b/lms/djangoapps/verified_track_content/migrations/0002_verifiedtrackcohortedcourse_verified_cohort_name.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('verified_track_content', '0001_initial'), + ] + + operations = [ + migrations.AddField( + model_name='verifiedtrackcohortedcourse', + name='verified_cohort_name', + field=models.CharField(default=b'Verified Learners', max_length=100), + ), + ] diff --git a/lms/djangoapps/verified_track_content/models.py b/lms/djangoapps/verified_track_content/models.py index ea09a8a543..595d0811e7 100644 --- a/lms/djangoapps/verified_track_content/models.py +++ b/lms/djangoapps/verified_track_content/models.py @@ -10,7 +10,7 @@ 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 verified_track_content.tasks import sync_cohort_with_mode from openedx.core.djangoapps.course_groups.cohorts import ( get_course_cohorts, CourseCohort, is_course_cohorted ) @@ -19,6 +19,8 @@ import logging log = logging.getLogger(__name__) +DEFAULT_VERIFIED_COHORT_NAME = "Verified Learners" + @receiver(post_save, sender=CourseEnrollment) def move_to_verified_cohort(sender, instance, **kwargs): # pylint: disable=unused-argument @@ -28,14 +30,19 @@ def move_to_verified_cohort(sender, instance, **kwargs): # pylint: disable=unus """ course_key = instance.course_id verified_cohort_enabled = VerifiedTrackCohortedCourse.is_verified_track_cohort_enabled(course_key) + verified_cohort_name = VerifiedTrackCohortedCourse.verified_cohort_name_for_course(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} + if any(cohort.name == verified_cohort_name for cohort in existing_cohorts): + args = { + 'course_id': unicode(course_key), + 'user_id': instance.user.id, + 'verified_cohort_name': verified_cohort_name + } # 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. @@ -46,8 +53,9 @@ def move_to_verified_cohort(sender, instance, **kwargs): # pylint: disable=unus 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 + "Automatic verified cohorting enabled for course '%s', but cohort named '%s' does not exist.", + course_key, + verified_cohort_name, ) @@ -72,11 +80,31 @@ class VerifiedTrackCohortedCourse(models.Model): help_text=ugettext_lazy(u"The course key for the course we would like to be auto-cohorted.") ) + verified_cohort_name = models.CharField(max_length=100, default=DEFAULT_VERIFIED_COHORT_NAME) + enabled = models.BooleanField() def __unicode__(self): return u"Course: {}, enabled: {}".format(unicode(self.course_key), self.enabled) + @classmethod + def verified_cohort_name_for_course(cls, course_key): + """ + Returns the given cohort name for the specific course. + + Args: + course_key (CourseKey): a course key representing the course we want the verified cohort name for + + Returns: + The cohort name if the course key has one associated to it. None otherwise. + + """ + try: + config = cls.objects.get(course_key=course_key) + return config.verified_cohort_name + except cls.DoesNotExist: + return None + @classmethod def is_verified_track_cohort_enabled(cls, course_key): """ diff --git a/lms/djangoapps/verified_track_content/tasks.py b/lms/djangoapps/verified_track_content/tasks.py index 63dffe72e3..6c73e502cb 100644 --- a/lms/djangoapps/verified_track_content/tasks.py +++ b/lms/djangoapps/verified_track_content/tasks.py @@ -12,12 +12,11 @@ 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): +def sync_cohort_with_mode(course_id, user_id, verified_cohort_name): """ 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 @@ -31,7 +30,7 @@ def sync_cohort_with_mode(course_id, user_id): # 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) + 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( diff --git a/lms/djangoapps/verified_track_content/tests/test_forms.py b/lms/djangoapps/verified_track_content/tests/test_forms.py index c1e7835faf..6526f57fd3 100644 --- a/lms/djangoapps/verified_track_content/tests/test_forms.py +++ b/lms/djangoapps/verified_track_content/tests/test_forms.py @@ -23,12 +23,14 @@ class TestVerifiedTrackCourseForm(SharedModuleStoreTestCase): cls.course = CourseFactory.create() def test_form_validation_success(self): - form_data = {'course_key': unicode(self.course.id), 'enabled': True} + form_data = { + 'course_key': unicode(self.course.id), 'verified_cohort_name': 'Verified Learners', 'enabled': True + } form = VerifiedTrackCourseForm(data=form_data) self.assertTrue(form.is_valid()) def test_form_validation_failure(self): - form_data = {'course_key': self.FAKE_COURSE, 'enabled': True} + form_data = {'course_key': self.FAKE_COURSE, 'verified_cohort_name': 'Verified Learners', 'enabled': True} form = VerifiedTrackCourseForm(data=form_data) self.assertFalse(form.is_valid()) self.assertEqual( @@ -36,7 +38,7 @@ class TestVerifiedTrackCourseForm(SharedModuleStoreTestCase): ['COURSE NOT FOUND. Please check that the course ID is valid.'] ) - form_data = {'course_key': self.BAD_COURSE_KEY, 'enabled': True} + form_data = {'course_key': self.BAD_COURSE_KEY, 'verified_cohort_name': 'Verified Learners', 'enabled': True} form = VerifiedTrackCourseForm(data=form_data) self.assertFalse(form.is_valid()) self.assertEqual( diff --git a/lms/djangoapps/verified_track_content/tests/test_models.py b/lms/djangoapps/verified_track_content/tests/test_models.py index f8c38b80e7..ebd609d22f 100644 --- a/lms/djangoapps/verified_track_content/tests/test_models.py +++ b/lms/djangoapps/verified_track_content/tests/test_models.py @@ -12,8 +12,8 @@ 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 verified_track_content.models import VerifiedTrackCohortedCourse, DEFAULT_VERIFIED_COHORT_NAME +from verified_track_content.tasks import sync_cohort_with_mode from openedx.core.djangoapps.course_groups.cohorts import ( set_course_cohort_settings, add_cohort, CourseCohort, DEFAULT_COHORT_NAME ) @@ -47,6 +47,20 @@ class TestVerifiedTrackCohortedCourse(TestCase): config.save() self.assertEqual(unicode(config), "Course: {}, enabled: True".format(self.SAMPLE_COURSE)) + def test_verified_cohort_name(self): + COHORT_NAME = 'verified cohort' + course_key = CourseKey.from_string(self.SAMPLE_COURSE) + config = VerifiedTrackCohortedCourse.objects.create( + course_key=course_key, enabled=True, verified_cohort_name=COHORT_NAME + ) + config.save() + self.assertEqual(VerifiedTrackCohortedCourse.verified_cohort_name_for_course(course_key), COHORT_NAME) + + def test_unset_verified_cohort_name(self): + fake_course_id = 'fake/course/key' + course_key = CourseKey.from_string(fake_course_id) + self.assertEqual(VerifiedTrackCohortedCourse.verified_cohort_name_for_course(course_key), None) + class TestMoveToVerified(SharedModuleStoreTestCase): """ Tests for the post-save listener. """ @@ -69,12 +83,17 @@ class TestMoveToVerified(SharedModuleStoreTestCase): 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 _create_verified_cohort(self, name=DEFAULT_VERIFIED_COHORT_NAME): + add_cohort(self.course.id, name, CourseCohort.MANUAL) - def _enable_verified_track_cohorting(self): + def _enable_verified_track_cohorting(self, cohort_name=None): """ Enable verified track cohorting for the default course. """ - config = VerifiedTrackCohortedCourse.objects.create(course_key=self.course.id, enabled=True) + if cohort_name: + config = VerifiedTrackCohortedCourse.objects.create( + course_key=self.course.id, enabled=True, verified_cohort_name=cohort_name + ) + else: + config = VerifiedTrackCohortedCourse.objects.create(course_key=self.course.id, enabled=True) config.save() def _enroll_in_course(self): @@ -140,7 +159,8 @@ class TestMoveToVerified(SharedModuleStoreTestCase): 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]) + error_message = "cohort named '%s' does not exist" + self.assertIn(error_message, error_logger.call_args[0][0]) def test_automatic_cohorting_enabled(self): """ @@ -161,7 +181,7 @@ class TestMoveToVerified(SharedModuleStoreTestCase): 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) + self.assertEqual(DEFAULT_VERIFIED_COHORT_NAME, get_cohort(self.user, self.course.id, assign=False).name) def test_unenrolled(self): """ @@ -174,12 +194,21 @@ class TestMoveToVerified(SharedModuleStoreTestCase): 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) + self.assertEqual(DEFAULT_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.assertEqual(DEFAULT_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) + self.assertEqual(DEFAULT_VERIFIED_COHORT_NAME, get_cohort(self.user, self.course.id, assign=False).name) + + def test_custom_verified_cohort_name(self): + CUSTOM_COHORT_NAME = 'special verified cohort' + self._enable_cohorting() + self._create_verified_cohort(name=CUSTOM_COHORT_NAME) + self._enable_verified_track_cohorting(cohort_name=CUSTOM_COHORT_NAME) + self._enroll_in_course() + self._upgrade_to_verified() + self.assertEqual(CUSTOM_COHORT_NAME, get_cohort(self.user, self.course.id, assign=False).name)