diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 8ed409fc8076..f1a5ed149471 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -283,6 +283,23 @@ def reverse_usage_url(handler_name, usage_key, kwargs=None): return reverse_url(handler_name, 'usage_key_string', usage_key, kwargs) +def get_group_display_name(user_partitions, xblock_display_name): + """ + Get the group name if matching group xblock is found. + + Arguments: + user_partitions (Dict): Locator of source item. + xblock_display_name (String): Display name of group xblock. + + Returns: + group name (String): Group name of the matching group. + """ + for user_partition in user_partitions: + for group in user_partition['groups']: + if str(group['id']) in xblock_display_name: + return group['name'] + + def get_user_partition_info(xblock, schemes=None, course=None): """ Retrieve user partition information for an XBlock for display in editors. diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index ecf6d4d44440..91c4f75bfb66 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -29,7 +29,7 @@ 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': diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 7c146e099ef9..2050dafaa197 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/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) diff --git a/cms/djangoapps/contentstore/views/tests/test_container_page.py b/cms/djangoapps/contentstore/views/tests/test_container_page.py index b22e06327fe6..2b8caff47517 100644 --- a/cms/djangoapps/contentstore/views/tests/test_container_page.py +++ b/cms/djangoapps/contentstore/views/tests/test_container_page.py @@ -12,11 +12,13 @@ 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 @@ def test_public_container_preview_html(self): 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. diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index eff9f8773011..afad862fd2c4 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -755,6 +755,13 @@ def setup_course(self, default_store=None): 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 @@ def setup_course(self, default_store=None): 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 @@ def test_move_source_index(self): """ 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 @@ def test_move_source_index(self): 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 @@ def test_move_current_parent(self): 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 @@ def test_create_groups(self): 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 diff --git a/cms/djangoapps/contentstore/views/tests/utils.py b/cms/djangoapps/contentstore/views/tests/utils.py index 00358d005fcf..6c2580fd1a4c 100644 --- a/cms/djangoapps/contentstore/views/tests/utils.py +++ b/cms/djangoapps/contentstore/views/tests/utils.py @@ -41,34 +41,48 @@ def get_preview_html(self, xblock, view_name): 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 = '' - self.assertIn(drag_handle_html, html) - - # Verify that there are no action buttons for public blocks - expected_button_html = [ - ' + <% } else { %> + + + <%- xblock.get('display_name') %> + + <% if(currentLocationIndex === i) { %> + + (<%- gettext('Currently selected') %>) + + <% } %> + <% } %> <% } %> diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index fad6e36e7b5b..969e4e7af73c 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -89,7 +89,8 @@ ${_("Duplicate")} - + % endif + % if can_move: