diff --git a/cms/djangoapps/contentstore/features/component.py b/cms/djangoapps/contentstore/features/component.py index 38fb905bcf..ce470fe81e 100644 --- a/cms/djangoapps/contentstore/features/component.py +++ b/cms/djangoapps/contentstore/features/component.py @@ -48,7 +48,10 @@ def add_a_multi_step_component(step, is_advanced, category): def see_a_multi_step_component(step, category): # Wait for all components to finish rendering - selector = 'li.studio-xblock-wrapper div.xblock-student_view' + if category == 'HTML': + selector = 'li.studio-xblock-wrapper div.xblock-student_view' + else: + selector = 'li.studio-xblock-wrapper div.xblock-author_view' world.wait_for(lambda _: len(world.css_find(selector)) == len(step.hashes)) for idx, step_hash in enumerate(step.hashes): diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index ce87bcc39f..2a2aef5a74 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -16,6 +16,7 @@ from xmodule.x_module import PREVIEW_VIEWS, STUDENT_VIEW, AUTHOR_VIEW from xmodule.contentstore.django import contentstore from xmodule.error_module import ErrorDescriptor from xmodule.exceptions import NotFoundError, ProcessingError +from xmodule.studio_editable import has_author_view from xmodule.services import SettingsService from xmodule.modulestore.django import modulestore, ModuleI18nService from xmodule.mixin import wrap_with_license @@ -122,6 +123,9 @@ class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method """ if not StudioConfig.asides_enabled(block.scope_ids.block_type): return [] + + # TODO: aside_type != 'acid_aside' check should be removed once AcidBlock is only installed during tests + # (see https://openedx.atlassian.net/browse/TE-811) return [ aside_type for aside_type in super(PreviewModuleSystem, self).applicable_aside_types(block) @@ -140,10 +144,13 @@ class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method result.add_frag_resources(frag) for aside, aside_fn in aside_frag_fns: - aside_frag = self.wrap_aside(block, aside, view_name, aside_fn(block, context), context) - aside.save() - result.add_frag_resources(aside_frag) - frag.content = frag.content.replace(position_for_asides, position_for_asides + aside_frag.content) + aside_frag = aside_fn(block, context) + if aside_frag.content != u'': + aside_frag_wrapped = self.wrap_aside(block, aside, view_name, aside_frag, context) + aside.save() + result.add_frag_resources(aside_frag_wrapped) + replacement = position_for_asides + aside_frag_wrapped.content + frag.content = frag.content.replace(position_for_asides, replacement) result.add_content(frag.content) return result @@ -231,7 +238,7 @@ def _load_preview_module(request, descriptor): descriptor: An XModuleDescriptor """ student_data = KvsFieldData(SessionKeyValueStore(request)) - if _has_author_view(descriptor): + if has_author_view(descriptor): wrapper = partial(CmsFieldData, student_data=student_data) else: wrapper = partial(LmsFieldData, student_data=student_data) @@ -288,7 +295,7 @@ def get_preview_fragment(request, descriptor, context): """ module = _load_preview_module(request, descriptor) - preview_view = AUTHOR_VIEW if _has_author_view(module) else STUDENT_VIEW + preview_view = AUTHOR_VIEW if has_author_view(module) else STUDENT_VIEW try: fragment = module.render(preview_view, context) @@ -296,12 +303,3 @@ def get_preview_fragment(request, descriptor, context): log.warning("Unable to render %s for %r", preview_view, module, exc_info=True) fragment = Fragment(render_to_string('html_error.html', {'message': str(exc)})) return fragment - - -def _has_author_view(descriptor): - """ - Returns True if the xmodule linked to the descriptor supports "author_view". - - If False, "student_view" and LmsFieldData should be used. - """ - return getattr(descriptor, 'has_author_view', False) diff --git a/cms/lib/xblock/tagging/tagging.py b/cms/lib/xblock/tagging/tagging.py index 15ac9c36bd..ddc4327716 100644 --- a/cms/lib/xblock/tagging/tagging.py +++ b/cms/lib/xblock/tagging/tagging.py @@ -6,7 +6,7 @@ Structured Tagging based on XBlockAsides from xblock.core import XBlockAside, XBlock from xblock.fragment import Fragment from xblock.fields import Scope, Dict -from xmodule.x_module import STUDENT_VIEW +from xmodule.x_module import AUTHOR_VIEW from xmodule.capa_module import CapaModule from edxmako.shortcuts import render_to_string from django.conf import settings @@ -37,7 +37,7 @@ class StructuredTagsAside(XBlockAside): """ return settings.STATIC_URL + relative_url - @XBlockAside.aside_for(STUDENT_VIEW) + @XBlockAside.aside_for(AUTHOR_VIEW) def student_view_aside(self, block, context): # pylint: disable=unused-argument """ Display the tag selector with specific categories and allowed values, @@ -90,3 +90,12 @@ class StructuredTagsAside(XBlockAside): return Response("Invalid 'tag' parameter", status=400) return Response() + + def get_event_context(self, event_type, event): # pylint: disable=unused-argument + """ + This method return data that should be associated with the "check_problem" event + """ + if self.saved_tags and event_type == "problem_check": + return {'saved_tags': self.saved_tags} + else: + return None diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index d8b01d575b..00b9e77b21 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -620,7 +620,7 @@ class CapaMixin(CapaFields): event_info['hint_index'] = hint_index event_info['hint_len'] = len(demand_hints) event_info['hint_text'] = hint_text - self.runtime.track_function('edx.problem.hint.demandhint_displayed', event_info) + self.runtime.publish(self, 'edx.problem.hint.demandhint_displayed', event_info) # We report the index of this hint, the client works out what index to use to get the next hint return { @@ -1145,7 +1145,7 @@ class CapaMixin(CapaFields): # avoiding problems where an event_info is unmasked twice. event_unmasked = copy.deepcopy(event_info) self.unmask_event(event_unmasked) - self.runtime.track_function(title, event_unmasked) + self.runtime.publish(self, title, event_unmasked) def unmask_event(self, event_info): """ diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 0f33644ca1..75ced83ea1 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -49,6 +49,12 @@ class CapaModule(CapaMixin, XModule): """ super(CapaModule, self).__init__(*args, **kwargs) + def author_view(self, context): + """ + Renders the Studio preview view. + """ + return self.student_view(context) + def handle_ajax(self, dispatch, data): """ This is called by courseware.module_render, to handle an AJAX call. @@ -139,6 +145,7 @@ class CapaDescriptor(CapaFields, RawDescriptor): mako_template = "widgets/problem-edit.html" js = {'coffee': [resource_string(__name__, 'js/src/problem/edit.coffee')]} js_module_name = "MarkdownEditingDescriptor" + has_author_view = True css = { 'scss': [ resource_string(__name__, 'css/editor/edit.scss'), diff --git a/common/lib/xmodule/xmodule/studio_editable.py b/common/lib/xmodule/xmodule/studio_editable.py index d06c20c4fe..a54ffb3978 100644 --- a/common/lib/xmodule/xmodule/studio_editable.py +++ b/common/lib/xmodule/xmodule/studio_editable.py @@ -43,7 +43,7 @@ class StudioEditableBlock(object): """ Helper method for getting preview view name (student_view or author_view) for a given module. """ - return AUTHOR_VIEW if hasattr(block, AUTHOR_VIEW) else STUDENT_VIEW + return AUTHOR_VIEW if has_author_view(block) else STUDENT_VIEW StudioEditableModule = StudioEditableBlock @@ -58,3 +58,10 @@ class StudioEditableDescriptor(object): """ author_view = module_attr(AUTHOR_VIEW) has_author_view = True + + +def has_author_view(descriptor): + """ + Returns True if the xmodule linked to the descriptor supports "author_view". + """ + return getattr(descriptor, 'has_author_view', False) diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index ad130868f8..3fad0fdc9d 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -1318,13 +1318,15 @@ class CapaModuleTest(unittest.TestCase): # Re-mock the module_id to a fixed string, so we can check the logging module.location = Mock(module.location) module.location.to_deprecated_string.return_value = 'i4x://edX/capa_test/problem/meh' - module.get_problem_html() - module.get_demand_hint(0) - module.runtime.track_function.assert_called_with( - 'edx.problem.hint.demandhint_displayed', - {'hint_index': 0, 'module_id': u'i4x://edX/capa_test/problem/meh', - 'hint_text': 'Demand 1', 'hint_len': 2} - ) + + with patch.object(module.runtime, 'publish') as mock_track_function: + module.get_problem_html() + module.get_demand_hint(0) + mock_track_function.assert_called_with( + module, 'edx.problem.hint.demandhint_displayed', + {'hint_index': 0, 'module_id': u'i4x://edX/capa_test/problem/meh', + 'hint_text': 'Demand 1', 'hint_len': 2} + ) def test_input_state_consistency(self): module1 = CapaFactory.create() @@ -1638,11 +1640,11 @@ class CapaModuleTest(unittest.TestCase): unmasked names should appear in the track_function event_info. """ module = CapaFactory.create(xml=self.common_shuffle_xml) - with patch.object(module.runtime, 'track_function') as mock_track_function: + with patch.object(module.runtime, 'publish') as mock_track_function: get_request_dict = {CapaFactory.input_key(): 'choice_3'} # the correct choice module.check_problem(get_request_dict) - mock_call = mock_track_function.mock_calls[0] - event_info = mock_call[1][1] + mock_call = mock_track_function.mock_calls[1] + event_info = mock_call[1][2] self.assertEqual(event_info['answers'][CapaFactory.answer_key()], 'choice_3') # 'permutation' key added to record how problem was shown self.assertEquals(event_info['permutation'][CapaFactory.answer_key()], @@ -1706,11 +1708,11 @@ class CapaModuleTest(unittest.TestCase): """) module = CapaFactory.create(xml=xml) - with patch.object(module.runtime, 'track_function') as mock_track_function: + with patch.object(module.runtime, 'publish') as mock_track_function: get_request_dict = {CapaFactory.input_key(): 'choice_2'} # mask_X form when masking enabled module.check_problem(get_request_dict) - mock_call = mock_track_function.mock_calls[0] - event_info = mock_call[1][1] + mock_call = mock_track_function.mock_calls[1] + event_info = mock_call[1][2] self.assertEqual(event_info['answers'][CapaFactory.answer_key()], 'choice_2') # 'permutation' key added to record how problem was shown self.assertEquals(event_info['permutation'][CapaFactory.answer_key()], @@ -2585,13 +2587,13 @@ class TestProblemCheckTracking(unittest.TestCase): return CustomCapaFactory def get_event_for_answers(self, module, answer_input_dict): - with patch.object(module.runtime, 'track_function') as mock_track_function: + with patch.object(module.runtime, 'publish') as mock_track_function: module.check_problem(answer_input_dict) - self.assertGreaterEqual(len(mock_track_function.mock_calls), 1) + self.assertGreaterEqual(len(mock_track_function.mock_calls), 2) # There are potentially 2 track logs: answers and hint. [-1]=answers. mock_call = mock_track_function.mock_calls[-1] - event = mock_call[1][1] + event = mock_call[1][2] return event diff --git a/common/lib/xmodule/xmodule/tests/test_library_root.py b/common/lib/xmodule/xmodule/tests/test_library_root.py index 574bcf4152..40b7089e2b 100644 --- a/common/lib/xmodule/xmodule/tests/test_library_root.py +++ b/common/lib/xmodule/xmodule/tests/test_library_root.py @@ -18,6 +18,7 @@ dummy_render = lambda block, _: Fragment(block.data) # pylint: disable=invalid- 'xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.render', VanillaRuntime.render ) @patch('xmodule.html_module.HtmlDescriptor.author_view', dummy_render, create=True) +@patch('xmodule.html_module.HtmlDescriptor.has_author_view', True, create=True) @patch('xmodule.x_module.DescriptorSystem.applicable_aside_types', lambda self, block: []) class TestLibraryRoot(MixedSplitTestCase): """ diff --git a/common/test/acceptance/tests/studio/test_studio_library.py b/common/test/acceptance/tests/studio/test_studio_library.py index 8fc50262b2..8dd0394c14 100644 --- a/common/test/acceptance/tests/studio/test_studio_library.py +++ b/common/test/acceptance/tests/studio/test_studio_library.py @@ -121,7 +121,7 @@ class LibraryEditPageTest(StudioLibraryTest): # Check that the save worked: self.assertEqual(len(self.lib_page.xblocks), 1) problem_block = self.lib_page.xblocks[0] - self.assertIn("Laura Roslin", problem_block.student_content) + self.assertIn("Laura Roslin", problem_block.author_content) def test_no_discussion_button(self): """ diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index f7f5fbf442..34253fb657 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -554,7 +554,14 @@ def get_module_system_for_user(user, student_data, # TODO # pylint: disable=to if event_type == 'grade' and not is_masquerading_as_specific_student(user, course_id): handle_grade_event(block, event_type, event) else: - track_function(event_type, event) + aside_context = {} + for aside in block.runtime.get_asides(block): + if hasattr(aside, 'get_event_context'): + aside_event_info = aside.get_event_context(event_type, event) + if aside_event_info is not None: + aside_context[aside.scope_ids.block_type] = aside_event_info + with tracker.get_tracker().context('asides', {'asides': aside_context}): + track_function(event_type, event) def rebind_noauth_module_to_user(module, real_user): """ diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 8cb1b81b26..965d9193d4 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -23,7 +23,7 @@ from courseware.module_render import hash_resource from xblock.field_data import FieldData from xblock.runtime import Runtime from xblock.fields import ScopeIds -from xblock.core import XBlock +from xblock.core import XBlock, XBlockAside from xblock.fragment import Fragment from capa.tests.response_xml_factory import OptionResponseXMLFactory @@ -51,6 +51,7 @@ from xmodule.lti_module import LTIDescriptor from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.factories import ItemFactory, CourseFactory, ToyCourseFactory, check_mongo_calls +from xmodule.modulestore.tests.test_asides import AsideTestType from xmodule.x_module import XModuleDescriptor, XModule, STUDENT_VIEW, CombinedSystem from openedx.core.djangoapps.credit.models import CreditCourse @@ -1703,9 +1704,36 @@ class TestModuleTrackingContext(SharedModuleStoreTestCase): module_info = self.handle_callback_and_get_module_info(mock_tracker, problem_display_name) self.assertEquals(problem_display_name, module_info['display_name']) - def handle_callback_and_get_module_info(self, mock_tracker, problem_display_name=None): + @XBlockAside.register_temp_plugin(AsideTestType, 'test_aside') + @patch('xmodule.modulestore.mongo.base.CachingDescriptorSystem.applicable_aside_types', + lambda self, block: ['test_aside']) + @patch('lms.djangoapps.lms_xblock.runtime.LmsModuleSystem.applicable_aside_types', + lambda self, block: ['test_aside']) + def test_context_contains_aside_info(self, mock_tracker): """ - Creates a fake module, invokes the callback and extracts the 'module' + Check that related xblock asides populate information in the 'problem_check' event in case + the 'get_event_context' method is exist + """ + problem_display_name = u'Test Problem' + + def get_event_context(self, event_type, event): # pylint: disable=unused-argument + """ + This method return data that should be associated with the "check_problem" event + """ + return {'content': 'test1', 'data_field': 'test2'} + + AsideTestType.get_event_context = get_event_context + context_info = self.handle_callback_and_get_context_info(mock_tracker, problem_display_name) + self.assertIn('asides', context_info) + self.assertIn('test_aside', context_info['asides']) + self.assertIn('content', context_info['asides']['test_aside']) + self.assertEquals(context_info['asides']['test_aside']['content'], 'test1') + self.assertIn('data_field', context_info['asides']['test_aside']) + self.assertEquals(context_info['asides']['test_aside']['data_field'], 'test2') + + def handle_callback_and_get_context_info(self, mock_tracker, problem_display_name=None): + """ + Creates a fake module, invokes the callback and extracts the 'context' metadata from the emitted problem_check event. """ descriptor_kwargs = { @@ -1730,7 +1758,15 @@ class TestModuleTrackingContext(SharedModuleStoreTestCase): event = mock_call[1][0] self.assertEquals(event['event_type'], 'problem_check') - return event['context']['module'] + return event['context'] + + def handle_callback_and_get_module_info(self, mock_tracker, problem_display_name=None): + """ + Creates a fake module, invokes the callback and extracts the 'module' + metadata from the emitted problem_check event. + """ + event = self.handle_callback_and_get_context_info(mock_tracker, problem_display_name) + return event['module'] def test_missing_display_name(self, mock_tracker): actual_display_name = self.handle_callback_and_get_module_info(mock_tracker)['display_name'] diff --git a/lms/djangoapps/lms_xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py index a9d44e9cf9..cfc99c5b08 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -281,4 +281,10 @@ class LmsModuleSystem(ModuleSystem): # pylint: disable=abstract-method if block.scope_ids.block_type in config.disabled_blocks.split(): return [] - return super(LmsModuleSystem, self).applicable_aside_types() + # TODO: aside_type != 'acid_aside' check should be removed once AcidBlock is only installed during tests + # (see https://openedx.atlassian.net/browse/TE-811) + return [ + aside_type + for aside_type in super(LmsModuleSystem, self).applicable_aside_types(block) + if aside_type != 'acid_aside' + ]