Merge pull request #4777 from edx/anton/sync-groups-and-verticals

Anton/sync groups and verticals

cms/djangoapps/contentstore/views/tests/test_item.py

 """Tests for items views."""
-
+import os
 import json
 from datetime import datetime, timedelta
 import ddt
+from unittest import skipUnless
 
 from mock import patch
 from pytz import UTC
@@ -12,7 +13,7 @@ from django.http import Http404
 from django.test import TestCase
 from django.test.client import RequestFactory
 from django.core.urlresolvers import reverse
-from contentstore.utils import reverse_usage_url
+from contentstore.utils import reverse_usage_url, reverse_course_url
 from contentstore.views.preview import StudioUserService
 
 from contentstore.views.component import (
@@ -179,6 +180,54 @@ 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.
+        """
+        self.course.user_partitions = [UserPartition(
+            0, 'first_partition', 'First Partition',
+            [Group("0", 'alpha'), Group("1", 'beta')]
+        )]
+        self.store.update_item(self.course, self.user.id)
+        root_usage_key = self._create_vertical()
+        resp = self.create_xblock(category='split_test', parent_usage_key=root_usage_key)
+        split_test_usage_key = self.response_usage_key(resp)
+        self.client.ajax_post(
+            reverse_usage_url("xblock_handler", split_test_usage_key), 
+            data={'metadata': {'user_partition_id': str(0)}}
+        )
+        html, __ = self._get_container_preview(split_test_usage_key)
+        self.assertIn('alpha', html)
+        self.assertIn('beta', html)
+
+        # Rename groups in group configuration
+        GROUP_CONFIGURATION_JSON = {
+            u'id': 0,
+            u'name': u'first_partition',
+            u'description': u'First Partition',
+            u'version': 1,
+            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},
+            ],
+        }
+
+        response = self.client.put(
+            reverse_course_url('group_configurations_detail_handler', self.course.id, kwargs={'group_configuration_id': 0}),
+            data=json.dumps(GROUP_CONFIGURATION_JSON),
+            content_type="application/json",
+            HTTP_ACCEPT="application/json",
+            HTTP_X_REQUESTED_WITH="XMLHttpRequest",
+        )
+        self.assertEqual(response.status_code, 201)
+        html, __ = self._get_container_preview(split_test_usage_key)
+        self.assertNotIn('alpha', html)
+        self.assertNotIn('beta', html)
+        self.assertIn('New_NAME_A', html)
+        self.assertIn('New_NAME_B', html)
+
+
 class DeleteItem(ItemTest):
     """Tests for '/xblock' DELETE url."""
     def test_delete_static_page(self):
@@ -806,8 +855,8 @@ class TestEditSplitModule(ItemTest):
         vertical_1 = self.get_item_from_modulestore(split_test.children[1], verify_is_draft=True)
         self.assertEqual("vertical", vertical_0.category)
         self.assertEqual("vertical", vertical_1.category)
-        self.assertEqual("alpha", vertical_0.display_name)
-        self.assertEqual("beta", vertical_1.display_name)
+        self.assertEqual("Group ID 0", vertical_0.display_name)
+        self.assertEqual("Group ID 1", vertical_1.display_name)
 
         # Verify that the group_id_to_child mapping is correct.
         self.assertEqual(2, len(split_test.group_id_to_child))

common/lib/xmodule/xmodule/split_test_module.py

@@ -24,6 +24,8 @@ log = logging.getLogger('edx.' + __name__)
 # Make '_' a no-op so we can scrape strings
 _ = lambda text: text
 
+DEFAULT_GROUP_NAME =  _(u'Group ID {group_id}')
+
 
 class ValidationMessageType(object):
     """
@@ -299,8 +301,16 @@ class SplitTestModule(SplitTestFields, XModule, StudioEditableModule):
         for active_child_descriptor in children:
             active_child = self.system.get_module(active_child_descriptor)
             rendered_child = active_child.render(StudioEditableModule.get_preview_view_name(active_child), context)
+            if active_child.category == 'vertical':
+                group_name, group_id  = self.get_data_for_vertical(active_child)
+                if group_name:
+                    rendered_child.content = rendered_child.content.replace(
+                        DEFAULT_GROUP_NAME.format(group_id=group_id),
+                        group_name
+                    )
             fragment.add_frag_resources(rendered_child)
             html = html + rendered_child.content
+
         return html
 
     def student_view(self, context):
@@ -343,6 +353,19 @@ class SplitTestModule(SplitTestFields, XModule, StudioEditableModule):
         progress = reduce(Progress.add_counts, progresses, None)
         return progress
 
+    def get_data_for_vertical(self, vertical):
+        """
+        Return name and id of a group corresponding to `vertical`.
+        """
+        user_partition = self.descriptor.get_selected_partition()
+        if user_partition:
+            for group in user_partition.groups:
+                group_id = unicode(group.id)
+                child_location = self.group_id_to_child.get(group_id, None)
+                if child_location == vertical.location:
+                    return (group.name, group.id)
+        return (None, None)
+
 
 @XBlock.needs('user_tags')  # pylint: disable=abstract-method
 @XBlock.wants('partitions')
@@ -595,7 +618,7 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor, StudioEditableDes
             "editor_saved should only be called when a mutable modulestore is available"
         modulestore = self.system.modulestore
         dest_usage_key = self.location.replace(category="vertical", name=uuid4().hex)
-        metadata = {'display_name': group.name}
+        metadata = {'display_name': DEFAULT_GROUP_NAME.format(group_id=group.id)}
         modulestore.create_item(
             user_id,
             self.location.course_key,

common/lib/xmodule/xmodule/vertical_module.py

@@ -6,6 +6,7 @@ from xmodule.studio_editable import StudioEditableModule, StudioEditableDescript
 from pkg_resources import resource_string
 from copy import copy
 
+
 # HACK: This shouldn't be hard-coded to two types
 # OBSOLETE: This obsoletes 'type'
 class_priority = ['video', 'problem']

common/test/acceptance/tests/test_studio_split_test.py

@@ -149,11 +149,11 @@ class SplitTest(ContainerBase, SplitTestMixin):
         container.edit()
         component_editor = ComponentEditorView(self.browser, container.locator)
         component_editor.set_select_value_and_save('Group Configuration', 'Configuration 0,1,2')
-        self.verify_groups(container, ['Group 0', 'Group 1', 'Group 2'], ['alpha', 'beta'])
+        self.verify_groups(container, ['Group 0', 'Group 1', 'Group 2'], ['Group ID 0', 'Group ID 1'])
 
         # Reload the page to make sure the groups were persisted.
         container = self.go_to_nested_container_page()
-        self.verify_groups(container, ['Group 0', 'Group 1', 'Group 2'], ['alpha', 'beta'])
+        self.verify_groups(container, ['Group 0', 'Group 1', 'Group 2'], ['Group ID 0', 'Group ID 1'])
 
     @skip("This fails periodically where it fails to trigger the add missing groups action.Dis")
     def test_missing_group(self):
@@ -289,6 +289,20 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
         # Collapse the configuration
         config.toggle()
 
+    def _add_split_test_to_vertical(self, number, group_configuration_metadata=None):
+        """
+        Add split test to vertical #`number`. 
+
+        If `group_configuration_metadata` is not None, use it to assign group configuration to split test.
+        """
+        vertical = self.course_fixture.get_nested_xblocks(category="vertical")[number]
+        if group_configuration_metadata:
+            split_test = XBlockFixtureDesc('split_test', 'Test Content Experiment', metadata=group_configuration_metadata)
+        else:
+            split_test = XBlockFixtureDesc('split_test', 'Test Content Experiment')
+        self.course_fixture.create_xblock(vertical.locator, split_test)
+        return split_test
+
     def populate_course_fixture(self, course_fixture):
         course_fixture.add_advanced_settings({
             u"advanced_modules": {"value": ["split_test"]},
@@ -453,16 +467,6 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
         And I add new advanced module "Content Experiment"
         When I assign created group configuration to the module
         Then I see the module has correct groups
-        And I go to the Group Configuration page in Studio
-        And I edit the name of the group configuration, add new group and remove old one
-        And I go to the unit page in Studio
-        And I edit the unit
-        Then I see the group configuration name is changed in `Group Configuration` dropdown
-        And the group configuration name is changed on container page
-        And I see the module has 2 active groups and one inactive
-        And I see "Add missing groups" link exists
-        When I click on "Add missing groups" link
-        The I see the module has 3 active groups and one inactive
         """
         self.page.visit()
         # Create new group configuration
@@ -475,18 +479,48 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
         # Save the configuration
         config.save()
 
-        unit = self.go_to_unit_page()
-        add_advanced_component(unit, 0, 'split_test')
-        container = self.go_to_nested_container_page()
+        split_test = self._add_split_test_to_vertical(number=0)
+
+        container = ContainerPage(self.browser, split_test.locator)
+        container.visit()
         container.edit()
         component_editor = ComponentEditorView(self.browser, container.locator)
         component_editor.set_select_value_and_save('Group Configuration', 'New Group Configuration Name')
         self.verify_groups(container, ['Group A', 'Group B', 'New group'], [])
 
+    def test_container_page_active_verticals_names_are_synced(self):
+        """
+        Scenario: Ensure that the Content Experiment display synced vertical names and correct groups.
+        Given I have a course with group configuration
+        And I go to the Group Configuration page in Studio
+        And I edit the name of the group configuration, add new group and remove old one
+        And I change the name for the group "New group" to "Second Group"
+        And I go to the Container page in Studio
+        And I edit the Content Experiment
+        Then I see the group configuration name is changed in `Group Configuration` dropdown
+        And the group configuration name is changed on container page
+        And I see the module has 2 active groups and one inactive
+        And I see "Add missing groups" link exists
+        When I click on "Add missing groups" link
+        The I see the module has 3 active groups and one inactive
+        """
+        self.course_fixture._update_xblock(self.course_fixture._course_location, {
+            "metadata": {
+                u"user_partitions": [
+                    UserPartition(0, 'Name of the Group Configuration', 'Description of the group configuration.', [Group("0", 'Group A'), Group("1", 'Group B'), Group("2", 'Group C')]).to_json(),
+                ],
+            },
+        })
+
+        # Add split test to vertical and assign newly created group configuration to it
+        split_test = self._add_split_test_to_vertical(number=0, group_configuration_metadata={'user_partition_id': 0})
+
         self.page.visit()
         config = self.page.group_configurations[0]
         config.edit()
         config.name = "Second Group Configuration Name"
+        # `Group C` -> `Second Group`
+        config.groups[2].name = "Second Group"
         # Add new group
         config.add_group()  # Group D
         # Remove Group A
@@ -494,7 +528,8 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
         # Save the configuration
         config.save()
 
-        container = self.go_to_nested_container_page()
+        container = ContainerPage(self.browser, split_test.locator)
+        container.visit()
         container.edit()
         component_editor = ComponentEditorView(self.browser, container.locator)
         self.assertEqual(
@@ -507,12 +542,12 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
             container.get_xblock_information_message()
         )
         self.verify_groups(
-            container, ['Group B', 'New group'], ['Group A'],
+            container, ['Group B', 'Second Group'], ['Group ID 0'],
             verify_missing_groups_not_present=False
         )
         # Click the add button and verify that the groups were added on the page
         container.add_missing_groups()
-        self.verify_groups(container, ['Group B', 'New group', 'Group D'], ['Group A'])
+        self.verify_groups(container, ['Group B', 'Second Group', 'Group D'], ['Group ID 0'])
 
     def test_can_cancel_creation_of_group_configuration(self):
         """