From 3dad2be55d5e406a49c92ee2f42e8ad1ad0d8a4d Mon Sep 17 00:00:00 2001 From: Gavin Sidebottom Date: Mon, 5 Mar 2018 12:55:16 -0500 Subject: [PATCH] Refactored XBlockAside rendering and added support for student view --- .../contentstore/views/component.py | 24 ++-- cms/djangoapps/contentstore/views/item.py | 10 +- .../contentstore/views/tests/test_item.py | 45 +++++++- cms/static/js/views/modals/edit_xblock.js | 13 ++- cms/static/js/views/xblock_editor.js | 28 +++-- cms/static/sass/elements/_modal-window.scss | 3 +- cms/static/sass/elements/_xblocks.scss | 8 ++ common/static/common/js/xblock/core.js | 4 + lms/djangoapps/courseware/module_render.py | 26 ++++- .../courseware/student_field_overrides.py | 12 +- .../courseware/tests/test_module_render.py | 106 +++++++++++++++++- lms/djangoapps/lms_xblock/runtime.py | 17 ++- openedx/core/lib/tests/test_xblock_utils.py | 36 +++++- openedx/core/lib/xblock_utils/__init__.py | 35 +++++- 14 files changed, 328 insertions(+), 39 deletions(-) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index e9c036588f..5d80a1b8de 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -9,7 +9,6 @@ from django.http import Http404, HttpResponseBadRequest from django.utils.translation import ugettext as _ from django.views.decorators.http import require_GET from opaque_keys import InvalidKeyError -from opaque_keys.edx.asides import AsideUsageKeyV1, AsideUsageKeyV2 from opaque_keys.edx.keys import UsageKey from xblock.core import XBlock from xblock.django.request import django_to_webob_request, webob_to_django_response @@ -17,10 +16,11 @@ from xblock.exceptions import NoSuchHandlerError from xblock.plugin import PluginMissingError from xblock.runtime import Mixologist -from contentstore.utils import get_lms_link_for_item, get_xblock_aside_instance, reverse_course_url +from contentstore.utils import get_lms_link_for_item, reverse_course_url from contentstore.views.helpers import get_parent_xblock, is_unit, xblock_type_display_name from contentstore.views.item import StudioEditModuleRuntime, add_container_page_publishing_info, create_xblock_info from edxmako.shortcuts import render_to_response +from openedx.core.lib.xblock_utils import is_xblock_aside, get_aside_from_xblock from student.auth import has_course_author_access from xblock_django.api import authorable_xblocks, disabled_xblocks from xblock_django.models import XBlockStudioConfigurationFlag @@ -441,25 +441,25 @@ def component_handler(request, usage_key_string, handler, suffix=''): :class:`django.http.HttpResponse`: The response from the handler, converted to a django response """ - usage_key = UsageKey.from_string(usage_key_string) + # Let the module handle the AJAX req = django_to_webob_request(request) - asides = [] - try: - if isinstance(usage_key, (AsideUsageKeyV1, AsideUsageKeyV2)): + if is_xblock_aside(usage_key): + # Get the descriptor for the block being wrapped by the aside (not the aside itself) descriptor = modulestore().get_item(usage_key.usage_key) - aside_instance = get_xblock_aside_instance(usage_key) - asides = [aside_instance] if aside_instance else [] - resp = aside_instance.handle(handler, req, suffix) + handler_descriptor = get_aside_from_xblock(descriptor, usage_key.aside_type) + asides = [handler_descriptor] else: descriptor = modulestore().get_item(usage_key) - descriptor.xmodule_runtime = StudioEditModuleRuntime(request.user) - resp = descriptor.handle(handler, req, suffix) + handler_descriptor = descriptor + asides = [] + handler_descriptor.xmodule_runtime = StudioEditModuleRuntime(request.user) + resp = handler_descriptor.handle(handler, req, suffix) except NoSuchHandlerError: - log.info("XBlock %s attempted to access missing handler %r", descriptor, handler, exc_info=True) + log.info("XBlock %s attempted to access missing handler %r", handler_descriptor, handler, exc_info=True) raise Http404 # unintentional update to handle any side effects of handle call diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index e6b6fa5d50..4149624b5d 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -52,7 +52,7 @@ from models.settings.course_grading import CourseGradingModel from openedx.core.djangoapps.schedules.config import COURSE_UPDATE_WAFFLE_FLAG from openedx.core.djangoapps.waffle_utils import WaffleSwitch from openedx.core.lib.gating import api as gating_api -from openedx.core.lib.xblock_utils import request_token, wrap_xblock +from openedx.core.lib.xblock_utils import request_token, wrap_xblock, wrap_xblock_aside from static_replace import replace_static_urls from student.auth import has_studio_read_access, has_studio_write_access from util.date_utils import get_default_time_display @@ -326,6 +326,14 @@ def xblock_view_handler(request, usage_key_string, view_name): request_token=request_token(request), )) + xblock.runtime.wrappers_asides.append(partial( + wrap_xblock_aside, + 'StudioRuntime', + usage_id_serializer=unicode, + request_token=request_token(request), + extra_classes=['wrapper-comp-plugins'] + )) + if view_name in (STUDIO_VIEW, VISIBILITY_VIEW): if view_name == STUDIO_VIEW and xblock.xmodule_runtime is None: xblock.xmodule_runtime = StudioEditModuleRuntime(request.user) diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 5d571fe2dd..dc50c5959d 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -10,10 +10,12 @@ from django.test import TestCase from django.test.client import RequestFactory from mock import Mock, PropertyMock, patch from opaque_keys import InvalidKeyError +from opaque_keys.edx.asides import AsideUsageKeyV2 from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator from pyquery import PyQuery from pytz import UTC +from six import text_type from web_fragments.fragment import Fragment from webob import Response from xblock.core import XBlockAside @@ -2135,6 +2137,7 @@ class TestEditSplitModule(ItemTest): @ddt.ddt class TestComponentHandler(TestCase): + """Tests for component handler api""" shard = 1 def setUp(self): @@ -2151,9 +2154,10 @@ class TestComponentHandler(TestCase): # of the xBlock descriptor. self.descriptor = self.modulestore.return_value.get_item.return_value - self.usage_key_string = unicode( - BlockUsageLocator(CourseLocator('dummy_org', 'dummy_course', 'dummy_run'), 'dummy_category', 'dummy_name') + self.usage_key = BlockUsageLocator( + CourseLocator('dummy_org', 'dummy_course', 'dummy_run'), 'dummy_category', 'dummy_name' ) + self.usage_key_string = text_type(self.usage_key) self.user = UserFactory() @@ -2192,6 +2196,43 @@ class TestComponentHandler(TestCase): self.assertEquals(component_handler(self.request, self.usage_key_string, 'dummy_handler').status_code, status_code) + @ddt.data((True, True), (False, False),) + @ddt.unpack + def test_aside(self, is_xblock_aside, is_get_aside_called): + """ + test get_aside_from_xblock called + """ + def create_response(handler, request, suffix): # pylint: disable=unused-argument + """create dummy response""" + return Response(status_code=200) + + def get_usage_key(): + """return usage key""" + return ( + text_type(AsideUsageKeyV2(self.usage_key, "aside")) + if is_xblock_aside + else self.usage_key_string + ) + + self.descriptor.handle = create_response + + with patch( + 'contentstore.views.component.is_xblock_aside', + return_value=is_xblock_aside + ), patch( + 'contentstore.views.component.get_aside_from_xblock' + ) as mocked_get_aside_from_xblock, patch( + "contentstore.views.component.webob_to_django_response" + ) as mocked_webob_to_django_response: + component_handler( + self.request, + get_usage_key(), + 'dummy_handler' + ) + assert mocked_webob_to_django_response.called is True + + assert mocked_get_aside_from_xblock.called is is_get_aside_called + class TestComponentTemplates(CourseTestCase): """ diff --git a/cms/static/js/views/modals/edit_xblock.js b/cms/static/js/views/modals/edit_xblock.js index 8b928520f4..af3ead91e6 100644 --- a/cms/static/js/views/modals/edit_xblock.js +++ b/cms/static/js/views/modals/edit_xblock.js @@ -19,7 +19,7 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/modals/base_mod view: 'studio_view', viewSpecificClasses: 'modal-editor confirm', // Translators: "title" is the name of the current component being edited. - titleFormat: gettext('Editing: %(title)s'), + titleFormat: gettext('Editing: {title}'), addPrimaryActionButton: true }), @@ -87,6 +87,10 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/modals/base_mod this.$('.modal-window-title').text(title); if (editorView.getDataEditor() && editorView.getMetadataEditor()) { this.addDefaultModes(); + // If the plugins content element exists, add a button to reveal it. + if (this.$('.wrapper-comp-plugins').length > 0) { + this.addModeButton('plugins', gettext('Plugins')); + } this.selectMode(editorView.mode); } } @@ -125,7 +129,11 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/modals/base_mod displayName = gettext('Component'); } } - return interpolate(this.options.titleFormat, {title: displayName}, true); + return edx.StringUtils.interpolate( + this.options.titleFormat, { + title: displayName + } + ); }, addDefaultModes: function() { @@ -192,6 +200,7 @@ define(['jquery', 'underscore', 'backbone', 'gettext', 'js/views/modals/base_mod addModeButton: function(mode, displayName) { var buttonPanel = this.$('.editor-modes'); + // xss-lint: disable=javascript-jquery-append buttonPanel.append(this.editorModeButtonTemplate({ mode: mode, displayName: displayName diff --git a/cms/static/js/views/xblock_editor.js b/cms/static/js/views/xblock_editor.js index 50b451ef96..1eb6c25d24 100644 --- a/cms/static/js/views/xblock_editor.js +++ b/cms/static/js/views/xblock_editor.js @@ -2,9 +2,9 @@ * XBlockEditorView displays the authoring view of an xblock, and allows the user to switch between * the available modes. */ -define(['jquery', 'underscore', 'gettext', 'js/views/xblock', 'js/views/metadata', 'js/collections/metadata', +define(['jquery', 'underscore', 'gettext', 'js/views/baseview', 'js/views/xblock', 'js/views/metadata', 'js/collections/metadata', 'jquery.inputnumber'], - function($, _, gettext, XBlockView, MetadataView, MetadataCollection) { + function($, _, gettext, BaseView, XBlockView, MetadataView, MetadataCollection) { var XBlockEditorView = XBlockView.extend({ // takes XBlockInfo as a model @@ -24,6 +24,7 @@ define(['jquery', 'underscore', 'gettext', 'js/views/xblock', 'js/views/metadata initializeEditors: function() { var metadataEditor, + pluginEl, defaultMode = 'editor'; metadataEditor = this.createMetadataEditor(); this.metadataEditor = metadataEditor; @@ -35,6 +36,12 @@ define(['jquery', 'underscore', 'gettext', 'js/views/xblock', 'js/views/metadata } this.selectMode(defaultMode); } + pluginEl = this.$('.wrapper-comp-plugins'); + if (pluginEl.length > 0) { + this.pluginEditor = new BaseView({ + el: pluginEl + }); + } }, getDefaultModes: function() { @@ -87,6 +94,10 @@ define(['jquery', 'underscore', 'gettext', 'js/views/xblock', 'js/views/metadata return this.metadataEditor; }, + getPluginEditor: function() { + return this.pluginEditor; + }, + /** * Returns the updated field data for the xblock. Note that this works for all * XModules as well as for XBlocks that provide a 'collectFieldData' API. @@ -144,14 +155,17 @@ define(['jquery', 'underscore', 'gettext', 'js/views/xblock', 'js/views/metadata }, selectMode: function(mode) { - var showEditor = mode === 'editor', - dataEditor = this.getDataEditor(), - metadataEditor = this.getMetadataEditor(); + var dataEditor = this.getDataEditor(), + metadataEditor = this.getMetadataEditor(), + pluginEditor = this.getPluginEditor(); if (dataEditor) { - this.setEditorActivation(dataEditor, showEditor); + this.setEditorActivation(dataEditor, mode === 'editor'); } if (metadataEditor) { - this.setEditorActivation(metadataEditor.$el, !showEditor); + this.setEditorActivation(metadataEditor.$el, mode === 'settings'); + } + if (pluginEditor) { + this.setEditorActivation(pluginEditor.$el, mode === 'plugins'); } this.mode = mode; }, diff --git a/cms/static/sass/elements/_modal-window.scss b/cms/static/sass/elements/_modal-window.scss index 44a6f05679..6d28251d2c 100644 --- a/cms/static/sass/elements/_modal-window.scss +++ b/cms/static/sass/elements/_modal-window.scss @@ -276,7 +276,8 @@ margin-left: ($baseline/2); .editor-button, - .settings-button { + .settings-button, + .plugins-button { @extend %btn-secondary-gray; @extend %t-copy-sub1; diff --git a/cms/static/sass/elements/_xblocks.scss b/cms/static/sass/elements/_xblocks.scss index 2a3e01252a..832200618b 100644 --- a/cms/static/sass/elements/_xblocks.scss +++ b/cms/static/sass/elements/_xblocks.scss @@ -887,6 +887,14 @@ } } +.wrapper-comp-plugins { + display: none; + + &.is-active { + display: block; + } +} + // +Case - Special Xblock Type Overrides // ==================== diff --git a/common/static/common/js/xblock/core.js b/common/static/common/js/xblock/core.js index b7e9a70fcf..0f264df56d 100644 --- a/common/static/common/js/xblock/core.js +++ b/common/static/common/js/xblock/core.js @@ -11,6 +11,9 @@ } else { selector = '.' + blockClass; } + // After an element is initialized, a class is added to it. To avoid repeat initialization, no + // elements with that class should be selected. + selector += ':not(.xblock-initialized)'; return $(element).immediateDescendents(selector).map(function(idx, elem) { return initializer(elem, requestToken); }).toArray(); @@ -112,6 +115,7 @@ initializeAside: function(element) { var blockUsageId = $(element).data('block-id'); var blockElement = $(element).siblings('[data-usage-id="' + blockUsageId + '"]')[0]; + return constructBlock(element, [blockElement, initArgs(element)]); }, diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 072b0d9d6e..2295045dda 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -69,7 +69,9 @@ from openedx.core.lib.xblock_utils import ( replace_course_urls, replace_jump_to_id_urls, replace_static_urls, - wrap_xblock + wrap_xblock, + is_xblock_aside, + get_aside_from_xblock, ) from student.models import anonymous_id_for_user, user_by_anonymous_id from student.roles import CourseBetaTesterRole @@ -1116,7 +1118,18 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course set_custom_metrics_for_course_key(course_key) with modulestore().bulk_operations(course_key): - instance, tracking_context = get_module_by_usage_id(request, course_id, usage_id, course=course) + try: + usage_key = UsageKey.from_string(unquote_slashes(usage_id)) + except InvalidKeyError: + raise Http404 + if is_xblock_aside(usage_key): + # Get the usage key for the block being wrapped by the aside (not the aside itself) + block_usage_key = usage_key.usage_key + else: + block_usage_key = usage_key + instance, tracking_context = get_module_by_usage_id( + request, course_id, unicode(block_usage_key), course=course + ) # Name the transaction so that we can view XBlock handlers separately in # New Relic. The suffix is necessary for XModule handlers because the @@ -1129,7 +1142,14 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course req = django_to_webob_request(request) try: with tracker.get_tracker().context(tracking_context_name, tracking_context): - resp = instance.handle(handler, req, suffix) + if is_xblock_aside(usage_key): + # In this case, 'instance' is the XBlock being wrapped by the aside, so + # the actual aside instance needs to be retrieved in order to invoke its + # handler method. + handler_instance = get_aside_from_xblock(instance, usage_key.aside_type) + else: + handler_instance = instance + resp = handler_instance.handle(handler, req, suffix) if suffix == 'problem_check' \ and course \ and getattr(course, 'entrance_exam_enabled', False) \ diff --git a/lms/djangoapps/courseware/student_field_overrides.py b/lms/djangoapps/courseware/student_field_overrides.py index e00cf83107..b5545bc0c0 100644 --- a/lms/djangoapps/courseware/student_field_overrides.py +++ b/lms/djangoapps/courseware/student_field_overrides.py @@ -5,6 +5,7 @@ by the individual due dates feature. import json from courseware.models import StudentFieldOverride +from openedx.core.lib.xblock_utils import is_xblock_aside from .field_overrides import FieldOverrideProvider @@ -44,9 +45,18 @@ def _get_overrides_for_user(user, block): Gets all of the individual student overrides for given user and block. Returns a dictionary of field override values keyed by field name. """ + if ( + hasattr(block, "scope_ids") and + hasattr(block.scope_ids, "usage_id") and + is_xblock_aside(block.scope_ids.usage_id) + ): + location = block.scope_ids.usage_id.usage_key + else: + location = block.location + query = StudentFieldOverride.objects.filter( course_id=block.runtime.course_id, - location=block.location, + location=location, student_id=user.id, ) overrides = {} diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index dacfa40d91..59107ae45b 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -24,6 +24,7 @@ from edx_proctoring.tests.test_services import MockCreditService, MockGradesServ from freezegun import freeze_time from milestones.tests.utils import MilestonesTestCaseMixin from mock import MagicMock, Mock, patch +from opaque_keys.edx.asides import AsideUsageKeyV2 from opaque_keys.edx.keys import CourseKey, UsageKey from pyquery import PyQuery from six import text_type @@ -32,7 +33,12 @@ from xblock.completable import CompletableXBlockMixin from xblock.core import XBlock, XBlockAside from xblock.field_data import FieldData from xblock.fields import ScopeIds -from xblock.runtime import Runtime +from xblock.runtime import ( + DictKeyValueStore, + KvsFieldData, + Runtime +) +from xblock.test.tools import TestRuntime from capa.tests.response_xml_factory import OptionResponseXMLFactory from course_modes.models import CourseMode @@ -379,16 +385,42 @@ class ModuleRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase): @override_settings(FIELD_OVERRIDE_PROVIDERS=( 'lms.djangoapps.courseware.student_field_overrides.IndividualStudentOverrideProvider', )) - def test_rebind_different_users(self): + @patch('xmodule.modulestore.xml.ImportSystem.applicable_aside_types', lambda self, block: ['test_aside']) + @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.applicable_aside_types', + lambda self, block: ['test_aside']) + @XBlockAside.register_temp_plugin(AsideTestType, 'test_aside') + @ddt.data('regular', 'test_aside') + def test_rebind_different_users(self, block_category): """ This tests the rebinding a descriptor to a student does not result in overly nested _field_data. """ + def create_aside(item, block_type): + """ + Helper function to create aside + """ + key_store = DictKeyValueStore() + field_data = KvsFieldData(key_store) + runtime = TestRuntime(services={'field-data': field_data}) + + def_id = runtime.id_generator.create_definition(block_type) + usage_id = AsideUsageKeyV2(runtime.id_generator.create_usage(def_id), "aside") + aside = AsideTestType(scope_ids=ScopeIds('user', block_type, def_id, usage_id), runtime=runtime) + aside.content = '%s_new_value11' % block_type + aside.data_field = '%s_new_value12' % block_type + aside.has_score = False + + modulestore().update_item(item, self.mock_user.id, asides=[aside]) + return item + request = self.request_factory.get('') request.user = self.mock_user course = CourseFactory.create() - descriptor = ItemFactory(category='html', parent=course) + descriptor = ItemFactory(category="html", parent=course) + if block_category == 'test_aside': + descriptor = create_aside(descriptor, "test_aside") + field_data_cache = FieldDataCache( [course, descriptor], course.id, self.mock_user ) @@ -407,7 +439,7 @@ class ModuleRenderTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase): self.assertIsNot(descriptor._unwrapped_field_data, descriptor._field_data) # now bind this module to a few other students - for user in [UserFactory(), UserFactory(), UserFactory()]: + for user in [UserFactory(), UserFactory(), self.mock_user]: render.get_module_for_descriptor( user, request, @@ -688,6 +720,72 @@ class TestHandleXBlockCallback(SharedModuleStoreTestCase, LoginEnrollmentTestCas completion = BlockCompletion.objects.get(block_key=block.scope_ids.usage_id) self.assertEqual(completion.completion, 0.625) + @XBlock.register_temp_plugin(StubCompletableXBlock, identifier='comp') + @ddt.data((True, True), (False, False),) + @ddt.unpack + def test_aside(self, is_xblock_aside, is_get_aside_called): + """ + test get_aside_from_xblock called + """ + course = CourseFactory.create() + block = ItemFactory.create(category='comp', parent=course) + request = self.request_factory.post( + '/', + data=json.dumps({'completion': 0.625}), + content_type='application/json', + ) + request.user = self.mock_user + + def get_usage_key(): + """return usage key""" + return ( + quote_slashes(text_type(AsideUsageKeyV2(block.scope_ids.usage_id, "aside"))) + if is_xblock_aside + else text_type(block.scope_ids.usage_id) + ) + + with patch( + 'courseware.module_render.is_xblock_aside', + return_value=is_xblock_aside + ), patch( + 'courseware.module_render.get_aside_from_xblock' + ) as mocked_get_aside_from_xblock, patch( + "courseware.module_render.webob_to_django_response" + ) as mocked_webob_to_django_response: + render.handle_xblock_callback( + request, + unicode(course.id), + get_usage_key(), + 'complete', + '', + ) + assert mocked_webob_to_django_response.called is True + assert mocked_get_aside_from_xblock.called is is_get_aside_called + + def test_aside_invalid_usage_id(self): + """ + test aside work when invalid usage id + """ + course = CourseFactory.create() + request = self.request_factory.post( + '/', + data=json.dumps({'completion': 0.625}), + content_type='application/json', + ) + request.user = self.mock_user + + with patch( + 'courseware.module_render.is_xblock_aside', + return_value=True + ), self.assertRaises(Http404): + render.handle_xblock_callback( + request, + unicode(course.id), + "foo@bar", + 'complete', + '', + ) + @XBlock.register_temp_plugin(StubCompletableXBlock, identifier='comp') def test_progress_signal_ignored_for_completable_xblock(self): with completion_waffle.waffle().override(completion_waffle.ENABLE_COMPLETION_TRACKING, True): diff --git a/lms/djangoapps/lms_xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py index 67bc712cfb..6cbeabf5c5 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -12,7 +12,7 @@ from badges.utils import badges_enabled from lms.djangoapps.lms_xblock.models import XBlockAsidesConfig from openedx.core.djangoapps.user_api.course_tag import api as user_course_tag_api from openedx.core.lib.url_utils import quote_slashes -from openedx.core.lib.xblock_utils import xblock_local_resource_url +from openedx.core.lib.xblock_utils import xblock_local_resource_url, wrap_xblock_aside from xmodule.library_tools import LibraryToolsService from xmodule.modulestore.django import ModuleI18nService, modulestore from xmodule.partitions.partitions_service import PartitionService @@ -184,19 +184,28 @@ class LmsModuleSystem(ModuleSystem): # pylint: disable=abstract-method The default implementation creates a frag to wraps frag w/ a div identifying the xblock. If you have javascript, you'll need to override this impl """ + if not frag.content: + return frag + + runtime_class = 'LmsRuntime' extra_data = { 'block-id': quote_slashes(unicode(block.scope_ids.usage_id)), + 'course-id': quote_slashes(unicode(block.course_id)), 'url-selector': 'asideBaseUrl', - 'runtime-class': 'LmsRuntime', + 'runtime-class': runtime_class, } if self.request_token: extra_data['request-token'] = self.request_token - return self._wrap_ele( + return wrap_xblock_aside( + runtime_class, aside, view, frag, - extra_data, + context, + usage_id_serializer=unicode, + request_token=self.request_token, + extra_data=extra_data, ) def applicable_aside_types(self, block): diff --git a/openedx/core/lib/tests/test_xblock_utils.py b/openedx/core/lib/tests/test_xblock_utils.py index 837575c15e..b3b1a78c5f 100644 --- a/openedx/core/lib/tests/test_xblock_utils.py +++ b/openedx/core/lib/tests/test_xblock_utils.py @@ -7,11 +7,16 @@ import uuid import ddt from django.test.client import RequestFactory +from mock import patch from web_fragments.fragment import Fragment +from six import text_type +from opaque_keys.edx.asides import AsideUsageKeyV1, AsideUsageKeyV2 from openedx.core.lib.url_utils import quote_slashes from openedx.core.lib.xblock_builtin import get_css_dependencies, get_js_dependencies from openedx.core.lib.xblock_utils import ( + is_xblock_aside, + get_aside_from_xblock, replace_course_urls, replace_jump_to_id_urls, replace_static_urls, @@ -20,9 +25,11 @@ from openedx.core.lib.xblock_utils import ( wrap_fragment, wrap_xblock ) +from xblock.core import XBlockAside from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.modulestore.tests.test_asides import AsideTestType @ddt.ddt @@ -216,3 +223,30 @@ class TestXblockUtils(SharedModuleStoreTestCase): with self.settings(PIPELINE_ENABLED=pipeline_enabled, PIPELINE_JS=pipeline_js): js_dependencies = get_js_dependencies("js-group") self.assertEqual(js_dependencies, expected_js_dependencies) + + +class TestXBlockAside(SharedModuleStoreTestCase): + """Test the xblock aside function.""" + + @classmethod + def setUpClass(cls): + super(TestXBlockAside, cls).setUpClass() + cls.course = CourseFactory.create() + cls.block = ItemFactory.create(category='aside', parent=cls.course) + cls.aside_v2 = AsideUsageKeyV2(cls.block.scope_ids.usage_id, "aside") + cls.aside_v1 = AsideUsageKeyV1(cls.block.scope_ids.usage_id, "aside") + + def test_is_xblock_aside(self): + """test if xblock is aside""" + assert is_xblock_aside(self.aside_v2) is True + assert is_xblock_aside(self.aside_v1) is True + + def test_is_not_xblock_aside(self): + """test if xblock is not aside""" + assert is_xblock_aside(self.block.scope_ids.usage_id) is False + + @patch('xmodule.modulestore.xml.ImportSystem.applicable_aside_types', lambda self, block: ['test_aside']) + @XBlockAside.register_temp_plugin(AsideTestType, 'test_aside') + def test_get_aside(self): + """test get aside success""" + assert get_aside_from_xblock(self.block, text_type("test_aside")) is not None diff --git a/openedx/core/lib/xblock_utils/__init__.py b/openedx/core/lib/xblock_utils/__init__.py index 26698af512..cc25e1e0c3 100644 --- a/openedx/core/lib/xblock_utils/__init__.py +++ b/openedx/core/lib/xblock_utils/__init__.py @@ -24,6 +24,7 @@ from web_fragments.fragment import Fragment from xblock.core import XBlock from xblock.exceptions import InvalidScopeError from xblock.scorable import ScorableXBlockMixin +from opaque_keys.edx.asides import AsideUsageKeyV1, AsideUsageKeyV2 from xmodule.seq_module import SequenceModule from xmodule.util.xmodule_django import add_webpack_to_fragment @@ -164,7 +165,8 @@ def wrap_xblock_aside( context, # pylint: disable=unused-argument usage_id_serializer, request_token, # pylint: disable=redefined-outer-name - extra_data=None + extra_data=None, + extra_classes=None ): """ Wraps the results of rendering an XBlockAside view in a standard
with identifying @@ -180,6 +182,7 @@ def wrap_xblock_aside( :param request_token: An identifier that is unique per-request, so that only xblocks rendered as part of this request will have their javascript initialized. :param extra_data: A dictionary with extra data values to be set on the wrapper + :param extra_classes: A list with extra classes to be set on the wrapper element """ if extra_data is None: @@ -196,6 +199,8 @@ def wrap_xblock_aside( ), 'xblock_asides-v1' ] + if extra_classes: + css_classes.extend(extra_classes) if frag.js_init_fn: data['init'] = frag.js_init_fn @@ -513,3 +518,31 @@ def xblock_resource_pkg(block): return module_name return module_name.rsplit('.', 1)[0] + + +def is_xblock_aside(usage_key): + """ + Returns True if the given usage key is for an XBlock aside + + Args: + usage_key (opaque_keys.edx.keys.UsageKey): A usage key + + Returns: + bool: Whether or not the usage key is an aside key type + """ + return isinstance(usage_key, (AsideUsageKeyV1, AsideUsageKeyV2)) + + +def get_aside_from_xblock(xblock, aside_type): + """ + Gets an instance of an XBlock aside from the XBlock that it's decorating. This also + configures the aside instance with the runtime and fields of the given XBlock. + + Args: + xblock (xblock.core.XBlock): The XBlock that the desired aside is decorating + aside_type (str): The aside type + + Returns: + xblock.core.XBlockAside: Instance of an xblock aside + """ + return xblock.runtime.get_aside_of_type(xblock, aside_type)