Commit d59be994 by Usman Khalid

Use cohort settings from CourseCohortSettings.

TNL-1258
parent b56e1233
...@@ -1046,6 +1046,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): ...@@ -1046,6 +1046,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
def is_cohorted(self): def is_cohorted(self):
""" """
Return whether the course is cohorted. Return whether the course is cohorted.
Note: No longer used. See openedx.core.djangoapps.course_groups.models.CourseCohortSettings.
""" """
config = self.cohort_config config = self.cohort_config
if config is None: if config is None:
...@@ -1057,6 +1059,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): ...@@ -1057,6 +1059,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
def auto_cohort(self): def auto_cohort(self):
""" """
Return whether the course is auto-cohorted. Return whether the course is auto-cohorted.
Note: No longer used. See openedx.core.djangoapps.course_groups.models.CourseCohortSettings.
""" """
if not self.is_cohorted: if not self.is_cohorted:
return False return False
...@@ -1070,6 +1074,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): ...@@ -1070,6 +1074,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
Return the list of groups to put students into. Returns [] if not 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 specified. Returns specified list even if is_cohorted and/or auto_cohort are
false. false.
Note: No longer used. See openedx.core.djangoapps.course_groups.models.CourseCohortSettings.
""" """
if self.cohort_config is None: if self.cohort_config is None:
return [] return []
...@@ -1090,6 +1096,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): ...@@ -1090,6 +1096,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
Return the set of discussions that is explicitly cohorted. It may be Return the set of discussions that is explicitly cohorted. It may be
the empty set. Note that all inline discussions are automatically the empty set. Note that all inline discussions are automatically
cohorted based on the course's is_cohorted setting. cohorted based on the course's is_cohorted setting.
Note: No longer used. See openedx.core.djangoapps.course_groups.models.CourseCohortSettings.
""" """
config = self.cohort_config config = self.cohort_config
if config is None: if config is None:
...@@ -1103,6 +1111,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor): ...@@ -1103,6 +1111,8 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
This allow to change the default behavior of inline discussions cohorting. By This allow to change the default behavior of inline discussions cohorting. By
setting this to False, all inline discussions are non-cohorted unless their setting this to False, all inline discussions are non-cohorted unless their
ids are specified in cohorted_discussions. ids are specified in cohorted_discussions.
Note: No longer used. See openedx.core.djangoapps.course_groups.models.CourseCohortSettings.
""" """
config = self.cohort_config config = self.cohort_config
if config is None: if config is None:
......
...@@ -643,6 +643,9 @@ class ImportTestCase(BaseCourseTestCase): ...@@ -643,6 +643,9 @@ class ImportTestCase(BaseCourseTestCase):
def test_cohort_config(self): def test_cohort_config(self):
""" """
Check that cohort config parsing works right. Check that cohort config parsing works right.
Note: The cohort config on the CourseModule is no longer used.
See openedx.core.djangoapps.course_groups.models.CourseCohortSettings.
""" """
modulestore = XMLModuleStore(DATA_DIR, source_dirs=['toy']) modulestore = XMLModuleStore(DATA_DIR, source_dirs=['toy'])
......
...@@ -60,13 +60,11 @@ class CohortTestMixin(object): ...@@ -60,13 +60,11 @@ class CohortTestMixin(object):
""" """
Disables cohorting for the current course fixture. Disables cohorting for the current course fixture.
""" """
course_fixture._update_xblock(course_fixture._course_location, { url = LMS_BASE_URL + "/courses/" + course_fixture._course_key + '/cohorts/settings' # pylint: disable=protected-access
"metadata": { data = json.dumps({'is_cohorted': False})
u"cohort_config": { response = course_fixture.session.post(url, data=data, headers=course_fixture.headers)
"cohorted": False self.assertTrue(response.ok, "Failed to disable cohorts")
},
},
})
def add_manual_cohort(self, course_fixture, cohort_name): def add_manual_cohort(self, course_fixture, cohort_name):
""" """
......
...@@ -315,9 +315,9 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase): ...@@ -315,9 +315,9 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase):
MODULESTORE = TEST_DATA_MONGO_MODULESTORE MODULESTORE = TEST_DATA_MONGO_MODULESTORE
@ddt.data( @ddt.data(
# old mongo with cache: number of responses plus 17. TODO: O(n)! # old mongo with cache: 15
(ModuleStoreEnum.Type.mongo, 1, 23, 18), (ModuleStoreEnum.Type.mongo, 1, 21, 15),
(ModuleStoreEnum.Type.mongo, 50, 366, 67), (ModuleStoreEnum.Type.mongo, 50, 315, 15),
# split mongo: 3 queries, regardless of thread response size. # split mongo: 3 queries, regardless of thread response size.
(ModuleStoreEnum.Type.split, 1, 3, 3), (ModuleStoreEnum.Type.split, 1, 3, 3),
(ModuleStoreEnum.Type.split, 50, 3, 3), (ModuleStoreEnum.Type.split, 50, 3, 3),
......
# -*- coding: utf-8 -*- # -*- coding: utf-8 -*-
from datetime import datetime from datetime import datetime
import json import json
import mock
from pytz import UTC from pytz import UTC
from django_comment_client.tests.factories import RoleFactory
from django_comment_client.tests.unicode import UnicodeTestMixin
import django_comment_client.utils as utils
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.test import TestCase from django.test import TestCase
from edxmako import add_lookup from edxmako import add_lookup
import mock
from django_comment_client.tests.factories import RoleFactory from django_comment_client.tests.factories import RoleFactory
from django_comment_client.tests.unicode import UnicodeTestMixin from django_comment_client.tests.unicode import UnicodeTestMixin
from django_comment_client.tests.utils import ContentGroupTestCase from django_comment_client.tests.utils import ContentGroupTestCase
import django_comment_client.utils as utils import django_comment_client.utils as utils
from openedx.core.djangoapps.course_groups.cohorts import set_course_cohort_settings
from student.tests.factories import UserFactory, CourseEnrollmentFactory from student.tests.factories import UserFactory, CourseEnrollmentFactory
from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
...@@ -218,20 +224,35 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): ...@@ -218,20 +224,35 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
check_cohorted_topics([]) # default (empty) cohort config check_cohorted_topics([]) # default (empty) cohort config
self.course.cohort_config = {"cohorted": False, "cohorted_discussions": []} set_course_cohort_settings(course_key=self.course.id, is_cohorted=False, cohorted_discussions=[])
check_cohorted_topics([]) check_cohorted_topics([])
self.course.cohort_config = {"cohorted": True, "cohorted_discussions": []} set_course_cohort_settings(course_key=self.course.id, is_cohorted=True, cohorted_discussions=[])
check_cohorted_topics([]) check_cohorted_topics([])
self.course.cohort_config = {"cohorted": True, "cohorted_discussions": ["Topic_B", "Topic_C"]} set_course_cohort_settings(
course_key=self.course.id,
is_cohorted=True,
cohorted_discussions=["Topic_B", "Topic_C"],
always_cohort_inline_discussions=False,
)
check_cohorted_topics(["Topic_B", "Topic_C"]) check_cohorted_topics(["Topic_B", "Topic_C"])
self.course.cohort_config = {"cohorted": True, "cohorted_discussions": ["Topic_A", "Some_Other_Topic"]} set_course_cohort_settings(
course_key=self.course.id,
is_cohorted=True,
cohorted_discussions=["Topic_A", "Some_Other_Topic"],
always_cohort_inline_discussions=False,
)
check_cohorted_topics(["Topic_A"]) check_cohorted_topics(["Topic_A"])
# unlikely case, but make sure it works. # unlikely case, but make sure it works.
self.course.cohort_config = {"cohorted": False, "cohorted_discussions": ["Topic_A"]} set_course_cohort_settings(
course_key=self.course.id,
is_cohorted=False,
cohorted_discussions=["Topic_A"],
always_cohort_inline_discussions=False,
)
check_cohorted_topics([]) check_cohorted_topics([])
def test_single_inline(self): def test_single_inline(self):
...@@ -352,11 +373,11 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): ...@@ -352,11 +373,11 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
check_cohorted(False) check_cohorted(False)
# explicitly disabled cohorting # explicitly disabled cohorting
self.course.cohort_config = {"cohorted": False} set_course_cohort_settings(course_key=self.course.id, is_cohorted=False)
check_cohorted(False) check_cohorted(False)
# explicitly enabled cohorting # explicitly enabled cohorting
self.course.cohort_config = {"cohorted": True} set_course_cohort_settings(course_key=self.course.id, is_cohorted=True)
check_cohorted(True) check_cohorted(True)
def test_tree_with_duplicate_targets(self): def test_tree_with_duplicate_targets(self):
......
...@@ -19,8 +19,9 @@ from django_comment_client.permissions import check_permissions_by_view, cached_ ...@@ -19,8 +19,9 @@ from django_comment_client.permissions import check_permissions_by_view, cached_
from edxmako import lookup_template from edxmako import lookup_template
from courseware.access import has_access from courseware.access import has_access
from openedx.core.djangoapps.course_groups.cohorts import get_cohort_by_id, get_cohort_id, is_commentable_cohorted, \ from openedx.core.djangoapps.course_groups.cohorts import (
is_course_cohorted get_course_cohort_settings, get_cohort_by_id, get_cohort_id, is_commentable_cohorted, is_course_cohorted
)
from openedx.core.djangoapps.course_groups.models import CourseUserGroup from openedx.core.djangoapps.course_groups.models import CourseUserGroup
...@@ -154,8 +155,7 @@ def get_discussion_category_map(course, user): ...@@ -154,8 +155,7 @@ def get_discussion_category_map(course, user):
modules = get_accessible_discussion_modules(course, user) modules = get_accessible_discussion_modules(course, user)
is_course_cohorted = course.is_cohorted course_cohort_settings = get_course_cohort_settings(course.id)
cohorted_discussion_ids = course.cohorted_discussions
for module in modules: for module in modules:
id = module.discussion_id id = module.discussion_id
...@@ -209,16 +209,19 @@ def get_discussion_category_map(course, user): ...@@ -209,16 +209,19 @@ def get_discussion_category_map(course, user):
node[level]["entries"][title] = {"id": entry["id"], node[level]["entries"][title] = {"id": entry["id"],
"sort_key": entry["sort_key"], "sort_key": entry["sort_key"],
"start_date": entry["start_date"], "start_date": entry["start_date"],
"is_cohorted": is_course_cohorted} "is_cohorted": course_cohort_settings.is_cohorted}
# TODO. BUG! : course location is not unique across multiple course runs! # TODO. BUG! : course location is not unique across multiple course runs!
# (I think Kevin already noticed this) Need to send course_id with requests, store it # (I think Kevin already noticed this) Need to send course_id with requests, store it
# in the backend. # in the backend.
for topic, entry in course.discussion_topics.items(): for topic, entry in course.discussion_topics.items():
category_map['entries'][topic] = {"id": entry["id"], category_map['entries'][topic] = {
"sort_key": entry.get("sort_key", topic), "id": entry["id"],
"start_date": datetime.now(UTC()), "sort_key": entry.get("sort_key", topic),
"is_cohorted": is_course_cohorted and entry["id"] in cohorted_discussion_ids} "start_date": datetime.now(UTC()),
"is_cohorted": (course_cohort_settings.is_cohorted and
entry["id"] in course_cohort_settings.cohorted_discussions)
}
_sort_map_entries(category_map, course.discussion_sort_alpha) _sort_map_entries(category_map, course.discussion_sort_alpha)
......
...@@ -58,6 +58,8 @@ from instructor.views.api import generate_unique_password ...@@ -58,6 +58,8 @@ from instructor.views.api import generate_unique_password
from instructor.views.api import _split_input_list, common_exceptions_400 from instructor.views.api import _split_input_list, common_exceptions_400
from instructor_task.api_helper import AlreadyRunningError from instructor_task.api_helper import AlreadyRunningError
from openedx.core.djangoapps.course_groups.cohorts import set_course_cohort_settings
from .test_tools import msk_from_problem_urlname from .test_tools import msk_from_problem_urlname
from ..views.tools import get_extended_due from ..views.tools import get_extended_due
...@@ -2002,8 +2004,7 @@ class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCa ...@@ -2002,8 +2004,7 @@ class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCa
cohorted, and does not when the course is not cohorted. cohorted, and does not when the course is not cohorted.
""" """
url = reverse('get_students_features', kwargs={'course_id': unicode(self.course.id)}) url = reverse('get_students_features', kwargs={'course_id': unicode(self.course.id)})
self.course.cohort_config = {'cohorted': is_cohorted} set_course_cohort_settings(self.course.id, is_cohorted=is_cohorted)
self.store.update_item(self.course, self.instructor.id)
response = self.client.get(url, {}) response = self.client.get(url, {})
res_json = json.loads(response.content) res_json = json.loads(response.content)
......
...@@ -104,6 +104,7 @@ from .tools import ( ...@@ -104,6 +104,7 @@ from .tools import (
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey
from opaque_keys import InvalidKeyError from opaque_keys import InvalidKeyError
from openedx.core.djangoapps.course_groups.cohorts import is_course_cohorted
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
...@@ -1002,7 +1003,7 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=red ...@@ -1002,7 +1003,7 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=red
'goals': _('Goals'), 'goals': _('Goals'),
} }
if course.is_cohorted: if is_course_cohorted(course.id):
# Translators: 'Cohort' refers to a group of students within a course. # Translators: 'Cohort' refers to a group of students within a course.
query_features.append('cohort') query_features.append('cohort')
query_features_names['cohort'] = _('Cohort') query_features_names['cohort'] = _('Cohort')
......
...@@ -56,6 +56,8 @@ from django.utils.translation import ugettext as _ ...@@ -56,6 +56,8 @@ from django.utils.translation import ugettext as _
from microsite_configuration import microsite from microsite_configuration import microsite
from opaque_keys.edx.locations import i4xEncoder from opaque_keys.edx.locations import i4xEncoder
from openedx.core.djangoapps.course_groups.cohorts import is_course_cohorted
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
...@@ -451,6 +453,7 @@ def instructor_dashboard(request, course_id): ...@@ -451,6 +453,7 @@ def instructor_dashboard(request, course_id):
context = { context = {
'course': course, 'course': course,
'course_is_cohorted': is_course_cohorted(course.id),
'staff_access': True, 'staff_access': True,
'admin_access': request.user.is_staff, 'admin_access': request.user.is_staff,
'instructor_access': instructor_access, 'instructor_access': instructor_access,
......
...@@ -34,7 +34,7 @@ from lms.djangoapps.lms_xblock.runtime import LmsPartitionService ...@@ -34,7 +34,7 @@ from lms.djangoapps.lms_xblock.runtime import LmsPartitionService
from openedx.core.djangoapps.course_groups.cohorts import get_cohort from openedx.core.djangoapps.course_groups.cohorts import get_cohort
from openedx.core.djangoapps.course_groups.models import CourseUserGroup from openedx.core.djangoapps.course_groups.models import CourseUserGroup
from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.keys import UsageKey
from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort, is_course_cohorted
from student.models import CourseEnrollment from student.models import CourseEnrollment
...@@ -578,7 +578,8 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input, ...@@ -578,7 +578,8 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input,
TASK_LOG.info(u'%s, Task type: %s, Starting task execution', task_info_string, action_name) TASK_LOG.info(u'%s, Task type: %s, Starting task execution', task_info_string, action_name)
course = get_course_by_id(course_id) course = get_course_by_id(course_id)
cohorts_header = ['Cohort Name'] if course.is_cohorted else [] course_is_cohorted = is_course_cohorted(course.id)
cohorts_header = ['Cohort Name'] if course_is_cohorted else []
experiment_partitions = get_split_user_partitions(course.user_partitions) experiment_partitions = get_split_user_partitions(course.user_partitions)
group_configs_header = [u'Experiment Group ({})'.format(partition.name) for partition in experiment_partitions] group_configs_header = [u'Experiment Group ({})'.format(partition.name) for partition in experiment_partitions]
...@@ -632,7 +633,7 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input, ...@@ -632,7 +633,7 @@ def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input,
} }
cohorts_group_name = [] cohorts_group_name = []
if course.is_cohorted: if course_is_cohorted:
group = get_cohort(student, course_id, assign=False) group = get_cohort(student, course_id, assign=False)
cohorts_group_name.append(group.name if group else '') cohorts_group_name.append(group.name if group else '')
......
...@@ -372,7 +372,7 @@ function goto( mode) ...@@ -372,7 +372,7 @@ function goto( mode)
%if modeflag.get('Manage Groups'): %if modeflag.get('Manage Groups'):
%if instructor_access: %if instructor_access:
%if course.is_cohorted: %if course_is_cohorted:
<p class="is-deprecated">${_("To manage beta tester roles and cohorts, visit the Membership section of the Instructor Dashboard.")}</p> <p class="is-deprecated">${_("To manage beta tester roles and cohorts, visit the Membership section of the Instructor Dashboard.")}</p>
%else: %else:
<p class="is-deprecated">${_("To manage beta tester roles, visit the Membership section of the Instructor Dashboard.")}</p> <p class="is-deprecated">${_("To manage beta tester roles, visit the Membership section of the Instructor Dashboard.")}</p>
......
...@@ -71,12 +71,9 @@ def _cohort_membership_changed(sender, **kwargs): ...@@ -71,12 +71,9 @@ def _cohort_membership_changed(sender, **kwargs):
tracker.emit(event_name, event) tracker.emit(event_name, event)
# A 'default cohort' is an auto-cohort that is automatically created for a course if no auto_cohort_groups have been # A 'default cohort' is an auto-cohort that is automatically created for a course if no cohort with automatic
# specified. It is intended to be used in a cohorted-course for users who have yet to be assigned to a cohort. # assignment have been specified. It is intended to be used in a cohorted-course for users who have yet to be assigned
# Note 1: If an administrator chooses to configure a cohort with the same name, the said cohort will be used as # to a cohort.
# 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" # Translation Note: We are NOT translating this string since it is the constant identifier for the "default group"
# and needed across product boundaries. # and needed across product boundaries.
DEFAULT_COHORT_NAME = "Default Group" DEFAULT_COHORT_NAME = "Default Group"
...@@ -110,7 +107,7 @@ def is_course_cohorted(course_key): ...@@ -110,7 +107,7 @@ def is_course_cohorted(course_key):
Raises: Raises:
Http404 if the course doesn't exist. Http404 if the course doesn't exist.
""" """
return courses.get_course_by_id(course_key).is_cohorted return get_course_cohort_settings(course_key).is_cohorted
def get_cohort_id(user, course_key): def get_cohort_id(user, course_key):
...@@ -135,18 +132,19 @@ def is_commentable_cohorted(course_key, commentable_id): ...@@ -135,18 +132,19 @@ def is_commentable_cohorted(course_key, commentable_id):
Http404 if the course doesn't exist. Http404 if the course doesn't exist.
""" """
course = courses.get_course_by_id(course_key) course = courses.get_course_by_id(course_key)
course_cohort_settings = get_course_cohort_settings(course_key)
if not course.is_cohorted: if not course_cohort_settings.is_cohorted:
# this is the easy case :) # this is the easy case :)
ans = False ans = False
elif ( elif (
commentable_id in course.top_level_discussion_topic_ids or commentable_id in course.top_level_discussion_topic_ids or
course.always_cohort_inline_discussions is False course_cohort_settings.always_cohort_inline_discussions is False
): ):
# top level discussions have to be manually configured as cohorted # top level discussions have to be manually configured as cohorted
# (default is not). # (default is not).
# Same thing for inline discussions if the default is explicitly set to False in settings # Same thing for inline discussions if the default is explicitly set to False in settings
ans = commentable_id in course.cohorted_discussions ans = commentable_id in course_cohort_settings.cohorted_discussions
else: else:
# inline discussions are cohorted by default # inline discussions are cohorted by default
ans = True ans = True
...@@ -162,13 +160,13 @@ def get_cohorted_commentables(course_key): ...@@ -162,13 +160,13 @@ def get_cohorted_commentables(course_key):
Given a course_key return a set of strings representing cohorted commentables. Given a course_key return a set of strings representing cohorted commentables.
""" """
course = courses.get_course_by_id(course_key) course_cohort_settings = get_course_cohort_settings(course_key)
if not course.is_cohorted: if not course_cohort_settings.is_cohorted:
# this is the easy case :) # this is the easy case :)
ans = set() ans = set()
else: else:
ans = course.cohorted_discussions ans = set(course_cohort_settings.cohorted_discussions)
return ans return ans
...@@ -193,12 +191,10 @@ def get_cohort(user, course_key, assign=True): ...@@ -193,12 +191,10 @@ def get_cohort(user, course_key, assign=True):
""" """
# First check whether the course is cohorted (users shouldn't be in a cohort # First check whether the course is cohorted (users shouldn't be in a cohort
# in non-cohorted courses, but settings can change after course starts) # in non-cohorted courses, but settings can change after course starts)
try: course = courses.get_course(course_key)
course = courses.get_course_by_id(course_key) course_cohort_settings = get_course_cohort_settings(course.id)
except Http404:
raise ValueError("Invalid course_key")
if not course.is_cohorted: if not course_cohort_settings.is_cohorted:
return None return None
try: try:
...@@ -232,9 +228,8 @@ def migrate_cohort_settings(course): ...@@ -232,9 +228,8 @@ def migrate_cohort_settings(course):
Migrate all the cohort settings associated with this course from modulestore to mysql. Migrate all the cohort settings associated with this course from modulestore to mysql.
After that we will never touch modulestore for any cohort related settings. After that we will never touch modulestore for any cohort related settings.
""" """
course_id = course.location.course_key
cohort_settings, created = CourseCohortsSettings.objects.get_or_create( cohort_settings, created = CourseCohortsSettings.objects.get_or_create(
course_id=course_id, course_id=course.id,
defaults={ defaults={
'is_cohorted': course.is_cohorted, 'is_cohorted': course.is_cohorted,
'cohorted_discussions': list(course.cohorted_discussions), 'cohorted_discussions': list(course.cohorted_discussions),
...@@ -246,14 +241,14 @@ def migrate_cohort_settings(course): ...@@ -246,14 +241,14 @@ def migrate_cohort_settings(course):
if created: if created:
# Update the manual cohorts already present in CourseUserGroup # Update the manual cohorts already present in CourseUserGroup
manual_cohorts = CourseUserGroup.objects.filter( manual_cohorts = CourseUserGroup.objects.filter(
course_id=course_id, course_id=course.id,
group_type=CourseUserGroup.COHORT group_type=CourseUserGroup.COHORT
).exclude(name__in=course.auto_cohort_groups) ).exclude(name__in=course.auto_cohort_groups)
for cohort in manual_cohorts: for cohort in manual_cohorts:
CourseCohort.create(course_user_group=cohort) CourseCohort.create(course_user_group=cohort)
for group_name in course.auto_cohort_groups: for group_name in course.auto_cohort_groups:
CourseCohort.create(cohort_name=group_name, course_id=course_id, assignment_type=CourseCohort.RANDOM) CourseCohort.create(cohort_name=group_name, course_id=course.id, assignment_type=CourseCohort.RANDOM)
return cohort_settings return cohort_settings
...@@ -454,7 +449,7 @@ def set_course_cohort_settings(course_key, **kwargs): ...@@ -454,7 +449,7 @@ def set_course_cohort_settings(course_key, **kwargs):
A CourseCohortSettings object. A CourseCohortSettings object.
Raises: Raises:
ValueError if course_key is invalid. Http404 if course_key is invalid.
""" """
fields = {'is_cohorted': bool, 'always_cohort_inline_discussions': bool, 'cohorted_discussions': list} fields = {'is_cohorted': bool, 'always_cohort_inline_discussions': bool, 'cohorted_discussions': list}
course_cohort_settings = get_course_cohort_settings(course_key) course_cohort_settings = get_course_cohort_settings(course_key)
...@@ -478,11 +473,11 @@ def get_course_cohort_settings(course_key): ...@@ -478,11 +473,11 @@ def get_course_cohort_settings(course_key):
A CourseCohortSettings object. A CourseCohortSettings object.
Raises: Raises:
ValueError if course_key is invalid. Http404 if course_key is invalid.
""" """
try: try:
course_cohort_settings = CourseCohortsSettings.objects.get(course_id=course_key) course_cohort_settings = CourseCohortsSettings.objects.get(course_id=course_key)
except CourseCohortsSettings.DoesNotExist: except CourseCohortsSettings.DoesNotExist:
course = courses.get_course(course_key) course = courses.get_course_by_id(course_key)
course_cohort_settings = migrate_cohort_settings(course) course_cohort_settings = migrate_cohort_settings(course)
return course_cohort_settings return course_cohort_settings
...@@ -4,13 +4,15 @@ Helper methods for testing cohorts. ...@@ -4,13 +4,15 @@ Helper methods for testing cohorts.
import factory import factory
from factory import post_generation, Sequence from factory import post_generation, Sequence
from factory.django import DjangoModelFactory from factory.django import DjangoModelFactory
import json
from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore import ModuleStoreEnum
from ..models import CourseUserGroup, CourseCohort, CourseCohortsSettings
import json import json
from ..cohorts import get_course_cohort_settings, set_course_cohort_settings
from ..models import CourseUserGroup, CourseCohort, CourseCohortsSettings
class CohortFactory(DjangoModelFactory): class CohortFactory(DjangoModelFactory):
...@@ -67,7 +69,7 @@ def topic_name_to_id(course, name): ...@@ -67,7 +69,7 @@ def topic_name_to_id(course, name):
) )
def config_course_cohorts( def config_course_cohorts_legacy(
course, course,
discussions, discussions,
cohorted, cohorted,
...@@ -77,7 +79,11 @@ def config_course_cohorts( ...@@ -77,7 +79,11 @@ def config_course_cohorts(
): ):
""" """
Given a course with no discussion set up, add the discussions and set Given a course with no discussion set up, add the discussions and set
the cohort config appropriately. the cohort config on the course descriptor.
Since cohort settings are now stored in models.CourseCohortSettings,
this is only used for testing data migration from the CourseDescriptor
to the table.
Arguments: Arguments:
course: CourseDescriptor course: CourseDescriptor
...@@ -105,7 +111,6 @@ def config_course_cohorts( ...@@ -105,7 +111,6 @@ def config_course_cohorts(
if cohorted_discussions is not None: if cohorted_discussions is not None:
config["cohorted_discussions"] = [to_id(name) config["cohorted_discussions"] = [to_id(name)
for name in cohorted_discussions] for name in cohorted_discussions]
if auto_cohort_groups is not None: if auto_cohort_groups is not None:
config["auto_cohort_groups"] = auto_cohort_groups config["auto_cohort_groups"] = auto_cohort_groups
...@@ -119,3 +124,57 @@ def config_course_cohorts( ...@@ -119,3 +124,57 @@ def config_course_cohorts(
modulestore().update_item(course, ModuleStoreEnum.UserID.test) modulestore().update_item(course, ModuleStoreEnum.UserID.test)
except NotImplementedError: except NotImplementedError:
pass pass
def config_course_cohorts(
course,
is_cohorted,
auto_cohorts=[],
manual_cohorts=[],
discussion_topics=[],
cohorted_discussions=[],
always_cohort_inline_discussions=True # pylint: disable=invalid-name
):
"""
Set discussions and configure cohorts for a course.
Arguments:
course: CourseDescriptor
is_cohorted (bool): Is the course cohorted?
auto_cohorts (list): Names of auto cohorts to create.
manual_cohorts (list): Names of manual cohorts to create.
discussion_topics (list): Discussion topic names. Picks ids and
sort_keys automatically.
cohorted_discussions: Discussion topics to cohort. Converts the
list to use the same ids as discussion topic names.
always_cohort_inline_discussions (bool): Whether inline discussions
should be cohorted by default.
Returns:
Nothing -- modifies course in place.
"""
def to_id(name):
return topic_name_to_id(course, name)
set_course_cohort_settings(
course.id,
is_cohorted = is_cohorted,
cohorted_discussions = [to_id(name) for name in cohorted_discussions],
always_cohort_inline_discussions = always_cohort_inline_discussions
)
for cohort_name in auto_cohorts:
cohort = CohortFactory(course_id=course.id, name=cohort_name)
CourseCohortFactory(course_user_group=cohort, assignment_type=CourseCohort.RANDOM)
for cohort_name in manual_cohorts:
cohort = CohortFactory(course_id=course.id, name=cohort_name)
CourseCohortFactory(course_user_group=cohort, assignment_type=CourseCohort.MANUAL)
course.discussion_topics = dict((name, {"sort_key": "A", "id": to_id(name)})
for name in discussion_topics)
try:
# Not implemented for XMLModulestore, which is used by test_cohorts.
modulestore().update_item(course, ModuleStoreEnum.UserID.test)
except NotImplementedError:
pass
...@@ -41,7 +41,7 @@ class TestCohortPartitionScheme(ModuleStoreTestCase): ...@@ -41,7 +41,7 @@ class TestCohortPartitionScheme(ModuleStoreTestCase):
self.course_key = SlashSeparatedCourseKey("edX", "toy", "2012_Fall") self.course_key = SlashSeparatedCourseKey("edX", "toy", "2012_Fall")
self.course = modulestore().get_course(self.course_key) self.course = modulestore().get_course(self.course_key)
config_course_cohorts(self.course, [], cohorted=True) config_course_cohorts(self.course, is_cohorted=True)
self.groups = [Group(10, 'Group 10'), Group(20, 'Group 20')] self.groups = [Group(10, 'Group 10'), Group(20, 'Group 20')]
self.user_partition = UserPartition( self.user_partition = UserPartition(
......
...@@ -25,7 +25,9 @@ from ..cohorts import ( ...@@ -25,7 +25,9 @@ from ..cohorts import (
get_cohort, get_cohort_by_name, get_cohort_by_id, get_cohort, get_cohort_by_name, get_cohort_by_id,
DEFAULT_COHORT_NAME, get_group_info_for_cohort DEFAULT_COHORT_NAME, get_group_info_for_cohort
) )
from .helpers import config_course_cohorts, CohortFactory, CourseCohortFactory, topic_name_to_id from .helpers import (
config_course_cohorts, config_course_cohorts_legacy, CohortFactory, CourseCohortFactory, topic_name_to_id
)
class CohortViewsTestCase(ModuleStoreTestCase): class CohortViewsTestCase(ModuleStoreTestCase):
...@@ -136,7 +138,7 @@ class CourseCohortSettingsHandlerTestCase(CohortViewsTestCase): ...@@ -136,7 +138,7 @@ class CourseCohortSettingsHandlerTestCase(CohortViewsTestCase):
Verify that course_cohort_settings_handler is working for HTTP GET. Verify that course_cohort_settings_handler is working for HTTP GET.
""" """
cohorted_discussions = ['Topic A', 'Topic B'] cohorted_discussions = ['Topic A', 'Topic B']
config_course_cohorts(self.course, [], cohorted=True, cohorted_discussions=cohorted_discussions) config_course_cohorts(self.course, is_cohorted=True, cohorted_discussions=cohorted_discussions)
response = self.get_handler(self.course, handler=course_cohort_settings_handler) response = self.get_handler(self.course, handler=course_cohort_settings_handler)
response['cohorted_discussions'].sort() response['cohorted_discussions'].sort()
...@@ -155,7 +157,7 @@ class CourseCohortSettingsHandlerTestCase(CohortViewsTestCase): ...@@ -155,7 +157,7 @@ class CourseCohortSettingsHandlerTestCase(CohortViewsTestCase):
""" """
Verify that course_cohort_settings_handler is working for HTTP POST. Verify that course_cohort_settings_handler is working for HTTP POST.
""" """
config_course_cohorts(self.course, [], cohorted=True) config_course_cohorts(self.course, is_cohorted=True)
response = self.get_handler(self.course, handler=course_cohort_settings_handler) response = self.get_handler(self.course, handler=course_cohort_settings_handler)
...@@ -176,7 +178,7 @@ class CourseCohortSettingsHandlerTestCase(CohortViewsTestCase): ...@@ -176,7 +178,7 @@ class CourseCohortSettingsHandlerTestCase(CohortViewsTestCase):
""" """
Verify that course_cohort_settings_handler return HTTP 400 if required data field is missing from post data. Verify that course_cohort_settings_handler return HTTP 400 if required data field is missing from post data.
""" """
config_course_cohorts(self.course, [], cohorted=True) config_course_cohorts(self.course, is_cohorted=True)
response = self.put_handler(self.course, expected_response_code=400, handler=course_cohort_settings_handler) response = self.put_handler(self.course, expected_response_code=400, handler=course_cohort_settings_handler)
self.assertEqual("Bad Request", response.get("error")) self.assertEqual("Bad Request", response.get("error"))
...@@ -185,7 +187,7 @@ class CourseCohortSettingsHandlerTestCase(CohortViewsTestCase): ...@@ -185,7 +187,7 @@ class CourseCohortSettingsHandlerTestCase(CohortViewsTestCase):
""" """
Verify that course_cohort_settings_handler return HTTP 400 if field data type is incorrect. Verify that course_cohort_settings_handler return HTTP 400 if field data type is incorrect.
""" """
config_course_cohorts(self.course, [], cohorted=True) config_course_cohorts(self.course, is_cohorted=True)
response = self.put_handler( response = self.put_handler(
self.course, self.course,
...@@ -268,23 +270,21 @@ class CohortHandlerTestCase(CohortViewsTestCase): ...@@ -268,23 +270,21 @@ class CohortHandlerTestCase(CohortViewsTestCase):
""" """
Verify that auto cohorts are included in the response. Verify that auto cohorts are included in the response.
""" """
config_course_cohorts(self.course, [], cohorted=True, config_course_cohorts(self.course, is_cohorted=True, auto_cohorts=["AutoGroup1", "AutoGroup2"])
auto_cohort_groups=["AutoGroup1", "AutoGroup2"])
# Will create cohort1, cohort2, and cohort3. Auto cohorts remain uncreated. # Will create manual cohorts cohort1, cohort2, and cohort3.
self._create_cohorts() self._create_cohorts()
# Get the cohorts from the course, which will cause auto cohorts to be created.
actual_cohorts = self.get_handler(self.course) actual_cohorts = self.get_handler(self.course)
# Get references to the created auto cohorts. # Get references to the created auto cohorts.
auto_cohort_1 = get_cohort_by_name(self.course.id, "AutoGroup1") auto_cohort_1 = get_cohort_by_name(self.course.id, "AutoGroup1")
auto_cohort_2 = get_cohort_by_name(self.course.id, "AutoGroup2") auto_cohort_2 = get_cohort_by_name(self.course.id, "AutoGroup2")
expected_cohorts = [ expected_cohorts = [
CohortHandlerTestCase.create_expected_cohort(auto_cohort_1, 0, CourseCohort.RANDOM),
CohortHandlerTestCase.create_expected_cohort(auto_cohort_2, 0, CourseCohort.RANDOM),
CohortHandlerTestCase.create_expected_cohort(self.cohort1, 3, CourseCohort.MANUAL), CohortHandlerTestCase.create_expected_cohort(self.cohort1, 3, CourseCohort.MANUAL),
CohortHandlerTestCase.create_expected_cohort(self.cohort2, 2, CourseCohort.MANUAL), CohortHandlerTestCase.create_expected_cohort(self.cohort2, 2, CourseCohort.MANUAL),
CohortHandlerTestCase.create_expected_cohort(self.cohort3, 2, CourseCohort.MANUAL), CohortHandlerTestCase.create_expected_cohort(self.cohort3, 2, CourseCohort.MANUAL),
CohortHandlerTestCase.create_expected_cohort(self.cohort4, 2, CourseCohort.RANDOM), CohortHandlerTestCase.create_expected_cohort(self.cohort4, 2, CourseCohort.RANDOM),
CohortHandlerTestCase.create_expected_cohort(auto_cohort_1, 0, CourseCohort.RANDOM),
CohortHandlerTestCase.create_expected_cohort(auto_cohort_2, 0, CourseCohort.RANDOM),
] ]
self.verify_lists_expected_cohorts(expected_cohorts, actual_cohorts) self.verify_lists_expected_cohorts(expected_cohorts, actual_cohorts)
...@@ -295,8 +295,8 @@ class CohortHandlerTestCase(CohortViewsTestCase): ...@@ -295,8 +295,8 @@ class CohortHandlerTestCase(CohortViewsTestCase):
# verify the default cohort is not created when the course is not cohorted # verify the default cohort is not created when the course is not cohorted
self.verify_lists_expected_cohorts([]) self.verify_lists_expected_cohorts([])
# create a cohorted course without any auto_cohort_groups # create a cohorted course without any auto_cohorts
config_course_cohorts(self.course, [], cohorted=True) config_course_cohorts(self.course, is_cohorted=True)
# verify the default cohort is not yet created until a user is assigned # verify the default cohort is not yet created until a user is assigned
self.verify_lists_expected_cohorts([]) self.verify_lists_expected_cohorts([])
...@@ -320,7 +320,7 @@ class CohortHandlerTestCase(CohortViewsTestCase): ...@@ -320,7 +320,7 @@ class CohortHandlerTestCase(CohortViewsTestCase):
# set auto_cohort_groups # set auto_cohort_groups
# these cohort config will have not effect on lms side as we are already done with migrations # these cohort config will have not effect on lms side as we are already done with migrations
config_course_cohorts(self.course, [], cohorted=True, auto_cohort_groups=["AutoGroup"]) config_course_cohorts_legacy(self.course, [], cohorted=True, auto_cohort_groups=["AutoGroup"])
# We should expect the DoesNotExist exception because above cohort config have # We should expect the DoesNotExist exception because above cohort config have
# no effect on lms side so as a result there will be no AutoGroup cohort present # no effect on lms side so as a result there will be no AutoGroup cohort present
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment