Merge branch 'master' into problemblock-refactor

Author: irtazaakram

cms/djangoapps/contentstore/rest_api/v2/views/home.py

@@ -15,6 +15,7 @@
 
 class HomePageCoursesPaginator(PageNumberPagination):
     """Custom paginator for the home page courses view version 2."""
+    page_size_query_param = 'page_size'
 
     def get_paginated_response(self, data):
         """Return a paginated style `Response` object for the given output data."""
@@ -77,6 +78,11 @@ class HomePageCoursesViewV2(APIView):
                 apidocs.ParameterLocation.QUERY,
                 description="Query param to paginate the courses",
             ),
+            apidocs.string_parameter(
+                "page_size",
+                apidocs.ParameterLocation.QUERY,
+                description="Query param to set page size",
+            ),
         ],
         responses={
             200: CourseHomeTabSerializerV2,
@@ -96,6 +102,7 @@ def get(self, request: Request):
             GET /api/contentstore/v2/home/courses?active_only=true
             GET /api/contentstore/v2/home/courses?archived_only=true
             GET /api/contentstore/v2/home/courses?page=2
+            GET /api/contentstore/v2/home/courses?page_size=20
 
         **Response Values**
 

cms/djangoapps/modulestore_migrator/tasks.py

@@ -417,7 +417,11 @@ def _populate_collection(user_id: int, migration: models.ModulestoreMigration) -
         log.warning("No target entities found to add to collection")
 
 
-def _create_collection(library_key: LibraryLocatorV2, title: str) -> Collection:
+def _create_collection(
+    library_key: LibraryLocatorV2,
+    title: str,
+    course_name: str | None = None,
+) -> Collection:
     """
     Creates a collection in the given library
 
@@ -428,7 +432,10 @@ def _create_collection(library_key: LibraryLocatorV2, title: str) -> Collection:
     collection: Collection | None = None
     attempt = 0
     created_at = strftime_localized(datetime.now(timezone.utc), DEFAULT_DATE_TIME_FORMAT)
-    description = f"{_('This collection contains content migrated from a legacy library on')}: {created_at}"
+    if course_name:
+        description = f"{_('This collection contains content imported from the course')} {course_name} on: {created_at}"
+    else:
+        description = f"{_('This collection contains content migrated from a legacy library on')}: {created_at}"
     while not collection:
         modified_key = key if attempt == 0 else key + '-' + str(attempt)
         try:
@@ -694,7 +701,11 @@ def bulk_migrate_from_modulestore(
                                 pass
                 migration.target_collection = (
                     existing_collection_to_use or
-                    _create_collection(library_key=target_library_locator, title=legacy_root_list[i].display_name)
+                    _create_collection(
+                        library_key=target_library_locator,
+                        title=legacy_root_list[i].display_name,
+                        course_name=legacy_root_list[i].display_name if source_data.source.key.is_course else None,
+                    )
                 )
             _populate_collection(user_id, migration)
         models.ModulestoreMigration.objects.bulk_update(

cms/djangoapps/modulestore_migrator/tests/test_tasks.py

@@ -1118,11 +1118,12 @@ def test_bulk_migrate_create_collections(self):
         """
         source = ModulestoreSource.objects.create(key=self.legacy_library.location.library_key)
         source_2 = ModulestoreSource.objects.create(key=self.legacy_library_2.location.library_key)
+        source_3 = ModulestoreSource.objects.create(key=self.course.id)
 
         task = bulk_migrate_from_modulestore.apply_async(
             kwargs={
                 "user_id": self.user.id,
-                "sources_pks": [source.id, source_2.id],
+                "sources_pks": [source.id, source_2.id, source_3.id],
                 "target_library_key": str(self.lib_key),
                 "target_collection_pks": [],
                 "create_collections": True,
@@ -1142,13 +1143,32 @@ def test_bulk_migrate_create_collections(self):
         self.assertEqual(migration.composition_level, CompositionLevel.Unit.value)
         self.assertEqual(migration.repeat_handling_strategy, RepeatHandlingStrategy.Skip.value)
         self.assertEqual(migration.target_collection.title, self.legacy_library.display_name)
+        self.assertIn(
+            "This collection contains content migrated from a legacy library on:",
+            migration.target_collection.description,
+        )
 
         migration_2 = ModulestoreMigration.objects.get(
             source=source_2, target=self.learning_package
         )
         self.assertEqual(migration_2.composition_level, CompositionLevel.Unit.value)
         self.assertEqual(migration_2.repeat_handling_strategy, RepeatHandlingStrategy.Skip.value)
         self.assertEqual(migration_2.target_collection.title, self.legacy_library_2.display_name)
+        self.assertIn(
+            "This collection contains content migrated from a legacy library on:",
+            migration_2.target_collection.description,
+        )
+
+        migration_3 = ModulestoreMigration.objects.get(
+            source=source_3, target=self.learning_package
+        )
+        self.assertEqual(migration_3.composition_level, CompositionLevel.Unit.value)
+        self.assertEqual(migration_3.repeat_handling_strategy, RepeatHandlingStrategy.Skip.value)
+        self.assertEqual(migration_3.target_collection.title, self.course.display_name)
+        self.assertIn(
+            f"This collection contains content imported from the course {self.course.display_name} on:",
+            migration_3.target_collection.description,
+        )
 
     @ddt.data(
         RepeatHandlingStrategy.Skip,

lms/djangoapps/courseware/tests/test_video_handlers.py

@@ -953,13 +953,14 @@ class TestStudioTranscriptTranslationPostDispatch(TestVideo):  # lint-amnesty, p
             "error_message": "A transcript file is required."
         },
     )
+    @patch('openedx.core.djangoapps.video_config.services.VideoConfigService.available_translations')
     @ddt.unpack
-    def test_studio_transcript_post_validations(self, post_data, error_message):
+    def test_studio_transcript_post_validations(self, mock_available_translations, post_data, error_message):
         """
         Verify that POST request validations works as expected.
         """
-        # mock available_translations method
-        self.block.available_translations = lambda transcripts, verify_assets: ['ur']
+        # mock available_translations method to return ['ur']
+        mock_available_translations.return_value = ['ur']
         request = Request.blank('/translation', POST=post_data)
         response = self.block.studio_transcript(request=request, dispatch='translation')
         assert response.json['error'] == error_message

openedx/core/djangoapps/video_config/services.py

@@ -7,7 +7,9 @@
 """
 
 import logging
+from typing import Any
 
+from django.conf import settings
 from opaque_keys.edx.keys import CourseKey, UsageKey
 from opaque_keys.edx.locator import LibraryLocatorV2
 from xblocks_contrib.video.exceptions import TranscriptNotFoundError
@@ -32,8 +34,13 @@
     clean_video_id,
     get_html5_ids,
     remove_subs_from_store,
+    get_transcript_for_video,
+    get_transcript,
 )
 
+from xmodule.exceptions import NotFoundError
+
+
 log = logging.getLogger(__name__)
 
 
@@ -127,17 +134,70 @@ def get_transcript(
             TranscriptsGenerationException: If the transcript cannot be found or retrieved
             TranscriptNotFoundError: If the transcript cannot be found or retrieved
         """
-        # Import here to avoid circular dependency
-        from openedx.core.djangoapps.video_config.transcripts_utils import get_transcript
-        from xmodule.exceptions import NotFoundError
-
         try:
             return get_transcript(video_block, lang, output_format, youtube_id)
         except NotFoundError as exc:
             raise TranscriptNotFoundError(
                 f"Failed to get transcript: {exc}"
             ) from exc
 
+    def available_translations(
+        self,
+        video_block,
+        transcripts: dict[str, Any],
+        verify_assets: bool | None = None,
+        is_bumper: bool = False,
+    ) -> list[str]:
+        """
+        Return a list of language codes for which we have transcripts.
+
+        Arguments:
+            video_block: The video XBlock instance
+            transcripts (dict): A dict with all transcripts and a sub.
+            verify_assets (boolean): If True, checks to ensure that the transcripts
+                really exist in the contentstore. If False, we just look at the
+                VideoBlock fields and do not query the contentstore. One reason
+                we might do this is to avoid slamming contentstore() with queries
+                when trying to make a listing of videos and their languages.
+
+                Defaults to `not FALLBACK_TO_ENGLISH_TRANSCRIPTS`.
+
+            is_bumper (boolean): If True, indicates this is a bumper video.
+
+        Returns:
+            list[str]: List of language codes for available transcripts.
+        """
+        translations = []
+        if verify_assets is None:
+            verify_assets = not settings.FEATURES.get('FALLBACK_TO_ENGLISH_TRANSCRIPTS')
+
+        sub, other_langs = transcripts["sub"], transcripts["transcripts"]
+
+        if verify_assets:
+            all_langs = dict(**other_langs)
+            if sub:
+                all_langs.update({'en': sub})
+
+            for language, filename in all_langs.items():
+                try:
+                    # for bumper videos, transcripts are stored in content store only
+                    if is_bumper:
+                        get_transcript_for_video(video_block.location, filename, filename, language)
+                    else:
+                        get_transcript(video_block, language)
+                except NotFoundError:
+                    continue
+
+                translations.append(language)
+        else:
+            # If we're not verifying the assets, we just trust our field values
+            translations = list(other_langs)
+            if not translations or sub:
+                translations += ['en']
+
+        # to clean redundant language codes.
+        return list(set(translations))
+
     def upload_transcript(
         self,
         *,

openedx/core/djangoapps/video_config/transcripts_utils.py

@@ -749,53 +749,6 @@ class VideoTranscriptsMixin:
     This is necessary for VideoBlock.
     """
 
-    def available_translations(self, transcripts, verify_assets=None, is_bumper=False):
-        """
-        Return a list of language codes for which we have transcripts.
-
-        Arguments:
-            verify_assets (boolean): If True, checks to ensure that the transcripts
-                really exist in the contentstore. If False, we just look at the
-                VideoBlock fields and do not query the contentstore. One reason
-                we might do this is to avoid slamming contentstore() with queries
-                when trying to make a listing of videos and their languages.
-
-                Defaults to `not FALLBACK_TO_ENGLISH_TRANSCRIPTS`.
-
-            transcripts (dict): A dict with all transcripts and a sub.
-            include_val_transcripts(boolean): If True, adds the edx-val transcript languages as well.
-        """
-        translations = []
-        if verify_assets is None:
-            verify_assets = not settings.FEATURES.get('FALLBACK_TO_ENGLISH_TRANSCRIPTS')
-
-        sub, other_langs = transcripts["sub"], transcripts["transcripts"]
-
-        if verify_assets:
-            all_langs = dict(**other_langs)
-            if sub:
-                all_langs.update({'en': sub})
-
-            for language, filename in all_langs.items():
-                try:
-                    # for bumper videos, transcripts are stored in content store only
-                    if is_bumper:
-                        get_transcript_for_video(self.location, filename, filename, language)
-                    else:
-                        get_transcript(self, language)
-                except NotFoundError:
-                    continue
-
-                translations.append(language)
-        else:
-            # If we're not verifying the assets, we just trust our field values
-            translations = list(other_langs)
-            if not translations or sub:
-                translations += ['en']
-
-        # to clean redundant language codes.
-        return list(set(translations))
-
     def get_default_transcript_language(self, transcripts, dest_lang=None):
         """
         Returns the default transcript language for this video block.

xmodule/tests/test_video.py

@@ -1102,7 +1102,8 @@ def test_video_with_multiple_transcripts_translation_retrieval(self):
         '''
 
         block = instantiate_block(data=xml_data_transcripts)
-        translations = block.available_translations(block.get_transcripts_info())
+        video_config_service = block.runtime.service(block, 'video_config')
+        translations = video_config_service.available_translations(block, block.get_transcripts_info())
         assert sorted(translations) == sorted(['hr', 'ge'])
 
     def test_video_with_no_transcripts_translation_retrieval(self):
@@ -1112,13 +1113,14 @@ def test_video_with_no_transcripts_translation_retrieval(self):
         does not throw an exception.
         """
         block = instantiate_block(data=None)
-        translations_with_fallback = block.available_translations(block.get_transcripts_info())
+        video_config_service = block.runtime.service(block, 'video_config')
+        translations_with_fallback = video_config_service.available_translations(block, block.get_transcripts_info())
         assert translations_with_fallback == ['en']
 
         with patch.dict(settings.FEATURES, FALLBACK_TO_ENGLISH_TRANSCRIPTS=False):
             # Some organizations don't have English transcripts for all videos
             # This feature makes it configurable
-            translations_no_fallback = block.available_translations(block.get_transcripts_info())
+            translations_no_fallback = video_config_service.available_translations(block, block.get_transcripts_info())
             assert translations_no_fallback == []
 
     @override_settings(ALL_LANGUAGES=ALL_LANGUAGES)
@@ -1142,7 +1144,12 @@ def test_video_with_language_do_not_have_transcripts_translation(self):
             </video>
         '''
         block = instantiate_block(data=xml_data_transcripts)
-        translations = block.available_translations(block.get_transcripts_info(), verify_assets=False)
+        video_config_service = block.runtime.service(block, 'video_config')
+        translations = video_config_service.available_translations(
+            block,
+            block.get_transcripts_info(),
+            verify_assets=False
+        )
         assert translations != ['ur']
 
     def assert_validation_message(self, validation, expected_msg):

xmodule/video_block/video_block.py

@@ -1204,7 +1204,14 @@ def student_view_data(self, context=None):
                     "file_size": 0,  # File size is not relevant for external link
                 }
 
-        available_translations = self.available_translations(self.get_transcripts_info())
+        video_config_service = self.runtime.service(self, 'video_config')
+        if video_config_service:
+            available_translations = video_config_service.available_translations(
+                self,
+                self.get_transcripts_info()
+            )
+        else:
+            available_translations = []
         transcripts = {
             lang: self.runtime.handler_url(self, 'transcript', 'download', query="lang=" + lang, thirdparty=True)
             for lang in available_translations

xmodule/video_block/video_handlers.py

@@ -322,7 +322,11 @@ def transcript(self, request, dispatch):
                 mimetype
             )
         elif dispatch.startswith('available_translations'):
-            available_translations = self.available_translations(
+            video_config_service = self.runtime.service(self, 'video_config')
+            if not video_config_service:
+                return Response(status=404)
+            available_translations = video_config_service.available_translations(
+                self,
                 transcripts,
                 verify_assets=True,
                 is_bumper=is_bumper
@@ -395,7 +399,14 @@ def validate_transcript_upload_data(self, data):
 
         # Get available transcript languages.
         transcripts = self.get_transcripts_info()
-        available_translations = self.available_translations(transcripts, verify_assets=True)
+        video_config_service = self.runtime.service(self, 'video_config')
+        if not video_config_service:
+            return error
+        available_translations = video_config_service.available_translations(
+            self,
+            transcripts,
+            verify_assets=True
+        )
 
         if missing:
             error = _('The following parameters are required: {missing}.').format(missing=', '.join(missing))