diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 00ab686c20..a8cc6d97ca 100644
--- a/CHANGELOG.rst
+++ b/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.
diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py
index ccf5626561..f41a5fe046 100644
--- a/cms/djangoapps/contentstore/tests/test_contentstore.py
+++ b/cms/djangoapps/contentstore/tests/test_contentstore.py
@@ -484,7 +484,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
"""
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')
@@ -492,7 +492,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
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 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
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)
diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py
index 39acda3d8a..6db416f04b 100644
--- a/cms/djangoapps/contentstore/tests/utils.py
+++ b/cms/djangoapps/contentstore/tests/utils.py
@@ -57,12 +57,6 @@ class AjaxEnabledTestClient(Client):
"""
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)
diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py
index 840a41d0ca..c1338d6230 100644
--- a/cms/djangoapps/contentstore/views/component.py
+++ b/cms/djangoapps/contentstore/views/component.py
@@ -6,7 +6,7 @@ from collections import defaultdict
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 xmodule.x_module import prefer_xmodules
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 @@ __all__ = ['OPEN_ENDED_COMPONENT_TYPES',
'ADVANCED_COMPONENT_POLICY_KEY',
'subsection_handler',
'unit_handler',
+ 'container_handler',
'component_handler'
]
@@ -65,7 +67,7 @@ ADVANCED_COMPONENT_CATEGORY = 'advanced'
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):
"""
diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py
index a10a489c9a..f351f481f5 100644
--- a/cms/djangoapps/contentstore/views/helpers.py
+++ b/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)
diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py
index 47b981b728..d9734773b0 100644
--- a/cms/djangoapps/contentstore/views/item.py
+++ b/cms/djangoapps/contentstore/views/item.py
@@ -12,7 +12,7 @@ from xmodule_modifiers import wrap_xblock
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
diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py
index e236d15981..8307c88d64 100644
--- a/cms/djangoapps/contentstore/views/preview.py
+++ b/cms/djangoapps/contentstore/views/preview.py
@@ -10,7 +10,7 @@ from django.http import Http404, HttpResponseBadRequest
from django.contrib.auth.decorators import login_required
from edxmako.shortcuts import render_to_string
-from xmodule_modifiers import replace_static_urls, wrap_xblock
+from xmodule_modifiers import replace_static_urls, wrap_xblock, wrap_fragment
from xmodule.error_module import ErrorDescriptor
from xmodule.exceptions import NotFoundError, ProcessingError
from xmodule.modulestore.django import modulestore, loc_mapper, ModuleI18nService
@@ -108,6 +108,17 @@ def _preview_module_system(request, descriptor):
course_id = course_location.course_id
else:
course_id = get_course_for_item(descriptor.location).location.course_id
+ display_name_only = (descriptor.category == 'static_tab')
+
+ wrappers = [
+ # This wrapper wraps the module in the template specified above
+ partial(wrap_xblock, 'PreviewRuntime', display_name_only=display_name_only),
+
+ # This wrapper replaces urls in the output that start with /static
+ # with the correct course-specific url for the static content
+ partial(replace_static_urls, None, course_id=course_id),
+ _studio_wrap_xblock,
+ ]
return PreviewModuleSystem(
static_url=settings.STATIC_URL,
@@ -125,14 +136,7 @@ def _preview_module_system(request, descriptor):
anonymous_student_id='student',
# Set up functions to modify the fragment produced by student_view
- wrappers=(
- # This wrapper wraps the module in the template specified above
- partial(wrap_xblock, 'PreviewRuntime', display_name_only=descriptor.category == 'static_tab'),
-
- # This wrapper replaces urls in the output that start with /static
- # with the correct course-specific url for the static content
- partial(replace_static_urls, None, course_id=course_id),
- ),
+ wrappers=wrappers,
error_descriptor_class=ErrorDescriptor,
# get_user_role accepts a location or a CourseLocator.
# If descriptor.location is a CourseLocator, course_id is unused.
@@ -159,14 +163,38 @@ def _load_preview_module(request, descriptor):
return descriptor
-def get_preview_fragment(request, descriptor):
+# pylint: disable=unused-argument
+def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False):
+ """
+ Wraps the results of rendering an XBlock view in a div which adds a header and Studio action buttons.
+ """
+ # 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':
+ locator = loc_mapper().translate_location(xblock.course_id, xblock.location)
+ template_context = {
+ 'xblock_context': context,
+ 'xblock': xblock,
+ 'locator': locator,
+ 'content': frag.content,
+ }
+ if xblock.category == 'vertical':
+ template = 'studio_vertical_wrapper.html'
+ else:
+ template = 'studio_xblock_wrapper.html'
+ html = render_to_string(template, template_context)
+ frag = wrap_fragment(frag, html)
+ return frag
+
+
+def get_preview_fragment(request, descriptor, context):
"""
Returns the HTML returned by the XModule's student_view,
specified by the descriptor and idx.
"""
module = _load_preview_module(request, descriptor)
+
try:
- fragment = module.render("student_view")
+ fragment = module.render("student_view", context)
except Exception as exc: # pylint: disable=W0703
log.warning("Unable to render student_view for %r", module, exc_info=True)
fragment = Fragment(render_to_string('html_error.html', {'message': str(exc)}))
diff --git a/cms/djangoapps/contentstore/views/tests/test_container.py b/cms/djangoapps/contentstore/views/tests/test_container.py
new file mode 100644
index 0000000000..b1a6faf96f
--- /dev/null
+++ b/cms/djangoapps/contentstore/views/tests/test_container.py
@@ -0,0 +1,33 @@
+"""
+Unit tests for the container view.
+"""
+
+from contentstore.tests.utils import CourseTestCase
+from contentstore.views.helpers import xblock_studio_url
+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):
+ url = xblock_studio_url(self.child_vertical)
+ resp = self.client.get_html(url)
+ self.assertEqual(resp.status_code, 200)
+ html = resp.content
+ self.assertIn('', html)
diff --git a/cms/djangoapps/contentstore/views/tests/test_helpers.py b/cms/djangoapps/contentstore/views/tests/test_helpers.py
new file mode 100644
index 0000000000..b0ca2f5529
--- /dev/null
+++ b/cms/djangoapps/contentstore/views/tests/test_helpers.py
@@ -0,0 +1,44 @@
+"""
+Unit tests for helpers.py.
+"""
+
+from contentstore.tests.utils import CourseTestCase
+from contentstore.views.helpers import xblock_studio_url
+from xmodule.modulestore.tests.factories import ItemFactory
+
+
+class HelpersTestCase(CourseTestCase):
+ """
+ Unit tests for helpers.py.
+ """
+ def test_xblock_studio_url(self):
+ # Verify course URL
+ self.assertEqual(xblock_studio_url(self.course),
+ u'/course/MITx.999.Robot_Super_Course/branch/published/block/Robot_Super_Course')
+
+ # Verify chapter URL
+ chapter = ItemFactory.create(parent_location=self.course.location, category='chapter',
+ display_name="Week 1")
+ self.assertIsNone(xblock_studio_url(chapter))
+
+ # Verify lesson URL
+ sequential = ItemFactory.create(parent_location=chapter.location, category='sequential',
+ display_name="Lesson 1")
+ self.assertIsNone(xblock_studio_url(sequential))
+
+ # Verify vertical URL
+ vertical = ItemFactory.create(parent_location=sequential.location, category='vertical',
+ display_name='Unit')
+ self.assertEqual(xblock_studio_url(vertical),
+ u'/unit/MITx.999.Robot_Super_Course/branch/published/block/Unit')
+
+ # Verify child vertical URL
+ child_vertical = ItemFactory.create(parent_location=vertical.location, category='vertical',
+ display_name='Child Vertical')
+ self.assertEqual(xblock_studio_url(child_vertical),
+ u'/container/MITx.999.Robot_Super_Course/branch/published/block/Child_Vertical')
+
+ # Verify video URL
+ video = ItemFactory.create(parent_location=child_vertical.location, category="video",
+ display_name="My Video")
+ self.assertIsNone(xblock_studio_url(video))
diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py
index 931a2be012..3aa3f15ad9 100644
--- a/cms/djangoapps/contentstore/views/tests/test_item.py
+++ b/cms/djangoapps/contentstore/views/tests/test_item.py
@@ -70,6 +70,28 @@ class ItemTest(CourseTestCase):
class GetItem(ItemTest):
"""Tests for '/xblock' GET url."""
+ def _create_vertical(self, parent_locator=None):
+ """
+ Creates a vertical, returning its locator.
+ """
+ resp = self.create_xblock(category='vertical', parent_locator=parent_locator)
+ self.assertEqual(resp.status_code, 200)
+ return self.response_locator(resp)
+
+ def _get_container_preview(self, locator):
+ """
+ Returns the HTML and resources required for the xblock at the specified locator
+ """
+ preview_url = '/xblock/{locator}/container_preview'.format(locator=locator)
+ resp = self.client.get(preview_url, HTTP_ACCEPT='application/json')
+ self.assertEqual(resp.status_code, 200)
+ resp_content = json.loads(resp.content)
+ html = resp_content['html']
+ self.assertTrue(html)
+ resources = resp_content['resources']
+ self.assertIsNotNone(resources)
+ return html, resources
+
def test_get_vertical(self):
# Add a vertical
resp = self.create_xblock(category='vertical')
@@ -80,6 +102,36 @@ class GetItem(ItemTest):
resp = self.client.get('/xblock/' + resp_content['locator'])
self.assertEqual(resp.status_code, 200)
+ def test_get_empty_container_fragment(self):
+ root_locator = self._create_vertical()
+ html, __ = self._get_container_preview(root_locator)
+
+ # Verify that the Studio wrapper is not added
+ self.assertNotIn('wrapper-xblock', html)
+
+ # Verify that the header and article tags are still added
+ self.assertIn('