Merge pull request #6299 from edx/cohorted-courseware

Cohorted courseware

CHANGELOG.rst

@@ -5,6 +5,8 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+Studio/LMS: Implement cohorted courseware. TNL-648
+
 LMS: Student Notes: Eventing for Student Notes. TNL-931
 
 LMS: Student Notes: Add course structure view. TNL-762

cms/djangoapps/contentstore/tests/test_utils.py

@@ -423,3 +423,63 @@ class InheritedStaffLockTest(StaffLockTest):
     def test_no_inheritance_for_orphan(self):
         """Tests that an orphaned xblock does not inherit staff lock"""
         self.assertFalse(utils.ancestor_has_staff_lock(self.orphan))
+
+
+class GroupVisibilityTest(CourseTestCase):
+    """
+    Test content group access rules.
+    """
+    def setUp(self):
+        super(GroupVisibilityTest, self).setUp()
+
+        chapter = ItemFactory.create(category='chapter', parent_location=self.course.location)
+        sequential = ItemFactory.create(category='sequential', parent_location=chapter.location)
+        vertical = ItemFactory.create(category='vertical', parent_location=sequential.location)
+        html = ItemFactory.create(category='html', parent_location=vertical.location)
+        problem = ItemFactory.create(
+            category='problem', parent_location=vertical.location, data="<problem></problem>"
+        )
+        self.sequential = self.store.get_item(sequential.location)
+        self.vertical = self.store.get_item(vertical.location)
+        self.html = self.store.get_item(html.location)
+        self.problem = self.store.get_item(problem.location)
+
+    def set_group_access(self, xblock, value):
+        """ Sets group_access to specified value and calls update_item to persist the change. """
+        xblock.group_access = value
+        self.store.update_item(xblock, self.user.id)
+
+    def test_no_visibility_set(self):
+        """ Tests when group_access has not been set on anything. """
+
+        def verify_all_components_visible_to_all():  # pylint: disable=invalid-name
+            """ Verifies when group_access has not been set on anything. """
+            for item in (self.sequential, self.vertical, self.html, self.problem):
+                self.assertFalse(utils.has_children_visible_to_specific_content_groups(item))
+                self.assertFalse(utils.is_visible_to_specific_content_groups(item))
+
+        verify_all_components_visible_to_all()
+
+        # Test with group_access set to Falsey values.
+        self.set_group_access(self.vertical, {1: []})
+        self.set_group_access(self.html, {2: None})
+
+        verify_all_components_visible_to_all()
+
+    def test_sequential_and_problem_have_group_access(self):
+        """ Tests when group_access is set on a few different components. """
+        self.set_group_access(self.sequential, {1: [0]})
+        # This is a no-op.
+        self.set_group_access(self.vertical, {1: []})
+        self.set_group_access(self.problem, {2: [3, 4]})
+
+        # Note that "has_children_visible_to_specific_content_groups" only checks immediate children.
+        self.assertFalse(utils.has_children_visible_to_specific_content_groups(self.sequential))
+        self.assertTrue(utils.has_children_visible_to_specific_content_groups(self.vertical))
+        self.assertFalse(utils.has_children_visible_to_specific_content_groups(self.html))
+        self.assertFalse(utils.has_children_visible_to_specific_content_groups(self.problem))
+
+        self.assertTrue(utils.is_visible_to_specific_content_groups(self.sequential))
+        self.assertFalse(utils.is_visible_to_specific_content_groups(self.vertical))
+        self.assertFalse(utils.is_visible_to_specific_content_groups(self.html))
+        self.assertTrue(utils.is_visible_to_specific_content_groups(self.problem))

cms/djangoapps/contentstore/utils.py

@@ -179,6 +179,36 @@ def is_currently_visible_to_students(xblock):
     return True
 
 
+def has_children_visible_to_specific_content_groups(xblock):
+    """
+    Returns True if this xblock has children that are limited to specific content groups.
+    Note that this method is not recursive (it does not check grandchildren).
+    """
+    if not xblock.has_children:
+        return False
+
+    for child in xblock.get_children():
+        if is_visible_to_specific_content_groups(child):
+            return True
+
+    return False
+
+
+def is_visible_to_specific_content_groups(xblock):
+    """
+    Returns True if this xblock has visibility limited to specific content groups.
+    """
+    if not xblock.group_access:
+        return False
+    for __, value in xblock.group_access.iteritems():
+        # value should be a list of group IDs. If it is an empty list or None, the xblock is visible
+        # to all groups in that particular partition. So if value is a truthy value, the xblock is
+        # restricted in some way.
+        if value:
+            return True
+    return False
+
+
 def find_release_date_source(xblock):
     """
     Finds the ancestor of xblock that set its release date.

cms/djangoapps/contentstore/views/component.py

@@ -21,7 +21,7 @@ from xblock.runtime import Mixologist
 
 from contentstore.utils import get_lms_link_for_item
 from contentstore.views.helpers import get_parent_xblock, is_unit, xblock_type_display_name
-from contentstore.views.item import create_xblock_info
+from contentstore.views.item import create_xblock_info, add_container_page_publishing_info
 
 from opaque_keys.edx.keys import UsageKey
 
@@ -186,8 +186,9 @@ def container_handler(request, usage_key_string):
             # about the block's ancestors and siblings for use by the Unit Outline.
             xblock_info = create_xblock_info(xblock, include_ancestor_info=is_unit_page)
 
-            # Create the link for preview.
-            preview_lms_base = settings.FEATURES.get('PREVIEW_LMS_BASE')
+            if is_unit_page:
+                add_container_page_publishing_info(xblock, xblock_info)
+
             # need to figure out where this item is in the list of children as the
             # preview will need this
             index = 1

cms/djangoapps/contentstore/views/course.py

@@ -16,6 +16,7 @@ from django.core.urlresolvers import reverse
 from django.http import HttpResponseBadRequest, HttpResponseNotFound, HttpResponse, Http404
 from util.json_request import JsonResponse, JsonResponseBadRequest
 from util.date_utils import get_default_time_display
+from util.db import generate_int_id, MYSQL_MAX_INT
 from edxmako.shortcuts import render_to_response
 
 from xmodule.course_module import DEFAULT_START_DATE
@@ -29,6 +30,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseErr
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.locations import Location
 from opaque_keys.edx.keys import CourseKey
+from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_user_partition
 
 from django_future.csrf import ensure_csrf_cookie
 from contentstore.course_info_model import get_course_updates, update_course_updates, delete_course_update
@@ -70,7 +72,15 @@ from course_action_state.models import CourseRerunState, CourseRerunUIStateManag
 from course_action_state.managers import CourseActionStateItemNotFoundError
 from microsite_configuration import microsite
 from xmodule.course_module import CourseFields
+from xmodule.split_test_module import get_split_user_partitions
 
+MINIMUM_GROUP_ID = 100
+
+# Note: the following content group configuration strings are not
+# translated since they are not visible to users.
+CONTENT_GROUP_CONFIGURATION_DESCRIPTION = 'The groups in this configuration can be mapped to cohort groups in the LMS.'
+
+CONTENT_GROUP_CONFIGURATION_NAME = 'Content Group Configuration'
 
 __all__ = ['course_info_handler', 'course_handler', 'course_listing',
            'course_info_update_handler',
@@ -1252,23 +1262,16 @@ class GroupConfiguration(object):
         if len(self.configuration.get('groups', [])) < 1:
             raise GroupConfigurationsValidationError(_("must have at least one group"))
 
-    def generate_id(self, used_ids):
-        """
-        Generate unique id for the group configuration.
-        If this id is already used, we generate new one.
-        """
-        cid = random.randint(100, 10 ** 12)
-
-        while cid in used_ids:
-            cid = random.randint(100, 10 ** 12)
-
-        return cid
-
     def assign_id(self, configuration_id=None):
         """
         Assign id for the json representation of group configuration.
         """
-        self.configuration['id'] = int(configuration_id) if configuration_id else self.generate_id(self.get_used_ids())
+        if configuration_id:
+            self.configuration['id'] = int(configuration_id)
+        else:
+            self.configuration['id'] = generate_int_id(
+                MINIMUM_GROUP_ID, MYSQL_MAX_INT, GroupConfiguration.get_used_ids(self.course)
+            )
 
     def assign_group_ids(self):
         """
@@ -1278,14 +1281,15 @@ class GroupConfiguration(object):
         # Assign ids to every group in configuration.
         for group in self.configuration.get('groups', []):
             if group.get('id') is None:
-                group["id"] = self.generate_id(used_ids)
+                group["id"] = generate_int_id(MINIMUM_GROUP_ID, MYSQL_MAX_INT, used_ids)
                 used_ids.append(group["id"])
 
-    def get_used_ids(self):
+    @staticmethod
+    def get_used_ids(course):
         """
         Return a list of IDs that already in use.
         """
-        return set([p.id for p in self.course.user_partitions])
+        return set([p.id for p in course.user_partitions])
 
     def get_user_partition(self):
         """
@@ -1296,21 +1300,19 @@ class GroupConfiguration(object):
     @staticmethod
     def get_usage_info(course, store):
         """
-        Get usage information for all Group Configurations.
+        Get usage information for all Group Configurations currently referenced by a split_test instance.
         """
         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):
+    def get_split_test_partitions_with_usage(course, store):
         """
-        Add usage information to group configurations jsons in course.
-
-        Returns json of group configurations updated with usage information.
+        Returns json split_test group configurations updated with usage information.
         """
         usage_info = GroupConfiguration.get_usage_info(course, store)
         configurations = []
-        for partition in course.user_partitions:
+        for partition in get_split_user_partitions(course.user_partitions):
             configuration = partition.to_json()
             configuration['usage'] = usage_info.get(partition.id, [])
             configurations.append(configuration)
@@ -1384,6 +1386,26 @@ class GroupConfiguration(object):
         configuration_json['usage'] = usage_information.get(configuration.id, [])
         return configuration_json
 
+    @staticmethod
+    def get_or_create_content_group_configuration(course):
+        """
+        Returns the first user partition from the course which uses the
+        CohortPartitionScheme, or generates one if no such partition is
+        found.  The created partition is not saved to the course until
+        the client explicitly creates a group within the partition and
+        POSTs back.
+        """
+        content_group_configuration = get_cohorted_user_partition(course.id)
+        if content_group_configuration is None:
+            content_group_configuration = UserPartition(
+                id=generate_int_id(MINIMUM_GROUP_ID, MYSQL_MAX_INT, GroupConfiguration.get_used_ids(course)),
+                name=CONTENT_GROUP_CONFIGURATION_NAME,
+                description=CONTENT_GROUP_CONFIGURATION_DESCRIPTION,
+                groups=[],
+                scheme_id='cohort'
+            )
+        return content_group_configuration
+
 
 @require_http_methods(("GET", "POST"))
 @login_required
@@ -1405,12 +1427,21 @@ def group_configurations_list_handler(request, course_key_string):
         if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'):
             group_configuration_url = reverse_course_url('group_configurations_list_handler', course_key)
             course_outline_url = reverse_course_url('course_handler', course_key)
-            configurations = GroupConfiguration.add_usage_info(course, store)
+            should_show_experiment_groups = are_content_experiments_enabled(course)
+            if should_show_experiment_groups:
+                experiment_group_configurations = GroupConfiguration.get_split_test_partitions_with_usage(course, store)
+            else:
+                experiment_group_configurations = None
+            content_group_configuration = GroupConfiguration.get_or_create_content_group_configuration(
+                course
+            ).to_json()
             return render_to_response('group_configurations.html', {
                 'context_course': course,
                 'group_configuration_url': group_configuration_url,
                 'course_outline_url': course_outline_url,
-                'configurations': configurations if should_show_group_configurations_page(course) else None,
+                'experiment_group_configurations': experiment_group_configurations,
+                'should_show_experiment_groups': should_show_experiment_groups,
+                'content_group_configuration': content_group_configuration
             })
         elif "application/json" in request.META.get('HTTP_ACCEPT'):
             if request.method == 'POST':
@@ -1489,9 +1520,9 @@ def group_configurations_detail_handler(request, course_key_string, group_config
             return JsonResponse(status=204)
 
 
-def should_show_group_configurations_page(course):
+def are_content_experiments_enabled(course):
     """
-    Returns true if Studio should show the "Group Configurations" page for the specified course.
+    Returns True if content experiments have been enabled for the course.
     """
     return (
         SPLIT_TEST_COMPONENT_TYPE in ADVANCED_COMPONENT_TYPES and

cms/djangoapps/contentstore/views/preview.py

@@ -17,6 +17,7 @@ from xmodule.exceptions import NotFoundError, ProcessingError
 from xmodule.library_tools import LibraryToolsService
 from xmodule.modulestore.django import modulestore, ModuleI18nService
 from opaque_keys.edx.keys import UsageKey
+from opaque_keys.edx.locator import LibraryUsageLocator
 from xmodule.x_module import ModuleSystem
 from xblock.runtime import KvsFieldData
 from xblock.django.request import webob_to_django_response, django_to_webob_request
@@ -242,6 +243,7 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False):
     # Only add the Studio wrapper when on the container page. The "Pages" page will remain as is for now.
     if not context.get('is_pages_view', None) and view in PREVIEW_VIEWS:
         root_xblock = context.get('root_xblock')
+        can_edit_visibility = not isinstance(xblock.location, LibraryUsageLocator)
         is_root = root_xblock and xblock.location == root_xblock.location
         is_reorderable = _is_xblock_reorderable(xblock, context)
         template_context = {
@@ -251,6 +253,7 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False):
             'is_root': is_root,
             'is_reorderable': is_reorderable,
             'can_edit': context.get('can_edit', True),
+            'can_edit_visibility': can_edit_visibility,
         }
         html = render_to_string('studio_xblock_wrapper.html', template_context)
         frag = wrap_fragment(frag, html)

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

@@ -207,17 +207,7 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, 'First name')
         self.assertContains(response, 'Group C')
-
-    def test_view_index_disabled(self):
-        """
-        Check that group configuration page is not displayed when turned off.
-        """
-        if SPLIT_TEST_COMPONENT_TYPE in self.course.advanced_modules:
-            self.course.advanced_modules.remove(SPLIT_TEST_COMPONENT_TYPE)
-            self.store.update_item(self.course, self.user.id)
-
-        resp = self.client.get(self._url())
-        self.assertContains(resp, "module is disabled")
+        self.assertContains(response, 'Content Group Configuration')
 
     def test_unsupported_http_accept_header(self):
         """
@@ -243,12 +233,9 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations
                 {u'name': u'Group B', u'version': 1},
             ],
         }
-        response = self.client.post(
+        response = self.client.ajax_post(
             self._url(),
-            data=json.dumps(GROUP_CONFIGURATION_JSON),
-            content_type="application/json",
-            HTTP_ACCEPT="application/json",
-            HTTP_X_REQUESTED_WITH="XMLHttpRequest",
+            data=GROUP_CONFIGURATION_JSON
         )
         self.assertEqual(response.status_code, 201)
         self.assertIn("Location", response)
@@ -267,6 +254,16 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations
         self.assertEqual(user_partititons[0].groups[0].name, u'Group A')
         self.assertEqual(user_partititons[0].groups[1].name, u'Group B')
 
+    def test_lazily_creates_cohort_configuration(self):
+        """
+        Test that a cohort schemed user partition is NOT created by
+        default for the user.
+        """
+        self.assertEqual(len(self.course.user_partitions), 0)
+        self.client.get(self._url())
+        self.reload_course()
+        self.assertEqual(len(self.course.user_partitions), 0)
+
 
 # pylint: disable=no-member
 class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfigurationsBaseTestCase, HelperMethods):
@@ -436,7 +433,7 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
         Test that right data structure will be created if group configuration is not used.
         """
         self._add_user_partitions()
-        actual = GroupConfiguration.add_usage_info(self.course, self.store)
+        actual = GroupConfiguration.get_split_test_partitions_with_usage(self.course, self.store)
         expected = [{
             'id': 0,
             'name': 'Name 0',
@@ -460,7 +457,7 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
         vertical, __ = self._create_content_experiment(cid=0, name_suffix='0')
         self._create_content_experiment(name_suffix='1')
 
-        actual = GroupConfiguration.add_usage_info(self.course, self.store)
+        actual = GroupConfiguration.get_split_test_partitions_with_usage(self.course, self.store)
 
         expected = [{
             'id': 0,
@@ -503,7 +500,7 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
         vertical, __ = self._create_content_experiment(cid=0, name_suffix='0')
         vertical1, __ = self._create_content_experiment(cid=0, name_suffix='1')
 
-        actual = GroupConfiguration.add_usage_info(self.course, self.store)
+        actual = GroupConfiguration.get_split_test_partitions_with_usage(self.course, self.store)
 
         expected = [{
             'id': 0,
@@ -567,7 +564,7 @@ class GroupConfigurationsValidationTestCase(CourseTestCase, HelperMethods):
         validation.add(mocked_message)
         mocked_validation_messages.return_value = validation
 
-        group_configuration = GroupConfiguration.add_usage_info(self.course, self.store)[0]
+        group_configuration = GroupConfiguration.get_split_test_partitions_with_usage(self.course, self.store)[0]
         self.assertEqual(expected_result.to_json(), group_configuration['usage'][0]['validation'])
 
     def test_error_message_present(self):

cms/envs/common.py

@@ -37,6 +37,7 @@ from path import path
 from warnings import simplefilter
 
 from lms.djangoapps.lms_xblock.mixin import LmsBlockMixin
+from cms.lib.xblock.authoring_mixin import AuthoringMixin
 import dealer.git
 from xmodule.modulestore.edit_info import EditInfoMixin
 
@@ -121,6 +122,12 @@ FEATURES = {
     # for consistency in user-experience, keep the value of this feature flag
     # in sync with the one in lms/envs/common.py
     'ENABLE_EDXNOTES': False,
+
+    # Enable support for content libraries. Note that content libraries are
+    # only supported in courses using split mongo. Change the setting
+    # DEFAULT_STORE_FOR_NEW_COURSE to be 'split' to have future courses
+    # and libraries created with split.
+    'ENABLE_CONTENT_LIBRARIES': False,
 }
 ENABLE_JASMINE = False
 
@@ -269,7 +276,13 @@ from xmodule.x_module import XModuleMixin
 
 # This should be moved into an XBlock Runtime/Application object
 # once the responsibility of XBlock creation is moved out of modulestore - cpennington
-XBLOCK_MIXINS = (LmsBlockMixin, InheritanceMixin, XModuleMixin, EditInfoMixin)
+XBLOCK_MIXINS = (
+    LmsBlockMixin,
+    InheritanceMixin,
+    XModuleMixin,
+    EditInfoMixin,
+    AuthoringMixin,
+)
 
 # Allow any XBlock in Studio
 # You should also enable the ALLOW_ALL_ADVANCED_COMPONENTS feature flag, so that

cms/lib/xblock/authoring_mixin.py

+"""
+Mixin class that provides authoring capabilities for XBlocks.
+"""
+
+import logging
+
+from xblock.core import XBlock
+from xblock.fields import XBlockMixin
+from xblock.fragment import Fragment
+
+log = logging.getLogger(__name__)
+
+VISIBILITY_VIEW = 'visibility_view'
+
+
+@XBlock.needs("i18n")
+class AuthoringMixin(XBlockMixin):
+    """
+    Mixin class that provides authoring capabilities for XBlocks.
+    """
+    _services_requested = {
+        'i18n': 'need',
+    }
+
+    def _get_studio_resource_url(self, relative_url):
+        """
+        Returns the Studio URL to a static resource.
+        """
+        # TODO: is there a cleaner way to do this?
+        from cms.envs.common import STATIC_URL
+        return STATIC_URL + relative_url
+
+    def visibility_view(self, _context=None):
+        """
+        Render the view to manage an xblock's visibility settings in Studio.
+        Args:
+            _context: Not actively used for this view.
+        Returns:
+            (Fragment): An HTML fragment for editing the visibility of this XBlock.
+        """
+        fragment = Fragment()
+        from contentstore.utils import reverse_course_url
+        fragment.add_content(self.system.render_template('visibility_editor.html', {
+            'xblock': self,
+            'manage_groups_url': reverse_course_url('group_configurations_list_handler', self.location.course_key),
+        }))
+        fragment.add_javascript_url(self._get_studio_resource_url('/js/xblock/authoring.js'))
+        fragment.initialize_js('VisibilityEditorInit')
+        return fragment

cms/lib/xblock/test/test_authoring_mixin.py

+"""
+Tests for the Studio authoring XBlock mixin.
+"""
+
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+from xmodule.partitions.partitions import Group, UserPartition
+
+
+class AuthoringMixinTestCase(ModuleStoreTestCase):
+    """
+    Tests the studio authoring XBlock mixin.
+    """
+    def setUp(self):
+        """
+        Create a simple course with a video component.
+        """
+        super(AuthoringMixinTestCase, self).setUp()
+        self.course = CourseFactory.create()
+        chapter = ItemFactory.create(
+            category='chapter',
+            parent_location=self.course.location,
+            display_name='Test Chapter'
+        )
+        sequential = ItemFactory.create(
+            category='sequential',
+            parent_location=chapter.location,
+            display_name='Test Sequential'
+        )
+        self.vertical = ItemFactory.create(
+            category='vertical',
+            parent_location=sequential.location,
+            display_name='Test Vertical'
+        )
+        self.video = ItemFactory.create(
+            category='video',
+            parent_location=self.vertical.location,
+            display_name='Test Vertical'
+        )
+        self.pet_groups = [Group(1, 'Cat Lovers'), Group(2, 'Dog Lovers')]
+
+    def create_content_groups(self, content_groups):
+        """
+        Create a cohorted user partition with the specified content groups.
+        """
+        # pylint: disable=attribute-defined-outside-init
+        self.content_partition = UserPartition(
+            1,
+            'Content Groups',
+            'Contains Groups for Cohorted Courseware',
+            content_groups,
+            scheme_id='cohort'
+        )
+        self.course.user_partitions = [self.content_partition]
+        self.store.update_item(self.course, self.user.id)
+
+    def set_staff_only(self, item):
+        """Make an item visible to staff only."""
+        item.visible_to_staff_only = True
+        self.store.update_item(item, self.user.id)
+
+    def set_group_access(self, item, group_ids):
+        """
+        Set group_access for the specified item to the specified group
+        ids within the content partition.
+        """
+        item.group_access[self.content_partition.id] = group_ids  # pylint: disable=no-member
+        self.store.update_item(item, self.user.id)
+
+    def verify_visibility_view_contains(self, item, substrings):
+        """
+        Verify that an item's visibility view returns an html string
+        containing all the expected substrings.
+        """
+        html = item.visibility_view().body_html()
+        for string in substrings:
+            self.assertIn(string, html)
+
+    def test_html_no_partition(self):
+        self.verify_visibility_view_contains(self.video, 'No content groups exist')
+
+    def test_html_empty_partition(self):
+        self.create_content_groups([])
+        self.verify_visibility_view_contains(self.video, 'No content groups exist')
+
+    def test_html_populated_partition(self):
+        self.create_content_groups(self.pet_groups)
+        self.verify_visibility_view_contains(self.video, ['Cat Lovers', 'Dog Lovers'])
+
+    def test_html_no_partition_staff_locked(self):
+        self.set_staff_only(self.vertical)
+        self.verify_visibility_view_contains(self.video, ['No content groups exist'])
+
+    def test_html_empty_partition_staff_locked(self):
+        self.create_content_groups([])
+        self.set_staff_only(self.vertical)
+        self.verify_visibility_view_contains(self.video, 'No content groups exist')
+
+    def test_html_populated_partition_staff_locked(self):
+        self.create_content_groups(self.pet_groups)
+        self.set_staff_only(self.vertical)
+        self.verify_visibility_view_contains(
+            self.video, ['The Unit this component is contained in is hidden from students.', 'Cat Lovers', 'Dog Lovers']
+        )
+
+    def test_html_false_content_group(self):
+        self.create_content_groups(self.pet_groups)
+        self.set_group_access(self.video, ['false_group_id'])
+        self.verify_visibility_view_contains(
+            self.video, ['Cat Lovers', 'Dog Lovers', 'Content group no longer exists.']
+        )
+
+    def test_html_false_content_group_staff_locked(self):
+        self.create_content_groups(self.pet_groups)
+        self.set_staff_only(self.vertical)
+        self.set_group_access(self.video, ['false_group_id'])
+        self.verify_visibility_view_contains(
+            self.video,
+            [
+                'Cat Lovers',
+                'Dog Lovers',
+                'The Unit this component is contained in is hidden from students.',
+                'Content group no longer exists.'
+            ]
+        )

cms/static/coffee/src/views/module_edit.coffee

@@ -52,7 +52,5 @@ define ["jquery", "underscore", "gettext", "xblock/runtime.v1",
 
     clickEditButton: (event) ->
       event.preventDefault()
-      modal = new EditXBlockModal({
-        view: 'student_view'
-      });
+      modal = new EditXBlockModal();
       modal.edit(this.$el, self.model, { refresh: _.bind(@render, this) })

cms/static/js/factories/container.js

 define([
-    'jquery', 'underscore', 'js/models/xblock_info', 'js/views/pages/container',
+    'jquery', 'underscore', 'js/models/xblock_container_info', 'js/views/pages/container',
     'js/collections/component_template', 'xmodule', 'coffee/src/main',
     'xblock/cms.runtime.v1'
 ],
-function($, _, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) {
+function($, _, XBlockContainerInfo, ContainerPage, ComponentTemplates, xmoduleLoader) {
     'use strict';
     return function (componentTemplates, XBlockInfoJson, action, options) {
         var main_options = {
             el: $('#content'),
-            model: new XBlockInfo(XBlockInfoJson, {parse: true}),
+            model: new XBlockContainerInfo(XBlockInfoJson, {parse: true}),
             action: action,
             templates: new ComponentTemplates(componentTemplates, {parse: true})
         };

cms/static/js/factories/group_configurations.js

 define([
-    'js/collections/group_configuration', 'js/views/pages/group_configurations'
-], function(GroupConfigurationCollection, GroupConfigurationsPage) {
+    'js/collections/group_configuration', 'js/models/group_configuration', 'js/views/pages/group_configurations'
+], function(GroupConfigurationCollection, GroupConfigurationModel, GroupConfigurationsPage) {
     'use strict';
-    return function (configurations, groupConfigurationUrl, courseOutlineUrl) {
-        var collection = new GroupConfigurationCollection(configurations, { parse: true }),
-            configurationsPage;
+    return function (experimentsEnabled, experimentGroupConfigurationsJson, contentGroupConfigurationJson,
+                     groupConfigurationUrl, courseOutlineUrl) {
+        var experimentGroupConfigurations = new GroupConfigurationCollection(
+                experimentGroupConfigurationsJson, {parse: true}
+            ),
+            contentGroupConfiguration = new GroupConfigurationModel(contentGroupConfigurationJson, {parse: true});
 
-        collection.url = groupConfigurationUrl;
-        collection.outlineUrl = courseOutlineUrl;
-        configurationsPage = new GroupConfigurationsPage({
+        experimentGroupConfigurations.url = groupConfigurationUrl;
+        experimentGroupConfigurations.outlineUrl = courseOutlineUrl;
+        contentGroupConfiguration.urlRoot = groupConfigurationUrl;
+        new GroupConfigurationsPage({
             el: $('#content'),
-            collection: collection
+            experimentsEnabled: experimentsEnabled,
+            experimentGroupConfigurations: experimentGroupConfigurations,
+            contentGroupConfiguration: contentGroupConfiguration
         }).render();
     };
 });

cms/static/js/models/custom_sync_xblock_info.js

+define(["js/models/xblock_info"],
+    function(XBlockInfo) {
+        var CustomSyncXBlockInfo = XBlockInfo.extend({
+            sync: function(method, model, options) {
+                options.url = (this.urlRoots[method] || this.urlRoot) + '/' + this.get('id');
+                return XBlockInfo.prototype.sync.call(this, method, model, options);
+            }
+        });
+        return CustomSyncXBlockInfo;
+    });

cms/static/js/models/xblock_container_info.js

+define(["js/models/custom_sync_xblock_info"],
+    function(CustomSyncXBlockInfo) {
+        var XBlockContainerInfo = CustomSyncXBlockInfo.extend({
+            urlRoots: {
+                'read': '/xblock/container'
+            }
+        });
+        return XBlockContainerInfo;
+    });

cms/static/js/models/xblock_info.js

@@ -32,7 +32,8 @@ function(Backbone, _, str, ModuleUtils) {
              */
             'edited_on':null,
             /**
-             * User who last edited the xblock or any of its descendants.
+             * User who last edited the xblock or any of its descendants. Will only be present if
+             * publishing info was explicitly requested.
              */
             'edited_by':null,
             /**
@@ -44,7 +45,8 @@ function(Backbone, _, str, ModuleUtils) {
              */
             'published_on': null,
             /**
-             * User who last published the xblock, or null if never published.
+             * User who last published the xblock, or null if never published. Will only be present if
+             * publishing info was explicitly requested.
              */
             'published_by': null,
             /**
@@ -70,12 +72,14 @@ function(Backbone, _, str, ModuleUtils) {
             /**
              * The xblock which is determining the release date. For instance, for a unit,
              * this will either be the parent subsection or the grandparent section.
-             * This can be null if the release date is unscheduled.
+             * This can be null if the release date is unscheduled. Will only be present if
+             * publishing info was explicitly requested.
              */
             'release_date_from':null,
             /**
              * True if this xblock is currently visible to students. This is computed server-side
-             * so that the logic isn't duplicated on the client.
+             * so that the logic isn't duplicated on the client. Will only be present if
+             * publishing info was explicitly requested.
              */
             'currently_visible_to_students': null,
             /**
@@ -114,13 +118,20 @@ function(Backbone, _, str, ModuleUtils) {
             /**
              * The xblock which is determining the staff lock value. For instance, for a unit,
              * this will either be the parent subsection or the grandparent section.
-             * This can be null if the xblock has no inherited staff lock.
+             * This can be null if the xblock has no inherited staff lock. Will only be present if
+             * publishing info was explicitly requested.
              */
             'staff_lock_from': null,
             /**
              * True iff this xblock should display a "Contains staff only content" message.
              */
-            'staff_only_message': null
+            'staff_only_message': null,
+            /**
+             * True iff this xblock is a unit, and it has children that are only visible to certain
+             * content groups. Note that this is not a recursive property. Will only be present if
+             * publishing info was explicitly requested.
+             */
+            'has_content_group_components': null
         },
 
         initialize: function () {

cms/static/js/models/xblock_outline_info.js

-define(["js/models/xblock_info"],
-    function(XBlockInfo) {
-        var XBlockOutlineInfo = XBlockInfo.extend({
+define(["js/models/custom_sync_xblock_info"],
+    function(CustomSyncXBlockInfo) {
+        var XBlockOutlineInfo = CustomSyncXBlockInfo.extend({
 
             urlRoots: {
                 'read': '/xblock/outline'
@@ -8,15 +8,6 @@ define(["js/models/xblock_info"],
 
             createChild: function(response) {
                 return new XBlockOutlineInfo(response, { parse: true });
-            },
-
-            sync: function(method, model, options) {
-                var urlRoot = this.urlRoots[method];
-                if (!urlRoot) {
-                    urlRoot = this.urlRoot;
-                }
-                options.url = urlRoot + '/' + this.get('id');
-                return XBlockInfo.prototype.sync.call(this, method, model, options);
             }
         });
         return XBlockOutlineInfo;

cms/static/js/spec/views/pages/container_spec.js

 define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_helpers",
         "js/common_helpers/template_helpers", "js/spec_helpers/edit_helpers",
-        "js/views/pages/container", "js/views/pages/paged_container", "js/models/xblock_info"],
+        "js/views/pages/container", "js/views/pages/paged_container", "js/models/xblock_info", "jquery.simulate"],
     function ($, _, str, AjaxHelpers, TemplateHelpers, EditHelpers, ContainerPage, PagedContainerPage, XBlockInfo) {
 
         function parameterized_suite(label, global_page_options, fixtures) {
@@ -14,7 +14,9 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel
                     mockBadXBlockContainerXBlockHtml = readFixtures('mock/mock-bad-xblock-container-xblock.underscore'),
                     mockUpdatedContainerXBlockHtml = readFixtures('mock/mock-updated-container-xblock.underscore'),
                     mockXBlockEditorHtml = readFixtures('mock/mock-xblock-editor.underscore'),
-                    PageClass = fixtures.page;
+                    mockXBlockVisibilityEditorHtml = readFixtures('mock/mock-xblock-visibility-editor.underscore'),
+                    PageClass = fixtures.page,
+                    hasVisibilityEditor = fixtures.has_visibility_editor;
 
                 beforeEach(function () {
                     var newDisplayName = 'New Display Name';
@@ -219,6 +221,26 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel
                         });
                         expect(EditHelpers.isShowingModal()).toBeTruthy();
                     });
+
+                    it('can show a visibility modal for a child xblock if supported for the page', function() {
+                        var visibilityButtons;
+                        renderContainerPage(this, mockContainerXBlockHtml);
+                        visibilityButtons = containerPage.$('.wrapper-xblock .visibility-button');
+                        if (hasVisibilityEditor) {
+                            expect(visibilityButtons.length).toBe(6);
+                            visibilityButtons[0].click();
+                            expect(str.startsWith(lastRequest().url, '/xblock/locator-component-A1/visibility_view'))
+                                .toBeTruthy();
+                            AjaxHelpers.respondWithJson(requests, {
+                                html: mockXBlockVisibilityEditorHtml,
+                                resources: []
+                            });
+                            expect(EditHelpers.isShowingModal()).toBeTruthy();
+                        }
+                        else {
+                            expect(visibilityButtons.length).toBe(0);
+                        }
+                    });
                 });
 
                 describe("Editing an xmodule", function () {
@@ -572,19 +594,25 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel
             });
         }
 
+        // Create a suite for a non-paged container that includes 'edit visibility' buttons
         parameterized_suite("Non paged",
             { },
             {
                 page: ContainerPage,
                 initial: 'mock/mock-container-xblock.underscore',
-                add_response: 'mock/mock-xblock.underscore'
+                add_response: 'mock/mock-xblock.underscore',
+                has_visibility_editor: true
             }
         );
+
+        // Create a suite for a paged container that does not include 'edit visibility' buttons
         parameterized_suite("Paged",
             { page_size: 42 },
             {
                 page: PagedContainerPage,
                 initial: 'mock/mock-container-paged-xblock.underscore',
-                add_response: 'mock/mock-xblock-paged.underscore'
-            });
+                add_response: 'mock/mock-xblock-paged.underscore',
+                has_visibility_editor: false
+            }
+        );
     });

cms/static/js/spec/views/pages/container_subviews_spec.js

@@ -80,7 +80,8 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel
 
             describe("PreviewActionController", function () {
                 var viewPublishedCss = '.button-view',
-                    previewCss = '.button-preview';
+                    previewCss = '.button-preview',
+                    visibilityNoteCss = '.note-visibility';
 
                 it('renders correctly for unscheduled unit', function () {
                     renderContainerPage(this, mockContainerXBlockHtml);
@@ -109,6 +110,18 @@ define(["jquery", "underscore", "underscore.string", "js/common_helpers/ajax_hel
                     fetch({published: false, has_changes: false});
                     expect(containerPage.$(previewCss)).not.toHaveClass(disabledCss);
                 });
+
+                it('updates when has_content_group_components attribute changes', function () {
+                    renderContainerPage(this, mockContainerXBlockHtml);
+                    fetch({has_content_group_components: false});
+                    expect(containerPage.$(visibilityNoteCss).length).toBe(0);
+
+                    fetch({has_content_group_components: true});
+                    expect(containerPage.$(visibilityNoteCss).length).toBe(1);
+
+                    fetch({has_content_group_components: false});
+                    expect(containerPage.$(visibilityNoteCss).length).toBe(0);
+                });
             });
 
             describe("Publisher", function () {

cms/static/js/spec/views/pages/group_configurations_spec.js

 define([
     'jquery', 'underscore', 'js/views/pages/group_configurations',
-    'js/collections/group_configuration', 'js/common_helpers/template_helpers'
-], function ($, _, GroupConfigurationsPage, GroupConfigurationCollection, TemplateHelpers) {
+    'js/models/group_configuration', 'js/collections/group_configuration',
+    'js/common_helpers/template_helpers'
+], function ($, _, GroupConfigurationsPage, GroupConfigurationModel, GroupConfigurationCollection, TemplateHelpers) {
     'use strict';
     describe('GroupConfigurationsPage', function() {
         var mockGroupConfigurationsPage = readFixtures(
                 'mock/mock-group-configuration-page.underscore'
             ),
-            itemClassName = '.group-configurations-list-item';
+            groupConfigItemClassName = '.group-configurations-list-item';
 
         var initializePage = function (disableSpy) {
             var view = new GroupConfigurationsPage({
                 el: $('#content'),
-                collection: new GroupConfigurationCollection({
+                experimentsEnabled: true,
+                experimentGroupConfigurations: new GroupConfigurationCollection({
                     id: 0,
                     name: 'Configuration 1'
-                })
+                }),
+                contentGroupConfiguration: new GroupConfigurationModel({groups: []})
             });
 
             if (!disableSpy) {
@@ -29,15 +32,11 @@ define([
             return initializePage().render();
         };
 
-        var  clickNewConfiguration = function (view) {
-            view.$('.nav-actions .new-button').click();
-        };
-
         beforeEach(function () {
             setFixtures(mockGroupConfigurationsPage);
             TemplateHelpers.installTemplates([
-                'no-group-configurations', 'group-configuration-edit',
-                'group-configuration-details'
+                'group-configuration-editor', 'group-configuration-details', 'content-group-details',
+                'content-group-editor', 'group-edit', 'list'
             ]);
 
             this.addMatchers({
@@ -52,69 +51,67 @@ define([
                 var view = initializePage();
                 expect(view.$('.ui-loading')).toBeVisible();
                 view.render();
-                expect(view.$(itemClassName)).toExist();
+                expect(view.$(groupConfigItemClassName)).toExist();
+                expect(view.$('.content-groups .no-content')).toExist();
                 expect(view.$('.ui-loading')).toHaveClass('is-hidden');
             });
         });
 
-        describe('on page close/change', function() {
-            it('I see notification message if the model is changed',
-            function() {
-                var view = initializePage(true),
-                    message;
-
-                view.render();
-                message = view.onBeforeUnload();
-                expect(message).toBeUndefined();
-            });
-
-            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;
-
-                view.collection.at(0).set('name', 'Configuration 2');
-                message = view.onBeforeUnload();
-                expect(message).toBe(expectedMessage);
-            });
-        });
-
-        describe('Check that Group Configuration will focus and expand depending on content of url hash', function() {
+        describe('Experiment group configurations', function() {
             beforeEach(function () {
                 spyOn($.fn, 'focus');
                 TemplateHelpers.installTemplate('group-configuration-details');
                 this.view = initializePage(true);
             });
 
-            it('should focus and expand group configuration if its id is part of url hash', function() {
+            it('should focus and expand 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();
+                expect(this.view.$(groupConfigItemClassName)).toBeExpanded();
             });
 
-            it('should not focus on any group configuration if url hash is empty', function() {
+            it('should not focus on any experiment 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();
+                expect(this.view.$(groupConfigItemClassName)).not.toBeExpanded();
             });
 
-            it('should not focus on any group configuration if url hash contains wrong id', function() {
+            it('should not focus on any experiment 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();
+                expect(this.view.$(groupConfigItemClassName)).not.toBeExpanded();
+            });
+
+            it('should not show a notification message if an experiment configuration is not changed', function () {
+                this.view.render();
+                expect(this.view.onBeforeUnload()).toBeUndefined();
+            });
+
+            it('should show a notification message if an experiment configuration is changed', function () {
+                this.view.experimentGroupConfigurations.at(0).set('name', 'Configuration 2');
+                expect(this.view.onBeforeUnload())
+                    .toBe('You have unsaved changes. Do you really want to leave this page?');
             });
         });
 
-        it('create new group configuration', function () {
-            var view = renderPage();
+        describe('Content groups', function() {
+            beforeEach(function() {
+                this.view = renderPage();
+            });
+
+            it('should not show a notification message if a content group is not changed', function () {
+                expect(this.view.onBeforeUnload()).toBeUndefined();
+            });
 
-            clickNewConfiguration(view);
-            expect($('.group-configuration-edit').length).toBeGreaterThan(0);
+            it('should show a notification message if a content group is changed', function () {
+                this.view.contentGroupConfiguration.get('groups').add({name: 'Content Group'});
+                expect(this.view.onBeforeUnload())
+                    .toBe('You have unsaved changes. Do you really want to leave this page?');
+            });
         });
     });
 });

cms/static/js/spec/views/xblock_editor_spec.js

@@ -85,7 +85,7 @@ define([ "jquery", "underscore", "js/common_helpers/ajax_helpers", "js/spec_help
                     });
                     // Give the mock xblock a save method...
                     editor.xblock.save = window.MockDescriptor.save;
-                    editor.model.save(editor.getXModuleData());
+                    editor.model.save(editor.getXBlockFieldData());
                     request = requests[requests.length - 1];
                     response = JSON.parse(request.requestBody);
                     expect(response.metadata.display_name).toBe(testDisplayName);

cms/static/js/views/content_group_details.js

+/**
+ * This class defines a simple display view for a content group.
+ * It is expected to be backed by a Group model.
+ */
+define([
+    'js/views/baseview'
+], function(BaseView) {
+    'use strict';
+
+    var ContentGroupDetailsView = BaseView.extend({
+        tagName: 'div',
+        className: 'content-group-details collection',
+
+        events: {
+            'click .edit': 'editGroup'
+        },
+
+        editGroup: function() {
+            this.model.set({'editing': true});
+        },
+
+        initialize: function() {
+            this.template = this.loadTemplate('content-group-details');
+        },
+
+        render: function() {
+            this.$el.html(this.template(this.model.toJSON()));
+        }
+    });
+
+    return ContentGroupDetailsView;
+});

cms/static/js/views/content_group_editor.js

+/**
+ * This class defines an editing view for content groups.
+ * It is expected to be backed by a Group model.
+ */
+define([
+    'js/views/list_item_editor', 'underscore'
+],
+function(ListItemEditorView, _) {
+    'use strict';
+
+    var ContentGroupEditorView = ListItemEditorView.extend({
+        tagName: 'div',
+        className: 'content-group-edit collection-edit',
+        events: {
+            'submit': 'setAndClose',
+            'click .action-cancel': 'cancel'
+        },
+
+        initialize: function() {
+            ListItemEditorView.prototype.initialize.call(this);
+            this.template = this.loadTemplate('content-group-editor');
+        },
+
+        getTemplateOptions: function() {
+            return {
+                name: this.model.escape('name'),
+                index: this.model.collection.indexOf(this.model),
+                isNew: this.model.isNew(),
+                uniqueId: _.uniqueId()
+            };
+        },
+
+        setValues: function() {
+            this.model.set({name: this.$('input').val().trim()});
+            return this;
+        },
+
+        getSaveableModel: function() {
+            return this.model.collection.parents[0];
+        }
+    });
+
+    return ContentGroupEditorView;
+});

cms/static/js/views/content_group_item.js

+/**
+ * This class defines an controller view for content groups.
+ * It renders an editor view or a details view depending on the state
+ * of the underlying model.
+ * It is expected to be backed by a Group model.
+ */
+define([
+    'js/views/list_item', 'js/views/content_group_editor', 'js/views/content_group_details'
+], function(ListItemView, ContentGroupEditorView, ContentGroupDetailsView) {
+    'use strict';
+
+    var ContentGroupItemView = ListItemView.extend({
+        tagName: 'section',
+
+        baseClassName: 'content-group',
+
+        createEditView: function() {
+            return new ContentGroupEditorView({model: this.model});
+        },
+
+        createDetailsView: function() {
+            return new ContentGroupDetailsView({model: this.model});
+        }
+    });
+
+    return ContentGroupItemView;
+});

cms/static/js/views/content_group_list.js

+/**
+ * This class defines a list view for content groups.
+ * It is expected to be backed by a Group collection.
+ */
+define([
+    'js/views/list', 'js/views/content_group_item', 'gettext'
+], function(ListView, ContentGroupItemView, gettext) {
+    'use strict';
+
+    var ContentGroupListView = ListView.extend({
+        tagName: 'div',
+
+        className: 'content-group-list',
+
+        // Translators: This refers to a content group that can be linked to a student cohort.
+        itemCategoryDisplayName: gettext('content group'),
+
+        emptyMessage: gettext('You have not created any content groups yet.'),
+
+        createItemView: function(options) {
+            return new ContentGroupItemView(options);
+        }
+    });
+
+    return ContentGroupListView;
+});

cms/static/js/views/group_edit.js → cms/static/js/views/experiment_group_edit.js

+/**
+ * This class defines an edit view for groups within content experiment group configurations.
+ * It is expected to be backed by a Group model.
+ */
 define([
-    'js/views/baseview', 'underscore', 'underscore.string', 'jquery', 'gettext'
+    'js/views/baseview', 'underscore', 'underscore.string', 'gettext'
 ],
-function(BaseView, _, str, $, gettext) {
+function(BaseView, _, str, gettext) {
     'use strict';
     _.str = str; // used in template
-    var GroupEdit = BaseView.extend({
+    var ExperimentGroupEditView = BaseView.extend({
         tagName: 'li',
         events: {
             'click .action-close': 'removeGroup',
@@ -38,7 +42,7 @@ function(BaseView, _, str, $, gettext) {
         },
 
         changeName: function(event) {
-            if(event && event.preventDefault) { event.preventDefault(); }
+            if (event && event.preventDefault) { event.preventDefault(); }
             this.model.set({
                 name: this.$('.group-name').val()
             }, { silent: true });
@@ -47,7 +51,7 @@ function(BaseView, _, str, $, gettext) {
         },
 
         removeGroup: function(event) {
-            if(event && event.preventDefault) { event.preventDefault(); }
+            if (event && event.preventDefault) { event.preventDefault(); }
             this.model.collection.remove(this.model);
             return this.remove();
         },
@@ -65,5 +69,5 @@ function(BaseView, _, str, $, gettext) {
         }
     });
 
-    return GroupEdit;
+    return ExperimentGroupEditView;
 });

cms/static/js/views/group_configuration_details.js

+/**
+ * This class defines a details view for content experiment group configurations.
+ * It is expected to be instantiated with a GroupConfiguration model.
+ */
 define([
     'js/views/baseview', 'underscore', 'gettext', 'underscore.string'
 ],
 function(BaseView, _, gettext, str) {
     'use strict';
-    var GroupConfigurationDetails = BaseView.extend({
+    var GroupConfigurationDetailsView = BaseView.extend({
         tagName: 'div',
         events: {
             'click .edit': 'editConfiguration',
@@ -15,6 +19,7 @@ function(BaseView, _, gettext, str) {
             var index = this.model.collection.indexOf(this.model);
 
             return [
+                'collection',
                 'group-configuration-details',
                 'group-configuration-details-' + index
             ].join(' ');
@@ -40,17 +45,17 @@ function(BaseView, _, gettext, str) {
         },
 
         editConfiguration: function(event) {
-            if(event && event.preventDefault) { event.preventDefault(); }
+            if (event && event.preventDefault) { event.preventDefault(); }
             this.model.set('editing', true);
         },
 
         showGroups: function(event) {
-            if(event && event.preventDefault) { event.preventDefault(); }
+            if (event && event.preventDefault) { event.preventDefault(); }
             this.model.set('showGroups', true);
         },
 
         hideGroups: function(event) {
-            if(event && event.preventDefault) { event.preventDefault(); }
+            if (event && event.preventDefault) { event.preventDefault(); }
             this.model.set('showGroups', false);
         },
 
@@ -107,5 +112,5 @@ function(BaseView, _, gettext, str) {
         }
     });
 
-    return GroupConfigurationDetails;
+    return GroupConfigurationDetailsView;
 });

cms/static/js/views/group_configuration_edit.js → cms/static/js/views/group_configuration_editor.js

+/**
+ * This class defines an editing view for content experiment group configurations.
+ * It is expected to be backed by a GroupConfiguration model.
+ */
 define([
-    'js/views/baseview', 'underscore', 'jquery', 'gettext',
-    'js/views/group_edit', 'js/views/utils/view_utils'
+    'js/views/list_item_editor', 'underscore', 'jquery', 'gettext',
+    'js/views/experiment_group_edit'
 ],
-function(BaseView, _, $, gettext, GroupEdit, ViewUtils) {
+function(ListItemEditorView, _, $, gettext, ExperimentGroupEditView) {
     'use strict';
-    var GroupConfigurationEdit = BaseView.extend({
+    var GroupConfigurationEditorView = ListItemEditorView.extend({
         tagName: 'div',
         events: {
-            'change .group-configuration-name-input': 'setName',
+            'change .collection-name-input': 'setName',
             'change .group-configuration-description-input': 'setDescription',
-            "click .action-add-group": "createGroup",
+            'click .action-add-group': 'createGroup',
             'focus .input-text': 'onFocus',
             'blur .input-text': 'onBlur',
             'submit': 'setAndClose',
@@ -20,49 +24,57 @@ function(BaseView, _, $, gettext, GroupEdit, ViewUtils) {
             var index = this.model.collection.indexOf(this.model);
 
             return [
+                'collection-edit',
                 'group-configuration-edit',
                 'group-configuration-edit-' + index
             ].join(' ');
         },
 
         initialize: function() {
-            var groups;
+            var groups = this.model.get('groups');
 
-            this.template = this.loadTemplate('group-configuration-edit');
-            this.listenTo(this.model, 'invalid', this.render);
-            groups = this.model.get('groups');
-            this.listenTo(groups, 'add', this.addOne);
+            ListItemEditorView.prototype.initialize.call(this);
+
+            this.template = this.loadTemplate('group-configuration-editor');
+            this.listenTo(groups, 'add', this.onAddItem);
             this.listenTo(groups, 'reset', this.addAll);
             this.listenTo(groups, 'all', this.render);
         },
 
         render: function() {
-            this.$el.html(this.template({
+            ListItemEditorView.prototype.render.call(this);
+            this.addAll();
+            return this;
+        },
+
+        getTemplateOptions: function() {
+            return {
                 id: this.model.get('id'),
                 uniqueId: _.uniqueId(),
                 name: this.model.escape('name'),
                 description: this.model.escape('description'),
                 usage: this.model.get('usage'),
-                isNew: this.model.isNew(),
-                error: this.model.validationError
-            }));
-            this.addAll();
-            return this;
+                isNew: this.model.isNew()
+            };
+        },
+
+        getSaveableModel: function() {
+            return this.model;
         },
 
-        addOne: function(group) {
-            var view = new GroupEdit({ model: group });
+        onAddItem: function(group) {
+            var view = new ExperimentGroupEditView({ model: group });
             this.$('ol.groups').append(view.render().el);
 
             return this;
         },
 
         addAll: function() {
-            this.model.get('groups').each(this.addOne, this);
+            this.model.get('groups').each(this.onAddItem, this);
         },
 
         createGroup: function(event) {
-            if(event && event.preventDefault) { event.preventDefault(); }
+            if (event && event.preventDefault) { event.preventDefault(); }
             var collection = this.model.get('groups');
             collection.add([{
                 name: collection.getNextDefaultGroupName(),
@@ -71,15 +83,15 @@ function(BaseView, _, $, gettext, GroupEdit, ViewUtils) {
         },
 
         setName: function(event) {
-            if(event && event.preventDefault) { event.preventDefault(); }
+            if (event && event.preventDefault) { event.preventDefault(); }
             this.model.set(
-                'name', this.$('.group-configuration-name-input').val(),
+                'name', this.$('.collection-name-input').val(),
                 { silent: true }
             );
         },
 
         setDescription: function(event) {
-            if(event && event.preventDefault) { event.preventDefault(); }
+            if (event && event.preventDefault) { event.preventDefault(); }
             this.model.set(
                 'description',
                 this.$('.group-configuration-description-input').val(),
@@ -94,7 +106,7 @@ function(BaseView, _, $, gettext, GroupEdit, ViewUtils) {
             _.each(this.$('.groups li'), function(li, i) {
                 var group = this.model.get('groups').at(i);
 
-                if(group) {
+                if (group) {
                     group.set({
                         'name': $('.group-name', li).val()
                     });
@@ -102,56 +114,8 @@ function(BaseView, _, $, gettext, GroupEdit, ViewUtils) {
             }, this);
 
             return this;
-        },
-
-        setAndClose: function(event) {
-            if(event && event.preventDefault) { event.preventDefault(); }
-
-            this.setValues();
-            if(!this.model.isValid()) {
-                return false;
-            }
-
-            ViewUtils.runOperationShowingMessage(
-                gettext('Saving'),
-                function () {
-                    var dfd = $.Deferred();
-
-                    this.model.save({}, {
-                        success: function() {
-                            this.model.setOriginalAttributes();
-                            this.close();
-                            dfd.resolve();
-                        }.bind(this)
-                    });
-
-                    return dfd;
-                }.bind(this)
-            );
-        },
-
-        cancel: function(event) {
-            if(event && event.preventDefault) { event.preventDefault(); }
-
-            this.model.reset();
-            return this.close();
-        },
-
-        close: function() {
-            var groupConfigurations = this.model.collection;
-
-            this.remove();
-            if(this.model.isNew()) {
-                // if the group configuration has never been saved, remove it
-                groupConfigurations.remove(this.model);
-            } else {
-                // tell the model that it's no longer being edited
-                this.model.set('editing', false);
-            }
-
-            return this;
         }
     });
 
-    return GroupConfigurationEdit;
+    return GroupConfigurationEditorView;
 });

cms/static/js/views/group_configuration_item.js

+/**
+ * This class defines an controller view for content experiment group configurations.
+ * It renders an editor view or a details view depending on the state
+ * of the underlying model.
+ * It is expected to be backed by a Group model.
+ */
 define([
-    'js/views/baseview', 'jquery', "gettext", 'js/views/group_configuration_details',
-    'js/views/group_configuration_edit', "js/views/utils/view_utils"
+    'js/views/list_item', 'js/views/group_configuration_details', 'js/views/group_configuration_editor', 'gettext'
 ], function(
-    BaseView, $, gettext, GroupConfigurationDetails, GroupConfigurationEdit, ViewUtils
+    ListItemView, GroupConfigurationDetailsView, GroupConfigurationEditorView, gettext
 ) {
     'use strict';
-    var GroupConfigurationsItem = BaseView.extend({
+
+    var GroupConfigurationItemView = ListItemView.extend({
+        events: {
+            'click .delete': 'deleteItem'
+        },
+
         tagName: 'section',
+
+        baseClassName: 'group-configuration',
+
+        canDelete: true,
+
+        // Translators: this refers to a collection of groups.
+        itemDisplayName: gettext('group configuration'),
+
         attributes: function () {
             return {
                 'id': this.model.get('id'),
                 'tabindex': -1
             };
         },
-        events: {
-            'click .delete': 'deleteConfiguration'
-        },
-
-        className: function () {
-            var index = this.model.collection.indexOf(this.model);
 
-            return [
-                'group-configuration',
-                'group-configurations-list-item',
-                'group-configurations-list-item-' + index
-            ].join(' ');
+        createEditView: function() {
+            return new GroupConfigurationEditorView({model: this.model});
         },
 
-        initialize: function() {
-            this.listenTo(this.model, 'change:editing', this.render);
-            this.listenTo(this.model, 'remove', this.remove);
-        },
-
-        deleteConfiguration: function(event) {
-            if(event && event.preventDefault) { event.preventDefault(); }
-            var self = this;
-            ViewUtils.confirmThenRunOperation(
-                gettext('Delete this Group Configuration?'),
-                gettext('Deleting this Group Configuration is permanent and cannot be undone.'),
-                gettext('Delete'),
-                function() {
-                    return ViewUtils.runOperationShowingMessage(
-                        gettext('Deleting'),
-                        function () {
-                            return self.model.destroy({ wait: true });
-                        }
-                    );
-                }
-            );
-        },
-
-        render: function() {
-            // Removes a view from the DOM, and calls stopListening to remove
-            // any bound events that the view has listened to.
-            if (this.view) {
-                this.view.remove();
-            }
-
-            if (this.model.get('editing')) {
-                this.view = new GroupConfigurationEdit({
-                    model: this.model
-                });
-            } else {
-                this.view = new GroupConfigurationDetails({
-                    model: this.model
-                });
-            }
-
-            this.$el.html(this.view.render().el);
-
-            return this;
+        createDetailsView: function() {
+            return new GroupConfigurationDetailsView({model: this.model});
         }
     });
 
-    return GroupConfigurationsItem;
+    return GroupConfigurationItemView;
 });

cms/static/js/views/group_configurations_list.js

+/**
+ * This class defines a list view for content experiment group configurations.
+ * It is expected to be backed by a GroupConfiguration collection.
+ */
 define([
-    'js/views/baseview', 'jquery', 'js/views/group_configuration_item'
-], function(
-    BaseView, $, GroupConfigurationItemView
-) {
+    'js/views/list', 'js/views/group_configuration_item', 'gettext'
+], function(ListView, GroupConfigurationItemView, gettext) {
     'use strict';
-    var GroupConfigurationsList = BaseView.extend({
-        tagName: 'div',
-        className: 'group-configurations-list',
-        events: {
-            'click .new-button': 'addOne'
-        },
-
-        initialize: function() {
-            this.emptyTemplate = this.loadTemplate('no-group-configurations');
-            this.listenTo(this.collection, 'add', this.addNewItemView);
-            this.listenTo(this.collection, 'remove', this.handleDestory);
-        },
-
-        render: function() {
-            var configurations = this.collection;
-
-            if(configurations.length === 0) {
-                this.$el.html(this.emptyTemplate());
-            } else {
-                var frag = document.createDocumentFragment();
 
-                configurations.each(function(configuration) {
-                    var view = new GroupConfigurationItemView({
-                        model: configuration
-                    });
-
-                    frag.appendChild(view.render().el);
-                });
-
-                this.$el.html([frag]);
-            }
-
-            return this;
-        },
+    var GroupConfigurationsListView = ListView.extend({
+        tagName: 'div',
 
-        addNewItemView: function (model) {
-            var view = new GroupConfigurationItemView({
-                model: model
-            });
+        className: 'group-configurations-list',
 
-            // If items already exist, just append one new. Otherwise, overwrite
-            // no-content message.
-            if (this.collection.length > 1) {
-                this.$el.append(view.render().el);
-            } else {
-                this.$el.html(view.render().el);
-            }
+        newModelOptions: {addDefaultGroups: true},
 
-            view.$el.focus();
-        },
+        // Translators: this refers to a collection of groups.
+        itemCategoryDisplayName: gettext('group configuration'),
 
-        addOne: function(event) {
-            if(event && event.preventDefault) { event.preventDefault(); }
-            this.collection.add([{ editing: true }]);
-        },
+        emptyMessage: gettext('You have not created any group configurations yet.'),
 
-        handleDestory: function () {
-            if(this.collection.length === 0) {
-                this.$el.html(this.emptyTemplate());
-            }
+        createItemView: function(options) {
+            return new GroupConfigurationItemView(options);
         }
     });
 
-    return GroupConfigurationsList;
+    return GroupConfigurationsListView;
 });

cms/static/js/views/list.js

+/**
+ * A generic list view class.
+ *
+ * Expects the following properties to be overriden:
+ *   render when the collection is empty.
+ * - createItemView (function): Create and return an item view for a
+ *   model in the collection.
+ * - newModelOptions (object): Options to pass to models which are
+ *   added to the collection.
+ * - itemCategoryDisplayName (string): Display name for the category
+ *   of items this list contains.  For example, 'Group Configuration'.
+ *   Note that it must be translated.
+ * - emptyMessage (string): Text to render when the list is empty.
+ */
+define([
+    'js/views/baseview'
+], function(BaseView) {
+    'use strict';
+    var ListView = BaseView.extend({
+        events: {
+            'click .action-add': 'onAddItem',
+            'click .new-button': 'onAddItem'
+        },
+
+        listContainerCss: '.list-items',
+
+        initialize: function() {
+            this.listenTo(this.collection, 'add', this.addNewItemView);
+            this.listenTo(this.collection, 'remove', this.onRemoveItem);
+            this.template = this.loadTemplate('list');
+
+            // Don't render the add button when editing a form
+            this.listenTo(this.collection, 'change:editing', this.toggleAddButton);
+            this.listenTo(this.collection, 'add', this.toggleAddButton);
+            this.listenTo(this.collection, 'remove', this.toggleAddButton);
+        },
+
+        render: function(model) {
+            this.$el.html(this.template({
+                itemCategoryDisplayName: this.itemCategoryDisplayName,
+                emptyMessage: this.emptyMessage,
+                length: this.collection.length,
+                isEditing: model && model.get('editing')
+            }));
+
+            this.collection.each(function(model) {
+                this.$(this.listContainerCss).append(this.createItemView({model: model}).render().el);
+            }, this);
+
+            return this;
+        },
+
+        hideOrShowAddButton: function(shouldShow) {
+            var addButtonCss = '.action-add';
+            if (this.collection.length) {
+                if (shouldShow) {
+                    this.$(addButtonCss).removeClass('is-hidden');
+                } else {
+                    this.$(addButtonCss).addClass('is-hidden');
+                }
+            }
+        },
+
+        toggleAddButton: function(model) {
+            if (model.get('editing') && this.collection.contains(model)) {
+                this.hideOrShowAddButton(false);
+            } else {
+                this.hideOrShowAddButton(true);
+            }
+        },
+
+        addNewItemView: function (model) {
+            var view = this.createItemView({model: model});
+
+            // If items already exist, just append one new.
+            // Otherwise re-render the empty list HTML.
+            if (this.collection.length > 1) {
+                this.$(this.listContainerCss).append(view.render().el);
+            } else {
+                this.render();
+            }
+
+            view.$el.focus();
+        },
+
+        onAddItem: function(event) {
+            if (event && event.preventDefault) { event.preventDefault(); }
+            this.collection.add({editing: true}, this.newModelOptions);
+        },
+
+        onRemoveItem: function () {
+            if (this.collection.length === 0) {
+                this.render();
+            }
+        }
+    });
+
+    return ListView;
+});

cms/static/js/views/list_item.js

+/**
+ * A generic view to represent an editable item in a list.  The item
+ * has a edit view and a details view.
+ *
+ * Subclasses must implement:
+ * - itemDisplayName (string): Display name for the list item.
+ *   Must be translated.
+ * - baseClassName (string): CSS class name representing the item.
+ * - createEditView (function): Render and append the edit view to the
+ *   DOM.
+ * - createDetailsView (function): Render and append the details view
+ *   to the DOM.
+ */
+define([
+    'js/views/baseview', 'jquery', "gettext", "js/views/utils/view_utils"
+], function(
+    BaseView, $, gettext, ViewUtils
+) {
+    'use strict';
+
+    var ListItemView     = BaseView.extend({
+        canDelete: false,
+
+        initialize: function() {
+            this.listenTo(this.model, 'change:editing', this.render);
+            this.listenTo(this.model, 'remove', this.remove);
+        },
+
+        className: function () {
+            var index = this.model.collection.indexOf(this.model);
+
+            return [
+                'wrapper-collection',
+                'wrapper-collection-' + index,
+                this.baseClassName,
+                this.baseClassName + 's-list-item',
+                this.baseClassName + 's-list-item-' + index
+            ].join(' ');
+        },
+
+        deleteItem: function(event) {
+            if (event && event.preventDefault) { event.preventDefault(); }
+            if (!this.canDelete) { return; }
+            var model = this.model,
+                itemDisplayName = this.itemDisplayName;
+            ViewUtils.confirmThenRunOperation(
+                interpolate(
+                    // Translators: "item_display_name" is the name of the item to be deleted.
+                    gettext('Delete this %(item_display_name)s?'),
+                    {item_display_name: itemDisplayName}, true
+                ),
+                interpolate(
+                    // Translators: "item_display_name" is the name of the item to be deleted.
+                    gettext('Deleting this %(item_display_name)s is permanent and cannot be undone.'),
+                    {item_display_name: itemDisplayName},
+                    true
+                ),
+                gettext('Delete'),
+                function() {
+                    return ViewUtils.runOperationShowingMessage(
+                        gettext('Deleting'),
+                        function () {
+                            return model.destroy({wait: true});
+                        }
+                    );
+                }
+            );
+        },
+
+        render: function() {
+            // Removes a view from the DOM, and calls stopListening to remove
+            // any bound events that the view has listened to.
+            if (this.view) {
+                this.view.remove();
+            }
+
+            if (this.model.get('editing')) {
+                this.view = this.createEditView();
+            } else {
+                this.view = this.createDetailsView();
+            }
+
+            this.$el.html(this.view.render().el);
+
+            return this;
+        }
+    });
+
+    return ListItemView;
+});

cms/static/js/views/list_item_editor.js

+/**
+ * A generic view to represent a list item in its editing state.
+ *
+ * Subclasses must implement:
+ * - getTemplateOptions (function): Return an object to pass to the
+ *   template.
+ * - setValues (function): Set values on the model according to the
+ *   DOM.
+ * - getSaveableModel (function): Return the model which should be
+ *   saved by this view.  Note this may be a parent model.
+ */
+define([
+    'js/views/baseview', 'js/views/utils/view_utils', 'underscore', 'gettext'
+], function(BaseView, ViewUtils, _, gettext) {
+    'use strict';
+
+    var ListItemEditorView = BaseView.extend({
+        initialize: function() {
+            this.listenTo(this.model, 'invalid', this.render);
+        },
+
+        render: function() {
+            this.$el.html(this.template(_.extend({
+                error: this.model.validationError
+            }, this.getTemplateOptions())));
+        },
+
+        setAndClose: function(event) {
+            if (event && event.preventDefault) { event.preventDefault(); }
+
+            this.setValues();
+            if (!this.model.isValid()) {
+                return false;
+            }
+
+            ViewUtils.runOperationShowingMessage(
+                gettext('Saving'),
+                function () {
+                    var dfd = $.Deferred();
+                    var actionableModel = this.getSaveableModel();
+
+                    actionableModel.save({}, {
+                        success: function() {
+                            actionableModel.setOriginalAttributes();
+                            this.close();
+                            dfd.resolve();
+                        }.bind(this)
+                    });
+
+                    return dfd;
+                }.bind(this));
+        },
+
+        cancel: function(event) {
+            if (event && event.preventDefault) { event.preventDefault(); }
+
+            this.getSaveableModel().reset();
+            return this.close();
+        },
+
+        close: function() {
+            this.remove();
+            if (this.model.isNew() && !_.isUndefined(this.model.collection)) {
+                // if the item has never been saved, remove it
+                this.model.collection.remove(this.model);
+            } else {
+                // tell the model that it's no longer being edited
+                this.model.set('editing', false);
+            }
+
+            return this;
+        }
+    });
+
+    return ListItemEditorView;
+});

cms/static/js/views/metadata.js

@@ -49,7 +49,7 @@ function(BaseView, _, MetadataModel, AbstractEditor, FileUpload, UploadDialog, V
         },
 
         /**
-         * Returns the just the modified metadata values, in the format used to persist to the server.
+         * Returns just the modified metadata values, in the format used to persist to the server.
          */
         getModifiedMetadataValues: function () {
             var modified_values = {};

cms/static/js/views/modals/base_modal.js

 /**
  * This is a base modal implementation that provides common utilities.
+ *
+ * A modal implementation should override the following methods:
+ *
+ *   getTitle():
+ *     returns the title for the modal.
+ *   getHTMLContent():
+ *     returns the HTML content to be shown inside the modal.
+ *
+ * A modal implementation should also provide the following options:
+ *
+ *   modalName: A string identifying the modal.
+ *   modalType: A string identifying the type of the modal.
+ *   modalSize: A string, either 'sm', 'med', or 'lg' indicating the
+ *     size of the modal.
+ *   viewSpecificClasses: A string of CSS classes to be attached to
+ *     the modal window.
+ *   addSaveButton: A boolean indicating whether to include a save
+ *     button on the modal.
  */
 define(["jquery", "underscore", "gettext", "js/views/baseview"],
     function($, _, gettext, BaseView) {
@@ -41,7 +59,7 @@ define(["jquery", "underscore", "gettext", "js/views/baseview"],
                     name: this.options.modalName,
                     type: this.options.modalType,
                     size: this.options.modalSize,
-                    title: this.options.title,
+                    title: this.getTitle(),
                     viewSpecificClasses: this.options.viewSpecificClasses
                 }));
                 this.addActionButtons();
@@ -49,6 +67,10 @@ define(["jquery", "underscore", "gettext", "js/views/baseview"],
                 this.parentElement.append(this.$el);
             },
 
+            getTitle: function() {
+                return this.options.title;
+            },
+
             renderContents: function() {
                 var contentHtml = this.getContentHtml();
                 this.$('.modal-content').html(contentHtml);

cms/static/js/views/modals/edit_xblock.js

@@ -6,6 +6,8 @@
 define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/views/utils/view_utils",
     "js/models/xblock_info", "js/views/xblock_editor"],
     function($, _, gettext, BaseModal, ViewUtils, XBlockInfo, XBlockEditorView) {
+        "strict mode";
+
         var EditXBlockModal = BaseModal.extend({
             events : {
                 "click .action-save": "save",
@@ -15,7 +17,10 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie
             options: $.extend({}, BaseModal.prototype.options, {
                 modalName: 'edit-xblock',
                 addSaveButton: true,
-                viewSpecificClasses: 'modal-editor confirm'
+                view: 'studio_view',
+                viewSpecificClasses: 'modal-editor confirm',
+                // Translators: "title" is the name of the current component being edited.
+                titleFormat: gettext("Editing: %(title)s")
             }),
 
             initialize: function() {
@@ -56,7 +61,8 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie
             displayXBlock: function() {
                 this.editorView = new XBlockEditorView({
                     el: this.$('.xblock-editor'),
-                    model: this.xblockInfo
+                    model: this.xblockInfo,
+                    view: this.options.view
                 });
                 this.editorView.render({
                     success: _.bind(this.onDisplayXBlock, this)
@@ -66,7 +72,7 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie
             onDisplayXBlock: function() {
                 var editorView = this.editorView,
                     title = this.getTitle(),
-                    readOnlyView = (this.editOptions && this.editOptions.readOnlyView) || !editorView.xblock.save;
+                    readOnlyView = (this.editOptions && this.editOptions.readOnlyView) || !this.canSave();
 
                 // Notify the runtime that the modal has been shown
                 editorView.notifyRuntime('modal-shown', this);
@@ -99,6 +105,10 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie
                 this.resize();
             },
 
+            canSave: function() {
+                return this.editorView.xblock.save || this.editorView.xblock.collectFieldData;
+            },
+
             disableSave: function() {
                 var saveButton = this.getActionButton('save'),
                     cancelButton = this.getActionButton('cancel');
@@ -112,7 +122,7 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie
                 if (!displayName) {
                     displayName = gettext('Component');
                 }
-                return interpolate(gettext("Editing: %(title)s"), { title: displayName }, true);
+                return interpolate(this.options.titleFormat, { title: displayName }, true);
             },
 
             addDefaultModes: function() {
@@ -147,7 +157,7 @@ define(["jquery", "underscore", "gettext", "js/views/modals/base_modal", "js/vie
                 var self = this,
                     editorView = this.editorView,
                     xblockInfo = this.xblockInfo,
-                    data = editorView.getXModuleData();
+                    data = editorView.getXBlockFieldData();
                 event.preventDefault();
                 if (data) {
                     ViewUtils.runOperationShowingMessage(gettext('Saving'),

cms/static/js/views/pages/container.js

@@ -15,6 +15,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views
 
             events: {
                 "click .edit-button": "editXBlock",
+                "click .visibility-button": "editVisibilitySettings",
                 "click .duplicate-button": "duplicateXBlock",
                 "click .delete-button": "deleteXBlock",
                 "click .new-component-button": "scrollToNewComponentButtons"
@@ -161,10 +162,10 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views
                 }
             },
 
-            editXBlock: function(event) {
+            editXBlock: function(event, options) {
                 var xblockElement = this.findXBlockElement(event.target),
                     self = this,
-                    modal = new EditXBlockModal({ });
+                    modal = new EditXBlockModal(options);
                 event.preventDefault();
 
                 modal.edit(xblockElement, this.model, {
@@ -175,6 +176,16 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views
                 });
             },
 
+            editVisibilitySettings: function(event) {
+                this.editXBlock(event, {
+                    view: 'visibility_view',
+                    // Translators: "title" is the name of the current component being edited.
+                    titleFormat: gettext("Editing visibility for: %(title)s"),
+                    viewSpecificClasses: '',
+                    modalSize: 'med'
+                });
+            },
+
             duplicateXBlock: function(event) {
                 event.preventDefault();
                 this.duplicateComponent(this.findXBlockElement(event.target));

cms/static/js/views/pages/container_subviews.js

@@ -100,7 +100,8 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/
 
             onSync: function(model) {
                 if (ViewUtils.hasChangedAttributes(model, [
-                    'has_changes', 'published', 'edited_on', 'edited_by', 'visibility_state', 'has_explicit_staff_lock'
+                    'has_changes', 'published', 'edited_on', 'edited_by', 'visibility_state',
+                    'has_explicit_staff_lock', 'has_content_group_components'
                 ])) {
                    this.render();
                 }
@@ -120,7 +121,8 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/utils/
                     releaseDate: this.model.get('release_date'),
                     releaseDateFrom: this.model.get('release_date_from'),
                     hasExplicitStaffLock: this.model.get('has_explicit_staff_lock'),
-                    staffLockFrom: this.model.get('staff_lock_from')
+                    staffLockFrom: this.model.get('staff_lock_from'),
+                    hasContentGroupComponents: this.model.get('has_content_group_components')
                 }));
 
                 return this;

cms/static/js/views/pages/course_outline.js

@@ -26,7 +26,7 @@ define(["jquery", "underscore", "gettext", "js/views/pages/base_page", "js/views
                 });
                 this.model.on('change', this.setCollapseExpandVisibility, this);
                 $('.dismiss-button').bind('click', ViewUtils.deleteNotificationHandler(function () {
-                    $('.wrapper-alert-announcement').removeClass('is-shown').addClass('is-hidden')
+                    $('.wrapper-alert-announcement').removeClass('is-shown').addClass('is-hidden');
                 }));
             },
 

cms/static/js/views/pages/group_configurations.js

 define([
     'jquery', 'underscore', 'gettext', 'js/views/pages/base_page',
-    'js/views/group_configurations_list'
+    'js/views/group_configurations_list', 'js/views/content_group_list'
 ],
-function ($, _, gettext, BasePage, GroupConfigurationsList) {
+function ($, _, gettext, BasePage, GroupConfigurationsListView, ContentGroupListView) {
     'use strict';
     var GroupConfigurationsPage = BasePage.extend({
-        initialize: function() {
+        initialize: function(options) {
             BasePage.prototype.initialize.call(this);
-            this.listView = new GroupConfigurationsList({
-                collection: this.collection
+            this.experimentsEnabled = options.experimentsEnabled;
+            if (this.experimentsEnabled) {
+                this.experimentGroupConfigurations = options.experimentGroupConfigurations;
+                this.experimentGroupsListView = new GroupConfigurationsListView({
+                    collection: this.experimentGroupConfigurations
+                });
+            }
+            this.contentGroupConfiguration = options.contentGroupConfiguration;
+            this.cohortGroupsListView = new ContentGroupListView({
+                collection: this.contentGroupConfiguration.get('groups')
             });
         },
 
         renderPage: function() {
             var hash = this.getLocationHash();
-            this.$('.content-primary').append(this.listView.render().el);
-            this.addButtonActions();
+            if (this.experimentsEnabled) {
+                this.$('.wrapper-groups.experiment-groups').append(this.experimentGroupsListView.render().el);
+            }
+            this.$('.wrapper-groups.content-groups').append(this.cohortGroupsListView.render().el);
             this.addWindowActions();
             if (hash) {
                 // Strip leading '#' to get id string to match
@@ -24,22 +34,17 @@ function ($, _, gettext, BasePage, GroupConfigurationsList) {
             return $.Deferred().resolve().promise();
         },
 
-        addButtonActions: function () {
-            this.$('.nav-actions .new-button').click(function (event) {
-                this.listView.addOne(event);
-            }.bind(this));
-        },
-
         addWindowActions: function () {
             $(window).on('beforeunload', this.onBeforeUnload.bind(this));
         },
 
         onBeforeUnload: function () {
-            var dirty = this.collection.find(function(configuration) {
-                return configuration.isDirty();
-            });
+            var dirty = this.contentGroupConfiguration.isDirty() ||
+                (this.experimentsEnabled && this.experimentGroupConfigurations.find(function(configuration) {
+                    return configuration.isDirty();
+                }));
 
-            if(dirty) {
+            if (dirty) {
                 return gettext('You have unsaved changes. Do you really want to leave this page?');
             }
         },
@@ -57,7 +62,7 @@ function ($, _, gettext, BasePage, GroupConfigurationsList) {
          * @param {String|Number} Id of the group configuration.
          */
         expandConfiguration: function (id) {
-            var groupConfig = this.collection.findWhere({
+            var groupConfig = this.experimentsEnabled && this.experimentGroupConfigurations.findWhere({
                 id: parseInt(id)
             });
 

cms/static/js/views/xblock_editor.js

@@ -89,17 +89,23 @@ define(["jquery", "underscore", "gettext", "js/views/xblock", "js/views/metadata
             },
 
             /**
-             * Returns the data saved for the xmodule. Note that this *does not* work for XBlocks.
+             * Returns the updated field data for the xblock. Note that this works for all
+             * XModules as well as for XBlocks that provide a 'collectFieldData' API.
              */
-            getXModuleData: function() {
+            getXBlockFieldData: function() {
                 var xblock = this.xblock,
                     metadataEditor = this.getMetadataEditor(),
                     data = null;
-                if (xblock.save) {
+                // If the xblock supports returning its field data then collect it
+                if (xblock.collectFieldData) {
+                    data = xblock.collectFieldData();
+                // ... else if this is an XModule then call its save method
+                } else if (xblock.save) {
                     data = xblock.save();
                     if (metadataEditor) {
                         data.metadata = _.extend(data.metadata || {}, this.getChangedMetadata());
                     }
+                // ... else log an error
                 } else {
                     console.error('Cannot save xblock as it has no save method');
                 }

cms/static/js/xblock/authoring.js

+/**
+ * Client-side logic to support XBlock authoring.
+ */
+(function($) {
+    'use strict';
+
+    function VisibilityEditorView(runtime, element) {
+        this.getGroupAccess = function() {
+            var groupAccess, userPartitionId, selectedGroupIds;
+            if (element.find('.visibility-level-all').prop('checked')) {
+                return {};
+            }
+            userPartitionId = element.find('.wrapper-visibility-specific').data('user-partition-id').toString();
+            selectedGroupIds = [];
+            element.find('.field-visibility-content-group input:checked').each(function(index, input) {
+                selectedGroupIds.push(parseInt($(input).val()));
+            });
+            groupAccess = {};
+            groupAccess[userPartitionId] = selectedGroupIds;
+            return groupAccess;
+        };
+
+        element.find('.field-visibility-level input').change(function(event) {
+            if ($(event.target).hasClass('visibility-level-all')) {
+                element.find('.field-visibility-content-group input').prop('checked', false);
+            }
+        });
+        element.find('.field-visibility-content-group input').change(function(event) {
+            element.find('.visibility-level-all').prop('checked', false);
+            element.find('.visibility-level-specific').prop('checked', true);
+        });
+    }
+
+    VisibilityEditorView.prototype.collectFieldData = function collectFieldData() {
+        return {
+            metadata: {
+                "group_access": this.getGroupAccess()
+            }
+        };
+    };
+
+    function initializeVisibilityEditor(runtime, element) {
+        return new VisibilityEditorView(runtime, element);
+    }
+
+    // XBlock initialization functions must be global
+    window.VisibilityEditorInit = initializeVisibilityEditor;
+})($);

cms/static/sass/_variables.scss

@@ -183,6 +183,7 @@ $color-ready: $green;
 $color-warning: $orange-l2;
 $color-error: $red-l2;
 $color-staff-only: $black;
+$color-visibility-set: $black;
 
 $color-heading-base: $gray-d2;
 $color-copy-base: $gray-l1;

cms/static/sass/elements/_forms.scss

 // studio - elements - forms
 // ====================
 
-// Table of Contents 
+// Table of Contents
 // * +Forms - General
 // * +Field - Is Editable
 // * +Field - With Error
@@ -12,7 +12,23 @@
 // * +Form - Grandfathered
 
 // +Forms - General
-// ==================== 
+// ====================
+// element-specific utilities
+// --------------------
+// UI: checkbox/radio inputs
+%input-tickable {
+
+  ~ label {
+    color: $color-copy-base;
+  }
+
+  // STATE: checked/selected
+  &:checked ~ label {
+    @extend %t-strong;
+    color: $ui-action-primary-color-focus;
+  }
+}
+
 input[type="text"],
 input[type="email"],
 input[type="password"],
@@ -77,7 +93,7 @@ form {
     }
 
     .input-checkbox-checked, .input-checkbox-unchecked {
-      width: $baseline;
+      width: ($baseline*0.75);
     }
 
     .input-checkbox {
@@ -107,8 +123,18 @@ form {
     }
   }
 
+  // CASE: checkbox input
+  .field-checkbox .input-checkbox {
+    @extend %input-tickable;
+  }
+
+  // CASE: radio input
+  .field-radio .input-radio {
+    @extend %input-tickable;
+  }
+
   // CASE: file input
-  input[type=file] {
+  input[type="file"] {
     @extend %t-copy-sub1;
   }
 

cms/static/sass/elements/_modal-window.scss

@@ -52,6 +52,45 @@
       }
     }
 
+    // UI: summary messages
+    .summary-message {
+      margin-bottom: $baseline;
+      padding: ($baseline*0.75);
+      background: $gray-d3;
+
+      .icon, .copy {
+        display: inline-block;
+        vertical-align: top;
+      }
+
+      .icon {
+        @extend %t-icon4;
+        @include margin-right($baseline/2);
+        color: $white;
+      }
+
+      .copy {
+        @extend %t-copy-sub1;
+        max-width: 85%;
+        color: $white;
+      }
+    }
+
+    // CASE: Warning summary message
+    .summary-message-warning {
+      border-top: ($baseline/5) solid $color-warning;
+
+      .icon {
+        color: $color-warning;
+      }
+    }
+
+    // visual dividers
+    .divider-visual {
+      margin: ($baseline*0.75) 0;
+      border: ($baseline/20) solid $gray-l4;
+    }
+
     // sections within a modal
     .modal-section {
       margin-bottom: ($baseline*0.75);
@@ -64,11 +103,20 @@
     .modal-section-title {
       @extend %t-title6;
       margin: 0 0 ($baseline/2) 0;
-      border-bottom: 1px solid $gray-l4;
+      border-bottom: ($baseline/10) solid $gray-l4;
       padding-bottom: ($baseline/4);
       color: $gray-d2;
     }
 
+    .modal-subsection-title {
+      @extend %t-title8;
+      @extend %t-strong;
+      margin-bottom: ($baseline/4);
+      text-transform: uppercase;
+      letter-spacing: 0.1;
+      color: $gray-l2;
+    }
+
     .modal-section-content {
 
       .list-fields, .list-actions {
@@ -238,143 +286,6 @@
     }
   }
 
-  // outline: edit item settings
-  .wrapper-modal-window-bulkpublish-section,
-  .wrapper-modal-window-bulkpublish-subsection,
-  .wrapper-modal-window-bulkpublish-unit,
-  .course-outline-modal {
-
-    .list-fields {
-
-      .field {
-        display: inline-block;
-        vertical-align: top;
-        margin-right: ($baseline/2);
-        margin-bottom: ($baseline/4);
-
-        label {
-          @extend %t-copy-sub1;
-          @extend %t-strong;
-          @include transition(color $tmg-f3 ease-in-out 0s);
-          margin: 0 0 ($baseline/4) 0;
-
-          &.is-focused {
-            color: $blue;
-          }
-        }
-
-
-        input, textarea {
-          @extend %t-copy-base;
-          @include transition(all $tmg-f2 ease-in-out 0s);
-          height: 100%;
-          width: 100%;
-          padding: ($baseline/2);
-
-          // CASE: long length
-          &.long {
-            width: 100%;
-          }
-
-          // CASE: short length
-          &.short {
-            width: 25%;
-          }
-        }
-
-        // CASE: specific release + due times/dates
-        .start-date,
-        .start-time,
-        .due-date,
-        .due-time {
-          width: ($baseline*7);
-        }
-
-        .tip {
-          @extend %t-copy-sub1;
-          @include transition(color, 0.15s, ease-in-out);
-          display: block;
-          margin-top: ($baseline/4);
-          color: $gray-l2;
-        }
-
-        .tip-warning {
-          color: $gray-d2;
-        }
-      }
-
-      // CASE: type-based input
-      .field-text {
-
-        // TODO: refactor the _forms.scss partial to allow for this area to inherit from it
-        label, input, textarea {
-          display: block;
-        }
-      }
-
-      // CASE: select input
-      .field-select {
-
-        .label, .input {
-          display: inline-block;
-          vertical-align: middle;
-        }
-
-        .label {
-          margin-right: ($baseline/2);
-        }
-
-        .input {
-          width: 100%;
-        }
-
-        // CASE: checkbox input
-        .field-checkbox {
-
-          .label, label {
-            margin-bottom: 0;
-          }
-        }
-      }
-    }
-
-
-
-    // UI: grading section
-    .edit-settings-grading {
-
-      .grading-type {
-        margin-bottom: $baseline;
-      }
-    }
-
-    // UI: staff lock section
-    .edit-staff-lock {
-
-      .checkbox-cosmetic .input-checkbox {
-        @extend %cont-text-sr;
-
-        // CASE: unchecked
-        ~ .tip-warning {
-          display: block;
-        }
-
-        // CASE: checked
-        &:checked {
-
-          ~ .tip-warning {
-            display: none;
-          }
-        }
-      }
-
-      // needed to override poorly scoped margin-bottom on any label element in a view (from _forms.scss)
-      .checkbox-cosmetic .label {
-        margin-bottom: 0;
-      }
-    }
-  }
-
   // xblock custom actions
   .modal-window .editor-with-buttons {
     margin-bottom: ($baseline*3);
@@ -394,7 +305,7 @@
   }
 
 
-  // special overrides for video module editor/hidden tab editors
+  // MODAL TYPE: component - video modal (includes special overrides for xblock-related editing view)
   .modal-lg.modal-type-video {
 
     .modal-content {
@@ -517,4 +428,225 @@
     opacity: 0.5;
     filter: alpha(opacity=50);
   }
+
+  // MODAL TYPE: component - visibility modal
+  .xblock-visibility_view {
+
+    .visibility-controls-secondary {
+      max-height: 100%;
+      overflow-y: auto;
+      @include margin(($baseline*0.75), 0, 0, $baseline);
+    }
+
+    .visibility-controls-group {
+      @extend %wipe-last-child;
+      margin-bottom: $baseline;
+    }
+
+    // UI: form fields
+    .list-fields {
+
+      .field {
+        @extend %wipe-last-child;
+        margin-bottom: ($baseline/4);
+
+        label {
+          @extend %t-copy-sub1;
+        }
+      }
+
+      // UI: radio and checkbox inputs
+      .field-radio, .field-checkbox {
+
+        label {
+          @include margin-left($baseline/4);
+        }
+      }
+    }
+
+    // CASE: content group has been removed
+    .field-visibility-content-group.was-removed {
+
+      .input-checkbox:checked ~ label {
+        color: $color-error;
+      }
+
+      .note {
+        @extend %t-copy-sub2;
+        @extend %t-regular;
+        display: block;
+        color: $color-error;
+      }
+    }
+
+    // CASE: no groups configured for visibility
+    .is-not-configured {
+      @extend %no-content;
+      padding: ($baseline);
+      @include text-align(left); // reset for %no-content's default styling
+
+      .title {
+        @extend %t-title6;
+        font-weight: 600;  // needed for poorly scoped .title rule in modals
+        margin: 0 0 ($baseline/2) 0; // needed for poorly scoped .title rule in modals
+      }
+
+      .copy {
+        @extend %t-copy-sub1;
+
+         p {
+          @extend %wipe-last-child;
+          margin-bottom: $baseline;
+        }
+      }
+
+      &.has-actions {
+
+        .actions {
+          margin-top: $baseline;
+        }
+
+        .action {
+          @include margin-left(0); // reset for %no-content's default styling
+        }
+      }
+    }
+  }
+
+  // MODAL TYPE: outline - edit item settings
+  .wrapper-modal-window-bulkpublish-section,
+  .wrapper-modal-window-bulkpublish-subsection,
+  .wrapper-modal-window-bulkpublish-unit,
+  .course-outline-modal {
+
+    .list-fields {
+
+      .field {
+        display: inline-block;
+        vertical-align: top;
+        @include margin-right($baseline/2);
+        margin-bottom: ($baseline/4);
+
+        label {
+          @extend %t-copy-sub1;
+          @extend %t-strong;
+          @include transition(color $tmg-f3 ease-in-out 0s);
+          margin: 0 0 ($baseline/4) 0;
+
+          &.is-focused {
+            color: $blue;
+          }
+        }
+
+
+        input, textarea {
+          @extend %t-copy-base;
+          @include transition(all $tmg-f2 ease-in-out 0s);
+          height: 100%;
+          width: 100%;
+          padding: ($baseline/2);
+
+          // CASE: long length
+          &.long {
+            width: 100%;
+          }
+
+          // CASE: short length
+          &.short {
+            width: 25%;
+          }
+        }
+
+        // CASE: specific release + due times/dates
+        .start-date,
+        .start-time,
+        .due-date,
+        .due-time {
+          width: ($baseline*7);
+        }
+
+        .tip {
+          @extend %t-copy-sub1;
+          @include transition(color, 0.15s, ease-in-out);
+          display: block;
+          margin-top: ($baseline/4);
+          color: $gray-l2;
+        }
+
+        .tip-warning {
+          color: $gray-d2;
+        }
+      }
+
+      // CASE: type-based input
+      .field-text {
+
+        // TODO: refactor the _forms.scss partial to allow for this area to inherit from it
+        label, input, textarea {
+          display: block;
+        }
+      }
+
+      // CASE: select input
+      .field-select {
+
+        .label, .input {
+          display: inline-block;
+          vertical-align: middle;
+        }
+
+        .label {
+          @include margin-right($baseline/2);
+        }
+
+        .input {
+          width: 100%;
+        }
+
+        // CASE: checkbox input
+        .field-checkbox {
+
+          .label, label {
+            margin-bottom: 0;
+          }
+        }
+      }
+    }
+
+
+
+    // UI: grading section
+    .edit-settings-grading {
+
+      .grading-type {
+        margin-bottom: $baseline;
+      }
+    }
+
+    // UI: staff lock section
+    .edit-staff-lock {
+
+      .checkbox-cosmetic .input-checkbox {
+        @extend %cont-text-sr;
+
+        // CASE: unchecked
+        ~ .tip-warning {
+          display: block;
+        }
+
+        // CASE: checked
+        &:checked {
+
+          ~ .tip-warning {
+            display: none;
+          }
+        }
+      }
+
+      // needed to override poorly scoped margin-bottom on any label element in a view (from _forms.scss)
+      .checkbox-cosmetic .label {
+        margin-bottom: 0;
+      }
+    }
+  }
 }

cms/static/sass/elements/_xblocks.scss

@@ -150,42 +150,50 @@
 
   // ====================
 
-  // UI: xblocks - calls-to-action
-  .wrapper-xblock .header-actions {
+  .wrapper-xblock {
 
-    .actions-list {
+    // UI: xblocks - calls-to-action
+    .header-actions .actions-list {
       @extend %actions-list;
     }
-  }
 
-  // UI: xblock is collapsible
-  .wrapper-xblock.is-collapsible,
-  .wrapper-xblock.xblock-type-container {
+    // CASE: xblock is collapsible
+    &.is-collapsible,
+    &.xblock-type-container {
 
-    .icon {
-      font-style: normal;
-    }
+      .icon {
+        font-style: normal;
+      }
 
-    .expand-collapse {
-      @extend %expand-collapse;
-      margin: 0 ($baseline/4);
-      height: ($baseline*1.25);
-      width: $baseline;
+      .expand-collapse {
+        @extend %expand-collapse;
+        margin: 0 ($baseline/4);
+        height: ($baseline*1.25);
+        width: $baseline;
 
-      &:focus {
-        outline: 0;
+        &:focus {
+          outline: 0;
+        }
       }
-    }
 
-    .action-view {
+      .action-view {
+
+        .action-button {
+          transition: none;
+        }
 
-      .action-button {
-        transition: none;
+        .action-button-text {
+          padding-right: ($baseline/5);
+          padding-left: 0;
+        }
       }
+    }
+
+    // CASE: xblock has specific visibility based on content groups set
+    &.has-group-visibility-set {
 
-      .action-button-text {
-        padding-right: ($baseline/5);
-        padding-left: 0;
+      .action-visibility .visibility-button.visibility-button { // needed to cascade in front of overscoped header-actions CSS rule
+        color: $color-visibility-set;
       }
     }
   }

cms/static/sass/views/_container.scss

@@ -6,7 +6,20 @@
 
 // ====================
 
+// view-specific utilities
+// --------------------
+%status-value-base {
+  @extend %t-title7;
+  @extend %t-strong;
+}
+
+%status-value-sub1 {
+  @extend %t-title8;
+  display: block;
+}
+
 // UI: container page view
+// --------------------
 .view-container {
   @extend %two-col-1;
 
@@ -102,6 +115,7 @@
       @extend %t-title8;
     }
 
+    // UI: publishing details/summary
     .bit-publishing {
       @extend %bar-module;
 
@@ -159,37 +173,43 @@
       .wrapper-release {
 
         .release-date {
-          @extend %t-strong;
+          @extend %status-value-base;
         }
 
         .release-with {
-          @extend %t-title8;
-          display: block;
+          @extend %status-value-sub1;
         }
       }
 
       .wrapper-visibility {
 
         .copy {
-          @extend %t-strong;
+          @extend %status-value-base;
           margin-bottom: ($baseline/10);
         }
 
         .icon {
-          margin-left: ($baseline/4);
           color: $gray-d1;
         }
 
         .inherited-from {
-          @extend %t-title8;
-          display: block;
+          @extend %status-value-sub1;
         }
 
+        // UI: note about specific access
+        .note-visibility {
+          @extend %status-value-sub1;
 
+          .icon {
+            @include margin-right($baseline/4);
+          }
+        }
       }
 
       .wrapper-pub-actions {
-        padding: ($baseline*0.75);
+        border-top: 1px solid $gray-l4;
+        margin-top: ($baseline/2);
+        padding: $baseline ($baseline*0.75) ($baseline*0.75) ($baseline*0.75);
 
         .action-publish {
           @extend %btn-primary-blue;
@@ -209,7 +229,6 @@
           }
         }
       }
-
     }
 
     // versioning widget
@@ -244,8 +263,7 @@
       .wrapper-unit-id, .wrapper-library-id {
 
         .unit-id-value, .library-id-value {
-          @extend %cont-text-wrap;
-          @extend %t-copy-sub1;
+          @extend %status-value-base;
           display: inline-block;
           width: 100%;
         }
@@ -308,5 +326,3 @@
     }
   }
 }
-
-

cms/templates/base.html

@@ -86,6 +86,9 @@ import json
     </div>
 
     <div id="page-prompt"></div>
+
+    <%block name="modal_placeholder"></%block>
+
     <%block name="jsextra"></%block>
     <script type="text/javascript">
       require(['js/factories/common_deps'], function () {

cms/templates/group_configurations.html

 <%inherit file="base.html" />
-<%def name="online_help_token()"><% return "group_configurations" %></%def>
+<%def name="content_groups_help_token()"><% return "content_groups" %></%def>
+<%def name="experiment_group_configurations_help_token()"><% return "group_configurations" %></%def>
 <%namespace name='static' file='static_content.html'/>
 <%! import json %>
 <%!
@@ -11,7 +12,7 @@
 <%block name="bodyclass">is-signedin course view-group-configurations</%block>
 
 <%block name="header_extras">
-% for template_name in ["group-configuration-details", "group-configuration-edit", "no-group-configurations", "group-edit", "basic-modal", "modal-button"]:
+% for template_name in ["group-configuration-details", "group-configuration-editor", "group-edit", "content-group-editor", "content-group-details", "basic-modal", "modal-button", "list"]:
   <script type="text/template" id="${template_name}-tpl">
     <%static:include path="js/${template_name}.underscore" />
   </script>
@@ -19,11 +20,9 @@
 </%block>
 
 <%block name="requirejs">
-% if configurations is not None:
   require(["js/factories/group_configurations"], function(GroupConfigurationsFactory) {
-      GroupConfigurationsFactory(${json.dumps(configurations)}, "${group_configuration_url}", "${course_outline_url}");
+      GroupConfigurationsFactory(${json.dumps(should_show_experiment_groups)}, ${json.dumps(experiment_group_configurations)}, ${json.dumps(content_group_configuration)}, "${group_configuration_url}", "${course_outline_url}");
   });
-% endif
 </%block>
 
 <%block name="content">
@@ -33,45 +32,56 @@
         <small class="subtitle">${_("Settings")}</small>
         <span class="sr">&gt; </span>${_("Group Configurations")}
       </h1>
-      <nav class="nav-actions">
-        <h3 class="sr">${_("Page Actions")}</h3>
-        <ul>
-          <li class="nav-item">
-            <a href="#" class="button new-button"><i class="icon fa fa-plus"></i> ${_("New Group Configuration")}</a>
-          </li>
-        </ul>
-      </nav>
     </header>
   </div>
 
   <div class="wrapper-content wrapper">
     <section class="content">
       <article class="content-primary" role="main">
-      % if configurations is None:
-        <div class="notice notice-incontext notice-moduledisabled">
-            <p class="copy">
-                ${_("This module is disabled at the moment.")}
-            </p>
+        <div class="wrapper-groups content-groups">
+          <h3 class="title">${_("Content Groups")}</h3>
+            <div class="ui-loading">
+                <p><span class="spin"><i class="icon fa fa-refresh"></i></span> <span class="copy">${_("Loading")}</span></p>
+            </div>
         </div>
-      % else:
-        <div class="ui-loading">
-            <p><span class="spin"><i class="icon fa fa-refresh"></i></span> <span class="copy">${_("Loading")}</span></p>
+
+      % if should_show_experiment_groups:
+        <div class="wrapper-groups experiment-groups">
+          <h3 class="title">${_("Experiment Group Configurations")}</h3>
+          % if experiment_group_configurations is None:
+            <div class="notice notice-incontext notice-moduledisabled">
+                <p class="copy">
+                    ${_("This module is disabled at the moment.")}
+                </p>
+            </div>
+          % else:
+            <div class="ui-loading">
+                <p><span class="spin"><i class="icon fa fa-refresh"></i></span> <span class="copy">${_("Loading")}</span></p>
+            </div>
+          % endif
         </div>
       % endif
       </article>
       <aside class="content-supplementary" role="complementary">
-      <div class="bit">
-        <h3 class="title-3">${_("What can I do on this page?")}</h3>
-        <p>${_("You can create, edit, and delete group configurations.")}</p>
-
-        <p>${_("A group configuration defines how many groups of students are in an experiment. When you create an experiment, you select the group configuration to use.")}</p>
-
-        <p>${_("Click {em_start}New Group Configuration{em_end} to add a new configuration. To edit a configuration, hover over its box and click {em_start}Edit{em_end}.").format(em_start='<strong>', em_end="</strong>")}</p>
-
-        <p>${_("You can delete a group configuration only if it is not in use in an experiment. To delete a configuration, hover over its box and click the delete icon.")}</p>
-
-        <p><a href="${get_online_help_info(online_help_token())['doc_url']}" target="_blank">${_("Learn More")}</a></p>
-      </div>
+        <div class="bit">
+          <div class="content-groups-doc">
+              <h3 class="title-3">${_("Content Groups")}</h3>
+              <p>${_("Use content groups to give groups of students access to a specific set of course content. In addition to course content that is intended for all students, each content group sees content that you specifically designate as visible to it. By associating a content group with one or more cohorts, you can customize the content that a particular cohort or cohorts sees in your course.")}</p>
+              <p>${_("Click {em_start}New content group{em_end} to add a new content group. To edit the name of a content group, hover over its box and click {em_start}Edit{em_end}. Content groups cannot be deleted.").format(em_start="<strong>", em_end="</strong>")}</p>
+              <p><a href="${get_online_help_info(content_groups_help_token())['doc_url']}" target="_blank" class="button external-help-button">${_("Learn More")}</a></p>
+          </div>
+        </div>
+      % if should_show_experiment_groups:
+        <div class="bit">
+          <div class="experiment-groups-doc">
+            <h3 class="title-3">${_("Experiment Group Configurations")}</h3>
+            <p>${_("Use experiment group configurations to define how many groups of students are in a content experiment. When you create a content experiment for a course, you select the group configuration to use.")}</p>
+            <p>${_("Click {em_start}New Group Configuration{em_end} to add a new configuration. To edit a configuration, hover over its box and click {em_start}Edit{em_end}.").format(em_start="<strong>", em_end="</strong>")}</p>
+            <p>${_("You can delete a group configuration only if it is not in use in an experiment. To delete a configuration, hover over its box and click the delete icon.")}</p>
+            <p><a href="${get_online_help_info(experiment_group_configurations_help_token())['doc_url']}" target="_blank" class="button external-help-button">${_("Learn More")}</a></p>
+          </div>
+        </div>
+      % endif
       <div class="bit">
       % if context_course:
         <%

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

+<div class="collection-details">
+    <header class="collection-header">
+        <h3 class="title">
+            <%- name %>
+        </h3>
+    </header>
+
+    <ul class="actions">
+        <li class="action action-edit">
+            <button class="edit"><i class="icon fa fa-pencil"></i> <%= gettext("Edit") %></button>
+        </li>
+    </ul>
+</div>

cms/templates/js/content-group-editor.underscore

+<form class="collection-edit-form">
+    <% if (error && error.message) { %>
+        <div class="content-group-edit-error message message-status message-status error is-shown">
+          <%= gettext(error.message) %>
+        </div>
+    <% } %>
+    <div class="wrapper-form">
+        <fieldset class="collection-fields">
+            <div class="input-wrap field text required add-collection-name <% if(error && error.attributes && error.attributes.name) { print('error'); } %>">
+                <label for="group-cohort-name-<%= uniqueId %>"><%= gettext("Content Group Name") %></label>
+                <input name="group-cohort-name" id="group-cohort-name-<%= uniqueId %>" class="collection-name-input input-text" value="<%- name %>" type="text" placeholder="<%= gettext("This is the name of the group") %>">
+            </div>
+        </fieldset>
+    </div>
+    <div class="actions">
+        <button class="action action-primary" type="submit"><% if (isNew) { print(gettext("Create")) } else { print(gettext("Save")) } %></button>
+        <button class="action action-secondary action-cancel"><%= gettext("Cancel") %></button>
+    </div>
+</form>

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

-<div class="wrapper-group-configuration">
-    <header class="group-configuration-header">
-        <h3 class="group-configuration-title">
-            <a href="#" class="group-toggle <% if(showGroups){ print('hide'); } else { print('show'); } %>-groups">
+<div class="collection-details wrapper-group-configuration">
+    <header class="collection-header group-configuration-header">
+        <h3 class="title group-configuration-title">
+            <a href="#" class="toggle group-toggle <% if(showGroups){ print('hide'); } else { print('show'); } %>-groups">
                 <i class="ui-toggle-expansion icon fa fa-caret-<% if(showGroups){ print('down'); } else { print('right'); } %>"></i>
                 <%= name %>
             </a>
         </h3>
     </header>
 
-    <ol class="group-configuration-info group-configuration-info-<% if(showGroups){ print('block'); } else { print('inline'); } %>">
+    <ol class="collection-info group-configuration-info group-configuration-info-<% if(showGroups){ print('block'); } else { print('inline'); } %>">
       <% if (!_.isUndefined(id)) { %>
         <li class="group-configuration-id"
             ><span class="group-configuration-label"><%= gettext('ID') %>: </span
@@ -16,7 +16,7 @@
         ></li>
       <% } %>
       <% if (showGroups) { %>
-        <li class="group-configuration-description">
+        <li class="collection-description group-configuration-description">
             <%= description %>
         </li>
       <% } else { %>
@@ -31,18 +31,18 @@
 
     <% if(showGroups) { %>
       <% allocation = Math.floor(100 / groups.length) %>
-      <ol class="groups groups-<%= index %>">
+      <ol class="collection-items groups groups-<%= index %>">
         <% groups.each(function(group, groupIndex) { %>
-          <li class="group group-<%= groupIndex %>">
-            <span class="group-name"><%= group.get('name') %></span>
-            <span class="group-allocation"><%= allocation %>%</span>
+          <li class="item group group-<%= groupIndex %>">
+            <span class="name group-name"><%= group.get('name') %></span>
+            <span class="meta group-allocation"><%= allocation %>%</span>
           </li>
         <% }) %>
       </ol>
     <% } %>
     <ul class="actions group-configuration-actions">
         <li class="action action-edit">
-            <button class="edit"><%= gettext("Edit") %></button>
+            <button class="edit"><i class="icon fa fa-pencil"></i> <%= gettext("Edit") %></button>
         </li>
         <% if (_.isEmpty(usage)) { %>
             <li class="action action-delete wrapper-delete-button">
@@ -56,12 +56,12 @@
     </ul>
 </div>
 <% if(showGroups) { %>
-  <div class="wrapper-group-configuration-usages">
+  <div class="collection-references wrapper-group-configuration-usages">
     <% if (!_.isEmpty(usage)) { %>
-      <h4 class="group-configuration-usage-text"><%= gettext('This Group Configuration is used in:') %></h4>
-      <ol class="group-configuration-usage">
+      <h4 class="intro group-configuration-usage-text"><%= gettext('This Group Configuration is used in:') %></h4>
+      <ol class="usage group-configuration-usage">
         <% _.each(usage, function(unit) { %>
-          <li class="group-configuration-usage-unit">
+          <li class="usage-unit group-configuration-usage-unit">
             <p><a href=<%= unit.url %> ><%= unit.label %></a></p>
             <% if (unit.validation) { %>
               <p>
@@ -70,7 +70,7 @@
                 <% } else if (unit.validation.type === 'error') { %>
                   <i class="icon fa fa-exclamation-circle"></i>
                 <% } %>
-                <span class="group-configuration-validation-message">
+                <span class="usage-validation-message group-configuration-validation-message">
                   <%= unit.validation.text %>
                 </span>
               </p>

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

-<form class="group-configuration-edit-form">
-    <div class="wrapper-form">
+<form class="collection-edit-form group-configuration-edit-form">
     <% if (error && error.message) { %>
         <div class="group-configuration-edit-error message message-status message-status error is-shown" name="group-configuration-edit-error">
           <%= gettext(error.message) %>
         </div>
     <% } %>
-        <fieldset class="group-configuration-fields">
+    <div class="wrapper-form">
+        <fieldset class="collection-fields group-configuration-fields">
             <legend class="sr"><%= gettext("Group Configuration information") %></legend>
-            <div class="input-wrap field text required add-group-configuration-name <% if(error && error.attributes && error.attributes.name) { print('error'); } %>">
+            <div class="input-wrap field text required add-collection-name add-group-configuration-name <% if(error && error.attributes && error.attributes.name) { print('error'); } %>">
                 <label for="group-configuration-name-<%= uniqueId %>"><%= gettext("Group Configuration Name") %></label><%
                 if (!_.isUndefined(id)) {
                     %><span class="group-configuration-id">
@@ -16,7 +16,7 @@
                     </span><%
                 }
                 %>
-                <input id="group-configuration-name-<%= uniqueId %>" class="group-configuration-name-input input-text" name="group-configuration-name" type="text" placeholder="<%= gettext("This is the Name of the Group Configuration") %>" value="<%= name %>">
+                <input id="group-configuration-name-<%= uniqueId %>" class="collection-name-input input-text" name="group-configuration-name" type="text" placeholder="<%= gettext("This is the Name of the Group Configuration") %>" value="<%= name %>">
                 <span class="tip tip-stacked"><%= gettext("Name or short description of the configuration") %></span>
             </div>
             <div class="input-wrap field text add-group-configuration-description">
@@ -30,10 +30,10 @@
             <label class="groups-fields-label required"><%= gettext("Groups") %></label>
             <span class="tip tip-stacked"><%= gettext("Name of the groups that students will be assigned to, for example, Control, Video, Problems. You must have two or more groups.") %></span>
             <ol class="groups list-input enum"></ol>
-            <button class="action action-add-group"><i class="icon fa fa-plus"></i> <%= gettext("Add another group") %></button>
+            <button class="action action-add-group action-add-item"><i class="icon fa fa-plus"></i> <%= gettext("Add another group") %></button>
         </fieldset>
         <% if (!_.isEmpty(usage)) { %>
-            <div class="wrapper-group-configuration-validation">
+            <div class="wrapper-group-configuration-validation usage-validation">
                 <i class="icon fa fa-warning"></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.') %>

cms/templates/js/list.underscore

+<% if (length === 0) { %>
+    <div class="no-content">
+        <p>
+            <%- emptyMessage %>
+            <a href="#" class="button new-button"><i class="icon fa fa-plus"></i> <%= interpolate(gettext("Add your first %(item_type)s"), {item_type: itemCategoryDisplayName}, true) %></a>
+        </p>
+    </div>
+<% } else { %>
+    <div class="list-items"></div>
+    <% if (!isEditing) { %>
+        <button class="action action-add">
+            <i class="icon fa fa-plus"></i><%- interpolate(gettext('New %(item_type)s'), {item_type: itemCategoryDisplayName}, true) %>
+        </button>
+    <% } %>
+<% } %>

cms/templates/js/mock/mock-container-xblock.underscore

@@ -46,6 +46,9 @@
                                                         <li class="action-item action-edit">
                                                             <a href="#" class="edit-button action-button"></a>
                                                         </li>
+                                                        <li class="action-item action-visibility">
+                                                            <a href="#" class="visibility-button action-button"></a>
+                                                        </li>
                                                         <li class="action-item action-duplicate">
                                                             <a href="#" class="duplicate-button action-button"></a>
                                                         </li>
@@ -71,6 +74,9 @@
                                                         <li class="action-item action-edit">
                                                             <a href="#" class="edit-button action-button"></a>
                                                         </li>
+                                                        <li class="action-item action-visibility">
+                                                            <a href="#" class="visibility-button action-button"></a>
+                                                        </li>
                                                         <li class="action-item action-duplicate">
                                                             <a href="#" class="duplicate-button action-button"></a>
                                                         </li>
@@ -96,6 +102,9 @@
                                                         <li class="action-item action-edit">
                                                             <a href="#" class="edit-button action-button"></a>
                                                         </li>
+                                                        <li class="action-item action-visibility">
+                                                            <a href="#" class="visibility-button action-button"></a>
+                                                        </li>
                                                         <li class="action-item action-duplicate">
                                                             <a href="#" class="duplicate-button action-button"></a>
                                                         </li>
@@ -151,6 +160,9 @@
                                                         <li class="action-item action-edit">
                                                             <a href="#" class="edit-button action-button"></a>
                                                         </li>
+                                                        <li class="action-item action-visibility">
+                                                            <a href="#" class="visibility-button action-button"></a>
+                                                        </li>
                                                         <li class="action-item action-duplicate">
                                                             <a href="#" class="duplicate-button action-button"></a>
                                                         </li>
@@ -176,6 +188,9 @@
                                                         <li class="action-item action-edit">
                                                             <a href="#" class="edit-button action-button"></a>
                                                         </li>
+                                                        <li class="action-item action-visibility">
+                                                            <a href="#" class="visibility-button action-button"></a>
+                                                        </li>
                                                         <li class="action-item action-duplicate">
                                                             <a href="#" class="duplicate-button action-button"></a>
                                                         </li>
@@ -201,6 +216,9 @@
                                                         <li class="action-item action-edit">
                                                             <a href="#" class="edit-button action-button"></a>
                                                         </li>
+                                                        <li class="action-item action-visibility">
+                                                            <a href="#" class="visibility-button action-button"></a>
+                                                        </li>
                                                         <li class="action-item action-duplicate">
                                                             <a href="#" class="duplicate-button action-button"></a>
                                                         </li>

cms/templates/js/mock/mock-group-configuration-page.underscore

@@ -3,24 +3,23 @@
         <header class="mast has-actions has-subtitle">
             <h1 class="page-header">
                 <small class="subtitle">Settings</small>
-                <span class="sr">&gt; </span>Group Configurations
+                <span class="sr">&gt; </span>Group Configurations"
             </h1>
-            <nav class="nav-actions">
-                <h3 class="sr">Page Actions</h3>
-                <ul>
-                    <li class="nav-item">
-                        <a href="#" class="button new-button"><i class="icon fa fa-plus"></i> New Group Configuration</a>
-                    </li>
-                </ul>
-            </nav>
         </header>
     </div>
 
     <div class="wrapper-content wrapper">
         <section class="content">
             <article class="content-primary" role="main">
-                <div class="ui-loading">
-                    <p><span class="spin"><i class="icon fa fa-refresh"></i></span> <span class="copy">Loading</span></p>
+                <div class="wrapper-groups content-groups">
+                    <div class="ui-loading">
+                        <p><span class="spin"><i class="icon fa fa-refresh"></i></span> <span class="copy">Loading</span></p>
+                    </div>
+                </div>
+                <div class="wrapper-groups experiment-groups">
+                    <div class="ui-loading">
+                        <p><span class="spin"><i class="icon fa fa-refresh"></i></span> <span class="copy">Loading</span></p>
+                    </div>
                 </div>
             </article>
             <aside class="content-supplementary" role="complementary"></aside>

cms/templates/js/mock/mock-xblock-visibility-editor.underscore

+<div class="xblock xblock-visibility_view" data-runtime-version="1" data-usage-id="i4x:;_;_edX;_mock"
+     data-init="MockXBlock" data-runtime-class="StudioRuntime" tabindex="0">
+</div>

cms/templates/js/no-group-configurations.underscore

-<div class="no-group-configurations-content">
-    <p><%= gettext("You haven't created any group configurations yet.") %><a href="#" class="button new-button"><i class="icon fa fa-plus"></i><%= gettext("Add your first Group Configuration") %></a></p>
-</div>

cms/templates/js/publish-xblock.underscore

@@ -66,7 +66,7 @@ var visibleToStaffOnly = visibilityState === 'staff_only';
             <% } %>
         </h5>
         <% if (visibleToStaffOnly) { %>
-            <p class="copy">
+            <p class="visbility-copy copy">
                 <%= gettext("Staff Only") %>
                 <% if (!hasExplicitStaffLock) { %>
                     <span class="inherited-from">
@@ -75,18 +75,26 @@ var visibleToStaffOnly = visibilityState === 'staff_only';
                 <% } %>
             </p>
         <% } else { %>
-            <p class="copy"><%= gettext("Staff and Students") %></p>
+            <p class="visbility-copy copy"><%= gettext("Staff and Students") %></p>
         <% } %>
-        <p class="action-inline">
-            <a href="" class="action-staff-lock" role="button" aria-pressed="<%= hasExplicitStaffLock %>">
+            <% if (hasContentGroupComponents) { %>
+                <p class="note-visibility">
+                    <i class="icon fa fa-eye" aria-hidden="true"></i>
+                    <span class="note-copy"><%= gettext("Some content in this unit is visible only to particular content groups") %></span>
+                </p>
+            <% } %>
+        <ul class="actions-inline">
+            <li class="action-inline">
+                <a href="" class="action-staff-lock" role="button" aria-pressed="<%= hasExplicitStaffLock %>">
                 <% if (hasExplicitStaffLock) { %>
-                    <i class="icon fa fa-check-square-o"></i>
+                    <i class="icon fa fa-check-square-o" aria-hidden="true"></i>
                 <% } else { %>
-                    <i class="icon fa fa-square-o"></i>
+                    <i class="icon fa fa-square-o" aria-hidden="true"></i>
                 <% } %>
                 <%= gettext('Hide from students') %>
             </a>
-        </p>
+            </li>
+        </ul>
     </div>
 
     <div class="wrapper-pub-actions bar-mod-actions">

cms/templates/settings.html

@@ -7,7 +7,6 @@
 <%!
   from django.utils.translation import ugettext as _
   from contentstore import utils
-  from contentstore.views.course import should_show_group_configurations_page
   import urllib
 %>
 
@@ -333,9 +332,7 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url}';
           <ul>
             <li class="nav-item"><a href="${grading_config_url}">${_("Grading")}</a></li>
             <li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li>
-            % if should_show_group_configurations_page(context_course):
-              <li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
-            % endif
+            <li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
             <li class="nav-item"><a href="${advanced_config_url}">${_("Advanced Settings")}</a></li>
           </ul>
         </nav>

cms/templates/settings_advanced.html

@@ -4,7 +4,6 @@
 <%!
   from django.utils.translation import ugettext as _
   from contentstore import utils
-  from contentstore.views.course import should_show_group_configurations_page
   from django.utils.html import escapejs
 %>
 <%block name="title">${_("Advanced Settings")}</%block>
@@ -92,9 +91,7 @@
             <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 should_show_group_configurations_page(context_course):
-              <li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
-            % endif
+            <li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
           </ul>
         </nav>
       % endif

cms/templates/settings_graders.html

@@ -6,7 +6,6 @@
 <%namespace name='static' file='static_content.html'/>
 <%!
   from contentstore import utils
-  from contentstore.views.course import should_show_group_configurations_page
   from django.utils.translation import ugettext as _
 %>
 
@@ -135,9 +134,7 @@
           <ul>
             <li class="nav-item"><a href="${detailed_settings_url}">${_("Details &amp; Schedule")}</a></li>
             <li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li>
-            % if should_show_group_configurations_page(context_course):
-              <li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
-            % endif
+            <li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
             <li class="nav-item"><a href="${advanced_settings_url}">${_("Advanced Settings")}</a></li>
           </ul>
         </nav>

cms/templates/studio_xblock_wrapper.html

 <%!
 from django.utils.translation import ugettext as _
 from contentstore.views.helpers import xblock_studio_url
+from contentstore.utils import is_visible_to_specific_content_groups
 import json
 %>
 <%
@@ -38,7 +39,11 @@ messages = json.dumps(xblock.validate().to_json())
     <div class="studio-xblock-wrapper" data-locator="${xblock.location | h}" data-course-key="${xblock.location.course_key | h}">
     % endif
 
-    <section class="wrapper-xblock ${section_class} ${collapsible_class}">
+    <section class="wrapper-xblock ${section_class} ${collapsible_class}
+    % if is_visible_to_specific_content_groups(xblock):
+        has-group-visibility-set
+    % endif
+    ">
 % endif
 
 <header class="xblock-header xblock-header-${xblock.category}">
@@ -63,6 +68,14 @@ messages = json.dumps(xblock.validate().to_json())
                                     <span class="action-button-text">${_("Edit")}</span>
                                 </a>
                             </li>
+                            % if can_edit_visibility:
+                                <li class="action-item action-visibility">
+                                    <a href="#" data-tooltip="${_("Visibility Settings")}" class="visibility-button action-button">
+                                        <i class="icon fa fa-eye" aria-hidden="true"></i>
+                                        <span class="sr">${_("Visibility")}</span>
+                                    </a>
+                                </li>
+                            % endif
                             <li class="action-item action-duplicate">
                                 <a href="#" data-tooltip="${_("Duplicate")}" class="duplicate-button action-button">
                                 <i class="icon fa fa-copy"></i>

cms/templates/ux/reference/modal_access-component.html

+<div class="wrapper wrapper-modal-window wrapper-modal-window-edit-xblock" aria-describedby="modal-window-description" aria-labelledby="modal-window-title" aria-hidden="" role="dialog">
+  <div class="modal-window-overlay"></div>
+  <div class="modal-window confirm modal-med modal-type-html modal-editor" style="top: 50px; left: 400px;">
+    <div class="edit-xblock-modal">
+      <div class="modal-header">
+        <h2 class="title modal-window-title">Editing visibility for: [Component Name]</h2>
+      </div>
+
+      <div class="modal-content">
+        <div class="xblock-editor" data-locator="i4x://TestU/cohorts001/chapter/748152225449412a846bc24811a5621c" data-course-key="">
+
+          <div class="xblock xblock-visibility_view">
+
+            <div class="modal-section visibility-summary">
+              <div class="summary-message summary-message-warning visibility-summary-message">
+                <i class="icon fa fa-exclamation-triangle" aria-hidden="true"></i>
+                <p class="copy"><span class="sr">Warning: </span>This component is contained in a unit that is hidden from students. Component visibility settings are overridden by the unit visibility settings.</p>
+              </div>
+
+              <!-- NOTE: use when no group configuration has been set -->
+              <div class="is-not-configured has-actions">
+                <h4 class="title">You have not set up any groups</h4>
+
+                <div class="copy">
+                  <p>Groups are a way for you to organize content in your course with a particular student experience in mind. They are commonly used to facilitate content and pedagogical experiments as well as to provide different tracks of content.</p>
+                </div>
+
+                <div class="actions">
+                  <a href="" class="action action-primary action-settings">Manage groups in this course</a>
+                </div>
+              </div>
+            </div>
+
+            <form class="visibility-controls-form" method="post" action="">
+
+              <div class="modal-section visibility-controls">
+                <h3 class="modal-section-title">Set visibility to:</h3>
+
+                <div class="modal-section-content">
+
+                  <section class="visibility-controls-primary">
+                    <ul class="list-fields list-radio">
+                      <li class="field field-radio field-visibility-level">
+                        <input type="radio" id="visibility-level-all" name="visibility-level" value="" class="input input-radio visibility-level-all"  />
+                        <label for="visibility-level-all" class="label">All Students and Staff</label>
+                      </li>
+
+                      <li class="field field-radio field-visibility-level">
+                        <input type="radio" id="visibility-level-specific" name="visibility-level" value="" class="input input-radio visibility-level-specific" checked="checked" />
+                        <label for="visibility-level-specific" class="label">Specific Groups</label>
+                      </li>
+                    </ul>
+                  </section>
+
+                  <!-- NOTE: @andyarmstrong, if you need this wrapper to show and hide, great. If not, please remove it from the DOM -->
+                  <div class="wrapper-visibility-specific">
+
+                    <section class="visibility-controls-secondary">
+                      <div class="visibility-controls-group">
+                        <h4 class="visibility-controls-title modal-subsection-title sr">Content Groups</h4>
+                        <ul class="list-fields list-checkbox">
+                          <li class="field field-checkbox field-visibility-content-group">
+                            <input type="checkbox" id="visibility-content-group-NAME1" name="visibility-content-group" value="" class="input input-checkbox visibility-content-group-NAME1" />
+                            <label for="visibility-content-group-NAME1" class="label">Content Group NAME 1</label>
+                          </li>
+
+                          <li class="field field-checkbox field-visibility-content-group">
+                            <input type="checkbox" id="visibility-content-group-NAME2" name="visibility-content-group" value="" class="input input-checkbox visibility-content-group-NAME2" />
+                            <label for="visibility-content-group-NAME2" class="label">Content Group NAME 2</label>
+                          </li>
+
+                          <li class="field field-checkbox field-visibility-content-group">
+                            <input type="checkbox" id="visibility-content-group-NAME3" name="visibility-content-group" value="" class="input input-checkbox visibility-content-group-NAME3" />
+                            <label for="visibility-content-group-NAME3" class="label">Content Group NAME 3</label>
+                          </li>
+
+                          <li class="field field-checkbox field-visibility-content-group">
+                            <input type="checkbox" id="visibility-content-group-NAME4" name="visibility-content-group" value="" class="input input-checkbox visibility-content-group-NAME4" />
+                            <label for="visibility-content-group-NAME4" class="label">Content Group NAME 4</label>
+                          </li>
+
+                          <!-- NOTE: @andyarmstrong, here's an example of how we would treat a group that was deleted/removed - we need a .was removed class and an additional UI element called a .note -->
+                          <li class="field field-checkbox field-visibility-content-group was-removed">
+                            <input type="checkbox" id="visibility-content-group-deleted1" name="visibility-content-group" value="" class="input input-checkbox visibility-content-group-deleted" checked="checked" />
+                            <label for="visibility-content-group-deleted1" class="label">
+                              Deleted Content Group
+                            </label>
+                            <span class="note">The selected group no longer exists. Choose another group or make the component visible to All Students and Staff</span>
+                          </li>
+
+                          <!-- NOTE: @andyarmstrong, here's an example of how we would treat a group that was deleted/removed - we need a .was removed class and an additional UI element called a .note -->
+                          <li class="field field-checkbox field-visibility-content-group was-removed">
+                            <input type="checkbox" id="visibility-content-group-deleted1" name="visibility-content-group" value="" class="input input-checkbox visibility-content-group-deleted" checked="checked" />
+                            <label for="visibility-content-group-deleted1" class="label">
+                              Deleted Content Group
+                            </label>
+                            <span class="note">The selected group no longer exists. Choose another group or make the component visible to All Students and Staff</span>
+                          </li>
+                        </ul>
+                      </div>
+                    </section>
+                  </div>
+                </div>
+              </div>
+
+            </form>
+
+          </div><!-- .xblock -->
+        </div><!-- .xblock-editor -->
+      </div><!-- .modal-content -->
+
+      <div class="modal-actions">
+        <h3 class="sr">Actions</h3>
+        <ul>
+          <li class="action-item">
+            <a href="#" class="button action-primary action-save">Save</a>
+          </li>
+
+          <li class="action-item">
+            <a href="#" class="button  action-cancel">Cancel</a>
+          </li>
+        </ul>
+      </div>
+    </div><!-- .xblock-visibility-modal -->
+  </div><!-- .modal-window -->
+</div><!-- .wrapper-modal-window -->

cms/templates/visibility_editor.html

+<%
+from django.utils.translation import ugettext as _
+
+from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_user_partition
+from contentstore.utils import ancestor_has_staff_lock
+
+cohorted_user_partition = get_cohorted_user_partition(xblock.location.course_key)
+unsorted_groups = cohorted_user_partition.groups if cohorted_user_partition else []
+groups = sorted(unsorted_groups, key=lambda group: group.name)
+selected_group_ids = xblock.group_access.get(cohorted_user_partition.id, []) if cohorted_user_partition else []
+has_selected_groups = len(selected_group_ids) > 0
+is_staff_locked = ancestor_has_staff_lock(xblock)
+%>
+
+<div class="modal-section visibility-summary">
+    % if len(groups) == 0:
+        <div class="is-not-configured has-actions">
+            <h4 class="title">${_('No content groups exist')}</h4>
+
+            <div class="copy">
+                <p>${_('Use content groups to give groups of students access to a specific set of course content. Create one or more content groups, and make specific components visible to them.')}</p>
+            </div>
+
+            <div class="actions">
+                <a href="${manage_groups_url}" class="action action-primary action-settings">${_('Manage content groups')}</a>
+            </div>
+        </div>
+    % elif is_staff_locked:
+        <div class="summary-message summary-message-warning visibility-summary-message">
+            <i class="icon fa fa-exclamation-triangle" aria-hidden="true"></i>
+            <p class="copy">
+                ## Translators: Any text between {screen_reader_start} and {screen_reader_end} is only read by screen readers and never shown in the browser.
+                ${_(
+                    "{screen_reader_start}Warning:{screen_reader_end} The Unit this component is contained in is hidden from students. Visibility settings here will be trumped by this."
+                    ).format(
+                        screen_reader_start='<span class="sr">',
+                        screen_reader_end='</span>',
+                    )
+                }
+            </p>
+        </div>
+    % endif
+</div>
+
+% if len(groups) > 0:
+    <form class="visibility-controls-form" method="post" action="">
+
+        <div class="modal-section visibility-controls">
+            <h3 class="modal-section-title">${_('Make visible to:')}</h3>
+
+            <div class="modal-section-content">
+
+                <section class="visibility-controls-primary">
+                    <ul class="list-fields list-radio">
+                        <li class="field field-radio field-visibility-level">
+                            <input type="radio" id="visibility-level-all" name="visibility-level" value="" class="input input-radio visibility-level-all" ${'checked="checked"' if not has_selected_groups else ''} />
+                            <label for="visibility-level-all" class="label">${_('All Students and Staff')}</label>
+                        </li>
+
+                        <li class="field field-radio field-visibility-level">
+                            <input type="radio" id="visibility-level-specific" name="visibility-level" value="" class="input input-radio visibility-level-specific"  ${'checked="checked"' if has_selected_groups else ''} />
+                            <label for="visibility-level-specific" class="label">${_('Specific Content Groups')}</label>
+                        </li>
+                    </ul>
+                </section>
+
+                <div class="wrapper-visibility-specific" data-user-partition-id="${cohorted_user_partition.id}">
+                    <section class="visibility-controls-secondary">
+                        <div class="visibility-controls-group">
+                            <h4 class="visibility-controls-title modal-subsection-title sr">${_('Content Groups')}</h4>
+                            <ul class="list-fields list-checkbox">
+                                <%
+                                missing_group_ids = set(selected_group_ids)
+                                %>
+                                % for group in groups:
+                                    <%
+                                    is_group_selected = group.id in selected_group_ids
+                                    if is_group_selected:
+                                        missing_group_ids.remove(group.id)
+                                    %>
+                                    <li class="field field-checkbox field-visibility-content-group">
+                                        <input type="checkbox" id="visibility-content-group-${group.id}" name="visibility-content-group" value="${group.id}" class="input input-checkbox"  ${'checked="checked"' if group.id in selected_group_ids else ''}/>
+                                        <label for="visibility-content-group-${group.id}" class="label">${group.name | h}</label>
+                                    </li>
+                                % endfor
+
+                                % for group_id in missing_group_ids:
+                                    <li class="field field-checkbox field-visibility-content-group was-removed">
+                                        <input type="checkbox" id="visibility-content-group-${group_id}" name="visibility-content-group" value="${group_id}" class="input input-checkbox" checked="checked" />
+                                        <label for="visibility-content-group-${group_id}" class="label">
+                                            ${_('Deleted Content Group')}
+                                            <span class="note">${_('Content group no longer exists. Please choose another or allow access to All Students and staff')}</span>
+                                        </label>
+                                    </li>
+                                % endfor
+                            </ul>
+                        </div>
+                    </section>
+                </div>
+            </div>
+        </div>
+    </form>
+% endif

cms/templates/widgets/header.html

@@ -3,7 +3,6 @@
   from django.core.urlresolvers import reverse
   from django.utils.translation import ugettext as _
   from contentstore.context_processors import doc_url
-  from contentstore.views.course import should_show_group_configurations_page
 %>
 <%page args="online_help_token"/>
 
@@ -93,11 +92,9 @@
                   <li class="nav-item nav-course-settings-team">
                     <a href="${course_team_url}">${_("Course Team")}</a>
                   </li>
-                  % if should_show_group_configurations_page(context_course):
-                    <li class="nav-item nav-course-settings-group-configurations">
-                      <a href="${reverse('contentstore.views.group_configurations_list_handler', kwargs={'course_key_string': unicode(course_key)})}">${_("Group Configurations")}</a>
-                    </li>
-                  % endif
+                  <li class="nav-item nav-course-settings-group-configurations">
+                    <a href="${reverse('contentstore.views.group_configurations_list_handler', kwargs={'course_key_string': unicode(course_key)})}">${_("Group Configurations")}</a>
+                  </li>
                   <li class="nav-item nav-course-settings-advanced">
                     <a href="${advanced_settings_url}">${_("Advanced Settings")}</a>
                   </li>

cms/urls.py

@@ -91,6 +91,7 @@ urlpatterns += patterns(
     url(r'^import_status/{}/(?P<filename>.+)$'.format(settings.COURSE_KEY_PATTERN), 'import_status_handler'),
     url(r'^export/{}$'.format(settings.COURSE_KEY_PATTERN), 'export_handler'),
     url(r'^xblock/outline/{}$'.format(settings.USAGE_KEY_PATTERN), 'xblock_outline_handler'),
+    url(r'^xblock/container/{}$'.format(settings.USAGE_KEY_PATTERN), 'xblock_container_handler'),
     url(r'^xblock/{}/(?P<view_name>[^/]+)$'.format(settings.USAGE_KEY_PATTERN), 'xblock_view_handler'),
     url(r'^xblock/{}?$'.format(settings.USAGE_KEY_PATTERN), 'xblock_handler'),
     url(r'^tabs/{}$'.format(settings.COURSE_KEY_PATTERN), 'tabs_handler'),

common/djangoapps/util/db.py

@@ -2,10 +2,14 @@
 Utility functions related to databases.
 """
 from functools import wraps
+import random
 
 from django.db import connection, transaction
 
 
+MYSQL_MAX_INT = (2 ** 31) - 1
+
+
 def commit_on_success_with_read_committed(func):  # pylint: disable=invalid-name
     """
     Decorator which executes the decorated function inside a transaction with isolation level set to READ COMMITTED.
@@ -38,3 +42,18 @@ def commit_on_success_with_read_committed(func):  # pylint: disable=invalid-name
             return func(*args, **kwargs)
 
     return wrapper
+
+
+def generate_int_id(minimum=0, maximum=MYSQL_MAX_INT, used_ids=None):
+    """
+    Return a unique integer in the range [minimum, maximum], inclusive.
+    """
+    if used_ids is None:
+        used_ids = []
+
+    cid = random.randint(minimum, maximum)
+
+    while cid in used_ids:
+        cid = random.randint(minimum, maximum)
+
+    return cid

common/djangoapps/util/tests/test_db.py

@@ -8,9 +8,9 @@ import unittest
 from django.contrib.auth.models import User
 from django.db import connection, IntegrityError
 from django.db.transaction import commit_on_success, TransactionManagementError
-from django.test import TransactionTestCase
+from django.test import TestCase, TransactionTestCase
 
-from util.db import commit_on_success_with_read_committed
+from util.db import commit_on_success_with_read_committed, generate_int_id
 
 
 @ddt.ddt
@@ -99,3 +99,31 @@ class TransactionIsolationLevelsTestCase(TransactionTestCase):
             with commit_on_success():
                 with commit_on_success():
                     commit_on_success_with_read_committed(do_nothing)()
+
+
+@ddt.ddt
+class GenerateIntIdTestCase(TestCase):
+    """Tests for `generate_int_id`"""
+    @ddt.data(10)
+    def test_no_used_ids(self, times):
+        """
+        Verify that we get a random integer within the specified range
+        when there are no used ids.
+        """
+        minimum = 1
+        maximum = times
+        for i in range(times):
+            self.assertIn(generate_int_id(minimum, maximum), range(minimum, maximum + 1))
+
+    @ddt.data(10)
+    def test_used_ids(self, times):
+        """
+        Verify that we get a random integer within the specified range
+        but not in a list of used ids.
+        """
+        minimum = 1
+        maximum = times
+        used_ids = {2, 4, 6, 8}
+        for i in range(times):
+            int_id = generate_int_id(minimum, maximum, used_ids)
+            self.assertIn(int_id, list(set(range(minimum, maximum + 1)) - used_ids))

common/lib/xmodule/xmodule/modulestore/inheritance.py

@@ -57,15 +57,6 @@ class InheritanceMixin(XBlockMixin):
         default=False,
         scope=Scope.settings,
     )
-    group_access = Dict(
-        help="A dictionary that maps which groups can be shown this block. The keys "
-             "are group configuration ids and the values are a list of group IDs. "
-             "If there is no key for a group configuration or if the list of group IDs "
-             "is empty then the block is considered visible to all. Note that this "
-             "field is ignored if the block is visible_to_staff_only.",
-        default={},
-        scope=Scope.settings,
-    )
     course_edit_method = String(
         display_name=_("Course Editor"),
         help=_("Enter the method by which this course is edited (\"XML\" or \"Studio\")."),

common/lib/xmodule/xmodule/modulestore/modulestore_settings.py

@@ -98,6 +98,7 @@ def update_module_store_settings(
         module_store_options=None,
         xml_store_options=None,
         default_store=None,
+        mappings=None,
 ):
     """
     Updates the settings for each store defined in the given module_store_setting settings
@@ -123,6 +124,9 @@ def update_module_store_settings(
                 return
         raise Exception("Could not find setting for requested default store: {}".format(default_store))
 
+    if mappings and 'mappings' in module_store_setting['default']['OPTIONS']:
+        module_store_setting['default']['OPTIONS']['mappings'] = mappings
+
 
 def get_mixed_stores(mixed_setting):
     """

common/lib/xmodule/xmodule/modulestore/mongo/draft.py

@@ -643,6 +643,9 @@ class DraftModuleStore(MongoModuleStore):
 
         Raises:
             ItemNotFoundError: if any of the draft subtree nodes aren't found
+
+        Returns:
+            The newly published xblock
         """
         # NOTE: cannot easily use self._breadth_first b/c need to get pub'd and draft as pairs
         # (could do it by having 2 breadth first scans, the first to just get all published children

common/lib/xmodule/xmodule/modulestore/tests/factories.py

@@ -210,26 +210,18 @@ class ItemFactory(XModuleFactory):
             # replace the display name with an optional parameter passed in from the caller
             if display_name is not None:
                 metadata['display_name'] = display_name
-            runtime = parent.runtime if parent else None
-            store.create_item(
+
+            module = store.create_child(
                 user_id,
-                location.course_key,
+                parent.location,
                 location.block_type,
                 block_id=location.block_id,
                 metadata=metadata,
                 definition_data=data,
-                runtime=runtime
+                runtime=parent.runtime,
+                fields=kwargs,
             )
 
-            module = store.get_item(location)
-
-            for attr, val in kwargs.items():
-                setattr(module, attr, val)
-            # Save the attributes we just set
-            module.save()
-
-            store.update_item(module, user_id)
-
             # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so
             # if we add one then we need to also add it to the policy information (i.e. metadata)
             # we should remove this once we can break this reference from the course to static tabs
@@ -248,12 +240,15 @@ class ItemFactory(XModuleFactory):
                 parent.children.append(location)
                 store.update_item(parent, user_id)
                 if publish_item:
-                    store.publish(parent.location, user_id)
+                    published_parent = store.publish(parent.location, user_id)
+                    # module is last child of parent
+                    return published_parent.get_children()[-1]
+                else:
+                    return store.get_item(location)
             elif publish_item:
-                store.publish(location, user_id)
-
-        # return the published item
-        return store.get_item(location)
+                return store.publish(location, user_id)
+            else:
+                return module
 
 
 @contextmanager

common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py

@@ -5,7 +5,6 @@ Unit tests for the Mixed Modulestore, with DDT for the various stores (Split, Dr
 from collections import namedtuple
 import datetime
 import ddt
-from importlib import import_module
 import itertools
 import mimetypes
 from uuid import uuid4
@@ -33,7 +32,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
 from xmodule.exceptions import InvalidVersionError
 from xmodule.modulestore import ModuleStoreEnum
-from xmodule.modulestore.draft_and_published import UnsupportedRevisionError, ModuleStoreDraftAndPublished
+from xmodule.modulestore.draft_and_published import UnsupportedRevisionError
 from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError, ReferentialIntegrityError, NoPathToItem
 from xmodule.modulestore.mixed import MixedModuleStore
 from xmodule.modulestore.search import path_to_location
@@ -358,12 +357,12 @@ class TestMixedModuleStore(CourseComparisonTest):
             self.store.has_item(self.fake_location, revision=ModuleStoreEnum.RevisionOption.draft_preferred)
 
     # draft queries:
-    #   problem: find draft item, find all items pertinent to inheritance computation
+    #   problem: find draft item, find all items pertinent to inheritance computation, find parent
     #   non-existent problem: find draft, find published
     # split:
     #   problem: active_versions, structure
     #   non-existent problem: ditto
-    @ddt.data(('draft', [2, 2], 0), ('split', [2, 2], 0))
+    @ddt.data(('draft', [3, 2], 0), ('split', [2, 2], 0))
     @ddt.unpack
     def test_get_item(self, default_ms, max_find, max_send):
         self.initdb(default_ms)
@@ -388,10 +387,10 @@ class TestMixedModuleStore(CourseComparisonTest):
             self.store.get_item(self.fake_location, revision=ModuleStoreEnum.RevisionOption.draft_preferred)
 
     # Draft:
-    #    wildcard query, 6! load pertinent items for inheritance calls, course root fetch (why)
+    #    wildcard query, 6! load pertinent items for inheritance calls, load parents, course root fetch (why)
     # Split:
     #    active_versions (with regex), structure, and spurious active_versions refetch
-    @ddt.data(('draft', 8, 0), ('split', 3, 0))
+    @ddt.data(('draft', 14, 0), ('split', 3, 0))
     @ddt.unpack
     def test_get_items(self, default_ms, max_find, max_send):
         self.initdb(default_ms)
@@ -405,7 +404,6 @@ class TestMixedModuleStore(CourseComparisonTest):
 
         course_locn = self.course_locations[self.MONGO_COURSEID]
         with check_mongo_calls(max_find, max_send):
-            # NOTE: use get_course if you just want the course. get_items is expensive
             modules = self.store.get_items(course_locn.course_key, qualifiers={'category': 'problem'})
         self.assertEqual(len(modules), 6)
 
@@ -416,12 +414,11 @@ class TestMixedModuleStore(CourseComparisonTest):
                 revision=ModuleStoreEnum.RevisionOption.draft_preferred
             )
 
-    # draft: get draft, count parents, get parents, count & get grandparents, count & get greatgrand,
-    #        count & get next ancestor (chapter's parent), count non-existent next ancestor, get inheritance
+    # draft: get draft, get ancestors up to course (2-6), compute inheritance
     #    sends: update problem and then each ancestor up to course (edit info)
     # split: active_versions, definitions (calculator field), structures
     #  2 sends to update index & structure (note, it would also be definition if a content field changed)
-    @ddt.data(('draft', 11, 5), ('split', 3, 2))
+    @ddt.data(('draft', 7, 5), ('split', 3, 2))
     @ddt.unpack
     def test_update_item(self, default_ms, max_find, max_send):
         """
@@ -886,9 +883,9 @@ class TestMixedModuleStore(CourseComparisonTest):
 
     # notice this doesn't test getting a public item via draft_preferred which draft would have 2 hits (split
     # still only 2)
-    # Draft: count via definition.children query, then fetch via that query
+    # Draft: get_parent
     # Split: active_versions, structure
-    @ddt.data(('draft', 2, 0), ('split', 2, 0))
+    @ddt.data(('draft', 1, 0), ('split', 2, 0))
     @ddt.unpack
     def test_get_parent_locations(self, default_ms, max_find, max_send):
         """
@@ -922,35 +919,47 @@ class TestMixedModuleStore(CourseComparisonTest):
         # publish the course
         self.course = self.store.publish(self.course.location, self.user_id)
 
-        # make drafts of verticals
-        self.store.convert_to_draft(self.vertical_x1a, self.user_id)
-        self.store.convert_to_draft(self.vertical_y1a, self.user_id)
-
-        # move child problem_x1a_1 to vertical_y1a
-        child_to_move_location = self.problem_x1a_1
-        new_parent_location = self.vertical_y1a
-        old_parent_location = self.vertical_x1a
-
-        old_parent = self.store.get_item(old_parent_location)
-        old_parent.children.remove(child_to_move_location.replace(version_guid=old_parent.location.version_guid))
-        self.store.update_item(old_parent, self.user_id)
-
-        new_parent = self.store.get_item(new_parent_location)
-        new_parent.children.append(child_to_move_location.replace(version_guid=new_parent.location.version_guid))
-        self.store.update_item(new_parent, self.user_id)
-
-        self.verify_get_parent_locations_results([
-            (child_to_move_location, new_parent_location, None),
-            (child_to_move_location, new_parent_location, ModuleStoreEnum.RevisionOption.draft_preferred),
-            (child_to_move_location, old_parent_location.for_branch(ModuleStoreEnum.BranchName.published), ModuleStoreEnum.RevisionOption.published_only),
-        ])
+        with self.store.bulk_operations(self.course.id):
+            # make drafts of verticals
+            self.store.convert_to_draft(self.vertical_x1a, self.user_id)
+            self.store.convert_to_draft(self.vertical_y1a, self.user_id)
+
+            # move child problem_x1a_1 to vertical_y1a
+            child_to_move_location = self.problem_x1a_1
+            new_parent_location = self.vertical_y1a
+            old_parent_location = self.vertical_x1a
+
+            with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred):
+                old_parent = self.store.get_item(child_to_move_location).get_parent()
+
+            self.assertEqual(old_parent_location, old_parent.location)
+
+            child_to_move_contextualized = child_to_move_location.map_into_course(old_parent.location.course_key)
+            old_parent.children.remove(child_to_move_contextualized)
+            self.store.update_item(old_parent, self.user_id)
+
+            new_parent = self.store.get_item(new_parent_location)
+            new_parent.children.append(child_to_move_location)
+            self.store.update_item(new_parent, self.user_id)
+
+            with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred):
+                self.assertEqual(new_parent_location, self.store.get_item(child_to_move_location).get_parent().location)
+            with self.store.branch_setting(ModuleStoreEnum.Branch.published_only):
+                self.assertEqual(old_parent_location, self.store.get_item(child_to_move_location).get_parent().location)
+            old_parent_published_location = old_parent_location.for_branch(ModuleStoreEnum.BranchName.published)
+            self.verify_get_parent_locations_results([
+                (child_to_move_location, new_parent_location, None),
+                (child_to_move_location, new_parent_location, ModuleStoreEnum.RevisionOption.draft_preferred),
+                (child_to_move_location, old_parent_published_location, ModuleStoreEnum.RevisionOption.published_only),
+            ])
 
         # publish the course again
         self.store.publish(self.course.location, self.user_id)
+        new_parent_published_location = new_parent_location.for_branch(ModuleStoreEnum.BranchName.published)
         self.verify_get_parent_locations_results([
             (child_to_move_location, new_parent_location, None),
             (child_to_move_location, new_parent_location, ModuleStoreEnum.RevisionOption.draft_preferred),
-            (child_to_move_location, new_parent_location.for_branch(ModuleStoreEnum.BranchName.published), ModuleStoreEnum.RevisionOption.published_only),
+            (child_to_move_location, new_parent_published_location, ModuleStoreEnum.RevisionOption.published_only),
         ])
 
     @ddt.data('draft')
@@ -1022,20 +1031,12 @@ class TestMixedModuleStore(CourseComparisonTest):
     # Draft:
     #   Problem path:
     #    1. Get problem
-    #    2-3. count matches definition.children called 2x?
-    #    4. get parent via definition.children query
-    #    5-7. 2 counts and 1 get grandparent via definition.children
-    #    8-10. ditto for great-grandparent
-    #    11-13. ditto for next ancestor
-    #    14. fail count query looking for parent of course (unnecessary)
-    #    15. get course record direct query (not via definition.children) (already fetched in 13)
-    #    16. get items for inheritance computation
-    #    17. get vertical (parent of problem)
-    #    18. get items for inheritance computation (why? caching should handle)
-    #    19-20. get vertical_x1b (? why? this is the only ref in trace) & items for inheritance computation
-    #   Chapter path: get chapter, count parents 2x, get parents, count non-existent grandparents
+    #    2-6. get parent and rest of ancestors up to course
+    #    7-8. get sequential, compute inheritance
+    #    8-9. get vertical, compute inheritance
+    #    10-11. get other vertical_x1b (why?) and compute inheritance
     # Split: active_versions & structure
-    @ddt.data(('draft', [20, 5], 0), ('split', [2, 2], 0))
+    @ddt.data(('draft', [12, 3], 0), ('split', [2, 2], 0))
     @ddt.unpack
     def test_path_to_location(self, default_ms, num_finds, num_sends):
         """

common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py

@@ -717,15 +717,16 @@ class TestMongoKeyValueStore(object):
     def setUp(self):
         self.data = {'foo': 'foo_value'}
         self.course_id = SlashSeparatedCourseKey('org', 'course', 'run')
+        self.parent = self.course_id.make_usage_key('parent', 'p')
         self.children = [self.course_id.make_usage_key('child', 'a'), self.course_id.make_usage_key('child', 'b')]
         self.metadata = {'meta': 'meta_val'}
-        self.kvs = MongoKeyValueStore(self.data, self.children, self.metadata)
+        self.kvs = MongoKeyValueStore(self.data, self.parent, self.children, self.metadata)
 
     def test_read(self):
         assert_equals(self.data['foo'], self.kvs.get(KeyValueStore.Key(Scope.content, None, None, 'foo')))
+        assert_equals(self.parent, self.kvs.get(KeyValueStore.Key(Scope.parent, None, None, 'parent')))
         assert_equals(self.children, self.kvs.get(KeyValueStore.Key(Scope.children, None, None, 'children')))
         assert_equals(self.metadata['meta'], self.kvs.get(KeyValueStore.Key(Scope.settings, None, None, 'meta')))
-        assert_equals(None, self.kvs.get(KeyValueStore.Key(Scope.parent, None, None, 'parent')))
 
     def test_read_invalid_scope(self):
         for scope in (Scope.preferences, Scope.user_info, Scope.user_state):
@@ -735,7 +736,7 @@ class TestMongoKeyValueStore(object):
             assert_false(self.kvs.has(key))
 
     def test_read_non_dict_data(self):
-        self.kvs = MongoKeyValueStore('xml_data', self.children, self.metadata)
+        self.kvs = MongoKeyValueStore('xml_data', self.parent, self.children, self.metadata)
         assert_equals('xml_data', self.kvs.get(KeyValueStore.Key(Scope.content, None, None, 'data')))
 
     def _check_write(self, key, value):
@@ -746,9 +747,10 @@ class TestMongoKeyValueStore(object):
         yield (self._check_write, KeyValueStore.Key(Scope.content, None, None, 'foo'), 'new_data')
         yield (self._check_write, KeyValueStore.Key(Scope.children, None, None, 'children'), [])
         yield (self._check_write, KeyValueStore.Key(Scope.settings, None, None, 'meta'), 'new_settings')
+        # write Scope.parent raises InvalidScope, which is covered in test_write_invalid_scope
 
     def test_write_non_dict_data(self):
-        self.kvs = MongoKeyValueStore('xml_data', self.children, self.metadata)
+        self.kvs = MongoKeyValueStore('xml_data', self.parent, self.children, self.metadata)
         self._check_write(KeyValueStore.Key(Scope.content, None, None, 'data'), 'new_data')
 
     def test_write_invalid_scope(self):

common/lib/xmodule/xmodule/modulestore/tests/test_publish.py

@@ -47,14 +47,10 @@ class TestPublish(SplitWMongoCourseBoostrapper):
             # For each (4) item created
             #   - try to find draft
             #   - try to find non-draft
-            #   - retrieve draft of new parent
-            #   - get last error
-            #   - load parent
-            #   - load inheritable data
-            #   - load parent
-            #   - load ancestors
+            #   - compute what is parent
+            #   - load draft parent again & compute its parent chain up to course
             # count for updates increased to 16 b/c of edit_info updating
-            with check_mongo_calls(40, 16):
+            with check_mongo_calls(36, 16):
                 self._create_item('html', 'Html1', "<p>Goodbye</p>", {'display_name': 'Parented Html'}, 'vertical', 'Vert1', split=False)
                 self._create_item(
                     'discussion', 'Discussion1',
@@ -96,22 +92,22 @@ class TestPublish(SplitWMongoCourseBoostrapper):
         item = self.draft_mongo.get_item(vert_location, 2)
         # Finds:
         #   1 get draft vert,
-        #   2-10 for each child: (3 children x 3 queries each)
-        #      get draft and then published child
+        #   2 compute parent
+        #   3-14 for each child: (3 children x 4 queries each)
+        #      get draft, compute parent, and then published child
         #      compute inheritance
-        #   11 get published vert
-        #   12-15 get each ancestor (count then get): (2 x 2),
-        #   16 then fail count of course parent (1)
-        #   17 compute inheritance
-        #   18-19 get draft and published vert
+        #   15 get published vert
+        #   16-18 get ancestor chain
+        #   19 compute inheritance
+        #   20-22 get draft and published vert, compute parent
         # Sends:
         #   delete the subtree of drafts (1 call),
         #   update the published version of each node in subtree (4 calls),
         #   update the ancestors up to course (2 calls)
         if mongo_uses_error_check(self.draft_mongo):
-            max_find = 20
+            max_find = 23
         else:
-            max_find = 19
+            max_find = 22
         with check_mongo_calls(max_find, 7):
             self.draft_mongo.publish(item.location, self.user_id)
 

common/lib/xmodule/xmodule/modulestore/tests/test_xml.py

@@ -31,7 +31,7 @@ class TestXMLModuleStore(unittest.TestCase):
     Test around the XML modulestore
     """
     def test_xml_modulestore_type(self):
-        store = XMLModuleStore(DATA_DIR, course_dirs=['toy', 'simple'])
+        store = XMLModuleStore(DATA_DIR, course_dirs=[])
         self.assertEqual(store.get_modulestore_type(), ModuleStoreEnum.Type.xml)
 
     def test_unicode_chars_in_xml_content(self):
@@ -102,14 +102,39 @@ class TestXMLModuleStore(unittest.TestCase):
         Test the branch setting context manager
         """
         store = XMLModuleStore(DATA_DIR, course_dirs=['toy'])
-        course_key = store.get_courses()[0]
+        course = store.get_courses()[0]
 
         # XML store allows published_only branch setting
-        with store.branch_setting(ModuleStoreEnum.Branch.published_only, course_key):
-            store.get_item(course_key.location)
+        with store.branch_setting(ModuleStoreEnum.Branch.published_only, course.id):
+            store.get_item(course.location)
 
         # XML store does NOT allow draft_preferred branch setting
         with self.assertRaises(ValueError):
-            with store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, course_key):
+            with store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, course.id):
                 # verify that the above context manager raises a ValueError
                 pass  # pragma: no cover
+
+    @patch('xmodule.modulestore.xml.log')
+    def test_dag_course(self, mock_logging):
+        """
+        Test a course whose structure is not a tree.
+        """
+        store = XMLModuleStore(DATA_DIR, course_dirs=['xml_dag'])
+        course_key = store.get_courses()[0].id
+
+        mock_logging.warning.assert_called_with(
+            "%s has more than one definition", course_key.make_usage_key('discussion', 'duplicate_def')
+        )
+
+        shared_item_loc = course_key.make_usage_key('html', 'toyhtml')
+        shared_item = store.get_item(shared_item_loc)
+        parent = shared_item.get_parent()
+        self.assertIsNotNone(parent, "get_parent failed to return a value")
+        parent_loc = course_key.make_usage_key('vertical', 'vertical_test')
+        self.assertEqual(parent.location, parent_loc)
+        self.assertIn(shared_item, parent.get_children())
+        # ensure it's still a child of the other parent even tho it doesn't claim the other parent as its parent
+        other_parent_loc = course_key.make_usage_key('vertical', 'zeta')
+        other_parent = store.get_item(other_parent_loc)
+        # children rather than get_children b/c the instance returned by get_children != shared_item
+        self.assertIn(shared_item_loc, other_parent.children)

common/lib/xmodule/xmodule/modulestore/xml.py

@@ -53,7 +53,7 @@ def clean_out_mako_templating(xml_string):
 
 class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
     def __init__(self, xmlstore, course_id, course_dir,
-                 error_tracker, parent_tracker,
+                 error_tracker,
                  load_error_modules=True, **kwargs):
         """
         A class that handles loading from xml.  Does some munging to ensure that
@@ -205,11 +205,20 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
 
             descriptor.data_dir = course_dir
 
+            if descriptor.scope_ids.usage_id in xmlstore.modules[course_id]:
+                # keep the parent pointer if any but allow everything else to overwrite
+                other_copy = xmlstore.modules[course_id][descriptor.scope_ids.usage_id]
+                descriptor.parent = other_copy.parent
+                if descriptor != other_copy:
+                    log.warning("%s has more than one definition", descriptor.scope_ids.usage_id)
             xmlstore.modules[course_id][descriptor.scope_ids.usage_id] = descriptor
 
             if descriptor.has_children:
                 for child in descriptor.get_children():
-                    parent_tracker.add_parent(child.scope_ids.usage_id, descriptor.scope_ids.usage_id)
+                    # parent is alphabetically least
+                    if child.parent is None or child.parent > descriptor.scope_ids.usage_id:
+                        child.parent = descriptor.location
+                        child.save()
 
             # After setting up the descriptor, save any changes that we have
             # made to attributes on the descriptor to the underlying KeyValueStore.
@@ -278,41 +287,6 @@ class CourseLocationManager(OpaqueKeyReader, AsideKeyGenerator):
         return usage_id
 
 
-class ParentTracker(object):
-    """A simple class to factor out the logic for tracking location parent pointers."""
-    def __init__(self):
-        """
-        Init
-        """
-        # location -> parent.  Not using defaultdict because we care about the empty case.
-        self._parents = dict()
-
-    def add_parent(self, child, parent):
-        """
-        Add a parent of child location to the set of parents.  Duplicate calls have no effect.
-
-        child and parent must be :class:`.Location` instances.
-        """
-        self._parents[child] = parent
-
-    def is_known(self, child):
-        """
-        returns True iff child has some parents.
-        """
-        return child in self._parents
-
-    def make_known(self, location):
-        """Tell the parent tracker about an object, without registering any
-        parents for it.  Used for the top level course descriptor locations."""
-        self._parents.setdefault(location, None)
-
-    def parent(self, child):
-        """
-        Return the parent of this child.  If not is_known(child), will throw a KeyError
-        """
-        return self._parents[child]
-
-
 class XMLModuleStore(ModuleStoreReadBase):
     """
     An XML backed ModuleStore
@@ -352,8 +326,6 @@ class XMLModuleStore(ModuleStoreReadBase):
             class_ = getattr(import_module(module_path), class_name)
             self.default_class = class_
 
-        self.parent_trackers = defaultdict(ParentTracker)
-
         # All field data will be stored in an inheriting field data.
         self.field_data = inheriting_field_data(kvs=DictKeyValueStore())
 
@@ -400,7 +372,7 @@ class XMLModuleStore(ModuleStoreReadBase):
         else:
             self.courses[course_dir] = course_descriptor
             self._course_errors[course_descriptor.id] = errorlog
-            self.parent_trackers[course_descriptor.id].make_known(course_descriptor.scope_ids.usage_id)
+            course_descriptor.parent = None
 
     def __unicode__(self):
         '''
@@ -512,7 +484,6 @@ class XMLModuleStore(ModuleStoreReadBase):
                 course_id=course_id,
                 course_dir=course_dir,
                 error_tracker=tracker,
-                parent_tracker=self.parent_trackers[course_id],
                 load_error_modules=self.load_error_modules,
                 get_policy=get_policy,
                 mixins=self.xblock_mixins,
@@ -756,10 +727,8 @@ class XMLModuleStore(ModuleStoreReadBase):
         '''Find the location that is the parent of this location in this
         course.  Needed for path_to_location().
         '''
-        if not self.parent_trackers[location.course_key].is_known(location):
-            raise ItemNotFoundError("{0} not in {1}".format(location, location.course_key))
-
-        return self.parent_trackers[location.course_key].parent(location)
+        block = self.get_item(location, 0)
+        return block.parent
 
     def get_modulestore_type(self, course_key=None):
         """

common/lib/xmodule/xmodule/modulestore/xml_importer.py

@@ -28,7 +28,7 @@ import json
 import re
 from lxml import etree
 
-from .xml import XMLModuleStore, ImportSystem, ParentTracker
+from .xml import XMLModuleStore, ImportSystem
 from xblock.runtime import KvsFieldData, DictKeyValueStore
 from xmodule.x_module import XModuleDescriptor
 from opaque_keys.edx.keys import UsageKey
@@ -479,11 +479,13 @@ def _import_module_and_update_references(
 
     fields = {}
     for field_name, field in module.fields.iteritems():
-        if field.is_set_on(module):
-            if field.scope == Scope.parent:
-                continue
+        if field.scope != Scope.parent and field.is_set_on(module):
             if isinstance(field, Reference):
-                fields[field_name] = _convert_reference_fields_to_new_namespace(field.read_from(module))
+                value = field.read_from(module)
+                if value is None:
+                    fields[field_name] = None
+                else:
+                    fields[field_name] = _convert_reference_fields_to_new_namespace(field.read_from(module))
             elif isinstance(field, ReferenceList):
                 references = field.read_from(module)
                 fields[field_name] = [_convert_reference_fields_to_new_namespace(reference) for reference in references]
@@ -548,7 +550,6 @@ def _import_course_draft(
         course_id=source_course_id,
         course_dir=draft_course_dir,
         error_tracker=errorlog.tracker,
-        parent_tracker=ParentTracker(),
         load_error_modules=False,
         mixins=xml_module_store.xblock_mixins,
         field_data=KvsFieldData(kvs=DictKeyValueStore()),

common/lib/xmodule/xmodule/partitions/partitions.py

@@ -10,7 +10,21 @@ from stevedore.extension import ExtensionManager
 
 class UserPartitionError(Exception):
     """
-    An error was found regarding user partitions.
+    Base Exception for when an error was found regarding user partitions.
+    """
+    pass
+
+
+class NoSuchUserPartitionError(UserPartitionError):
+    """
+    Exception to be raised when looking up a UserPartition by its ID fails.
+    """
+    pass
+
+
+class NoSuchUserPartitionGroupError(UserPartitionError):
+    """
+    Exception to be raised when looking up a UserPartition Group by its ID fails.
     """
     pass
 
@@ -171,9 +185,14 @@ class UserPartition(namedtuple("UserPartition", "id name description groups sche
 
     def get_group(self, group_id):
         """
-        Returns the group with the specified id.
+        Returns the group with the specified id.  Raises NoSuchUserPartitionGroupError if not found.
         """
-        for group in self.groups:    # pylint: disable=no-member
+        # pylint: disable=no-member
+
+        for group in self.groups:
             if group.id == group_id:
                 return group
-        return None
+
+        raise NoSuchUserPartitionGroupError(
+            "could not find a Group with ID [{}] in UserPartition [{}]".format(group_id, self.id)
+        )