Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Create auto cohorts when list_cohorts is called and return number of users per cohort.

Browse Source 
TNL-186
This commit is contained in:
cahrens
2014-09-08 16:33:03 -04:00
parent 3a0dea20e1
commit 020094b8fb
6 changed files with 215 additions and 158 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
common/djangoapps/course_groups/cohorts.py
View File

@@ -160,19 +160,30 @@ def get_cohort(user, course_key):
return group
def get_course_cohorts(course_key):
def get_course_cohorts(course):
"""
Get a list of all the cohorts in the given course.
Get a list of all the cohorts in the given course. This will include auto cohorts,
regardless of whether or not the auto cohorts include any users.
Arguments:
course_key: CourseKey
course: the course for which cohorts should be returned
Returns:
A list of CourseUserGroup objects. Empty if there are no cohorts. Does
not check whether the course is cohorted.
"""
# TODO: remove auto_cohort check with TNL-160
if course.auto_cohort:
# Ensure all auto cohorts are created.
for group_name in course.auto_cohort_groups:
CourseUserGroup.objects.get_or_create(
course_id=course.location.course_key,
group_type=CourseUserGroup.COHORT,
name=group_name
)
return list(CourseUserGroup.objects.filter(
course_id=course_key,
course_id=course.location.course_key,
group_type=CourseUserGroup.COHORT
))
@@ -269,13 +280,6 @@ def add_user_to_cohort(cohort, username_or_email):
return (user, previous_cohort)
def get_course_cohort_names(course_key):
"""
Return a list of the cohort names in a course.
"""
return [c.name for c in get_course_cohorts(course_key)]
def delete_empty_cohort(course_key, name):
"""
Remove an empty cohort. Raise ValueError if cohort is not empty.

68
common/djangoapps/course_groups/tests/helpers.py Normal file
View File

@@ -0,0 +1,68 @@
"""
Helper methods for testing cohorts.
"""
from xmodule.modulestore.django import modulestore
from xmodule.modulestore import ModuleStoreEnum
def topic_name_to_id(course, name):
"""
Given a discussion topic name, return an id for that name (includes
course and url_name).
"""
return "{course}_{run}_{name}".format(
course=course.location.course,
run=course.url_name,
name=name
)
def config_course_cohorts(course, discussions,
cohorted,
cohorted_discussions=None,
auto_cohort=None,
auto_cohort_groups=None):
"""
Given a course with no discussion set up, add the discussions and set
the cohort config appropriately.
Arguments:
course: CourseDescriptor
discussions: list of topic names strings. Picks ids and sort_keys
automatically.
cohorted: bool.
cohorted_discussions: optional list of topic names. If specified,
converts them to use the same ids as topic names.
auto_cohort: optional bool.
auto_cohort_groups: optional list of strings
(names of groups to put students into).
Returns:
Nothing -- modifies course in place.
"""
def to_id(name):
return topic_name_to_id(course, name)
topics = dict((name, {"sort_key": "A",
"id": to_id(name)})
for name in discussions)
course.discussion_topics = topics
d = {"cohorted": cohorted}
if cohorted_discussions is not None:
d["cohorted_discussions"] = [to_id(name)
for name in cohorted_discussions]
if auto_cohort is not None:
d["auto_cohort"] = auto_cohort
if auto_cohort_groups is not None:
d["auto_cohort_groups"] = auto_cohort_groups
course.cohort_config = d
try:
# Not implemented for XMLModulestore, which is used by test_cohorts.
modulestore().update_item(course, ModuleStoreEnum.UserID.test)
except NotImplementedError:
pass

216
common/djangoapps/course_groups/tests/test_cohorts.py
View File

@@ -8,6 +8,7 @@ from django.test.utils import override_settings
from student.models import CourseEnrollment
from course_groups.models import CourseUserGroup
from course_groups import cohorts
from course_groups.tests.helpers import topic_name_to_id, config_course_cohorts
from xmodule.modulestore.django import modulestore, clear_existing_modulestores
from opaque_keys.edx.locations import SlashSeparatedCourseKey
@@ -26,61 +27,6 @@ TEST_DATA_MIXED_MODULESTORE = mixed_store_config(TEST_DATA_DIR, TEST_MAPPING)
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
class TestCohorts(django.test.TestCase):
@staticmethod
def topic_name_to_id(course, name):
"""
Given a discussion topic name, return an id for that name (includes
course and url_name).
"""
return "{course}_{run}_{name}".format(course=course.location.course,
run=course.url_name,
name=name)
@staticmethod
def config_course_cohorts(course, discussions,
cohorted,
cohorted_discussions=None,
auto_cohort=None,
auto_cohort_groups=None):
"""
Given a course with no discussion set up, add the discussions and set
the cohort config appropriately.
Arguments:
course: CourseDescriptor
discussions: list of topic names strings. Picks ids and sort_keys
automatically.
cohorted: bool.
cohorted_discussions: optional list of topic names. If specified,
converts them to use the same ids as topic names.
auto_cohort: optional bool.
auto_cohort_groups: optional list of strings
(names of groups to put students into).
Returns:
Nothing -- modifies course in place.
"""
def to_id(name):
return TestCohorts.topic_name_to_id(course, name)
topics = dict((name, {"sort_key": "A",
"id": to_id(name)})
for name in discussions)
course.discussion_topics = topics
d = {"cohorted": cohorted}
if cohorted_discussions is not None:
d["cohorted_discussions"] = [to_id(name)
for name in cohorted_discussions]
if auto_cohort is not None:
d["auto_cohort"] = auto_cohort
if auto_cohort_groups is not None:
d["auto_cohort_groups"] = auto_cohort_groups
course.cohort_config = d
def setUp(self):
"""
Make sure that course is reloaded every time--clear out the modulestore.
@@ -96,7 +42,7 @@ class TestCohorts(django.test.TestCase):
self.assertFalse(course.is_cohorted)
self.assertFalse(cohorts.is_course_cohorted(course.id))
self.config_course_cohorts(course, [], cohorted=True)
config_course_cohorts(course, [], cohorted=True)
self.assertTrue(course.is_cohorted)
self.assertTrue(cohorts.is_course_cohorted(course.id))
@@ -116,7 +62,7 @@ class TestCohorts(django.test.TestCase):
user = User.objects.create(username="test", email="a@b.com")
self.assertIsNone(cohorts.get_cohort_id(user, course.id))
self.config_course_cohorts(course, [], cohorted=True)
config_course_cohorts(course, [], cohorted=True)
cohort = CourseUserGroup.objects.create(name="TestCohort",
course_id=course.id,
group_type=CourseUserGroup.COHORT)
@@ -151,7 +97,7 @@ class TestCohorts(django.test.TestCase):
"Course isn't cohorted, so shouldn't have a cohort")
# Make the course cohorted...
self.config_course_cohorts(course, [], cohorted=True)
config_course_cohorts(course, [], cohorted=True)
self.assertEquals(cohorts.get_cohort(user, course.id).id, cohort.id,
"Should find the right cohort")
@@ -178,9 +124,11 @@ class TestCohorts(django.test.TestCase):
cohort.users.add(user1)
# Make the course auto cohorted...
self.config_course_cohorts(course, [], cohorted=True,
auto_cohort=True,
auto_cohort_groups=["AutoGroup"])
config_course_cohorts(
course, [], cohorted=True,
auto_cohort=True,
auto_cohort_groups=["AutoGroup"]
)
self.assertEquals(cohorts.get_cohort(user1, course.id).id, cohort.id,
"user1 should stay put")
@@ -189,17 +137,21 @@ class TestCohorts(django.test.TestCase):
"user2 should be auto-cohorted")
# Now make the group list empty
self.config_course_cohorts(course, [], cohorted=True,
auto_cohort=True,
auto_cohort_groups=[])
config_course_cohorts(
course, [], cohorted=True,
auto_cohort=True,
auto_cohort_groups=[]
)
self.assertEquals(cohorts.get_cohort(user3, course.id), None,
"No groups->no auto-cohorting")
# Now make it different
self.config_course_cohorts(course, [], cohorted=True,
auto_cohort=True,
auto_cohort_groups=["OtherGroup"])
config_course_cohorts(
course, [], cohorted=True,
auto_cohort=True,
auto_cohort_groups=["OtherGroup"]
)
self.assertEquals(cohorts.get_cohort(user3, course.id).name, "OtherGroup",
"New list->new group")
@@ -214,9 +166,11 @@ class TestCohorts(django.test.TestCase):
self.assertFalse(course.is_cohorted)
groups = ["group_{0}".format(n) for n in range(5)]
self.config_course_cohorts(course, [], cohorted=True,
auto_cohort=True,
auto_cohort_groups=groups)
config_course_cohorts(
course, [], cohorted=True,
auto_cohort=True,
auto_cohort_groups=groups
)
# Assign 100 users to cohorts
for i in range(100):
@@ -234,46 +188,73 @@ class TestCohorts(django.test.TestCase):
self.assertGreater(num_users, 1)
self.assertLess(num_users, 50)
def test_get_course_cohorts(self):
course1_key = SlashSeparatedCourseKey('a', 'b', 'c')
course2_key = SlashSeparatedCourseKey('e', 'f', 'g')
def test_get_course_cohorts_noop(self):
"""
Tests get_course_cohorts returns an empty list when no cohorts exist.
"""
course = modulestore().get_course(self.toy_course_key)
config_course_cohorts(course, [], cohorted=True)
self.assertEqual([], cohorts.get_course_cohorts(course))
# add some cohorts to course 1
cohort = CourseUserGroup.objects.create(name="TestCohort",
course_id=course1_key,
group_type=CourseUserGroup.COHORT)
def _verify_course_cohorts(self, auto_cohort, expected_cohort_set):
"""
Helper method for testing get_course_cohorts with both manual and auto cohorts.
"""
course = modulestore().get_course(self.toy_course_key)
config_course_cohorts(
course, [], cohorted=True, auto_cohort=auto_cohort,
auto_cohort_groups=["AutoGroup1", "AutoGroup2"]
)
cohort = CourseUserGroup.objects.create(name="TestCohort2",
course_id=course1_key,
group_type=CourseUserGroup.COHORT)
# add manual cohorts to course 1
CourseUserGroup.objects.create(
name="ManualCohort",
course_id=course.location.course_key,
group_type=CourseUserGroup.COHORT
)
# second course should have no cohorts
self.assertEqual(cohorts.get_course_cohorts(course2_key), [])
CourseUserGroup.objects.create(
name="ManualCohort2",
course_id=course.location.course_key,
group_type=CourseUserGroup.COHORT
)
course1_cohorts = sorted([c.name for c in cohorts.get_course_cohorts(course1_key)])
self.assertEqual(course1_cohorts, ['TestCohort', 'TestCohort2'])
cohort_set = {c.name for c in cohorts.get_course_cohorts(course)}
self.assertEqual(cohort_set, expected_cohort_set)
def test_get_course_cohorts_auto_cohort_enabled(self):
"""
Tests that get_course_cohorts returns all cohorts, including auto cohorts,
when auto_cohort is True.
"""
self._verify_course_cohorts(True, {"AutoGroup1", "AutoGroup2", "ManualCohort", "ManualCohort2"})
# TODO: Update test case with TNL-160 (auto cohorts WILL be returned).
def test_get_course_cohorts_auto_cohort_disabled(self):
"""
Tests that get_course_cohorts does not return auto cohorts if auto_cohort is False.
"""
self._verify_course_cohorts(False, {"ManualCohort", "ManualCohort2"})
def test_is_commentable_cohorted(self):
course = modulestore().get_course(self.toy_course_key)
self.assertFalse(course.is_cohorted)
def to_id(name):
return self.topic_name_to_id(course, name)
return topic_name_to_id(course, name)
# no topics
self.assertFalse(cohorts.is_commentable_cohorted(course.id, to_id("General")),
"Course doesn't even have a 'General' topic")
# not cohorted
self.config_course_cohorts(course, ["General", "Feedback"],
cohorted=False)
config_course_cohorts(course, ["General", "Feedback"], cohorted=False)
self.assertFalse(cohorts.is_commentable_cohorted(course.id, to_id("General")),
"Course isn't cohorted")
# cohorted, but top level topics aren't
self.config_course_cohorts(course, ["General", "Feedback"],
cohorted=True)
config_course_cohorts(course, ["General", "Feedback"], cohorted=True)
self.assertTrue(course.is_cohorted)
self.assertFalse(cohorts.is_commentable_cohorted(course.id, to_id("General")),
@@ -284,9 +265,11 @@ class TestCohorts(django.test.TestCase):
"Non-top-level discussion is always cohorted in cohorted courses.")
# cohorted, including "Feedback" top-level topics aren't
self.config_course_cohorts(course, ["General", "Feedback"],
cohorted=True,
cohorted_discussions=["Feedback"])
config_course_cohorts(
course, ["General", "Feedback"],
cohorted=True,
cohorted_discussions=["Feedback"]
)
self.assertTrue(course.is_cohorted)
self.assertFalse(cohorts.is_commentable_cohorted(course.id, to_id("General")),
@@ -305,28 +288,27 @@ class TestCohorts(django.test.TestCase):
self.assertEqual(cohorts.get_cohorted_commentables(course.id), set())
self.config_course_cohorts(course, [], cohorted=True)
config_course_cohorts(course, [], cohorted=True)
self.assertEqual(cohorts.get_cohorted_commentables(course.id), set())
self.config_course_cohorts(
config_course_cohorts(
course, ["General", "Feedback"],
cohorted=True,
cohorted_discussions=["Feedback"]
)
self.assertItemsEqual(
cohorts.get_cohorted_commentables(course.id),
set([self.topic_name_to_id(course, "Feedback")])
set([topic_name_to_id(course, "Feedback")])
)
self.config_course_cohorts(
config_course_cohorts(
course, ["General", "Feedback"],
cohorted=True,
cohorted_discussions=["General", "Feedback"]
)
self.assertItemsEqual(
cohorts.get_cohorted_commentables(course.id),
set([self.topic_name_to_id(course, "General"),
self.topic_name_to_id(course, "Feedback")])
set([topic_name_to_id(course, "General"), topic_name_to_id(course, "Feedback")])
)
self.assertRaises(
Http404,
@@ -442,44 +424,6 @@ class TestCohorts(django.test.TestCase):
lambda: cohorts.add_user_to_cohort(first_cohort, "non_existent_username")
)
def test_get_course_cohort_names(self):
"""
Make sure cohorts.get_course_cohort_names() properly returns a list of cohort
names for a given course.
"""
course = modulestore().get_course(self.toy_course_key)
self.assertItemsEqual(
cohorts.get_course_cohort_names(course.id),
[]
)
self.assertItemsEqual(
cohorts.get_course_cohort_names(SlashSeparatedCourseKey('course', 'does_not', 'exist')),
[]
)
CourseUserGroup.objects.create(
name="FirstCohort",
course_id=course.id,
group_type=CourseUserGroup.COHORT
)
self.assertItemsEqual(
cohorts.get_course_cohort_names(course.id),
["FirstCohort"]
)
CourseUserGroup.objects.create(
name="SecondCohort",
course_id=course.id,
group_type=CourseUserGroup.COHORT
)
self.assertItemsEqual(
cohorts.get_course_cohort_names(course.id),
["FirstCohort", "SecondCohort"]
)
def test_delete_empty_cohort(self):
"""
Make sure that cohorts.delete_empty_cohort() properly removes an empty cohort

52
common/djangoapps/course_groups/tests/test_views.py
View File

@@ -4,6 +4,8 @@ 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
from collections import namedtuple
from django.http import Http404
from django.contrib.auth.models import User
@@ -123,11 +125,19 @@ class ListCohortsTestCase(CohortViewsTestCase):
self.assertItemsEqual(
response_dict.get("cohorts"),
[
{"name": cohort.name, "id": cohort.id}
{"name": cohort.name, "id": cohort.id, "user_count": cohort.user_count}
for cohort in expected_cohorts
]
)
@staticmethod
def create_expected_cohort(cohort, user_count):
"""
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)
def test_non_staff(self):
"""
Verify that we cannot access list_cohorts if we're a non-staff user.
@@ -145,12 +155,44 @@ class ListCohortsTestCase(CohortViewsTestCase):
Verify that cohorts are in response for a course with some cohorts.
"""
self._create_cohorts()
expected_cohorts = CourseUserGroup.objects.filter(
course_id=self.course.id,
group_type=CourseUserGroup.COHORT
)
expected_cohorts = [
ListCohortsTestCase.create_expected_cohort(self.cohort1, 3),
ListCohortsTestCase.create_expected_cohort(self.cohort2, 2),
ListCohortsTestCase.create_expected_cohort(self.cohort3, 2),
]
self.verify_lists_expected_cohorts(self.request_list_cohorts(self.course), expected_cohorts)
def test_auto_cohorts(self):
"""
Verify that auto cohorts are included in the response.
"""
config_course_cohorts(self.course, [], cohorted=True, auto_cohort=True,
auto_cohort_groups=["AutoGroup1", "AutoGroup2"])
# Will create cohort1, cohort2, and cohort3. Auto cohorts remain uncreated.
self._create_cohorts()
# 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 = CourseUserGroup.objects.get(
course_id=self.course.location.course_key,
group_type=CourseUserGroup.COHORT,
name="AutoGroup1"
)
auto_cohort_2 = CourseUserGroup.objects.get(
course_id=self.course.location.course_key,
group_type=CourseUserGroup.COHORT,
name="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),
]
self.verify_lists_expected_cohorts(actual_cohorts, expected_cohorts)
class AddCohortTestCase(CohortViewsTestCase):
"""

6
common/djangoapps/course_groups/views.py
View File

@@ -46,10 +46,10 @@ def list_cohorts(request, course_key_string):
# this is a string when we get it here
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_key_string)
get_course_with_access(request.user, 'staff', course_key)
course = get_course_with_access(request.user, 'staff', course_key)
all_cohorts = [{'name': c.name, 'id': c.id}
for c in cohorts.get_course_cohorts(course_key)]
all_cohorts = [{'name': c.name, 'id': c.id, 'user_count': c.users.count()}
for c in cohorts.get_course_cohorts(course)]
return json_http_response({'success': True,
'cohorts': all_cohorts})

5
lms/djangoapps/django_comment_client/forum/views.py
View File

@@ -11,8 +11,7 @@ import newrelic.agent
from edxmako.shortcuts import render_to_response
from courseware.courses import get_course_with_access
from course_groups.cohorts import (is_course_cohorted, get_cohort_id, is_commentable_cohorted,
get_cohorted_commentables, get_course_cohorts, get_cohort_by_id)
from course_groups.cohorts import is_course_cohorted, get_cohort_id, get_course_cohorts, is_commentable_cohorted
from courseware.access import has_access
from django_comment_client.permissions import cached_has_permission
@@ -52,7 +51,7 @@ def make_course_settings(course, include_category_map=False):
'is_cohorted': is_course_cohorted(course.id),
'allow_anonymous': course.allow_anonymous,
'allow_anonymous_to_peers': course.allow_anonymous_to_peers,
'cohorts': [{"id": str(g.id), "name": g.name} for g in get_course_cohorts(course.id)],
'cohorts': [{"id": str(g.id), "name": g.name} for g in get_course_cohorts(course)],
}
if include_category_map: