From 459c241e3b1d7056bb4e5453ad024d20de681210 Mon Sep 17 00:00:00 2001
From: jmclaus <jmc@edx.org>
Date: Tue, 29 Jul 2014 13:33:38 +0200
Subject: [PATCH] BLD 1020: Easy access to Group Configurations page from
 Content Experiment component.

---
 .../views/pages/group_configurations_spec.js  | 71 +++++++++++++------
 .../js/views/group_configuration_item.js      |  8 ++-
 .../js/views/pages/group_configurations.js    | 28 ++++++++
 .../lib/xmodule/xmodule/split_test_module.py  | 20 ++++++
 .../xmodule/tests/test_split_test_module.py   | 19 ++++-
 .../studio/settings_group_configurations.py   |  7 ++
 common/test/acceptance/pages/studio/unit.py   | 13 ++++
 .../tests/test_studio_split_test.py           | 48 ++++++++++++-
 lms/templates/split_test_author_view.html     |  5 +-
 9 files changed, 190 insertions(+), 29 deletions(-)

diff --git a/cms/static/js/spec/views/pages/group_configurations_spec.js b/cms/static/js/spec/views/pages/group_configurations_spec.js
index e37d95448f..e871ad6a35 100644
--- a/cms/static/js/spec/views/pages/group_configurations_spec.js
+++ b/cms/static/js/spec/views/pages/group_configurations_spec.js
@@ -1,23 +1,19 @@
 define([
     'jquery', 'underscore', 'js/views/pages/group_configurations',
-    'js/collections/group_configuration'
-], function ($, _, GroupConfigurationsPage, GroupConfigurationCollection) {
+    'js/collections/group_configuration', 'js/models/group_configuration', 'js/spec_helpers/edit_helpers'
+], function ($, _, GroupConfigurationsPage, GroupConfigurationCollection, GroupConfigurationModel, view_helpers) {
     'use strict';
     describe('GroupConfigurationsPage', function() {
         var mockGroupConfigurationsPage = readFixtures(
                 'mock/mock-group-configuration-page.underscore'
             ),
-            noGroupConfigurationsTpl = readFixtures(
-                'no-group-configurations.underscore'
-            ),
-            groupConfigurationEditTpl = readFixtures(
-                'group-configuration-edit.underscore'
-            );
+            itemClassName = '.group-configurations-list-item';
 
         var initializePage = function (disableSpy) {
             var view = new GroupConfigurationsPage({
                 el: $('#content'),
                 collection: new GroupConfigurationCollection({
+                    id: 0,
                     name: 'Configuration 1'
                 })
             });
@@ -38,17 +34,17 @@ define([
         };
 
         beforeEach(function () {
-            setFixtures($('<script>', {
-                id: 'no-group-configurations-tpl',
-                type: 'text/template'
-            }).text(noGroupConfigurationsTpl));
+            setFixtures(mockGroupConfigurationsPage);
+            view_helpers.installTemplates([
+                'no-group-configurations', 'group-configuration-edit',
+                'group-configuration-details'
+            ]);
 
-            appendSetFixtures($('<script>', {
-                id: 'group-configuration-edit-tpl',
-                type: 'text/template'
-            }).text(groupConfigurationEditTpl));
-
-            appendSetFixtures(mockGroupConfigurationsPage);
+            this.addMatchers({
+                toBeExpanded: function () {
+                    return Boolean($('a.group-toggle.hide-groups', $(this.actual)).length);
+                }
+            });
         });
 
         describe('Initial display', function() {
@@ -56,7 +52,7 @@ define([
                 var view = initializePage();
                 expect(view.$('.ui-loading')).toBeVisible();
                 view.render();
-                expect(view.$('.no-group-configurations-content')).toBeTruthy();
+                expect(view.$(itemClassName)).toExist()
                 expect(view.$('.ui-loading')).toBeHidden();
             });
         });
@@ -74,10 +70,9 @@ define([
 
             it('I do not see notification message if the model is not changed',
             function() {
-                var expectedMessage = 
-    		        'You have unsaved changes. Do you really want to leave this page?',
-                        view = renderPage(),
-                        message;
+                var expectedMessage ='You have unsaved changes. Do you really want to leave this page?',
+                    view = renderPage(),
+                    message;
 
                 view.collection.at(0).set('name', 'Configuration 2');
                 message = view.onBeforeUnload();
@@ -85,6 +80,36 @@ define([
             });
         });
 
+        describe('Check that Group Configuration will focus and expand depending on content of url hash', function() {
+            beforeEach(function () {
+                spyOn($.fn, 'focus');
+                view_helpers.installTemplate('group-configuration-details');
+                this.view = initializePage(true);
+            });
+
+            it('should focus and expand group configuration if its id is part of url hash', function() {
+                spyOn(this.view, 'getLocationHash').andReturn('#0');
+                this.view.render();
+                // We cannot use .toBeFocused due to flakiness.
+                expect($.fn.focus).toHaveBeenCalled();
+                expect(this.view.$(itemClassName)).toBeExpanded();
+            });
+
+            it('should not focus on any group configuration if url hash is empty', function() {
+                spyOn(this.view, 'getLocationHash').andReturn('');
+                this.view.render();
+                expect($.fn.focus).not.toHaveBeenCalled();
+                expect(this.view.$(itemClassName)).not.toBeExpanded();
+            });
+
+            it('should not focus on any group configuration if url hash contains wrong id', function() {
+                spyOn(this.view, 'getLocationHash').andReturn('#1');
+                this.view.render();
+                expect($.fn.focus).not.toHaveBeenCalled();
+                expect(this.view.$(itemClassName)).not.toBeExpanded();
+            });
+        });
+
         it('create new group configuration', function () {
             var view = renderPage();
 
diff --git a/cms/static/js/views/group_configuration_item.js b/cms/static/js/views/group_configuration_item.js
index c04e686884..a7279f6d65 100644
--- a/cms/static/js/views/group_configuration_item.js
+++ b/cms/static/js/views/group_configuration_item.js
@@ -7,8 +7,11 @@ define([
     'use strict';
     var GroupConfigurationsItem = BaseView.extend({
         tagName: 'section',
-        attributes: {
-            'tabindex': -1
+        attributes: function () {
+            return {
+                'id': this.model.get('id'),
+                'tabindex': -1
+            };
         },
         events: {
             'click .delete': 'deleteConfiguration'
@@ -65,7 +68,6 @@ define([
             }
 
             this.$el.html(this.view.render().el);
-            this.$el.focus();
 
             return this;
         }
diff --git a/cms/static/js/views/pages/group_configurations.js b/cms/static/js/views/pages/group_configurations.js
index c06cb70d78..d36b3f9d0c 100644
--- a/cms/static/js/views/pages/group_configurations.js
+++ b/cms/static/js/views/pages/group_configurations.js
@@ -13,10 +13,15 @@ function ($, _, gettext, BaseView, GroupConfigurationsList) {
         },
 
         render: function() {
+            var hash = this.getLocationHash();
             this.hideLoadingIndicator();
             this.$('.content-primary').append(this.listView.render().el);
             this.addButtonActions();
             this.addWindowActions();
+            if (hash) {
+                // Strip leading '#' to get id string to match
+                this.expandConfiguration(hash.replace('#', ''))
+            }
         },
 
         addButtonActions: function () {
@@ -39,6 +44,29 @@ function ($, _, gettext, BaseView, GroupConfigurationsList) {
                     'You have unsaved changes. Do you really want to leave this page?'
                 );
             }
+        },
+
+        /**
+         * Helper method that returns url hash.
+         * @return {String} Returns anchor part of current url.
+         */
+        getLocationHash: function() {
+            return window.location.hash;
+        },
+
+        /**
+         * Focus on and expand group configuration with peculiar id.
+         * @param {String|Number} Id of the group configuration.
+         */
+        expandConfiguration: function (id) {
+            var groupConfig = this.collection.findWhere({
+                id: parseInt(id)
+            });
+
+            if (groupConfig) {
+                groupConfig.set('showGroups', true);
+                this.$('#' + id).focus();
+            }
         }
     });
 
diff --git a/common/lib/xmodule/xmodule/split_test_module.py b/common/lib/xmodule/xmodule/split_test_module.py
index f8de2bc990..f5cec8c6d0 100644
--- a/common/lib/xmodule/xmodule/split_test_module.py
+++ b/common/lib/xmodule/xmodule/split_test_module.py
@@ -127,6 +127,7 @@ class SplitTestFields(object):
         scope=Scope.content
     )
 
+
 @XBlock.needs('user_tags')  # pylint: disable=abstract-method
 @XBlock.wants('partitions')
 class SplitTestModule(SplitTestFields, XModule, StudioEditableModule):
@@ -266,6 +267,7 @@ class SplitTestModule(SplitTestFields, XModule, StudioEditableModule):
         is_root = root_xblock and root_xblock.location == self.location
         active_groups_preview = None
         inactive_groups_preview = None
+
         if is_root:
             [active_children, inactive_children] = self.descriptor.active_and_inactive_children()
             active_groups_preview = self.studio_render_children(
@@ -281,6 +283,7 @@ class SplitTestModule(SplitTestFields, XModule, StudioEditableModule):
             'is_configured': is_configured,
             'active_groups_preview': active_groups_preview,
             'inactive_groups_preview': inactive_groups_preview,
+            'group_configuration_url': self.descriptor.group_configuration_url,
         }))
         fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/split_test_author_view.js'))
         fragment.initialize_js('SplitTestAuthorView')
@@ -563,6 +566,23 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor, StudioEditableDes
             self.system.modulestore.update_item(self, None)
         return Response()
 
+    @property
+    def group_configuration_url(self):
+        assert hasattr(self.system, 'modulestore') and hasattr(self.system.modulestore, 'get_course'), \
+            "modulestore has to be available"
+
+        course_module = self.system.modulestore.get_course(self.location.course_key)
+        group_configuration_url = None
+        if 'split_test' in course_module.advanced_modules:
+            user_partition = self.get_selected_partition()
+            if user_partition:
+                group_configuration_url = "{url}#{configuration_id}".format(
+                    url='/group_configurations/' + unicode(self.location.course_key),
+                    configuration_id=str(user_partition.id)
+                )
+
+        return group_configuration_url
+
     def _create_vertical_for_group(self, group, user_id):
         """
         Creates a vertical to associate with the group.
diff --git a/common/lib/xmodule/xmodule/tests/test_split_test_module.py b/common/lib/xmodule/xmodule/tests/test_split_test_module.py
index 20e4aa73b5..267f1703df 100644
--- a/common/lib/xmodule/xmodule/tests/test_split_test_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_split_test_module.py
@@ -160,7 +160,8 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
     Unit tests for how split test interacts with Studio.
     """
 
-    def test_render_author_view(self):
+    @patch('xmodule.split_test_module.SplitTestDescriptor.group_configuration_url', return_value='http://example.com')
+    def test_render_author_view(self, group_configuration_url):
         """
         Test the rendering of the Studio author view.
         """
@@ -197,6 +198,22 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
         self.assertIn('HTML FOR GROUP 0', html)
         self.assertIn('HTML FOR GROUP 1', html)
 
+    def test_group_configuration_url(self):
+        """
+        Test creation of correct Group Configuration URL.
+        """
+        mocked_course = Mock(advanced_modules=['split_test'])
+        mocked_modulestore = Mock()
+        mocked_modulestore.get_course.return_value = mocked_course
+        self.split_test_module.system.modulestore = mocked_modulestore
+
+        self.split_test_module.user_partitions = [
+            UserPartition(0, 'first_partition', 'First Partition', [Group("0", 'alpha'), Group("1", 'beta')])
+        ]
+
+        expected_url = '/group_configurations/edX/xml_test_course/101#0'
+        self.assertEqual(expected_url, self.split_test_module.group_configuration_url)
+
     def test_editable_settings(self):
         """
         Test the setting information passed back from editable_metadata_fields.
diff --git a/common/test/acceptance/pages/studio/settings_group_configurations.py b/common/test/acceptance/pages/studio/settings_group_configurations.py
index 2cf17eccf0..500f24f043 100644
--- a/common/test/acceptance/pages/studio/settings_group_configurations.py
+++ b/common/test/acceptance/pages/studio/settings_group_configurations.py
@@ -56,6 +56,13 @@ class GroupConfiguration(object):
         """
         self.find_css('a.group-toggle').first.click()
 
+    @property
+    def is_expanded(self):
+        """
+        Group configuration usage information is expanded.
+        """
+        return self.find_css('a.group-toggle.hide-groups').present
+
     def add_group(self):
         """
         Add new group.
diff --git a/common/test/acceptance/pages/studio/unit.py b/common/test/acceptance/pages/studio/unit.py
index e30ec1f037..2b86cc1fc2 100644
--- a/common/test/acceptance/pages/studio/unit.py
+++ b/common/test/acceptance/pages/studio/unit.py
@@ -179,3 +179,16 @@ class Component(PageObject):
         Click on settings Save button.
         """
         self._click_button('save_settings')
+
+    def go_to_group_configuration_page(self):
+        """
+        Go to the Group Configuration used by the component.
+        """
+        self.q(css=self._bounded_selector('span.message-text a')).first.click()
+
+    @property
+    def group_configuration_link_name(self):
+        """
+        Get Group Configuration name from link.
+        """
+        return self.q(css=self._bounded_selector('span.message-text a')).first.text[0]
diff --git a/common/test/acceptance/tests/test_studio_split_test.py b/common/test/acceptance/tests/test_studio_split_test.py
index cbd0f27a32..1b25e98918 100644
--- a/common/test/acceptance/tests/test_studio_split_test.py
+++ b/common/test/acceptance/tests/test_studio_split_test.py
@@ -713,7 +713,6 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
                 ],
             },
         })
-
         vertical = self.course_fixture.get_nested_xblocks(category="vertical")[0]
         self.course_fixture.create_xblock(
             vertical.locator,
@@ -729,3 +728,50 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
         config.edit()
         self.assertTrue(config.delete_button_is_disabled)
         self.assertIn('Cannot delete when in use by an experiment', config.delete_note)
+
+    def test_easy_access_from_experiment(self):
+        """
+        Scenario: When a Content Experiment uses a Group Configuration,
+        ensure that the link to that Group Configuration works correctly.
+
+        Given I have a course with two Group Configurations
+        And Content Experiment is assigned to one Group Configuration
+        Then I see a link to Group Configuration
+        When I click on the Group Configuration link
+        Then I see the Group Configurations page
+        And I see that appropriate Group Configuration is expanded.
+        """
+        # Create a new group configurations
+        self.course_fixture._update_xblock(self.course_fixture._course_location, {
+            "metadata": {
+                u"user_partitions": [
+                    UserPartition(0, "Name", "Description.", [Group("0", "Group A"), Group("1", "Group B")]).to_json(),
+                    UserPartition(1, 'Name of second Group Configuration', 'Second group configuration.', [Group("0", 'Alpha'), Group("1", 'Beta'), Group("2", 'Gamma')]).to_json(),
+                ],
+            },
+        })
+
+        # Assign newly created group configuration to unit
+        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': 1})
+        )
+
+        unit = UnitPage(self.browser, vertical.locator)
+        unit.visit()
+        experiment = unit.components[0]
+
+        group_configuration_link_name = experiment.group_configuration_link_name
+
+        experiment.go_to_group_configuration_page()
+        self.page.wait_for_page()
+
+        # Appropriate Group Configuration is expanded.
+        self.assertFalse(self.page.group_configurations[0].is_expanded)
+        self.assertTrue(self.page.group_configurations[1].is_expanded)
+
+        self.assertEqual(
+            group_configuration_link_name,
+            self.page.group_configurations[1].name
+        )
diff --git a/lms/templates/split_test_author_view.html b/lms/templates/split_test_author_view.html
index cd4e239811..0e0e9e5fe9 100644
--- a/lms/templates/split_test_author_view.html
+++ b/lms/templates/split_test_author_view.html
@@ -5,6 +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
 %>
 
 % if is_root and not is_configured:
@@ -17,7 +18,9 @@ messages = split_test.descriptor.validation_messages()
     <div class="xblock-message information">
         <p>
             <span class="message-text">
-            ${_("This content experiment uses group configuration '{experiment_name}'.").format(experiment_name=user_partition.name)}
+                ${_("This content experiment uses group configuration '{group_configuration_name}'.").format(
+                    group_configuration_name="<a href='{}'>{}</a>".format(group_configuration_url, user_partition.name) if show_link else user_partition.name
+                )}
             </span>
         </p>
     </div>