Skip to content

E
edx-platform
Commit 095b0727 by Alexander Kryklia Committed by cahrens
Options

Unflag Group Configurations, Render only existing children in staff view of split_test.

parent b875a429
Showing with 134 additions and 46 deletions
cms/djangoapps/contentstore/views/tests/test_group_configurations.py
......@@ -7,7 +7,6 @@ from contentstore.utils import reverse_course_url, reverse_usage_url
from contentstore.views.component import SPLIT_TEST_COMPONENT_TYPE
from contentstore.views.course import GroupConfiguration
from contentstore.tests.utils import CourseTestCase
from util.testing import UrlResetMixin
from xmodule.partitions.partitions import Group, UserPartition
from xmodule.modulestore.tests.factories import ItemFactory
from xmodule.split_test_module import ValidationMessage, ValidationMessageType
......@@ -165,11 +164,10 @@ class GroupConfigurationsBaseTestCase(object):
# pylint: disable=no-member
class GroupConfigurationsListHandlerTestCase(UrlResetMixin, CourseTestCase, GroupConfigurationsBaseTestCase, HelperMethods):
class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurationsBaseTestCase, HelperMethods):
"""
Test cases for group_configurations_list_handler.
"""
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_GROUP_CONFIGURATIONS": True})
def setUp(self):
"""
Set up GroupConfigurationsListHandlerTestCase.
......@@ -261,14 +259,13 @@ class GroupConfigurationsListHandlerTestCase(UrlResetMixin, CourseTestCase, Grou
# pylint: disable=no-member
class GroupConfigurationsDetailHandlerTestCase(UrlResetMixin, CourseTestCase, GroupConfigurationsBaseTestCase, HelperMethods):
class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfigurationsBaseTestCase, HelperMethods):
"""
Test cases for group_configurations_detail_handler.
"""
ID = 0
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_GROUP_CONFIGURATIONS": True})
def setUp(self):
"""
Set up GroupConfigurationsDetailHandlerTestCase.
......@@ -420,12 +417,11 @@ class GroupConfigurationsDetailHandlerTestCase(UrlResetMixin, CourseTestCase, Gr
# pylint: disable=no-member
class GroupConfigurationsUsageInfoTestCase(UrlResetMixin, CourseTestCase, HelperMethods):
class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
"""
Tests for usage information of configurations.
"""
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_GROUP_CONFIGURATIONS": True})
def setUp(self):
super(GroupConfigurationsUsageInfoTestCase, self).setUp()
......@@ -542,7 +538,6 @@ class GroupConfigurationsValidationTestCase(CourseTestCase, HelperMethods):
"""
Tests for validation in Group Configurations.
"""
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_GROUP_CONFIGURATIONS": True})
def setUp(self):
super(GroupConfigurationsValidationTestCase, self).setUp()
......
cms/djangoapps/contentstore/views/tests/test_item.py
......@@ -180,7 +180,6 @@ class GetItem(ItemTest):
self.assertIn('Zooming', html)
@skipUnless(os.environ.get('FEATURE_GROUP_CONFIGURATIONS'), 'Tests Group Configurations feature')
def test_split_test_edited(self):
"""
Test that rename of a group changes display name of child vertical.
......
cms/djangoapps/models/settings/course_metadata.py
......@@ -24,6 +24,7 @@ class CourseMetadata(object):
'graded',
'hide_from_toc',
'pdf_textbooks',
'user_partitions',
'name', # from xblock
'tags', # from xblock
'visible_to_staff_only'
......
cms/envs/common.py
......@@ -104,9 +104,6 @@ FEATURES = {
# Turn off Advanced Security by default
'ADVANCED_SECURITY': False,
# Toggles Group Configuration editing functionality
'ENABLE_GROUP_CONFIGURATIONS': os.environ.get('FEATURE_GROUP_CONFIGURATIONS'),
# Modulestore to use for new courses
'DEFAULT_STORE_FOR_NEW_COURSE': 'mongo',
}
......
cms/static/js/spec/views/group_configuration_spec.js
......@@ -381,7 +381,7 @@ define([
this.view.$('form').submit();
// See error message
expect(this.view.$(SELECTORS.errorMessage)).toContainText(
'Group Configuration name is required'
'Group Configuration name is required.'
);
// No request
expect(requests.length).toBe(0);
......
cms/templates/settings.html
......@@ -330,7 +330,7 @@ require(["domReady!", "jquery", "js/models/settings/course_details", "js/views/s
<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>
<li class="nav-item"><a href="${advanced_config_url}">${_("Advanced Settings")}</a></li>
% if settings.FEATURES.get('ENABLE_GROUP_CONFIGURATIONS') and "split_test" in context_course.advanced_modules:
% 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>
......
cms/templates/settings_advanced.html
......@@ -126,7 +126,7 @@ require(["domReady!", "jquery", "gettext", "js/models/settings/advanced", "js/vi
<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 settings.FEATURES.get('ENABLE_GROUP_CONFIGURATIONS') and "split_test" in context_course.advanced_modules:
% 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>
......
cms/templates/settings_graders.html
......@@ -148,7 +148,7 @@ require(["domReady!", "jquery", "js/views/settings/grading", "js/models/settings
<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>
<li class="nav-item"><a href="${advanced_settings_url}">${_("Advanced Settings")}</a></li>
% if settings.FEATURES.get('ENABLE_GROUP_CONFIGURATIONS') and "split_test" in context_course.advanced_modules:
% 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>
......
cms/templates/widgets/header.html
......@@ -84,7 +84,7 @@
<li class="nav-item nav-course-settings-advanced">
<a href="${advanced_settings_url}">${_("Advanced Settings")}</a>
</li>
% if settings.FEATURES.get('ENABLE_GROUP_CONFIGURATIONS') and "split_test" in context_course.advanced_modules:
% 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>
......
cms/urls.py
......@@ -91,14 +91,10 @@ urlpatterns += patterns(
url(r'^settings/advanced/{}$'.format(settings.COURSE_KEY_PATTERN), 'advanced_settings_handler'),
url(r'^textbooks/{}$'.format(settings.COURSE_KEY_PATTERN), 'textbooks_list_handler'),
url(r'^textbooks/{}/(?P<textbook_id>\d[^/]*)$'.format(settings.COURSE_KEY_PATTERN), 'textbooks_detail_handler'),
)
if settings.FEATURES.get('ENABLE_GROUP_CONFIGURATIONS'):
urlpatterns += patterns('contentstore.views',
url(r'^group_configurations/{}$'.format(settings.COURSE_KEY_PATTERN), 'group_configurations_list_handler'),
url(r'^group_configurations/{}/(?P<group_configuration_id>\d+)/?$'.format(settings.COURSE_KEY_PATTERN),
'group_configurations_detail_handler'),
)
)
js_info_dict = {
'domain': 'djangojs',
......
common/lib/xmodule/xmodule/split_test_module.py
......@@ -6,6 +6,7 @@ import logging
import json
from webob import Response
from uuid import uuid4
from operator import itemgetter
from xmodule.progress import Progress
from xmodule.seq_module import SequenceDescriptor
......@@ -235,24 +236,41 @@ class SplitTestModule(SplitTestFields, XModule, StudioEditableModule):
Render the staff view for a split test module.
"""
fragment = Fragment()
contents = []
active_contents = []
inactive_contents = []
for group_id in self.group_id_to_child:
child_location = self.group_id_to_child[group_id]
for child_location in self.children: # pylint: disable=no-member
child_descriptor = self.get_child_descriptor_by_location(child_location)
child = self.system.get_module(child_descriptor)
rendered_child = child.render(STUDENT_VIEW, context)
fragment.add_frag_resources(rendered_child)
group_name, updated_group_id = self.get_data_for_vertical(child)
contents.append({
'group_id': group_id,
if updated_group_id is None: # inactive group
group_name = child.display_name
updated_group_id = [g_id for g_id, loc in self.group_id_to_child.items() if loc == child_location][0]
inactive_contents.append({
'group_name': _(u'{group_name} (inactive)').format(group_name=group_name),
'id': child.location.to_deprecated_string(),
'content': rendered_child.content
'content': rendered_child.content,
'group_id': updated_group_id,
})
continue
active_contents.append({
'group_name': group_name,
'id': child.location.to_deprecated_string(),
'content': rendered_child.content,
'group_id': updated_group_id,
})
# Sort active and inactive contents by group name.
sorted_active_contents = sorted(active_contents, key=itemgetter('group_name'))
sorted_inactive_contents = sorted(inactive_contents, key=itemgetter('group_name'))
# Use the new template
fragment.add_content(self.system.render_template('split_test_staff_view.html', {
'items': contents,
'items': sorted_active_contents + sorted_inactive_contents,
}))
fragment.add_css('.split-test-child { display: none; }')
fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/split_test_staff.js'))
......
common/test/acceptance/tests/test_studio_split_test.py
......@@ -7,6 +7,7 @@ import os
import math
from unittest import skip, skipUnless
from nose.plugins.attrib import attr
from selenium.webdriver.support.ui import Select
from xmodule.partitions.partitions import Group, UserPartition
from bok_choy.promise import Promise, EmptyPromise
......@@ -17,8 +18,9 @@ from ..pages.studio.overview import CourseOutlinePage, CourseOutlineUnit
from ..pages.studio.settings_advanced import AdvancedSettingsPage
from ..pages.studio.container import ContainerPage
from ..pages.studio.settings_group_configurations import GroupConfigurationsPage
from ..pages.studio.utils import add_advanced_component
from ..pages.studio.utils import add_advanced_component, click_css
from ..pages.xblock.utils import wait_for_xblock_initialization
from ..pages.lms.courseware import CoursewarePage
from .base_studio_test import StudioCourseTest
......@@ -187,7 +189,6 @@ class SplitTest(ContainerBase, SplitTestMixin):
@attr('shard_1')
@skipUnless(os.environ.get('FEATURE_GROUP_CONFIGURATIONS'), 'Tests Group Configurations feature')
class SettingsMenuTest(StudioCourseTest):
"""
Tests that Setting menu is rendered correctly in Studio
......@@ -236,7 +237,6 @@ class SettingsMenuTest(StudioCourseTest):
@attr('shard_1')
@skipUnless(os.environ.get('FEATURE_GROUP_CONFIGURATIONS'), 'Tests Group Configurations feature')
class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
"""
Tests that Group Configurations page works correctly with previously
......@@ -319,6 +319,8 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
"""
Creates a Group Configuration containing a list of groups.
Optionally creates a Content Experiment and associates it with previous Group Configuration.
Returns group configuration or (group configuration, experiment xblock)
"""
# Create a new group configurations
self.course_fixture._update_xblock(self.course_fixture._course_location, {
......@@ -332,16 +334,40 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
if associate_experiment:
# Assign newly created group configuration to experiment
vertical = self.course_fixture.get_nested_xblocks(category="vertical")[0]
self.course_fixture.create_xblock(
vertical.locator,
XBlockFixtureDesc('split_test', 'Test Content Experiment', metadata={'user_partition_id': 0})
)
split_test = XBlockFixtureDesc('split_test', 'Test Content Experiment', metadata={'user_partition_id': 0})
self.course_fixture.create_xblock(vertical.locator, split_test)
# Go to the Group Configuration Page
self.page.visit()
config = self.page.group_configurations[0]
if associate_experiment:
return config, split_test
return config
def publish_unit_in_LMS_and_view(self, courseware_page):
"""
Given course outline page, publish first unit and view it in LMS
"""
self.outline_page.visit()
self.outline_page.expand_all_subsections()
section = self.outline_page.section_at(0)
unit = section.subsection_at(0).unit_at(0).go_to()
# I publish and view in LMS and it is rendered correctly
unit.publish_action.click()
unit.view_published_version()
self.assertEqual(len(self.browser.window_handles), 2)
courseware_page.wait_for_page()
def get_select_options(self, page, selector):
"""
Get list of options of dropdown that is specified by selector on a given page.
"""
select_element = page.q(css=selector)
self.assertTrue(select_element.is_present())
return [option.text for option in Select(select_element[0]).options]
def test_no_group_configurations_added(self):
"""
Scenario: Ensure that message telling me to create a new group configuration is
......@@ -614,12 +640,12 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
Given I have a course without group configurations
And I create new group configuration with 2 default groups
When I set only description and try to save
Then I see error message "Group Configuration name is required"
Then I see error message "Group Configuration name is required."
When I set a name
And I delete the name of one of the groups and try to save
Then I see error message "All groups must have a name"
When I delete the group without name and try to save
Then I see error message "Please add at least two groups"
Then I see error message "Please add at least two groups."
When I add new group and try to save
Then I see the group configuration is saved successfully
"""
......@@ -638,14 +664,13 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
config = self.page.group_configurations[0]
config.description = "Description of the group configuration."
try_to_save_and_verify_error_message("Group Configuration name is required")
try_to_save_and_verify_error_message("Group Configuration name is required.")
# Set required field
config.name = "Name of the Group Configuration"
config.groups[1].name = ''
try_to_save_and_verify_error_message("All groups must have a name")
try_to_save_and_verify_error_message("All groups must have a name.")
config.groups[1].remove()
try_to_save_and_verify_error_message("There must be at least two groups")
config.add_group()
# Save the configuration
......@@ -860,7 +885,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
"""
# Create group configuration and associated experiment
config = self.create_group_configuration_experiment([Group("0", "Group A"), Group("1", "Group B")], True)
config, _ = self.create_group_configuration_experiment([Group("0", "Group A"), Group("1", "Group B")], True)
# Display details view
config.toggle()
......@@ -898,7 +923,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
"""
# Create group configuration and associated experiment
config = self.create_group_configuration_experiment([Group("0", "Group A"), Group("1", "Group B"), Group("2", "Group C")], True)
config, _ = self.create_group_configuration_experiment([Group("0", "Group A"), Group("1", "Group B"), Group("2", "Group C")], True)
# Display details view
config.toggle()
......@@ -948,7 +973,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
"""
# Create a group configuration with an associated experiment and display edit view
config = self.create_group_configuration_experiment([Group("0", "Group A"), Group("1", "Group B")], True)
config, _ = self.create_group_configuration_experiment([Group("0", "Group A"), Group("1", "Group B")], True)
config.edit()
# Check that warning icon and message are present
self.assertTrue(config.edit_warning_icon_is_present)
......@@ -957,3 +982,60 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
"This configuration is currently used in content experiments. If you make changes to the groups, you may need to edit those experiments.",
config.edit_warning_message_text
)
def publish_unit_and_verify_groups_in_LMS(self, courseware_page, group_names):
"""
Publish first unit in LMS and verify that Courseware page has given Groups
"""
self.publish_unit_in_LMS_and_view(courseware_page)
self.assertEqual(u'split_test', courseware_page.xblock_component_type())
self.assertTrue(courseware_page.q(css=".split-test-select").is_present())
rendered_group_names = self.get_select_options(page=courseware_page, selector=".split-test-select")
self.assertListEqual(group_names, rendered_group_names)
def test_split_test_LMS_staff_view(self):
"""
Scenario: Ensure that split test is correctly rendered in LMS staff mode as it is
and after inactive group removal.
Given I have a course with group configurations and split test that assigned to first group configuration
Then I publish split test and view it in LMS in staff view
And it is rendered correctly
Then I go to group configuration and delete group
Then I publish split test and view it in LMS in staff view
And it is rendered correctly
Then I go to split test and delete inactive vertical
Then I publish unit and view unit in LMS in staff view
And it is rendered correctly
"""
config, split_test = self.create_group_configuration_experiment([Group("0", "Group A"), Group("1", "Group B"), Group("2", "Group C")], True)
container = ContainerPage(self.browser, split_test.locator)
# render in LMS correctly
courseware_page = CoursewarePage(self.browser, self.course_id)
self.publish_unit_and_verify_groups_in_LMS(courseware_page, [u'Group A', u'Group B', u'Group C'])
# I go to group configuration and delete group
self.page.visit()
self.page.q(css='.group-toggle').first.click()
config.edit()
config.groups[2].remove()
config.save()
self.page.q(css='.group-toggle').first.click()
self._assert_fields(config, name="Name", description="Description", groups=["Group A", "Group B"])
self.browser.close()
self.browser.switch_to_window(self.browser.window_handles[0])
# render in LMS to see how inactive vertical is rendered
self.publish_unit_and_verify_groups_in_LMS(courseware_page, [u'Group A', u'Group B', u'Group ID 2 (inactive)'])
self.browser.close()
self.browser.switch_to_window(self.browser.window_handles[0])
# I go to split test and delete inactive vertical
container.visit()
container.delete(0)
# render in LMS again
self.publish_unit_and_verify_groups_in_LMS(courseware_page, [u'Group A', u'Group B'])
lms/templates/split_test_author_view.html
......@@ -5,7 +5,7 @@
split_test = context.get('split_test')
user_partition = split_test.descriptor.get_selected_partition()
messages = split_test.descriptor.validation_messages()
show_link = settings.FEATURES.get('ENABLE_GROUP_CONFIGURATIONS') and group_configuration_url is not None
show_link = group_configuration_url is not None
%>
% if is_root and not is_configured:
......
lms/templates/split_test_staff_view.html
......@@ -4,7 +4,7 @@
<select class="split-test-select">
% for idx, item in enumerate(items):
## Translators: The 'Group' here refers to the group of users that has been sorted into group_id
<option value="${item['group_id']}">${_("Group {group_id}").format(group_id=item['group_id'])}</option>
<option value="${item['group_id']}">${item['group_name']}</option>
%endfor
</select>
......
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