From 3f355241be7c55e53d8a4b1ec0fa781404ab86ff Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Thu, 22 Dec 2016 19:09:00 +0500 Subject: [PATCH 01/12] Get concise course outline data for move dialog box Get ancestor info for the given xblock - TNL-6061 --- cms/djangoapps/contentstore/views/course.py | 9 +- cms/djangoapps/contentstore/views/item.py | 171 ++++++++++-------- .../views/tests/test_course_index.py | 26 ++- .../contentstore/views/tests/test_item.py | 56 +++++- 4 files changed, 173 insertions(+), 89 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index df5ba575aea2..20878b839319 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -336,11 +336,16 @@ def _course_outline_json(request, course_module): """ Returns a JSON representation of the course module and recursively all of its children. """ + is_concise = request.GET.get('formats') == 'concise' + include_children_predicate = lambda xblock: not xblock.category == 'vertical' + if is_concise: + include_children_predicate = lambda xblock: xblock.has_children return create_xblock_info( course_module, include_child_info=True, - course_outline=True, - include_children_predicate=lambda xblock: not xblock.category == 'vertical', + course_outline=False if is_concise else True, + include_children_predicate=include_children_predicate, + is_concise=is_concise, user=request.user ) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index f1789eeff81c..36a2834cb2d2 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -98,6 +98,7 @@ def xblock_handler(request, usage_key_string): GET json: returns representation of the xblock (locator id, data, and metadata). if ?fields=graderType, it returns the graderType for the unit instead of the above. + if ?fields=ancestorInfo, it returns ancestor info of the xblock. html: returns HTML for rendering the xblock (which includes both the "preview" view and the "editor" view) PUT or POST or PATCH json: if xblock locator is specified, update the xblock instance. The json payload can contain @@ -149,6 +150,10 @@ def xblock_handler(request, usage_key_string): if 'graderType' in fields: # right now can't combine output of this w/ output of _get_module_info, but worthy goal return JsonResponse(CourseGradingModel.get_section_grader_type(usage_key)) + elif 'ancestorInfo' in fields: + xblock = _get_xblock(usage_key, request.user) + ancestor_info = _create_xblock_ancestor_info(xblock, is_concise=True) + return JsonResponse(ancestor_info) # TODO: pass fields to _get_module_info and only return those with modulestore().bulk_operations(usage_key.course_key): response = _get_module_info(_get_xblock(usage_key, request.user)) @@ -887,7 +892,7 @@ def _get_gating_info(course, xblock): def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=False, include_child_info=False, course_outline=False, include_children_predicate=NEVER, parent_xblock=None, graders=None, - user=None, course=None): + user=None, course=None, is_concise=False): """ Creates the information needed for client-side XBlockInfo. @@ -897,6 +902,7 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F There are three optional boolean parameters: include_ancestor_info - if true, ancestor info is added to the response include_child_info - if true, direct child info is included in the response + is_concise - if true, returns the concise version of xblock info, default is false. course_outline - if true, the xblock is being rendered on behalf of the course outline. There are certain expensive computations that do not need to be included in this case. @@ -933,20 +939,22 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F graders, include_children_predicate=include_children_predicate, user=user, - course=course + course=course, + is_concise=is_concise ) else: child_info = None release_date = _get_release_date(xblock, user) - if xblock.category != 'course': + if xblock.category != 'course' and not is_concise: visibility_state = _compute_visibility_state( xblock, child_info, is_xblock_unit and has_changes, is_self_paced(course) ) else: visibility_state = None published = modulestore().has_published_version(xblock) if not is_library_block else None + published_on = get_default_time_display(xblock.published_on) if published and xblock.published_on else None # defining the default value 'True' for delete, duplicate, drag and add new child actions # in xblock_actions for each xblock. @@ -970,83 +978,89 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F pct_sign=_('%')) xblock_info = { - "id": unicode(xblock.location), - "display_name": xblock.display_name_with_default, - "category": xblock.category, - "edited_on": get_default_time_display(xblock.subtree_edited_on) if xblock.subtree_edited_on else None, - "published": published, - "published_on": get_default_time_display(xblock.published_on) if published and xblock.published_on else None, - "studio_url": xblock_studio_url(xblock, parent_xblock), - "released_to_students": datetime.now(UTC) > xblock.start, - "release_date": release_date, - "visibility_state": visibility_state, - "has_explicit_staff_lock": xblock.fields['visible_to_staff_only'].is_set_on(xblock), - "self_paced": is_self_paced(course), - "start": xblock.fields['start'].to_json(xblock.start), - "graded": xblock.graded, - "due_date": get_default_time_display(xblock.due), - "due": xblock.fields['due'].to_json(xblock.due), - "format": xblock.format, - "course_graders": [grader.get('type') for grader in graders], - "has_changes": has_changes, - "actions": xblock_actions, - "explanatory_message": explanatory_message, - "group_access": xblock.group_access, - "user_partitions": get_user_partition_info(xblock, course=course), + 'id': unicode(xblock.location), + 'display_name': xblock.display_name_with_default, + 'category': xblock.category } - - if xblock.category == 'sequential': + if is_concise: + if child_info and len(child_info.get('children', [])) > 0: + xblock_info['child_info'] = child_info + else: xblock_info.update({ - "hide_after_due": xblock.hide_after_due, + 'edited_on': get_default_time_display(xblock.subtree_edited_on) if xblock.subtree_edited_on else None, + 'published': published, + 'published_on': published_on, + 'studio_url': xblock_studio_url(xblock, parent_xblock), + 'released_to_students': datetime.now(UTC) > xblock.start, + 'release_date': release_date, + 'visibility_state': visibility_state, + 'has_explicit_staff_lock': xblock.fields['visible_to_staff_only'].is_set_on(xblock), + 'start': xblock.fields['start'].to_json(xblock.start), + 'graded': xblock.graded, + 'due_date': get_default_time_display(xblock.due), + 'due': xblock.fields['due'].to_json(xblock.due), + 'format': xblock.format, + 'course_graders': [grader.get('type') for grader in graders], + 'has_changes': has_changes, + 'actions': xblock_actions, + 'explanatory_message': explanatory_message, + 'group_access': xblock.group_access, + 'user_partitions': get_user_partition_info(xblock, course=course), }) - # update xblock_info with special exam information if the feature flag is enabled - if settings.FEATURES.get('ENABLE_SPECIAL_EXAMS'): - if xblock.category == 'course': + if xblock.category == 'sequential': xblock_info.update({ - "enable_proctored_exams": xblock.enable_proctored_exams, - "create_zendesk_tickets": xblock.create_zendesk_tickets, - "enable_timed_exams": xblock.enable_timed_exams - }) - elif xblock.category == 'sequential': - xblock_info.update({ - "is_proctored_exam": xblock.is_proctored_exam, - "is_practice_exam": xblock.is_practice_exam, - "is_time_limited": xblock.is_time_limited, - "exam_review_rules": xblock.exam_review_rules, - "default_time_limit_minutes": xblock.default_time_limit_minutes, + 'hide_after_due': xblock.hide_after_due, }) - # Update with gating info - xblock_info.update(_get_gating_info(course, xblock)) - - if xblock.category == 'sequential': - # Entrance exam subsection should be hidden. in_entrance_exam is - # inherited metadata, all children will have it. - if getattr(xblock, "in_entrance_exam", False): - xblock_info["is_header_visible"] = False - - if data is not None: - xblock_info["data"] = data - if metadata is not None: - xblock_info["metadata"] = metadata - if include_ancestor_info: - xblock_info['ancestor_info'] = _create_xblock_ancestor_info(xblock, course_outline) - if child_info: - xblock_info['child_info'] = child_info - if visibility_state == VisibilityState.staff_only: - xblock_info["ancestor_has_staff_lock"] = ancestor_has_staff_lock(xblock, parent_xblock) - else: - xblock_info["ancestor_has_staff_lock"] = False + # update xblock_info with special exam information if the feature flag is enabled + if settings.FEATURES.get('ENABLE_SPECIAL_EXAMS'): + if xblock.category == 'course': + xblock_info.update({ + 'enable_proctored_exams': xblock.enable_proctored_exams, + 'create_zendesk_tickets': xblock.create_zendesk_tickets, + 'enable_timed_exams': xblock.enable_timed_exams + }) + elif xblock.category == 'sequential': + xblock_info.update({ + 'is_proctored_exam': xblock.is_proctored_exam, + 'is_practice_exam': xblock.is_practice_exam, + 'is_time_limited': xblock.is_time_limited, + 'exam_review_rules': xblock.exam_review_rules, + 'default_time_limit_minutes': xblock.default_time_limit_minutes, + }) - if course_outline: - if xblock_info["has_explicit_staff_lock"]: - xblock_info["staff_only_message"] = True - elif child_info and child_info["children"]: - xblock_info["staff_only_message"] = all([child["staff_only_message"] for child in child_info["children"]]) + # Update with gating info + xblock_info.update(_get_gating_info(course, xblock)) + + if xblock.category == 'sequential': + # Entrance exam subsection should be hidden. in_entrance_exam is + # inherited metadata, all children will have it. + if getattr(xblock, 'in_entrance_exam', False): + xblock_info['is_header_visible'] = False + + if data is not None: + xblock_info['data'] = data + if metadata is not None: + xblock_info['metadata'] = metadata + if include_ancestor_info: + xblock_info['ancestor_info'] = _create_xblock_ancestor_info(xblock, course_outline, include_child_info=True) + if child_info: + xblock_info['child_info'] = child_info + if visibility_state == VisibilityState.staff_only: + xblock_info['ancestor_has_staff_lock'] = ancestor_has_staff_lock(xblock, parent_xblock) else: - xblock_info["staff_only_message"] = False - + xblock_info['ancestor_has_staff_lock'] = False + + if course_outline: + if xblock_info['has_explicit_staff_lock']: + xblock_info['staff_only_message'] = True + elif child_info and child_info['children']: + xblock_info['staff_only_message'] = all( + [child['staff_only_message'] for child in child_info['children']] + ) + else: + xblock_info['staff_only_message'] = False return xblock_info @@ -1156,14 +1170,14 @@ def _compute_visibility_state(xblock, child_info, is_unit_with_changes, is_cours return VisibilityState.ready -def _create_xblock_ancestor_info(xblock, course_outline): +def _create_xblock_ancestor_info(xblock, course_outline=False, include_child_info=False, is_concise=False): """ Returns information about the ancestors of an xblock. Note that the direct parent will also return information about all of its children. """ ancestors = [] - def collect_ancestor_info(ancestor, include_child_info=False): + def collect_ancestor_info(ancestor, include_child_info=False, is_concise=False): """ Collect xblock info regarding the specified xblock and its ancestors. """ @@ -1173,16 +1187,18 @@ def collect_ancestor_info(ancestor, include_child_info=False): ancestor, include_child_info=include_child_info, course_outline=course_outline, - include_children_predicate=direct_children_only + include_children_predicate=direct_children_only, + is_concise=is_concise )) - collect_ancestor_info(get_parent_xblock(ancestor)) - collect_ancestor_info(get_parent_xblock(xblock), include_child_info=True) + collect_ancestor_info(get_parent_xblock(ancestor), is_concise=is_concise) + collect_ancestor_info(get_parent_xblock(xblock), include_child_info=include_child_info, is_concise=is_concise) return { 'ancestors': ancestors } -def _create_xblock_child_info(xblock, course_outline, graders, include_children_predicate=NEVER, user=None, course=None): # pylint: disable=line-too-long +def _create_xblock_child_info(xblock, course_outline, graders, include_children_predicate=NEVER, user=None, + course=None, is_concise=False): # pylint: disable=line-too-long """ Returns information about the children of an xblock, as well as about the primary category of xblock expected as children. @@ -1203,6 +1219,7 @@ def _create_xblock_child_info(xblock, course_outline, graders, include_children_ graders=graders, user=user, course=course, + is_concise=is_concise ) for child in xblock.get_children() ] return child_info diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index c38daa60200c..f2b89be6bea0 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -352,11 +352,16 @@ def setUp(self): parent_location=self.vertical.location, category="video", display_name="My Video" ) - def test_json_responses(self): + @ddt.data(True, False) + def test_json_responses(self, is_concise): """ Verify the JSON responses returned for the course. + + Arguments: + is_concise (Boolean) : If True, fetch concise version of course outline. """ outline_url = reverse_course_url('course_handler', self.course.id) + outline_url = outline_url + '?format=concise' if is_concise else outline_url resp = self.client.get(outline_url, HTTP_ACCEPT='application/json') json_response = json.loads(resp.content) @@ -364,8 +369,9 @@ def test_json_responses(self): self.assertEqual(json_response['category'], 'course') self.assertEqual(json_response['id'], unicode(self.course.location)) self.assertEqual(json_response['display_name'], self.course.display_name) - self.assertTrue(json_response['published']) - self.assertIsNone(json_response['visibility_state']) + if not is_concise: + self.assertTrue(json_response['published']) + self.assertIsNone(json_response['visibility_state']) # Now verify the first child children = json_response['child_info']['children'] @@ -374,24 +380,26 @@ def test_json_responses(self): self.assertEqual(first_child_response['category'], 'chapter') self.assertEqual(first_child_response['id'], unicode(self.chapter.location)) self.assertEqual(first_child_response['display_name'], 'Week 1') - self.assertTrue(json_response['published']) - self.assertEqual(first_child_response['visibility_state'], VisibilityState.unscheduled) + if not is_concise: + self.assertTrue(json_response['published']) + self.assertEqual(first_child_response['visibility_state'], VisibilityState.unscheduled) self.assertGreater(len(first_child_response['child_info']['children']), 0) # Finally, validate the entire response for consistency - self.assert_correct_json_response(json_response) + self.assert_correct_json_response(json_response, is_concise) - def assert_correct_json_response(self, json_response): + def assert_correct_json_response(self, json_response, is_concise=False): """ Asserts that the JSON response is syntactically consistent """ self.assertIsNotNone(json_response['display_name']) self.assertIsNotNone(json_response['id']) self.assertIsNotNone(json_response['category']) - self.assertTrue(json_response['published']) + if not is_concise: + self.assertTrue(json_response['published']) if json_response.get('child_info', None): for child_response in json_response['child_info']['children']: - self.assert_correct_json_response(child_response) + self.assert_correct_json_response(child_response, is_concise) def test_course_outline_initial_state(self): course_module = modulestore().get_item(self.course.location) diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 93637793b985..0df8ae36e247 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -20,7 +20,8 @@ ) from contentstore.views.item import ( - create_xblock_info, ALWAYS, VisibilityState, _xblock_type_and_display_name, add_container_page_publishing_info + create_xblock_info, _get_module_info, ALWAYS, VisibilityState, _xblock_type_and_display_name, + add_container_page_publishing_info ) from contentstore.tests.utils import CourseTestCase from student.tests.factories import UserFactory @@ -384,6 +385,59 @@ def test_get_user_partitions_and_groups(self): ]) self.assertEqual(result["group_access"], {}) + @ddt.data('ancestorInfo', '') + def test_ancestor_info(self, field_type): + """ + Test that we get correct ancestor info. + + Arguments: + field_type (string): If field_type=ancestorInfo, fetch ancestor info of the XBlock otherwise not. + """ + + # Create a parent chapter + chap1 = self.create_xblock(parent_usage_key=self.course.location, display_name='chapter1', category='chapter') + chapter_usage_key = self.response_usage_key(chap1) + + # create a sequential + seq1 = self.create_xblock(parent_usage_key=chapter_usage_key, display_name='seq1', category='sequential') + seq_usage_key = self.response_usage_key(seq1) + + # create a vertical + vert1 = self.create_xblock(parent_usage_key=seq_usage_key, display_name='vertical1', category='vertical') + vert_usage_key = self.response_usage_key(vert1) + + # create problem and an html component + problem1 = self.create_xblock(parent_usage_key=vert_usage_key, display_name='problem1', category='problem') + problem_usage_key = self.response_usage_key(problem1) + + def assert_xblock_info(xblock, xblock_info): + """ + Assert we have correct xblock info. + + Arguments: + xblock (XBlock): An XBlock item. + xblock_info (dict): A dict containing xblock information. + """ + self.assertEqual(unicode(xblock.location), xblock_info['id']) + self.assertEqual(xblock.display_name, xblock_info['display_name']) + self.assertEqual(xblock.category, xblock_info['category']) + + for usage_key in (problem_usage_key, vert_usage_key, seq_usage_key, chapter_usage_key): + xblock = self.get_item_from_modulestore(usage_key) + url = reverse_usage_url('xblock_handler', usage_key) + '?fields={field_type}'.format(field_type=field_type) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + response = json.loads(response.content) + if field_type == 'ancestorInfo': + self.assertIn('ancestors', response) + for ancestor_info in response['ancestors']: + parent_xblock = xblock.get_parent() + assert_xblock_info(parent_xblock, ancestor_info) + xblock = parent_xblock + else: + self.assertNotIn('ancestors', response) + self.assertEqual(_get_module_info(xblock), response) + @ddt.ddt class DeleteItem(ItemTest): From 52f8c9763a60922f541b554dc1c14decab362aad Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Wed, 28 Dec 2016 18:06:23 +0500 Subject: [PATCH 02/12] Move an item - TNL-6064 --- cms/djangoapps/contentstore/views/item.py | 128 +++++++++- .../contentstore/views/tests/test_item.py | 232 +++++++++++++++++- 2 files changed, 353 insertions(+), 7 deletions(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 36a2834cb2d2..ecf6d4d44440 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -19,6 +19,8 @@ from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryUsageLocator from pytz import UTC + +from xblock.core import XBlock from xblock.fields import Scope from xblock.fragment import Fragment from xblock_django.user_service import DjangoXBlockUserService @@ -127,8 +129,11 @@ def xblock_handler(request, usage_key_string): if usage_key_string is not specified, create a new xblock instance, either by duplicating an existing xblock, or creating an entirely new one. The json playload can contain these fields: - :parent_locator: parent for new xblock, required for both duplicate and create new instance + :parent_locator: parent for new xblock, required for duplicate, move and create new instance :duplicate_source_locator: if present, use this as the source for creating a duplicate copy + :move_source_locator: if present, use this as the source item for moving + :target_index: if present, use this as the target index for moving an item to a particular index + otherwise target_index is calculated. It is sent back in the response. :category: type of xblock, required if duplicate_source_locator is not present. :display_name: name for new xblock, optional :boilerplate: template name for populating fields, optional and only used @@ -198,14 +203,26 @@ def xblock_handler(request, usage_key_string): request.user, request.json.get('display_name'), ) - - return JsonResponse({"locator": unicode(dest_usage_key), "courseKey": unicode(dest_usage_key.course_key)}) + return JsonResponse({'locator': unicode(dest_usage_key), 'courseKey': unicode(dest_usage_key.course_key)}) else: return _create_item(request) + elif request.method == 'PATCH': + if 'move_source_locator' in request.json: + move_source_usage_key = usage_key_with_run(request.json.get('move_source_locator')) + target_parent_usage_key = usage_key_with_run(request.json.get('parent_locator')) + target_index = request.json.get('target_index') + if ( + not has_studio_write_access(request.user, target_parent_usage_key.course_key) or + not has_studio_read_access(request.user, target_parent_usage_key.course_key) + ): + raise PermissionDenied() + return _move_item(move_source_usage_key, target_parent_usage_key, request.user, target_index) + + return JsonResponse({'error': 'Patch request did not recognise any parameters to handle.'}, status=400) else: return HttpResponseBadRequest( - "Only instance creation is supported without a usage key.", - content_type="text/plain" + 'Only instance creation is supported without a usage key.', + content_type='text/plain' ) @@ -636,10 +653,109 @@ def _create_item(request): ) return JsonResponse( - {"locator": unicode(created_block.location), "courseKey": unicode(created_block.location.course_key)} + {'locator': unicode(created_block.location), 'courseKey': unicode(created_block.location.course_key)} ) +def _get_source_index(source_usage_key, source_parent): + """ + Get source index position of the XBlock. + + Arguments: + source_usage_key (BlockUsageLocator): Locator of source item. + source_parent (XBlock): A parent of the source XBlock. + + Returns: + source_index (int): Index position of the xblock in a parent. + """ + try: + source_index = source_parent.children.index(source_usage_key) + return source_index + except ValueError: + return None + + +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. + + Arguments: + source_usage_key (BlockUsageLocator): Locator of source item. + target_parent_usage_key (BlockUsageLocator): Locator of target parent. + target_index (int): If provided, insert source item at provided index location in target_parent_usage_key item. + + Returns: + 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)) + + store = modulestore() + with store.bulk_operations(source_usage_key.course_key): + source_item = store.get_item(source_usage_key) + source_parent = source_item.get_parent() + target_parent = store.get_item(target_parent_usage_key) + source_type = source_item.category + target_parent_type = target_parent.category + error = None + + # Store actual/initial index of the source item. This would be sent back with response, + # so that with Undo operation, it would easier to move back item to it's original/old index. + 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: + 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_index is None: + error = '{source_usage_key} not found in {parent_usage_key}.'.format( + source_usage_key=unicode(source_usage_key), + parent_usage_key=unicode(source_parent.location) + ) + else: + try: + target_index = int(target_index) if target_index is not None else None + if len(target_parent.children) < target_index: + error = 'You can not move {source_usage_key} at an invalid index ({target_index}).'.format( + source_usage_key=unicode(source_usage_key), + target_index=target_index + ) + except ValueError: + error = 'You must provide target_index ({target_index}) as an integer.'.format( + target_index=target_index + ) + if error: + return JsonResponse({'error': error}, status=400) + + # Remove reference from old parent. + source_parent.children.remove(source_item.location) + store.update_item(source_parent, user.id) + + # When target_index is provided, insert xblock at target_index position, otherwise insert at the end. + insert_at = target_index if target_index is not None else len(target_parent.children) + + # Add to new parent at particular location. + target_parent.children.insert(insert_at, source_item.location) + store.update_item(target_parent, user.id) + + context = { + 'move_source_locator': unicode(source_usage_key), + 'parent_locator': unicode(target_parent_usage_key), + 'source_index': target_index if target_index is not None else source_index + } + return JsonResponse(context) + + def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_name=None, is_child=False): """ Duplicate an existing xblock as a child of the supplied parent_usage_key. diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 0df8ae36e247..f5e7e59fcbc7 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -14,13 +14,14 @@ from django.core.urlresolvers import reverse from contentstore.utils import reverse_usage_url, reverse_course_url +from opaque_keys import InvalidKeyError from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration from contentstore.views.component import ( component_handler, get_component_templates ) from contentstore.views.item import ( - create_xblock_info, _get_module_info, ALWAYS, VisibilityState, _xblock_type_and_display_name, + create_xblock_info, _get_source_index, _get_module_info, ALWAYS, VisibilityState, _xblock_type_and_display_name, add_container_page_publishing_info ) from contentstore.tests.utils import CourseTestCase @@ -734,6 +735,235 @@ def verify_name(source_usage_key, parent_usage_key, expected_name, display_name= verify_name(self.seq_usage_key, self.chapter_usage_key, "customized name", display_name="customized name") +class TestMoveItem(ItemTest): + """ + Tests for move item. + """ + def setUp(self): + """ + Creates the test course structure to build course outline tree. + """ + super(TestMoveItem, self).setUp() + + # 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) + + 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 + 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 + 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 + 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) + + def _move_component(self, source_usage_key, target_usage_key, target_index=None): + """ + Helper method to send move request and returns the response. + + Arguments: + source_usage_key (BlockUsageLocator): Locator of source item. + target_usage_key (BlockUsageLocator): Locator of target parent. + target_index (int): If provided, insert source item at the provided index location in target_usage_key item. + + Returns: + resp (JsonResponse): Response after the move operation is complete. + """ + data = { + 'move_source_locator': unicode(source_usage_key), + 'parent_locator': unicode(target_usage_key) + } + if target_index is not None: + data['target_index'] = target_index + + return self.client.patch( + reverse('contentstore.views.xblock_handler'), + json.dumps(data), + content_type='application/json' + ) + + def assert_move_item(self, source_usage_key, target_usage_key, target_index=None): + """ + Assert move component. + + Arguments: + source_usage_key (BlockUsageLocator): Locator of source item. + target_usage_key (BlockUsageLocator): Locator of target parent. + target_index (int): If provided, insert source item at the provided index location in target_usage_key item. + """ + parent_loc = self.store.get_parent_location(source_usage_key) + parent = self.get_item_from_modulestore(parent_loc) + source_index = _get_source_index(source_usage_key, parent) + expected_index = target_index if target_index is not None else source_index + response = self._move_component(source_usage_key, target_usage_key, target_index) + self.assertEqual(response.status_code, 200) + response = json.loads(response.content) + self.assertEqual(response['move_source_locator'], unicode(source_usage_key)) + self.assertEqual(response['parent_locator'], unicode(target_usage_key)) + self.assertEqual(response['source_index'], expected_index) + new_parent_loc = self.store.get_parent_location(source_usage_key) + self.assertEqual(new_parent_loc, target_usage_key) + self.assertNotEqual(parent_loc, new_parent_loc) + + def test_move_component(self): + """ + Test move component with different xblock types. + """ + for source_usage_key, target_usage_key in [ + (self.html_usage_key, self.vert2_usage_key), + (self.vert_usage_key, self.seq2_usage_key), + (self.seq_usage_key, self.chapter2_usage_key) + ]: + self.assert_move_item(source_usage_key, target_usage_key) + + def test_move_source_index(self): + """ + Test moving an item to a particular index. + """ + parent = self.get_item_from_modulestore(self.vert_usage_key) + children = parent.get_children() + self.assertEqual(len(children), 2) + + # Create a component within vert2. + resp = self.create_xblock(parent_usage_key=self.vert2_usage_key, display_name='html2', category='html') + html2_usage_key = self.response_usage_key(resp) + + # Move html2_usage_key inside vert_usage_key at second position. + 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(children[1].location, html2_usage_key) + + def test_move_undo(self): + """ + Test move a component and move it back (undo). + """ + # Get the initial index of the component + parent = self.get_item_from_modulestore(self.vert_usage_key) + original_index = _get_source_index(self.html_usage_key, parent) + + # Move component and verify that response contains initial index + response = self._move_component(self.html_usage_key, self.vert2_usage_key) + response = json.loads(response.content) + self.assertEquals(original_index, response['source_index']) + + # Verify that new parent has the moved component at the last index. + parent = self.get_item_from_modulestore(self.vert2_usage_key) + self.assertEqual(self.html_usage_key, parent.children[-1]) + + # Verify original and new index is different now. + source_index = _get_source_index(self.html_usage_key, parent) + self.assertNotEquals(original_index, source_index) + + # Undo Move to the original index, use the source index fetched from the response. + response = self._move_component(self.html_usage_key, self.vert_usage_key, response['source_index']) + response = json.loads(response.content) + self.assertEquals(original_index, response['source_index']) + + def test_move_large_target_index(self): + """ + Test moving an item at a large index would generate an error message. + """ + parent = self.get_item_from_modulestore(self.vert2_usage_key) + parent_children_length = len(parent.children) + response = self._move_component(self.html_usage_key, self.vert2_usage_key, parent_children_length + 10) + self.assertEqual(response.status_code, 400) + response = json.loads(response.content) + + expected_error = 'You can not move {usage_key} at an invalid index ({target_index}).'.format( + usage_key=self.html_usage_key, + target_index=parent_children_length + 10 + ) + self.assertEqual(expected_error, response['error']) + new_parent_loc = self.store.get_parent_location(self.html_usage_key) + self.assertEqual(new_parent_loc, self.vert_usage_key) + + def test_invalid_move(self): + """ + Test invalid move. + """ + parent_loc = self.store.get_parent_location(self.chapter_usage_key) + response = self._move_component(self.chapter_usage_key, self.usage_key) + self.assertEqual(response.status_code, 400) + response = json.loads(response.content) + + expected_error = 'You can not move {source_type} into {target_type}.'.format( + source_type=self.chapter_usage_key.block_type, + target_type=self.usage_key.block_type + ) + self.assertEqual(expected_error, response['error']) + new_parent_loc = self.store.get_parent_location(self.chapter_usage_key) + self.assertEqual(new_parent_loc, parent_loc) + + def test_move_current_parent(self): + """ + Test that a component can not be moved to it's current parent. + """ + parent_loc = self.store.get_parent_location(self.html_usage_key) + self.assertEqual(parent_loc, self.vert_usage_key) + response = self._move_component(self.html_usage_key, self.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 the same parent.') + self.assertEqual(self.store.get_parent_location(self.html_usage_key), parent_loc) + + def test_move_invalid_source_index(self): + """ + Test moving an item to an invalid index. + """ + target_index = 'test_index' + parent_loc = self.store.get_parent_location(self.html_usage_key) + response = self._move_component(self.html_usage_key, self.vert2_usage_key, target_index) + self.assertEqual(response.status_code, 400) + response = json.loads(response.content) + + error = 'You must provide target_index ({target_index}) as an integer.'.format(target_index=target_index) + self.assertEqual(response['error'], error) + new_parent_loc = self.store.get_parent_location(self.html_usage_key) + self.assertEqual(new_parent_loc, parent_loc) + + def test_move_no_target_locator(self): + """ + Test move an item without specifying the target location. + """ + data = {'move_source_locator': unicode(self.html_usage_key)} + with self.assertRaises(InvalidKeyError): + self.client.patch( + reverse('contentstore.views.xblock_handler'), + json.dumps(data), + content_type='application/json' + ) + + def test_no_move_source_locator(self): + """ + Test patch request without providing a move source locator. + """ + response = self.client.patch( + reverse('contentstore.views.xblock_handler') + ) + self.assertEqual(response.status_code, 400) + response = json.loads(response.content) + self.assertEqual(response['error'], 'Patch request did not recognise any parameters to handle.') + + class TestDuplicateItemWithAsides(ItemTest, DuplicateHelper): """ Test the duplicate method for blocks with asides. From 1b011af7e2eddba700aca4e629660613cd956ea6 Mon Sep 17 00:00:00 2001 From: muhammad-ammar Date: Wed, 4 Jan 2017 11:02:18 +0500 Subject: [PATCH 03/12] move dialog TNL-6047 --- .../component_settings_editor_helpers.py | 4 +- cms/djangoapps/contentstore/features/pages.py | 2 +- .../contentstore/views/tests/utils.py | 7 +- cms/static/cms/js/spec/main.js | 1 + cms/static/js/spec/views/container_spec.js | 4 +- .../js/spec/views/modals/move_xblock_spec.js | 36 ++++++++++ .../js/spec/views/pages/container_spec.js | 22 ++++++- cms/static/js/views/modals/base_modal.js | 28 +++++--- .../js/views/modals/course_outline_modals.js | 2 +- cms/static/js/views/modals/edit_xblock.js | 34 ++-------- .../js/views/modals/move_xblock_modal.js | 53 +++++++++++++++ cms/static/js/views/pages/container.js | 20 ++++-- cms/static/js/views/utils/xblock_utils.js | 47 +++++++++---- cms/static/sass/elements/_controls.scss | 12 +++- cms/templates/component.html | 18 +++-- .../js/mock/mock-container-xblock.underscore | 66 ++++++++++++------- cms/templates/js/move-xblock-modal.underscore | 4 ++ cms/templates/studio_xblock_wrapper.html | 25 ++++--- .../common/js/components/views/feedback.js | 9 +-- .../test/acceptance/pages/studio/container.py | 8 +-- 20 files changed, 290 insertions(+), 112 deletions(-) create mode 100644 cms/static/js/spec/views/modals/move_xblock_spec.js create mode 100644 cms/static/js/views/modals/move_xblock_modal.js create mode 100644 cms/templates/js/move-xblock-modal.underscore diff --git a/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py b/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py index 6f572e9ae614..0a342f08e8d8 100644 --- a/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py +++ b/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py @@ -129,8 +129,8 @@ def edit_component(index=0): # Verify that the "loading" indication has been hidden. world.wait_for_loading() # Verify that the "edit" button is present. - world.wait_for(lambda _driver: world.css_visible('a.edit-button')) - world.css_click('a.edit-button', index) + world.wait_for(lambda _driver: world.css_visible('.edit-button')) + world.css_click('.edit-button', index) world.wait_for_ajax_complete() diff --git a/cms/djangoapps/contentstore/features/pages.py b/cms/djangoapps/contentstore/features/pages.py index bb3e113b643e..f16aec9d6070 100644 --- a/cms/djangoapps/contentstore/features/pages.py +++ b/cms/djangoapps/contentstore/features/pages.py @@ -38,7 +38,7 @@ def not_see_any_static_pages(step): @step(u'I "(edit|delete)" the static page$') def click_edit_or_delete(step, edit_or_delete): - button_css = 'ul.component-actions a.%s-button' % edit_or_delete + button_css = 'ul.component-actions .%s-button' % edit_or_delete world.css_click(button_css) diff --git a/cms/djangoapps/contentstore/views/tests/utils.py b/cms/djangoapps/contentstore/views/tests/utils.py index 094a789214be..00358d005fcf 100644 --- a/cms/djangoapps/contentstore/views/tests/utils.py +++ b/cms/djangoapps/contentstore/views/tests/utils.py @@ -54,9 +54,10 @@ def validate_preview_html(self, xblock, view_name, can_add=True): # Verify that there are no action buttons for public blocks expected_button_html = [ - '', - '', - '' + '
  • - + +
    • +
  • +
  • - +
    • diff --git a/cms/templates/js/mock/mock-container-xblock.underscore b/cms/templates/js/mock/mock-container-xblock.underscore index 606f1ff03766..e66ca58701dc 100644 --- a/cms/templates/js/mock/mock-container-xblock.underscore +++ b/cms/templates/js/mock/mock-container-xblock.underscore @@ -44,16 +44,19 @@
    • - +
    • - +
    • - + +
      • +
    • +
    • - +
    • @@ -72,16 +75,19 @@
      • - +
      • - +
      • - + +
        • +
      • +
      • - +
      • @@ -100,16 +106,19 @@
        • - +
        • - +
        • - + +
          • +
        • +
        • - +
        • @@ -158,16 +167,19 @@
          • - +
          • - +
          • - + +
            • +
          • +
          • - +
          • @@ -186,16 +198,19 @@
            • - +
            • - +
            • - + +
              • +
            • +
            • - +
            • @@ -214,16 +229,19 @@
              • - +
              • - +
              • - + +
                • +
              • +
              • - +
              • diff --git a/cms/templates/js/move-xblock-modal.underscore b/cms/templates/js/move-xblock-modal.underscore new file mode 100644 index 000000000000..468c1893ba9b --- /dev/null +++ b/cms/templates/js/move-xblock-modal.underscore @@ -0,0 +1,4 @@ +
                +
                +
                +
                diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index 5a0cdf47dafc..25842d1d0724 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -69,35 +69,42 @@ % if can_edit: % if not show_inline:
              • - +
                • % if can_edit_visibility:
              • - +
                • % endif % if can_add:
              • - + +
                • + +
              • +
                • % endif % endif % if can_add:
              • - +
                • % endif % if is_reorderable: @@ -149,7 +156,7 @@ ${content}
              % endif - % endif + % endif % if not is_root: diff --git a/common/static/common/js/components/views/feedback.js b/common/static/common/js/components/views/feedback.js index 0e8b59ac2f27..901aaa61fafd 100644 --- a/common/static/common/js/components/views/feedback.js +++ b/common/static/common/js/components/views/feedback.js @@ -78,16 +78,17 @@ return this; }, - inFocus: function() { + inFocus: function(wrapperElementSelector) { + var wrapper = wrapperElementSelector || '.wrapper', + tabbables; this.options.outFocusElement = this.options.outFocusElement || document.activeElement; // Set focus to the container. - this.$('.wrapper').first().focus(); - + this.$(wrapper).first().focus(); // Make tabs within the prompt loop rather than setting focus // back to the main content of the page. - var tabbables = this.$(tabbable_elements.join()); + tabbables = this.$(tabbable_elements.join()); tabbables.on('keydown', function(event) { // On tab backward from the first tabbable item in the prompt if (event.which === 9 && event.shiftKey && event.target === tabbables.first()[0]) { diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index fc71890d55c4..644f926ef8f3 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -243,7 +243,7 @@ def duplicate(self, source_index): """ Duplicate the item with index source_index (based on vertical placement in page). """ - click_css(self, 'a.duplicate-button', source_index) + click_css(self, '.duplicate-button', source_index) def delete(self, source_index): """ @@ -252,7 +252,7 @@ def delete(self, source_index): The index of the first item is 0. """ # Click the delete button - click_css(self, 'a.delete-button', source_index, require_notification=False) + click_css(self, '.delete-button', source_index, require_notification=False) # Click the confirmation dialog button confirm_prompt(self) @@ -451,14 +451,14 @@ def has_duplicate_button(self): """ Returns true if this xblock has a 'duplicate' button """ - return self.q(css=self._bounded_selector('a.duplicate-button')) + return self.q(css=self._bounded_selector('.duplicate-button')) @property def has_delete_button(self): """ Returns true if this xblock has a 'delete' button """ - return self.q(css=self._bounded_selector('a.delete-button')) + return self.q(css=self._bounded_selector('.delete-button')) @property def has_edit_visibility_button(self): From bfeeeff7083cb35afe5ec3c8522a900b7611a6da Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Mon, 23 Jan 2017 16:47:00 +0500 Subject: [PATCH 04/12] Fix format parameter and test --- cms/djangoapps/contentstore/views/course.py | 2 +- .../contentstore/views/tests/test_course_index.py | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 20878b839319..2971ffa9a7ba 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -336,7 +336,7 @@ def _course_outline_json(request, course_module): """ Returns a JSON representation of the course module and recursively all of its children. """ - is_concise = request.GET.get('formats') == 'concise' + is_concise = request.GET.get('format') == 'concise' include_children_predicate = lambda xblock: not xblock.category == 'vertical' if is_concise: include_children_predicate = lambda xblock: xblock.has_children diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index f2b89be6bea0..37c9792b3fc5 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -369,9 +369,8 @@ def test_json_responses(self, is_concise): self.assertEqual(json_response['category'], 'course') self.assertEqual(json_response['id'], unicode(self.course.location)) self.assertEqual(json_response['display_name'], self.course.display_name) - if not is_concise: - self.assertTrue(json_response['published']) - self.assertIsNone(json_response['visibility_state']) + self.assertNotEqual(json_response.get('published', False), is_concise) + self.assertIsNone(json_response.get('visibility_state')) # Now verify the first child children = json_response['child_info']['children'] @@ -380,8 +379,8 @@ def test_json_responses(self, is_concise): self.assertEqual(first_child_response['category'], 'chapter') self.assertEqual(first_child_response['id'], unicode(self.chapter.location)) self.assertEqual(first_child_response['display_name'], 'Week 1') + self.assertNotEqual(json_response.get('published', False), is_concise) if not is_concise: - self.assertTrue(json_response['published']) self.assertEqual(first_child_response['visibility_state'], VisibilityState.unscheduled) self.assertGreater(len(first_child_response['child_info']['children']), 0) @@ -395,8 +394,7 @@ def assert_correct_json_response(self, json_response, is_concise=False): self.assertIsNotNone(json_response['display_name']) self.assertIsNotNone(json_response['id']) self.assertIsNotNone(json_response['category']) - if not is_concise: - self.assertTrue(json_response['published']) + self.assertNotEqual(json_response.get('published', False), is_concise) if json_response.get('child_info', None): for child_response in json_response['child_info']['children']: self.assert_correct_json_response(child_response, is_concise) From c7dc83c3784c8371db88970c5e0ca2695d6fff52 Mon Sep 17 00:00:00 2001 From: muhammad-ammar Date: Mon, 23 Jan 2017 12:54:41 +0500 Subject: [PATCH 05/12] Move modal show course outline with breadcrumb TNL-6060 --- .../contentstore/views/component.py | 3 +- cms/static/cms/js/spec/main.js | 3 +- .../views/modals/move_xblock_modal_spec.js | 75 ++++ .../js/spec/views/modals/move_xblock_spec.js | 36 -- cms/static/js/spec/views/move_xblock_spec.js | 345 ++++++++++++++++++ cms/static/js/views/modals/base_modal.js | 6 +- .../js/views/modals/move_xblock_modal.js | 69 +++- cms/static/js/views/move_xblock_breadcrumb.js | 52 +++ cms/static/js/views/move_xblock_list.js | 201 ++++++++++ cms/static/js/views/pages/container.js | 3 +- cms/static/sass/elements/_controls.scss | 14 + cms/static/sass/elements/_modal-window.scss | 14 + cms/static/sass/partials/_variables.scss | 2 + cms/static/sass/views/_container.scss | 111 ++++++ cms/templates/component.html | 7 +- cms/templates/container.html | 3 +- cms/templates/js/basic-modal.underscore | 15 +- .../js/move-xblock-breadcrumb.underscore | 17 + cms/templates/js/move-xblock-list.underscore | 46 +++ cms/templates/js/move-xblock-modal.underscore | 12 +- cms/templates/studio_xblock_wrapper.html | 7 +- 21 files changed, 985 insertions(+), 56 deletions(-) create mode 100644 cms/static/js/spec/views/modals/move_xblock_modal_spec.js delete mode 100644 cms/static/js/spec/views/modals/move_xblock_spec.js create mode 100644 cms/static/js/spec/views/move_xblock_spec.js create mode 100644 cms/static/js/views/move_xblock_breadcrumb.js create mode 100644 cms/static/js/views/move_xblock_list.js create mode 100644 cms/templates/js/move-xblock-breadcrumb.underscore create mode 100644 cms/templates/js/move-xblock-list.underscore diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index c33f195a019a..378c4b2fd266 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -20,7 +20,7 @@ from xblock.plugin import PluginMissingError from xblock.runtime import Mixologist -from contentstore.utils import get_lms_link_for_item, get_xblock_aside_instance +from contentstore.utils import get_lms_link_for_item, reverse_course_url, get_xblock_aside_instance from contentstore.views.helpers import get_parent_xblock, is_unit, xblock_type_display_name from contentstore.views.item import create_xblock_info, add_container_page_publishing_info, StudioEditModuleRuntime @@ -165,6 +165,7 @@ def container_handler(request, usage_key_string): 'subsection': subsection, 'section': section, 'new_unit_category': 'vertical', + 'outline_url': '{url}?format=concise'.format(url=reverse_course_url('course_handler', course.id)), 'ancestor_xblocks': ancestor_xblocks, 'component_templates': component_templates, 'xblock_info': xblock_info, diff --git a/cms/static/cms/js/spec/main.js b/cms/static/cms/js/spec/main.js index a08508349cea..393474e1f7f1 100644 --- a/cms/static/cms/js/spec/main.js +++ b/cms/static/cms/js/spec/main.js @@ -282,8 +282,9 @@ 'js/spec/views/pages/library_users_spec', 'js/spec/views/modals/base_modal_spec', 'js/spec/views/modals/edit_xblock_spec', - 'js/spec/views/modals/move_xblock_spec', + 'js/spec/views/modals/move_xblock_modal_spec', 'js/spec/views/modals/validation_error_modal_spec', + 'js/spec/views/move_xblock_spec', 'js/spec/views/settings/main_spec', 'js/spec/factories/xblock_validation_spec', 'js/certificates/spec/models/certificate_spec', diff --git a/cms/static/js/spec/views/modals/move_xblock_modal_spec.js b/cms/static/js/spec/views/modals/move_xblock_modal_spec.js new file mode 100644 index 000000000000..28bb1792cff5 --- /dev/null +++ b/cms/static/js/spec/views/modals/move_xblock_modal_spec.js @@ -0,0 +1,75 @@ +define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', + 'common/js/spec_helpers/template_helpers', 'common/js/spec_helpers/view_helpers', + 'js/views/modals/move_xblock_modal', 'js/models/xblock_info'], + function($, _, AjaxHelpers, TemplateHelpers, ViewHelpers, MoveXBlockModal, XBlockInfo) { + 'use strict'; + describe('MoveXBlockModal', function() { + var modal, + showModal, + DISPLAY_NAME = 'HTML 101', + OUTLINE_URL = '/course/cid?format=concise', + ANCESTORS_URL = '/xblock/USAGE_ID?fields=ancestorInfo'; + + showModal = function() { + modal = new MoveXBlockModal({ + sourceXBlockInfo: new XBlockInfo({ + id: 'USAGE_ID', + display_name: DISPLAY_NAME, + category: 'html' + }), + XBlockURLRoot: '/xblock', + outlineURL: OUTLINE_URL, + XBlockAncestorInfoURL: ANCESTORS_URL + + }); + modal.show(); + }; + + beforeEach(function() { + setFixtures('
              '); + TemplateHelpers.installTemplates([ + 'basic-modal', + 'modal-button', + 'move-xblock-modal' + ]); + }); + + afterEach(function() { + modal.hide(); + }); + + it('rendered as expected', function() { + showModal(); + expect( + modal.$el.find('.modal-header .title').contents().get(0).nodeValue.trim() + ).toEqual('Move: ' + DISPLAY_NAME); + expect( + modal.$el.find('.modal-sr-title').text().trim() + ).toEqual('Choose a location to move your component to'); + expect(modal.$el.find('.modal-actions .action-primary.action-move').text()).toEqual('Move'); + }); + + it('sends request to fetch course outline', function() { + var requests = AjaxHelpers.requests(this), + renderViewsSpy; + showModal(); + expect(modal.$el.find('.ui-loading.is-hidden')).not.toExist(); + renderViewsSpy = spyOn(modal, 'renderViews'); + expect(requests.length).toEqual(2); + AjaxHelpers.expectRequest(requests, 'GET', OUTLINE_URL); + AjaxHelpers.respondWithJson(requests, {}); + AjaxHelpers.expectRequest(requests, 'GET', ANCESTORS_URL); + AjaxHelpers.respondWithJson(requests, {}); + expect(renderViewsSpy).toHaveBeenCalled(); + expect(modal.$el.find('.ui-loading.is-hidden')).toExist(); + }); + + it('shows error notification when fetch course outline request fails', function() { + var requests = AjaxHelpers.requests(this), + notificationSpy = ViewHelpers.createNotificationSpy('Error'); + showModal(); + AjaxHelpers.respondWithError(requests); + ViewHelpers.verifyNotificationShowing(notificationSpy, "Studio's having trouble saving your work"); + }); + }); + }); diff --git a/cms/static/js/spec/views/modals/move_xblock_spec.js b/cms/static/js/spec/views/modals/move_xblock_spec.js deleted file mode 100644 index 61cb9505bb16..000000000000 --- a/cms/static/js/spec/views/modals/move_xblock_spec.js +++ /dev/null @@ -1,36 +0,0 @@ -define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', - 'common/js/spec_helpers/template_helpers', 'js/views/modals/move_xblock_modal', 'js/models/xblock_info'], - function($, _, AjaxHelpers, TemplateHelpers, MoveXBlockModal, XBlockInfo) { - 'use strict'; - describe('MoveXBlockModal', function() { - var modal, - showModal, - DISPLAY_NAME = 'HTML 101'; - - showModal = function() { - modal = new MoveXBlockModal({ - sourceXBlockInfo: new XBlockInfo({ - id: 'testCourse/branch/draft/block/verticalFFF', - display_name: DISPLAY_NAME, - category: 'html' - }), - XBlockUrlRoot: '/xblock' - }); - modal.show(); - }; - - beforeEach(function() { - TemplateHelpers.installTemplates([ - 'basic-modal', - 'modal-button', - 'move-xblock-modal' - ]); - showModal(); - }); - - it('rendered as expected', function() { - expect(modal.$el.find('.modal-header .title').text()).toEqual('Move: ' + DISPLAY_NAME); - expect(modal.$el.find('.modal-actions .action-primary.action-move').text()).toEqual('Move'); - }); - }); - }); diff --git a/cms/static/js/spec/views/move_xblock_spec.js b/cms/static/js/spec/views/move_xblock_spec.js new file mode 100644 index 000000000000..2df7c7ebc810 --- /dev/null +++ b/cms/static/js/spec/views/move_xblock_spec.js @@ -0,0 +1,345 @@ +define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', + 'common/js/spec_helpers/template_helpers', 'js/views/move_xblock_list', + 'js/views/move_xblock_breadcrumb', 'js/models/xblock_info'], + function($, _, AjaxHelpers, TemplateHelpers, MoveXBlockListView, MoveXBlockBreadcrumbView, + XBlockInfoModel) { + 'use strict'; + describe('MoveXBlock', function() { + var renderViews, createXBlockInfo, createCourseOutline, moveXBlockBreadcrumbView, + moveXBlockListView, parentChildMap, categoryMap, createChildXBlockInfo, + verifyBreadcrumbViewInfo, verifyListViewInfo, getDisplayedInfo, clickForwardButton, + clickBreadcrumbButton, verifyXBlockInfo, nextCategory; + + parentChildMap = { + course: 'section', + section: 'subsection', + subsection: 'unit', + unit: 'component' + }; + + categoryMap = { + section: 'chapter', + subsection: 'sequential', + unit: 'vertical', + component: 'component' + }; + + beforeEach(function() { + setFixtures( + "
              " + ); + TemplateHelpers.installTemplates([ + 'move-xblock-list', + 'move-xblock-breadcrumb' + ]); + }); + + afterEach(function() { + moveXBlockBreadcrumbView.remove(); + moveXBlockListView.remove(); + }); + + /** + * Create child XBlock info. + * + * @param {String} category XBlock category + * @param {Object} outlineOptions options according to which outline was created + * @param {Object} xblockIndex XBlock Index + * @returns + */ + createChildXBlockInfo = function(category, outlineOptions, xblockIndex) { + var childInfo = { + category: categoryMap[category], + display_name: category + '_display_name_' + xblockIndex, + id: category + '_ID' + }; + return createXBlockInfo(parentChildMap[category], outlineOptions, childInfo); + }; + + /** + * Create parent XBlock info. + * + * @param {String} category XBlock category + * @param {Object} outlineOptions options according to which outline was created + * @param {Object} outline ouline info being constructed + * @returns {Object} + */ + createXBlockInfo = function(category, outlineOptions, outline) { + var childInfo = { + category: categoryMap[category], + display_name: category, + children: [] + }, + xblocks; + + xblocks = outlineOptions[category]; + if (!xblocks) { + return outline; + } + + outline.child_info = childInfo; // eslint-disable-line no-param-reassign + _.each(_.range(xblocks), function(xblockIndex) { + childInfo.children.push( + createChildXBlockInfo(category, outlineOptions, xblockIndex) + ); + }); + return outline; + }; + + /** + * Create course outline. + * + * @param {Object} outlineOptions options according to which outline was created + * @returns {Object} + */ + createCourseOutline = function(outlineOptions) { + var courseOutline = { + category: 'course', + display_name: 'Demo Course', + id: 'COURSE_ID_101' + }; + return createXBlockInfo('section', outlineOptions, courseOutline); + }; + + /** + * Render breadcrumb and XBlock list view. + * + * @param {any} courseOutlineInfo course outline info + * @param {any} ancestorInfo ancestors info + */ + renderViews = function(courseOutlineInfo, ancestorInfo) { + moveXBlockBreadcrumbView = new MoveXBlockBreadcrumbView({}); + moveXBlockListView = new MoveXBlockListView( + { + model: new XBlockInfoModel(courseOutlineInfo, {parse: true}), + ancestorInfo: ancestorInfo || {ancestors: []} + } + ); + }; + + /** + * Extract displayed XBlock list info. + * + * @returns {Object} + */ + getDisplayedInfo = function() { + var viewEl = moveXBlockListView.$el; + return { + categoryText: viewEl.find('.category-text').text().trim(), + currentLocationText: viewEl.find('.current-location').text().trim(), + xblockCount: viewEl.find('.xblock-item').length, + xblockDisplayNames: viewEl.find('.xblock-item .xblock-displayname').map( + function() { return $(this).text().trim(); } + ).get(), + forwardButtonSRTexts: viewEl.find('.xblock-item .forward-sr-text').map( + function() { return $(this).text().trim(); } + ).get(), + forwardButtonCount: viewEl.find('.fa-arrow-right.forward-sr-icon').length + }; + }; + + /** + * Verify displayed XBlock list info. + * + * @param {String} category XBlock category + * @param {Integer} expectedXBlocksCount number of XBlock childs displayed + * @param {Boolean} hasCurrentLocation do we need to check current location + */ + verifyListViewInfo = function(category, expectedXBlocksCount, hasCurrentLocation) { + var displayedInfo = getDisplayedInfo(); + expect(displayedInfo.categoryText).toEqual(moveXBlockListView.categoriesText[category] + ':'); + expect(displayedInfo.xblockCount).toEqual(expectedXBlocksCount); + expect(displayedInfo.xblockDisplayNames).toEqual( + _.map(_.range(expectedXBlocksCount), function(xblockIndex) { + return category + '_display_name_' + xblockIndex; + }) + ); + if (category !== 'component') { + if (hasCurrentLocation) { + expect(displayedInfo.currentLocationText).toEqual('(Current location)'); + } + expect(displayedInfo.forwardButtonSRTexts).toEqual( + _.map(_.range(expectedXBlocksCount), function() { + return 'Click for children'; + }) + ); + expect(displayedInfo.forwardButtonCount).toEqual(expectedXBlocksCount); + } + }; + + /** + * Verify rendered breadcrumb info. + * + * @param {any} category XBlock category + * @param {any} xblockIndex XBlock index + */ + verifyBreadcrumbViewInfo = function(category, xblockIndex) { + var displayedBreadcrumbs = moveXBlockBreadcrumbView.$el.find('.breadcrumbs .bc-container').map( + function() { return $(this).text().trim(); } + ).get(), + categories = _.keys(parentChildMap).concat(['component']), + visitedCategories = categories.slice(0, _.indexOf(categories, category)); + + expect(displayedBreadcrumbs).toEqual( + _.map(visitedCategories, function(visitedCategory) { + return visitedCategory === 'course' ? + 'Course Outline' : visitedCategory + '_display_name_' + xblockIndex; + }) + ); + }; + + /** + * Click forward button in the list of displayed XBlocks. + * + * @param {any} buttonIndex forward button index + */ + clickForwardButton = function(buttonIndex) { + buttonIndex = buttonIndex || 0; // eslint-disable-line no-param-reassign + moveXBlockListView.$el.find('[data-item-index="' + buttonIndex + '"] button').click(); + }; + + /** + * Click on last clickable breadcrumb button. + */ + clickBreadcrumbButton = function() { + moveXBlockBreadcrumbView.$el.find('.bc-container button').last().click(); + }; + + /** + * Returns the parent or child category of current XBlock. + * + * @param {String} direction `forward` or `backward` + * @param {String} category XBlock category + * @returns {String} + */ + nextCategory = function(direction, category) { + return direction === 'forward' ? parentChildMap[category] : _.invert(parentChildMap)[category]; + }; + + /** + * Verify renderd info of breadcrumbs and XBlock list. + * + * @param {Object} outlineOptions options according to which outline was created + * @param {String} category XBlock category + * @param {Integer} buttonIndex forward button index + * @param {String} direction `forward` or `backward` + * @param {String} hasCurrentLocation do we need to check current location + * @returns + */ + verifyXBlockInfo = function(outlineOptions, category, buttonIndex, direction, hasCurrentLocation) { + var expectedXBlocksCount = outlineOptions[category]; + + verifyListViewInfo(category, expectedXBlocksCount, hasCurrentLocation); + verifyBreadcrumbViewInfo(category, buttonIndex); + + if (direction === 'forward') { + if (category === 'component') { + return; + } + clickForwardButton(buttonIndex); + } else if (direction === 'backward') { + if (category === 'section') { + return; + } + clickBreadcrumbButton(); + } + category = nextCategory(direction, category); // eslint-disable-line no-param-reassign + + verifyXBlockInfo(outlineOptions, category, buttonIndex, direction, hasCurrentLocation); + }; + + it('renders views with correct information', function() { + var outlineOptions = {section: 1, subsection: 1, unit: 1, component: 1}, + outline = createCourseOutline(outlineOptions); + + renderViews(outline); + verifyXBlockInfo(outlineOptions, 'section', 0, 'forward', false); + verifyXBlockInfo(outlineOptions, 'component', 0, 'backward', false); + }); + + it('shows correct behavior on breadcrumb navigation', function() { + var outline = createCourseOutline({section: 1, subsection: 1, unit: 1, component: 1}); + + renderViews(outline); + _.each(_.range(3), function() { + clickForwardButton(); + }); + + _.each(['component', 'unit', 'subsection', 'section'], function(category) { + verifyListViewInfo(category, 1); + if (category !== 'section') { + moveXBlockBreadcrumbView.$el.find('.bc-container button').last().click(); + } + }); + }); + + it('shows the correct current location', function() { + var outlineOptions = {section: 2, subsection: 2, unit: 2, component: 2}, + outline = createCourseOutline(outlineOptions), + ancestorInfo = { + ancestors: [ + { + category: 'vertical', + display_name: 'unit_display_name_0', + id: 'unit_ID' + }, + { + category: 'sequential', + display_name: 'subsection_display_name_0', + id: 'subsection_ID' + }, + { + category: 'chapter', + display_name: 'section_display_name_0', + id: 'section_ID' + }, + { + category: 'course', + display_name: 'Demo Course', + id: 'COURSE_ID_101' + } + ] + }; + + renderViews(outline, ancestorInfo); + verifyXBlockInfo(outlineOptions, 'section', 0, 'forward', true); + // click the outline breadcrumb to render sections + moveXBlockBreadcrumbView.$el.find('.bc-container button').first().click(); + verifyXBlockInfo(outlineOptions, 'section', 1, 'forward', false); + }); + + it('shows correct message when parent has no children', function() { + var outlinesInfo = [ + { + outline: createCourseOutline({}), + message: 'This course has no sections' + }, + { + outline: createCourseOutline({section: 1}), + message: 'This section has no subsections', + forwardClicks: 1 + }, + { + outline: createCourseOutline({section: 1, subsection: 1}), + message: 'This subsection has no units', + forwardClicks: 2 + }, + { + outline: createCourseOutline({section: 1, subsection: 1, unit: 1}), + message: 'This unit has no components', + forwardClicks: 3 + } + ]; + + _.each(outlinesInfo, function(info) { + renderViews(info.outline); + _.each(_.range(info.forwardClicks), function() { + clickForwardButton(); + }); + expect(moveXBlockListView.$el.find('.xblock-no-child-message').text().trim()).toEqual(info.message); + moveXBlockListView.undelegateEvents(); + moveXBlockBreadcrumbView.undelegateEvents(); + }); + }); + }); + }); diff --git a/cms/static/js/views/modals/base_modal.js b/cms/static/js/views/modals/base_modal.js index bd6f2981ff64..2974a7720a45 100644 --- a/cms/static/js/views/modals/base_modal.js +++ b/cms/static/js/views/modals/base_modal.js @@ -20,6 +20,7 @@ * button on the modal. * primaryActionButtonType: A string to be used as type for primary action button. * primaryActionButtonTitle: A string to be used as title for primary action button. + * showEditorModeButtons: Whether to show editor mode button in the modal header. */ define(['jquery', 'underscore', 'gettext', 'js/views/baseview'], function($, _, gettext, BaseView) { @@ -41,7 +42,8 @@ define(['jquery', 'underscore', 'gettext', 'js/views/baseview'], viewSpecificClasses: '', addPrimaryActionButton: false, primaryActionButtonType: 'save', - primaryActionButtonTitle: gettext('Save') + primaryActionButtonTitle: gettext('Save'), + showEditorModeButtons: true }), initialize: function() { @@ -66,6 +68,8 @@ define(['jquery', 'underscore', 'gettext', 'js/views/baseview'], type: this.options.modalType, size: this.options.modalSize, title: this.getTitle(), + modalSRTitle: this.options.modalSRTitle, + showEditorModeButtons: this.options.showEditorModeButtons, viewSpecificClasses: this.options.viewSpecificClasses })); this.addActionButtons(); diff --git a/cms/static/js/views/modals/move_xblock_modal.js b/cms/static/js/views/modals/move_xblock_modal.js index 92f1363a363d..8efabd6a4371 100644 --- a/cms/static/js/views/modals/move_xblock_modal.js +++ b/cms/static/js/views/modals/move_xblock_modal.js @@ -4,27 +4,44 @@ define([ 'jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', 'js/views/modals/base_modal', + 'js/models/xblock_info', 'js/views/move_xblock_list', 'js/views/move_xblock_breadcrumb', 'common/js/components/views/feedback', 'edx-ui-toolkit/js/utils/string-utils', 'text!templates/move-xblock-modal.underscore' ], -function($, Backbone, _, gettext, BaseView, BaseModal, Feedback, StringUtils, MoveXblockModalTemplate) { +function($, Backbone, _, gettext, BaseView, BaseModal, XBlockInfoModel, MoveXBlockListView, MoveXBlockBreadcrumbView, + Feedback, StringUtils, MoveXblockModalTemplate) { 'use strict'; var MoveXblockModal = BaseModal.extend({ options: $.extend({}, BaseModal.prototype.options, { modalName: 'move-xblock', - modalSize: 'med', + modalSize: 'lg', + showEditorModeButtons: false, addPrimaryActionButton: true, primaryActionButtonType: 'move', - primaryActionButtonTitle: gettext('Move') + viewSpecificClasses: 'move-modal', + primaryActionButtonTitle: gettext('Move'), + modalSRTitle: gettext('Choose a location to move your component to') }), initialize: function() { + var self = this; BaseModal.prototype.initialize.call(this); + this.listenTo(Backbone, 'move:breadcrumbRendered', this.focusModal); this.sourceXBlockInfo = this.options.sourceXBlockInfo; - this.XBlockUrlRoot = this.options.sourceXBlockInfo; + this.XBlockURLRoot = this.options.XBlockURLRoot; + this.XBlockAncestorInfoURL = StringUtils.interpolate( + '{urlRoot}/{usageId}?fields=ancestorInfo', + {urlRoot: this.XBlockURLRoot, usageId: this.sourceXBlockInfo.get('id')} + ); + this.outlineURL = this.options.outlineURL; this.options.title = this.getTitle(); + this.fetchCourseOutline().done(function(courseOutlineInfo, ancestorInfo) { + $('.ui-loading').addClass('is-hidden'); + $('.breadcrumb-container').removeClass('is-hidden'); + self.renderViews(courseOutlineInfo, ancestorInfo); + }); }, getTitle: function() { @@ -40,12 +57,54 @@ function($, Backbone, _, gettext, BaseView, BaseModal, Feedback, StringUtils, Mo show: function() { BaseModal.prototype.show.apply(this, [false]); - Feedback.prototype.inFocus.apply(this, [this.options.modalWindowClass]); }, hide: function() { + if (this.moveXBlockListView) { + this.moveXBlockListView.remove(); + } + if (this.moveXBlockBreadcrumbView) { + this.moveXBlockBreadcrumbView.remove(); + } BaseModal.prototype.hide.apply(this); Feedback.prototype.outFocus.apply(this); + }, + + focusModal: function() { + Feedback.prototype.inFocus.apply(this, [this.options.modalWindowClass]); + $(this.options.modalWindowClass).focus(); + }, + + fetchCourseOutline: function() { + return $.when( + this.fetchData(this.outlineURL), + this.fetchData(this.XBlockAncestorInfoURL) + ); + }, + + fetchData: function(url) { + var deferred = $.Deferred(); + $.ajax({ + url: url, + contentType: 'application/json', + dataType: 'json', + type: 'GET' + }).done(function(data) { + deferred.resolve(data); + }).fail(function() { + deferred.reject(); + }); + return deferred.promise(); + }, + + renderViews: function(courseOutlineInfo, ancestorInfo) { + this.moveXBlockBreadcrumbView = new MoveXBlockBreadcrumbView({}); + this.moveXBlockListView = new MoveXBlockListView( + { + model: new XBlockInfoModel(courseOutlineInfo, {parse: true}), + ancestorInfo: ancestorInfo + } + ); } }); diff --git a/cms/static/js/views/move_xblock_breadcrumb.js b/cms/static/js/views/move_xblock_breadcrumb.js new file mode 100644 index 000000000000..2d891b3c5c34 --- /dev/null +++ b/cms/static/js/views/move_xblock_breadcrumb.js @@ -0,0 +1,52 @@ +/** + * MoveXBlockBreadcrumb show breadcrumbs to move back to parent. + */ +define([ + 'jquery', 'backbone', 'underscore', 'gettext', + 'edx-ui-toolkit/js/utils/html-utils', + 'edx-ui-toolkit/js/utils/string-utils', + 'text!templates/move-xblock-breadcrumb.underscore' +], +function($, Backbone, _, gettext, HtmlUtils, StringUtils, MoveXBlockBreadcrumbViewTemplate) { + 'use strict'; + + var MoveXBlockBreadcrumb = Backbone.View.extend({ + el: '.breadcrumb-container', + + defaultRenderOptions: { + breadcrumbs: ['Course Outline'] + }, + + events: { + 'click .parent-nav-button': 'handleBreadcrumbButtonPress' + }, + + initialize: function() { + this.template = HtmlUtils.template(MoveXBlockBreadcrumbViewTemplate); + this.listenTo(Backbone, 'move:childrenRendered', this.render); + }, + + render: function(options) { + HtmlUtils.setHtml( + this.$el, + this.template(_.extend({}, this.defaultRenderOptions, options)) + ); + Backbone.trigger('move:breadcrumbRendered'); + return this; + }, + + /** + * Event handler for breadcrumb button press. + * + * @param {Object} event + */ + handleBreadcrumbButtonPress: function(event) { + Backbone.trigger( + 'move:breadcrumbButtonPressed', + $(event.target).data('parentIndex') + ); + } + }); + + return MoveXBlockBreadcrumb; +}); diff --git a/cms/static/js/views/move_xblock_list.js b/cms/static/js/views/move_xblock_list.js new file mode 100644 index 000000000000..4e302de97f93 --- /dev/null +++ b/cms/static/js/views/move_xblock_list.js @@ -0,0 +1,201 @@ +/** + * XBlockListView shows list of XBlocks in a particular category(section, subsection, vertical etc). + */ +define([ + 'jquery', 'backbone', 'underscore', 'gettext', + 'edx-ui-toolkit/js/utils/html-utils', + 'edx-ui-toolkit/js/utils/string-utils', + 'js/views/utils/xblock_utils', + 'text!templates/move-xblock-list.underscore' +], +function($, Backbone, _, gettext, HtmlUtils, StringUtils, XBlockUtils, MoveXBlockListViewTemplate) { + 'use strict'; + + var XBlockListView = Backbone.View.extend({ + el: '.xblock-list-container', + + // parent info of currently displayed children + parentInfo: {}, + // currently displayed children XBlocks info + childrenInfo: {}, + // list of visited parent XBlocks, needed for backward navigation + visitedAncestors: null, + + // parent to child relation map + categoryRelationMap: { + course: 'section', + section: 'subsection', + subsection: 'unit', + unit: 'component' + }, + + categoriesText: { + section: gettext('Sections'), + subsection: gettext('Subsections'), + unit: gettext('Units'), + component: gettext('Components') + }, + + events: { + 'click .button-forward': 'renderChildren' + }, + + initialize: function(options) { + this.visitedAncestors = []; + this.template = HtmlUtils.template(MoveXBlockListViewTemplate); + this.ancestorInfo = options.ancestorInfo; + this.listenTo(Backbone, 'move:breadcrumbButtonPressed', this.handleBreadcrumbButtonPress); + this.renderXBlockInfo(); + }, + + render: function() { + HtmlUtils.setHtml( + this.$el, + this.template( + { + xblocks: this.childrenInfo.children, + noChildText: this.getNoChildText(), + categoryText: this.getCategoryText(), + parentDisplayname: this.parentInfo.parent.get('display_name'), + XBlocksCategory: this.childrenInfo.category, + currentLocationIndex: this.getCurrentLocationIndex() + } + ) + ); + Backbone.trigger('move:childrenRendered', this.breadcrumbInfo()); + return this; + }, + + /** + * Forward button press handler. This will render all the childs of an XBlock. + * + * @param {Object} event + */ + renderChildren: function(event) { + this.renderXBlockInfo( + 'forward', + $(event.target).closest('.xblock-item').data('itemIndex') + ); + }, + + /** + * Breadcrumb button press event handler. Render all the childs of an XBlock. + * + * @param {any} newParentIndex Index of a parent XBlock + */ + handleBreadcrumbButtonPress: function(newParentIndex) { + this.renderXBlockInfo('backward', newParentIndex); + }, + + /** + * Render XBlocks based on `forward` or `backward` navigation. + * + * @param {any} direction `forward` or `backward` + * @param {any} newParentIndex Index of a parent XBlock + */ + renderXBlockInfo: function(direction, newParentIndex) { + if (direction === undefined) { + this.parentInfo.parent = this.model; + } else if (direction === 'forward') { + // clicked child is the new parent + this.parentInfo.parent = this.childrenInfo.children[newParentIndex]; + } else if (direction === 'backward') { + // new parent will be one of visitedAncestors + this.parentInfo.parent = this.visitedAncestors[newParentIndex]; + // remove visited ancestors + this.visitedAncestors.splice(newParentIndex); + } + + this.visitedAncestors.push(this.parentInfo.parent); + + if (this.parentInfo.parent.get('child_info')) { + this.childrenInfo.children = this.parentInfo.parent.get('child_info').children; + } else { + this.childrenInfo.children = []; + } + + this.setDisplayedXBlocksCategories(); + this.render(); + }, + + /** + * Set parent and child XBlock categories. + */ + setDisplayedXBlocksCategories: function() { + this.parentInfo.category = XBlockUtils.getXBlockType( + this.parentInfo.parent.get('category'), + this.visitedAncestors[this.visitedAncestors.length - 2] + ); + this.childrenInfo.category = this.categoryRelationMap[this.parentInfo.category]; + }, + + /** + * Get index of source XBlock. + * + * @returns {any} Integer or undefined + */ + getCurrentLocationIndex: function() { + var category, ancestorXBlock, currentLocationIndex; + + if (this.childrenInfo.category === 'component' || this.childrenInfo.children.length === 0) { + return currentLocationIndex; + } + + category = this.childrenInfo.children[0].get('category'); + ancestorXBlock = _.find( + this.ancestorInfo.ancestors, function(ancestor) { return ancestor.category === category; } + ); + + if (ancestorXBlock) { + _.each(this.childrenInfo.children, function(xblock, index) { + if (ancestorXBlock.display_name === xblock.get('display_name') && + ancestorXBlock.id === xblock.get('id')) { + currentLocationIndex = index; + } + }); + } + + return currentLocationIndex; + }, + + /** + * Get category text for currently displayed children. + * + * @returns {String} + */ + getCategoryText: function() { + return this.categoriesText[this.childrenInfo.category]; + }, + + /** + * Get text when a parent XBlock has no children. + * + * @returns {String} + */ + getNoChildText: function() { + return StringUtils.interpolate( + gettext('This {parentCategory} has no {childCategory}'), + { + parentCategory: this.parentInfo.category, + childCategory: this.categoriesText[this.childrenInfo.category].toLowerCase() + } + ); + }, + + /** + * Construct breadcurmb info. + * + * @returns {Object} + */ + breadcrumbInfo: function() { + return { + breadcrumbs: _.map(this.visitedAncestors, function(ancestor) { + return ancestor.get('category') === 'course' ? + gettext('Course Outline') : ancestor.get('display_name'); + }) + }; + } + }); + + return XBlockListView; +}); diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 1a476bcde458..05f6409deb71 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -196,7 +196,8 @@ define(['jquery', 'underscore', 'gettext', 'js/views/pages/base_page', 'common/j var xblockElement = this.findXBlockElement(event.target), modal = new MoveXBlockModal({ sourceXBlockInfo: XBlockUtils.findXBlockInfo(xblockElement, this.model), - XBlockUrlRoot: this.getURLRoot() + XBlockURLRoot: this.getURLRoot(), + outlineURL: this.options.outlineURL }); event.preventDefault(); diff --git a/cms/static/sass/elements/_controls.scss b/cms/static/sass/elements/_controls.scss index e3afc0e16d5f..00f23ae0cd77 100644 --- a/cms/static/sass/elements/_controls.scss +++ b/cms/static/sass/elements/_controls.scss @@ -344,6 +344,20 @@ .btn-default.edit-button { font-weight: 300; } + + .stack-move-icon { + font-size: 0.52em; + + @include rtl { + .fa-file-o { + @include transform(rotateY(180deg)); + } + + .fa-arrow-right { + @include transform(rotate(180deg)); + } + } + } } } diff --git a/cms/static/sass/elements/_modal-window.scss b/cms/static/sass/elements/_modal-window.scss index 4a5ec294c957..893ac32d17c5 100644 --- a/cms/static/sass/elements/_modal-window.scss +++ b/cms/static/sass/elements/_modal-window.scss @@ -285,6 +285,20 @@ // specific modal overrides // ------------------------ + // Move XBlock Modal + .modal-window.move-modal { + top: 10% !important; + } + + .move-xblock-modal { + .modal-content { + padding: ($baseline/2) ($baseline/2) ($baseline*1.25) ($baseline/2); + } + .ui-loading { + box-shadow: none; + } + } + // upload modal .assetupload-modal { diff --git a/cms/static/sass/partials/_variables.scss b/cms/static/sass/partials/_variables.scss index 69912569b1e0..473fc49b5c51 100644 --- a/cms/static/sass/partials/_variables.scss +++ b/cms/static/sass/partials/_variables.scss @@ -278,3 +278,5 @@ $body-line-height: golden-ratio(.875em, 1); // carried over from LMS for xmodules $action-primary-active-bg: #1AA1DE !default; // $m-blue $very-light-text: $white !default; + +$color-background-alternate: rgb(242, 248, 251) !default; diff --git a/cms/static/sass/views/_container.scss b/cms/static/sass/views/_container.scss index 3c26cf1a0b50..84a9264c5988 100644 --- a/cms/static/sass/views/_container.scss +++ b/cms/static/sass/views/_container.scss @@ -331,3 +331,114 @@ } } } + + +.move-xblock-modal { + + button { + background: transparent; + border-color: transparent; + padding: 0; + border: none; + } + + .breadcrumb-container { + margin-bottom: ($baseline/4); + border: 1px solid $btn-lms-border; + padding: ($baseline/2); + background: $color-background-alternate; + + .breadcrumbs { + + .bc-container { + @include font-size(14); + display: inline-block; + + .breadcrumb-fa-icon { + padding: 0 ($baseline/4); + + @include rtl { + @include transform(rotate(180deg)); + } + } + + &.last { + .parent-displayname { + @include font-size(18); + } + } + } + + .bc-container:not(.last) { + button, .parent-displayname { + text-decoration: underline; + color: $ui-link-color; + } + } + } + } + + .category-text { + @include margin-left($baseline/2); + @include font-size(14); + color: $black; + } + + .xblock-items-container { + max-height: ($baseline*15); + overflow-y: auto; + + .xblock-item { + & > * { + width: 100%; + color: $uxpl-blue-hover-active; + } + + .component { + display: block; + color: $black; + } + + .button-forward, .component { + border: none; + padding: ($baseline/2); + } + + .button-forward { + .xblock-displayname { + @include float(left); + } + + .forward-sr-icon { + @include float(right); + + @include rtl { + @include transform(rotate(180deg)); + } + } + + &:hover, &:focus { + background: $color-background-alternate; + } + } + } + + .xblock-no-child-message { + @include text-align(center); + display: block; + padding: ($baseline*2); + } + } + + .truncate { + max-width: 90%; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + } + + .current-location { + @include float(left); + @include margin-left($baseline); + } +} diff --git a/cms/templates/component.html b/cms/templates/component.html index 900b9d5fae7f..c42e3718984f 100644 --- a/cms/templates/component.html +++ b/cms/templates/component.html @@ -21,8 +21,11 @@
            • diff --git a/cms/templates/container.html b/cms/templates/container.html index ebabe6764e9f..a313733c3b9b 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -43,7 +43,8 @@ "${action | n, js_escaped_string}", { isUnitPage: ${is_unit_page | n, dump_js_escaped_json}, - canEdit: true + canEdit: true, + outlineURL: "${outline_url | n, js_escaped_string}" } ); }); diff --git a/cms/templates/js/basic-modal.underscore b/cms/templates/js/basic-modal.underscore index 84bdb2b339bd..4273fe4f9956 100644 --- a/cms/templates/js/basic-modal.underscore +++ b/cms/templates/js/basic-modal.underscore @@ -5,9 +5,18 @@
              - -
                -
              + + <% if (showEditorModeButtons) { %> +
                +
              + <% } %>
              diff --git a/cms/templates/js/move-xblock-breadcrumb.underscore b/cms/templates/js/move-xblock-breadcrumb.underscore new file mode 100644 index 000000000000..937219fa9923 --- /dev/null +++ b/cms/templates/js/move-xblock-breadcrumb.underscore @@ -0,0 +1,17 @@ + diff --git a/cms/templates/js/move-xblock-list.underscore b/cms/templates/js/move-xblock-list.underscore new file mode 100644 index 000000000000..02c1f5038414 --- /dev/null +++ b/cms/templates/js/move-xblock-list.underscore @@ -0,0 +1,46 @@ +
              + + <% + // Translators: message will be like `Units in Homework - Question Styles`, `Subsections in Example 1 - Getting started` etc. + %> + <%- StringUtils.interpolate( + gettext("{categoryText} in {parentDisplayname}"), + {categoryText: categoryText, parentDisplayname: parentDisplayname} + ) + %> + + +
              +
                + <% for (var i = 0; i < xblocks.length; i++) { + var xblock = xblocks[i]; + %> +
              • + <% if (XBlocksCategory === 'component') { %> + + <%- xblock.get('display_name') %> + + <% } else { %> + + <% } %> +
                • + <% } %> + <% if(xblocks.length === 0) { %> + + <%- noChildText %> + + <% } %> +
              diff --git a/cms/templates/js/move-xblock-modal.underscore b/cms/templates/js/move-xblock-modal.underscore index 468c1893ba9b..4189002fd180 100644 --- a/cms/templates/js/move-xblock-modal.underscore +++ b/cms/templates/js/move-xblock-modal.underscore @@ -1,4 +1,10 @@ -
              -
              -
              +
              +

              + + + + <%- gettext('Loading') %> +

              + +
              diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index 25842d1d0724..fad6e36e7b5b 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -92,8 +92,11 @@
              • % endif From e856f07b875b51eaff391b484901ccebecf5fb83 Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Fri, 20 Jan 2017 18:39:04 +0500 Subject: [PATCH 06/12] Move and undo move XBlock - TNL-6062 - TNL-6229 --- cms/static/cms/js/main.js | 12 +- .../views/modals/move_xblock_modal_spec.js | 224 +++++++++++++++++- .../js/views/modals/move_xblock_modal.js | 67 +++++- cms/static/js/views/pages/container.js | 4 +- .../js/views/utils/move_xblock_utils.js | 69 ++++++ cms/static/js/views/utils/xblock_utils.js | 32 ++- .../common/js/components/views/feedback.js | 2 + .../js/components/views/feedback_move.js | 49 ++++ .../components/system-feedback.underscore | 3 +- 9 files changed, 449 insertions(+), 13 deletions(-) create mode 100644 cms/static/js/views/utils/move_xblock_utils.js create mode 100644 common/static/common/js/components/views/feedback_move.js diff --git a/cms/static/cms/js/main.js b/cms/static/cms/js/main.js index 74e654241b2b..6a463521c042 100644 --- a/cms/static/cms/js/main.js +++ b/cms/static/cms/js/main.js @@ -6,7 +6,7 @@ 'common/js/components/views/feedback_notification', 'coffee/src/ajax_prefix', 'jquery.cookie'], function(domReady, $, str, Backbone, gettext, NotificationView) { - var main; + var main, sendJSON; main = function() { AjaxPrefix.addAjaxPrefix(jQuery, function() { return $("meta[name='path_prefix']").attr('content'); @@ -45,20 +45,26 @@ }); return msg.show(); }); - $.postJSON = function(url, data, callback) { + sendJSON = function(url, data, callback, type) { // eslint-disable-line no-param-reassign if ($.isFunction(data)) { callback = data; data = undefined; } return $.ajax({ url: url, - type: 'POST', + type: type, contentType: 'application/json; charset=utf-8', dataType: 'json', data: JSON.stringify(data), success: callback }); }; + $.postJSON = function(url, data, callback) { + return sendJSON(url, data, callback, 'POST'); + }; + $.patchJSON = function(url, data, callback) { + return sendJSON(url, data, callback, 'PATCH'); + }; return domReady(function() { if (window.onTouchBasedDevice()) { return $('body').addClass('touch-based-device'); diff --git a/cms/static/js/spec/views/modals/move_xblock_modal_spec.js b/cms/static/js/spec/views/modals/move_xblock_modal_spec.js index 28bb1792cff5..64a89aed4b9a 100644 --- a/cms/static/js/spec/views/modals/move_xblock_modal_spec.js +++ b/cms/static/js/spec/views/modals/move_xblock_modal_spec.js @@ -1,8 +1,24 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', 'common/js/spec_helpers/template_helpers', 'common/js/spec_helpers/view_helpers', - 'js/views/modals/move_xblock_modal', 'js/models/xblock_info'], - function($, _, AjaxHelpers, TemplateHelpers, ViewHelpers, MoveXBlockModal, XBlockInfo) { + 'js/views/modals/move_xblock_modal', 'edx-ui-toolkit/js/utils/html-utils', + 'edx-ui-toolkit/js/utils/string-utils', 'js/models/xblock_info'], + function($, _, AjaxHelpers, TemplateHelpers, ViewHelpers, MoveXBlockModal, HtmlUtils, StringUtils, XBlockInfo) { 'use strict'; + + var modal, + showModal, + verifyNotificationStatus, + selectTargetParent, + getConfirmationFeedbackTitle, + getUndoConfirmationFeedbackTitle, + getConfirmationFeedbackTitleHtml, + getConfirmationFeedbackMessageHtml, + sourceDisplayName = 'HTML 101', + outlineUrl = '/course/cid?formats=concise', + sourceLocator = 'source-xblock-locator', + targetParentLocator = 'target-parent-xblock-locator', + sourceParentLocator = 'source-parent-xblock-locator'; + describe('MoveXBlockModal', function() { var modal, showModal, @@ -42,7 +58,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe showModal(); expect( modal.$el.find('.modal-header .title').contents().get(0).nodeValue.trim() - ).toEqual('Move: ' + DISPLAY_NAME); + ).toEqual('Move: ' + sourceDisplayName); expect( modal.$el.find('.modal-sr-title').text().trim() ).toEqual('Choose a location to move your component to'); @@ -56,7 +72,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe expect(modal.$el.find('.ui-loading.is-hidden')).not.toExist(); renderViewsSpy = spyOn(modal, 'renderViews'); expect(requests.length).toEqual(2); - AjaxHelpers.expectRequest(requests, 'GET', OUTLINE_URL); + AjaxHelpers.expectRequest(requests, 'GET', outlineUrl); AjaxHelpers.respondWithJson(requests, {}); AjaxHelpers.expectRequest(requests, 'GET', ANCESTORS_URL); AjaxHelpers.respondWithJson(requests, {}); @@ -72,4 +88,204 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe ViewHelpers.verifyNotificationShowing(notificationSpy, "Studio's having trouble saving your work"); }); }); + + showModal = function() { + modal = new MoveXBlockModal({ + sourceXBlockInfo: new XBlockInfo({ + id: sourceLocator, + display_name: sourceDisplayName, + category: 'html' + }), + sourceParentXBlockInfo: new XBlockInfo({ + id: sourceParentLocator, + display_name: 'VERT 101', + category: 'vertical' + }), + XBlockUrlRoot: '/xblock', + outlineURL: outlineUrl + }); + modal.show(); + }; + + selectTargetParent = function(parentLocator) { + modal.moveXBlockListView = { + parent_info: { + parent: { + id: parentLocator + } + }, + remove: function() {} // attach a fake remove method + }; + }; + + getConfirmationFeedbackTitle = function(displayName) { + return StringUtils.interpolate( + 'Success! "{displayName}" has been moved.', + { + displayName: displayName + } + ); + }; + + getUndoConfirmationFeedbackTitle = function(displayName) { + return StringUtils.interpolate( + 'Move cancelled. "{sourceDisplayName}" has been moved back to its original location.', + { + sourceDisplayName: displayName + } + ); + }; + + getConfirmationFeedbackTitleHtml = function(parentLocator) { + return StringUtils.interpolate( + '{link_start}Take me to the new location{link_end}', + { + link_start: HtmlUtils.HTML(''), + link_end: HtmlUtils.HTML('') + } + ); + }; + + getConfirmationFeedbackMessageHtml = function(displayName, locator, parentLocator, sourceIndex) { + return HtmlUtils.interpolateHtml( + HtmlUtils.HTML( + '{undoMove}'), + { + displayName: displayName, + sourceLocator: locator, + parentSourceLocator: parentLocator, + targetIndex: sourceIndex, + undoMove: gettext('Undo move') + } + ); + }; + + verifyNotificationStatus = function(requests, notificationSpy, notificationText, sourceIndex) { + var sourceIndex = sourceIndex || 0; // eslint-disable-line no-redeclare + ViewHelpers.verifyNotificationShowing(notificationSpy, notificationText); + AjaxHelpers.respondWithJson(requests, { + move_source_locator: sourceLocator, + parent_locator: sourceParentLocator, + target_index: sourceIndex + }); + ViewHelpers.verifyNotificationHidden(notificationSpy); + }; + + describe('Move an xblock', function() { + var sendMoveXBlockRequest, + moveXBlockWithSuccess; + + beforeEach(function() { + TemplateHelpers.installTemplates([ + 'basic-modal', + 'modal-button', + 'move-xblock-modal' + ]); + showModal(); + }); + + afterEach(function() { + modal.hide(); + }); + + sendMoveXBlockRequest = function(requests, xblockLocator, parentLocator, targetIndex, sourceIndex) { + var responseData, + expectedData, + sourceIndex = sourceIndex || 0, // eslint-disable-line no-redeclare + moveButton = modal.$el.find('.modal-actions .action-move')[sourceIndex]; + + // select a target item and click + selectTargetParent(parentLocator); + moveButton.click(); + + responseData = expectedData = { + move_source_locator: xblockLocator, + parent_locator: parentLocator + }; + + if (targetIndex !== undefined) { + expectedData = _.extend(expectedData, { + targetIndex: targetIndex + }); + } + + // verify content of request + AjaxHelpers.expectJsonRequest(requests, 'PATCH', '/xblock/', expectedData); + + // send the response + AjaxHelpers.respondWithJson(requests, _.extend(responseData, { + source_index: sourceIndex + })); + }; + + moveXBlockWithSuccess = function(requests) { + var sourceIndex = 0; + sendMoveXBlockRequest(requests, sourceLocator, targetParentLocator); + expect(modal.movedAlertView).toBeDefined(); + expect(modal.movedAlertView.options.title).toEqual(getConfirmationFeedbackTitle(sourceDisplayName)); + expect(modal.movedAlertView.options.titleHtml).toEqual( + getConfirmationFeedbackTitleHtml(targetParentLocator) + ); + expect(modal.movedAlertView.options.messageHtml).toEqual( + getConfirmationFeedbackMessageHtml( + sourceDisplayName, + sourceLocator, + sourceParentLocator, + sourceIndex + ) + ); + }; + + it('moves an xblock when move button is clicked', function() { + var requests = AjaxHelpers.requests(this); + moveXBlockWithSuccess(requests); + }); + + it('undo move an xblock when undo move button is clicked', function() { + var sourceIndex = 0, + requests = AjaxHelpers.requests(this); + moveXBlockWithSuccess(requests); + modal.movedAlertView.undoMoveXBlock({ + target: $(modal.movedAlertView.options.messageHtml.text) + }); + AjaxHelpers.respondWithJson(requests, { + move_source_locator: sourceLocator, + parent_locator: sourceParentLocator, + target_index: sourceIndex + }); + expect(modal.movedAlertView.movedAlertView.options.title).toEqual( + getUndoConfirmationFeedbackTitle(sourceDisplayName) + ); + }); + + it('does not move an xblock when cancel button is clicked', function() { + var sourceIndex = 0; + // select a target parent and click cancel button + selectTargetParent(targetParentLocator); + modal.$el.find('.modal-actions .action-cancel')[sourceIndex].click(); + expect(modal.movedAlertView).toBeNull(); + }); + + it('shows a notification when moving', function() { + var requests = AjaxHelpers.requests(this), + notificationSpy = ViewHelpers.createNotificationSpy(); + // select a target item and click on move + selectTargetParent(targetParentLocator); + modal.$el.find('.modal-actions .action-move').click(); + verifyNotificationStatus(requests, notificationSpy, 'Moving'); + }); + + it('shows a notification when undo moving', function() { + var notificationSpy, + requests = AjaxHelpers.requests(this); + moveXBlockWithSuccess(requests); + notificationSpy = ViewHelpers.createNotificationSpy(); + modal.movedAlertView.undoMoveXBlock({ + target: $(modal.movedAlertView.options.messageHtml.text) + }); + verifyNotificationStatus(requests, notificationSpy, 'Undo moving'); + }); + }); }); diff --git a/cms/static/js/views/modals/move_xblock_modal.js b/cms/static/js/views/modals/move_xblock_modal.js index 8efabd6a4371..acb45caf3fc1 100644 --- a/cms/static/js/views/modals/move_xblock_modal.js +++ b/cms/static/js/views/modals/move_xblock_modal.js @@ -6,14 +6,23 @@ define([ 'js/views/baseview', 'js/views/modals/base_modal', 'js/models/xblock_info', 'js/views/move_xblock_list', 'js/views/move_xblock_breadcrumb', 'common/js/components/views/feedback', + 'js/views/utils/xblock_utils', + 'js/views/utils/move_xblock_utils', + 'edx-ui-toolkit/js/utils/html-utils', 'edx-ui-toolkit/js/utils/string-utils', 'text!templates/move-xblock-modal.underscore' ], function($, Backbone, _, gettext, BaseView, BaseModal, XBlockInfoModel, MoveXBlockListView, MoveXBlockBreadcrumbView, - Feedback, StringUtils, MoveXblockModalTemplate) { + Feedback, XBlockViewUtils, MoveXBlockUtils, HtmlUtils, StringUtils, MoveXblockModalTemplate) { 'use strict'; var MoveXblockModal = BaseModal.extend({ + modalSRTitle: gettext('Choose a location to move your component to'), + + events: _.extend({}, BaseModal.prototype.events, { + 'click .action-move': 'moveXBlock' + }), + options: $.extend({}, BaseModal.prototype.options, { modalName: 'move-xblock', modalSize: 'lg', @@ -30,6 +39,7 @@ function($, Backbone, _, gettext, BaseView, BaseModal, XBlockInfoModel, MoveXBlo BaseModal.prototype.initialize.call(this); this.listenTo(Backbone, 'move:breadcrumbRendered', this.focusModal); this.sourceXBlockInfo = this.options.sourceXBlockInfo; + this.sourceParentXBlockInfo = this.options.sourceParentXBlockInfo; this.XBlockURLRoot = this.options.XBlockURLRoot; this.XBlockAncestorInfoURL = StringUtils.interpolate( '{urlRoot}/{usageId}?fields=ancestorInfo', @@ -42,12 +52,16 @@ function($, Backbone, _, gettext, BaseView, BaseModal, XBlockInfoModel, MoveXBlo $('.breadcrumb-container').removeClass('is-hidden'); self.renderViews(courseOutlineInfo, ancestorInfo); }); + this.targetParentXBlockInfo = null; + this.movedAlertView = null; + this.moveXBlockBreadcrumbView = null; + this.moveXBlockListView = null; }, getTitle: function() { return StringUtils.interpolate( - gettext('Move: {display_name}'), - {display_name: this.sourceXBlockInfo.get('display_name')} + gettext('Move: {displayName}'), + {displayName: this.sourceXBlockInfo.get('display_name')} ); }, @@ -57,6 +71,7 @@ function($, Backbone, _, gettext, BaseView, BaseModal, XBlockInfoModel, MoveXBlo show: function() { BaseModal.prototype.show.apply(this, [false]); + Feedback.prototype.inFocus.apply(this, [this.options.modalWindowClass]); }, hide: function() { @@ -105,6 +120,52 @@ function($, Backbone, _, gettext, BaseView, BaseModal, XBlockInfoModel, MoveXBlo ancestorInfo: ancestorInfo } ); + }, + + moveXBlock: function() { + var self = this; + XBlockViewUtils.moveXBlock(self.sourceXBlockInfo.id, self.moveXBlockListView.parent_info.parent.id) + .done(function(response) { + if (response.move_source_locator) { + // hide modal + self.hide(); + // hide xblock element + $("li.studio-xblock-wrapper[data-locator='" + self.sourceXBlockInfo.id + "']").hide(); + if (self.movedAlertView) { + self.movedAlertView.hide(); + } + self.movedAlertView = MoveXBlockUtils.showMovedNotification( + StringUtils.interpolate( + gettext('Success! "{displayName}" has been moved.'), + { + displayName: self.sourceXBlockInfo.get('display_name') + } + ), + StringUtils.interpolate( + gettext('{link_start}Take me to the new location{link_end}'), + { + link_start: HtmlUtils.HTML(''), + link_end: HtmlUtils.HTML('') + } + ), + HtmlUtils.interpolateHtml( + HtmlUtils.HTML( + '{undoMove}' + ), + { + displayName: self.sourceXBlockInfo.get('display_name'), + sourceLocator: self.sourceXBlockInfo.id, + sourceParentLocator: self.sourceParentXBlockInfo.id, + targetIndex: response.source_index, + undoMove: gettext('Undo move') + } + ) + ); + } + }); } }); diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 05f6409deb71..1f89554e3ae4 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -194,9 +194,11 @@ define(['jquery', 'underscore', 'gettext', 'js/views/pages/base_page', 'common/j showMoveXBlockModal: function(event) { var xblockElement = this.findXBlockElement(event.target), + parentXBlockElement = xblockElement.parents('.studio-xblock-wrapper'), modal = new MoveXBlockModal({ sourceXBlockInfo: XBlockUtils.findXBlockInfo(xblockElement, this.model), - XBlockURLRoot: this.getURLRoot(), + sourceParentXBlockInfo: XBlockUtils.findXBlockInfo(parentXBlockElement, this.model), + XBlockUrlRoot: this.getURLRoot(), outlineURL: this.options.outlineURL }); diff --git a/cms/static/js/views/utils/move_xblock_utils.js b/cms/static/js/views/utils/move_xblock_utils.js new file mode 100644 index 000000000000..22ee26fb9cac --- /dev/null +++ b/cms/static/js/views/utils/move_xblock_utils.js @@ -0,0 +1,69 @@ +/** + * Provides utilities for move xblock. + */ +define(['jquery', 'underscore', 'common/js/components/views/feedback_alert', 'js/views/utils/xblock_utils', + 'js/views/utils/move_xblock_utils', 'edx-ui-toolkit/js/utils/string-utils'], + function($, _, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtils) { + 'use strict'; + var MovedAlertView, showMovedNotification; + + MovedAlertView = AlertView.Confirmation.extend({ + events: _.extend({}, AlertView.Confirmation.prototype.events, { + 'click .action-undo-move': 'undoMoveXBlock' + }), + + options: $.extend({}, AlertView.Confirmation.prototype.options), + + initialize: function() { + AlertView.prototype.initialize.apply(this, arguments); + this.movedAlertView = null; + }, + + undoMoveXBlock: function(event) { + var self = this, + $moveButton = $(event.target), + sourceLocator = $moveButton.data('source-locator'), + sourceDisplayName = $moveButton.data('source-display-name'), + sourceParentLocator = $moveButton.data('source-parent-locator'), + targetIndex = $moveButton.data('target-index'); + XBlockViewUtils.moveXBlock(sourceLocator, sourceParentLocator, targetIndex) + .done(function(response) { + // show XBlock element + $('.studio-xblock-wrapper[data-locator="' + response.move_source_locator + '"]').show(); + if (self.movedAlertView) { + self.movedAlertView.hide(); + } + self.movedAlertView = showMovedNotification( + StringUtils.interpolate( + gettext('Move cancelled. "{sourceDisplayName}" has been moved back to its original ' + + 'location.'), + { + sourceDisplayName: sourceDisplayName + } + ) + ); + }); + } + }); + + showMovedNotification = function(title, titleHtml, messageHtml) { + var movedAlertView = new MovedAlertView({ + title: title, + titleHtml: titleHtml, + messageHtml: messageHtml, + maxShown: 10000 + }); + movedAlertView.show(); + // scroll to top + $.smoothScroll({ + offset: 0, + easing: 'swing', + speed: 1000 + }); + return movedAlertView; + }; + + return { + showMovedNotification: showMovedNotification + }; + }); diff --git a/cms/static/js/views/utils/xblock_utils.js b/cms/static/js/views/utils/xblock_utils.js index be1f1faadd24..addf611bf700 100644 --- a/cms/static/js/views/utils/xblock_utils.js +++ b/cms/static/js/views/utils/xblock_utils.js @@ -6,7 +6,8 @@ define(['jquery', 'underscore', 'gettext', 'common/js/components/utils/view_util function($, _, gettext, ViewUtils, ModuleUtils, XBlockInfo, StringUtils) { 'use strict'; var addXBlock, duplicateXBlock, deleteXBlock, createUpdateRequestData, updateXBlockField, VisibilityState, - getXBlockVisibilityClass, getXBlockListTypeClass, updateXBlockFields, getXBlockType, findXBlockInfo; + getXBlockVisibilityClass, getXBlockListTypeClass, updateXBlockFields, getXBlockType, findXBlockInfo, + moveXBlock; /** * Represents the possible visibility states for an xblock: @@ -91,6 +92,34 @@ define(['jquery', 'underscore', 'gettext', 'common/js/components/utils/view_util }); }; + /** + * Moves the specified xblock in a new parent xblock. + * @param {String} sourceLocator The xblock element to be moved. + * @param {String} targetParentLocator Target parent xblock locator of the xblock to be moved, + * new moved xblock would be placed under this xblock. + * @param {String} targetIndex Intended index position of the xblock in parent xblock. If provided, + * xblock would be placed at the particular index in the parent xblock. + * @returns {jQuery promise} A promise representing the moving of the xblock. + */ + moveXBlock = function(sourceLocator, targetParentLocator, targetIndex) { + var moveOperation = $.Deferred(), + operationText = targetIndex !== undefined ? gettext('Undo moving') : gettext('Moving'); + return ViewUtils.runOperationShowingMessage(operationText, + function() { + $.patchJSON(ModuleUtils.getUpdateUrl(), { + move_source_locator: sourceLocator, + parent_locator: targetParentLocator, + target_index: targetIndex + }, function(data) { + moveOperation.resolve(data); + }) + .fail(function() { + moveOperation.reject(); + }); + return moveOperation.promise(); + }); + }; + /** * Deletes the specified xblock. * @param xblockInfo The model for the xblock to be deleted. @@ -267,6 +296,7 @@ define(['jquery', 'underscore', 'gettext', 'common/js/components/utils/view_util return { VisibilityState: VisibilityState, addXBlock: addXBlock, + moveXBlock: moveXBlock, duplicateXBlock: duplicateXBlock, deleteXBlock: deleteXBlock, updateXBlockField: updateXBlockField, diff --git a/common/static/common/js/components/views/feedback.js b/common/static/common/js/components/views/feedback.js index 901aaa61fafd..4551d522fc93 100644 --- a/common/static/common/js/components/views/feedback.js +++ b/common/static/common/js/components/views/feedback.js @@ -21,6 +21,8 @@ options: { title: '', message: '', + titleHtml: '', // an optional html that comes after the title. + messageHtml: '', // an optional html that comes after the message. intent: null, // "warning", "confirmation", "error", "announcement", "step-required", etc type: null, // "alert", "notification", or "prompt": set by subclass shown: true, // is this view currently being shown? diff --git a/common/static/common/js/components/views/feedback_move.js b/common/static/common/js/components/views/feedback_move.js new file mode 100644 index 000000000000..740521af1c22 --- /dev/null +++ b/common/static/common/js/components/views/feedback_move.js @@ -0,0 +1,49 @@ +/** + * The MovedAlertView to show confirmation message when moving XBlocks. + */ +(function(define) { + 'use strict'; + define(['jquery', 'underscore', 'common/js/components/views/feedback_alert', 'js/views/utils/xblock_utils', + 'js/views/utils/move_xblock_utils', 'edx-ui-toolkit/js/utils/string-utils'], + function($, _, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtils) { + var MovedAlertView = AlertView.Confirmation.extend({ + events: _.extend({}, AlertView.Confirmation.prototype.events, { + 'click .action-undo-move': 'undoMoveXBlock' + }), + + options: $.extend({}, AlertView.Confirmation.prototype.options), + + initialize: function() { + AlertView.prototype.initialize.apply(this, arguments); + this.movedAlertView = null; + }, + + undoMoveXBlock: function(event) { + var self = this, + $moveButton = $(event.target), + sourceLocator = $moveButton.data('source-locator'), + sourceDisplayName = $moveButton.data('source-display-name'), + sourceParentLocator = $moveButton.data('source-parent-locator'), + targetIndex = $moveButton.data('target-index'); + XBlockViewUtils.moveXBlock(sourceLocator, sourceParentLocator, targetIndex) + .done(function(response) { + // show XBlock element + $('.studio-xblock-wrapper[data-locator="' + response.move_source_locator + '"]').show(); + if (self.movedAlertView) { + self.movedAlertView.hide(); + } + self.movedAlertView = MoveXBlockUtils.showMovedNotification( + StringUtils.interpolate( + gettext('Move cancelled. "{sourceDisplayName}" has been moved back to its original ' + + 'location.'), + { + sourceDisplayName: sourceDisplayName + } + ) + ); + }); + } + }); + return MovedAlertView; + }); +}).call(this, define || RequireJS.define); diff --git a/common/static/common/templates/components/system-feedback.underscore b/common/static/common/templates/components/system-feedback.underscore index 9168abc9d551..625bc574a1c4 100644 --- a/common/static/common/templates/components/system-feedback.underscore +++ b/common/static/common/templates/components/system-feedback.underscore @@ -15,8 +15,9 @@ <% } %>
              -

              <%- title %>

              +

              <%- title %><% if(titleHtml) { %> <%= titleHtml %> <% } %>

              <% if(obj.message) { %>

              <%- message %>

              <% } %> + <% if(messageHtml) { %> <%= messageHtml %> <% } %>
              <% if(obj.actions) { %> From e9b8e17fc8e0799aa5dcc51bfe6656373ee105a9 Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Tue, 24 Jan 2017 17:54:36 +0500 Subject: [PATCH 07/12] Enable disable move - TNL-6063 --- .../contentstore/views/tests/test_item.py | 17 +- cms/static/cms/js/main.js | 4 +- .../views/modals/move_xblock_modal_spec.js | 229 +--------- cms/static/js/spec/views/move_xblock_spec.js | 409 +++++++++++++++--- .../js/views/modals/move_xblock_modal.js | 112 ++--- cms/static/js/views/move_xblock_list.js | 1 + cms/static/js/views/pages/container.js | 2 +- .../js/views/utils/move_xblock_utils.js | 151 ++++--- cms/static/js/views/utils/xblock_utils.js | 12 +- cms/static/sass/elements/_modal-window.scss | 5 + .../common/js/components/views/feedback.js | 2 - .../js/components/views/feedback_move.js | 49 --- .../components/system-feedback.underscore | 3 +- 13 files changed, 539 insertions(+), 457 deletions(-) delete mode 100644 common/static/common/js/components/views/feedback_move.js diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index f5e7e59fcbc7..eff9f8773011 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -735,6 +735,7 @@ def verify_name(source_usage_key, parent_usage_key, expected_name, display_name= verify_name(self.seq_usage_key, self.chapter_usage_key, "customized name", display_name="customized name") +@ddt.ddt class TestMoveItem(ItemTest): """ Tests for move item. @@ -744,7 +745,16 @@ def setUp(self): Creates the test course structure to build course outline tree. """ super(TestMoveItem, self).setUp() + self.setup_course() + + def setup_course(self, default_store=None): + """ + Helper method to create the course. + """ + if not default_store: + default_store = self.store.default_modulestore.get_modulestore_type() + self.course = CourseFactory.create(default_store=default_store) # 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) @@ -821,10 +831,15 @@ def assert_move_item(self, source_usage_key, target_usage_key, target_index=None self.assertEqual(new_parent_loc, target_usage_key) self.assertNotEqual(parent_loc, new_parent_loc) - def test_move_component(self): + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_move_component(self, store_type): """ Test move component with different xblock types. + + Arguments: + store_type (ModuleStoreEnum.Type): Type of modulestore to create test course in. """ + self.setup_course(default_store=store_type) for source_usage_key, target_usage_key in [ (self.html_usage_key, self.vert2_usage_key), (self.vert_usage_key, self.seq2_usage_key), diff --git a/cms/static/cms/js/main.js b/cms/static/cms/js/main.js index 6a463521c042..1e5fb698d1c0 100644 --- a/cms/static/cms/js/main.js +++ b/cms/static/cms/js/main.js @@ -59,10 +59,10 @@ success: callback }); }; - $.postJSON = function(url, data, callback) { + $.postJSON = function(url, data, callback) { // eslint-disable-line no-param-reassign return sendJSON(url, data, callback, 'POST'); }; - $.patchJSON = function(url, data, callback) { + $.patchJSON = function(url, data, callback) { // eslint-disable-line no-param-reassign return sendJSON(url, data, callback, 'PATCH'); }; return domReady(function() { diff --git a/cms/static/js/spec/views/modals/move_xblock_modal_spec.js b/cms/static/js/spec/views/modals/move_xblock_modal_spec.js index 64a89aed4b9a..f2e8f457b4ac 100644 --- a/cms/static/js/spec/views/modals/move_xblock_modal_spec.js +++ b/cms/static/js/spec/views/modals/move_xblock_modal_spec.js @@ -1,24 +1,8 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', 'common/js/spec_helpers/template_helpers', 'common/js/spec_helpers/view_helpers', - 'js/views/modals/move_xblock_modal', 'edx-ui-toolkit/js/utils/html-utils', - 'edx-ui-toolkit/js/utils/string-utils', 'js/models/xblock_info'], - function($, _, AjaxHelpers, TemplateHelpers, ViewHelpers, MoveXBlockModal, HtmlUtils, StringUtils, XBlockInfo) { + 'js/views/modals/move_xblock_modal', 'js/models/xblock_info'], + function($, _, AjaxHelpers, TemplateHelpers, ViewHelpers, MoveXBlockModal, XBlockInfo) { 'use strict'; - - var modal, - showModal, - verifyNotificationStatus, - selectTargetParent, - getConfirmationFeedbackTitle, - getUndoConfirmationFeedbackTitle, - getConfirmationFeedbackTitleHtml, - getConfirmationFeedbackMessageHtml, - sourceDisplayName = 'HTML 101', - outlineUrl = '/course/cid?formats=concise', - sourceLocator = 'source-xblock-locator', - targetParentLocator = 'target-parent-xblock-locator', - sourceParentLocator = 'source-parent-xblock-locator'; - describe('MoveXBlockModal', function() { var modal, showModal, @@ -33,6 +17,11 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe display_name: DISPLAY_NAME, category: 'html' }), + sourceParentXBlockInfo: new XBlockInfo({ + id: 'PARENT_ID', + display_name: 'VERT 101', + category: 'vertical' + }), XBlockURLRoot: '/xblock', outlineURL: OUTLINE_URL, XBlockAncestorInfoURL: ANCESTORS_URL @@ -58,7 +47,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe showModal(); expect( modal.$el.find('.modal-header .title').contents().get(0).nodeValue.trim() - ).toEqual('Move: ' + sourceDisplayName); + ).toEqual('Move: ' + DISPLAY_NAME); expect( modal.$el.find('.modal-sr-title').text().trim() ).toEqual('Choose a location to move your component to'); @@ -72,7 +61,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe expect(modal.$el.find('.ui-loading.is-hidden')).not.toExist(); renderViewsSpy = spyOn(modal, 'renderViews'); expect(requests.length).toEqual(2); - AjaxHelpers.expectRequest(requests, 'GET', outlineUrl); + AjaxHelpers.expectRequest(requests, 'GET', OUTLINE_URL); AjaxHelpers.respondWithJson(requests, {}); AjaxHelpers.expectRequest(requests, 'GET', ANCESTORS_URL); AjaxHelpers.respondWithJson(requests, {}); @@ -88,204 +77,4 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe ViewHelpers.verifyNotificationShowing(notificationSpy, "Studio's having trouble saving your work"); }); }); - - showModal = function() { - modal = new MoveXBlockModal({ - sourceXBlockInfo: new XBlockInfo({ - id: sourceLocator, - display_name: sourceDisplayName, - category: 'html' - }), - sourceParentXBlockInfo: new XBlockInfo({ - id: sourceParentLocator, - display_name: 'VERT 101', - category: 'vertical' - }), - XBlockUrlRoot: '/xblock', - outlineURL: outlineUrl - }); - modal.show(); - }; - - selectTargetParent = function(parentLocator) { - modal.moveXBlockListView = { - parent_info: { - parent: { - id: parentLocator - } - }, - remove: function() {} // attach a fake remove method - }; - }; - - getConfirmationFeedbackTitle = function(displayName) { - return StringUtils.interpolate( - 'Success! "{displayName}" has been moved.', - { - displayName: displayName - } - ); - }; - - getUndoConfirmationFeedbackTitle = function(displayName) { - return StringUtils.interpolate( - 'Move cancelled. "{sourceDisplayName}" has been moved back to its original location.', - { - sourceDisplayName: displayName - } - ); - }; - - getConfirmationFeedbackTitleHtml = function(parentLocator) { - return StringUtils.interpolate( - '{link_start}Take me to the new location{link_end}', - { - link_start: HtmlUtils.HTML(''), - link_end: HtmlUtils.HTML('') - } - ); - }; - - getConfirmationFeedbackMessageHtml = function(displayName, locator, parentLocator, sourceIndex) { - return HtmlUtils.interpolateHtml( - HtmlUtils.HTML( - '{undoMove}'), - { - displayName: displayName, - sourceLocator: locator, - parentSourceLocator: parentLocator, - targetIndex: sourceIndex, - undoMove: gettext('Undo move') - } - ); - }; - - verifyNotificationStatus = function(requests, notificationSpy, notificationText, sourceIndex) { - var sourceIndex = sourceIndex || 0; // eslint-disable-line no-redeclare - ViewHelpers.verifyNotificationShowing(notificationSpy, notificationText); - AjaxHelpers.respondWithJson(requests, { - move_source_locator: sourceLocator, - parent_locator: sourceParentLocator, - target_index: sourceIndex - }); - ViewHelpers.verifyNotificationHidden(notificationSpy); - }; - - describe('Move an xblock', function() { - var sendMoveXBlockRequest, - moveXBlockWithSuccess; - - beforeEach(function() { - TemplateHelpers.installTemplates([ - 'basic-modal', - 'modal-button', - 'move-xblock-modal' - ]); - showModal(); - }); - - afterEach(function() { - modal.hide(); - }); - - sendMoveXBlockRequest = function(requests, xblockLocator, parentLocator, targetIndex, sourceIndex) { - var responseData, - expectedData, - sourceIndex = sourceIndex || 0, // eslint-disable-line no-redeclare - moveButton = modal.$el.find('.modal-actions .action-move')[sourceIndex]; - - // select a target item and click - selectTargetParent(parentLocator); - moveButton.click(); - - responseData = expectedData = { - move_source_locator: xblockLocator, - parent_locator: parentLocator - }; - - if (targetIndex !== undefined) { - expectedData = _.extend(expectedData, { - targetIndex: targetIndex - }); - } - - // verify content of request - AjaxHelpers.expectJsonRequest(requests, 'PATCH', '/xblock/', expectedData); - - // send the response - AjaxHelpers.respondWithJson(requests, _.extend(responseData, { - source_index: sourceIndex - })); - }; - - moveXBlockWithSuccess = function(requests) { - var sourceIndex = 0; - sendMoveXBlockRequest(requests, sourceLocator, targetParentLocator); - expect(modal.movedAlertView).toBeDefined(); - expect(modal.movedAlertView.options.title).toEqual(getConfirmationFeedbackTitle(sourceDisplayName)); - expect(modal.movedAlertView.options.titleHtml).toEqual( - getConfirmationFeedbackTitleHtml(targetParentLocator) - ); - expect(modal.movedAlertView.options.messageHtml).toEqual( - getConfirmationFeedbackMessageHtml( - sourceDisplayName, - sourceLocator, - sourceParentLocator, - sourceIndex - ) - ); - }; - - it('moves an xblock when move button is clicked', function() { - var requests = AjaxHelpers.requests(this); - moveXBlockWithSuccess(requests); - }); - - it('undo move an xblock when undo move button is clicked', function() { - var sourceIndex = 0, - requests = AjaxHelpers.requests(this); - moveXBlockWithSuccess(requests); - modal.movedAlertView.undoMoveXBlock({ - target: $(modal.movedAlertView.options.messageHtml.text) - }); - AjaxHelpers.respondWithJson(requests, { - move_source_locator: sourceLocator, - parent_locator: sourceParentLocator, - target_index: sourceIndex - }); - expect(modal.movedAlertView.movedAlertView.options.title).toEqual( - getUndoConfirmationFeedbackTitle(sourceDisplayName) - ); - }); - - it('does not move an xblock when cancel button is clicked', function() { - var sourceIndex = 0; - // select a target parent and click cancel button - selectTargetParent(targetParentLocator); - modal.$el.find('.modal-actions .action-cancel')[sourceIndex].click(); - expect(modal.movedAlertView).toBeNull(); - }); - - it('shows a notification when moving', function() { - var requests = AjaxHelpers.requests(this), - notificationSpy = ViewHelpers.createNotificationSpy(); - // select a target item and click on move - selectTargetParent(targetParentLocator); - modal.$el.find('.modal-actions .action-move').click(); - verifyNotificationStatus(requests, notificationSpy, 'Moving'); - }); - - it('shows a notification when undo moving', function() { - var notificationSpy, - requests = AjaxHelpers.requests(this); - moveXBlockWithSuccess(requests); - notificationSpy = ViewHelpers.createNotificationSpy(); - modal.movedAlertView.undoMoveXBlock({ - target: $(modal.movedAlertView.options.messageHtml.text) - }); - verifyNotificationStatus(requests, notificationSpy, 'Undo moving'); - }); - }); }); diff --git a/cms/static/js/spec/views/move_xblock_spec.js b/cms/static/js/spec/views/move_xblock_spec.js index 2df7c7ebc810..19acf80381e5 100644 --- a/cms/static/js/spec/views/move_xblock_spec.js +++ b/cms/static/js/spec/views/move_xblock_spec.js @@ -1,14 +1,20 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', - 'common/js/spec_helpers/template_helpers', 'js/views/move_xblock_list', - 'js/views/move_xblock_breadcrumb', 'js/models/xblock_info'], - function($, _, AjaxHelpers, TemplateHelpers, MoveXBlockListView, MoveXBlockBreadcrumbView, - XBlockInfoModel) { + 'common/js/spec_helpers/template_helpers', 'common/js/spec_helpers/view_helpers', + 'js/views/modals/move_xblock_modal', 'edx-ui-toolkit/js/utils/html-utils', + 'edx-ui-toolkit/js/utils/string-utils', 'js/models/xblock_info'], + function($, _, AjaxHelpers, TemplateHelpers, ViewHelpers, MoveXBlockModal, HtmlUtils, StringUtils, XBlockInfo) { 'use strict'; describe('MoveXBlock', function() { - var renderViews, createXBlockInfo, createCourseOutline, moveXBlockBreadcrumbView, - moveXBlockListView, parentChildMap, categoryMap, createChildXBlockInfo, + var modal, showModal, renderViews, createXBlockInfo, createCourseOutline, courseOutlineOptions, + parentChildMap, categoryMap, createChildXBlockInfo, xblockAncestorInfo, courseOutline, verifyBreadcrumbViewInfo, verifyListViewInfo, getDisplayedInfo, clickForwardButton, - clickBreadcrumbButton, verifyXBlockInfo, nextCategory; + clickBreadcrumbButton, verifyXBlockInfo, nextCategory, verifyMoveEnabled, getSentRequests, + verifyNotificationStatus, sendMoveXBlockRequest, moveXBlockWithSuccess, + verifyConfirmationFeedbackTitleHtml, verifyConfirmationFeedbackRedirectLinkHtml, + verifyUndoConfirmationFeedbackTitleHtml, verifyConfirmationFeedbackUndoMoveActionHtml, + sourceDisplayName = 'component_display_name_0', + sourceLocator = 'component_ID_0', + sourceParentLocator = 'unit_ID_0'; parentChildMap = { course: 'section', @@ -24,21 +30,71 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe component: 'component' }; + courseOutlineOptions = { + section: 2, + subsection: 2, + unit: 2, + component: 2 + }; + + xblockAncestorInfo = { + ancestors: [ + { + category: 'vertical', + display_name: 'unit_display_name_0', + id: 'unit_ID_0' + }, + { + category: 'sequential', + display_name: 'subsection_display_name_0', + id: 'subsection_ID_0' + }, + { + category: 'chapter', + display_name: 'section_display_name_0', + id: 'section_ID_0' + }, + { + category: 'course', + display_name: 'Demo Course', + id: 'COURSE_ID_101' + } + ] + }; + beforeEach(function() { - setFixtures( - "
              " - ); + setFixtures("
              "); TemplateHelpers.installTemplates([ - 'move-xblock-list', - 'move-xblock-breadcrumb' + 'basic-modal', + 'modal-button', + 'move-xblock-modal' ]); + courseOutline = createCourseOutline(courseOutlineOptions); + showModal(); }); afterEach(function() { - moveXBlockBreadcrumbView.remove(); - moveXBlockListView.remove(); + modal.hide(); + courseOutline = null; }); + showModal = function() { + modal = new MoveXBlockModal({ + sourceXBlockInfo: new XBlockInfo({ + id: sourceLocator, + display_name: sourceDisplayName, + category: 'component' + }), + sourceParentXBlockInfo: new XBlockInfo({ + id: sourceParentLocator, + display_name: 'unit_display_name_0', + category: 'vertical' + }), + XBlockUrlRoot: '/xblock' + }); + modal.show(); + }; + /** * Create child XBlock info. * @@ -51,7 +107,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe var childInfo = { category: categoryMap[category], display_name: category + '_display_name_' + xblockIndex, - id: category + '_ID' + id: category + '_ID_' + xblockIndex }; return createXBlockInfo(parentChildMap[category], outlineOptions, childInfo); }; @@ -93,12 +149,12 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe * @returns {Object} */ createCourseOutline = function(outlineOptions) { - var courseOutline = { + var courseXBlockInfo = { category: 'course', display_name: 'Demo Course', id: 'COURSE_ID_101' }; - return createXBlockInfo('section', outlineOptions, courseOutline); + return createXBlockInfo('section', outlineOptions, courseXBlockInfo); }; /** @@ -108,13 +164,8 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe * @param {any} ancestorInfo ancestors info */ renderViews = function(courseOutlineInfo, ancestorInfo) { - moveXBlockBreadcrumbView = new MoveXBlockBreadcrumbView({}); - moveXBlockListView = new MoveXBlockListView( - { - model: new XBlockInfoModel(courseOutlineInfo, {parse: true}), - ancestorInfo: ancestorInfo || {ancestors: []} - } - ); + var ancestorInfo = ancestorInfo || {ancestors: []}; // eslint-disable-line no-redeclare + modal.renderViews(courseOutlineInfo, ancestorInfo); }; /** @@ -123,7 +174,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe * @returns {Object} */ getDisplayedInfo = function() { - var viewEl = moveXBlockListView.$el; + var viewEl = modal.moveXBlockListView.$el; return { categoryText: viewEl.find('.category-text').text().trim(), currentLocationText: viewEl.find('.current-location').text().trim(), @@ -147,7 +198,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe */ verifyListViewInfo = function(category, expectedXBlocksCount, hasCurrentLocation) { var displayedInfo = getDisplayedInfo(); - expect(displayedInfo.categoryText).toEqual(moveXBlockListView.categoriesText[category] + ':'); + expect(displayedInfo.categoryText).toEqual(modal.moveXBlockListView.categoriesText[category] + ':'); expect(displayedInfo.xblockCount).toEqual(expectedXBlocksCount); expect(displayedInfo.xblockDisplayNames).toEqual( _.map(_.range(expectedXBlocksCount), function(xblockIndex) { @@ -174,7 +225,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe * @param {any} xblockIndex XBlock index */ verifyBreadcrumbViewInfo = function(category, xblockIndex) { - var displayedBreadcrumbs = moveXBlockBreadcrumbView.$el.find('.breadcrumbs .bc-container').map( + var displayedBreadcrumbs = modal.moveXBlockBreadcrumbView.$el.find('.breadcrumbs .bc-container').map( function() { return $(this).text().trim(); } ).get(), categories = _.keys(parentChildMap).concat(['component']), @@ -195,14 +246,14 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe */ clickForwardButton = function(buttonIndex) { buttonIndex = buttonIndex || 0; // eslint-disable-line no-param-reassign - moveXBlockListView.$el.find('[data-item-index="' + buttonIndex + '"] button').click(); + modal.moveXBlockListView.$el.find('[data-item-index="' + buttonIndex + '"] button').click(); }; /** * Click on last clickable breadcrumb button. */ clickBreadcrumbButton = function() { - moveXBlockBreadcrumbView.$el.find('.bc-container button').last().click(); + modal.moveXBlockBreadcrumbView.$el.find('.bc-container button').last().click(); }; /** @@ -231,6 +282,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe verifyListViewInfo(category, expectedXBlocksCount, hasCurrentLocation); verifyBreadcrumbViewInfo(category, buttonIndex); + verifyMoveEnabled(category, hasCurrentLocation); if (direction === 'forward') { if (category === 'component') { @@ -248,13 +300,162 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe verifyXBlockInfo(outlineOptions, category, buttonIndex, direction, hasCurrentLocation); }; + /** + * Verify move button is enabled. + * + * @param {String} category XBlock category + * @param {String} hasCurrentLocation do we need to check current location + */ + verifyMoveEnabled = function(category, hasCurrentLocation) { + var isMoveEnabled = !modal.$el.find('.modal-actions .action-move').hasClass('is-disabled'); + if (category === 'component' && !hasCurrentLocation) { + expect(isMoveEnabled).toBeTruthy(); + } else { + expect(isMoveEnabled).toBeFalsy(); + } + }; + + /** + * Verify notification status. + * + * @param {Object} requests requests object + * @param {Object} notificationSpy notification spy + * @param {String} notificationText notification text to be verified + * @param {Integer} sourceIndex source index of the xblock + */ + verifyNotificationStatus = function(requests, notificationSpy, notificationText, sourceIndex) { + var sourceIndex = sourceIndex || 0; // eslint-disable-line no-redeclare + ViewHelpers.verifyNotificationShowing(notificationSpy, notificationText); + AjaxHelpers.respondWithJson(requests, { + move_source_locator: sourceLocator, + parent_locator: sourceParentLocator, + target_index: sourceIndex + }); + ViewHelpers.verifyNotificationHidden(notificationSpy); + }; + + /** + * Send move xblock request. + * + * @param {Object} requests requests object + * @param {Object} xblockLocator Xblock id location + * @param {Integer} targetIndex target index of the xblock + * @param {Integer} sourceIndex source index of the xblock + */ + sendMoveXBlockRequest = function(requests, xblockLocator, targetIndex, sourceIndex) { + var responseData, + expectedData, + sourceIndex = sourceIndex || 0; // eslint-disable-line no-redeclare + + responseData = expectedData = { + move_source_locator: xblockLocator, + parent_locator: modal.targetParentXBlockInfo.id + }; + + if (targetIndex !== undefined) { + expectedData = _.extend(expectedData, { + targetIndex: targetIndex + }); + } + + // verify content of request + AjaxHelpers.expectJsonRequest(requests, 'PATCH', '/xblock/', expectedData); + + // send the response + AjaxHelpers.respondWithJson(requests, _.extend(responseData, { + source_index: sourceIndex + })); + }; + + /** + * Move xblock with success. + * + * @param {Object} requests requests object + */ + moveXBlockWithSuccess = function(requests) { + // select a target item and click + renderViews(courseOutline); + _.each(_.range(3), function() { + clickForwardButton(1); + }); + modal.$el.find('.modal-actions .action-move').click(); + sendMoveXBlockRequest(requests, sourceLocator); + expect(modal.movedAlertView).toBeDefined(); + verifyConfirmationFeedbackTitleHtml(sourceDisplayName); + verifyConfirmationFeedbackRedirectLinkHtml(); + verifyConfirmationFeedbackUndoMoveActionHtml(); + }; + + /** + * Verify success banner message html has correct title html. + * + * @param {String} displayName XBlock display name + */ + verifyConfirmationFeedbackTitleHtml = function(displayName) { + expect(modal.movedAlertView.$el.find('.title').html().trim()) + .toEqual(StringUtils.interpolate('Success! "{displayName}" has been moved.', + { + displayName: displayName + }) + ); + }; + + /** + * Verify undo success banner message html has correct title html. + * + * @param {String} displayName XBlock display name + */ + verifyUndoConfirmationFeedbackTitleHtml = function(displayName) { + expect(modal.movedAlertView.$el.find('.title').html()).toEqual( + StringUtils.interpolate( + 'Move cancelled. "{sourceDisplayName}" has been moved back to its original location.', + { + sourceDisplayName: displayName + } + ) + ); + }; + + /** + * Verify success banner message html has correct redirect link html. + */ + verifyConfirmationFeedbackRedirectLinkHtml = function() { + expect(modal.movedAlertView.$el.find('.copy').html().indexOf( + HtmlUtils.HTML( + '' + ) !== -1 + )).toBeTruthy(); + }; + + /** + * Verify success banner message html has correct undo move button html. + */ + verifyConfirmationFeedbackUndoMoveActionHtml = function() { + expect(modal.movedAlertView.$el.find('.copy').html().indexOf( + HtmlUtils.HTML( + '' + ) !== -1 + )).toBeTruthy(); + }; + + /** + * Get sent requests. + * + * @returns {Object} + */ + getSentRequests = function() { + return jasmine.Ajax.requests.filter(function(request) { + return request.readyState > 0; + }); + }; + it('renders views with correct information', function() { var outlineOptions = {section: 1, subsection: 1, unit: 1, component: 1}, outline = createCourseOutline(outlineOptions); - renderViews(outline); - verifyXBlockInfo(outlineOptions, 'section', 0, 'forward', false); - verifyXBlockInfo(outlineOptions, 'component', 0, 'backward', false); + renderViews(outline, xblockAncestorInfo); + verifyXBlockInfo(outlineOptions, 'section', 0, 'forward', true); + verifyXBlockInfo(outlineOptions, 'component', 0, 'backward', true); }); it('shows correct behavior on breadcrumb navigation', function() { @@ -268,43 +469,18 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe _.each(['component', 'unit', 'subsection', 'section'], function(category) { verifyListViewInfo(category, 1); if (category !== 'section') { - moveXBlockBreadcrumbView.$el.find('.bc-container button').last().click(); + modal.moveXBlockBreadcrumbView.$el.find('.bc-container button').last().click(); } }); }); it('shows the correct current location', function() { var outlineOptions = {section: 2, subsection: 2, unit: 2, component: 2}, - outline = createCourseOutline(outlineOptions), - ancestorInfo = { - ancestors: [ - { - category: 'vertical', - display_name: 'unit_display_name_0', - id: 'unit_ID' - }, - { - category: 'sequential', - display_name: 'subsection_display_name_0', - id: 'subsection_ID' - }, - { - category: 'chapter', - display_name: 'section_display_name_0', - id: 'section_ID' - }, - { - category: 'course', - display_name: 'Demo Course', - id: 'COURSE_ID_101' - } - ] - }; - - renderViews(outline, ancestorInfo); + outline = createCourseOutline(outlineOptions); + renderViews(outline, xblockAncestorInfo); verifyXBlockInfo(outlineOptions, 'section', 0, 'forward', true); // click the outline breadcrumb to render sections - moveXBlockBreadcrumbView.$el.find('.bc-container button').first().click(); + modal.moveXBlockBreadcrumbView.$el.find('.bc-container button').first().click(); verifyXBlockInfo(outlineOptions, 'section', 1, 'forward', false); }); @@ -336,9 +512,114 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe _.each(_.range(info.forwardClicks), function() { clickForwardButton(); }); - expect(moveXBlockListView.$el.find('.xblock-no-child-message').text().trim()).toEqual(info.message); - moveXBlockListView.undelegateEvents(); - moveXBlockBreadcrumbView.undelegateEvents(); + expect(modal.moveXBlockListView.$el.find('.xblock-no-child-message').text().trim()) + .toEqual(info.message); + modal.moveXBlockListView.undelegateEvents(); + modal.moveXBlockBreadcrumbView.undelegateEvents(); + }); + }); + + describe('Move an xblock', function() { + it('can not move in a disabled state', function() { + verifyMoveEnabled(false); + modal.$el.find('.modal-actions .action-move').click(); + expect(modal.movedAlertView).toBeNull(); + expect(getSentRequests().length).toEqual(0); + }); + + it('move button is disabled when navigating to same parent', function() { + // select a target parent as the same as source parent and click + renderViews(courseOutline); + _.each(_.range(3), function() { + clickForwardButton(0); + }); + verifyMoveEnabled('component', true); + }); + + it('move button is enabled when navigating to different parent', function() { + // select a target parent as the different as source parent and click + renderViews(courseOutline); + _.each(_.range(3), function() { + clickForwardButton(1); + }); + verifyMoveEnabled('component', false); + }); + + it('verify move state while navigating', function() { + renderViews(courseOutline, xblockAncestorInfo); + verifyXBlockInfo(courseOutlineOptions, 'section', 0, 'forward', true); + // start from course outline again + modal.moveXBlockBreadcrumbView.$el.find('.bc-container button').first().click(); + verifyXBlockInfo(courseOutlineOptions, 'section', 1, 'forward', false); + }); + + it('move an xblock when move button is clicked', function() { + var requests = AjaxHelpers.requests(this); + moveXBlockWithSuccess(requests); + }); + + it('do not move an xblock when cancel button is clicked', function() { + modal.$el.find('.modal-actions .action-cancel').click(); + expect(modal.movedAlertView).toBeNull(); + expect(getSentRequests().length).toEqual(0); + }); + + it('undo move an xblock when undo move link is clicked', function() { + var sourceIndex = 0, + requests = AjaxHelpers.requests(this); + moveXBlockWithSuccess(requests); + modal.movedAlertView.$el.find('.action-save').click(); + AjaxHelpers.respondWithJson(requests, { + move_source_locator: sourceLocator, + parent_locator: sourceParentLocator, + target_index: sourceIndex + }); + verifyUndoConfirmationFeedbackTitleHtml(sourceDisplayName); + }); + }); + + describe('shows a notification', function() { + it('mini operation message when moving an xblock', function() { + var requests = AjaxHelpers.requests(this), + notificationSpy = ViewHelpers.createNotificationSpy(); + // navigate to a target parent and click + renderViews(courseOutline); + _.each(_.range(3), function() { + clickForwardButton(1); + }); + modal.$el.find('.modal-actions .action-move').click(); + verifyNotificationStatus(requests, notificationSpy, 'Moving'); + }); + + it('mini operation message when undo moving an xblock', function() { + var notificationSpy, + requests = AjaxHelpers.requests(this); + moveXBlockWithSuccess(requests); + notificationSpy = ViewHelpers.createNotificationSpy(); + modal.movedAlertView.$el.find('.action-save').click(); + verifyNotificationStatus(requests, notificationSpy, 'Undo moving'); + }); + + it('error message when move request fails', function() { + var requests = AjaxHelpers.requests(this), + notificationSpy = ViewHelpers.createNotificationSpy('Error'); + // select a target item and click + renderViews(courseOutline); + _.each(_.range(3), function() { + clickForwardButton(1); + }); + modal.$el.find('.modal-actions .action-move').click(); + AjaxHelpers.respondWithError(requests); + ViewHelpers.verifyNotificationShowing(notificationSpy, "Studio's having trouble saving your work"); + }); + + it('error message when undo move request fails', function() { + var requests = AjaxHelpers.requests(this), + notificationSpy = ViewHelpers.createNotificationSpy('Error'); + moveXBlockWithSuccess(requests); + modal.movedAlertView.$el.find('.action-save').click(); + AjaxHelpers.respondWithError(requests); + ViewHelpers.verifyNotificationShowing(notificationSpy, "Studio's having trouble saving your work"); }); }); }); diff --git a/cms/static/js/views/modals/move_xblock_modal.js b/cms/static/js/views/modals/move_xblock_modal.js index acb45caf3fc1..f6d1d3fb7ea7 100644 --- a/cms/static/js/views/modals/move_xblock_modal.js +++ b/cms/static/js/views/modals/move_xblock_modal.js @@ -2,25 +2,29 @@ * The MoveXblockModal to move XBlocks in course. */ define([ - 'jquery', 'backbone', 'underscore', 'gettext', - 'js/views/baseview', 'js/views/modals/base_modal', - 'js/models/xblock_info', 'js/views/move_xblock_list', 'js/views/move_xblock_breadcrumb', - 'common/js/components/views/feedback', + 'jquery', + 'backbone', + 'underscore', + 'gettext', + 'js/views/baseview', 'js/views/utils/xblock_utils', 'js/views/utils/move_xblock_utils', 'edx-ui-toolkit/js/utils/html-utils', 'edx-ui-toolkit/js/utils/string-utils', + 'common/js/components/views/feedback', + 'js/models/xblock_info', + 'js/views/modals/base_modal', + 'js/views/move_xblock_list', + 'js/views/move_xblock_breadcrumb', 'text!templates/move-xblock-modal.underscore' ], -function($, Backbone, _, gettext, BaseView, BaseModal, XBlockInfoModel, MoveXBlockListView, MoveXBlockBreadcrumbView, - Feedback, XBlockViewUtils, MoveXBlockUtils, HtmlUtils, StringUtils, MoveXblockModalTemplate) { +function($, Backbone, _, gettext, BaseView, XBlockViewUtils, MoveXBlockUtils, HtmlUtils, StringUtils, Feedback, + XBlockInfoModel, BaseModal, MoveXBlockListView, MoveXBlockBreadcrumbView, MoveXblockModalTemplate) { 'use strict'; var MoveXblockModal = BaseModal.extend({ - modalSRTitle: gettext('Choose a location to move your component to'), - events: _.extend({}, BaseModal.prototype.events, { - 'click .action-move': 'moveXBlock' + 'click .action-move:not(.is-disabled)': 'moveXBlock' }), options: $.extend({}, BaseModal.prototype.options, { @@ -40,6 +44,7 @@ function($, Backbone, _, gettext, BaseView, BaseModal, XBlockInfoModel, MoveXBlo this.listenTo(Backbone, 'move:breadcrumbRendered', this.focusModal); this.sourceXBlockInfo = this.options.sourceXBlockInfo; this.sourceParentXBlockInfo = this.options.sourceParentXBlockInfo; + this.targetParentXBlockInfo = null; this.XBlockURLRoot = this.options.XBlockURLRoot; this.XBlockAncestorInfoURL = StringUtils.interpolate( '{urlRoot}/{usageId}?fields=ancestorInfo', @@ -52,10 +57,9 @@ function($, Backbone, _, gettext, BaseView, BaseModal, XBlockInfoModel, MoveXBlo $('.breadcrumb-container').removeClass('is-hidden'); self.renderViews(courseOutlineInfo, ancestorInfo); }); - this.targetParentXBlockInfo = null; this.movedAlertView = null; - this.moveXBlockBreadcrumbView = null; - this.moveXBlockListView = null; + this.isValidMove = false; + this.listenTo(Backbone, 'move:enableMoveOperation', this.enableMoveOperation); }, getTitle: function() { @@ -71,7 +75,8 @@ function($, Backbone, _, gettext, BaseView, BaseModal, XBlockInfoModel, MoveXBlo show: function() { BaseModal.prototype.show.apply(this, [false]); - Feedback.prototype.inFocus.apply(this, [this.options.modalWindowClass]); + this.updateMoveState(false); + MoveXBlockUtils.hideMovedNotification(); }, hide: function() { @@ -122,50 +127,51 @@ function($, Backbone, _, gettext, BaseView, BaseModal, XBlockInfoModel, MoveXBlo ); }, + updateMoveState: function(isValidMove) { + var $moveButton = this.$el.find('.action-move'); + if (isValidMove) { + $moveButton.removeClass('is-disabled'); + } else { + $moveButton.addClass('is-disabled'); + } + }, + + enableMoveOperation: function(targetParentXBlockInfo) { + var isValidMove = false, + sourceParentType = this.sourceParentXBlockInfo.get('category'), + targetParentType = targetParentXBlockInfo.get('category'); + + if (targetParentType === sourceParentType && this.sourceParentXBlockInfo.id !== targetParentXBlockInfo.id) { + isValidMove = true; + this.targetParentXBlockInfo = targetParentXBlockInfo; + } + this.updateMoveState(isValidMove); + }, + moveXBlock: function() { var self = this; - XBlockViewUtils.moveXBlock(self.sourceXBlockInfo.id, self.moveXBlockListView.parent_info.parent.id) - .done(function(response) { - if (response.move_source_locator) { - // hide modal - self.hide(); - // hide xblock element - $("li.studio-xblock-wrapper[data-locator='" + self.sourceXBlockInfo.id + "']").hide(); - if (self.movedAlertView) { - self.movedAlertView.hide(); + XBlockViewUtils.moveXBlock(self.sourceXBlockInfo.id, self.targetParentXBlockInfo.id) + .done(function(response) { + // hide modal + self.hide(); + // hide xblock element + $("li.studio-xblock-wrapper[data-locator='" + self.sourceXBlockInfo.id + "']").hide(); + self.movedAlertView = MoveXBlockUtils.showMovedNotification( + StringUtils.interpolate( + gettext('Success! "{displayName}" has been moved.'), + { + displayName: self.sourceXBlockInfo.get('display_name') } - self.movedAlertView = MoveXBlockUtils.showMovedNotification( - StringUtils.interpolate( - gettext('Success! "{displayName}" has been moved.'), - { - displayName: self.sourceXBlockInfo.get('display_name') - } - ), - StringUtils.interpolate( - gettext('{link_start}Take me to the new location{link_end}'), - { - link_start: HtmlUtils.HTML(''), - link_end: HtmlUtils.HTML('') - } - ), - HtmlUtils.interpolateHtml( - HtmlUtils.HTML( - '{undoMove}' - ), - { - displayName: self.sourceXBlockInfo.get('display_name'), - sourceLocator: self.sourceXBlockInfo.id, - sourceParentLocator: self.sourceParentXBlockInfo.id, - targetIndex: response.source_index, - undoMove: gettext('Undo move') - } - ) - ); + ), + { + sourceDisplayName: self.sourceXBlockInfo.get('display_name'), + sourceLocator: self.sourceXBlockInfo.id, + sourceParentLocator: self.sourceParentXBlockInfo.id, + targetParentLocator: response.parent_locator, + targetIndex: response.source_index } - }); + ); + }); } }); diff --git a/cms/static/js/views/move_xblock_list.js b/cms/static/js/views/move_xblock_list.js index 4e302de97f93..5815c9bd1817 100644 --- a/cms/static/js/views/move_xblock_list.js +++ b/cms/static/js/views/move_xblock_list.js @@ -63,6 +63,7 @@ function($, Backbone, _, gettext, HtmlUtils, StringUtils, XBlockUtils, MoveXBloc ) ); Backbone.trigger('move:childrenRendered', this.breadcrumbInfo()); + Backbone.trigger('move:enableMoveOperation', this.parentInfo.parent); return this; }, diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 1f89554e3ae4..6334ace61885 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -198,7 +198,7 @@ define(['jquery', 'underscore', 'gettext', 'js/views/pages/base_page', 'common/j modal = new MoveXBlockModal({ sourceXBlockInfo: XBlockUtils.findXBlockInfo(xblockElement, this.model), sourceParentXBlockInfo: XBlockUtils.findXBlockInfo(parentXBlockElement, this.model), - XBlockUrlRoot: this.getURLRoot(), + XBlockURLRoot: this.getURLRoot(), outlineURL: this.options.outlineURL }); diff --git a/cms/static/js/views/utils/move_xblock_utils.js b/cms/static/js/views/utils/move_xblock_utils.js index 22ee26fb9cac..820c8abc59b9 100644 --- a/cms/static/js/views/utils/move_xblock_utils.js +++ b/cms/static/js/views/utils/move_xblock_utils.js @@ -1,69 +1,106 @@ /** * Provides utilities for move xblock. */ -define(['jquery', 'underscore', 'common/js/components/views/feedback_alert', 'js/views/utils/xblock_utils', - 'js/views/utils/move_xblock_utils', 'edx-ui-toolkit/js/utils/string-utils'], - function($, _, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtils) { - 'use strict'; - var MovedAlertView, showMovedNotification; +define([ + 'jquery', + 'underscore', + 'common/js/components/views/feedback', + 'common/js/components/views/feedback_alert', + 'js/views/utils/xblock_utils', + 'js/views/utils/move_xblock_utils', + 'edx-ui-toolkit/js/utils/string-utils' +], +function($, _, Feedback, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtils) { + 'use strict'; + var redirectLink, undoMoveXBlock, showMovedNotification, hideMovedNotification; - MovedAlertView = AlertView.Confirmation.extend({ - events: _.extend({}, AlertView.Confirmation.prototype.events, { - 'click .action-undo-move': 'undoMoveXBlock' - }), + redirectLink = function(link) { + window.location.href = link; + }; - options: $.extend({}, AlertView.Confirmation.prototype.options), - - initialize: function() { - AlertView.prototype.initialize.apply(this, arguments); - this.movedAlertView = null; - }, - - undoMoveXBlock: function(event) { - var self = this, - $moveButton = $(event.target), - sourceLocator = $moveButton.data('source-locator'), - sourceDisplayName = $moveButton.data('source-display-name'), - sourceParentLocator = $moveButton.data('source-parent-locator'), - targetIndex = $moveButton.data('target-index'); - XBlockViewUtils.moveXBlock(sourceLocator, sourceParentLocator, targetIndex) - .done(function(response) { - // show XBlock element - $('.studio-xblock-wrapper[data-locator="' + response.move_source_locator + '"]').show(); - if (self.movedAlertView) { - self.movedAlertView.hide(); + undoMoveXBlock = function(data) { + XBlockViewUtils.moveXBlock(data.sourceLocator, data.sourceParentLocator, data.targetIndex) + .done(function(response) { + // show XBlock element + $('.studio-xblock-wrapper[data-locator="' + response.move_source_locator + '"]').show(); + showMovedNotification( + StringUtils.interpolate( + gettext('Move cancelled. "{sourceDisplayName}" has been moved back to its original location.'), + { + sourceDisplayName: data.sourceDisplayName } - self.movedAlertView = showMovedNotification( - StringUtils.interpolate( - gettext('Move cancelled. "{sourceDisplayName}" has been moved back to its original ' + - 'location.'), - { - sourceDisplayName: sourceDisplayName - } - ) - ); - }); - } + ) + ); }); + }; - showMovedNotification = function(title, titleHtml, messageHtml) { - var movedAlertView = new MovedAlertView({ + showMovedNotification = function(title, data) { + var movedAlertView; + // data is provided when we click undo move button. + if (data) { + movedAlertView = new AlertView.Confirmation({ title: title, - titleHtml: titleHtml, - messageHtml: messageHtml, - maxShown: 10000 + actions: { + primary: { + text: gettext('Undo move'), + class: 'action-save', + data: JSON.stringify({ + sourceDisplayName: data.sourceDisplayName, + sourceLocator: data.sourceLocator, + sourceParentLocator: data.sourceParentLocator, + targetIndex: data.targetIndex + }), + click: function() { + undoMoveXBlock( + { + sourceDisplayName: data.sourceDisplayName, + sourceLocator: data.sourceLocator, + sourceParentLocator: data.sourceParentLocator, + targetIndex: data.targetIndex + } + ); + } + }, + secondary: [ + { + text: gettext('Take me to the new location'), + class: 'action-cancel', + data: JSON.stringify({ + targetParentLocator: data.targetParentLocator + }), + click: function() { + redirectLink('/container/' + data.targetParentLocator); + } + } + ] + } }); - movedAlertView.show(); - // scroll to top - $.smoothScroll({ - offset: 0, - easing: 'swing', - speed: 1000 + } else { + movedAlertView = new AlertView.Confirmation({ + title: title }); - return movedAlertView; - }; + } + movedAlertView.show(); + // scroll to top + $.smoothScroll({ + offset: 0, + easing: 'swing', + speed: 1000 + }); + movedAlertView.$('.wrapper').first().focus(); + return movedAlertView; + }; + + hideMovedNotification = function() { + var movedAlertView = Feedback.active_alert; + if (movedAlertView) { + AlertView.prototype.hide.apply(movedAlertView); + } + }; - return { - showMovedNotification: showMovedNotification - }; - }); + return { + redirectLink: redirectLink, + showMovedNotification: showMovedNotification, + hideMovedNotification: hideMovedNotification + }; +}); diff --git a/cms/static/js/views/utils/xblock_utils.js b/cms/static/js/views/utils/xblock_utils.js index addf611bf700..860d055f3ad8 100644 --- a/cms/static/js/views/utils/xblock_utils.js +++ b/cms/static/js/views/utils/xblock_utils.js @@ -94,10 +94,10 @@ define(['jquery', 'underscore', 'gettext', 'common/js/components/utils/view_util /** * Moves the specified xblock in a new parent xblock. - * @param {String} sourceLocator The xblock element to be moved. - * @param {String} targetParentLocator Target parent xblock locator of the xblock to be moved, - * new moved xblock would be placed under this xblock. - * @param {String} targetIndex Intended index position of the xblock in parent xblock. If provided, + * @param {String} sourceLocator Locator of xblock element to be moved. + * @param {String} targetParentLocator Locator of the target parent xblock, moved xblock would be placed + * under this xblock. + * @param {Integer} targetIndex Intended index position of the xblock in parent xblock. If provided, * xblock would be placed at the particular index in the parent xblock. * @returns {jQuery promise} A promise representing the moving of the xblock. */ @@ -110,8 +110,8 @@ define(['jquery', 'underscore', 'gettext', 'common/js/components/utils/view_util move_source_locator: sourceLocator, parent_locator: targetParentLocator, target_index: targetIndex - }, function(data) { - moveOperation.resolve(data); + }, function(response) { + moveOperation.resolve(response); }) .fail(function() { moveOperation.reject(); diff --git a/cms/static/sass/elements/_modal-window.scss b/cms/static/sass/elements/_modal-window.scss index 893ac32d17c5..472c869a270e 100644 --- a/cms/static/sass/elements/_modal-window.scss +++ b/cms/static/sass/elements/_modal-window.scss @@ -297,6 +297,11 @@ .ui-loading { box-shadow: none; } + + .modal-actions .action-move.is-disabled { + border: 1px solid $gray-l1 !important; + background: $gray-l1 !important; + } } // upload modal diff --git a/common/static/common/js/components/views/feedback.js b/common/static/common/js/components/views/feedback.js index 4551d522fc93..901aaa61fafd 100644 --- a/common/static/common/js/components/views/feedback.js +++ b/common/static/common/js/components/views/feedback.js @@ -21,8 +21,6 @@ options: { title: '', message: '', - titleHtml: '', // an optional html that comes after the title. - messageHtml: '', // an optional html that comes after the message. intent: null, // "warning", "confirmation", "error", "announcement", "step-required", etc type: null, // "alert", "notification", or "prompt": set by subclass shown: true, // is this view currently being shown? diff --git a/common/static/common/js/components/views/feedback_move.js b/common/static/common/js/components/views/feedback_move.js deleted file mode 100644 index 740521af1c22..000000000000 --- a/common/static/common/js/components/views/feedback_move.js +++ /dev/null @@ -1,49 +0,0 @@ -/** - * The MovedAlertView to show confirmation message when moving XBlocks. - */ -(function(define) { - 'use strict'; - define(['jquery', 'underscore', 'common/js/components/views/feedback_alert', 'js/views/utils/xblock_utils', - 'js/views/utils/move_xblock_utils', 'edx-ui-toolkit/js/utils/string-utils'], - function($, _, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtils) { - var MovedAlertView = AlertView.Confirmation.extend({ - events: _.extend({}, AlertView.Confirmation.prototype.events, { - 'click .action-undo-move': 'undoMoveXBlock' - }), - - options: $.extend({}, AlertView.Confirmation.prototype.options), - - initialize: function() { - AlertView.prototype.initialize.apply(this, arguments); - this.movedAlertView = null; - }, - - undoMoveXBlock: function(event) { - var self = this, - $moveButton = $(event.target), - sourceLocator = $moveButton.data('source-locator'), - sourceDisplayName = $moveButton.data('source-display-name'), - sourceParentLocator = $moveButton.data('source-parent-locator'), - targetIndex = $moveButton.data('target-index'); - XBlockViewUtils.moveXBlock(sourceLocator, sourceParentLocator, targetIndex) - .done(function(response) { - // show XBlock element - $('.studio-xblock-wrapper[data-locator="' + response.move_source_locator + '"]').show(); - if (self.movedAlertView) { - self.movedAlertView.hide(); - } - self.movedAlertView = MoveXBlockUtils.showMovedNotification( - StringUtils.interpolate( - gettext('Move cancelled. "{sourceDisplayName}" has been moved back to its original ' + - 'location.'), - { - sourceDisplayName: sourceDisplayName - } - ) - ); - }); - } - }); - return MovedAlertView; - }); -}).call(this, define || RequireJS.define); diff --git a/common/static/common/templates/components/system-feedback.underscore b/common/static/common/templates/components/system-feedback.underscore index 625bc574a1c4..9168abc9d551 100644 --- a/common/static/common/templates/components/system-feedback.underscore +++ b/common/static/common/templates/components/system-feedback.underscore @@ -15,9 +15,8 @@ <% } %>
              -

              <%- title %><% if(titleHtml) { %> <%= titleHtml %> <% } %>

              +

              <%- title %>

              <% if(obj.message) { %>

              <%- message %>

              <% } %> - <% if(messageHtml) { %> <%= messageHtml %> <% } %>
              <% if(obj.actions) { %> From 485ffb1b68308433f8c2b83fb024c0e2802b8ec7 Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Thu, 2 Feb 2017 21:11:11 +0500 Subject: [PATCH 08/12] restrict move action --- cms/djangoapps/contentstore/utils.py | 17 ++ cms/djangoapps/contentstore/views/item.py | 39 +++- cms/djangoapps/contentstore/views/preview.py | 1 + .../views/tests/test_container_page.py | 44 ++++- .../contentstore/views/tests/test_item.py | 171 +++++++++++++++++- .../contentstore/views/tests/utils.py | 56 +++--- cms/static/js/spec/views/move_xblock_spec.js | 126 +++++++++++-- .../js/views/modals/move_xblock_modal.js | 23 ++- cms/static/js/views/move_xblock_list.js | 49 ++--- cms/static/sass/views/_container.scss | 13 +- cms/templates/js/move-xblock-list.underscore | 19 +- cms/templates/studio_xblock_wrapper.html | 3 +- .../xmodule/xmodule/library_content_module.py | 1 + .../xmodule/xmodule/library_root_xblock.py | 1 + 14 files changed, 478 insertions(+), 85 deletions(-) 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:
            -
              +
                <% for (var i = 0; i < xblocks.length; i++) { var xblock = xblocks[i]; %> diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index 644f926ef8f3..3b96d36db828 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -63,11 +63,16 @@ def _is_finished_loading(): is_done = num_wrappers == (num_initialized_xblocks + num_failed_xblocks) return (is_done, is_done) + def _loading_spinner_hidden(): + """ promise function to check loading spinner state """ + is_spinner_hidden = self.q(css='div.ui-loading.is-hidden').present + return is_spinner_hidden, is_spinner_hidden + # First make sure that an element with the view-container class is present on the page, # and then wait for the loading spinner to go away and all the xblocks to be initialized. return ( self.q(css='body.view-container').present and - self.q(css='div.ui-loading.is-hidden').present and + Promise(_loading_spinner_hidden, 'loading spinner is hidden.').fulfill() and Promise(_is_finished_loading, 'Finished rendering the xblock wrappers.').fulfill() ) @@ -101,6 +106,13 @@ def active_xblocks(self): """ return self._get_xblocks(".is-active ") + @property + def displayed_children(self): + """ + Return a list of displayed xblocks loaded on the container page. + """ + return self._get_xblocks()[0].children + @property def publish_title(self): """ @@ -262,6 +274,29 @@ def edit(self): """ return _click_edit(self, '.edit-button', '.xblock-studio_view') + def verify_confirmation_message(self, message): + """ + Verify for confirmation message. + """ + def _verify_message(): + """ promise function to check confirmation message state """ + text = self.q(css='#page-alert .alert.confirmation #alert-confirmation-title').text + return text and message in text[0] + + self.wait_for(_verify_message, description='confirmation message present') + + def click_undo_move_link(self): + """ + Click undo move link. + """ + click_css(self, '#page-alert .alert.confirmation .nav-actions .action-primary') + + def click_take_me_link(self): + """ + Click take me there link. + """ + click_css(self, '#page-alert .alert.confirmation .nav-actions .action-secondary', require_notification=False) + def add_missing_groups(self): """ Click the "add missing groups" link. @@ -382,7 +417,7 @@ def children(self): """ Will return any first-generation descendant xblocks of this xblock. """ - descendants = self.q(css=self._bounded_selector(self.BODY_SELECTOR)).map( + descendants = self.q(css=self._bounded_selector(self.BODY_SELECTOR)).filter(lambda el: el.is_displayed()).map( lambda el: XBlockWrapper(self.browser, el.get_attribute('data-locator'))).results # Now remove any non-direct descendants. @@ -468,6 +503,13 @@ def has_edit_visibility_button(self): """ return self.q(css=self._bounded_selector('.visibility-button')).is_present() + @property + def has_move_modal_button(self): + """ + Returns True if this xblock has move modal button else False + """ + return self.q(css=self._bounded_selector('.move-button')).is_present() + def go_to_container(self): """ Open the container page linked to by this xblock, and return @@ -505,6 +547,15 @@ def open_settings_tab(self): """ self._click_button('settings_tab') + def open_move_modal(self): + """ + Opens the move modal. + """ + click_css(self, '.move-button', require_notification=False) + self.wait_for( + lambda: self.q(css='.modal-window.move-modal').visible, description='move modal is visible' + ) + def set_field_val(self, field_display_name, field_value): """ If editing, set the value of a field. diff --git a/common/test/acceptance/pages/studio/move_xblock.py b/common/test/acceptance/pages/studio/move_xblock.py new file mode 100644 index 000000000000..2b89b4235970 --- /dev/null +++ b/common/test/acceptance/pages/studio/move_xblock.py @@ -0,0 +1,78 @@ +""" +Move XBlock Modal Page Object +""" +from bok_choy.page_object import PageObject +from common.test.acceptance.pages.common.utils import click_css + + +class MoveModalView(PageObject): + """ + A base class for move xblock + """ + + def __init__(self, browser): + """ + Arguments: + browser (selenium.webdriver): The Selenium-controlled browser that this page is loaded in. + """ + super(MoveModalView, self).__init__(browser) + + def is_browser_on_page(self): + return self.q(css='.modal-window.move-modal').present + + def url(self): + """ + Returns None because this is not directly accessible via URL. + """ + return None + + def save(self): + """ + Clicks save button. + """ + click_css(self, 'a.action-save') + + def cancel(self): + """ + Clicks cancel button. + """ + click_css(self, 'a.action-cancel', require_notification=False) + + def click_forward_button(self, source_index): + """ + Click forward button at specified `source_index`. + """ + css = '.move-modal .xblock-items-container .xblock-item' + self.q(css='.button-forward').nth(source_index).click() + self.wait_for( + lambda: len(self.q(css=css).results) > 0, description='children are visible' + ) + + def click_move_button(self): + """ + Click move button. + """ + self.q(css='.modal-actions .action-move').first.click() + + @property + def is_move_button_enabled(self): + """ + Returns True if move button on modal is enabled else False. + """ + return not self.q(css='.modal-actions .action-move.is-disabled').present + + @property + def children_category(self): + """ + Get displayed children category. + """ + return self.q(css='.xblock-items-container').attrs('data-items-category')[0] + + def navigate_to_category(self, category, navigation_options): + """ + Navigates to specifec `category` for a specified `source_index`. + """ + child_category = self.children_category + while child_category != category: + self.click_forward_button(navigation_options[child_category]) + child_category = self.children_category diff --git a/common/test/acceptance/tests/studio/test_studio_container.py b/common/test/acceptance/tests/studio/test_studio_container.py index 4efa7ed849a1..59e1947180dd 100644 --- a/common/test/acceptance/tests/studio/test_studio_container.py +++ b/common/test/acceptance/tests/studio/test_studio_container.py @@ -10,6 +10,7 @@ from common.test.acceptance.pages.studio.component_editor import ComponentEditorView, ComponentVisibilityEditorView from common.test.acceptance.pages.studio.container import ContainerPage from common.test.acceptance.pages.studio.html_component_editor import HtmlComponentEditorView +from common.test.acceptance.pages.studio.move_xblock import MoveModalView from common.test.acceptance.pages.studio.utils import add_discussion, drag from common.test.acceptance.pages.lms.courseware import CoursewarePage from common.test.acceptance.pages.lms.staff_view import StaffPage @@ -1136,3 +1137,136 @@ def test_common_problem_types_tab(self): "Text Input with Hints and Feedback", ] self.assertEqual(page.get_category_tab_components('problem', 1), expected_components) + + +@attr(shard=1) +class MoveComponentTest(ContainerBase): + """ + Tests of moving an XBlock to another XBlock. + """ + def setUp(self, is_staff=True): + super(MoveComponentTest, self).setUp(is_staff=is_staff) + self.container = ContainerPage(self.browser, None) + self.move_modal_view = MoveModalView(self.browser) + + self.navigation_options = { + 'section': 0, + 'subsection': 0, + 'unit': 1, + } + self.source_xblock_category = 'component' + self.message_move = 'Success! "HTML 11" has been moved.' + self.message_undo = 'Move cancelled. "HTML 11" has been moved back to its original location.' + + def populate_course_fixture(self, course_fixture): + """ + Sets up a course structure. + """ + # pylint: disable=attribute-defined-outside-init + self.unit_page1 = XBlockFixtureDesc('vertical', 'Test Unit 1').add_children( + XBlockFixtureDesc('html', 'HTML 11'), + XBlockFixtureDesc('html', 'HTML 12') + ) + self.unit_page2 = XBlockFixtureDesc('vertical', 'Test Unit 2').add_children( + XBlockFixtureDesc('html', 'HTML 21'), + XBlockFixtureDesc('html', 'HTML 22') + ) + course_fixture.add_children( + XBlockFixtureDesc('chapter', 'Test Section').add_children( + XBlockFixtureDesc('sequential', 'Test Subsection').add_children( + self.unit_page1, + self.unit_page2 + ) + ) + ) + + def verify_move_opertions(self, operation, component_display_names_after_operation): + """ + Verify move operations. + + Arguments: + operation (str), `move` or `undo move` operation + component_display_names_after_operation (dict) display names of components after operation in source/dest + """ + unit_page = self.go_to_unit_page(unit_name='Test Unit 1') + components = unit_page.displayed_children + self.assertEqual(len(components), 2) + + components[0].open_move_modal() + self.move_modal_view.navigate_to_category(self.source_xblock_category, self.navigation_options) + self.assertEqual(self.move_modal_view.is_move_button_enabled, True) + + self.move_modal_view.click_move_button() + self.container.verify_confirmation_message(self.message_move) + self.assertEqual(len(unit_page.displayed_children), 1) + + if operation == 'move': + self.container.click_take_me_link() + elif operation == 'undo_move': + self.container.click_undo_move_link() + self.container.verify_confirmation_message(self.message_undo) + + unit_page = ContainerPage(self.browser, self.unit_page2.locator) + components = unit_page.displayed_children + self.assertEqual( + [component.name for component in components], + component_display_names_after_operation + ) + + def test_move(self): + """ + Test if we can move a component successfully. + + Given I am a staff user + When I go to unit page in first section + Then I open the move modal + Then I navigate to unit in second section from within move modal + Then I see move button is enabled + Then I click on the move button + Then I see move operation successfull message + When I go to unit page in second section + Then I see move compoenent there + """ + self.verify_move_opertions( + operation='move', + component_display_names_after_operation=['HTML 21', 'HTML 22', 'HTML 11'] + ) + + def test_undo_move(self): + """ + Test if we can undo move a component successfully. + + Given I am a staff user + When I go to unit page in first section + Then I open the move modal + Then I click on the move button + Then I see move operation successfull message + When I clicked on undo move link + Then I verified that undo move operation is successfull + """ + self.verify_move_opertions( + operation='undo_move', + component_display_names_after_operation=['HTML 11', 'HTML 12'] + ) + + def test_a11y(self): + """ + Verify move modal a11y. + """ + unit_page = self.go_to_unit_page(unit_name='Test Unit 1') + + unit_page.a11y_audit.config.set_scope( + include=[".modal-window.move-modal"] + ) + unit_page.a11y_audit.config.set_rules({ + 'ignore': [ + 'color-contrast', + 'link-href', + ] + }) + + unit_page.displayed_children[0].open_move_modal() + + for category in ['section', 'subsection', 'component']: + self.move_modal_view.navigate_to_category(category, self.navigation_options) + unit_page.a11y_audit.check_for_accessibility_errors() From 83de1eb99f23455fb3fedd1ff4ac1f2e0966d08e Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Mon, 27 Feb 2017 18:29:11 +0500 Subject: [PATCH 10/12] Add content experiment test --- .../test/acceptance/pages/studio/container.py | 2 +- .../tests/studio/test_studio_container.py | 157 ++++++++++++++---- 2 files changed, 127 insertions(+), 32 deletions(-) diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index 3b96d36db828..85a6f83a77cb 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -291,7 +291,7 @@ def click_undo_move_link(self): """ click_css(self, '#page-alert .alert.confirmation .nav-actions .action-primary') - def click_take_me_link(self): + def click_take_me_there_link(self): """ Click take me there link. """ diff --git a/common/test/acceptance/tests/studio/test_studio_container.py b/common/test/acceptance/tests/studio/test_studio_container.py index 59e1947180dd..e496a14a0921 100644 --- a/common/test/acceptance/tests/studio/test_studio_container.py +++ b/common/test/acceptance/tests/studio/test_studio_container.py @@ -1154,9 +1154,10 @@ def setUp(self, is_staff=True): 'subsection': 0, 'unit': 1, } + self.source_component_display_name = 'HTML 11' self.source_xblock_category = 'component' - self.message_move = 'Success! "HTML 11" has been moved.' - self.message_undo = 'Move cancelled. "HTML 11" has been moved back to its original location.' + self.message_move = 'Success! "{display_name}" has been moved.' + self.message_undo = 'Move cancelled. "{display_name}" has been moved back to its original location.' def populate_course_fixture(self, course_fixture): """ @@ -1180,75 +1181,169 @@ def populate_course_fixture(self, course_fixture): ) ) - def verify_move_opertions(self, operation, component_display_names_after_operation): + def verify_move_opertions(self, unit_page, source_component, operation, component_display_names_after_operation): """ Verify move operations. Arguments: - operation (str), `move` or `undo move` operation + unit_page (Object) Unit container page. + source_component (Object) source XBlock object to be moved. + operation (str), `move` or `undo move` operation. component_display_names_after_operation (dict) display names of components after operation in source/dest """ - unit_page = self.go_to_unit_page(unit_name='Test Unit 1') - components = unit_page.displayed_children - self.assertEqual(len(components), 2) - - components[0].open_move_modal() + source_component.open_move_modal() self.move_modal_view.navigate_to_category(self.source_xblock_category, self.navigation_options) self.assertEqual(self.move_modal_view.is_move_button_enabled, True) self.move_modal_view.click_move_button() - self.container.verify_confirmation_message(self.message_move) + self.container.verify_confirmation_message( + self.message_move.format(display_name=self.source_component_display_name) + ) self.assertEqual(len(unit_page.displayed_children), 1) if operation == 'move': - self.container.click_take_me_link() + self.container.click_take_me_there_link() elif operation == 'undo_move': self.container.click_undo_move_link() - self.container.verify_confirmation_message(self.message_undo) + self.container.verify_confirmation_message( + self.message_undo.format(display_name=self.source_component_display_name) + ) - unit_page = ContainerPage(self.browser, self.unit_page2.locator) + unit_page = ContainerPage(self.browser, None) components = unit_page.displayed_children self.assertEqual( [component.name for component in components], component_display_names_after_operation ) - def test_move(self): + def test_move_component_successfully(self): """ Test if we can move a component successfully. Given I am a staff user - When I go to unit page in first section - Then I open the move modal - Then I navigate to unit in second section from within move modal - Then I see move button is enabled - Then I click on the move button - Then I see move operation successfull message - When I go to unit page in second section - Then I see move compoenent there + And I go to unit page in first section + And I open the move modal + And I navigate to unit in second section + And I see move button is enabled + When I click on the move button + Then I see move operation success message + And When I click on take me there link + Then I see moved component there. """ + unit_page = self.go_to_unit_page(unit_name='Test Unit 1') + components = unit_page.displayed_children + self.assertEqual(len(components), 2) + self.verify_move_opertions( + unit_page=unit_page, + source_component=components[0], operation='move', component_display_names_after_operation=['HTML 21', 'HTML 22', 'HTML 11'] ) - def test_undo_move(self): + def test_undo_move_component_successfully(self): """ Test if we can undo move a component successfully. Given I am a staff user - When I go to unit page in first section - Then I open the move modal - Then I click on the move button - Then I see move operation successfull message - When I clicked on undo move link - Then I verified that undo move operation is successfull + And I go to unit page in first section + And I open the move modal + When I click on the move button + Then I see move operation successful message + And When I clicked on undo move link + Then I see that undo move operation is successful """ + unit_page = self.go_to_unit_page(unit_name='Test Unit 1') + components = unit_page.displayed_children + self.assertEqual(len(components), 2) + self.verify_move_opertions( + unit_page=unit_page, + source_component=components[0], operation='undo_move', component_display_names_after_operation=['HTML 11', 'HTML 12'] ) + def test_content_experiment(self): + """ + Test if we can move a component of content experiment successfully. + + Given that I am a staff user + And I go to content experiment page + And I open the move dialogue modal + When I navigate to the unit in second section + Then I see move button is enabled + And when I click on the move button + Then I see move operation success message + And when I click on take me there link + Then I see moved component there + And when I undo move a component + Then I see that undo move operation success message + """ + # Add content experiment support to course. + self.course_fixture.add_advanced_settings({ + u'advanced_modules': {'value': ['split_test']}, + }) + + # Create group configurations + # pylint: disable=protected-access + self.course_fixture._update_xblock(self.course_fixture._course_location, { + 'metadata': { + u'user_partitions': [ + create_user_partition_json( + 0, + 'Test Group Configuration', + 'Description of the group configuration.', + [Group('0', 'Group A'), Group('1', 'Group B')] + ), + ], + }, + }) + + # Add split test to unit_page1 and assign newly created group configuration to it + split_test = XBlockFixtureDesc('split_test', 'Test Content Experiment', metadata={'user_partition_id': 0}) + self.course_fixture.create_xblock(self.unit_page1.locator, split_test) + + # Visit content experiment container page. + unit_page = ContainerPage(self.browser, split_test.locator) + unit_page.visit() + + group_a_locator = unit_page.displayed_children[0].locator + + # Add some components to Group A. + self.course_fixture.create_xblock( + group_a_locator, XBlockFixtureDesc('html', 'HTML 311') + ) + self.course_fixture.create_xblock( + group_a_locator, XBlockFixtureDesc('html', 'HTML 312') + ) + + # Go to group page to move it's component. + group_container_page = ContainerPage(self.browser, group_a_locator) + group_container_page.visit() + + # Verify content experiment block has correct groups and components. + components = group_container_page.displayed_children + self.assertEqual(len(components), 2) + + self.source_component_display_name = 'HTML 311' + + # Verify undo move operation for content experiment. + self.verify_move_opertions( + unit_page=group_container_page, + source_component=components[0], + operation='undo_move', + component_display_names_after_operation=['HTML 311', 'HTML 312'] + ) + + # Verify move operation for content experiment. + self.verify_move_opertions( + unit_page=group_container_page, + source_component=components[0], + operation='move', + component_display_names_after_operation=['HTML 21', 'HTML 22', 'HTML 311'] + ) + def test_a11y(self): """ Verify move modal a11y. @@ -1260,8 +1355,8 @@ def test_a11y(self): ) unit_page.a11y_audit.config.set_rules({ 'ignore': [ - 'color-contrast', - 'link-href', + 'color-contrast', # TODO: AC-716 + 'link-href', # TODO: AC-716 ] }) From e83bee65bddf0ed93fe5266e4a4a9c6d329d5114 Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Fri, 3 Mar 2017 16:20:27 +0500 Subject: [PATCH 11/12] Fixed Backend: Move component directly to content experiment level (component will now be the sibling of groups) Fixed Move button is disabled when moving components inside conditional module Fixed When navigating upward/backward, target parent is not set correct, resulting source to be moved in the previously selected parent --- cms/djangoapps/contentstore/views/item.py | 6 +- .../contentstore/views/tests/test_item.py | 24 +++- cms/static/js/models/xblock_info.js | 4 + cms/static/js/spec/views/move_xblock_spec.js | 130 +++++++++++------- .../js/spec/views/pages/container_spec.js | 3 + .../views/pages/container_subviews_spec.js | 3 + .../js/views/modals/move_xblock_modal.js | 64 ++++----- cms/static/js/views/pages/container.js | 20 ++- .../js/views/utils/move_xblock_utils.js | 49 +++++-- cms/static/js/views/utils/xblock_utils.js | 7 +- .../test/acceptance/pages/studio/container.py | 12 ++ .../tests/studio/test_studio_container.py | 70 +++++----- 12 files changed, 249 insertions(+), 143 deletions(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 91c4f75bfb66..5e902c5898b3 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -739,6 +739,8 @@ def _move_item(source_usage_key, target_parent_usage_key, user, target_index=Non 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 target_parent_type == 'split_test': + error = 'You can not move an item directly into content experiment.' elif source_index is None: error = '{source_usage_key} not found in {parent_usage_key}.'.format( source_usage_key=unicode(source_usage_key), @@ -1119,7 +1121,8 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F xblock_info = { 'id': unicode(xblock.location), 'display_name': xblock.display_name_with_default, - 'category': xblock.category + 'category': xblock.category, + 'has_children': xblock.has_children } if is_concise: if child_info and len(child_info.get('children', [])) > 0: @@ -1127,7 +1130,6 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F # 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, diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index afad862fd2c4..28e2b5f11f25 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -948,17 +948,17 @@ def test_invalid_move(self): """ Test invalid move. """ - parent_loc = self.store.get_parent_location(self.chapter_usage_key) - response = self._move_component(self.chapter_usage_key, self.usage_key) + parent_loc = self.store.get_parent_location(self.html_usage_key) + response = self._move_component(self.html_usage_key, self.seq_usage_key) self.assertEqual(response.status_code, 400) response = json.loads(response.content) expected_error = 'You can not move {source_type} into {target_type}.'.format( - source_type=self.chapter_usage_key.block_type, - target_type=self.usage_key.block_type + source_type=self.html_usage_key.block_type, + target_type=self.seq_usage_key.block_type ) self.assertEqual(expected_error, response['error']) - new_parent_loc = self.store.get_parent_location(self.chapter_usage_key) + new_parent_loc = self.store.get_parent_location(self.html_usage_key) self.assertEqual(new_parent_loc, parent_loc) def test_move_current_parent(self): @@ -1039,11 +1039,23 @@ def test_move_content_experiment_components(self): def test_move_into_content_experiment_groups(self): """ - Test that a component can be moved to content experiment. + Test that a component can be moved to content experiment groups. """ 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_into_content_experiment_level(self): + """ + Test that a component can not be moved directly to content experiment level. + """ + self.setup_and_verify_content_experiment(0) + response = self._move_component(self.html_usage_key, self.split_test_usage_key) + self.assertEqual(response.status_code, 400) + response = json.loads(response.content) + + self.assertEqual(response['error'], 'You can not move an item directly into content experiment.') + self.assertEqual(self.store.get_parent_location(self.html_usage_key), self.vert_usage_key) + 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. diff --git a/cms/static/js/models/xblock_info.js b/cms/static/js/models/xblock_info.js index e3fd1c706cc5..fd7fdd8d9054 100644 --- a/cms/static/js/models/xblock_info.js +++ b/cms/static/js/models/xblock_info.js @@ -49,6 +49,10 @@ function(Backbone, _, str, ModuleUtils) { * publishing info was explicitly requested. */ 'published_by': null, + /** + * True if the xblock is a parentable xblock. + */ + has_children: null, /** * True if the xblock has changes. * Note: this is not always provided as a performance optimization. It is only provided for diff --git a/cms/static/js/spec/views/move_xblock_spec.js b/cms/static/js/spec/views/move_xblock_spec.js index 6e7f9664089d..3911124f0cb5 100644 --- a/cms/static/js/spec/views/move_xblock_spec.js +++ b/cms/static/js/spec/views/move_xblock_spec.js @@ -1,17 +1,19 @@ -define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', +define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpers', 'js/spec_helpers/edit_helpers', 'common/js/spec_helpers/template_helpers', 'common/js/spec_helpers/view_helpers', - 'js/views/modals/move_xblock_modal', 'edx-ui-toolkit/js/utils/html-utils', + 'js/views/modals/move_xblock_modal', 'js/views/pages/container', 'edx-ui-toolkit/js/utils/html-utils', 'edx-ui-toolkit/js/utils/string-utils', 'js/models/xblock_info'], - function($, _, AjaxHelpers, TemplateHelpers, ViewHelpers, MoveXBlockModal, HtmlUtils, StringUtils, XBlockInfo) { + function($, _, AjaxHelpers, EditHelpers, TemplateHelpers, ViewHelpers, MoveXBlockModal, ContainerPage, HtmlUtils, + StringUtils, XBlockInfo) { 'use strict'; describe('MoveXBlock', function() { var modal, showModal, renderViews, createXBlockInfo, createCourseOutline, courseOutlineOptions, parentChildMap, categoryMap, createChildXBlockInfo, xblockAncestorInfo, courseOutline, verifyBreadcrumbViewInfo, verifyListViewInfo, getDisplayedInfo, clickForwardButton, clickBreadcrumbButton, verifyXBlockInfo, nextCategory, verifyMoveEnabled, getSentRequests, - verifyNotificationStatus, sendMoveXBlockRequest, moveXBlockWithSuccess, - verifyConfirmationFeedbackTitleHtml, verifyConfirmationFeedbackRedirectLinkHtml, - verifyUndoConfirmationFeedbackTitleHtml, verifyConfirmationFeedbackUndoMoveActionHtml, + verifyNotificationStatus, sendMoveXBlockRequest, moveXBlockWithSuccess, getMovedAlertNotification, + verifyConfirmationFeedbackTitleText, verifyConfirmationFeedbackRedirectLinkText, + verifyUndoConfirmationFeedbackTitleText, verifyConfirmationFeedbackUndoMoveActionText, + sourceParentXBlockInfo, mockContainerPage, createContainerPage, containerPage, sourceDisplayName = 'component_display_name_0', sourceLocator = 'component_ID_0', sourceParentLocator = 'unit_ID_0'; @@ -62,13 +64,31 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe ] }; + sourceParentXBlockInfo = new XBlockInfo({ + id: sourceParentLocator, + display_name: 'unit_display_name_0', + category: 'vertical' + }); + + createContainerPage = function() { + containerPage = new ContainerPage({ + model: sourceParentXBlockInfo, + templates: EditHelpers.mockComponentTemplates, + el: $('#content'), + isUnitPage: true + }); + }; + beforeEach(function() { setFixtures("
                "); + mockContainerPage = readFixtures('mock/mock-container-page.underscore'); TemplateHelpers.installTemplates([ 'basic-modal', 'modal-button', 'move-xblock-modal' ]); + appendSetFixtures(mockContainerPage); + createContainerPage(); courseOutline = createCourseOutline(courseOutlineOptions); showModal(); }); @@ -76,6 +96,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe afterEach(function() { modal.hide(); courseOutline = null; + containerPage.remove(); }); showModal = function() { @@ -85,11 +106,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe display_name: sourceDisplayName, category: 'component' }), - sourceParentXBlockInfo: new XBlockInfo({ - id: sourceParentLocator, - display_name: 'unit_display_name_0', - category: 'vertical' - }), + sourceParentXBlockInfo: sourceParentXBlockInfo, XBlockUrlRoot: '/xblock' }); modal.show(); @@ -338,6 +355,13 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe ViewHelpers.verifyNotificationHidden(notificationSpy); }; + /** + * Get move alert confirmation message HTML + */ + getMovedAlertNotification = function() { + return $('#page-alert'); + }; + /** * Send move xblock request. * @@ -384,33 +408,36 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe }); modal.$el.find('.modal-actions .action-move').click(); sendMoveXBlockRequest(requests, sourceLocator); - expect(modal.movedAlertView).toBeDefined(); - verifyConfirmationFeedbackTitleHtml(sourceDisplayName); - verifyConfirmationFeedbackRedirectLinkHtml(); - verifyConfirmationFeedbackUndoMoveActionHtml(); + AjaxHelpers.expectJsonRequest(requests, 'GET', '/xblock/' + sourceParentLocator); + AjaxHelpers.respondWithJson(requests, sourceParentXBlockInfo); + expect(getMovedAlertNotification().html().length).not.toEqual(0); + verifyConfirmationFeedbackTitleText(sourceDisplayName); + verifyConfirmationFeedbackRedirectLinkText(); + verifyConfirmationFeedbackUndoMoveActionText(); }; /** - * Verify success banner message html has correct title html. + * Verify success banner message html has correct title text. * * @param {String} displayName XBlock display name */ - verifyConfirmationFeedbackTitleHtml = function(displayName) { - expect(modal.movedAlertView.$el.find('.title').html().trim()) - .toEqual(StringUtils.interpolate('Success! "{displayName}" has been moved.', - { - displayName: displayName - }) - ); + verifyConfirmationFeedbackTitleText = function(displayName) { + expect(getMovedAlertNotification().find('.title').html() + .trim()) + .toEqual(StringUtils.interpolate('Success! "{displayName}" has been moved.', + { + displayName: displayName + }) + ); }; /** - * Verify undo success banner message html has correct title html. + * Verify undo success banner message html has correct title text. * * @param {String} displayName XBlock display name */ - verifyUndoConfirmationFeedbackTitleHtml = function(displayName) { - expect(modal.movedAlertView.$el.find('.title').html()).toEqual( + verifyUndoConfirmationFeedbackTitleText = function(displayName) { + expect(getMovedAlertNotification().find('.title').html()).toEqual( StringUtils.interpolate( 'Move cancelled. "{sourceDisplayName}" has been moved back to its original location.', { @@ -421,25 +448,18 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe }; /** - * Verify success banner message html has correct redirect link html. + * Verify success banner message html has correct redirect link text. */ - verifyConfirmationFeedbackRedirectLinkHtml = function() { - expect(modal.movedAlertView.$el.find('.copy').html().indexOf( - HtmlUtils.HTML( - '' - ) !== -1 - )).toBeTruthy(); + verifyConfirmationFeedbackRedirectLinkText = function() { + expect(getMovedAlertNotification().find('.nav-actions .action-secondary').html()) + .toEqual('Take me to the new location'); }; /** - * Verify success banner message html has correct undo move button html. + * Verify success banner message html has correct undo move text. */ - verifyConfirmationFeedbackUndoMoveActionHtml = function() { - expect(modal.movedAlertView.$el.find('.copy').html().indexOf( - HtmlUtils.HTML( - '' - ) !== -1 - )).toBeTruthy(); + verifyConfirmationFeedbackUndoMoveActionText = function() { + expect(getMovedAlertNotification().find('.nav-actions .action-primary').html()).toEqual('Undo move'); }; /** @@ -633,6 +653,24 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe expect(modal.$el.find('.modal-actions .action-move').hasClass('is-disabled')).toBeFalsy(); }); + it('is enabled when moving a component inside a parentable component', function() { + // create a source parent with has_childern set true + modal.sourceParentXBlockInfo = new XBlockInfo({ + category: 'conditional', + display_name: 'Parentable Component', + has_children: true, + id: 'PARENTABLE_ID' + }); + // navigate and verify move button is enabled + renderViews(courseOutline); + _.each(_.range(3), function() { + clickForwardButton(0); + }); + + // move is enabled when moving a component. + expect(modal.$el.find('.modal-actions .action-move').hasClass('is-disabled')).toBeFalsy(); + }); + it('is disabled when navigating to any non-parentable component', function() { var nonParentableXBlockInfo = { category: 'html', @@ -651,7 +689,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe it('can not move in a disabled state', function() { verifyMoveEnabled(false); modal.$el.find('.modal-actions .action-move').click(); - expect(modal.movedAlertView).toBeNull(); + expect(getMovedAlertNotification().html().length).toEqual(0); expect(getSentRequests().length).toEqual(0); }); @@ -662,7 +700,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe it('do not move an xblock when cancel button is clicked', function() { modal.$el.find('.modal-actions .action-cancel').click(); - expect(modal.movedAlertView).toBeNull(); + expect(getMovedAlertNotification().html().length).toEqual(0); expect(getSentRequests().length).toEqual(0); }); @@ -670,13 +708,13 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe var sourceIndex = 0, requests = AjaxHelpers.requests(this); moveXBlockWithSuccess(requests); - modal.movedAlertView.$el.find('.action-save').click(); + getMovedAlertNotification().find('.action-save').click(); AjaxHelpers.respondWithJson(requests, { move_source_locator: sourceLocator, parent_locator: sourceParentLocator, target_index: sourceIndex }); - verifyUndoConfirmationFeedbackTitleHtml(sourceDisplayName); + verifyUndoConfirmationFeedbackTitleText(sourceDisplayName); }); }); @@ -698,7 +736,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe requests = AjaxHelpers.requests(this); moveXBlockWithSuccess(requests); notificationSpy = ViewHelpers.createNotificationSpy(); - modal.movedAlertView.$el.find('.action-save').click(); + getMovedAlertNotification().find('.action-save').click(); verifyNotificationStatus(requests, notificationSpy, 'Undo moving'); }); @@ -719,7 +757,7 @@ define(['jquery', 'underscore', 'edx-ui-toolkit/js/utils/spec-helpers/ajax-helpe var requests = AjaxHelpers.requests(this), notificationSpy = ViewHelpers.createNotificationSpy('Error'); moveXBlockWithSuccess(requests); - modal.movedAlertView.$el.find('.action-save').click(); + getMovedAlertNotification().find('.action-save').click(); AjaxHelpers.respondWithError(requests); ViewHelpers.verifyNotificationShowing(notificationSpy, "Studio's having trouble saving your work"); }); diff --git a/cms/static/js/spec/views/pages/container_spec.js b/cms/static/js/spec/views/pages/container_spec.js index 717e5318f770..4b56b837b66c 100644 --- a/cms/static/js/spec/views/pages/container_spec.js +++ b/cms/static/js/spec/views/pages/container_spec.js @@ -49,6 +49,9 @@ define(['jquery', 'underscore', 'underscore.string', 'edx-ui-toolkit/js/utils/sp afterEach(function() { EditHelpers.uninstallMockXBlock(); + if (containerPage !== undefined) { + containerPage.remove(); + } }); respondWithHtml = function(html) { diff --git a/cms/static/js/spec/views/pages/container_subviews_spec.js b/cms/static/js/spec/views/pages/container_subviews_spec.js index 947e73c1dc13..13bfc8d3e271 100644 --- a/cms/static/js/spec/views/pages/container_subviews_spec.js +++ b/cms/static/js/spec/views/pages/container_subviews_spec.js @@ -35,6 +35,9 @@ define(['jquery', 'underscore', 'underscore.string', 'edx-ui-toolkit/js/utils/sp afterEach(function() { delete window.course; + if (containerPage !== undefined) { + containerPage.remove(); + } }); defaultXBlockInfo = { diff --git a/cms/static/js/views/modals/move_xblock_modal.js b/cms/static/js/views/modals/move_xblock_modal.js index 932dd86c2c40..2da157b4be96 100644 --- a/cms/static/js/views/modals/move_xblock_modal.js +++ b/cms/static/js/views/modals/move_xblock_modal.js @@ -41,7 +41,6 @@ function($, Backbone, _, gettext, BaseView, XBlockViewUtils, MoveXBlockUtils, Ht initialize: function() { var self = this; BaseModal.prototype.initialize.call(this); - this.listenTo(Backbone, 'move:breadcrumbRendered', this.focusModal); this.sourceXBlockInfo = this.options.sourceXBlockInfo; this.sourceParentXBlockInfo = this.options.sourceParentXBlockInfo; this.targetParentXBlockInfo = null; @@ -57,9 +56,9 @@ function($, Backbone, _, gettext, BaseView, XBlockViewUtils, MoveXBlockUtils, Ht $('.breadcrumb-container').removeClass('is-hidden'); self.renderViews(courseOutlineInfo, ancestorInfo); }); - this.movedAlertView = null; - this.isValidMove = false; + this.listenTo(Backbone, 'move:breadcrumbRendered', this.focusModal); this.listenTo(Backbone, 'move:enableMoveOperation', this.enableMoveOperation); + this.listenTo(Backbone, 'move:hideMoveModal', this.hide); }, getTitle: function() { @@ -137,15 +136,22 @@ function($, Backbone, _, gettext, BaseView, XBlockViewUtils, MoveXBlockUtils, Ht } }, - isValidCategory: function(sourceParentType, targetParentType, targetHasChildren) { - var basicBlockTypes = ['course', 'chapter', 'sequential', 'vertical']; + isValidCategory: function(targetParentXBlockInfo) { + var basicBlockTypes = ['course', 'chapter', 'sequential', 'vertical'], + sourceParentType = this.sourceParentXBlockInfo.get('category'), + targetParentType = targetParentXBlockInfo.get('category'), + sourceParentHasChildren = this.sourceParentXBlockInfo.get('has_children'), + targetParentHasChildren = targetParentXBlockInfo.get('has_children'); + // Treat source parent component as vertical to support move child components under content experiment // and other similar xblocks. - // eslint-disable-next-line no-param-reassign - sourceParentType = sourceParentType === 'split_test' ? 'vertical' : sourceParentType; + if (sourceParentHasChildren && !_.contains(basicBlockTypes, sourceParentType)) { + sourceParentType = 'vertical'; // eslint-disable-line no-param-reassign + } + // Treat target parent component as a vertical to support move to parentable target parent components. // Also, moving a component directly to content experiment is not allowed, we need to visit to group level. - if (targetHasChildren && !_.contains(basicBlockTypes, targetParentType) && + if (targetParentHasChildren && !_.contains(basicBlockTypes, targetParentType) && targetParentType !== 'split_test') { targetParentType = 'vertical'; // eslint-disable-line no-param-reassign } @@ -153,44 +159,28 @@ function($, Backbone, _, gettext, BaseView, XBlockViewUtils, MoveXBlockUtils, Ht }, enableMoveOperation: function(targetParentXBlockInfo) { - var isValidMove = false, - sourceParentType = this.sourceParentXBlockInfo.get('category'), - targetParentType = targetParentXBlockInfo.get('category'), - targetHasChildren = targetParentXBlockInfo.get('has_children'); + var isValidMove = false; - if (this.isValidCategory(sourceParentType, targetParentType, targetHasChildren) && + // update target parent on navigation + this.targetParentXBlockInfo = targetParentXBlockInfo; + if (this.isValidCategory(targetParentXBlockInfo) && this.sourceParentXBlockInfo.id !== targetParentXBlockInfo.id && // same parent case this.sourceXBlockInfo.id !== targetParentXBlockInfo.id) { // same source item case isValidMove = true; - this.targetParentXBlockInfo = targetParentXBlockInfo; } this.updateMoveState(isValidMove); }, moveXBlock: function() { - var self = this; - XBlockViewUtils.moveXBlock(self.sourceXBlockInfo.id, self.targetParentXBlockInfo.id) - .done(function(response) { - // hide modal - self.hide(); - // hide xblock element - $("li.studio-xblock-wrapper[data-locator='" + self.sourceXBlockInfo.id + "']").hide(); - self.movedAlertView = MoveXBlockUtils.showMovedNotification( - StringUtils.interpolate( - gettext('Success! "{displayName}" has been moved.'), - { - displayName: self.sourceXBlockInfo.get('display_name') - } - ), - { - sourceDisplayName: self.sourceXBlockInfo.get('display_name'), - sourceLocator: self.sourceXBlockInfo.id, - sourceParentLocator: self.sourceParentXBlockInfo.id, - targetParentLocator: response.parent_locator, - targetIndex: response.source_index - } - ); - }); + MoveXBlockUtils.moveXBlock( + { + sourceXBlockElement: $("li.studio-xblock-wrapper[data-locator='" + this.sourceXBlockInfo.id + "']"), + sourceDisplayName: this.sourceXBlockInfo.get('display_name'), + sourceLocator: this.sourceXBlockInfo.id, + sourceParentLocator: this.sourceParentXBlockInfo.id, + targetParentLocator: this.targetParentXBlockInfo.id + } + ); } }); diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index 6334ace61885..f26723270b43 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -2,11 +2,12 @@ * XBlockContainerPage is used to display Studio's container page for an xblock which has children. * This page allows the user to understand and manipulate the xblock and its children. */ -define(['jquery', 'underscore', 'gettext', 'js/views/pages/base_page', 'common/js/components/utils/view_utils', - 'js/views/container', 'js/views/xblock', 'js/views/components/add_xblock', 'js/views/modals/edit_xblock', - 'js/views/modals/move_xblock_modal', 'js/models/xblock_info', 'js/views/xblock_string_field_editor', - 'js/views/pages/container_subviews', 'js/views/unit_outline', 'js/views/utils/xblock_utils'], - function($, _, gettext, BasePage, ViewUtils, ContainerView, XBlockView, AddXBlockComponent, +define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/pages/base_page', + 'common/js/components/utils/view_utils', 'js/views/container', 'js/views/xblock', + 'js/views/components/add_xblock', 'js/views/modals/edit_xblock', 'js/views/modals/move_xblock_modal', + 'js/models/xblock_info', 'js/views/xblock_string_field_editor', 'js/views/pages/container_subviews', + 'js/views/unit_outline', 'js/views/utils/xblock_utils'], + function($, _, Backbone, gettext, BasePage, ViewUtils, ContainerView, XBlockView, AddXBlockComponent, EditXBlockModal, MoveXBlockModal, XBlockInfo, XBlockStringFieldEditor, ContainerSubviews, UnitOutlineView, XBlockUtils) { 'use strict'; @@ -81,6 +82,8 @@ define(['jquery', 'underscore', 'gettext', 'js/views/pages/base_page', 'common/j }); this.unitOutlineView.render(); } + + this.listenTo(Backbone, 'move:onXBlockMoved', this.onXBlockMoved); }, getViewParameters: function() { @@ -283,6 +286,13 @@ define(['jquery', 'underscore', 'gettext', 'js/views/pages/base_page', 'common/j this.model.fetch(); }, + /* + After move operation is complete, updates the xblock information from server . + */ + onXBlockMoved: function() { + this.model.fetch(); + }, + onNewXBlock: function(xblockElement, scrollOffset, is_duplicate, data) { ViewUtils.setScrollOffset(xblockElement, scrollOffset); xblockElement.data('locator', data.locator); diff --git a/cms/static/js/views/utils/move_xblock_utils.js b/cms/static/js/views/utils/move_xblock_utils.js index 820c8abc59b9..e3344c5cb801 100644 --- a/cms/static/js/views/utils/move_xblock_utils.js +++ b/cms/static/js/views/utils/move_xblock_utils.js @@ -4,25 +4,53 @@ define([ 'jquery', 'underscore', + 'backbone', 'common/js/components/views/feedback', 'common/js/components/views/feedback_alert', 'js/views/utils/xblock_utils', 'js/views/utils/move_xblock_utils', 'edx-ui-toolkit/js/utils/string-utils' ], -function($, _, Feedback, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtils) { +function($, _, Backbone, Feedback, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtils) { 'use strict'; - var redirectLink, undoMoveXBlock, showMovedNotification, hideMovedNotification; + var redirectLink, moveXBlock, undoMoveXBlock, showMovedNotification, hideMovedNotification; redirectLink = function(link) { window.location.href = link; }; + moveXBlock = function(data) { + XBlockViewUtils.moveXBlock(data.sourceLocator, data.targetParentLocator) + .done(function(response) { + // hide modal + Backbone.trigger('move:hideMoveModal'); + // hide xblock element + data.sourceXBlockElement.hide(); + showMovedNotification( + StringUtils.interpolate( + gettext('Success! "{displayName}" has been moved.'), + { + displayName: data.sourceDisplayName + } + ), + { + sourceXBlockElement: data.sourceXBlockElement, + sourceDisplayName: data.sourceDisplayName, + sourceLocator: data.sourceLocator, + sourceParentLocator: data.sourceParentLocator, + targetParentLocator: data.targetParentLocator, + targetIndex: response.source_index + } + ); + Backbone.trigger('move:onXBlockMoved'); + }); + }; + undoMoveXBlock = function(data) { XBlockViewUtils.moveXBlock(data.sourceLocator, data.sourceParentLocator, data.targetIndex) - .done(function(response) { + .done(function() { // show XBlock element - $('.studio-xblock-wrapper[data-locator="' + response.move_source_locator + '"]').show(); + data.sourceXBlockElement.show(); showMovedNotification( StringUtils.interpolate( gettext('Move cancelled. "{sourceDisplayName}" has been moved back to its original location.'), @@ -31,6 +59,7 @@ function($, _, Feedback, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtil } ) ); + Backbone.trigger('move:onXBlockMoved'); }); }; @@ -44,15 +73,10 @@ function($, _, Feedback, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtil primary: { text: gettext('Undo move'), class: 'action-save', - data: JSON.stringify({ - sourceDisplayName: data.sourceDisplayName, - sourceLocator: data.sourceLocator, - sourceParentLocator: data.sourceParentLocator, - targetIndex: data.targetIndex - }), click: function() { undoMoveXBlock( { + sourceXBlockElement: data.sourceXBlockElement, sourceDisplayName: data.sourceDisplayName, sourceLocator: data.sourceLocator, sourceParentLocator: data.sourceParentLocator, @@ -65,9 +89,6 @@ function($, _, Feedback, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtil { text: gettext('Take me to the new location'), class: 'action-cancel', - data: JSON.stringify({ - targetParentLocator: data.targetParentLocator - }), click: function() { redirectLink('/container/' + data.targetParentLocator); } @@ -100,6 +121,8 @@ function($, _, Feedback, AlertView, XBlockViewUtils, MoveXBlockUtils, StringUtil return { redirectLink: redirectLink, + moveXBlock: moveXBlock, + undoMoveXBlock: undoMoveXBlock, showMovedNotification: showMovedNotification, hideMovedNotification: hideMovedNotification }; diff --git a/cms/static/js/views/utils/xblock_utils.js b/cms/static/js/views/utils/xblock_utils.js index 860d055f3ad8..970ad6162dd4 100644 --- a/cms/static/js/views/utils/xblock_utils.js +++ b/cms/static/js/views/utils/xblock_utils.js @@ -272,7 +272,8 @@ define(['jquery', 'underscore', 'gettext', 'common/js/components/utils/view_util findXBlockInfo = function(xblockWrapperElement, defaultXBlockInfo) { var xblockInfo = defaultXBlockInfo, xblockElement, - displayName; + displayName, + hasChildren; if (xblockWrapperElement.length > 0) { xblockElement = xblockWrapperElement.find('.xblock'); displayName = xblockWrapperElement.find( @@ -283,11 +284,13 @@ define(['jquery', 'underscore', 'gettext', 'common/js/components/utils/view_util if (!displayName) { displayName = xblockElement.find('.component-header').text().trim(); } + hasChildren = defaultXBlockInfo ? defaultXBlockInfo.get('has_children') : false; xblockInfo = new XBlockInfo({ id: xblockWrapperElement.data('locator'), courseKey: xblockWrapperElement.data('course-key'), category: xblockElement.data('block-type'), - display_name: displayName + display_name: displayName, + has_children: hasChildren }); } return xblockInfo; diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index 85a6f83a77cb..99d1e41fa892 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -229,6 +229,18 @@ def view_published_version(self): self.q(css='.button-view').first.click() self._switch_to_lms() + def verify_publish_title(self, expected_title): + """ + Waits for the publish title to change to the expected value. + """ + def wait_for_title_change(): + """ + Promise function to check publish title. + """ + return (self.publish_title == expected_title, self.publish_title) + + Promise(wait_for_title_change, "Publish title incorrect. Found '" + self.publish_title + "'").fulfill() + def preview(self): """ Clicks "Preview", which will open the draft version of the unit page in the LMS. diff --git a/common/test/acceptance/tests/studio/test_studio_container.py b/common/test/acceptance/tests/studio/test_studio_container.py index e496a14a0921..ab3ae1e9283c 100644 --- a/common/test/acceptance/tests/studio/test_studio_container.py +++ b/common/test/acceptance/tests/studio/test_studio_container.py @@ -664,7 +664,7 @@ def test_publishing(self): And the last saved text contains "Last published" """ unit = self.go_to_unit_page() - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) # Start date set in course fixture to 1970. self._verify_release_date_info( unit, self.RELEASE_TITLE_RELEASED, 'Jan 01, 1970 at 00:00 UTC\nwith Section "Test Section"' @@ -675,11 +675,11 @@ def test_publishing(self): # Add a component to the page so it will have unpublished changes. add_discussion(unit) - self._verify_publish_title(unit, self.DRAFT_STATUS) + unit.verify_publish_title(self.DRAFT_STATUS) self._verify_last_published_and_saved(unit, self.LAST_PUBLISHED, self.LAST_SAVED) unit.publish_action.click() unit.wait_for_ajax() - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) self._verify_last_published_and_saved(unit, self.LAST_PUBLISHED, self.LAST_PUBLISHED) def test_discard_changes(self): @@ -696,9 +696,9 @@ def test_discard_changes(self): """ unit = self.go_to_unit_page() add_discussion(unit) - self._verify_publish_title(unit, self.DRAFT_STATUS) + unit.verify_publish_title(self.DRAFT_STATUS) unit.discard_changes() - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) def test_view_live_no_changes(self): """ @@ -757,7 +757,7 @@ def test_initially_unlocked_visible_to_students(self): Then I see the content in the unit """ unit = self.go_to_unit_page("Unlocked Section", "Unlocked Subsection", "Unlocked Unit") - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) self.assertTrue(unit.currently_visible_to_students) self._verify_release_date_info( unit, self.RELEASE_TITLE_RELEASED, self.past_start_date_text + '\n' + 'with Section "Unlocked Section"' @@ -783,7 +783,7 @@ def test_locked_visible_to_staff_only(self): self.assertTrue(checked) self.assertFalse(unit.currently_visible_to_students) self.assertFalse(unit.shows_inherited_staff_lock()) - self._verify_publish_title(unit, self.LOCKED_STATUS) + unit.verify_publish_title(self.LOCKED_STATUS) self._view_published_version(unit) # Will initially be in staff view, locked component should be visible. self._verify_components_visible(['problem']) @@ -802,7 +802,7 @@ def test_initially_locked_not_visible_to_students(self): Then I do not see any content in the unit """ unit = self.go_to_unit_page("Section With Locked Unit", "Subsection With Locked Unit", "Locked Unit") - self._verify_publish_title(unit, self.LOCKED_STATUS) + unit.verify_publish_title(self.LOCKED_STATUS) self.assertFalse(unit.currently_visible_to_students) self._verify_release_date_info( unit, self.RELEASE_TITLE_RELEASE, @@ -826,7 +826,7 @@ def test_unlocked_visible_to_all(self): unit = self.go_to_unit_page("Section With Locked Unit", "Subsection With Locked Unit", "Locked Unit") checked = unit.toggle_staff_lock() self.assertFalse(checked) - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) self.assertTrue(unit.currently_visible_to_students) self._view_published_version(unit) # Will initially be in staff view, components always visible. @@ -894,10 +894,10 @@ def test_published_unit_with_draft_child(self): component.edit() HtmlComponentEditorView(self.browser, component.locator).set_content_and_save(modified_content) self.assertEqual(component.student_content, modified_content) - self._verify_publish_title(unit, self.DRAFT_STATUS) + unit.verify_publish_title(self.DRAFT_STATUS) unit.publish_action.click() unit.wait_for_ajax() - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) self._view_published_version(unit) self.assertIn(modified_content, self.courseware.xblock_component_html_content(0)) @@ -917,10 +917,10 @@ def test_cancel_does_not_create_draft(self): component.edit() HtmlComponentEditorView(self.browser, component.locator).set_content_and_cancel("modified content") self.assertEqual(component.student_content, "Body of HTML Unit.") - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) self.browser.refresh() unit.wait_for_page() - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) def test_delete_child_in_published_unit(self): """ @@ -936,10 +936,10 @@ def test_delete_child_in_published_unit(self): """ unit = self.go_to_unit_page() unit.delete(0) - self._verify_publish_title(unit, self.DRAFT_STATUS) + unit.verify_publish_title(self.DRAFT_STATUS) unit.publish_action.click() unit.wait_for_ajax() - self._verify_publish_title(unit, self.PUBLISHED_LIVE_STATUS) + unit.verify_publish_title(self.PUBLISHED_LIVE_STATUS) self._view_published_version(unit) self.assertEqual(0, self.courseware.num_xblock_components) @@ -955,12 +955,12 @@ def test_published_not_live(self): Then the title in the Publish information box is "Published (not yet released)" """ unit = self.go_to_unit_page('Unreleased Section', 'Unreleased Subsection', 'Unreleased Unit') - self._verify_publish_title(unit, self.PUBLISHED_STATUS) + unit.verify_publish_title(self.PUBLISHED_STATUS) add_discussion(unit) - self._verify_publish_title(unit, self.DRAFT_STATUS) + unit.verify_publish_title(self.DRAFT_STATUS) unit.publish_action.click() unit.wait_for_ajax() - self._verify_publish_title(unit, self.PUBLISHED_STATUS) + unit.verify_publish_title(self.PUBLISHED_STATUS) def _view_published_version(self, unit): """ @@ -1007,15 +1007,6 @@ def _verify_release_date_info(self, unit, expected_title, expected_date): self.assertEqual(expected_title, unit.release_title) self.assertEqual(expected_date, unit.release_date) - def _verify_publish_title(self, unit, expected_title): - """ - Waits for the publish title to change to the expected value. - """ - def wait_for_title_change(): - return (unit.publish_title == expected_title, unit.publish_title) - - Promise(wait_for_title_change, "Publish title incorrect. Found '" + unit.publish_title + "'").fulfill() - def _verify_last_published_and_saved(self, unit, expected_published_prefix, expected_saved_prefix): """ Verifies that last published and last saved messages respectively contain the given strings. @@ -1144,6 +1135,9 @@ class MoveComponentTest(ContainerBase): """ Tests of moving an XBlock to another XBlock. """ + PUBLISHED_LIVE_STATUS = "Publishing Status\nPublished and Live" + DRAFT_STATUS = "Publishing Status\nDraft (Unpublished changes)" + def setUp(self, is_staff=True): super(MoveComponentTest, self).setUp(is_staff=is_staff) self.container = ContainerPage(self.browser, None) @@ -1181,26 +1175,36 @@ def populate_course_fixture(self, course_fixture): ) ) - def verify_move_opertions(self, unit_page, source_component, operation, component_display_names_after_operation): + def verify_move_opertions(self, unit_page, source_component, operation, component_display_names_after_operation, + should_verify_publish_title=True): """ Verify move operations. Arguments: unit_page (Object) Unit container page. - source_component (Object) source XBlock object to be moved. + source_component (Object) Source XBlock object to be moved. operation (str), `move` or `undo move` operation. - component_display_names_after_operation (dict) display names of components after operation in source/dest + component_display_names_after_operation (dict) Display names of components after operation in source/dest + should_verify_publish_title (Boolean) Should verify publish title ot not. Default is True. """ source_component.open_move_modal() self.move_modal_view.navigate_to_category(self.source_xblock_category, self.navigation_options) self.assertEqual(self.move_modal_view.is_move_button_enabled, True) + # Verify unit is in published state before move operation + if should_verify_publish_title: + self.container.verify_publish_title(self.PUBLISHED_LIVE_STATUS) + self.move_modal_view.click_move_button() self.container.verify_confirmation_message( self.message_move.format(display_name=self.source_component_display_name) ) self.assertEqual(len(unit_page.displayed_children), 1) + # Verify unit in draft state now + if should_verify_publish_title: + self.container.verify_publish_title(self.DRAFT_STATUS) + if operation == 'move': self.container.click_take_me_there_link() elif operation == 'undo_move': @@ -1333,7 +1337,8 @@ def test_content_experiment(self): unit_page=group_container_page, source_component=components[0], operation='undo_move', - component_display_names_after_operation=['HTML 311', 'HTML 312'] + component_display_names_after_operation=['HTML 311', 'HTML 312'], + should_verify_publish_title=False ) # Verify move operation for content experiment. @@ -1341,7 +1346,8 @@ def test_content_experiment(self): unit_page=group_container_page, source_component=components[0], operation='move', - component_display_names_after_operation=['HTML 21', 'HTML 22', 'HTML 311'] + component_display_names_after_operation=['HTML 21', 'HTML 22', 'HTML 311'], + should_verify_publish_title=False ) def test_a11y(self): From cb2d78bf34d94eb019cd0dd8ffc983558e945a4b Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Thu, 9 Mar 2017 15:38:26 +0500 Subject: [PATCH 12/12] Add logging on move xblock Add i18n to backend error messages in case they appear to end user Remove success banner when trying to publish or discard changes on container page --- cms/djangoapps/contentstore/views/item.py | 24 +++++--- .../contentstore/views/tests/test_item.py | 18 ++++++ .../js/views/pages/container_subviews.js | 13 ++-- .../test/acceptance/pages/studio/container.py | 10 +-- .../tests/studio/test_studio_container.py | 61 ++++++++++++++++++- 5 files changed, 109 insertions(+), 17 deletions(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 5e902c5898b3..d5e2f7febf6f 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -729,20 +729,20 @@ def _move_item(source_usage_key, target_parent_usage_key, user, target_index=Non 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( + 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.' + 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.' + 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.' + error = _('You can not move an item into it\'s child.') elif target_parent_type == 'split_test': - error = 'You can not move an item directly into content experiment.' + error = _('You can not move an item directly into content experiment.') elif source_index is None: - error = '{source_usage_key} not found in {parent_usage_key}.'.format( + error = _('{source_usage_key} not found in {parent_usage_key}.').format( source_usage_key=unicode(source_usage_key), parent_usage_key=unicode(source_parent.location) ) @@ -750,12 +750,12 @@ def _move_item(source_usage_key, target_parent_usage_key, user, target_index=Non try: target_index = int(target_index) if target_index is not None else None if len(target_parent.children) < target_index: - error = 'You can not move {source_usage_key} at an invalid index ({target_index}).'.format( + error = _('You can not move {source_usage_key} at an invalid index ({target_index}).').format( source_usage_key=unicode(source_usage_key), target_index=target_index ) except ValueError: - error = 'You must provide target_index ({target_index}) as an integer.'.format( + error = _('You must provide target_index ({target_index}) as an integer.').format( target_index=target_index ) if error: @@ -772,6 +772,14 @@ def _move_item(source_usage_key, target_parent_usage_key, user, target_index=Non target_parent.children.insert(insert_at, source_item.location) store.update_item(target_parent, user.id) + log.info( + 'MOVE: %s moved from %s to %s at %d index', + unicode(source_usage_key), + unicode(source_parent.location), + unicode(target_parent_usage_key), + insert_at + ) + context = { 'move_source_locator': unicode(source_usage_key), 'parent_locator': unicode(target_parent_usage_key), diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 28e2b5f11f25..3e28f0e20595 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -1126,6 +1126,24 @@ def test_no_move_source_locator(self): response = json.loads(response.content) self.assertEqual(response['error'], 'Patch request did not recognise any parameters to handle.') + @patch('contentstore.views.item.log') + def test_move_logging(self, mock_logger): + """ + Test logging when an item is successfully moved. + + Arguments: + mock_logger (object): A mock logger object. + """ + insert_at = 0 + self.assert_move_item(self.html_usage_key, self.vert2_usage_key, insert_at) + mock_logger.info.assert_called_with( + 'MOVE: %s moved from %s to %s at %d index', + unicode(self.html_usage_key), + unicode(self.vert_usage_key), + unicode(self.vert2_usage_key), + insert_at + ) + class TestDuplicateItemWithAsides(ItemTest, DuplicateHelper): """ diff --git a/cms/static/js/views/pages/container_subviews.js b/cms/static/js/views/pages/container_subviews.js index c3dabf3a754d..0a0e028b23ce 100644 --- a/cms/static/js/views/pages/container_subviews.js +++ b/cms/static/js/views/pages/container_subviews.js @@ -2,10 +2,11 @@ * Subviews (usually small side panels) for XBlockContainerPage. */ define(['jquery', 'underscore', 'gettext', 'js/views/baseview', 'common/js/components/utils/view_utils', - 'js/views/utils/xblock_utils'], - function($, _, gettext, BaseView, ViewUtils, XBlockViewUtils) { - var VisibilityState = XBlockViewUtils.VisibilityState, - disabledCss = 'is-disabled'; + 'js/views/utils/xblock_utils', 'js/views/utils/move_xblock_utils'], + function($, _, gettext, BaseView, ViewUtils, XBlockViewUtils, MoveXBlockUtils) { + 'use strict'; + + var disabledCss = 'is-disabled'; /** * A view that refreshes the view when certain values in the XBlockInfo have changed @@ -132,6 +133,8 @@ define(['jquery', 'underscore', 'gettext', 'js/views/baseview', 'common/js/compo return xblockInfo.save({publish: 'make_public'}, {patch: true}); }).always(function() { xblockInfo.set('publish', null); + // Hide any move notification if present. + MoveXBlockUtils.hideMovedNotification(); }).done(function() { xblockInfo.fetch(); }); @@ -151,6 +154,8 @@ define(['jquery', 'underscore', 'gettext', 'js/views/baseview', 'common/js/compo return xblockInfo.save({publish: 'discard_changes'}, {patch: true}); }).always(function() { xblockInfo.set('publish', null); + // Hide any move notification if present. + MoveXBlockUtils.hideMovedNotification(); }).done(function() { renderPage(); }); diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index 99d1e41fa892..092346967ccf 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -286,16 +286,18 @@ def edit(self): """ return _click_edit(self, '.edit-button', '.xblock-studio_view') - def verify_confirmation_message(self, message): + def verify_confirmation_message(self, message, verify_hidden=False): """ - Verify for confirmation message. + Verify for confirmation message is present or hidden. """ def _verify_message(): """ promise function to check confirmation message state """ text = self.q(css='#page-alert .alert.confirmation #alert-confirmation-title').text - return text and message in text[0] + return text and message not in text[0] if verify_hidden else text and message in text[0] - self.wait_for(_verify_message, description='confirmation message present') + self.wait_for(_verify_message, description='confirmation message {status}'.format( + status='hidden' if verify_hidden else 'present' + )) def click_undo_move_link(self): """ diff --git a/common/test/acceptance/tests/studio/test_studio_container.py b/common/test/acceptance/tests/studio/test_studio_container.py index ab3ae1e9283c..eb74e9854e3a 100644 --- a/common/test/acceptance/tests/studio/test_studio_container.py +++ b/common/test/acceptance/tests/studio/test_studio_container.py @@ -17,7 +17,7 @@ from common.test.acceptance.tests.helpers import create_user_partition_json import datetime -from bok_choy.promise import Promise, EmptyPromise +import ddt from base_studio_test import ContainerBase from xmodule.partitions.partitions import Group @@ -1131,6 +1131,7 @@ def test_common_problem_types_tab(self): @attr(shard=1) +@ddt.ddt class MoveComponentTest(ContainerBase): """ Tests of moving an XBlock to another XBlock. @@ -1220,6 +1221,30 @@ def verify_move_opertions(self, unit_page, source_component, operation, componen component_display_names_after_operation ) + def verify_state_change(self, unit_page, operation): + """ + Verify that after state change, confirmation message is hidden. + + Arguments: + unit_page (Object) Unit container page. + operation (String) Publish or discard changes operation. + """ + # Verify unit in draft state now + self.container.verify_publish_title(self.DRAFT_STATUS) + + # Now click publish/discard button + if operation == 'publish': + unit_page.publish_action.click() + else: + unit_page.discard_changes() + + # Now verify success message is hidden + self.container.verify_publish_title(self.PUBLISHED_LIVE_STATUS) + self.container.verify_confirmation_message( + message=self.message_move.format(display_name=self.source_component_display_name), + verify_hidden=True + ) + def test_move_component_successfully(self): """ Test if we can move a component successfully. @@ -1268,6 +1293,40 @@ def test_undo_move_component_successfully(self): component_display_names_after_operation=['HTML 11', 'HTML 12'] ) + @ddt.data('publish', 'discard') + def test_publish_discard_changes_afer_move(self, operation): + """ + Test if success banner is hidden when we discard changes or publish the unit after a move operation. + + Given I am a staff user + And I go to unit page in first section + And I open the move modal + And I navigate to unit in second section + And I see move button is enabled + When I click on the move button + Then I see move operation success message + And When I click on publish or discard changes button + Then I see move operation success message is hidden. + """ + unit_page = self.go_to_unit_page(unit_name='Test Unit 1') + components = unit_page.displayed_children + self.assertEqual(len(components), 2) + + components[0].open_move_modal() + self.move_modal_view.navigate_to_category(self.source_xblock_category, self.navigation_options) + self.assertEqual(self.move_modal_view.is_move_button_enabled, True) + + # Verify unit is in published state before move operation + self.container.verify_publish_title(self.PUBLISHED_LIVE_STATUS) + + self.move_modal_view.click_move_button() + self.container.verify_confirmation_message( + self.message_move.format(display_name=self.source_component_display_name) + ) + self.assertEqual(len(unit_page.displayed_children), 1) + + self.verify_state_change(unit_page, operation) + def test_content_experiment(self): """ Test if we can move a component of content experiment successfully.