From f72974d4dfc013ed175aa3d920ee9bc3d6ee8e04 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Fri, 12 Sep 2014 21:37:35 -0400 Subject: [PATCH] TNL-328 Server-side work for displaying type of cohort. --- common/djangoapps/course_groups/cohorts.py | 37 ++++- .../djangoapps/course_groups/tests/helpers.py | 41 ++---- .../course_groups/tests/test_cohorts.py | 105 ++++---------- .../course_groups/tests/test_views.py | 137 +++++++++--------- common/djangoapps/course_groups/views.py | 11 +- 5 files changed, 156 insertions(+), 175 deletions(-) diff --git a/common/djangoapps/course_groups/cohorts.py b/common/djangoapps/course_groups/cohorts.py index 4f2fbf5d31..a21d99d31a 100644 --- a/common/djangoapps/course_groups/cohorts.py +++ b/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 diff --git a/common/djangoapps/course_groups/tests/helpers.py b/common/djangoapps/course_groups/tests/helpers.py index 8afa541f11..da367f0eae 100644 --- a/common/djangoapps/course_groups/tests/helpers.py +++ b/common/djangoapps/course_groups/tests/helpers.py @@ -1,10 +1,24 @@ """ 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, diff --git a/common/djangoapps/course_groups/tests/test_cohorts.py b/common/djangoapps/course_groups/tests/test_cohorts.py index fd1a5b0dcd..16d971dded 100644 --- a/common/djangoapps/course_groups/tests/test_cohorts.py +++ b/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, diff --git a/common/djangoapps/course_groups/tests/test_views.py b/common/djangoapps/course_groups/tests/test_views.py index 38f724d1f4..4de119b046 100644 --- a/common/djangoapps/course_groups/tests/test_views.py +++ b/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) diff --git a/common/djangoapps/course_groups/views.py b/common/djangoapps/course_groups/views.py index 5f670f1f83..4326a839c9 100644 --- a/common/djangoapps/course_groups/views.py +++ b/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})