diff --git a/AUTHORS b/AUTHORS index 8a215a3edc..60fdb8d4c4 100644 --- a/AUTHORS +++ b/AUTHORS @@ -146,3 +146,4 @@ Ben Weeks David Bodor Sébastien Hinderer Kristin Stephens +Ben Patterson diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 6f004b9657..c9d2bd1d8f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,10 +5,18 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +Blades: Fix displaying transcripts on touch devices. BLD-1033. + +Blades: Tolerance expressed in percentage now computes correctly. BLD-522. + +Studio: Support add, delete and duplicate on the container page. STUD-1490. + Studio: Add drag-and-drop support to the container page. STUD-1309. Common: Add extensible third-party auth module. +Blades: Added new error message that displays when HTML5 video is not supported altogether. Make sure spinner gets hidden when error message is shown. BLD-638. + LMS: Switch default instructor dashboard to the new (formerly "beta") instructor dashboard. Puts the old (now "legacy") dash behind a feature flag. LMS-1296 @@ -16,7 +24,8 @@ LMS: Switch default instructor dashboard to the new (formerly "beta") Blades: Handle situation if no response were sent from XQueue to LMS in Matlab problem after Run Code button press. BLD-994. -Blades: Set initial video quality to large instead of default to avoid automatic switch to HD when iframe resizes. BLD-981. +Blades: Set initial video quality to large instead of default to avoid automatic +switch to HD when iframe resizes. BLD-981. Blades: Add an upload button for authors to provide students with an option to download a handout associated with a video (of arbitrary file format). BLD-1000. diff --git a/cms/djangoapps/contentstore/features/common.py b/cms/djangoapps/contentstore/features/common.py index aabbe9928b..12924fba22 100644 --- a/cms/djangoapps/contentstore/features/common.py +++ b/cms/djangoapps/contentstore/features/common.py @@ -204,8 +204,9 @@ def add_subsection(name='Subsection One'): def set_date_and_time(date_css, desired_date, time_css, desired_time, key=None): set_element_value(date_css, desired_date, key) - set_element_value(time_css, desired_time, key) + world.wait_for_ajax_complete() + set_element_value(time_css, desired_time, key) world.wait_for_ajax_complete() diff --git a/cms/djangoapps/contentstore/features/course-export.feature b/cms/djangoapps/contentstore/features/course-export.feature index ff92c2e646..9106ffd4e9 100644 --- a/cms/djangoapps/contentstore/features/course-export.feature +++ b/cms/djangoapps/contentstore/features/course-export.feature @@ -10,10 +10,11 @@ Feature: Course export Then I get an error dialog And I can click to go to the unit with the error - Scenario: User is directed to problem with & in it when export fails - Given I am in Studio editing a new unit - When I add a "Blank Advanced Problem" "Advanced Problem" component - And I edit and enter an ampersand - And I export the course - Then I get an error dialog - And I can click to go to the unit with the error + # Disabling due to failure on master. 05/21/2014 TODO: fix + # Scenario: User is directed to problem with & in it when export fails + # Given I am in Studio editing a new unit + # When I add a "Blank Advanced Problem" "Advanced Problem" component + # And I edit and enter an ampersand + # And I export the course + # Then I get an error dialog + # And I can click to go to the unit with the error diff --git a/cms/djangoapps/contentstore/features/html-editor.feature b/cms/djangoapps/contentstore/features/html-editor.feature index df44d06420..67e39e1531 100644 --- a/cms/djangoapps/contentstore/features/html-editor.feature +++ b/cms/djangoapps/contentstore/features/html-editor.feature @@ -105,26 +105,28 @@ Feature: CMS.HTML Editor
  • zzzz
      """ - Scenario: Can switch from Visual Editor to Raw - Given I have created a Blank HTML Page - When I edit the component and select the Raw Editor - And I save the page - When I edit the page - And type "fancy html" into the Raw Editor - And I save the page - Then the page text contains: - """ - fancy html - """ +# Skipping in master due to brittleness JZ 05/22/2014 +# Scenario: Can switch from Visual Editor to Raw +# Given I have created a Blank HTML Page +# When I edit the component and select the Raw Editor +# And I save the page +# When I edit the page +# And type "fancy html" into the Raw Editor +# And I save the page +# Then the page text contains: +# """ +# fancy html +# """ - Scenario: Can switch from Raw Editor to Visual - Given I have created a raw HTML component - And I edit the component and select the Visual Editor - And I save the page - When I edit the page - And type "less fancy html" in the code editor and press OK - And I save the page - Then the page text contains: - """ - less fancy html - """ +# Skipping in master due to brittleness JZ 05/22/2014 +# Scenario: Can switch from Raw Editor to Visual +# Given I have created a raw HTML component +# And I edit the component and select the Visual Editor +# And I save the page +# When I edit the page +# And type "less fancy html" in the code editor and press OK +# And I save the page +# Then the page text contains: +# """ +# less fancy html +# """ diff --git a/cms/djangoapps/contentstore/features/subsection.feature b/cms/djangoapps/contentstore/features/subsection.feature index 9c4d4cecdb..77440190b3 100644 --- a/cms/djangoapps/contentstore/features/subsection.feature +++ b/cms/djangoapps/contentstore/features/subsection.feature @@ -38,14 +38,16 @@ Feature: CMS.Create Subsection Then I see the subsection release date is 12/25/2011 03:00 And I see the subsection due date is 01/02/2012 04:00 -# Disabling due to failure on master. JZ 05/14/2014 TODO: fix -# Scenario: Set release and due dates of subsection on enter -# Given I have opened a new subsection in Studio -# And I set the subsection release date on enter to 04/04/2014 03:00 -# And I set the subsection due date on enter to 04/04/2014 04:00 -# And I reload the page -# Then I see the subsection release date is 04/04/2014 03:00 -# And I see the subsection due date is 04/04/2014 04:00 + @skip_safari + Scenario: Set release and due dates of subsection on enter + Given I have opened a new subsection in Studio + And I set the subsection release date on enter to 04/04/2014 03:00 + And I set the subsection due date on enter to 04/04/2014 04:00 + Then I see the subsection release date is 04/04/2014 03:00 + And I see the subsection due date is 04/04/2014 04:00 + And I reload the page + Then I see the subsection release date is 04/04/2014 03:00 + And I see the subsection due date is 04/04/2014 04:00 Scenario: Delete a subsection Given I have opened a new course section in Studio @@ -56,16 +58,18 @@ Feature: CMS.Create Subsection And I confirm the prompt Then the subsection does not exist -# Disabling due to failure on master. JZ 05/14/2014 TODO: fix -# Scenario: Sync to Section -# Given I have opened a new course section in Studio -# And I click the Edit link for the release date -# And I set the section release date to 01/02/2103 -# And I have added a new subsection -# And I click on the subsection -# And I set the subsection release date to 01/20/2103 -# And I reload the page -# And I click the link to sync release date to section -# And I wait for "1" second -# And I reload the page -# Then I see the subsection release date is 01/02/2103 + @skip_safari + Scenario: Sync to Section + Given I have opened a new course section in Studio + And I click the Edit link for the release date + And I set the section release date to 01/02/2103 + And I have added a new subsection + And I click on the subsection + And I set the subsection release date to 06/20/2104 + Then I see the subsection release date is 06/20/2104 + And I reload the page + Then I see the subsection release date is 06/20/2104 + And I click the link to sync release date to section + And I wait for "1" second + And I reload the page + Then I see the subsection release date is 01/02/2103 diff --git a/cms/djangoapps/contentstore/features/subsection.py b/cms/djangoapps/contentstore/features/subsection.py index 49a7c88383..6f8489beb3 100644 --- a/cms/djangoapps/contentstore/features/subsection.py +++ b/cms/djangoapps/contentstore/features/subsection.py @@ -64,19 +64,17 @@ def set_subsection_release_date_on_enter(_step, datestring, timestring): # pyli @step('I set the subsection due date to ([0-9/-]+)( [0-9:]+)?') -def set_subsection_due_date(_step, datestring, timestring): +def set_subsection_due_date(_step, datestring, timestring, key=None): if not world.css_visible('input#due_date'): world.css_click('.due-date-input .set-date') - set_subsection_date('input#due_date', datestring, 'input#due_time', timestring) + assert world.css_visible('input#due_date') + set_subsection_date('input#due_date', datestring, 'input#due_time', timestring, key) @step('I set the subsection due date on enter to ([0-9/-]+)( [0-9:]+)?') def set_subsection_due_date_on_enter(_step, datestring, timestring): # pylint: disable-msg=invalid-name - if not world.css_visible('input#due_date'): - world.css_click('.due-date-input .set-date') - - set_subsection_date('input#due_date', datestring, 'input#due_time', timestring, 'ENTER') + set_subsection_due_date(_step, datestring, timestring, 'ENTER') @step('I mark it as Homework$') diff --git a/cms/djangoapps/contentstore/features/upload.feature b/cms/djangoapps/contentstore/features/upload.feature index 9c538bad1e..189ab287fe 100644 --- a/cms/djangoapps/contentstore/features/upload.feature +++ b/cms/djangoapps/contentstore/features/upload.feature @@ -6,9 +6,12 @@ Feature: CMS.Upload Files @skip_safari Scenario: Users can upload files Given I am at the files and upload page of a Studio course - When I upload the file "test" + When I upload the file "test" by clicking "Upload your first asset" Then I should see the file "test" was uploaded And The url for the file "test" is valid + When I upload the file "test2" + Then I should see the file "test2" was uploaded + And The url for the file "test2" is valid # Uploading isn't working on safari with sauce labs @skip_safari diff --git a/cms/djangoapps/contentstore/features/upload.py b/cms/djangoapps/contentstore/features/upload.py index 75572dac18..b905c935b5 100644 --- a/cms/djangoapps/contentstore/features/upload.py +++ b/cms/djangoapps/contentstore/features/upload.py @@ -25,8 +25,11 @@ def go_to_uploads(_step): @step(u'I upload the( test)? file "([^"]*)"$') -def upload_file(_step, is_test_file, file_name): - world.click_link('Upload New File') +def upload_file(_step, is_test_file, file_name, button_text=None): + if button_text: + world.click_link(button_text) + else: + world.click_link('Upload New File') if not is_test_file: _write_test_file(file_name, "test file") @@ -39,6 +42,11 @@ def upload_file(_step, is_test_file, file_name): world.css_click(close_css) +@step(u'I upload the file "([^"]*)" by clicking "([^"]*)"') +def upload_file_on_button_press(_step, file_name, button_text=None): + upload_file(_step, '', file_name, button_text) + + @step(u'I upload the files "([^"]*)"$') def upload_files(_step, files_string): # files_string should be comma separated with no spaces. diff --git a/cms/djangoapps/contentstore/views/checklist.py b/cms/djangoapps/contentstore/views/checklist.py index 54df237633..ba326c6e64 100644 --- a/cms/djangoapps/contentstore/views/checklist.py +++ b/cms/djangoapps/contentstore/views/checklist.py @@ -16,6 +16,8 @@ from contentstore.utils import get_modulestore, reverse_course_url from .access import has_course_access from xmodule.course_module import CourseDescriptor +from django.utils.translation import ugettext + __all__ = ['checklists_handler'] @@ -76,7 +78,7 @@ def checklists_handler(request, course_key_string, checklist_index=None): course_module.save() get_modulestore(course_module.location).update_item(course_module, request.user.id) expanded_checklist = expand_checklist_action_url(course_module, persisted_checklist) - return JsonResponse(expanded_checklist) + return JsonResponse(localize_checklist_text(expanded_checklist)) else: return HttpResponseBadRequest( ("Could not save checklist state because the checklist index " @@ -96,7 +98,7 @@ def expand_all_action_urls(course_module): """ expanded_checklists = [] for checklist in course_module.checklists: - expanded_checklists.append(expand_checklist_action_url(course_module, checklist)) + expanded_checklists.append(localize_checklist_text(expand_checklist_action_url(course_module, checklist))) return expanded_checklists @@ -121,3 +123,20 @@ def expand_checklist_action_url(course_module, checklist): item['action_url'] = reverse_course_url(urlconf_map[action_url], course_module.id) return expanded_checklist + +def localize_checklist_text(checklist): + """ + Localize texts for a given checklist and returns the modified version. + + The method does an in-place operation so the input checklist is modified directly. + """ + # Localize checklist name + checklist['short_description'] = ugettext(checklist['short_description']) + + # Localize checklist items + for item in checklist.get('items'): + item['short_description'] = ugettext(item['short_description']) + item['long_description'] = ugettext(item['long_description']) + item['action_text'] = ugettext(item['action_text']) if item['action_text'] != "" else u"" + + return checklist diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 508885f976..a2acd86d4f 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -157,70 +157,7 @@ def unit_handler(request, usage_key_string): except ItemNotFoundError: return HttpResponseBadRequest() - component_templates = defaultdict(list) - for category in COMPONENT_TYPES: - component_class = _load_mixed_class(category) - # add the default template - # TODO: Once mixins are defined per-application, rather than per-runtime, - # this should use a cms mixed-in class. (cpennington) - if hasattr(component_class, 'display_name'): - display_name = component_class.display_name.default or 'Blank' - else: - display_name = 'Blank' - component_templates[category].append(( - display_name, - category, - False, # No defaults have markdown (hardcoded current default) - None # no boilerplate for overrides - )) - # add boilerplates - if hasattr(component_class, 'templates'): - for template in component_class.templates(): - filter_templates = getattr(component_class, 'filter_templates', None) - if not filter_templates or filter_templates(template, course): - component_templates[category].append(( - template['metadata'].get('display_name'), - category, - template['metadata'].get('markdown') is not None, - template.get('template_id') - )) - - # Check if there are any advanced modules specified in the course policy. - # These modules should be specified as a list of strings, where the strings - # are the names of the modules in ADVANCED_COMPONENT_TYPES that should be - # enabled for the course. - course_advanced_keys = course.advanced_modules - - # Set component types according to course policy file - if isinstance(course_advanced_keys, list): - for category in course_advanced_keys: - if category in ADVANCED_COMPONENT_TYPES: - # Do I need to allow for boilerplates or just defaults on the - # class? i.e., can an advanced have more than one entry in the - # menu? one for default and others for prefilled boilerplates? - try: - component_class = _load_mixed_class(category) - - component_templates['advanced'].append( - ( - component_class.display_name.default or category, - category, - False, - None # don't override default data - ) - ) - except PluginMissingError: - # dhm: I got this once but it can happen any time the - # course author configures an advanced component which does - # not exist on the server. This code here merely - # prevents any authors from trying to instantiate the - # non-existent component type by not showing it in the menu - pass - else: - log.error( - "Improper format for course advanced keys! %s", - course_advanced_keys - ) + component_templates = _get_component_templates(course) xblocks = item.get_children() @@ -259,9 +196,9 @@ def unit_handler(request, usage_key_string): return render_to_response('unit.html', { 'context_course': course, 'unit': item, - 'unit_locator': usage_key, - 'xblocks': xblocks, - 'component_templates': component_templates, + 'unit_usage_key': usage_key, + 'child_usage_keys': [block.scope_ids.usage_id for block in xblocks], + 'component_templates': json.dumps(component_templates), 'draft_preview_link': preview_lms_link, 'published_preview_link': lms_link, 'subsection': containing_subsection, @@ -293,14 +230,14 @@ def container_handler(request, usage_key_string): json: not currently supported """ if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): + usage_key = UsageKey.from_string(usage_key_string) - if not has_course_access(request.user, usage_key.course_key): - raise PermissionDenied() try: - xblock = get_modulestore(usage_key).get_item(usage_key) + course, xblock, __ = _get_item_in_course(request, usage_key) except ItemNotFoundError: return HttpResponseBadRequest() + component_templates = _get_component_templates(course) ancestor_xblocks = [] parent = get_parent_xblock(xblock) while parent and parent.category != 'sequential': @@ -317,11 +254,106 @@ def container_handler(request, usage_key_string): 'xblock_locator': usage_key, 'unit': None if not ancestor_xblocks else ancestor_xblocks[0], 'ancestor_xblocks': ancestor_xblocks, + 'component_templates': json.dumps(component_templates), }) else: return HttpResponseBadRequest("Only supports html requests") +def _get_component_templates(course): + """ + Returns the applicable component templates that can be used by the specified course. + """ + def create_template_dict(name, cat, boilerplate_name=None, is_common=False): + """ + Creates a component template dict. + + Parameters + display_name: the user-visible name of the component + category: the type of component (problem, html, etc.) + boilerplate_name: name of boilerplate for filling in default values. May be None. + is_common: True if "common" problem, False if "advanced". May be None, as it is only used for problems. + + """ + return { + "display_name": name, + "category": cat, + "boilerplate_name": boilerplate_name, + "is_common": is_common + } + + component_templates = [] + # The component_templates array is in the order of "advanced" (if present), followed + # by the components in the order listed in COMPONENT_TYPES. + for category in COMPONENT_TYPES: + templates_for_category = [] + component_class = _load_mixed_class(category) + # add the default template + # TODO: Once mixins are defined per-application, rather than per-runtime, + # this should use a cms mixed-in class. (cpennington) + if hasattr(component_class, 'display_name'): + display_name = component_class.display_name.default or 'Blank' + else: + display_name = 'Blank' + templates_for_category.append(create_template_dict(display_name, category)) + + # add boilerplates + if hasattr(component_class, 'templates'): + for template in component_class.templates(): + filter_templates = getattr(component_class, 'filter_templates', None) + if not filter_templates or filter_templates(template, course): + templates_for_category.append( + create_template_dict( + template['metadata'].get('display_name'), + category, + template.get('template_id'), + template['metadata'].get('markdown') is not None + ) + ) + component_templates.append({"type": category, "templates": templates_for_category}) + + # Check if there are any advanced modules specified in the course policy. + # These modules should be specified as a list of strings, where the strings + # are the names of the modules in ADVANCED_COMPONENT_TYPES that should be + # enabled for the course. + course_advanced_keys = course.advanced_modules + advanced_component_templates = {"type": "advanced", "templates": []} + # Set component types according to course policy file + if isinstance(course_advanced_keys, list): + for category in course_advanced_keys: + if category in ADVANCED_COMPONENT_TYPES: + # boilerplates not supported for advanced components + try: + component_class = _load_mixed_class(category) + + advanced_component_templates['templates'].append( + create_template_dict( + component_class.display_name.default or category, + category + ) + ) + except PluginMissingError: + # dhm: I got this once but it can happen any time the + # course author configures an advanced component which does + # not exist on the server. This code here merely + # prevents any authors from trying to instantiate the + # non-existent component type by not showing it in the menu + log.warning( + "Advanced component %s does not exist. It will not be added to the Studio new component menu.", + category + ) + pass + else: + log.error( + "Improper format for course advanced keys! %s", + course_advanced_keys + ) + if len(advanced_component_templates['templates']) > 0: + component_templates.insert(0, advanced_component_templates) + + return component_templates + + @login_required def _get_item_in_course(request, usage_key): """ diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py index eb320a0e31..f2550adc4e 100644 --- a/cms/djangoapps/contentstore/views/helpers.py +++ b/cms/djangoapps/contentstore/views/helpers.py @@ -9,7 +9,9 @@ from contentstore.utils import reverse_course_url, reverse_usage_url __all__ = ['edge', 'event', 'landing'] EDITING_TEMPLATES = [ - "basic-modal", "modal-button", "edit-xblock-modal", "editor-mode-button", "upload-dialog", "image-modal" + "basic-modal", "modal-button", "edit-xblock-modal", "editor-mode-button", "upload-dialog", "image-modal", + "add-xblock-component", "add-xblock-component-button", "add-xblock-component-menu", + "add-xblock-component-menu-problem" ] # points to the temporary course landing page with log in and sign up @@ -37,11 +39,20 @@ def render_from_lms(template_name, dictionary, context=None, namespace='main'): return render_to_string(template_name, dictionary, context, namespace="lms." + namespace) -def _xmodule_recurse(item, action): - for child in item.get_children(): - _xmodule_recurse(child, action) +def _xmodule_recurse(item, action, ignore_exception=()): + """ + Recursively apply provided action on item and its children - action(item) + ignore_exception (Exception Object): A optional argument; when passed ignores the corresponding + exception raised during xmodule recursion, + """ + for child in item.get_children(): + _xmodule_recurse(child, action, ignore_exception) + + try: + return action(item) + except ignore_exception: + return def get_parent_xblock(xblock): @@ -58,40 +69,54 @@ def get_parent_xblock(xblock): return modulestore().get_item(parent_locations[0]) -def _xblock_has_studio_page(xblock): +def is_unit(xblock): + """ + Returns true if the specified xblock is a vertical that is treated as a unit. + A unit is a vertical that is a direct child of a sequential (aka a subsection). + """ + if xblock.category == 'vertical': + parent_xblock = get_parent_xblock(xblock) + parent_category = parent_xblock.category if parent_xblock else None + return parent_category == 'sequential' + return False + + +def xblock_has_own_studio_page(xblock): """ Returns true if the specified xblock has an associated Studio page. Most xblocks do not have their own page but are instead shown on the page of their parent. There are a few exceptions: 1. Courses - 2. Verticals + 2. Verticals that are either: + - themselves treated as units (in which case they are shown on a unit page) + - a direct child of a unit (in which case they are shown on a container page) 3. XBlocks with children, except for: - - subsections (aka sequential blocks) - - chapters + - sequentials (aka subsections) + - chapters (aka sections) """ category = xblock.category - if category in ('course', 'vertical'): + + if is_unit(xblock): return True + elif category == 'vertical': + parent_xblock = get_parent_xblock(xblock) + return is_unit(parent_xblock) if parent_xblock else False elif category in ('sequential', 'chapter'): return False - elif xblock.has_children: - return True - else: - return False + + # All other xblocks with children have their own page + return xblock.has_children def xblock_studio_url(xblock): """ Returns the Studio editing URL for the specified xblock. """ - if not _xblock_has_studio_page(xblock): + if not xblock_has_own_studio_page(xblock): return None category = xblock.category parent_xblock = get_parent_xblock(xblock) - if parent_xblock: - parent_category = parent_xblock.category - else: - parent_category = None + parent_category = parent_xblock.category if parent_xblock else None if category == 'course': return reverse_course_url('course_handler', xblock.location.course_key) elif category == 'vertical' and parent_category == 'sequential': diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index a8a36f6aec..6eba2559db 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -21,7 +21,7 @@ from xblock.fragment import Fragment import xmodule from xmodule.modulestore.django import modulestore -from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError +from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError, DuplicateItemError from xmodule.modulestore.inheritance import own_metadata from xmodule.video_module import manage_video_subtitles_save @@ -31,7 +31,7 @@ from util.string_utils import str_to_bool from ..utils import get_modulestore from .access import has_course_access -from .helpers import _xmodule_recurse +from .helpers import _xmodule_recurse, xblock_has_own_studio_page from contentstore.utils import compute_publish_state, PublishState from xmodule.modulestore.draft import DIRECT_ONLY_CATEGORIES from contentstore.views.preview import get_preview_fragment @@ -178,46 +178,56 @@ def xblock_view_handler(request, usage_key_string, view_name): if 'application/json' in accept_header: store = get_modulestore(usage_key) - component = store.get_item(usage_key) - is_read_only = _xblock_is_read_only(component) + xblock = store.get_item(usage_key) + is_read_only = _is_xblock_read_only(xblock) + container_views = ['container_preview', 'reorderable_container_child_preview'] + unit_views = ['student_view'] # wrap the generated fragment in the xmodule_editor div so that the javascript # can bind to it correctly - component.runtime.wrappers.append(partial(wrap_xblock, 'StudioRuntime', usage_id_serializer=unicode)) + xblock.runtime.wrappers.append(partial(wrap_xblock, 'StudioRuntime', usage_id_serializer=unicode)) if view_name == 'studio_view': try: - fragment = component.render('studio_view') + fragment = xblock.render('studio_view') # catch exceptions indiscriminately, since after this point they escape the # dungeon and surface as uneditable, unsaveable, and undeletable # component-goblins. except Exception as exc: # pylint: disable=w0703 - log.debug("unable to render studio_view for %r", component, exc_info=True) + log.debug("unable to render studio_view for %r", xblock, exc_info=True) fragment = Fragment(render_to_string('html_error.html', {'message': str(exc)})) # change not authored by requestor but by xblocks. - store.update_item(component, None) + store.update_item(xblock, None) - elif view_name == 'student_view' and component.has_children: + elif view_name == 'student_view' and xblock_has_own_studio_page(xblock): context = { 'runtime_type': 'studio', 'container_view': False, 'read_only': is_read_only, - 'root_xblock': component, + 'root_xblock': xblock, } # For non-leaf xblocks on the unit page, show the special rendering # which links to the new container page. html = render_to_string('container_xblock_component.html', { 'xblock_context': context, - 'xblock': component, + 'xblock': xblock, 'locator': usage_key, }) return JsonResponse({ 'html': html, 'resources': [], }) - elif view_name in ('student_view', 'container_preview'): - is_container_view = (view_name == 'container_preview') + elif view_name in (unit_views + container_views): + is_container_view = (view_name in container_views) + + # Determine the items to be shown as reorderable. Note that the view + # 'reorderable_container_child_preview' is only rendered for xblocks that + # are being shown in a reorderable container, so the xblock is automatically + # added to the list. + reorderable_items = set() + if view_name == 'reorderable_container_child_preview': + reorderable_items.add(xblock.location) # Only show the new style HTML for the container view, i.e. for non-verticals # Note: this special case logic can be removed once the unit page is replaced @@ -226,10 +236,11 @@ def xblock_view_handler(request, usage_key_string, view_name): 'runtime_type': 'studio', 'container_view': is_container_view, 'read_only': is_read_only, - 'root_xblock': component, + 'root_xblock': xblock if (view_name == 'container_preview') else None, + 'reorderable_items': reorderable_items } - fragment = get_preview_fragment(request, component, context) + fragment = get_preview_fragment(request, xblock, context) # For old-style pages (such as unit and static pages), wrap the preview with # the component div. Note that the container view recursively adds headers # into the preview fragment, so we don't want to add another header here. @@ -237,7 +248,7 @@ def xblock_view_handler(request, usage_key_string, view_name): fragment.content = render_to_string('component.html', { 'xblock_context': context, 'preview': fragment.content, - 'label': component.display_name or component.scope_ids.block_type, + 'label': xblock.display_name or xblock.scope_ids.block_type, }) else: raise Http404 @@ -255,7 +266,7 @@ def xblock_view_handler(request, usage_key_string, view_name): return HttpResponse(status=406) -def _xblock_is_read_only(xblock): +def _is_xblock_read_only(xblock): """ Returns true if the specified xblock is read-only, meaning that it cannot be edited. """ @@ -293,11 +304,19 @@ def _save_item(request, usage_key, data=None, children=None, metadata=None, null if publish: if publish == 'make_private': - _xmodule_recurse(existing_item, lambda i: modulestore().unpublish(i.location)) + _xmodule_recurse( + existing_item, + lambda i: modulestore().unpublish(i.location), + ignore_exception=ItemNotFoundError + ) elif publish == 'create_draft': # This recursively clones the existing item location to a draft location (the draft is # implicit, because modulestore is a Draft modulestore) - _xmodule_recurse(existing_item, lambda i: modulestore().convert_to_draft(i.location)) + _xmodule_recurse( + existing_item, + lambda i: modulestore().convert_to_draft(i.location), + ignore_exception=DuplicateItemError + ) if data: # TODO Allow any scope.content fields not just "data" (exactly like the get below this) @@ -393,7 +412,7 @@ def _create_item(request): metadata = {} data = None template_id = request.json.get('boilerplate') - if template_id is not None: + if template_id: clz = parent.runtime.load_block_type(category) if clz is not None: template = clz.get_template(template_id) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 7aa6846dea..87f1beb9a5 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -27,7 +27,7 @@ from util.sandboxing import can_execute_unsafe_code import static_replace from .session_kv_store import SessionKeyValueStore -from .helpers import render_from_lms +from .helpers import render_from_lms, xblock_has_own_studio_page from contentstore.views.access import get_user_role @@ -156,6 +156,13 @@ def _load_preview_module(request, descriptor): return descriptor +def _is_xblock_reorderable(xblock, context): + """ + Returns true if the specified xblock is in the set of reorderable xblocks. + """ + return xblock.location in context['reorderable_items'] + + # pylint: disable=unused-argument def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): """ @@ -163,15 +170,19 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): """ # Only add the Studio wrapper when on the container page. The unit page will remain as is for now. if context.get('container_view', None) and view == 'student_view': + root_xblock = context.get('root_xblock') + is_root = root_xblock and xblock.location == root_xblock.location + is_reorderable = _is_xblock_reorderable(xblock, context) template_context = { 'xblock_context': context, 'xblock': xblock, 'content': frag.content, + 'is_root': is_root, + 'is_reorderable': is_reorderable, } - if xblock.category == 'vertical': - template = 'studio_vertical_wrapper.html' - elif xblock.location != context.get('root_xblock').location and xblock.has_children: - template = 'container_xblock_component.html' + # For child xblocks with their own page, render a link to the page + if xblock_has_own_studio_page(xblock) and not is_root: + template = 'studio_container_wrapper.html' else: template = 'studio_xblock_wrapper.html' html = render_to_string(template, template_context) diff --git a/cms/djangoapps/contentstore/views/tests/test_container.py b/cms/djangoapps/contentstore/views/tests/test_container.py deleted file mode 100644 index c3914fabb7..0000000000 --- a/cms/djangoapps/contentstore/views/tests/test_container.py +++ /dev/null @@ -1,144 +0,0 @@ -""" -Unit tests for the container view. -""" - -import json - -from contentstore.tests.utils import CourseTestCase -from contentstore.utils import compute_publish_state, PublishState -from contentstore.views.helpers import xblock_studio_url -from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.factories import ItemFactory - - -class ContainerViewTestCase(CourseTestCase): - """ - Unit tests for the container view. - """ - - def setUp(self): - super(ContainerViewTestCase, self).setUp() - self.chapter = ItemFactory.create(parent_location=self.course.location, - category='chapter', display_name="Week 1") - self.sequential = ItemFactory.create(parent_location=self.chapter.location, - category='sequential', display_name="Lesson 1") - self.vertical = ItemFactory.create(parent_location=self.sequential.location, - category='vertical', display_name='Unit') - self.child_vertical = ItemFactory.create(parent_location=self.vertical.location, - category='vertical', display_name='Child Vertical') - self.video = ItemFactory.create(parent_location=self.child_vertical.location, - category="video", display_name="My Video") - - def test_container_html(self): - self._test_html_content( - self.child_vertical, - expected_location_in_section_tag=self.child_vertical.location, - expected_breadcrumbs=( - r'Unit\s*' - r'Child Vertical' - ).format(unit_location=(unicode(self.vertical.location).replace("+", "\\+"))) - ) - - def test_container_on_container_html(self): - """ - Create the scenario of an xblock with children (non-vertical) on the container page. - This should create a container page that is a child of another container page. - """ - published_xblock_with_child = ItemFactory.create( - parent_location=self.child_vertical.location, - category="wrapper", display_name="Wrapper" - ) - ItemFactory.create( - parent_location=published_xblock_with_child.location, - category="html", display_name="Child HTML" - ) - expected_breadcrumbs = ( - r'Unit\s*' - r'Child Vertical\s*' - r'Wrapper' - ).format( - unit_location=unicode(self.vertical.location).replace("+", "\\+"), - child_vertical_location=unicode(self.child_vertical.location).replace("+", "\\+"), - ) - self._test_html_content( - published_xblock_with_child, - expected_location_in_section_tag=published_xblock_with_child.location, - expected_breadcrumbs=expected_breadcrumbs - ) - - # Now make the unit and its children into a draft and validate the container again - modulestore('draft').convert_to_draft(self.vertical.location) - modulestore('draft').convert_to_draft(self.child_vertical.location) - draft_xblock_with_child = modulestore('draft').convert_to_draft(published_xblock_with_child.location) - self._test_html_content( - draft_xblock_with_child, - expected_location_in_section_tag=draft_xblock_with_child.location, - expected_breadcrumbs=expected_breadcrumbs - ) - - def _test_html_content(self, xblock, expected_location_in_section_tag, expected_breadcrumbs): - """ - Get the HTML for a container page and verify the section tag is correct - and the breadcrumbs trail is correct. - """ - publish_state = compute_publish_state(xblock) - url = xblock_studio_url(xblock) - resp = self.client.get_html(url) - self.assertEqual(resp.status_code, 200) - html = resp.content - expected_section_tag = \ - '