Skip to content

Commit 804a91f

Browse files
committed
refactor: Move available_translations into VideoConfigService
This moves edx-platform-specific logic out of the VideoBlock, in preparation for the VideoBlock extraction: #36282
1 parent c5333b3 commit 804a91f

File tree

6 files changed

+103
-58
lines changed

6 files changed

+103
-58
lines changed

lms/djangoapps/courseware/tests/test_video_handlers.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -953,13 +953,14 @@ class TestStudioTranscriptTranslationPostDispatch(TestVideo): # lint-amnesty, p
953953
"error_message": "A transcript file is required."
954954
},
955955
)
956+
@patch('openedx.core.djangoapps.video_config.services.VideoConfigService.available_translations')
956957
@ddt.unpack
957-
def test_studio_transcript_post_validations(self, post_data, error_message):
958+
def test_studio_transcript_post_validations(self, mock_available_translations, post_data, error_message):
958959
"""
959960
Verify that POST request validations works as expected.
960961
"""
961-
# mock available_translations method
962-
self.block.available_translations = lambda transcripts, verify_assets: ['ur']
962+
# mock available_translations method to return ['ur']
963+
mock_available_translations.return_value = ['ur']
963964
request = Request.blank('/translation', POST=post_data)
964965
response = self.block.studio_transcript(request=request, dispatch='translation')
965966
assert response.json['error'] == error_message

openedx/core/djangoapps/video_config/services.py

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77
"""
88

99
import logging
10+
from typing import Any
1011

12+
from django.conf import settings
1113
from opaque_keys.edx.keys import CourseKey, UsageKey
1214
from opaque_keys.edx.locator import LibraryLocatorV2
1315
from xblocks_contrib.video.exceptions import TranscriptNotFoundError
@@ -34,6 +36,9 @@
3436
remove_subs_from_store,
3537
)
3638

39+
from xmodule.exceptions import NotFoundError
40+
41+
3742
log = logging.getLogger(__name__)
3843

3944

@@ -129,7 +134,6 @@ def get_transcript(
129134
"""
130135
# Import here to avoid circular dependency
131136
from openedx.core.djangoapps.video_config.transcripts_utils import get_transcript
132-
from xmodule.exceptions import NotFoundError
133137

134138
try:
135139
return get_transcript(video_block, lang, output_format, youtube_id)
@@ -138,6 +142,68 @@ def get_transcript(
138142
f"Failed to get transcript: {exc}"
139143
) from exc
140144

145+
def available_translations(
146+
self,
147+
video_block,
148+
transcripts: dict[str, Any],
149+
verify_assets: bool | None = None,
150+
is_bumper: bool = False,
151+
) -> list[str]:
152+
"""
153+
Return a list of language codes for which we have transcripts.
154+
155+
Arguments:
156+
video_block: The video XBlock instance
157+
transcripts (dict): A dict with all transcripts and a sub.
158+
verify_assets (boolean): If True, checks to ensure that the transcripts
159+
really exist in the contentstore. If False, we just look at the
160+
VideoBlock fields and do not query the contentstore. One reason
161+
we might do this is to avoid slamming contentstore() with queries
162+
when trying to make a listing of videos and their languages.
163+
164+
Defaults to `not FALLBACK_TO_ENGLISH_TRANSCRIPTS`.
165+
166+
is_bumper (boolean): If True, indicates this is a bumper video.
167+
168+
Returns:
169+
list[str]: List of language codes for available transcripts.
170+
"""
171+
from openedx.core.djangoapps.video_config.transcripts_utils import (
172+
get_transcript_for_video,
173+
get_transcript,
174+
)
175+
176+
translations = []
177+
if verify_assets is None:
178+
verify_assets = not settings.FEATURES.get('FALLBACK_TO_ENGLISH_TRANSCRIPTS')
179+
180+
sub, other_langs = transcripts["sub"], transcripts["transcripts"]
181+
182+
if verify_assets:
183+
all_langs = dict(**other_langs)
184+
if sub:
185+
all_langs.update({'en': sub})
186+
187+
for language, filename in all_langs.items():
188+
try:
189+
# for bumper videos, transcripts are stored in content store only
190+
if is_bumper:
191+
get_transcript_for_video(video_block.location, filename, filename, language)
192+
else:
193+
get_transcript(video_block, language)
194+
except NotFoundError:
195+
continue
196+
197+
translations.append(language)
198+
else:
199+
# If we're not verifying the assets, we just trust our field values
200+
translations = list(other_langs)
201+
if not translations or sub:
202+
translations += ['en']
203+
204+
# to clean redundant language codes.
205+
return list(set(translations))
206+
141207
def upload_transcript(
142208
self,
143209
*,

openedx/core/djangoapps/video_config/transcripts_utils.py

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -749,53 +749,6 @@ class VideoTranscriptsMixin:
749749
This is necessary for VideoBlock.
750750
"""
751751

752-
def available_translations(self, transcripts, verify_assets=None, is_bumper=False):
753-
"""
754-
Return a list of language codes for which we have transcripts.
755-
756-
Arguments:
757-
verify_assets (boolean): If True, checks to ensure that the transcripts
758-
really exist in the contentstore. If False, we just look at the
759-
VideoBlock fields and do not query the contentstore. One reason
760-
we might do this is to avoid slamming contentstore() with queries
761-
when trying to make a listing of videos and their languages.
762-
763-
Defaults to `not FALLBACK_TO_ENGLISH_TRANSCRIPTS`.
764-
765-
transcripts (dict): A dict with all transcripts and a sub.
766-
include_val_transcripts(boolean): If True, adds the edx-val transcript languages as well.
767-
"""
768-
translations = []
769-
if verify_assets is None:
770-
verify_assets = not settings.FEATURES.get('FALLBACK_TO_ENGLISH_TRANSCRIPTS')
771-
772-
sub, other_langs = transcripts["sub"], transcripts["transcripts"]
773-
774-
if verify_assets:
775-
all_langs = dict(**other_langs)
776-
if sub:
777-
all_langs.update({'en': sub})
778-
779-
for language, filename in all_langs.items():
780-
try:
781-
# for bumper videos, transcripts are stored in content store only
782-
if is_bumper:
783-
get_transcript_for_video(self.location, filename, filename, language)
784-
else:
785-
get_transcript(self, language)
786-
except NotFoundError:
787-
continue
788-
789-
translations.append(language)
790-
else:
791-
# If we're not verifying the assets, we just trust our field values
792-
translations = list(other_langs)
793-
if not translations or sub:
794-
translations += ['en']
795-
796-
# to clean redundant language codes.
797-
return list(set(translations))
798-
799752
def get_default_transcript_language(self, transcripts, dest_lang=None):
800753
"""
801754
Returns the default transcript language for this video block.

xmodule/tests/test_video.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,7 +1102,8 @@ def test_video_with_multiple_transcripts_translation_retrieval(self):
11021102
'''
11031103

11041104
block = instantiate_block(data=xml_data_transcripts)
1105-
translations = block.available_translations(block.get_transcripts_info())
1105+
video_config_service = block.runtime.service(block, 'video_config')
1106+
translations = video_config_service.available_translations(block, block.get_transcripts_info())
11061107
assert sorted(translations) == sorted(['hr', 'ge'])
11071108

11081109
def test_video_with_no_transcripts_translation_retrieval(self):
@@ -1112,13 +1113,14 @@ def test_video_with_no_transcripts_translation_retrieval(self):
11121113
does not throw an exception.
11131114
"""
11141115
block = instantiate_block(data=None)
1115-
translations_with_fallback = block.available_translations(block.get_transcripts_info())
1116+
video_config_service = block.runtime.service(block, 'video_config')
1117+
translations_with_fallback = video_config_service.available_translations(block, block.get_transcripts_info())
11161118
assert translations_with_fallback == ['en']
11171119

11181120
with patch.dict(settings.FEATURES, FALLBACK_TO_ENGLISH_TRANSCRIPTS=False):
11191121
# Some organizations don't have English transcripts for all videos
11201122
# This feature makes it configurable
1121-
translations_no_fallback = block.available_translations(block.get_transcripts_info())
1123+
translations_no_fallback = video_config_service.available_translations(block, block.get_transcripts_info())
11221124
assert translations_no_fallback == []
11231125

11241126
@override_settings(ALL_LANGUAGES=ALL_LANGUAGES)
@@ -1142,7 +1144,12 @@ def test_video_with_language_do_not_have_transcripts_translation(self):
11421144
</video>
11431145
'''
11441146
block = instantiate_block(data=xml_data_transcripts)
1145-
translations = block.available_translations(block.get_transcripts_info(), verify_assets=False)
1147+
video_config_service = block.runtime.service(block, 'video_config')
1148+
translations = video_config_service.available_translations(
1149+
block,
1150+
block.get_transcripts_info(),
1151+
verify_assets=False
1152+
)
11461153
assert translations != ['ur']
11471154

11481155
def assert_validation_message(self, validation, expected_msg):

xmodule/video_block/video_block.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1204,7 +1204,14 @@ def student_view_data(self, context=None):
12041204
"file_size": 0, # File size is not relevant for external link
12051205
}
12061206

1207-
available_translations = self.available_translations(self.get_transcripts_info())
1207+
video_config_service = self.runtime.service(self, 'video_config')
1208+
if video_config_service:
1209+
available_translations = video_config_service.available_translations(
1210+
self,
1211+
self.get_transcripts_info()
1212+
)
1213+
else:
1214+
available_translations = []
12081215
transcripts = {
12091216
lang: self.runtime.handler_url(self, 'transcript', 'download', query="lang=" + lang, thirdparty=True)
12101217
for lang in available_translations

xmodule/video_block/video_handlers.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,11 @@ def transcript(self, request, dispatch):
322322
mimetype
323323
)
324324
elif dispatch.startswith('available_translations'):
325-
available_translations = self.available_translations(
325+
video_config_service = self.runtime.service(self, 'video_config')
326+
if not video_config_service:
327+
return Response(status=404)
328+
available_translations = video_config_service.available_translations(
329+
self,
326330
transcripts,
327331
verify_assets=True,
328332
is_bumper=is_bumper
@@ -395,7 +399,14 @@ def validate_transcript_upload_data(self, data):
395399

396400
# Get available transcript languages.
397401
transcripts = self.get_transcripts_info()
398-
available_translations = self.available_translations(transcripts, verify_assets=True)
402+
video_config_service = self.runtime.service(self, 'video_config')
403+
if not video_config_service:
404+
return error
405+
available_translations = video_config_service.available_translations(
406+
self,
407+
transcripts,
408+
verify_assets=True
409+
)
399410

400411
if missing:
401412
error = _('The following parameters are required: {missing}.').format(missing=', '.join(missing))

0 commit comments

Comments
 (0)