Merge pull request #12212 from CredoReference/assigned-tags-included-in-the-emitted-event

Connected aside information is included in the emitted event (on check)

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):

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)

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

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):
         """

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'),

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)

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):
             </problem>
         """)
         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
 

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):
     """

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):
         """

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):
         """

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']

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'
+        ]