Merge pull request #14568 from edx/ziafazal/YONK-523-b

Moved tagging aside xblock data save/update logic out of preview_hanlder

cms/djangoapps/contentstore/utils.py

@@ -438,6 +438,20 @@ def get_visibility_partition_info(xblock):
     }
 
 
+def get_xblock_aside_instance(usage_key):
+    """
+    Returns: aside instance of a aside xblock
+    :param usage_key: Usage key of aside xblock
+    """
+    try:
+        descriptor = modulestore().get_item(usage_key.usage_key)
+        for aside in descriptor.runtime.get_asides(descriptor):
+            if aside.scope_ids.block_type == usage_key.aside_type:
+                return aside
+    except ItemNotFoundError:
+        log.warning(u'Unable to load item %s', usage_key.usage_key)
+
+
 def is_self_paced(course):
     """
     Returns True if course is self-paced, False otherwise.

cms/djangoapps/contentstore/views/component.py

@@ -8,6 +8,7 @@ from django.views.decorators.http import require_GET
 from django.core.exceptions import PermissionDenied
 from django.conf import settings
 from opaque_keys import InvalidKeyError
+from opaque_keys.edx.asides import AsideUsageKeyV1, AsideUsageKeyV2
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from edxmako.shortcuts import render_to_response
 
@@ -19,7 +20,7 @@ from xblock.exceptions import NoSuchHandlerError
 from xblock.plugin import PluginMissingError
 from xblock.runtime import Mixologist
 
-from contentstore.utils import get_lms_link_for_item
+from contentstore.utils import get_lms_link_for_item, get_xblock_aside_instance
 from contentstore.views.helpers import get_parent_xblock, is_unit, xblock_type_display_name
 from contentstore.views.item import create_xblock_info, add_container_page_publishing_info, StudioEditModuleRuntime
 
@@ -445,21 +446,27 @@ def component_handler(request, usage_key_string, handler, suffix=''):
     """
 
     usage_key = UsageKey.from_string(usage_key_string)
-
-    descriptor = modulestore().get_item(usage_key)
-    descriptor.xmodule_runtime = StudioEditModuleRuntime(request.user)
     # Let the module handle the AJAX
     req = django_to_webob_request(request)
 
-    try:
-        resp = descriptor.handle(handler, req, suffix)
+    asides = []
 
+    try:
+        if isinstance(usage_key, (AsideUsageKeyV1, AsideUsageKeyV2)):
+            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)
+        else:
+            descriptor = modulestore().get_item(usage_key)
+            descriptor.xmodule_runtime = StudioEditModuleRuntime(request.user)
+            resp = descriptor.handle(handler, req, suffix)
     except NoSuchHandlerError:
         log.info("XBlock %s attempted to access missing handler %r", descriptor, handler, exc_info=True)
         raise Http404
 
     # unintentional update to handle any side effects of handle call
     # could potentially be updating actual course data or simply caching its values
-    modulestore().update_item(descriptor, request.user.id)
+    modulestore().update_item(descriptor, request.user.id, asides=asides)
 
     return webob_to_django_response(resp)

cms/djangoapps/contentstore/views/preview.py

@@ -21,7 +21,6 @@ from xmodule.services import SettingsService
 from xmodule.modulestore.django import modulestore, ModuleI18nService
 from xmodule.mixin import wrap_with_license
 from opaque_keys.edx.keys import UsageKey
-from opaque_keys.edx.asides import AsideUsageKeyV1, AsideUsageKeyV2
 from xmodule.x_module import ModuleSystem
 from xblock.runtime import KvsFieldData
 from xblock.django.request import webob_to_django_response, django_to_webob_request
@@ -57,17 +56,8 @@ def preview_handler(request, usage_key_string, handler, suffix=''):
     """
     usage_key = UsageKey.from_string(usage_key_string)
 
-    if isinstance(usage_key, (AsideUsageKeyV1, AsideUsageKeyV2)):
-        descriptor = modulestore().get_item(usage_key.usage_key)
-        for aside in descriptor.runtime.get_asides(descriptor):
-            if aside.scope_ids.block_type == usage_key.aside_type:
-                asides = [aside]
-                instance = aside
-                break
-    else:
-        descriptor = modulestore().get_item(usage_key)
-        instance = _load_preview_module(request, descriptor)
-        asides = []
+    descriptor = modulestore().get_item(usage_key)
+    instance = _load_preview_module(request, descriptor)
 
     # Let the module handle the AJAX
     req = django_to_webob_request(request)
@@ -91,7 +81,6 @@ def preview_handler(request, usage_key_string, handler, suffix=''):
         log.exception("error processing ajax call")
         raise
 
-    modulestore().update_item(descriptor, request.user.id, asides=asides)
     return webob_to_django_response(resp)
 
 

cms/djangoapps/contentstore/views/tests/test_preview.py

@@ -22,6 +22,7 @@ from xblock_config.models import StudioConfig
 from xmodule.modulestore.django import modulestore
 
 
+@ddt.ddt
 class GetPreviewHtmlTestCase(ModuleStoreTestCase):
     """
     Tests for get_preview_fragment.
@@ -137,6 +138,31 @@ class GetPreviewHtmlTestCase(ModuleStoreTestCase):
             response = client.post(url)
             self.assertEqual(response.status_code, 200)
 
+    @ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo)
+    def test_block_branch_not_changed_by_preview_handler(self, default_store):
+        """
+        Tests preview_handler should not update blocks being previewed
+        """
+        client = Client()
+        client.login(username=self.user.username, password=self.user_password)
+
+        with self.store.default_store(default_store):
+            course = CourseFactory.create()
+
+            block = ItemFactory.create(
+                parent_location=course.location,
+                category="problem"
+            )
+
+            url = reverse_usage_url(
+                'preview_handler',
+                block.location,
+                kwargs={'handler': 'xmodule_handler/problem_check'}
+            )
+            response = client.post(url)
+            self.assertEqual(response.status_code, 200)
+            self.assertFalse(modulestore().has_changes(modulestore().get_item(block.location)))
+
 
 @XBlock.needs("field-data")
 @XBlock.needs("i18n")

cms/lib/xblock/tagging/test.py

@@ -189,7 +189,7 @@ class StructuredTagsAsideTestCase(ModuleStoreTestCase):
         Checks that handler to save tags in StructuredTagsAside works properly
         """
         handler_url = reverse_usage_url(
-            'preview_handler',
+            'component_handler',
             unicode(aside_key_class(self.problem.location, self.aside_name)),
             kwargs={'handler': 'save_tags'}
         )

cms/static/js/xblock_asides/structured_tags.js

@@ -4,6 +4,8 @@
     function StructuredTagsView(runtime, element) {
         var $element = $(element);
         var saveTagsInProgress = false;
+        // we need studio runtime to get handler capable of saving xblock data
+        var studioRuntime = new window.StudioRuntime.v1();
 
         $($element).find('.save_tags').click(function(e) {
             var dataToPost = {};
@@ -23,7 +25,7 @@
 
                 $.ajax({
                     type: 'POST',
-                    url: runtime.handlerUrl(element, 'save_tags'),
+                    url: studioRuntime.handlerUrl(element, 'save_tags'),
                     data: JSON.stringify(dataToPost),
                     dataType: 'json',
                     contentType: 'application/json; charset=utf-8'