BLD-1125: Validation of content experiments after managing a group.

cms/djangoapps/contentstore/views/course.py

@@ -106,7 +106,7 @@ def course_notifications_handler(request, course_key_string=None, action_state_i
     Handle incoming requests for notifications in a RESTful way.
 
     course_key_string and action_state_id must both be set; else a HttpBadResponseRequest is returned.
-    
+
     For each of these operations, the requesting user must have access to the course;
     else a PermissionDenied error is returned.
 
@@ -1123,20 +1123,49 @@ class GroupConfiguration(object):
     @staticmethod
     def get_usage_info(course, store):
         """
-        Get all units names and their urls that have experiments and associated
-        with configurations.
+        Get usage information for all Group Configurations.
+        """
+        split_tests = store.get_items(course.id, qualifiers={'category': 'split_test'})
+        return GroupConfiguration._get_usage_info(store, course, split_tests)
+
+    @staticmethod
+    def add_usage_info(course, store):
+        """
+        Add usage information to group configurations jsons in course.
+
+        Returns json of group configurations updated with usage information.
+        """
+        usage_info = GroupConfiguration.get_usage_info(course, store)
+        configurations = []
+        for partition in course.user_partitions:
+            configuration = partition.to_json()
+            configuration['usage'] = usage_info.get(partition.id, [])
+            configurations.append(configuration)
+        return configurations
+
+    @staticmethod
+    def _get_usage_info(store, course, split_tests):
+        """
+        Returns all units names, their urls and validation messages.
 
         Returns:
         {'user_partition_id':
             [
-                {'label': 'Unit Name / Experiment Name', 'url': 'url_to_unit_1'},
-                {'label': 'Another Unit Name / Another Experiment Name', 'url': 'url_to_unit_1'}
+                {
+                    'label': 'Unit 1 / Experiment 1',
+                    'url': 'url_to_unit_1',
+                    'validation': {'message': 'a validation message', 'type': 'warning'}
+                },
+                {
+                    'label': 'Unit 2 / Experiment 2',
+                    'url': 'url_to_unit_2',
+                    'validation': {'message': 'another validation message', 'type': 'error'}
+                }
             ],
         }
         """
         usage_info = {}
-        descriptors = store.get_items(course.id, qualifiers={'category': 'split_test'})
-        for split_test in descriptors:
+        for split_test in split_tests:
             if split_test.user_partition_id not in usage_info:
                 usage_info[split_test.user_partition_id] = []
 
@@ -1157,24 +1186,28 @@ class GroupConfiguration(object):
             )
             usage_info[split_test.user_partition_id].append({
                 'label': '{} / {}'.format(unit.display_name, split_test.display_name),
-                'url': unit_url
+                'url': unit_url,
+                'validation': split_test.general_validation_message,
             })
         return usage_info
 
     @staticmethod
-    def add_usage_info(course, store):
+    def update_usage_info(store, course, configuration):
         """
-        Add usage information to group configurations json.
+        Update usage information for particular Group Configuration.
 
-        Returns json of group configurations updated with usage information.
+        Returns json of particular group configuration updated with usage information.
         """
-        usage_info = GroupConfiguration.get_usage_info(course, store)
-        configurations = []
-        for partition in course.user_partitions:
-            configuration = partition.to_json()
-            configuration['usage'] = usage_info.get(partition.id, [])
-            configurations.append(configuration)
-        return configurations
+        # Get all Experiments that use particular Group Configuration in course.
+        split_tests = store.get_items(
+            course.id,
+            category='split_test',
+            content={'user_partition_id': configuration.id}
+        )
+        configuration_json = configuration.to_json()
+        usage_information = GroupConfiguration._get_usage_info(store, course, split_tests)
+        configuration_json['usage'] = usage_information.get(configuration.id, [])
+        return configuration_json
 
 
 @require_http_methods(("GET", "POST"))
@@ -1262,7 +1295,8 @@ def group_configurations_detail_handler(request, course_key_string, group_config
         else:
             course.user_partitions.append(new_configuration)
         store.update_item(course, request.user.id)
-        return JsonResponse(new_configuration.to_json(), status=201)
+        configuration = GroupConfiguration.update_usage_info(store, course, new_configuration)
+        return JsonResponse(configuration, status=201)
     elif request.method == "DELETE":
         if not configuration:
             return JsonResponse(status=404)

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

@@ -3,14 +3,14 @@ Group Configuration Tests.
 """
 import json
 from mock import patch
-from contentstore.utils import reverse_course_url
+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
 
 GROUP_CONFIGURATION_JSON = {
     u'name': u'Test name',
@@ -27,7 +27,7 @@ class HelperMethods(object):
     """
     Mixin that provides useful methods for Group Configuration tests.
     """
-    def _create_content_experiment(self, cid=None, name_suffix=''):
+    def _create_content_experiment(self, cid=-1, name_suffix=''):
         """
         Create content experiment.
 
@@ -38,12 +38,42 @@ class HelperMethods(object):
             parent_location=self.course.location,
             display_name='Test Unit {}'.format(name_suffix)
         )
+        c0_url = self.course.id.make_usage_key("vertical", "split_test_cond0")
+        c1_url = self.course.id.make_usage_key("vertical", "split_test_cond1")
+        c2_url = self.course.id.make_usage_key("vertical", "split_test_cond2")
         split_test = ItemFactory.create(
             category='split_test',
             parent_location=vertical.location,
             user_partition_id=cid,
-            display_name='Test Content Experiment {}'.format(name_suffix)
+            display_name='Test Content Experiment {}'.format(name_suffix),
+            group_id_to_child={"0": c0_url, "1": c1_url, "2": c2_url}
+        )
+        ItemFactory.create(
+            parent_location=split_test.location,
+            category="vertical",
+            display_name="Condition 0 vertical",
+            location=c0_url,
         )
+        ItemFactory.create(
+            parent_location=split_test.location,
+            category="vertical",
+            display_name="Condition 1 vertical",
+            location=c1_url,
+        )
+        ItemFactory.create(
+            parent_location=split_test.location,
+            category="vertical",
+            display_name="Condition 2 vertical",
+            location=c2_url,
+        )
+
+        partitions_json = [p.to_json() for p in self.course.user_partitions]
+
+        self.client.ajax_post(
+            reverse_usage_url("xblock_handler", split_test.location),
+            data={'metadata': {'user_partitions': partitions_json}}
+        )
+
         self.save_course()
         return (vertical, split_test)
 
@@ -54,7 +84,7 @@ class HelperMethods(object):
         partitions = [
             UserPartition(
                 i, 'Name ' + str(i), 'Description ' + str(i), [Group(0, 'Group A'), Group(1, 'Group B'), Group(2, 'Group C')]
-            ) for i in xrange(count)
+            ) for i in xrange(0, count)
         ]
 
         self.course.user_partitions = partitions
@@ -238,7 +268,7 @@ class GroupConfigurationsDetailHandlerTestCase(UrlResetMixin, CourseTestCase, Gr
     Test cases for group_configurations_detail_handler.
     """
 
-    ID = 000000000000
+    ID = 0
 
     @patch.dict("django.conf.settings.FEATURES", {"ENABLE_GROUP_CONFIGURATIONS": True})
     def setUp(self):
@@ -247,11 +277,11 @@ class GroupConfigurationsDetailHandlerTestCase(UrlResetMixin, CourseTestCase, Gr
         """
         super(GroupConfigurationsDetailHandlerTestCase, self).setUp()
 
-    def _url(self, cid=None):
+    def _url(self, cid=-1):
         """
         Return url for the handler.
         """
-        cid = cid if cid is not None else self.ID
+        cid = cid if cid > 0 else self.ID
         return reverse_course_url(
             'group_configurations_detail_handler',
             self.course.id,
@@ -271,6 +301,7 @@ class GroupConfigurationsDetailHandlerTestCase(UrlResetMixin, CourseTestCase, Gr
                 {u'id': 0, u'name': u'Group A', u'version': 1},
                 {u'id': 1, u'name': u'Group B', u'version': 1},
             ],
+            u'usage': [],
         }
 
         response = self.client.put(
@@ -307,7 +338,9 @@ class GroupConfigurationsDetailHandlerTestCase(UrlResetMixin, CourseTestCase, Gr
                 {u'id': 0, u'name': u'New Group Name', u'version': 1},
                 {u'id': 2, u'name': u'Group C', u'version': 1},
             ],
+            u'usage': [],
         }
+
         response = self.client.put(
             self._url(),
             data=json.dumps(expected),
@@ -318,8 +351,10 @@ class GroupConfigurationsDetailHandlerTestCase(UrlResetMixin, CourseTestCase, Gr
         content = json.loads(response.content)
         self.assertEqual(content, expected)
         self.reload_course()
+
         # Verify that user_partitions is properly updated in the course.
         user_partititons = self.course.user_partitions
+
         self.assertEqual(len(user_partititons), 1)
         self.assertEqual(user_partititons[0].name, u'New Test name')
         self.assertEqual(len(user_partititons[0].groups), 2)
@@ -344,7 +379,7 @@ class GroupConfigurationsDetailHandlerTestCase(UrlResetMixin, CourseTestCase, Gr
         # Verify that user_partitions is properly updated in the course.
         user_partititons = self.course.user_partitions
         self.assertEqual(len(user_partititons), 1)
-        self.assertEqual(user_partititons[0].name, u'Name 1')
+        self.assertEqual(user_partititons[0].name, 'Name 1')
 
     def test_cannot_delete_used_group_configuration(self):
         """
@@ -366,7 +401,7 @@ class GroupConfigurationsDetailHandlerTestCase(UrlResetMixin, CourseTestCase, Gr
         # Verify that user_partitions is still the same.
         user_partititons = self.course.user_partitions
         self.assertEqual(len(user_partititons), 2)
-        self.assertEqual(user_partititons[0].name, u'Name 0')
+        self.assertEqual(user_partititons[0].name, 'Name 0')
 
     def test_cannot_delete_non_existent_group_configuration(self):
         """
@@ -383,7 +418,7 @@ class GroupConfigurationsDetailHandlerTestCase(UrlResetMixin, CourseTestCase, Gr
         # Verify that user_partitions is still the same.
         user_partititons = self.course.user_partitions
         self.assertEqual(len(user_partititons), 2)
-        self.assertEqual(user_partititons[0].name, u'Name 0')
+        self.assertEqual(user_partititons[0].name, 'Name 0')
 
 
 # pylint: disable=no-member
@@ -391,11 +426,9 @@ class GroupConfigurationsUsageInfoTestCase(UrlResetMixin, CourseTestCase, Helper
     """
     Tests for usage information of configurations.
     """
+
     @patch.dict("django.conf.settings.FEATURES", {"ENABLE_GROUP_CONFIGURATIONS": True})
     def setUp(self):
-        """
-        Set up group configurations and split test module.
-        """
         super(GroupConfigurationsUsageInfoTestCase, self).setUp()
 
     def test_group_configuration_not_used(self):
@@ -405,16 +438,16 @@ class GroupConfigurationsUsageInfoTestCase(UrlResetMixin, CourseTestCase, Helper
         self._add_user_partitions()
         actual = GroupConfiguration.add_usage_info(self.course, self.store)
         expected = [{
-            u'id': 0,
-            u'name': u'Name 0',
-            u'description': u'Description 0',
-            u'version': 1,
-            u'groups': [
-                {u'id': 0, u'name': u'Group A', u'version': 1},
-                {u'id': 1, u'name': u'Group B', u'version': 1},
-                {u'id': 2, u'name': u'Group C', u'version': 1},
+            'id': 0,
+            'name': 'Name 0',
+            'description': 'Description 0',
+            'version': 1,
+            'groups': [
+                {'id': 0, 'name': 'Group A', 'version': 1},
+                {'id': 1, 'name': 'Group B', 'version': 1},
+                {'id': 2, 'name': 'Group C', 'version': 1},
             ],
-            u'usage': [],
+            'usage': [],
         }]
         self.assertEqual(actual, expected)
 
@@ -429,30 +462,31 @@ class GroupConfigurationsUsageInfoTestCase(UrlResetMixin, CourseTestCase, Helper
         actual = GroupConfiguration.add_usage_info(self.course, self.store)
 
         expected = [{
-            u'id': 0,
-            u'name': u'Name 0',
-            u'description': u'Description 0',
-            u'version': 1,
-            u'groups': [
-                {u'id': 0, u'name': u'Group A', u'version': 1},
-                {u'id': 1, u'name': u'Group B', u'version': 1},
-                {u'id': 2, u'name': u'Group C', u'version': 1},
+            'id': 0,
+            'name': 'Name 0',
+            'description': 'Description 0',
+            'version': 1,
+            'groups': [
+                {'id': 0, 'name': 'Group A', 'version': 1},
+                {'id': 1, 'name': 'Group B', 'version': 1},
+                {'id': 2, 'name': 'Group C', 'version': 1},
             ],
-            u'usage': [{
+            'usage': [{
                 'url': '/container/i4x://MITx/999/vertical/Test_Unit_0',
                 'label': 'Test Unit 0 / Test Content Experiment 0',
+                'validation': None,
             }],
         }, {
-            u'id': 1,
-            u'name': u'Name 1',
-            u'description': u'Description 1',
-            u'version': 1,
-            u'groups': [
-                {u'id': 0, u'name': u'Group A', u'version': 1},
-                {u'id': 1, u'name': u'Group B', u'version': 1},
-                {u'id': 2, u'name': u'Group C', u'version': 1},
+            'id': 1,
+            'name': 'Name 1',
+            'description': 'Description 1',
+            'version': 1,
+            'groups': [
+                {'id': 0, 'name': 'Group A', 'version': 1},
+                {'id': 1, 'name': 'Group B', 'version': 1},
+                {'id': 2, 'name': 'Group C', 'version': 1},
             ],
-            u'usage': [],
+            'usage': [],
         }]
 
         self.assertEqual(actual, expected)
@@ -469,21 +503,23 @@ class GroupConfigurationsUsageInfoTestCase(UrlResetMixin, CourseTestCase, Helper
         actual = GroupConfiguration.add_usage_info(self.course, self.store)
 
         expected = [{
-            u'id': 0,
-            u'name': u'Name 0',
-            u'description': u'Description 0',
-            u'version': 1,
-            u'groups': [
-                {u'id': 0, u'name': u'Group A', u'version': 1},
-                {u'id': 1, u'name': u'Group B', u'version': 1},
-                {u'id': 2, u'name': u'Group C', u'version': 1},
+            'id': 0,
+            'name': 'Name 0',
+            'description': 'Description 0',
+            'version': 1,
+            'groups': [
+                {'id': 0, 'name': 'Group A', 'version': 1},
+                {'id': 1, 'name': 'Group B', 'version': 1},
+                {'id': 2, 'name': 'Group C', 'version': 1},
             ],
-            u'usage': [{
+            'usage': [{
                 'url': '/container/i4x://MITx/999/vertical/Test_Unit_0',
                 'label': 'Test Unit 0 / Test Content Experiment 0',
+                'validation': None,
             }, {
                 'url': '/container/i4x://MITx/999/vertical/Test_Unit_1',
                 'label': 'Test Unit 1 / Test Content Experiment 1',
+                'validation': None,
             }],
         }]
         self.assertEqual(actual, expected)
@@ -502,3 +538,97 @@ class GroupConfigurationsUsageInfoTestCase(UrlResetMixin, CourseTestCase, Helper
         self.save_course()
         actual = GroupConfiguration.get_usage_info(self.course, self.store)
         self.assertEqual(actual, {0: []})
+
+
+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()
+
+    @patch('xmodule.split_test_module.SplitTestDescriptor.validation_messages')
+    def test_error_message_present(self, mocked_validation_messages):
+        """
+        Tests if validation message is present.
+        """
+        self._add_user_partitions()
+        split_test = self._create_content_experiment(cid=0, name_suffix='0')[1]
+
+        mocked_validation_messages.return_value = [
+            ValidationMessage(
+                split_test,
+                u"Validation message",
+                ValidationMessageType.error
+            )
+        ]
+        group_configuration = GroupConfiguration.add_usage_info(self.course, self.store)[0]
+        self.assertEqual(
+            group_configuration['usage'][0]['validation'],
+            {
+                'message': u'This content experiment has issues that affect content visibility.',
+                'type': 'error'
+            }
+        )
+
+    @patch('xmodule.split_test_module.SplitTestDescriptor.validation_messages')
+    def test_warning_message_present(self, mocked_validation_messages):
+        """
+        Tests if validation message is present.
+        """
+        self._add_user_partitions()
+        split_test = self._create_content_experiment(cid=0, name_suffix='0')[1]
+
+        mocked_validation_messages.return_value = [
+            ValidationMessage(
+                split_test,
+                u"Validation message",
+                ValidationMessageType.warning
+            )
+        ]
+        group_configuration = GroupConfiguration.add_usage_info(self.course, self.store)[0]
+        self.assertEqual(
+            group_configuration['usage'][0]['validation'],
+            {
+                'message': u'This content experiment has issues that affect content visibility.',
+                'type': 'warning'
+            }
+        )
+
+    @patch('xmodule.split_test_module.SplitTestDescriptor.validation_messages')
+    def test_update_usage_info(self, mocked_validation_messages):
+        """
+        Tests if validation message is present when updating usage info.
+        """
+        self._add_user_partitions()
+        split_test = self._create_content_experiment(cid=0, name_suffix='0')[1]
+
+        mocked_validation_messages.return_value = [
+            ValidationMessage(
+                split_test,
+                u"Validation message",
+                ValidationMessageType.warning
+            )
+        ]
+
+        group_configuration = GroupConfiguration.update_usage_info(self.store, self.course, self.course.user_partitions[0])
+
+        self.assertEqual(
+            group_configuration['usage'][0]['validation'],
+            {
+                'message': u'This content experiment has issues that affect content visibility.',
+                'type': 'warning'
+            }
+        )
+
+    @patch('xmodule.split_test_module.SplitTestDescriptor.validation_messages')
+    def test_update_usage_info_no_message(self, mocked_validation_messages):
+        """
+        Tests if validation message is not present when updating usage info.
+        """
+        self._add_user_partitions()
+        self._create_content_experiment(cid=0, name_suffix='0')
+        mocked_validation_messages.return_value = []
+        group_configuration = GroupConfiguration.update_usage_info(self.store, self.course, self.course.user_partitions[0])
+        self.assertEqual(group_configuration['usage'][0]['validation'], None)

cms/static/js/spec/views/group_configuration_spec.js

@@ -33,7 +33,12 @@ define([
         usageText: '.group-configuration-usage-text',
         usageTextAnchor: '.group-configuration-usage-text > a',
         usageUnit: '.group-configuration-usage-unit',
-        usageUnitAnchor: '.group-configuration-usage-unit > a',
+        usageUnitAnchor: '.group-configuration-usage-unit a',
+        usageUnitMessage: '.group-configuration-validation-message',
+        usageUnitWarningIcon: '.group-configuration-usage-unit i.icon-warning-sign',
+        usageUnitErrorIcon: '.group-configuration-usage-unit i.icon-exclamation-sign',
+        warningMessage: '.group-configuration-validation-text',
+        warningIcon: '.wrapper-group-configuration-validation > i',
         note: '.wrapper-delete-button'
     };
 
@@ -162,12 +167,10 @@ define([
         it('should show non-empty usage appropriately', function() {
             var usageUnitAnchors;
 
-            this.model.set('usage',
-                [
-                    {'label': 'label1', 'url': 'url1'},
-                    {'label': 'label2', 'url': 'url2'}
-                ]
-            );
+            this.model.set('usage', [
+                {'label': 'label1', 'url': 'url1'},
+                {'label': 'label2', 'url': 'url2'}
+            ]);
             this.model.set('showGroups', false);
             this.view.$('.show-groups').click();
 
@@ -205,6 +208,55 @@ define([
             expect(this.view.$(SELECTORS.usageCount))
                 .toContainText('Used in 2 units');
         });
+
+        it('should show validation warning icon and message appropriately', function() {
+            this.model.set('usage', [
+                {
+                    'label': 'label1',
+                    'url': 'url1',
+                    'validation': {
+                        'message': "Warning message",
+                        'type': 'warning'
+                    }
+                }
+            ]);
+            this.model.set('showGroups', false);
+            this.view.$('.show-groups').click();
+
+            expect(this.view.$(SELECTORS.usageUnitMessage)).toContainText('Warning message');
+            expect(this.view.$(SELECTORS.usageUnitWarningIcon)).toExist();
+        });
+
+        it('should show validation error icon and message appropriately', function() {
+            this.model.set('usage', [
+                {
+                    'label': 'label1',
+                    'url': 'url1',
+                    'validation': {
+                        'message': "Error message",
+                        'type': 'error'
+                    }
+                }
+            ]);
+            this.model.set('showGroups', false);
+            this.view.$('.show-groups').click();
+
+            expect(this.view.$(SELECTORS.usageUnitMessage)).toContainText('Error message');
+            expect(this.view.$(SELECTORS.usageUnitErrorIcon)).toExist();
+        });
+
+        it('should hide validation icons and messages appropriately', function() {
+            this.model.set('usage', [
+                {'label': 'label1', 'url': 'url1'},
+                {'label': 'label2', 'url': 'url2'}
+            ]);
+            this.model.set('showGroups', true);
+            this.view.$('.hide-groups').click();
+
+            expect(this.view.$(SELECTORS.usageUnitMessage)).not.toExist();
+            expect(this.view.$(SELECTORS.usageUnitWarningIcon)).not.toExist();
+            expect(this.view.$(SELECTORS.usageUnitErrorIcon)).not.toExist();
+        });
     });
 
     describe('GroupConfigurationEdit', function() {
@@ -418,6 +470,24 @@ define([
             );
             expect(this.view.$('.delete')).toHaveClass('is-disabled');
         });
+
+        it('contains warning message if it is in use', function () {
+            this.model.set('usage', [ {'label': 'label1', 'url': 'url1'} ]);
+            this.view.render();
+            expect(this.view.$(SELECTORS.warningMessage)).toContainText(
+                'This configuration is currently used in content ' +
+                'experiments. If you make changes to the groups, you may ' +
+                'need to edit those experiments.'
+            );
+            expect(this.view.$(SELECTORS.warningIcon)).toExist();
+        });
+
+        it('does not contain warning message if it is not in use', function () {
+            this.model.set('usage', []);
+            this.view.render();
+            expect(this.view.$(SELECTORS.warningMessage)).not.toExist();
+            expect(this.view.$(SELECTORS.warningIcon)).not.toExist();
+        });
     });
 
     describe('GroupConfigurationsList', function() {

cms/static/sass/views/_group-configuration.scss

@@ -190,21 +190,51 @@
 
         .wrapper-group-configuration-usages {
           @extend %t-copy-sub1;
-          background-color: #f8f8f8;
           box-shadow: 0 2px 2px 0 $shadow inset;
           padding: $baseline ($baseline*1.5) $baseline ($baseline*2.5);
+          color: $gray-l1;
 
           .group-configuration-usage {
-            color: $gray-l1;
             margin-left: $baseline;
 
             .group-configuration-usage-unit {
               padding: ($baseline/4) 0;
+
+              a {
+                font-weight: 600;
+              }
+
+              .icon-warning-sign {
+                margin: ($baseline/4) ($baseline/2) 0 ($baseline*1.5);
+                color: $orange;
+              }
+
+              .icon-exclamation-sign {
+                margin: ($baseline/4) ($baseline/2) 0 ($baseline*1.5);
+                color: $red-l2;
+              }
             }
           }
         }
       }
 
+      .wrapper-group-configuration-validation {
+        @extend %t-copy-sub1;
+        background-color: #f8f8f8;
+        margin-top: $baseline;
+        padding: $baseline ($baseline*1.5) $baseline ($baseline*1.5);
+
+        .icon-warning-sign {
+          margin: ($baseline/2) $baseline 0 0;
+          color: $orange;
+          float: left;
+        }
+
+        .group-configuration-validation-text {
+          overflow: auto;
+        }
+      }
+
       .group-configuration-edit {
         @include box-sizing(border-box);
         border-radius: 2px;

cms/templates/js/group-configuration-details.underscore

@@ -62,7 +62,19 @@
       <ol class="group-configuration-usage">
         <% _.each(usage, function(unit) { %>
           <li class="group-configuration-usage-unit">
-            <a href=<%= unit.url %> ><%= unit.label %></a>
+            <p><a href=<%= unit.url %> ><%= unit.label %></a></p>
+            <% if (unit.validation) { %>
+              <p>
+                <% if (unit.validation.type === 'warning') { %>
+                  <i class="icon-warning-sign"></i>
+                <% } else if (unit.validation.type === 'error') { %>
+                  <i class="icon-exclamation-sign"></i>
+                <% } %>
+                <span class="group-configuration-validation-message">
+                  <%= unit.validation.message %>
+                </span>
+              </p>
+            <% } %>
           </li>
         <% }) %>
       </ol>

cms/templates/js/group-configuration-edit.underscore

@@ -32,6 +32,14 @@
             <ol class="groups list-input enum"></ol>
             <button class="action action-add-group"><i class="icon-plus"></i> <%= gettext("Add another group") %></button>
         </fieldset>
+        <% if (!_.isEmpty(usage)) { %>
+            <div class="wrapper-group-configuration-validation">
+                <i class="icon-warning-sign"></i>
+                <p class="group-configuration-validation-text">
+                    <%= gettext('This configuration is currently used in content experiments. If you make changes to the groups, you may need to edit those experiments.') %>
+                </p>
+            </div>
+        <% } %>
     </div>
     <div class="actions">
         <button class="action action-primary" type="submit"><% if (isNew) { print(gettext("Create")) } else { print(gettext("Save")) } %></button>

common/lib/xmodule/xmodule/split_test_module.py

@@ -607,3 +607,19 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor, StudioEditableDes
         )
         self.children.append(dest_usage_key)  # pylint: disable=no-member
         self.group_id_to_child[unicode(group.id)] = dest_usage_key
+
+    @property
+    def general_validation_message(self):
+        """
+        Message for either error or warning validation message/s.
+
+        Returns message and type. Priority given to error type message.
+        """
+        validation_messages = self.validation_messages()
+        if validation_messages:
+            has_error = any(message.message_type == ValidationMessageType.error for message in validation_messages)
+            return {
+                'message': _(u"This content experiment has issues that affect content visibility."),
+                'type': ValidationMessageType.error if has_error else ValidationMessageType.warning,
+            }
+        return None

common/lib/xmodule/xmodule/tests/test_split_test_module.py

@@ -344,6 +344,13 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
             self.assertEqual(message.action_class, expected_action_class)
             self.assertEqual(message.action_label, expected_action_label)
 
+        def verify_general_validation_message(general_validation, expected_message, expected_message_type):
+            """
+            Verify that the general validation message has the expected validation message and type.
+            """
+            self.assertEqual(unicode(general_validation['message']), expected_message)
+            self.assertEqual(general_validation['type'], expected_message_type)
+
         # Verify the messages for an unconfigured user partition
         split_test_module.user_partition_id = -1
         messages = split_test_module.validation_messages()
@@ -355,6 +362,11 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
             'edit-button',
             u"Select a Group Configuration",
         )
+        verify_general_validation_message(
+            split_test_module.general_validation_message,
+            u"This content experiment has issues that affect content visibility.",
+            ValidationMessageType.warning
+        )
 
         # Verify the messages for a correctly configured split_test
         split_test_module.user_partition_id = 0
@@ -363,6 +375,7 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
         ]
         messages = split_test_module.validation_messages()
         self.assertEqual(len(messages), 0)
+        self.assertIsNone(split_test_module.general_validation_message, None)
 
         # Verify the messages for a split test with too few groups
         split_test_module.user_partitions = [
@@ -378,7 +391,11 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
             'add-missing-groups-button',
             u"Add Missing Groups"
         )
-
+        verify_general_validation_message(
+            split_test_module.general_validation_message,
+            u"This content experiment has issues that affect content visibility.",
+            ValidationMessageType.error
+        )
         # Verify the messages for a split test with children that are not associated with any group
         split_test_module.user_partitions = [
             UserPartition(0, 'first_partition', 'First Partition',
@@ -391,7 +408,11 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
             u"The experiment has an inactive group. Move content into active groups, then delete the inactive group.",
             ValidationMessageType.warning
         )
-
+        verify_general_validation_message(
+            split_test_module.general_validation_message,
+            u"This content experiment has issues that affect content visibility.",
+            ValidationMessageType.warning
+        )
         # Verify the messages for a split test with both missing and inactive children
         split_test_module.user_partitions = [
             UserPartition(0, 'first_partition', 'First Partition',
@@ -411,6 +432,12 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
             u"The experiment has an inactive group. Move content into active groups, then delete the inactive group.",
             ValidationMessageType.warning
         )
+        # With two messages of type error and warning priority given to error.
+        verify_general_validation_message(
+            split_test_module.general_validation_message,
+            u"This content experiment has issues that affect content visibility.",
+            ValidationMessageType.error
+        )
 
         # Verify the messages for a split test referring to a non-existent user partition
         split_test_module.user_partition_id = 2
@@ -422,3 +449,8 @@ class SplitTestModuleStudioTest(SplitTestModuleTest):
             u"Select a valid group configuration or delete this experiment.",
             ValidationMessageType.error
         )
+        verify_general_validation_message(
+            split_test_module.general_validation_message,
+            u"This content experiment has issues that affect content visibility.",
+            ValidationMessageType.error
+        )

common/test/acceptance/pages/studio/settings_group_configurations.py

@@ -30,6 +30,14 @@ class GroupConfigurationsPage(CoursePage):
         """
         self.q(css=".new-button").first.click()
 
+    @property
+    def no_group_configuration_message_is_present(self):
+        return self.q(css='.wrapper-content .no-group-configurations-content').present
+
+    @property
+    def no_group_configuration_message_text(self):
+        return self.q(css='.wrapper-content .no-group-configurations-content').text[0]
+
 
 class GroupConfiguration(object):
     """
@@ -198,6 +206,34 @@ class GroupConfiguration(object):
         """
         return self.find_css('.wrapper-delete-button').first.attrs('data-tooltip')[0]
 
+    @property
+    def details_error_icon_is_present(self):
+        return self.find_css('.wrapper-group-configuration-usages .icon-exclamation-sign').present
+
+    @property
+    def details_warning_icon_is_present(self):
+        return self.find_css('.wrapper-group-configuration-usages .icon-warning-sign').present
+
+    @property
+    def details_message_is_present(self):
+        return self.find_css('.wrapper-group-configuration-usages .group-configuration-validation-message').present
+
+    @property
+    def details_message_text(self):
+        return self.find_css('.wrapper-group-configuration-usages .group-configuration-validation-message').text[0]
+
+    @property
+    def edit_warning_icon_is_present(self):
+        return self.find_css('.wrapper-group-configuration-validation .icon-warning-sign').present
+
+    @property
+    def edit_warning_message_is_present(self):
+        return self.find_css('.wrapper-group-configuration-validation .group-configuration-validation-text').present
+
+    @property
+    def edit_warning_message_text(self):
+        return self.find_css('.wrapper-group-configuration-validation .group-configuration-validation-text').text[0]
+
     def __repr__(self):
         return "<{}:{}>".format(self.__class__.__name__, self.name)
 

common/test/acceptance/tests/test_studio_split_test.py

@@ -301,6 +301,33 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
             )
         )
 
+    def create_group_configuration_experiment(self, groups, associate_experiment):
+        """
+        Creates a Group Configuration containing a list of groups.
+        Optionally creates a Content Experiment and associates it with previous Group Configuration.
+        """
+        # Create a new group configurations
+        self.course_fixture._update_xblock(self.course_fixture._course_location, {
+            "metadata": {
+                u"user_partitions": [
+                    UserPartition(0, "Name", "Description.", groups).to_json(),
+                ],
+            },
+        })
+
+        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})
+            )
+
+        # Go to the Group Configuration Page
+        self.page.visit()
+        config = self.page.group_configurations[0]
+        return config
+
     def test_no_group_configurations_added(self):
         """
         Scenario: Ensure that message telling me to create a new group configuration is
@@ -311,11 +338,10 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
         And "Create new Group Configuration" button is available
         """
         self.page.visit()
-        css = ".wrapper-content .no-group-configurations-content"
-        self.assertTrue(self.page.q(css=css).present)
+        self.assertTrue(self.page.no_group_configuration_message_is_present)
         self.assertIn(
             "You haven't created any group configurations yet.",
-            self.page.q(css=css).text[0]
+            self.page.no_group_configuration_message_text
         )
 
     def test_group_configurations_have_correct_data(self):
@@ -784,3 +810,115 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
             group_configuration_link_name,
             self.page.group_configurations[1].name
         )
+
+    def test_details_error_validation_message(self):
+        """
+        Scenario: When a Content Experiment uses a Group Configuration, ensure
+        that an error validation message appears if necessary.
+
+        Given I have a course with a Group Configuration containing two Groups
+        And a Content Experiment is assigned to that Group Configuration
+        When I go to the Group Configuration Page
+        Then I do not see a error icon and message in the Group Configuration details view.
+        When I add a Group
+        Then I see an error icon and message in the Group Configuration details view
+        """
+
+        # Create group configuration and associated experiment
+        config = self.create_group_configuration_experiment([Group("0", "Group A"), Group("1", "Group B")], True)
+
+        # Display details view
+        config.toggle()
+        # Check that error icon and message are not present
+        self.assertFalse(config.details_error_icon_is_present)
+        self.assertFalse(config.details_message_is_present)
+
+        # Add a group
+        config.toggle()
+        config.edit()
+        config.add_group()
+        config.save()
+
+        # Display details view
+        config.toggle()
+        # Check that error icon and message are present
+        self.assertTrue(config.details_error_icon_is_present)
+        self.assertTrue(config.details_message_is_present)
+        self.assertIn(
+            "This content experiment has issues that affect content visibility.",
+            config.details_message_text
+        )
+
+    def test_details_warning_validation_message(self):
+        """
+        Scenario: When a Content Experiment uses a Group Configuration, ensure
+        that a warning validation message appears if necessary.
+
+        Given I have a course with a Group Configuration containing three Groups
+        And a Content Experiment is assigned to that Group Configuration
+        When I go to the Group Configuration Page
+        Then I do not see a warning icon and message in the Group Configuration details view.
+        When I remove a Group
+        Then I see a warning icon and message in the Group Configuration details view
+        """
+
+        # 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)
+
+        # Display details view
+        config.toggle()
+        # Check that warning icon and message are not present
+        self.assertFalse(config.details_warning_icon_is_present)
+        self.assertFalse(config.details_message_is_present)
+
+        # Remove a group
+        config.toggle()
+        config.edit()
+        config.groups[2].remove()
+        config.save()
+
+        # Display details view
+        config.toggle()
+        # Check that warning icon and message are present
+        self.assertTrue(config.details_warning_icon_is_present)
+        self.assertTrue(config.details_message_is_present)
+        self.assertIn(
+            "This content experiment has issues that affect content visibility.",
+            config.details_message_text
+        )
+
+    def test_edit_warning_message_empty_usage(self):
+        """
+        Scenario: When a Group Configuration is not used, ensure that there are no warning icon and message.
+
+        Given I have a course with a Group Configuration containing two Groups
+        When I edit the Group Configuration
+        Then I do not see a warning icon and message
+        """
+
+        # Create a group configuration with no associated experiment and display edit view
+        config = self.create_group_configuration_experiment([Group("0", "Group A"), Group("1", "Group B")], False)
+        config.edit()
+        # Check that warning icon and message are not present
+        self.assertFalse(config.edit_warning_icon_is_present)
+        self.assertFalse(config.edit_warning_message_is_present)
+
+    def test_edit_warning_message_non_empty_usage(self):
+        """
+        Scenario: When a Group Configuration is used, ensure that there are a warning icon and message.
+
+        Given I have a course with a Group Configuration containing two Groups
+        When I edit the Group Configuration
+        Then I see a warning icon and message
+        """
+
+        # 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.edit()
+        # Check that warning icon and message are present
+        self.assertTrue(config.edit_warning_icon_is_present)
+        self.assertTrue(config.edit_warning_message_is_present)
+        self.assertIn(
+            "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
+        )

lms/templates/split_test_author_view.html

@@ -27,22 +27,20 @@ show_link = settings.FEATURES.get('ENABLE_GROUP_CONFIGURATIONS') and group_confi
 % endif
 % if len(messages) > 0:
     <%
+    general_validation = split_test.descriptor.general_validation_message
     def get_validation_icon(validation_type):
-        if validation_type == 'error':
+        if validation_type == ValidationMessageType.error:
             return 'icon-exclamation-sign'
-        elif validation_type == 'warning':
+        elif validation_type == ValidationMessageType.warning:
             return 'icon-warning-sign'
         return None
 
-    error_messages = (message for message in messages if message.message_type==ValidationMessageType.error)
-    has_errors = next(error_messages, False)
-    aggregate_validation_class = 'has-errors' if has_errors else 'has-warnings'
-    aggregate_validation_type = 'error' if has_errors else 'warning'
+    aggregate_validation_class = 'has-errors' if general_validation['type']==ValidationMessageType.error else ' has-warnings'
     %>
     <div class="xblock-message validation ${aggregate_validation_class}">
     % if is_configured:
-        <p class="${aggregate_validation_type}"><i class="${get_validation_icon(aggregate_validation_type)}"></i>
-        ${_("This content experiment has issues that affect content visibility.")}
+        <p class="${general_validation['type']}"><i class="${get_validation_icon(general_validation['type'])}"></i>
+        ${general_validation['message']}
         </p>
     % endif
     % if is_root or not is_configured: