TNL-328 Server-side work for displaying type of cohort.

common/djangoapps/course_groups/cohorts.py

@@ -15,13 +15,40 @@ from .models import CourseUserGroup
 log = logging.getLogger(__name__)
 
 
-# A 'default cohort' is an auto-cohort that is automatically created for a course if no auto-cohorts have been
+# A 'default cohort' is an auto-cohort that is automatically created for a course if no auto_cohort_groups have been
 # specified. It is intended to be used in a cohorted-course for users who have yet to be assigned to a cohort.
-# If an administrator chooses to configure a cohort with the same name, the said cohort will also be used as
-# the "default cohort".
+# Note 1: If an administrator chooses to configure a cohort with the same name, the said cohort will be used as
+#         the "default cohort".
+# Note 2: If auto_cohort_groups are configured after the 'default cohort' has been created and populated, the
+#         stagnant 'default cohort' will still remain (now as a manual cohort) with its previously assigned students.
 # Translation Note: We are NOT translating this string since it is the constant identifier for the "default group"
-# and needed across product boundaries.
-DEFAULT_COHORT_NAME = "Default Cohort Group"
+#                   and needed across product boundaries.
+DEFAULT_COHORT_NAME = "Default Group"
+
+
+class CohortAssignmentType(object):
+    """
+    The various types of rule-based cohorts
+    """
+    # No automatic rules are applied to this cohort; users must be manually added.
+    NONE = "none"
+
+    # One of (possibly) multiple cohort groups to which users are randomly assigned.
+    # Note: The 'default cohort' group is included in this category iff it exists and
+    # there are no other random groups. (Also see Note 2 above.)
+    RANDOM = "random"
+
+    @staticmethod
+    def get(cohort, course):
+        """
+        Returns the assignment type of the given cohort for the given course
+        """
+        if cohort.name in course.auto_cohort_groups:
+            return CohortAssignmentType.RANDOM
+        elif len(course.auto_cohort_groups) == 0 and cohort.name == DEFAULT_COHORT_NAME:
+            return CohortAssignmentType.RANDOM
+        else:
+            return CohortAssignmentType.NONE
 
 
 # tl;dr: global state is bad.  capa reseeds random every time a problem is loaded.  Even

common/djangoapps/course_groups/tests/helpers.py

 """
 Helper methods for testing cohorts.
 """
+from factory import post_generation, Sequence
+from factory.django import DjangoModelFactory
+from course_groups.models import CourseUserGroup
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore import ModuleStoreEnum
-from course_groups.models import CourseUserGroup
-from course_groups.cohorts import DEFAULT_COHORT_NAME
+
+
+class CohortFactory(DjangoModelFactory):
+    FACTORY_FOR = CourseUserGroup
+
+    name = Sequence("cohort{}".format)
+    course_id = "dummy_id"
+    group_type = CourseUserGroup.COHORT
+
+    @post_generation
+    def users(self, create, extracted, **kwargs):  # pylint: disable=W0613
+        if extracted:
+            self.users.add(*extracted)
 
 
 def topic_name_to_id(course, name):
@@ -19,29 +33,6 @@ def topic_name_to_id(course, name):
     )
 
 
-def get_default_cohort(course):
-    """
-    Returns the default cohort for a course.
-    Returns None if the default cohort hasn't yet been created.
-    """
-    return get_cohort_in_course(DEFAULT_COHORT_NAME, course)
-
-
-def get_cohort_in_course(cohort_name, course):
-    """
-    Returns the cohort with the name `cohort_name` in the given `course`.
-    Returns None if it doesn't exist.
-    """
-    try:
-        return CourseUserGroup.objects.get(
-            course_id=course.id,
-            group_type=CourseUserGroup.COHORT,
-            name=cohort_name
-        )
-    except CourseUserGroup.DoesNotExist:
-        return None
-
-
 def config_course_cohorts(
         course,
         discussions,

common/djangoapps/course_groups/tests/test_cohorts.py

@@ -6,9 +6,10 @@ from django.http import Http404
 from django.test.utils import override_settings
 
 from student.models import CourseEnrollment
+from student.tests.factories import UserFactory
 from course_groups.models import CourseUserGroup
 from course_groups import cohorts
-from course_groups.tests.helpers import topic_name_to_id, config_course_cohorts, get_default_cohort
+from course_groups.tests.helpers import topic_name_to_id, config_course_cohorts, CohortFactory
 
 from xmodule.modulestore.django import modulestore, clear_existing_modulestores
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
@@ -59,15 +60,11 @@ class TestCohorts(django.test.TestCase):
         course = modulestore().get_course(self.toy_course_key)
         self.assertFalse(course.is_cohorted)
 
-        user = User.objects.create(username="test", email="a@b.com")
+        user = UserFactory(username="test", email="a@b.com")
         self.assertIsNone(cohorts.get_cohort_id(user, course.id))
 
         config_course_cohorts(course, discussions=[], cohorted=True)
-        cohort = CourseUserGroup.objects.create(
-            name="TestCohort",
-            course_id=course.id,
-            group_type=CourseUserGroup.COHORT
-        )
+        cohort = CohortFactory(course_id=course.id, name="TestCohort")
         cohort.users.add(user)
         self.assertEqual(cohorts.get_cohort_id(user, course.id), cohort.id)
 
@@ -84,17 +81,12 @@ class TestCohorts(django.test.TestCase):
         self.assertEqual(course.id, self.toy_course_key)
         self.assertFalse(course.is_cohorted)
 
-        user = User.objects.create(username="test", email="a@b.com")
-        other_user = User.objects.create(username="test2", email="a2@b.com")
+        user = UserFactory(username="test", email="a@b.com")
+        other_user = UserFactory(username="test2", email="a2@b.com")
 
         self.assertIsNone(cohorts.get_cohort(user, course.id), "No cohort created yet")
 
-        cohort = CourseUserGroup.objects.create(
-            name="TestCohort",
-            course_id=course.id,
-            group_type=CourseUserGroup.COHORT
-        )
-
+        cohort = CohortFactory(course_id=course.id, name="TestCohort")
         cohort.users.add(user)
 
         self.assertIsNone(
@@ -112,7 +104,7 @@ class TestCohorts(django.test.TestCase):
         )
         self.assertEquals(
             cohorts.get_cohort(other_user, course.id).id,
-            get_default_cohort(course).id,
+            cohorts.get_cohort_by_name(course.id, cohorts.DEFAULT_COHORT_NAME).id,
             "other_user should be assigned to the default cohort"
         )
 
@@ -123,16 +115,12 @@ class TestCohorts(django.test.TestCase):
         course = modulestore().get_course(self.toy_course_key)
         self.assertFalse(course.is_cohorted)
 
-        user1 = User.objects.create(username="test", email="a@b.com")
-        user2 = User.objects.create(username="test2", email="a2@b.com")
-        user3 = User.objects.create(username="test3", email="a3@b.com")
-        user4 = User.objects.create(username="test4", email="a4@b.com")
+        user1 = UserFactory(username="test", email="a@b.com")
+        user2 = UserFactory(username="test2", email="a2@b.com")
+        user3 = UserFactory(username="test3", email="a3@b.com")
+        user4 = UserFactory(username="test4", email="a4@b.com")
 
-        cohort = CourseUserGroup.objects.create(
-            name="TestCohort",
-            course_id=course.id,
-            group_type=CourseUserGroup.COHORT
-        )
+        cohort = CohortFactory(course_id=course.id, name="TestCohort")
 
         # user1 manually added to a cohort
         cohort.users.add(user1)
@@ -159,7 +147,7 @@ class TestCohorts(django.test.TestCase):
 
         self.assertEquals(
             cohorts.get_cohort(user3, course.id).id,
-            get_default_cohort(course).id,
+            cohorts.get_cohort_by_name(course.id, cohorts.DEFAULT_COHORT_NAME).id,
             "No groups->default cohort"
         )
 
@@ -182,7 +170,7 @@ class TestCohorts(django.test.TestCase):
         )
         self.assertEquals(
             cohorts.get_cohort(user3, course.id).name,
-            get_default_cohort(course).name,
+            cohorts.get_cohort_by_name(course.id, cohorts.DEFAULT_COHORT_NAME).name,
             "user3 should still be in the default cohort"
         )
 
@@ -200,7 +188,7 @@ class TestCohorts(django.test.TestCase):
 
         # Assign 100 users to cohorts
         for i in range(100):
-            user = User.objects.create(
+            user = UserFactory(
                 username="test_{0}".format(i),
                 email="a@b{0}.com".format(i)
             )
@@ -235,17 +223,8 @@ class TestCohorts(django.test.TestCase):
         )
 
         # add manual cohorts to course 1
-        CourseUserGroup.objects.create(
-            name="ManualCohort",
-            course_id=course.location.course_key,
-            group_type=CourseUserGroup.COHORT
-        )
-
-        CourseUserGroup.objects.create(
-            name="ManualCohort2",
-            course_id=course.location.course_key,
-            group_type=CourseUserGroup.COHORT
-        )
+        CohortFactory(course_id=course.id, name="ManualCohort")
+        CohortFactory(course_id=course.id, name="ManualCohort2")
 
         cohort_set = {c.name for c in cohorts.get_course_cohorts(course)}
         self.assertEqual(cohort_set, {"AutoGroup1", "AutoGroup2", "ManualCohort", "ManualCohort2"})
@@ -349,11 +328,7 @@ class TestCohorts(django.test.TestCase):
             lambda: cohorts.get_cohort_by_name(course.id, "CohortDoesNotExist")
         )
 
-        cohort = CourseUserGroup.objects.create(
-            name="MyCohort",
-            course_id=course.id,
-            group_type=CourseUserGroup.COHORT
-        )
+        cohort = CohortFactory(course_id=course.id, name="MyCohort")
 
         self.assertEqual(cohorts.get_cohort_by_name(course.id, "MyCohort"), cohort)
 
@@ -368,11 +343,7 @@ class TestCohorts(django.test.TestCase):
         course.
         """
         course = modulestore().get_course(self.toy_course_key)
-        cohort = CourseUserGroup.objects.create(
-            name="MyCohort",
-            course_id=course.id,
-            group_type=CourseUserGroup.COHORT
-        )
+        cohort = CohortFactory(course_id=course.id, name="MyCohort")
 
         self.assertEqual(cohorts.get_cohort_by_id(course.id, cohort.id), cohort)
 
@@ -406,20 +377,12 @@ class TestCohorts(django.test.TestCase):
         Make sure cohorts.add_user_to_cohort() properly adds a user to a cohort and
         handles errors.
         """
-        course_user = User.objects.create(username="Username", email="a@b.com")
-        User.objects.create(username="RandomUsername", email="b@b.com")
+        course_user = UserFactory(username="Username", email="a@b.com")
+        UserFactory(username="RandomUsername", email="b@b.com")
         course = modulestore().get_course(self.toy_course_key)
         CourseEnrollment.enroll(course_user, self.toy_course_key)
-        first_cohort = CourseUserGroup.objects.create(
-            name="FirstCohort",
-            course_id=course.id,
-            group_type=CourseUserGroup.COHORT
-        )
-        second_cohort = CourseUserGroup.objects.create(
-            name="SecondCohort",
-            course_id=course.id,
-            group_type=CourseUserGroup.COHORT
-        )
+        first_cohort = CohortFactory(course_id=course.id, name="FirstCohort")
+        second_cohort = CohortFactory(course_id=course.id, name="SecondCohort")
 
         # Success cases
         # We shouldn't get back a previous cohort, since the user wasn't in one
@@ -452,17 +415,9 @@ class TestCohorts(django.test.TestCase):
         for a given course.
         """
         course = modulestore().get_course(self.toy_course_key)
-        user = User.objects.create(username="Username", email="a@b.com")
-        empty_cohort = CourseUserGroup.objects.create(
-            name="EmptyCohort",
-            course_id=course.id,
-            group_type=CourseUserGroup.COHORT
-        )
-        nonempty_cohort = CourseUserGroup.objects.create(
-            name="NonemptyCohort",
-            course_id=course.id,
-            group_type=CourseUserGroup.COHORT
-        )
+        user = UserFactory(username="Username", email="a@b.com")
+        empty_cohort = CohortFactory(course_id=course.id, name="EmptyCohort")
+        nonempty_cohort = CohortFactory(course_id=course.id, name="NonemptyCohort")
         nonempty_cohort.users.add(user)
 
         cohorts.delete_empty_cohort(course.id, "EmptyCohort")
@@ -470,11 +425,7 @@ class TestCohorts(django.test.TestCase):
         # Make sure we cannot access the deleted cohort
         self.assertRaises(
             CourseUserGroup.DoesNotExist,
-            lambda: CourseUserGroup.objects.get(
-                course_id=course.id,
-                group_type=CourseUserGroup.COHORT,
-                id=empty_cohort.id
-            )
+            lambda: cohorts.get_cohort_by_id(course.id, empty_cohort.id)
         )
         self.assertRaises(
             ValueError,

common/djangoapps/course_groups/tests/test_views.py

@@ -2,14 +2,11 @@ import json
 
 from django.test.client import RequestFactory
 from django.test.utils import override_settings
-from factory import post_generation, Sequence
-from factory.django import DjangoModelFactory
-from course_groups.tests.helpers import config_course_cohorts, get_cohort_in_course, get_default_cohort
+from course_groups.tests.helpers import config_course_cohorts, CohortFactory
 from collections import namedtuple
 
 from django.http import Http404
 from django.contrib.auth.models import User
-from course_groups.models import CourseUserGroup
 from courseware.tests.tests import TEST_DATA_MIXED_MODULESTORE
 from student.models import CourseEnrollment
 from student.tests.factories import UserFactory
@@ -17,19 +14,9 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 
+from course_groups.models import CourseUserGroup
 from course_groups.views import list_cohorts, add_cohort, users_in_cohort, add_users_to_cohort, remove_user_from_cohort
-from course_groups.cohorts import get_cohort
-
-class CohortFactory(DjangoModelFactory):
-    FACTORY_FOR = CourseUserGroup
-
-    name = Sequence("cohort{}".format)
-    course_id = "dummy_id"
-    group_type = CourseUserGroup.COHORT
-
-    @post_generation
-    def users(self, create, extracted, **kwargs):  # pylint: disable=W0613
-        self.users.add(*extracted)
+from course_groups.cohorts import get_cohort, CohortAssignmentType, get_cohort_by_name, DEFAULT_COHORT_NAME
 
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@@ -39,8 +26,8 @@ class CohortViewsTestCase(ModuleStoreTestCase):
     """
     def setUp(self):
         self.course = CourseFactory.create()
-        self.staff_user = UserFactory.create(is_staff=True, username="staff")
-        self.non_staff_user = UserFactory.create(username="nonstaff")
+        self.staff_user = UserFactory(is_staff=True, username="staff")
+        self.non_staff_user = UserFactory(username="nonstaff")
 
     def _enroll_users(self, users, course_key):
         """Enroll each user in the specified course"""
@@ -49,18 +36,18 @@ class CohortViewsTestCase(ModuleStoreTestCase):
 
     def _create_cohorts(self):
         """Creates cohorts for testing"""
-        self.cohort1_users = [UserFactory.create() for _ in range(3)]
-        self.cohort2_users = [UserFactory.create() for _ in range(2)]
-        self.cohort3_users = [UserFactory.create() for _ in range(2)]
-        self.cohortless_users = [UserFactory.create() for _ in range(3)]
-        self.unenrolled_users = [UserFactory.create() for _ in range(3)]
+        self.cohort1_users = [UserFactory() for _ in range(3)]
+        self.cohort2_users = [UserFactory() for _ in range(2)]
+        self.cohort3_users = [UserFactory() for _ in range(2)]
+        self.cohortless_users = [UserFactory() for _ in range(3)]
+        self.unenrolled_users = [UserFactory() for _ in range(3)]
         self._enroll_users(
             self.cohort1_users + self.cohort2_users + self.cohort3_users + self.cohortless_users,
             self.course.id
         )
-        self.cohort1 = CohortFactory.create(course_id=self.course.id, users=self.cohort1_users)
-        self.cohort2 = CohortFactory.create(course_id=self.course.id, users=self.cohort2_users)
-        self.cohort3 = CohortFactory.create(course_id=self.course.id, users=self.cohort3_users)
+        self.cohort1 = CohortFactory(course_id=self.course.id, users=self.cohort1_users)
+        self.cohort2 = CohortFactory(course_id=self.course.id, users=self.cohort2_users)
+        self.cohort3 = CohortFactory(course_id=self.course.id, users=self.cohort3_users)
 
     def _user_in_cohort(self, username, cohort):
         """
@@ -114,18 +101,25 @@ class ListCohortsTestCase(CohortViewsTestCase):
         self.assertItemsEqual(
             response_dict.get("cohorts"),
             [
-                {"name": cohort.name, "id": cohort.id, "user_count": cohort.user_count}
+                {
+                    "name": cohort.name,
+                    "id": cohort.id,
+                    "user_count": cohort.user_count,
+                    "assignment_type": cohort.assignment_type
+                }
                 for cohort in expected_cohorts
             ]
         )
 
     @staticmethod
-    def create_expected_cohort(cohort, user_count):
+    def create_expected_cohort(cohort, user_count, assignment_type):
         """
         Create a tuple storing the expected cohort information.
         """
-        cohort_tuple = namedtuple("Cohort", "name id user_count")
-        return cohort_tuple(name=cohort.name, id=cohort.id, user_count=user_count)
+        cohort_tuple = namedtuple("Cohort", "name id user_count assignment_type")
+        return cohort_tuple(
+            name=cohort.name, id=cohort.id, user_count=user_count, assignment_type=assignment_type
+        )
 
     def test_non_staff(self):
         """
@@ -145,9 +139,9 @@ class ListCohortsTestCase(CohortViewsTestCase):
         """
         self._create_cohorts()
         expected_cohorts = [
-            ListCohortsTestCase.create_expected_cohort(self.cohort1, 3),
-            ListCohortsTestCase.create_expected_cohort(self.cohort2, 2),
-            ListCohortsTestCase.create_expected_cohort(self.cohort3, 2),
+            ListCohortsTestCase.create_expected_cohort(self.cohort1, 3, CohortAssignmentType.NONE),
+            ListCohortsTestCase.create_expected_cohort(self.cohort2, 2, CohortAssignmentType.NONE),
+            ListCohortsTestCase.create_expected_cohort(self.cohort3, 2, CohortAssignmentType.NONE),
         ]
         self.verify_lists_expected_cohorts(expected_cohorts)
 
@@ -163,14 +157,14 @@ class ListCohortsTestCase(CohortViewsTestCase):
         # Get the cohorts from the course, which will cause auto cohorts to be created.
         actual_cohorts = self.request_list_cohorts(self.course)
         # Get references to the created auto cohorts.
-        auto_cohort_1 = get_cohort_in_course("AutoGroup1", self.course)
-        auto_cohort_2 = get_cohort_in_course("AutoGroup2", self.course)
+        auto_cohort_1 = get_cohort_by_name(self.course.id, "AutoGroup1")
+        auto_cohort_2 = get_cohort_by_name(self.course.id, "AutoGroup2")
         expected_cohorts = [
-            ListCohortsTestCase.create_expected_cohort(self.cohort1, 3),
-            ListCohortsTestCase.create_expected_cohort(self.cohort2, 2),
-            ListCohortsTestCase.create_expected_cohort(self.cohort3, 2),
-            ListCohortsTestCase.create_expected_cohort(auto_cohort_1, 0),
-            ListCohortsTestCase.create_expected_cohort(auto_cohort_2, 0),
+            ListCohortsTestCase.create_expected_cohort(self.cohort1, 3, CohortAssignmentType.NONE),
+            ListCohortsTestCase.create_expected_cohort(self.cohort2, 2, CohortAssignmentType.NONE),
+            ListCohortsTestCase.create_expected_cohort(self.cohort3, 2, CohortAssignmentType.NONE),
+            ListCohortsTestCase.create_expected_cohort(auto_cohort_1, 0, CohortAssignmentType.RANDOM),
+            ListCohortsTestCase.create_expected_cohort(auto_cohort_2, 0, CohortAssignmentType.RANDOM),
         ]
         self.verify_lists_expected_cohorts(expected_cohorts, actual_cohorts)
 
@@ -188,21 +182,32 @@ class ListCohortsTestCase(CohortViewsTestCase):
         self.verify_lists_expected_cohorts([])
 
         # create enrolled users
-        unassigned_users = [UserFactory.create() for _ in range(3)]
-        self._enroll_users(unassigned_users, self.course.id)
+        users = [UserFactory() for _ in range(3)]
+        self._enroll_users(users, self.course.id)
 
         # mimic users accessing the discussion forum
-        for user in unassigned_users:
+        for user in users:
             get_cohort(user, self.course.id)
 
         # verify the default cohort is automatically created
+        default_cohort = get_cohort_by_name(self.course.id, DEFAULT_COHORT_NAME)
         actual_cohorts = self.request_list_cohorts(self.course)
-        default_cohort = get_default_cohort(self.course)
         self.verify_lists_expected_cohorts(
-            [ListCohortsTestCase.create_expected_cohort(default_cohort, len(unassigned_users))],
+            [ListCohortsTestCase.create_expected_cohort(default_cohort, len(users), CohortAssignmentType.RANDOM)],
             actual_cohorts,
         )
 
+        # set auto_cohort_groups and verify the default cohort is no longer listed as RANDOM
+        config_course_cohorts(self.course, [], cohorted=True, auto_cohort_groups=["AutoGroup"])
+        actual_cohorts = self.request_list_cohorts(self.course)
+        auto_cohort = get_cohort_by_name(self.course.id, "AutoGroup")
+        self.verify_lists_expected_cohorts(
+            [
+                ListCohortsTestCase.create_expected_cohort(default_cohort, len(users), CohortAssignmentType.NONE),
+                ListCohortsTestCase.create_expected_cohort(auto_cohort, 0, CohortAssignmentType.RANDOM),
+            ],
+            actual_cohorts,
+        )
 
 class AddCohortTestCase(CohortViewsTestCase):
     """
@@ -236,7 +241,7 @@ class AddCohortTestCase(CohortViewsTestCase):
                 response_dict.get("cohort").get("name"),
                 cohort_name
             )
-        self.assertIsNotNone(get_cohort_in_course(cohort_name, self.course))
+        self.assertIsNotNone(get_cohort_by_name(self.course.id, cohort_name))
 
     def test_non_staff(self):
         """
@@ -316,14 +321,14 @@ class UsersInCohortTestCase(CohortViewsTestCase):
         """
         Verify that non-staff users cannot access `check_users_in_cohort`.
         """
-        cohort = CohortFactory.create(course_id=self.course.id, users=[])
+        cohort = CohortFactory(course_id=self.course.id, users=[])
         self._verify_non_staff_cannot_access(users_in_cohort, "GET", [self.course.id.to_deprecated_string(), cohort.id])
 
     def test_no_users(self):
         """
         Verify that we don't get back any users for a cohort with no users.
         """
-        cohort = CohortFactory.create(course_id=self.course.id, users=[])
+        cohort = CohortFactory(course_id=self.course.id, users=[])
         response_dict = self.request_users_in_cohort(cohort, self.course, 1)
         self.verify_users_in_cohort_and_response(
             cohort,
@@ -338,8 +343,8 @@ class UsersInCohortTestCase(CohortViewsTestCase):
         Verify that we get back all users for a cohort when the cohort has
         <=100 users.
         """
-        users = [UserFactory.create() for _ in range(5)]
-        cohort = CohortFactory.create(course_id=self.course.id, users=users)
+        users = [UserFactory() for _ in range(5)]
+        cohort = CohortFactory(course_id=self.course.id, users=users)
         response_dict = self.request_users_in_cohort(cohort, self.course, 1)
         self.verify_users_in_cohort_and_response(
             cohort,
@@ -353,8 +358,8 @@ class UsersInCohortTestCase(CohortViewsTestCase):
         """
         Verify that pagination works correctly for cohorts with >100 users.
         """
-        users = [UserFactory.create() for _ in range(101)]
-        cohort = CohortFactory.create(course_id=self.course.id, users=users)
+        users = [UserFactory() for _ in range(101)]
+        cohort = CohortFactory(course_id=self.course.id, users=users)
         response_dict_1 = self.request_users_in_cohort(cohort, self.course, 1)
         response_dict_2 = self.request_users_in_cohort(cohort, self.course, 2)
         self.verify_users_in_cohort_and_response(
@@ -377,8 +382,8 @@ class UsersInCohortTestCase(CohortViewsTestCase):
         Verify that we get a blank page of users when requesting page 0 or a
         page greater than the actual number of pages.
         """
-        users = [UserFactory.create() for _ in range(5)]
-        cohort = CohortFactory.create(course_id=self.course.id, users=users)
+        users = [UserFactory() for _ in range(5)]
+        cohort = CohortFactory(course_id=self.course.id, users=users)
         response = self.request_users_in_cohort(cohort, self.course, 0)
         self.verify_users_in_cohort_and_response(
             cohort,
@@ -401,8 +406,8 @@ class UsersInCohortTestCase(CohortViewsTestCase):
         Verify that we get a `HttpResponseBadRequest` (bad request) when the
         page we request isn't a positive integer.
         """
-        users = [UserFactory.create() for _ in range(5)]
-        cohort = CohortFactory.create(course_id=self.course.id, users=users)
+        users = [UserFactory() for _ in range(5)]
+        cohort = CohortFactory(course_id=self.course.id, users=users)
         self.request_users_in_cohort(cohort, self.course, "invalid", should_return_bad_request=True)
         self.request_users_in_cohort(cohort, self.course, -1, should_return_bad_request=True)
 
@@ -484,7 +489,7 @@ class AddUsersToCohortTestCase(CohortViewsTestCase):
         """
         Verify that non-staff users cannot access `check_users_in_cohort`.
         """
-        cohort = CohortFactory.create(course_id=self.course.id, users=[])
+        cohort = CohortFactory(course_id=self.course.id, users=[])
         self._verify_non_staff_cannot_access(
             add_users_to_cohort,
             "POST",
@@ -694,10 +699,10 @@ class AddUsersToCohortTestCase(CohortViewsTestCase):
         Verify that an error is raised when trying to add users to a cohort
         which does not belong to the given course.
         """
-        users = [UserFactory.create(username="user{0}".format(i)) for i in range(3)]
+        users = [UserFactory(username="user{0}".format(i)) for i in range(3)]
         usernames = [user.username for user in users]
         wrong_course_key = SlashSeparatedCourseKey("some", "arbitrary", "course")
-        wrong_course_cohort = CohortFactory.create(name="wrong_cohort", course_id=wrong_course_key, users=[])
+        wrong_course_cohort = CohortFactory(name="wrong_cohort", course_id=wrong_course_key, users=[])
         self.request_add_users_to_cohort(
             ",".join(usernames),
             wrong_course_cohort,
@@ -741,7 +746,7 @@ class RemoveUserFromCohortTestCase(CohortViewsTestCase):
         """
         Verify that non-staff users cannot access `check_users_in_cohort`.
         """
-        cohort = CohortFactory.create(course_id=self.course.id, users=[])
+        cohort = CohortFactory(course_id=self.course.id, users=[])
         self._verify_non_staff_cannot_access(
             remove_user_from_cohort,
             "POST",
@@ -752,7 +757,7 @@ class RemoveUserFromCohortTestCase(CohortViewsTestCase):
         """
         Verify that we get an error message when omitting a username.
         """
-        cohort = CohortFactory.create(course_id=self.course.id, users=[])
+        cohort = CohortFactory(course_id=self.course.id, users=[])
         response_dict = self.request_remove_user_from_cohort(None, cohort)
         self.verify_removed_user_from_cohort(
             None,
@@ -767,7 +772,7 @@ class RemoveUserFromCohortTestCase(CohortViewsTestCase):
         does not exist.
         """
         username = "bogus"
-        cohort = CohortFactory.create(course_id=self.course.id, users=[])
+        cohort = CohortFactory(course_id=self.course.id, users=[])
         response_dict = self.request_remove_user_from_cohort(
             username,
             cohort
@@ -784,8 +789,8 @@ class RemoveUserFromCohortTestCase(CohortViewsTestCase):
         Verify that we can "remove" a user from a cohort even if they are not a
         member of that cohort.
         """
-        user = UserFactory.create()
-        cohort = CohortFactory.create(course_id=self.course.id, users=[])
+        user = UserFactory()
+        cohort = CohortFactory(course_id=self.course.id, users=[])
         response_dict = self.request_remove_user_from_cohort(user.username, cohort)
         self.verify_removed_user_from_cohort(user.username, response_dict, cohort)
 
@@ -793,7 +798,7 @@ class RemoveUserFromCohortTestCase(CohortViewsTestCase):
         """
         Verify that we can remove a user from a cohort.
         """
-        user = UserFactory.create()
-        cohort = CohortFactory.create(course_id=self.course.id, users=[user])
+        user = UserFactory()
+        cohort = CohortFactory(course_id=self.course.id, users=[user])
         response_dict = self.request_remove_user_from_cohort(user.username, cohort)
         self.verify_removed_user_from_cohort(user.username, response_dict, cohort)

common/djangoapps/course_groups/views.py

@@ -48,8 +48,15 @@ def list_cohorts(request, course_key_string):
 
     course = get_course_with_access(request.user, 'staff', course_key)
 
-    all_cohorts = [{'name': c.name, 'id': c.id, 'user_count': c.users.count()}
-                   for c in cohorts.get_course_cohorts(course)]
+    all_cohorts = [
+        {
+            'name': c.name,
+            'id': c.id,
+            'user_count': c.users.count(),
+            'assignment_type': cohorts.CohortAssignmentType.get(c, course)
+        }
+        for c in cohorts.get_course_cohorts(course)
+    ]
 
     return json_http_response({'success': True,
                                'cohorts': all_cohorts})