Merge pull request #12091 from edx/diana/verified-cohort-naming

Add configurable name for verified track cohort.

lms/djangoapps/verified_track_content/migrations/0002_verifiedtrackcohortedcourse_verified_cohort_name.py

+# -*- 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),
+        ),
+    ]

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):
         """

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(

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(

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)