Add new container page that can display nested xblocks

CHANGELOG.rst

@@ -5,12 +5,14 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+Studio: Add new container page that can display nested xblocks. STUD-1244.
+
 Blades: Allow multiple transcripts with video. BLD-642.
 
 CMS: Add feature to allow exporting a course to a git repository by
 specifying the giturl in the course settings.
 
-Studo: Fix import/export bug with conditional modules. STUD-149
+Studio: Fix import/export bug with conditional modules. STUD-149
 
 Blades: Persist student progress in video. BLD-385.
 

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -484,15 +484,15 @@ def test_module_preview_in_whitelist(self):
         """
         Tests the ajax callback to render an XModule
         """
-        resp = self._test_preview(Location('i4x', 'edX', 'toy', 'vertical', 'vertical_test', None))
+        resp = self._test_preview(Location('i4x', 'edX', 'toy', 'vertical', 'vertical_test', None), 'container_preview')
         # These are the data-ids of the xblocks contained in the vertical.
         # Ultimately, these must be converted to new locators.
         self.assertContains(resp, 'i4x://edX/toy/video/sample_video')
         self.assertContains(resp, 'i4x://edX/toy/video/separate_file_video')
         self.assertContains(resp, 'i4x://edX/toy/video/video_with_end_time')
         self.assertContains(resp, 'i4x://edX/toy/poll_question/T1_changemind_poll_foo_2')
 
-    def _test_preview(self, location):
+    def _test_preview(self, location, view_name):
         """ Preview test case. """
         direct_store = modulestore('direct')
         _, course_items = import_from_xml(direct_store, 'common/test/data/', ['toy'])
@@ -501,7 +501,7 @@ def _test_preview(self, location):
         locator = loc_mapper().translate_location(
             course_items[0].location.course_id, location, True, True
         )
-        resp = self.client.get_fragment(locator.url_reverse('xblock', 'student_view'))
+        resp = self.client.get_json(locator.url_reverse('xblock', view_name))
         self.assertEqual(resp.status_code, 200)
         # TODO: uncomment when preview no longer has locations being returned.
         # _test_no_locations(self, resp)

cms/djangoapps/contentstore/tests/utils.py

@@ -57,12 +57,6 @@ def get_json(self, path, data=None, follow=False, **extra):
         """
         return self.get(path, data or {}, follow, HTTP_ACCEPT="application/json", **extra)
 
-    def get_fragment(self, path, data=None, follow=False, **extra):
-        """
-        Convenience method for client.get which sets the accept type to application/x-fragment+json
-        """
-        return self.get(path, data or {}, follow, HTTP_ACCEPT="application/x-fragment+json", **extra)
-
 
 
 @override_settings(MODULESTORE=TEST_MODULESTORE)

cms/djangoapps/contentstore/views/component.py

@@ -6,7 +6,7 @@
 
 from django.http import HttpResponseBadRequest, Http404
 from django.contrib.auth.decorators import login_required
-from django.views.decorators.http import require_http_methods
+from django.views.decorators.http import require_GET
 from django.core.exceptions import PermissionDenied
 from django.conf import settings
 from xmodule.modulestore.exceptions import ItemNotFoundError
@@ -28,6 +28,7 @@
 from lms.lib.xblock.runtime import unquote_slashes
 
 from contentstore.utils import get_lms_link_for_item, compute_unit_state, UnitState
+from contentstore.views.helpers import get_parent_xblock
 
 from models.settings.course_grading import CourseGradingModel
 
@@ -37,6 +38,7 @@
            'ADVANCED_COMPONENT_POLICY_KEY',
            'subsection_handler',
            'unit_handler',
+           'container_handler',
            'component_handler'
            ]
 
@@ -65,7 +67,7 @@
 ADVANCED_COMPONENT_POLICY_KEY = 'advanced_modules'
 
 
-@require_http_methods(["GET"])
+@require_GET
 @login_required
 def subsection_handler(request, tag=None, package_id=None, branch=None, version_guid=None, block=None):
     """
@@ -89,17 +91,7 @@ def subsection_handler(request, tag=None, package_id=None, branch=None, version_
         if item.location.category != 'sequential':
             return HttpResponseBadRequest()
 
-        parent_locs = modulestore().get_parent_locations(old_location, None)
-
-        # we're for now assuming a single parent
-        if len(parent_locs) != 1:
-            logging.error(
-                'Multiple (or none) parents have been found for %s',
-                unicode(locator)
-            )
-
-        # this should blow up if we don't find any parents, which would be erroneous
-        parent = modulestore().get_item(parent_locs[0])
+        parent = get_parent_xblock(item)
 
         # remove all metadata from the generic dictionary that is presented in a
         # more normalized UI. We only want to display the XBlocks fields, not
@@ -154,7 +146,7 @@ def _load_mixed_class(category):
     return mixologist.mix(component_class)
 
 
-@require_http_methods(["GET"])
+@require_GET
 @login_required
 def unit_handler(request, tag=None, package_id=None, branch=None, version_guid=None, block=None):
     """
@@ -236,24 +228,19 @@ def unit_handler(request, tag=None, package_id=None, branch=None, version_guid=N
                 course_advanced_keys
             )
 
-        components = [
+        xblocks = item.get_children()
+        locators = [
             loc_mapper().translate_location(
-                course.location.course_id, component.location, False, True
+                course.location.course_id, xblock.location, False, True
             )
-            for component
-            in item.get_children()
+            for xblock in xblocks
         ]
 
         # TODO (cpennington): If we share units between courses,
         # this will need to change to check permissions correctly so as
         # to pick the correct parent subsection
-
-        containing_subsection_locs = modulestore().get_parent_locations(old_location, None)
-        containing_subsection = modulestore().get_item(containing_subsection_locs[0])
-        containing_section_locs = modulestore().get_parent_locations(
-            containing_subsection.location, None
-        )
-        containing_section = modulestore().get_item(containing_section_locs[0])
+        containing_subsection = get_parent_xblock(item)
+        containing_section = get_parent_xblock(containing_subsection)
 
         # cdodge hack. We're having trouble previewing drafts via jump_to redirect
         # so let's generate the link url here
@@ -285,7 +272,7 @@ def unit_handler(request, tag=None, package_id=None, branch=None, version_guid=N
             'context_course': course,
             'unit': item,
             'unit_locator': locator,
-            'components': components,
+            'locators': locators,
             'component_templates': component_templates,
             'draft_preview_link': preview_lms_link,
             'published_preview_link': lms_link,
@@ -306,6 +293,35 @@ def unit_handler(request, tag=None, package_id=None, branch=None, version_guid=N
         return HttpResponseBadRequest("Only supports html requests")
 
 
+# pylint: disable=unused-argument
+@require_GET
+@login_required
+def container_handler(request, tag=None, package_id=None, branch=None, version_guid=None, block=None):
+    """
+    The restful handler for container xblock requests.
+
+    GET
+        html: returns the HTML page for editing a container
+        json: not currently supported
+    """
+    if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'):
+        locator = BlockUsageLocator(package_id=package_id, branch=branch, version_guid=version_guid, block_id=block)
+        try:
+            old_location, course, xblock, __ = _get_item_in_course(request, locator)
+        except ItemNotFoundError:
+            return HttpResponseBadRequest()
+        parent_xblock = get_parent_xblock(xblock)
+
+        return render_to_response('container.html', {
+            'context_course': course,
+            'xblock': xblock,
+            'xblock_locator': locator,
+            'parent_xblock': parent_xblock,
+        })
+    else:
+        return HttpResponseBadRequest("Only supports html requests")
+
+
 @login_required
 def _get_item_in_course(request, locator):
     """

cms/djangoapps/contentstore/views/helpers.py

@@ -1,6 +1,9 @@
+import logging
+
 from django.http import HttpResponse
 from django.shortcuts import redirect
 from edxmako.shortcuts import render_to_string, render_to_response
+from xmodule.modulestore.django import loc_mapper, modulestore
 
 __all__ = ['edge', 'event', 'landing']
 
@@ -35,3 +38,64 @@ def _xmodule_recurse(item, action):
         _xmodule_recurse(child, action)
 
     action(item)
+
+
+def get_parent_xblock(xblock):
+    """
+    Returns the xblock that is the parent of the specified xblock, or None if it has no parent.
+    """
+    locator = xblock.location
+    parent_locations = modulestore().get_parent_locations(locator, None)
+
+    if len(parent_locations) == 0:
+        return None
+    elif len(parent_locations) > 1:
+        logging.error('Multiple parents have been found for %s', unicode(locator))
+    return modulestore().get_item(parent_locations[0])
+
+
+def _xblock_has_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
+      3. XBlocks with children, except for:
+          - subsections (aka sequential blocks)
+          - chapters
+    """
+    category = xblock.category
+    if category in ('course', 'vertical'):
+        return True
+    elif category in ('sequential', 'chapter'):
+        return False
+    elif xblock.has_children:
+        return True
+    else:
+        return False
+
+
+def xblock_studio_url(xblock, course=None):
+    """
+    Returns the Studio editing URL for the specified xblock.
+    """
+    if not _xblock_has_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
+    if category == 'course':
+        prefix = 'course'
+    elif category == 'vertical' and parent_category == 'sequential':
+        prefix = 'unit'     # only show the unit page for verticals directly beneath a subsection
+    else:
+        prefix = 'container'
+    course_id = None
+    if course:
+        course_id = course.location.course_id
+    locator = loc_mapper().translate_location(course_id, xblock.location)
+    return locator.url_reverse(prefix)

cms/djangoapps/contentstore/views/item.py

@@ -12,7 +12,7 @@
 
 from django.core.exceptions import PermissionDenied
 from django.contrib.auth.decorators import login_required
-from django.http import HttpResponseBadRequest, HttpResponse
+from django.http import HttpResponseBadRequest, HttpResponse, Http404
 from django.utils.translation import ugettext as _
 from django.views.decorators.http import require_http_methods
 
@@ -164,7 +164,6 @@ def xblock_handler(request, tag=None, package_id=None, branch=None, version_guid
             content_type="text/plain"
         )
 
-
 # pylint: disable=unused-argument
 @require_http_methods(("GET"))
 @login_required
@@ -185,7 +184,7 @@ def xblock_view_handler(request, package_id, view_name, tag=None, branch=None, v
 
     accept_header = request.META.get('HTTP_ACCEPT', 'application/json')
 
-    if 'application/x-fragment+json' in accept_header:
+    if 'application/json' in accept_header:
         store = get_modulestore(old_location)
         component = store.get_item(old_location)
 
@@ -204,17 +203,46 @@ def xblock_view_handler(request, package_id, view_name, tag=None, branch=None, v
                 fragment = Fragment(render_to_string('html_error.html', {'message': str(exc)}))
 
             store.save_xmodule(component)
-
-        elif view_name == 'student_view':
-            fragment = get_preview_fragment(request, component)
-            fragment.content = render_to_string('component.html', {
-                'preview': fragment.content,
-                'label': component.display_name or component.scope_ids.block_type,
-
-                # Native XBlocks are responsible for persisting their own data,
-                # so they are also responsible for providing save/cancel buttons.
-                'show_save_cancel': isinstance(component, xmodule.x_module.XModuleDescriptor),
+        elif view_name == 'student_view' and component.has_children:
+            # For non-leaf xblocks on the unit page, show the special rendering
+            # which links to the new container page.
+            course_location = loc_mapper().translate_locator_to_location(locator, True)
+            course = store.get_item(course_location)
+            html = render_to_string('unit_container_xblock_component.html', {
+                'course': course,
+                'xblock': component,
+                'locator': locator
+            })
+            return JsonResponse({
+                'html': html,
+                'resources': [],
             })
+        elif view_name in ('student_view', 'container_preview'):
+            is_container_view = (view_name == 'container_preview')
+
+            # 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
+            # with the new container view.
+            is_read_only_view = is_container_view
+            context = {
+                'container_view': is_container_view,
+                'read_only': is_read_only_view,
+                'root_xblock': component
+            }
+
+            fragment = get_preview_fragment(request, component, 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.
+            if not is_container_view:
+                fragment.content = render_to_string('component.html', {
+                    'preview': fragment.content,
+                    'label': component.display_name or component.scope_ids.block_type,
+
+                    # Native XBlocks are responsible for persisting their own data,
+                    # so they are also responsible for providing save/cancel buttons.
+                    'show_save_cancel': isinstance(component, xmodule.x_module.XModuleDescriptor),
+                })
         else:
             raise Http404