From 360a97fdd36e394886cf9e4516e5e2cfe556bff4 Mon Sep 17 00:00:00 2001 From: Muhammad Farhan Khan Date: Fri, 26 Dec 2025 18:56:57 +0500 Subject: [PATCH] refactor: Move available_translations into VideoConfigService (#37808) This moves edx-platform-specific logic out of the VideoBlock, in preparation for the VideoBlock extraction: openedx#36282 --- .../courseware/tests/test_video_handlers.py | 7 +- .../core/djangoapps/video_config/services.py | 68 +++++++++++++++++-- .../video_config/transcripts_utils.py | 47 ------------- xmodule/tests/test_video.py | 15 ++-- xmodule/video_block/video_block.py | 9 ++- xmodule/video_block/video_handlers.py | 15 +++- 6 files changed, 100 insertions(+), 61 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_video_handlers.py b/lms/djangoapps/courseware/tests/test_video_handlers.py index 8f136c75fd..6353e2aec7 100644 --- a/lms/djangoapps/courseware/tests/test_video_handlers.py +++ b/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 diff --git a/openedx/core/djangoapps/video_config/services.py b/openedx/core/djangoapps/video_config/services.py index 63d1445dfe..3e311cf779 100644 --- a/openedx/core/djangoapps/video_config/services.py +++ b/openedx/core/djangoapps/video_config/services.py @@ -7,7 +7,9 @@ for the extracted video block in xblocks-contrib repository. """ 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 @@ from openedx.core.djangoapps.video_config.transcripts_utils import ( 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,10 +134,6 @@ class VideoConfigService: 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: @@ -138,6 +141,63 @@ class VideoConfigService: 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, *, diff --git a/openedx/core/djangoapps/video_config/transcripts_utils.py b/openedx/core/djangoapps/video_config/transcripts_utils.py index cb3869456d..2ad873930e 100644 --- a/openedx/core/djangoapps/video_config/transcripts_utils.py +++ b/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. diff --git a/xmodule/tests/test_video.py b/xmodule/tests/test_video.py index be605aa129..aa03c15851 100644 --- a/xmodule/tests/test_video.py +++ b/xmodule/tests/test_video.py @@ -1102,7 +1102,8 @@ class VideoBlockIndexingTestCase(unittest.TestCase): ''' 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 @@ class VideoBlockIndexingTestCase(unittest.TestCase): 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 @@ class VideoBlockIndexingTestCase(unittest.TestCase): ''' 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): diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index e8a7b7e87e..34e2b2d877 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -1204,7 +1204,14 @@ class _BuiltInVideoBlock( "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 diff --git a/xmodule/video_block/video_handlers.py b/xmodule/video_block/video_handlers.py index 4c338787f0..41ae314d59 100644 --- a/xmodule/video_block/video_handlers.py +++ b/xmodule/video_block/video_handlers.py @@ -322,7 +322,11 @@ class VideoStudentViewHandlers: 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 @@ class VideoStudioViewHandlers: # 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))