Merge branch 'master' into dvz/studio-editable-block-callback-refactor

cms/djangoapps/cms_user_tasks/signals.py

@@ -2,6 +2,7 @@
 Receivers of signals sent from django-user-tasks
 """
 import logging
+import re
 from urllib.parse import urljoin
 
 from django.dispatch import receiver
@@ -15,6 +16,7 @@
 from .tasks import send_task_complete_email
 
 LOGGER = logging.getLogger(__name__)
+LIBRARY_CONTENT_TASK_NAME_TEMPLATE = 'updating .*type@library_content.* from library'
 
 
 @receiver(user_task_stopped, dispatch_uid="cms_user_task_stopped")
@@ -33,6 +35,22 @@ def user_task_stopped_handler(sender, **kwargs):  # pylint: disable=unused-argum
     Returns:
         None
     """
+
+    def is_library_content_update(task_name: str) -> bool:
+        """
+        Decides whether to suppress an end-of-task email on the basis that the just-ended task was a library content
+        XBlock update operation, and that emails following such operations amount to spam
+        Arguments:
+            task_name: The name of the just-ended task. By convention, if this was a library content XBlock update
+            task, then the task name follows the pattern prescribed in LibrarySyncChildrenTask
+            (content_libraries under openedx) 'Updating {key} from library'. Moreover, the block type
+            in the task name is always of type 'library_content' for such operations
+        Returns:
+            True if the end-of-task email should be suppressed
+        """
+        p = re.compile(LIBRARY_CONTENT_TASK_NAME_TEMPLATE)
+        return p.match(task_name)
+
     def get_olx_validation_from_artifact():
         """
         Get olx validation error if available for current task.
@@ -47,9 +65,17 @@ def get_olx_validation_from_artifact():
             return olx_artifact.text
 
     status = kwargs['status']
+
     # Only send email when the entire task is complete, should only send when
     # a chain / chord / etc completes, not on sub-tasks.
     if status.parent is None:
+        task_name = status.name.lower()
+
+        # Also suppress emails on library content XBlock updates (too much like spam)
+        if is_library_content_update(task_name):
+            LOGGER.info(f"Suppressing end-of-task email on task {task_name}")
+            return
+
         # `name` and `status` are not unique, first is our best guess
         artifact = UserTaskArtifact.objects.filter(status=status, name="BASE_URL").first()
 
@@ -61,7 +87,6 @@ def get_olx_validation_from_artifact():
             )
 
         user_email = status.user.email
-        task_name = status.name.lower()
         olx_validation_text = get_olx_validation_from_artifact()
         task_args = [task_name, str(status.state_text), user_email, detail_url, olx_validation_text]
         try:

cms/djangoapps/cms_user_tasks/tests.py

@@ -208,6 +208,19 @@ def test_email_sent_with_site(self):
         self.assert_msg_subject(msg)
         self.assert_msg_body_fragments(msg, body_fragments)
 
+    def test_email_not_sent_with_libary_content_update(self):
+        """
+        Check the signal receiver and email sending.
+        """
+        UserTaskArtifact.objects.create(
+            status=self.status, name='BASE_URL', url='https://test.edx.org/'
+        )
+        end_of_task_status = self.status
+        end_of_task_status.name = "updating block-v1:course+type@library_content+block@uuid from library"
+        user_task_stopped.send(sender=UserTaskStatus, status=end_of_task_status)
+
+        self.assertEqual(len(mail.outbox), 0)
+
     def test_email_sent_with_olx_validations_with_config_enabled(self):
         """
         Tests that email is sent with olx validation errors.

cms/djangoapps/contentstore/asset_storage_handlers.py

@@ -126,28 +126,41 @@ def _get_asset_usage_path(course_key, assets):
             asset_key_string = str(asset_key)
             static_path = StaticContent.get_static_path_from_location(asset_key)
             is_video_block = getattr(block, 'category', '') == 'video'
-            if is_video_block:
-                handout = getattr(block, 'handout', '')
-                if handout and asset_key_string in handout:
-                    unit = block.get_parent()
-                    subsection = unit.get_parent()
-                    subsection_display_name = getattr(subsection, 'display_name', '')
-                    unit_display_name = getattr(unit, 'display_name', '')
-                    xblock_display_name = getattr(block, 'display_name', '')
-                    current_locations = usage_locations[asset_key_string]
-                    new_location = f'{subsection_display_name} - {unit_display_name} / {xblock_display_name}'
-                    usage_locations[asset_key_string] = [*current_locations, new_location]
-            else:
-                data = getattr(block, 'data', '')
-                if static_path in data or asset_key_string in data:
-                    unit = block.get_parent()
-                    subsection = unit.get_parent()
-                    subsection_display_name = getattr(subsection, 'display_name', '')
-                    unit_display_name = getattr(unit, 'display_name', '')
-                    xblock_display_name = getattr(block, 'display_name', '')
-                    current_locations = usage_locations[asset_key_string]
-                    new_location = f'{subsection_display_name} - {unit_display_name} / {xblock_display_name}'
-                    usage_locations[asset_key_string] = [*current_locations, new_location]
+            try:
+                if is_video_block:
+                    handout = getattr(block, 'handout', '')
+                    if handout and asset_key_string in handout:
+                        usage_dict = {'display_location': '', 'url': ''}
+                        xblock_display_name = getattr(block, 'display_name', '')
+                        xblock_location = str(block.location)
+                        unit = block.get_parent()
+                        unit_location = str(block.parent)
+                        unit_display_name = getattr(unit, 'display_name', '')
+                        subsection = unit.get_parent()
+                        subsection_display_name = getattr(subsection, 'display_name', '')
+                        current_locations = usage_locations[asset_key_string]
+                        usage_dict['display_location'] = (f'{subsection_display_name} - '
+                                                          f'{unit_display_name} / {xblock_display_name}')
+                        usage_dict['url'] = f'/container/{unit_location}#{xblock_location}'
+                        usage_locations[asset_key_string] = [*current_locations, usage_dict]
+                else:
+                    data = getattr(block, 'data', '')
+                    if static_path in data or asset_key_string in data:
+                        usage_dict = {'display_location': '', 'url': ''}
+                        xblock_display_name = getattr(block, 'display_name', '')
+                        xblock_location = str(block.location)
+                        unit = block.get_parent()
+                        unit_location = str(block.parent)
+                        unit_display_name = getattr(unit, 'display_name', '')
+                        subsection = unit.get_parent()
+                        subsection_display_name = getattr(subsection, 'display_name', '')
+                        current_locations = usage_locations[asset_key_string]
+                        usage_dict['display_location'] = (f'{subsection_display_name} - '
+                                                          f'{unit_display_name} / {xblock_display_name}')
+                        usage_dict['url'] = f'/container/{unit_location}#{xblock_location}'
+                        usage_locations[asset_key_string] = [*current_locations, usage_dict]
+            except AttributeError:
+                continue
     return usage_locations
 
 

cms/djangoapps/contentstore/helpers.py

@@ -302,6 +302,16 @@ def _import_xml_node_to_parent(
     # and VAL will thus make the transcript available.
 
     child_nodes = []
+
+    if issubclass(xblock_class, XmlMixin):
+        # Hack: XBlocks that use "XmlMixin" have their own XML parsing behavior, and in particular if they encounter
+        # an XML node that has no children and has only a "url_name" attribute, they'll try to load the XML data
+        # from an XML file in runtime.resources_fs. But that file doesn't exist here. So we set at least one
+        # additional attribute here to make sure that url_name is not the only attribute; otherwise in some cases,
+        # XmlMixin.parse_xml will try to load an XML file that doesn't exist, giving an error. The name and value
+        # of this attribute don't matter and should be ignored.
+        node.attrib["x-is-pointer-node"] = "no"
+
     if not xblock_class.has_children:
         # No children to worry about. The XML may contain child nodes, but they're not XBlocks.
         temp_xblock = xblock_class.parse_xml(node, runtime, keys, id_generator)
@@ -314,14 +324,6 @@ def _import_xml_node_to_parent(
         # serialization of a child block, in order. For blocks that don't support children, their XML content/nodes
         # could be anything (e.g. HTML, capa)
         node_without_children = etree.Element(node.tag, **node.attrib)
-        if issubclass(xblock_class, XmlMixin):
-            # Hack: XBlocks that use "XmlMixin" have their own XML parsing behavior, and in particular if they encounter
-            # an XML node that has no children and has only a "url_name" attribute, they'll try to load the XML data
-            # from an XML file in runtime.resources_fs. But that file doesn't exist here. So we set at least one
-            # additional attribute here to make sure that url_name is not the only attribute; otherwise in some cases,
-            # XmlMixin.parse_xml will try to load an XML file that doesn't exist, giving an error. The name and value
-            # of this attribute don't matter and should be ignored.
-            node_without_children.attrib["x-is-pointer-node"] = "no"
         temp_xblock = xblock_class.parse_xml(node_without_children, runtime, keys, id_generator)
         child_nodes = list(node)
     if xblock_class.has_children and temp_xblock.children:

cms/djangoapps/contentstore/rest_api/v1/serializers/videos.py

@@ -57,7 +57,7 @@ class VideoModelSerializer(serializers.Serializer):
         child=serializers.CharField()
     )
     usage_locations = serializers.ListField(
-        child=serializers.CharField()
+        child=serializers.DictField()
     )
 
 
@@ -89,6 +89,7 @@ class CourseVideosSerializer(serializers.Serializer):
     video_upload_max_file_size = serializers.CharField()
     video_image_settings = VideoImageSettingsSerializer(required=True, allow_null=False)
     is_video_transcript_enabled = serializers.BooleanField()
+    is_ai_translations_enabled = serializers.BooleanField()
     active_transcript_preferences = VideoActiveTranscriptPreferencesSerializer(required=False, allow_null=True)
     transcript_credentials = serializers.DictField(
         child=serializers.BooleanField()
@@ -110,7 +111,7 @@ class CourseVideosSerializer(serializers.Serializer):
 class VideoUsageSerializer(serializers.Serializer):
     """Serializer for video usage"""
     usage_locations = serializers.ListField(
-        child=serializers.CharField()
+        child=serializers.DictField()
     )
 
 

cms/djangoapps/contentstore/rest_api/v1/views/tests/test_videos.py

@@ -56,6 +56,7 @@ def test_course_videos_response(self):
                 "supported_file_formats": settings.VIDEO_IMAGE_SUPPORTED_FILE_FORMATS
             },
             "is_video_transcript_enabled": False,
+            "is_ai_translations_enabled": False,
             "active_transcript_preferences": None,
             "transcript_credentials": None,
             "transcript_available_languages": get_all_transcript_languages(),
@@ -126,3 +127,10 @@ def test_VideoTranscriptEnabledFlag_enabled(self):
             )
             self.assertIn("transcript_credentials_handler_url", transcript_settings)
             self.assertEqual(expected_credentials_handler, transcript_settings["transcript_credentials_handler_url"])
+        with patch(
+            'openedx.core.djangoapps.video_config.toggles.XPERT_TRANSLATIONS_UI.is_enabled'
+        ) as xpertTranslationfeature:
+            xpertTranslationfeature.return_value = True
+            response = self.client.get(self.url)
+            self.assertIn("is_ai_translations_enabled", response.data)
+            self.assertTrue(response.data["is_ai_translations_enabled"])

cms/djangoapps/contentstore/tasks.py

@@ -715,6 +715,7 @@ def read_chunk():
                 from .views.entrance_exam import add_entrance_exam_milestone
                 add_entrance_exam_milestone(course.id, entrance_exam_chapter)
                 LOGGER.info(f'Course import {course.id}: Entrance exam imported')
+    if is_course:
         sync_discussion_settings(courselike_key, user)
 
 

cms/djangoapps/contentstore/tests/test_libraries.py

@@ -440,10 +440,6 @@ def test_sync_if_source_library_changed(self):
         html_block_2 = modulestore().get_item(lc_block.children[0])
         self.assertEqual(html_block_2.data, data2)
 
-    @patch(
-        'openedx.core.djangoapps.content_libraries.tasks.SearchEngine.get_search_engine',
-        Mock(return_value=None, autospec=True),
-    )
     def test_sync_if_capa_type_changed(self):
         """ Tests that children are automatically refreshed if capa type field changes """
         name1, name2 = "Option Problem", "Multiple Choice Problem"

cms/djangoapps/contentstore/utils.py

@@ -74,6 +74,7 @@
     use_new_video_uploads_page,
     use_new_custom_pages,
     use_tagging_taxonomy_list_page,
+    # use_xpert_translations_component,
 )
 from cms.djangoapps.models.settings.course_grading import CourseGradingModel
 from xmodule.library_tools import LibraryToolsService
@@ -1537,6 +1538,7 @@ def get_course_videos_context(course_block, pagination_conf, course_key=None):
         get_transcript_preferences,
     )
     from openedx.core.djangoapps.video_config.models import VideoTranscriptEnabledFlag
+    from openedx.core.djangoapps.video_config.toggles import use_xpert_translations_component
     from xmodule.video_block.transcripts_utils import Transcript  # lint-amnesty, pylint: disable=wrong-import-order
 
     from .video_storage_handlers import (
@@ -1561,6 +1563,7 @@ def get_course_videos_context(course_block, pagination_conf, course_key=None):
             course = modulestore().get_course(course_key)
 
     is_video_transcript_enabled = VideoTranscriptEnabledFlag.feature_enabled(course.id)
+    is_ai_translations_enabled = use_xpert_translations_component(course.id)
     previous_uploads, pagination_context = _get_index_videos(course, pagination_conf)
     course_video_context = {
         'context_course': course,
@@ -1581,6 +1584,7 @@ def get_course_videos_context(course_block, pagination_conf, course_key=None):
             'supported_file_formats': settings.VIDEO_IMAGE_SUPPORTED_FILE_FORMATS
         },
         'is_video_transcript_enabled': is_video_transcript_enabled,
+        'is_ai_translations_enabled': is_ai_translations_enabled,
         'active_transcript_preferences': None,
         'transcript_credentials': None,
         'transcript_available_languages': get_all_transcript_languages(),

cms/djangoapps/contentstore/video_storage_handlers.py

@@ -234,15 +234,26 @@ def get_video_usage_path(course_key, edx_video_id):
             'category': 'video'
         },
     )
+
     for video in videos:
         video_id = getattr(video, 'edx_video_id', '')
-        if video_id == edx_video_id:
-            unit = video.get_parent()
-            subsection = unit.get_parent()
-            subsection_display_name = getattr(subsection, 'display_name', '')
-            unit_display_name = getattr(unit, 'display_name', '')
-            xblock_display_name = getattr(video, 'display_name', '')
-            usage_locations.append(f'{subsection_display_name} - {unit_display_name} / {xblock_display_name}')
+        try:
+            if video_id == edx_video_id:
+                usage_dict = {'display_location': '', 'url': ''}
+                video_location = str(video.location)
+                xblock_display_name = getattr(video, 'display_name', '')
+                unit = video.get_parent()
+                unit_location = str(video.parent)
+                unit_display_name = getattr(unit, 'display_name', '')
+                subsection = unit.get_parent()
+                subsection_display_name = getattr(subsection, 'display_name', '')
+                usage_dict['display_location'] = (f'{subsection_display_name} - '
+                                                  f'{unit_display_name} / {xblock_display_name}')
+                usage_dict['url'] = f'/container/{unit_location}#{video_location}'
+                usage_locations.append(usage_dict)
+        except AttributeError:
+            continue
+
     return {'usage_locations': usage_locations}
 
 
@@ -666,12 +677,12 @@ def videos_index_html(course, pagination_conf=None):
     """
     Returns an HTML page to display previous video uploads and allow new ones
     """
+    if use_new_video_uploads_page(course.id):
+        return redirect(get_video_uploads_url(course.id))
     context = get_course_videos_context(
         course,
         pagination_conf,
     )
-    if use_new_video_uploads_page(course.id):
-        return redirect(get_video_uploads_url(course.id))
     return render_to_response('videos_index.html', context)
 
 

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

@@ -3,6 +3,7 @@
 allow users to paste XBlocks that were copied using the staged_content/clipboard
 APIs.
 """
+import ddt
 from opaque_keys.edx.keys import UsageKey
 from rest_framework.test import APIClient
 from xmodule.modulestore.django import contentstore
@@ -13,6 +14,7 @@
 XBLOCK_ENDPOINT = "/xblock/"
 
 
+@ddt.ddt
 class ClipboardPasteTestCase(ModuleStoreTestCase):
     """
     Test Clipboard Paste functionality
@@ -99,6 +101,42 @@ def test_copy_and_paste_unit(self):
         # The new block should store a reference to where it was copied from
         assert dest_unit.copied_from_block == str(unit_key)
 
+    @ddt.data(
+        # A problem with absolutely no fields set. A previous version of copy-paste had an error when pasting this.
+        {"category": "problem", "display_name": None, "data": ""},
+        {"category": "problem", "display_name": "Emoji Land 😎", "data": "<problem>emoji in the body 😎</problem>"},
+    )
+    def test_copy_and_paste_component(self, block_args):
+        """
+        Test copying a component (XBlock) from one course into another
+        """
+        source_course = CourseFactory.create(display_name='Destination Course')
+        source_block = BlockFactory.create(parent_location=source_course.location, **block_args)
+
+        dest_course = CourseFactory.create(display_name='Destination Course')
+        with self.store.bulk_operations(dest_course.id):
+            dest_chapter = BlockFactory.create(parent=dest_course, category='chapter', display_name='Section')
+            dest_sequential = BlockFactory.create(parent=dest_chapter, category='sequential', display_name='Subsection')
+
+        # Copy the block
+        client = APIClient()
+        client.login(username=self.user.username, password=self.user_password)
+        copy_response = client.post(CLIPBOARD_ENDPOINT, {"usage_key": str(source_block.location)}, format="json")
+        assert copy_response.status_code == 200
+
+        # Paste the unit
+        paste_response = client.post(XBLOCK_ENDPOINT, {
+            "parent_locator": str(dest_sequential.location),
+            "staged_content": "clipboard",
+        }, format="json")
+        assert paste_response.status_code == 200
+        dest_block_key = UsageKey.from_string(paste_response.json()["locator"])
+
+        dest_block = self.store.get_item(dest_block_key)
+        assert dest_block.display_name == source_block.display_name
+        # The new block should store a reference to where it was copied from
+        assert dest_block.copied_from_block == str(source_block.location)
+
     def test_paste_with_assets(self):
         """
         When pasting into a different course, any required static assets should