From 439acf2df7581b64021f6ed1fc1a2fa1d0676a2b Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sat, 26 Jan 2013 13:06:09 -0500 Subject: [PATCH] Fix docstring and test for get_cohort Returns None if course isn't cohorted, even if cohorts exist. Shouldn't happen on prod, but could if cohorting is turned off mid-course. --- common/djangoapps/course_groups/cohorts.py | 10 +++-- .../djangoapps/course_groups/tests/tests.py | 43 ++++++++----------- 2 files changed, 24 insertions(+), 29 deletions(-) diff --git a/common/djangoapps/course_groups/cohorts.py b/common/djangoapps/course_groups/cohorts.py index 3fe333f9c8..df8b22ef26 100644 --- a/common/djangoapps/course_groups/cohorts.py +++ b/common/djangoapps/course_groups/cohorts.py @@ -70,15 +70,19 @@ def get_cohort(user, course_id): def _get_cohort(user, course_id): """ - Given a django User and a course_id, return the user's cohort. In classes with - auto-cohorting, put the user in a cohort if they aren't in one already. + Given a django User and a course_id, return the user's cohort in that + cohort. + + TODO: In classes with auto-cohorting, put the user in a cohort if they + aren't in one already. Arguments: user: a Django User object. course_id: string in the format 'org/course/run' Returns: - A CourseUserGroup object if the User has a cohort, or None. + A CourseUserGroup object if the course is cohorted and the User has a + cohort, else None. Raises: ValueError if the course_id doesn't exist. diff --git a/common/djangoapps/course_groups/tests/tests.py b/common/djangoapps/course_groups/tests/tests.py index 509797b6f6..0303eaaa55 100644 --- a/common/djangoapps/course_groups/tests/tests.py +++ b/common/djangoapps/course_groups/tests/tests.py @@ -1,32 +1,14 @@ -from nose import SkipTest import django.test from django.contrib.auth.models import User from django.conf import settings from override_settings import override_settings - from course_groups.models import CourseUserGroup from course_groups.cohorts import get_cohort, get_course_cohorts -from xmodule.tests.test_import import BaseCourseTestCase from xmodule.modulestore.django import modulestore -def xml_store_config(data_dir): - return { - 'default': { - 'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore', - 'OPTIONS': { - 'data_dir': data_dir, - 'default_class': 'xmodule.hidden_module.HiddenDescriptor', - } - } -} - -TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT -TEST_DATA_XML_MODULESTORE = xml_store_config(TEST_DATA_DIR) - -@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE) class TestCohorts(django.test.TestCase): def test_get_cohort(self): @@ -36,22 +18,31 @@ class TestCohorts(django.test.TestCase): # Proper fix: give all tests a standard modulestore that uses the test # dir. - raise SkipTest() course = modulestore().get_course("edX/toy/2012_Fall") - cohort = CourseUserGroup.objects.create(name="TestCohort", - course_id=course.id, - group_type=CourseUserGroup.COHORT) + self.assertEqual(course.id, "edX/toy/2012_Fall") user = User.objects.create(username="test", email="a@b.com") other_user = User.objects.create(username="test2", email="a2@b.com") + self.assertIsNone(get_cohort(user, course.id), "No cohort created yet") + + cohort = CourseUserGroup.objects.create(name="TestCohort", + course_id=course.id, + group_type=CourseUserGroup.COHORT) + cohort.users.add(user) - got = get_cohort(user, course.id) - self.assertEquals(got.id, cohort.id, "Should find the right cohort") + self.assertIsNone(get_cohort(user, course.id), + "Course isn't cohorted, so shouldn't have a cohort") - got = get_cohort(other_user, course.id) - self.assertEquals(got, None, "other_user shouldn't have a cohort") + # Make the course cohorted... + course.metadata["cohort_config"] = {"cohorted": True} + + self.assertEquals(get_cohort(user, course.id).id, cohort.id, + "Should find the right cohort") + + self.assertEquals(get_cohort(other_user, course.id), None, + "other_user shouldn't have a cohort") def test_get_course_cohorts(self):