From 01a38c29bedb7276ffee4d6a9d0b7e42c83641f7 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Sun, 24 Feb 2013 18:48:01 -0500 Subject: [PATCH] Add auto-cohorting functionality. See changes in xml-format.md or wiki (dynamic cohorts v1) for spec. --- common/djangoapps/course_groups/cohorts.py | 24 ++++++- .../djangoapps/course_groups/tests/tests.py | 71 +++++++++++++++++-- common/lib/xmodule/xmodule/course_module.py | 24 ++++++- doc/xml-format.md | 8 ++- 4 files changed, 115 insertions(+), 12 deletions(-) diff --git a/common/djangoapps/course_groups/cohorts.py b/common/djangoapps/course_groups/cohorts.py index 155f82e0c7..27f388b498 100644 --- a/common/djangoapps/course_groups/cohorts.py +++ b/common/djangoapps/course_groups/cohorts.py @@ -6,6 +6,7 @@ forums, and to the cohort admin views. from django.contrib.auth.models import User from django.http import Http404 import logging +import random from courseware import courses from student.models import get_user_by_username_or_email @@ -96,9 +97,30 @@ def get_cohort(user, course_id): group_type=CourseUserGroup.COHORT, users__id=user.id) except CourseUserGroup.DoesNotExist: - # TODO: add auto-cohorting logic here once we know what that will be. + # Didn't find the group. We'll go on to create one if needed. + pass + + if not course.auto_cohort: return None + choices = course.auto_cohort_groups + if len(choices) == 0: + # Nowhere to put user + log.warning("Course %s is auto-cohorted, but there are no" + " auto_cohort_groups specified", + course_id) + return None + + # Put user in a random group, creating it if needed + group_name = random.choice(choices) + group, created = CourseUserGroup.objects.get_or_create( + course_id=course_id, + group_type=CourseUserGroup.COHORT, + name=group_name) + + user.course_groups.add(group) + return group + def get_course_cohorts(course_id): """ diff --git a/common/djangoapps/course_groups/tests/tests.py b/common/djangoapps/course_groups/tests/tests.py index b3ad928b39..efed39d536 100644 --- a/common/djangoapps/course_groups/tests/tests.py +++ b/common/djangoapps/course_groups/tests/tests.py @@ -47,7 +47,10 @@ class TestCohorts(django.test.TestCase): @staticmethod def config_course_cohorts(course, discussions, - cohorted, cohorted_discussions=None): + 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. @@ -59,6 +62,9 @@ class TestCohorts(django.test.TestCase): 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. @@ -76,6 +82,12 @@ class TestCohorts(django.test.TestCase): 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.metadata["cohort_config"] = d @@ -89,12 +101,9 @@ class TestCohorts(django.test.TestCase): def test_get_cohort(self): - # Need to fix this, but after we're testing on staging. (Looks like - # problem is that when get_cohort internally tries to look up the - # course.id, it fails, even though we loaded it through the modulestore. - - # Proper fix: give all tests a standard modulestore that uses the test - # dir. + """ + Make sure get_cohort() does the right thing when the course is cohorted + """ course = modulestore().get_course("edX/toy/2012_Fall") self.assertEqual(course.id, "edX/toy/2012_Fall") self.assertFalse(course.is_cohorted) @@ -122,6 +131,54 @@ class TestCohorts(django.test.TestCase): self.assertEquals(get_cohort(other_user, course.id), None, "other_user shouldn't have a cohort") + def test_auto_cohorting(self): + """ + Make sure get_cohort() does the right thing when the course is auto_cohorted + """ + course = modulestore().get_course("edX/toy/2012_Fall") + self.assertEqual(course.id, "edX/toy/2012_Fall") + 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") + + cohort = CourseUserGroup.objects.create(name="TestCohort", + course_id=course.id, + group_type=CourseUserGroup.COHORT) + + # user1 manually added to a cohort + cohort.users.add(user1) + + # Make the course auto cohorted... + self.config_course_cohorts(course, [], cohorted=True, + auto_cohort=True, + auto_cohort_groups=["AutoGroup"]) + + self.assertEquals(get_cohort(user1, course.id).id, cohort.id, + "user1 should stay put") + + self.assertEquals(get_cohort(user2, course.id).name, "AutoGroup", + "user2 should be auto-cohorted") + + # Now make the group list empty + self.config_course_cohorts(course, [], cohorted=True, + auto_cohort=True, + auto_cohort_groups=[]) + + self.assertEquals(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"]) + + self.assertEquals(get_cohort(user3, course.id).name, "OtherGroup", + "New list->new group") + self.assertEquals(get_cohort(user2, course.id).name, "AutoGroup", + "user2 should still be in originally placed cohort") + def test_get_course_cohorts(self): course1_id = 'a/b/c' diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 8b2d5a6c92..2ed780fcae 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -378,6 +378,28 @@ class CourseDescriptor(SequenceDescriptor): return bool(config.get("cohorted")) + @property + def auto_cohort(self): + """ + Return whether the course is auto-cohorted. + """ + if not self.is_cohorted: + return False + + return bool(self.metadata.get("cohort_config", {}).get( + "auto_cohort", False)) + + @property + def auto_cohort_groups(self): + """ + Return the list of groups to put students into. Returns [] if not + specified. Returns specified list even if is_cohorted and/or auto_cohort are + false. + """ + return self.metadata.get("cohort_config", {}).get( + "auto_cohort_groups", []) + + @property def top_level_discussion_topic_ids(self): """ @@ -714,7 +736,7 @@ class CourseDescriptor(SequenceDescriptor): def get_test_center_exam(self, exam_series_code): exams = [exam for exam in self.test_center_exams if exam.exam_series_code == exam_series_code] return exams[0] if len(exams) == 1 else None - + @property def title(self): return self.display_name diff --git a/doc/xml-format.md b/doc/xml-format.md index c59db690a1..f387de1f52 100644 --- a/doc/xml-format.md +++ b/doc/xml-format.md @@ -277,9 +277,11 @@ Supported fields at the course level: * "show_calculator" (value "Yes" if desired) * "days_early_for_beta" -- number of days (floating point ok) early that students in the beta-testers group get to see course content. Can also be specified for any other course element, and overrides values set at higher levels. * "cohort_config" : dictionary with keys - - "cohorted" : boolean. Set to true if this course uses student cohorts. If so, all inline discussions are automatically cohorted, and top-level discussion topics are configurable with an optional 'cohorted': bool parameter (with default value false). - - "cohorted_discussions": list of discussions that should be cohorted. - - ... more to come. ('auto_cohort', how to auto cohort, etc) + - "cohorted" : boolean. Set to true if this course uses student cohorts. If so, all inline discussions are automatically cohorted, and top-level discussion topics are configurable via the cohorted_discussions list. Default is not cohorted). + - "cohorted_discussions": list of discussions that should be cohorted. Any not specified in this list are not cohorted. + - "auto_cohort": Truthy. + - "auto_cohort_groups": ["group name 1", "group name 2", ...] + - If cohorted and auto_cohort is true, automatically put each student into a random group from the auto_cohort_groups list, creating the group if needed. * TODO: there are others