restrict move action

cms/djangoapps/contentstore/views/item.py

@@ -29,7 +29,7 @@ from cms.lib.xblock.authoring_mixin import VISIBILITY_VIEW
 from contentstore.utils import (
     find_release_date_source, find_staff_lock_source, is_currently_visible_to_students,
     ancestor_has_staff_lock, has_children_visible_to_specific_content_groups,
-    get_user_partition_info,
+    get_user_partition_info, get_group_display_name,
 )
 from contentstore.views.helpers import is_unit, xblock_studio_url, xblock_primary_child_category, \
     xblock_type_display_name, get_parent_xblock, create_xblock, usage_key_with_run
@@ -675,6 +675,21 @@ def _get_source_index(source_usage_key, source_parent):
         return None
 
 
+def is_source_item_in_target_parents(source_item, target_parent):
+    """
+    Returns True if source item is found in target parents otherwise False.
+
+    Arguments:
+        source_item (XBlock): Source Xblock.
+        target_parent (XBlock): Target XBlock.
+    """
+    target_ancestors = _create_xblock_ancestor_info(target_parent, is_concise=True)['ancestors']
+    for target_ancestor in target_ancestors:
+        if unicode(source_item.location) == target_ancestor['id']:
+            return True
+    return False
+
+
 def _move_item(source_usage_key, target_parent_usage_key, user, target_index=None):
     """
     Move an existing xblock as a child of the supplied target_parent_usage_key.
@@ -688,8 +703,11 @@ def _move_item(source_usage_key, target_parent_usage_key, user, target_index=Non
         JsonResponse: Information regarding move operation. It may contains error info if an invalid move operation
             is performed.
     """
-    # Get the list of all component type XBlocks
-    component_types = sorted(set(name for name, class_ in XBlock.load_classes()) - set(DIRECT_ONLY_CATEGORIES))
+    # Get the list of all parentable component type XBlocks.
+    parent_component_types = list(
+        set(name for name, class_ in XBlock.load_classes() if getattr(class_, 'has_children', False)) -
+        set(DIRECT_ONLY_CATEGORIES)
+    )
 
     store = modulestore()
     with store.bulk_operations(source_usage_key.course_key):
@@ -705,18 +723,22 @@ def _move_item(source_usage_key, target_parent_usage_key, user, target_index=Non
         source_index = _get_source_index(source_usage_key, source_parent)
 
         valid_move_type = {
-            'vertical': source_type if source_type in component_types else 'component',
             'sequential': 'vertical',
             'chapter': 'sequential',
         }
 
-        if valid_move_type.get(target_parent_type, '') != source_type:
+        if (valid_move_type.get(target_parent_type, '') != source_type and
+                target_parent_type not in parent_component_types):
             error = 'You can not move {source_type} into {target_parent_type}.'.format(
                 source_type=source_type,
                 target_parent_type=target_parent_type,
             )
         elif source_parent.location == target_parent.location:
             error = 'You can not move an item into the same parent.'
+        elif source_item.location == target_parent.location:
+            error = 'You can not move an item into itself.'
+        elif is_source_item_in_target_parents(source_item, target_parent):
+            error = 'You can not move an item into it\'s child.'
         elif source_index is None:
             error = '{source_usage_key} not found in {parent_usage_key}.'.format(
                 source_usage_key=unicode(source_usage_key),
@@ -1093,6 +1115,7 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F
             # a percent value out of 100, e.g. "58%" means "58/100".
             pct_sign=_('%'))
 
+    user_partitions = get_user_partition_info(xblock, course=course)
     xblock_info = {
         'id': unicode(xblock.location),
         'display_name': xblock.display_name_with_default,
@@ -1101,6 +1124,10 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F
     if is_concise:
         if child_info and len(child_info.get('children', [])) > 0:
             xblock_info['child_info'] = child_info
+        # Groups are labelled with their internal ids, rather than with the group name. Replace id with display name.
+        group_display_name = get_group_display_name(user_partitions, xblock_info['display_name'])
+        xblock_info['display_name'] = group_display_name if group_display_name else xblock_info['display_name']
+        xblock_info['has_children'] = xblock.has_children
     else:
         xblock_info.update({
             'edited_on': get_default_time_display(xblock.subtree_edited_on) if xblock.subtree_edited_on else None,
@@ -1121,7 +1148,7 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F
             'actions': xblock_actions,
             'explanatory_message': explanatory_message,
             'group_access': xblock.group_access,
-            'user_partitions': get_user_partition_info(xblock, course=course),
+            'user_partitions': user_partitions,
         })
 
         if xblock.category == 'sequential':

cms/djangoapps/contentstore/views/preview.py

@@ -274,6 +274,7 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False):
             'can_edit': context.get('can_edit', True),
             'can_edit_visibility': context.get('can_edit_visibility', True),
             'can_add': context.get('can_add', True),
+            'can_move': context.get('can_move', True)
         }
         html = render_to_string('studio_xblock_wrapper.html', template_context)
         frag = wrap_fragment(frag, html)

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

@@ -12,11 +12,13 @@ from django.utils import http
 
 import contentstore.views.component as views
 from contentstore.views.tests.utils import StudioPageTestCase
+from contentstore.tests.test_libraries import LibraryTestCase
+from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
-from xmodule.modulestore.tests.factories import ItemFactory
+from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 
 
-class ContainerPageTestCase(StudioPageTestCase):
+class ContainerPageTestCase(StudioPageTestCase, LibraryTestCase):
     """
     Unit tests for the container page.
     """
@@ -128,6 +130,44 @@ class ContainerPageTestCase(StudioPageTestCase):
         self.validate_preview_html(published_child_container, self.container_view)
         self.validate_preview_html(published_child_vertical, self.reorderable_child_view)
 
+    def test_library_page_preview_html(self):
+        """
+        Verify that a library xblock's container (library page) preview returns the expected HTML.
+        """
+        # Add some content to library.
+        self._add_simple_content_block()
+        self.validate_preview_html(self.library, self.container_view, can_reorder=False, can_move=False)
+
+    def test_library_content_preview_html(self):
+        """
+        Verify that a library content block container page preview returns the expected HTML.
+        """
+        # Library content block is only supported in split courses.
+        with modulestore().default_store(ModuleStoreEnum.Type.split):
+            course = CourseFactory.create()
+
+        # Add some content to library
+        self._add_simple_content_block()
+
+        # Create a library content block
+        lc_block = self._add_library_content_block(course, self.lib_key)
+        self.assertEqual(len(lc_block.children), 0)
+
+        # Refresh children to be reflected in lc_block
+        lc_block = self._refresh_children(lc_block)
+        self.assertEqual(len(lc_block.children), 1)
+
+        self.validate_preview_html(
+            lc_block,
+            self.container_view,
+            can_add=False,
+            can_reorder=False,
+            can_move=False,
+            can_edit=True,
+            can_duplicate=False,
+            can_delete=False
+        )
+
     def test_draft_container_preview_html(self):
         """
         Verify that a draft xblock's container preview returns the expected HTML.

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

@@ -755,6 +755,13 @@ class TestMoveItem(ItemTest):
             default_store = self.store.default_modulestore.get_modulestore_type()
 
         self.course = CourseFactory.create(default_store=default_store)
+
+        # Create group configurations
+        self.course.user_partitions = [
+            UserPartition(0, 'first_partition', 'Test Partition', [Group("0", 'alpha'), Group("1", 'beta')])
+        ]
+        self.store.update_item(self.course, self.user.id)
+
         # Create a parent chapter
         chap1 = self.create_xblock(parent_usage_key=self.course.location, display_name='chapter1', category='chapter')
         self.chapter_usage_key = self.response_usage_key(chap1)
@@ -762,27 +769,54 @@ class TestMoveItem(ItemTest):
         chap2 = self.create_xblock(parent_usage_key=self.course.location, display_name='chapter2', category='chapter')
         self.chapter2_usage_key = self.response_usage_key(chap2)
 
-        # create a sequential
+        # Create a sequential
         seq1 = self.create_xblock(parent_usage_key=self.chapter_usage_key, display_name='seq1', category='sequential')
         self.seq_usage_key = self.response_usage_key(seq1)
 
         seq2 = self.create_xblock(parent_usage_key=self.chapter_usage_key, display_name='seq2', category='sequential')
         self.seq2_usage_key = self.response_usage_key(seq2)
 
-        # create a vertical
+        # Create a vertical
         vert1 = self.create_xblock(parent_usage_key=self.seq_usage_key, display_name='vertical1', category='vertical')
         self.vert_usage_key = self.response_usage_key(vert1)
 
         vert2 = self.create_xblock(parent_usage_key=self.seq_usage_key, display_name='vertical2', category='vertical')
         self.vert2_usage_key = self.response_usage_key(vert2)
 
-        # create problem and an html component
+        # Create problem and an html component
         problem1 = self.create_xblock(parent_usage_key=self.vert_usage_key, display_name='problem1', category='problem')
         self.problem_usage_key = self.response_usage_key(problem1)
 
         html1 = self.create_xblock(parent_usage_key=self.vert_usage_key, display_name='html1', category='html')
         self.html_usage_key = self.response_usage_key(html1)
 
+        # Create a content experiment
+        resp = self.create_xblock(category='split_test', parent_usage_key=self.vert_usage_key)
+        self.split_test_usage_key = self.response_usage_key(resp)
+
+    def setup_and_verify_content_experiment(self, partition_id):
+        """
+        Helper method to set up group configurations to content experiment.
+
+        Arguments:
+            partition_id (int): User partition id.
+        """
+        split_test = self.get_item_from_modulestore(self.split_test_usage_key, verify_is_draft=True)
+
+        # Initially, no user_partition_id is set, and the split_test has no children.
+        self.assertEqual(split_test.user_partition_id, -1)
+        self.assertEqual(len(split_test.children), 0)
+
+        # Set group configuration
+        self.client.ajax_post(
+            reverse_usage_url("xblock_handler", self.split_test_usage_key),
+            data={'metadata': {'user_partition_id': str(partition_id)}}
+        )
+        split_test = self.get_item_from_modulestore(self.split_test_usage_key, verify_is_draft=True)
+        self.assertEqual(split_test.user_partition_id, partition_id)
+        self.assertEqual(len(split_test.children), len(self.course.user_partitions[partition_id].groups))
+        return split_test
+
     def _move_component(self, source_usage_key, target_usage_key, target_index=None):
         """
         Helper method to send move request and returns the response.
@@ -853,7 +887,7 @@ class TestMoveItem(ItemTest):
         """
         parent = self.get_item_from_modulestore(self.vert_usage_key)
         children = parent.get_children()
-        self.assertEqual(len(children), 2)
+        self.assertEqual(len(children), 3)
 
         # Create a component within vert2.
         resp = self.create_xblock(parent_usage_key=self.vert2_usage_key, display_name='html2', category='html')
@@ -863,7 +897,7 @@ class TestMoveItem(ItemTest):
         self.assert_move_item(html2_usage_key, self.vert_usage_key, 1)
         parent = self.get_item_from_modulestore(self.vert_usage_key)
         children = parent.get_children()
-        self.assertEqual(len(children), 3)
+        self.assertEqual(len(children), 4)
         self.assertEqual(children[1].location, html2_usage_key)
 
     def test_move_undo(self):
@@ -940,6 +974,108 @@ class TestMoveItem(ItemTest):
         self.assertEqual(response['error'], 'You can not move an item into the same parent.')
         self.assertEqual(self.store.get_parent_location(self.html_usage_key), parent_loc)
 
+    def test_can_not_move_into_itself(self):
+        """
+        Test that a component can not be moved to itself.
+        """
+        library_content = self.create_xblock(
+            parent_usage_key=self.vert_usage_key, display_name='library content block', category='library_content'
+        )
+        library_content_usage_key = self.response_usage_key(library_content)
+        parent_loc = self.store.get_parent_location(library_content_usage_key)
+        self.assertEqual(parent_loc, self.vert_usage_key)
+        response = self._move_component(library_content_usage_key, library_content_usage_key)
+        self.assertEqual(response.status_code, 400)
+        response = json.loads(response.content)
+
+        self.assertEqual(response['error'], 'You can not move an item into itself.')
+        self.assertEqual(self.store.get_parent_location(self.html_usage_key), parent_loc)
+
+    def test_move_library_content(self):
+        """
+        Test that library content  can be moved to any other valid location.
+        """
+        library_content = self.create_xblock(
+            parent_usage_key=self.vert_usage_key, display_name='library content block', category='library_content'
+        )
+        library_content_usage_key = self.response_usage_key(library_content)
+        parent_loc = self.store.get_parent_location(library_content_usage_key)
+        self.assertEqual(parent_loc, self.vert_usage_key)
+        self.assert_move_item(library_content_usage_key, self.vert2_usage_key)
+
+    def test_move_into_library_content(self):
+        """
+        Test that a component can be moved into library content.
+        """
+        library_content = self.create_xblock(
+            parent_usage_key=self.vert_usage_key, display_name='library content block', category='library_content'
+        )
+        library_content_usage_key = self.response_usage_key(library_content)
+        self.assert_move_item(self.html_usage_key, library_content_usage_key)
+
+    def test_move_content_experiment(self):
+        """
+        Test that a content experiment can be moved.
+        """
+        self.setup_and_verify_content_experiment(0)
+
+        # Move content experiment
+        self.assert_move_item(self.split_test_usage_key, self.vert2_usage_key)
+
+    def test_move_content_experiment_components(self):
+        """
+        Test that component inside content experiment can be moved to any other valid location.
+        """
+        split_test = self.setup_and_verify_content_experiment(0)
+
+        # Add html component to Group A.
+        html1 = self.create_xblock(
+            parent_usage_key=split_test.children[0], display_name='html1', category='html'
+        )
+        html_usage_key = self.response_usage_key(html1)
+
+        # Move content experiment
+        self.assert_move_item(html_usage_key, self.vert2_usage_key)
+
+    def test_move_into_content_experiment_groups(self):
+        """
+        Test that a component can be moved to content experiment.
+        """
+        split_test = self.setup_and_verify_content_experiment(0)
+        self.assert_move_item(self.html_usage_key, split_test.children[0])
+
+    def test_can_not_move_content_experiment_into_its_children(self):
+        """
+        Test that a content experiment can not be moved inside any of it's children.
+        """
+        split_test = self.setup_and_verify_content_experiment(0)
+
+        # Try to move content experiment inside it's child groups.
+        for child_vert_usage_key in split_test.children:
+            response = self._move_component(self.split_test_usage_key, child_vert_usage_key)
+            self.assertEqual(response.status_code, 400)
+            response = json.loads(response.content)
+
+            self.assertEqual(response['error'], 'You can not move an item into it\'s child.')
+            self.assertEqual(self.store.get_parent_location(self.split_test_usage_key), self.vert_usage_key)
+
+        # Create content experiment inside group A and set it's group configuration.
+        resp = self.create_xblock(category='split_test', parent_usage_key=split_test.children[0])
+        child_split_test_usage_key = self.response_usage_key(resp)
+        self.client.ajax_post(
+            reverse_usage_url("xblock_handler", child_split_test_usage_key),
+            data={'metadata': {'user_partition_id': str(0)}}
+        )
+        child_split_test = self.get_item_from_modulestore(self.split_test_usage_key, verify_is_draft=True)
+
+        # Try to move content experiment further down the level to a child group A nested inside main group A.
+        response = self._move_component(self.split_test_usage_key, child_split_test.children[0])
+        self.assertEqual(response.status_code, 400)
+        response = json.loads(response.content)
+
+        self.assertEqual(response['error'], 'You can not move an item into it\'s child.')
+        self.assertEqual(self.store.get_parent_location(self.split_test_usage_key), self.vert_usage_key)
+
     def test_move_invalid_source_index(self):
         """
         Test moving an item to an invalid index.
@@ -1611,6 +1747,31 @@ class TestEditSplitModule(ItemTest):
         self.assertEqual(vertical_0.location, split_test.group_id_to_child['0'])
         self.assertEqual(vertical_1.location, split_test.group_id_to_child['1'])
 
+    def test_split_xblock_info_group_name(self):
+        """
+        Test that concise outline for split test component gives display name as group name.
+        """
+        split_test = self.get_item_from_modulestore(self.split_test_usage_key, verify_is_draft=True)
+        # Initially, no user_partition_id is set, and the split_test has no children.
+        self.assertEqual(split_test.user_partition_id, -1)
+        self.assertEqual(len(split_test.children), 0)
+        # Set the user_partition_id to 0.
+        split_test = self._update_partition_id(0)
+        # Verify that child verticals have been set to match the groups
+        self.assertEqual(len(split_test.children), 2)
+
+        # Get xblock outline
+        xblock_info = create_xblock_info(
+            split_test,
+            is_concise=True,
+            include_child_info=True,
+            include_children_predicate=lambda xblock: xblock.has_children,
+            course=self.course,
+            user=self.request.user
+        )
+        self.assertEqual(xblock_info['child_info']['children'][0]['display_name'], 'alpha')
+        self.assertEqual(xblock_info['child_info']['children'][1]['display_name'], 'beta')
+
     def test_change_user_partition_id(self):
         """
         Test what happens when the user_partition_id is changed to a different groups

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

@@ -41,34 +41,48 @@ class StudioPageTestCase(CourseTestCase):
         resp_content = json.loads(resp.content)
         return resp_content['html']
 
-    def validate_preview_html(self, xblock, view_name, can_add=True):
+    def validate_preview_html(self, xblock, view_name, can_add=True, can_reorder=True, can_move=True,
+                              can_edit=True, can_duplicate=True, can_delete=True):
         """
         Verify that the specified xblock's preview has the expected HTML elements.
         """
         html = self.get_preview_html(xblock, view_name)
-        self.validate_html_for_add_buttons(html, can_add)
-
-        # Verify drag handles always appear.
-        drag_handle_html = '<span data-tooltip="Drag to reorder" class="drag-handle action"></span>'
-        self.assertIn(drag_handle_html, html)
-
-        # Verify that there are no action buttons for public blocks
-        expected_button_html = [
-            '<button class="btn-default edit-button action-button">',
+        self.validate_html_for_action_button(
+            html,
+            '<div class="add-xblock-component new-component-item adding"></div>',
+            can_add
+        )
+        self.validate_html_for_action_button(
+            html,
+            '<span data-tooltip="Drag to reorder" class="drag-handle action"></span>',
+            can_reorder
+        )
+        self.validate_html_for_action_button(
+            html,
+            '<button data-tooltip="Move" class="btn-default move-button action-button">',
+            can_move
+        )
+        self.validate_html_for_action_button(
+            html,
+            'button class="btn-default edit-button action-button">',
+            can_edit
+        )
+        self.validate_html_for_action_button(
+            html,
             '<button data-tooltip="Delete" class="btn-default delete-button action-button">',
+            can_duplicate
+        )
+        self.validate_html_for_action_button(
+            html,
             '<button data-tooltip="Duplicate" class="btn-default duplicate-button action-button">',
-            '<button data-tooltip="Move" class="btn-default move-button action-button">'
-        ]
-        for button_html in expected_button_html:
-            self.assertIn(button_html, html)
+            can_delete
+        )
 
-    def validate_html_for_add_buttons(self, html, can_add=True):
+    def validate_html_for_action_button(self, html, expected_html, can_action=True):
         """
-        Validate that the specified HTML has the appropriate add actions for the current publish state.
+        Validate that the specified HTML has specific action..
         """
-        # Verify that there are no add buttons for public blocks
-        add_button_html = '<div class="add-xblock-component new-component-item adding"></div>'
-        if can_add:
-            self.assertIn(add_button_html, html)
+        if can_action:
+            self.assertIn(expected_html, html)
         else:
-            self.assertNotIn(add_button_html, html)
+            self.assertNotIn(expected_html, html)