Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ These are notable changes in edx-platform. This is a rolling list of changes,
in roughly chronological order, most recent first. Add your entries at or near
the top. Include a label indicating the component affected.

Platform: Add group_access field to all xblocks. TNL-670

LMS: Add support for user partitioning based on cohort. TNL-710

Platform: Add base support for cohorted group configurations. TNL-649

Common: Add configurable reset button to units

Studio: Add support xblock validation messages on Studio unit/container page. TNL-683
Expand Down
28 changes: 14 additions & 14 deletions cms/djangoapps/contentstore/views/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from xmodule.modulestore.django import modulestore
from xmodule.contentstore.content import StaticContent
from xmodule.tabs import PDFTextbookTabs
from xmodule.partitions.partitions import UserPartition, Group
from xmodule.partitions.partitions import UserPartition
from xmodule.modulestore import EdxJSONEncoder
from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError
from opaque_keys import InvalidKeyError
Expand Down Expand Up @@ -1173,7 +1173,7 @@ def parse(json_string):
configuration = json.loads(json_string)
except ValueError:
raise GroupConfigurationsValidationError(_("invalid JSON"))

configuration["version"] = UserPartition.VERSION
return configuration

def validate(self):
Expand Down Expand Up @@ -1224,14 +1224,7 @@ def get_user_partition(self):
"""
Get user partition for saving in course.
"""
groups = [Group(g["id"], g["name"]) for g in self.configuration["groups"]]

return UserPartition(
self.configuration["id"],
self.configuration["name"],
self.configuration["description"],
groups
)
return UserPartition.from_json(self.configuration)

@staticmethod
def get_usage_info(course, store):
Expand Down Expand Up @@ -1345,15 +1338,12 @@ def group_configurations_list_handler(request, course_key_string):
if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'):
group_configuration_url = reverse_course_url('group_configurations_list_handler', course_key)
course_outline_url = reverse_course_url('course_handler', course_key)
split_test_enabled = SPLIT_TEST_COMPONENT_TYPE in ADVANCED_COMPONENT_TYPES and SPLIT_TEST_COMPONENT_TYPE in course.advanced_modules

configurations = GroupConfiguration.add_usage_info(course, store)

return render_to_response('group_configurations.html', {
'context_course': course,
'group_configuration_url': group_configuration_url,
'course_outline_url': course_outline_url,
'configurations': configurations if split_test_enabled else None,
'configurations': configurations if should_show_group_configurations_page(course) else None,
})
elif "application/json" in request.META.get('HTTP_ACCEPT'):
if request.method == 'POST':
Expand Down Expand Up @@ -1432,6 +1422,16 @@ def group_configurations_detail_handler(request, course_key_string, group_config
return JsonResponse(status=204)


def should_show_group_configurations_page(course):
"""
Returns true if Studio should show the "Group Configurations" page for the specified course.
"""
return (
SPLIT_TEST_COMPONENT_TYPE in ADVANCED_COMPONENT_TYPES and
SPLIT_TEST_COMPONENT_TYPE in course.advanced_modules
)


def _get_course_creator_status(user):
"""
Helper method for returning the course creator status for a particular user,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,17 @@

GROUP_CONFIGURATION_JSON = {
u'name': u'Test name',
u'scheme': u'random',
u'description': u'Test description',
u'version': UserPartition.VERSION,
u'groups': [
{u'name': u'Group A'},
{u'name': u'Group B'},
{
u'name': u'Group A',
u'version': 1,
}, {
u'name': u'Group B',
u'version': 1,
},
],
}

Expand Down Expand Up @@ -229,7 +236,8 @@ def test_can_create_group_configuration(self):
expected = {
u'description': u'Test description',
u'name': u'Test name',
u'version': 1,
u'scheme': u'random',
u'version': UserPartition.VERSION,
u'groups': [
{u'name': u'Group A', u'version': 1},
{u'name': u'Group B', u'version': 1},
Expand Down Expand Up @@ -279,15 +287,16 @@ def _url(self, cid=-1):
kwargs={'group_configuration_id': cid},
)

def test_can_create_new_group_configuration_if_it_is_not_exist(self):
def test_can_create_new_group_configuration_if_it_does_not_exist(self):
"""
PUT new group configuration when no configurations exist in the course.
"""
expected = {
u'id': 999,
u'name': u'Test name',
u'scheme': u'random',
u'description': u'Test description',
u'version': 1,
u'version': UserPartition.VERSION,
u'groups': [
{u'id': 0, u'name': u'Group A', u'version': 1},
{u'id': 1, u'name': u'Group B', u'version': 1},
Expand All @@ -306,12 +315,12 @@ def test_can_create_new_group_configuration_if_it_is_not_exist(self):
self.assertEqual(content, expected)
self.reload_course()
# Verify that user_partitions in the course contains the new group configuration.
user_partititons = self.course.user_partitions
self.assertEqual(len(user_partititons), 1)
self.assertEqual(user_partititons[0].name, u'Test name')
self.assertEqual(len(user_partititons[0].groups), 2)
self.assertEqual(user_partititons[0].groups[0].name, u'Group A')
self.assertEqual(user_partititons[0].groups[1].name, u'Group B')
user_partitions = self.course.user_partitions
self.assertEqual(len(user_partitions), 1)
self.assertEqual(user_partitions[0].name, u'Test name')
self.assertEqual(len(user_partitions[0].groups), 2)
self.assertEqual(user_partitions[0].groups[0].name, u'Group A')
self.assertEqual(user_partitions[0].groups[1].name, u'Group B')

def test_can_edit_group_configuration(self):
"""
Expand All @@ -323,8 +332,9 @@ def test_can_edit_group_configuration(self):
expected = {
u'id': self.ID,
u'name': u'New Test name',
u'scheme': u'random',
u'description': u'New Test description',
u'version': 1,
u'version': UserPartition.VERSION,
u'groups': [
{u'id': 0, u'name': u'New Group Name', u'version': 1},
{u'id': 2, u'name': u'Group C', u'version': 1},
Expand Down Expand Up @@ -430,8 +440,9 @@ def test_group_configuration_not_used(self):
expected = [{
'id': 0,
'name': 'Name 0',
'scheme': 'random',
'description': 'Description 0',
'version': 1,
'version': UserPartition.VERSION,
'groups': [
{'id': 0, 'name': 'Group A', 'version': 1},
{'id': 1, 'name': 'Group B', 'version': 1},
Expand All @@ -454,8 +465,9 @@ def test_can_get_correct_usage_info(self):
expected = [{
'id': 0,
'name': 'Name 0',
'scheme': 'random',
'description': 'Description 0',
'version': 1,
'version': UserPartition.VERSION,
'groups': [
{'id': 0, 'name': 'Group A', 'version': 1},
{'id': 1, 'name': 'Group B', 'version': 1},
Expand All @@ -469,8 +481,9 @@ def test_can_get_correct_usage_info(self):
}, {
'id': 1,
'name': 'Name 1',
'scheme': 'random',
'description': 'Description 1',
'version': 1,
'version': UserPartition.VERSION,
'groups': [
{'id': 0, 'name': 'Group A', 'version': 1},
{'id': 1, 'name': 'Group B', 'version': 1},
Expand All @@ -495,8 +508,9 @@ def test_can_use_one_configuration_in_multiple_experiments(self):
expected = [{
'id': 0,
'name': 'Name 0',
'scheme': 'random',
'description': 'Description 0',
'version': 1,
'version': UserPartition.VERSION,
'groups': [
{'id': 0, 'name': 'Group A', 'version': 1},
{'id': 1, 'name': 'Group B', 'version': 1},
Expand Down
3 changes: 2 additions & 1 deletion cms/djangoapps/contentstore/views/tests/test_item.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,9 @@ def test_split_test_edited(self):
GROUP_CONFIGURATION_JSON = {
u'id': 0,
u'name': u'first_partition',
u'scheme': u'random',
u'description': u'First Partition',
u'version': 1,
u'version': UserPartition.VERSION,
u'groups': [
{u'id': 0, u'name': u'New_NAME_A', u'version': 1},
{u'id': 1, u'name': u'New_NAME_B', u'version': 1},
Expand Down
1 change: 1 addition & 0 deletions cms/djangoapps/models/settings/course_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class CourseMetadata(object):
'name', # from xblock
'tags', # from xblock
'visible_to_staff_only',
'group_access',
]

@classmethod
Expand Down
4 changes: 2 additions & 2 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@
'contentstore',
'course_creators',
'student', # misleading name due to sharing with lms
'course_groups', # not used in cms (yet), but tests run
'openedx.core.djangoapps.course_groups', # not used in cms (yet), but tests run

# Tracking
'track',
Expand Down Expand Up @@ -607,7 +607,7 @@
'reverification',

# User preferences
'user_api',
'openedx.core.djangoapps.user_api',
'django_openid_auth',

'embargo',
Expand Down
2 changes: 1 addition & 1 deletion cms/static/js/models/group.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ define([
defaults: function() {
return {
name: '',
version: null,
version: 1,
order: null
};
},
Expand Down
4 changes: 3 additions & 1 deletion cms/static/js/models/group_configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) {
defaults: function() {
return {
name: '',
scheme: 'random',
description: '',
version: null,
version: 2,
groups: new GroupCollection([
{
name: gettext('Group A'),
Expand Down Expand Up @@ -71,6 +72,7 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) {
return {
id: this.get('id'),
name: this.get('name'),
scheme: this.get('scheme'),
description: this.get('description'),
version: this.get('version'),
groups: this.get('groups').toJSON()
Expand Down
6 changes: 4 additions & 2 deletions cms/static/js/spec/models/group_configuration_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ define([
'id': 10,
'name': 'My Group Configuration',
'description': 'Some description',
'version': 1,
'version': 2,
'scheme': 'random',
'groups': [
{
'version': 1,
Expand All @@ -114,9 +115,10 @@ define([
'id': 10,
'name': 'My Group Configuration',
'description': 'Some description',
'scheme': 'random',
'showGroups': false,
'editing': false,
'version': 1,
'version': 2,
'groups': [
{
'version': 1,
Expand Down
2 changes: 1 addition & 1 deletion cms/templates/group_configurations.html
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ <h3 class="sr">${_("Page Actions")}</h3>
</div>
% else:
<div class="ui-loading">
<p><span class="spin"><i class="icon-refresh"></i></span> <span class="copy">${_("Loading...")}</span></p>
<p><span class="spin"><i class="icon-refresh"></i></span> <span class="copy">${_("Loading")}</span></p>
</div>
% endif
</article>
Expand Down
7 changes: 4 additions & 3 deletions cms/templates/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<%!
from django.utils.translation import ugettext as _
from contentstore import utils
from contentstore.views.course import should_show_group_configurations_page
import urllib
%>

Expand Down Expand Up @@ -312,10 +313,10 @@ <h3 class="title-3">${_("Other Course Settings")}</h3>
<ul>
<li class="nav-item"><a href="${grading_config_url}">${_("Grading")}</a></li>
<li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li>
% if should_show_group_configurations_page(context_course):
<li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
% endif
<li class="nav-item"><a href="${advanced_config_url}">${_("Advanced Settings")}</a></li>
% if "split_test" in context_course.advanced_modules:
<li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
% endif
</ul>
</nav>
% endif
Expand Down
7 changes: 4 additions & 3 deletions cms/templates/settings_advanced.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<%!
from django.utils.translation import ugettext as _
from contentstore import utils
from contentstore.views.course import should_show_group_configurations_page
from django.utils.html import escapejs
%>
<%block name="title">${_("Advanced Settings")}</%block>
Expand Down Expand Up @@ -91,9 +92,9 @@ <h3 class="title-3">${_("Other Course Settings")}</h3>
<li class="nav-item"><a href="${details_url}">${_("Details &amp; Schedule")}</a></li>
<li class="nav-item"><a href="${grading_url}">${_("Grading")}</a></li>
<li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li>
% if "split_test" in context_course.advanced_modules:
<li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
% endif
% if should_show_group_configurations_page(context_course):
<li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
% endif
</ul>
</nav>
% endif
Expand Down
7 changes: 4 additions & 3 deletions cms/templates/settings_graders.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<%namespace name='static' file='static_content.html'/>
<%!
from contentstore import utils
from contentstore.views.course import should_show_group_configurations_page
from django.utils.translation import ugettext as _
%>

Expand Down Expand Up @@ -134,10 +135,10 @@ <h3 class="title-3">${_("Other Course Settings")}</h3>
<ul>
<li class="nav-item"><a href="${detailed_settings_url}">${_("Details &amp; Schedule")}</a></li>
<li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li>
% if should_show_group_configurations_page(context_course):
<li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
% endif
<li class="nav-item"><a href="${advanced_settings_url}">${_("Advanced Settings")}</a></li>
% if "split_test" in context_course.advanced_modules:
<li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
% endif
</ul>
</nav>
% endif
Expand Down
11 changes: 6 additions & 5 deletions cms/templates/widgets/header.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from django.core.urlresolvers import reverse
from django.utils.translation import ugettext as _
from contentstore.context_processors import doc_url
from contentstore.views.course import should_show_group_configurations_page
%>
<%page args="online_help_token"/>

Expand Down Expand Up @@ -81,14 +82,14 @@ <h3 class="title"><span class="label"><span class="label-prefix sr">${_("Course"
<li class="nav-item nav-course-settings-team">
<a href="${course_team_url}">${_("Course Team")}</a>
</li>
% if should_show_group_configurations_page(context_course):
<li class="nav-item nav-course-settings-group-configurations">
<a href="${reverse('contentstore.views.group_configurations_list_handler', kwargs={'course_key_string': unicode(course_key)})}">${_("Group Configurations")}</a>
</li>
% endif
<li class="nav-item nav-course-settings-advanced">
<a href="${advanced_settings_url}">${_("Advanced Settings")}</a>
</li>
% if "split_test" in context_course.advanced_modules:
<li class="nav-item nav-course-settings-group-configurations">
<a href="${reverse('contentstore.views.group_configurations_list_handler', kwargs={'course_key_string': unicode(course_key)})}">${_("Group Configurations")}</a>
</li>
% endif
</ul>
</div>
</div>
Expand Down
Loading