From ea3aebdfe56ccff14c9a493bdf8e0ae402fab2ea Mon Sep 17 00:00:00 2001
From: Victor Shnayder <victor@edx.org>
Date: Thu, 24 Jan 2013 15:46:22 -0500
Subject: [PATCH] add logging, explicit check whether course is cohorted

---
 common/djangoapps/course_groups/cohorts.py | 23 +++++++++++++++++++++-
 doc/xml-format.md                          |  2 +-
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/common/djangoapps/course_groups/cohorts.py b/common/djangoapps/course_groups/cohorts.py
index dfede1147a..33b683776c 100644
--- a/common/djangoapps/course_groups/cohorts.py
+++ b/common/djangoapps/course_groups/cohorts.py
@@ -40,10 +40,18 @@ def is_commentable_cohorted(course_id, commentable_id):
         Http404 if the course doesn't exist.
     """
     course = courses.get_course_by_id(course_id)
-    return commentable_id in course.cohorted_discussions()
+    ans = commentable_id in course.cohorted_discussions()
+    log.debug("is_commentable_cohorted({0}, {1}) = {2}".format(course_id,
+                                                               commentable_id,
+                                                               ans))
+    return ans
 
 
 def get_cohort(user, course_id):
+    c = _get_cohort(user, course_id)
+    log.debug("get_cohort({0}, {1}) = {2}", user, course_id, c.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.
@@ -54,7 +62,20 @@ def get_cohort(user, course_id):
 
     Returns:
         A CourseUserGroup object if the User has a cohort, or None.
+
+    Raises:
+       ValueError if the course_id doesn't exist.
     """
+    # First check whether the course is cohorted (users shouldn't be in a cohort
+    # in non-cohorted courses, but settings can change after )
+    try:
+        course = courses.get_course_by_id(course_id)
+    except Http404:
+        raise ValueError("Invalid course_id")
+
+    if not course.is_cohorted:
+        return None
+
     try:
         group = CourseUserGroup.objects.get(course_id=course_id,
                                             group_type=CourseUserGroup.COHORT,
diff --git a/doc/xml-format.md b/doc/xml-format.md
index afa357aaef..f4fd1054cb 100644
--- a/doc/xml-format.md
+++ b/doc/xml-format.md
@@ -260,7 +260,7 @@ Supported fields at the course level:
 * "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_discussion": list of discussions that should be cohorted.  
+    - "cohorted_discussions": list of discussions that should be cohorted.  
     - ... more to come.  ('auto_cohort', how to auto cohort, etc)
 
 * TODO: there are others