diff --git a/common/djangoapps/django_comment_common/migrations/0005_coursediscussionsettings.py b/common/djangoapps/django_comment_common/migrations/0005_coursediscussionsettings.py new file mode 100644 index 0000000000..5ffb378118 --- /dev/null +++ b/common/djangoapps/django_comment_common/migrations/0005_coursediscussionsettings.py @@ -0,0 +1,25 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +import openedx.core.djangoapps.xmodule_django.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('django_comment_common', '0004_auto_20161117_1209'), + ] + + operations = [ + migrations.CreateModel( + name='CourseDiscussionSettings', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('course_id', openedx.core.djangoapps.xmodule_django.models.CourseKeyField(help_text=b'Which course are these settings associated with?', unique=True, max_length=255, db_index=True)), + ('always_divide_inline_discussions', models.BooleanField(default=False)), + ('_divided_discussions', models.TextField(null=True, db_column=b'divided_discussions', blank=True)), + ('division_scheme', models.CharField(default=b'none', max_length=20, choices=[(b'none', b'None'), (b'cohort', b'Cohort'), (b'enrollment_track', b'Enrollment Track')])), + ], + ), + ] diff --git a/common/djangoapps/django_comment_common/models.py b/common/djangoapps/django_comment_common/models.py index 1a6370a0fb..6e8ede6fd4 100644 --- a/common/djangoapps/django_comment_common/models.py +++ b/common/djangoapps/django_comment_common/models.py @@ -1,3 +1,4 @@ +import json import logging from config_models.models import ConfigurationModel @@ -162,3 +163,30 @@ class ForumsConfig(ConfigurationModel): def __unicode__(self): """Simple representation so the admin screen looks less ugly.""" return u"ForumsConfig: timeout={}".format(self.connection_timeout) + + +class CourseDiscussionSettings(models.Model): + course_id = CourseKeyField( + unique=True, + max_length=255, + db_index=True, + help_text="Which course are these settings associated with?", + ) + always_divide_inline_discussions = models.BooleanField(default=False) + _divided_discussions = models.TextField(db_column='divided_discussions', null=True, blank=True) # JSON list + + COHORT = 'cohort' + ENROLLMENT_TRACK = 'enrollment_track' + NONE = 'none' + ASSIGNMENT_TYPE_CHOICES = ((NONE, 'None'), (COHORT, 'Cohort'), (ENROLLMENT_TRACK, 'Enrollment Track')) + division_scheme = models.CharField(max_length=20, choices=ASSIGNMENT_TYPE_CHOICES, default=NONE) + + @property + def divided_discussions(self): + """Jsonify the divided_discussions""" + return json.loads(self._divided_discussions) + + @divided_discussions.setter + def divided_discussions(self, value): + """Un-Jsonify the divided_discussions""" + self._divided_discussions = json.dumps(value) diff --git a/common/djangoapps/django_comment_common/tests.py b/common/djangoapps/django_comment_common/tests.py index a76012bb47..d5db1189f0 100644 --- a/common/djangoapps/django_comment_common/tests.py +++ b/common/djangoapps/django_comment_common/tests.py @@ -1,10 +1,19 @@ +from nose.plugins.attrib import attr + from django.test import TestCase -from opaque_keys.edx.locations import SlashSeparatedCourseKey - from django_comment_common.models import Role +from models import CourseDiscussionSettings +from opaque_keys.edx.locations import SlashSeparatedCourseKey +from openedx.core.djangoapps.course_groups.cohorts import CourseCohortsSettings from student.models import CourseEnrollment, User +from utils import get_course_discussion_settings, set_course_discussion_settings +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory +@attr(shard=1) class RoleAssignmentTest(TestCase): """ Basic checks to make sure our Roles get assigned and unassigned as students @@ -55,3 +64,73 @@ class RoleAssignmentTest(TestCase): # ) # self.assertNotIn(student_role, self.student_user.roles.all()) # self.assertIn(student_role, another_student.roles.all()) + + +@attr(shard=1) +class CourseDiscussionSettingsTest(ModuleStoreTestCase): + + def setUp(self): + super(CourseDiscussionSettingsTest, self).setUp() + self.course = CourseFactory.create() + + def test_get_course_discussion_settings(self): + discussion_settings = get_course_discussion_settings(self.course.id) + self.assertEqual(CourseDiscussionSettings.NONE, discussion_settings.division_scheme) + self.assertEqual([], discussion_settings.divided_discussions) + self.assertFalse(discussion_settings.always_divide_inline_discussions) + + def test_get_course_discussion_settings_legacy_settings(self): + self.course.cohort_config = { + 'cohorted': True, + 'always_cohort_inline_discussions': True, + 'cohorted_discussions': ['foo'] + } + modulestore().update_item(self.course, ModuleStoreEnum.UserID.system) + discussion_settings = get_course_discussion_settings(self.course.id) + self.assertEqual(CourseDiscussionSettings.COHORT, discussion_settings.division_scheme) + self.assertEqual(['foo'], discussion_settings.divided_discussions) + self.assertTrue(discussion_settings.always_divide_inline_discussions) + + def test_get_course_discussion_settings_cohort_settings(self): + CourseCohortsSettings.objects.get_or_create( + course_id=self.course.id, + defaults={ + 'is_cohorted': True, + 'always_cohort_inline_discussions': True, + 'cohorted_discussions': ['foo', 'bar'] + } + ) + discussion_settings = get_course_discussion_settings(self.course.id) + self.assertEqual(CourseDiscussionSettings.COHORT, discussion_settings.division_scheme) + self.assertEqual(['foo', 'bar'], discussion_settings.divided_discussions) + self.assertTrue(discussion_settings.always_divide_inline_discussions) + + def test_set_course_discussion_settings(self): + set_course_discussion_settings( + course_key=self.course.id, + divided_discussions=['cohorted_topic'], + division_scheme=CourseDiscussionSettings.ENROLLMENT_TRACK, + always_divide_inline_discussions=True, + ) + discussion_settings = get_course_discussion_settings(self.course.id) + self.assertEqual(CourseDiscussionSettings.ENROLLMENT_TRACK, discussion_settings.division_scheme) + self.assertEqual(['cohorted_topic'], discussion_settings.divided_discussions) + self.assertTrue(discussion_settings.always_divide_inline_discussions) + + def test_invalid_data_types(self): + exception_msg_template = "Incorrect field type for `{}`. Type must be `{}`" + fields = [ + {'name': 'division_scheme', 'type': str}, + {'name': 'always_divide_inline_discussions', 'type': bool}, + {'name': 'divided_discussions', 'type': list} + ] + invalid_value = 3.14 + + for field in fields: + with self.assertRaises(ValueError) as value_error: + set_course_discussion_settings(self.course.id, **{field['name']: invalid_value}) + + self.assertEqual( + value_error.exception.message, + exception_msg_template.format(field['name'], field['type'].__name__) + ) diff --git a/common/djangoapps/django_comment_common/utils.py b/common/djangoapps/django_comment_common/utils.py index 978c06b1e3..5886131e0d 100644 --- a/common/djangoapps/django_comment_common/utils.py +++ b/common/djangoapps/django_comment_common/utils.py @@ -9,6 +9,10 @@ from django_comment_common.models import ( FORUM_ROLE_STUDENT, Role ) +from openedx.core.djangoapps.course_groups.cohorts import get_legacy_discussion_settings +from request_cache.middleware import request_cached + +from .models import CourseDiscussionSettings class ThreadContext(object): @@ -91,3 +95,47 @@ def are_permissions_roles_seeded(course_id): return False return True + + +@request_cached +def get_course_discussion_settings(course_key): + try: + course_discussion_settings = CourseDiscussionSettings.objects.get(course_id=course_key) + except CourseDiscussionSettings.DoesNotExist: + legacy_discussion_settings = get_legacy_discussion_settings(course_key) + course_discussion_settings, _ = CourseDiscussionSettings.objects.get_or_create( + course_id=course_key, + defaults={ + 'always_divide_inline_discussions': legacy_discussion_settings['always_cohort_inline_discussions'], + 'divided_discussions': legacy_discussion_settings['cohorted_discussions'], + 'division_scheme': CourseDiscussionSettings.COHORT if legacy_discussion_settings['is_cohorted'] + else CourseDiscussionSettings.NONE + } + ) + + return course_discussion_settings + + +def set_course_discussion_settings(course_key, **kwargs): + """ + Set discussion settings for a course. + + Arguments: + course_key: CourseKey + always_divide_inline_discussions (bool): If inline discussions should always be divided. + divided_discussions (list): List of discussion ids. + division_scheme (str): `CourseDiscussionSettings.NONE`, `CourseDiscussionSettings.COHORT`, + or `CourseDiscussionSettings.ENROLLMENT_TRACK` + + Returns: + A CourseDiscussionSettings object. + """ + fields = {'division_scheme': str, 'always_divide_inline_discussions': bool, 'divided_discussions': list} + course_discussion_settings = get_course_discussion_settings(course_key) + for field, field_type in fields.items(): + if field in kwargs: + if not isinstance(kwargs[field], field_type): + raise ValueError("Incorrect field type for `{}`. Type must be `{}`".format(field, field_type.__name__)) + setattr(course_discussion_settings, field, kwargs[field]) + course_discussion_settings.save() + return course_discussion_settings diff --git a/common/lib/xmodule/xmodule/partitions/partitions_service.py b/common/lib/xmodule/xmodule/partitions/partitions_service.py index 39c8dbe930..c970b76888 100644 --- a/common/lib/xmodule/xmodule/partitions/partitions_service.py +++ b/common/lib/xmodule/xmodule/partitions/partitions_service.py @@ -133,7 +133,7 @@ class PartitionService(object): if self._cache and (cache_key in self._cache): return self._cache[cache_key] - user_partition = self._get_user_partition(user_partition_id) + user_partition = self.get_user_partition(user_partition_id) if user_partition is None: raise ValueError( "Configuration problem! No user_partition with id {0} " @@ -148,7 +148,7 @@ class PartitionService(object): return group_id - def _get_user_partition(self, user_partition_id): + def get_user_partition(self, user_partition_id): """ Look for a user partition with a matching id in the course's partitions. Note that this method can return an inactive user partition. diff --git a/common/static/common/js/discussion/views/discussion_thread_list_view.js b/common/static/common/js/discussion/views/discussion_thread_list_view.js index 6bdf7305f1..c79acd4a0b 100644 --- a/common/static/common/js/discussion/views/discussion_thread_list_view.js +++ b/common/static/common/js/discussion/views/discussion_thread_list_view.js @@ -32,8 +32,8 @@ this.retrieveFollowed = function() { return DiscussionThreadListView.prototype.retrieveFollowed.apply(self, arguments); }; - this.chooseCohort = function() { - return DiscussionThreadListView.prototype.chooseCohort.apply(self, arguments); + this.chooseGroup = function() { + return DiscussionThreadListView.prototype.chooseGroup.apply(self, arguments); }; this.chooseFilter = function() { return DiscussionThreadListView.prototype.chooseFilter.apply(self, arguments); @@ -85,7 +85,7 @@ 'click .forum-nav-thread-link': 'threadSelected', 'click .forum-nav-load-more-link': 'loadMorePages', 'change .forum-nav-filter-main-control': 'chooseFilter', - 'change .forum-nav-filter-cohort-control': 'chooseCohort' + 'change .forum-nav-filter-cohort-control': 'chooseGroup' }; DiscussionThreadListView.prototype.initialize = function(options) { @@ -194,7 +194,7 @@ edx.HtmlUtils.append( this.$el, this.template({ - isCohorted: this.courseSettings.get('is_cohorted'), + isDiscussionDivisionEnabled: this.courseSettings.get('is_discussion_division_enabled'), isPrivilegedUser: DiscussionUtil.isPrivilegedUser() }) ); @@ -404,7 +404,7 @@ return $(elem).data('discussion-id'); }).get(); this.retrieveDiscussions(discussionIds); - return this.$('.forum-nav-filter-cohort').toggle($item.data('cohorted') === true); + return this.$('.forum-nav-filter-cohort').toggle($item.data('divided') === true); } }; @@ -413,7 +413,7 @@ return this.retrieveFirstPage(); }; - DiscussionThreadListView.prototype.chooseCohort = function() { + DiscussionThreadListView.prototype.chooseGroup = function() { this.group_id = this.$('.forum-nav-filter-cohort-control :selected').val(); return this.retrieveFirstPage(); }; diff --git a/common/static/common/js/discussion/views/discussion_topic_menu_view.js b/common/static/common/js/discussion/views/discussion_topic_menu_view.js index 9e6797d322..e92d66e0db 100644 --- a/common/static/common/js/discussion/views/discussion_topic_menu_view.js +++ b/common/static/common/js/discussion/views/discussion_topic_menu_view.js @@ -35,7 +35,7 @@ '[data-discussion-id="' + this.getCurrentTopicId() + '"]' )); } else if ($general.length > 0) { - this.setTopic($general); + this.setTopic($general.first()); } else { this.setTopic(this.$('.post-topic option').first()); } diff --git a/common/static/common/js/discussion/views/new_post_view.js b/common/static/common/js/discussion/views/new_post_view.js index 87c5e58705..3ebcafec7f 100644 --- a/common/static/common/js/discussion/views/new_post_view.js +++ b/common/static/common/js/discussion/views/new_post_view.js @@ -51,7 +51,7 @@ threadTypeTemplate; context = _.clone(this.course_settings.attributes); _.extend(context, { - cohort_options: this.getCohortOptions(), + group_options: this.getGroupOptions(), is_commentable_divided: this.is_commentable_divided, mode: this.mode, startHeader: this.startHeader, @@ -84,15 +84,15 @@ return this.mode === 'tab'; }; - NewPostView.prototype.getCohortOptions = function() { + NewPostView.prototype.getGroupOptions = function() { var userGroupId; - if (this.course_settings.get('is_cohorted') && DiscussionUtil.isPrivilegedUser()) { + if (this.course_settings.get('is_discussion_division_enabled') && DiscussionUtil.isPrivilegedUser()) { userGroupId = $('#discussion-container').data('user-group-id'); - return _.map(this.course_settings.get('cohorts'), function(cohort) { + return _.map(this.course_settings.get('groups'), function(group) { return { - value: cohort.id, - text: cohort.name, - selected: cohort.id === userGroupId + value: group.id, + text: group.name, + selected: group.id === userGroupId }; }); } else { @@ -112,7 +112,7 @@ }; NewPostView.prototype.toggleGroupDropdown = function($target) { - if ($target.data('cohorted')) { + if ($target.data('divided')) { $('.js-group-select').prop('disabled', false); return $('.group-selector-wrapper').removeClass('disabled'); } else { diff --git a/common/static/common/js/spec/discussion/view/discussion_thread_list_view_spec.js b/common/static/common/js/spec/discussion/view/discussion_thread_list_view_spec.js index dfd9d15c8f..14d9183ae5 100644 --- a/common/static/common/js/spec/discussion/view/discussion_thread_list_view_spec.js +++ b/common/static/common/js/spec/discussion/view/discussion_thread_list_view_spec.js @@ -62,7 +62,7 @@ ' ' + ' Child' + ' ' + @@ -70,7 +70,7 @@ ' ' + ' Sibling' + ' ' + @@ -79,7 +79,7 @@ ' ' + ' Other Category' + ' ' + @@ -95,11 +95,11 @@ ' ' + ' ' + ' ' + - ' <% if (isCohorted && isPrivilegedUser) { %>' + + ' <% if (isDiscussionDivisionEnabled && isPrivilegedUser) { %>' + ' ' + - ' <% if (isCohorted && isPrivilegedUser) { %>' + + ' <% if (isDiscussionDivisionEnabled && isPrivilegedUser) { %>' + '