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
This commit is contained in:
committed by
GitHub
parent
e26a3fb7e6
commit
360a97fdd3
@@ -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
|
||||
|
||||
@@ -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,
|
||||
*,
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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):
|
||||
</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):
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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))
|
||||
|
||||
Reference in New Issue
Block a user