Skip to content

Commit 5b237b8

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 24cb3cf commit 5b237b8

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
@@ -955,13 +955,14 @@ class TestStudioTranscriptTranslationPostDispatch(TestVideo): # lint-amnesty, p
955955
"error_message": "A transcript file is required."
956956
},
957957
)
958+
@patch('openedx.core.djangoapps.video_config.services.VideoConfigService.available_translations')
958959
@ddt.unpack
959-
def test_studio_transcript_post_validations(self, post_data, error_message):
960+
def test_studio_transcript_post_validations(self, mock_available_translations, post_data, error_message):
960961
"""
961962
Verify that POST request validations works as expected.
962963
"""
963-
# mock available_translations method
964-
self.block.available_translations = lambda transcripts, verify_assets: ['ur']
964+
# mock available_translations method to return ['ur']
965+
mock_available_translations.return_value = ['ur']
965966
request = Request.blank('/translation', POST=post_data)
966967
response = self.block.studio_transcript(request=request, dispatch='translation')
967968
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 xblocks_contrib.video.exceptions import TranscriptNotFoundError
1315

@@ -20,6 +22,9 @@
2022
from openedx.core.djangoapps.video_config.toggles import TRANSCRIPT_FEEDBACK
2123
from openedx.core.djangoapps.video_pipeline.config.waffle import DEPRECATE_YOUTUBE
2224

25+
from xmodule.exceptions import NotFoundError
26+
27+
2328
log = logging.getLogger(__name__)
2429

2530

@@ -112,11 +117,72 @@ def get_transcript(
112117
"""
113118
# Import here to avoid circular dependency
114119
from openedx.core.djangoapps.video_config.transcripts_utils import get_transcript
115-
from xmodule.exceptions import NotFoundError
116120

117121
try:
118122
return get_transcript(video_block, lang, output_format, youtube_id)
119123
except NotFoundError as exc:
120124
raise TranscriptNotFoundError(
121125
f"Failed to get transcript: {exc}"
122126
) from exc
127+
128+
def available_translations(
129+
self,
130+
video_block,
131+
transcripts: dict[str, Any],
132+
verify_assets: bool | None = None,
133+
is_bumper: bool = False,
134+
) -> list[str]:
135+
"""
136+
Return a list of language codes for which we have transcripts.
137+
138+
Arguments:
139+
video_block: The video XBlock instance
140+
transcripts (dict): A dict with all transcripts and a sub.
141+
verify_assets (boolean): If True, checks to ensure that the transcripts
142+
really exist in the contentstore. If False, we just look at the
143+
VideoBlock fields and do not query the contentstore. One reason
144+
we might do this is to avoid slamming contentstore() with queries
145+
when trying to make a listing of videos and their languages.
146+
147+
Defaults to `not FALLBACK_TO_ENGLISH_TRANSCRIPTS`.
148+
149+
is_bumper (boolean): If True, indicates this is a bumper video.
150+
151+
Returns:
152+
list[str]: List of language codes for available transcripts.
153+
"""
154+
from openedx.core.djangoapps.video_config.transcripts_utils import (
155+
get_transcript_for_video,
156+
get_transcript,
157+
)
158+
159+
translations = []
160+
if verify_assets is None:
161+
verify_assets = not settings.FEATURES.get('FALLBACK_TO_ENGLISH_TRANSCRIPTS')
162+
163+
sub, other_langs = transcripts["sub"], transcripts["transcripts"]
164+
165+
if verify_assets:
166+
all_langs = dict(**other_langs)
167+
if sub:
168+
all_langs.update({'en': sub})
169+
170+
for language, filename in all_langs.items():
171+
try:
172+
# for bumper videos, transcripts are stored in content store only
173+
if is_bumper:
174+
get_transcript_for_video(video_block.location, filename, filename, language)
175+
else:
176+
get_transcript(video_block, language)
177+
except NotFoundError:
178+
continue
179+
180+
translations.append(language)
181+
else:
182+
# If we're not verifying the assets, we just trust our field values
183+
translations = list(other_langs)
184+
if not translations or sub:
185+
translations += ['en']
186+
187+
# to clean redundant language codes.
188+
return list(set(translations))

openedx/core/djangoapps/video_config/transcripts_utils.py

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -825,53 +825,6 @@ class VideoTranscriptsMixin:
825825
This is necessary for VideoBlock.
826826
"""
827827

828-
def available_translations(self, transcripts, verify_assets=None, is_bumper=False):
829-
"""
830-
Return a list of language codes for which we have transcripts.
831-
832-
Arguments:
833-
verify_assets (boolean): If True, checks to ensure that the transcripts
834-
really exist in the contentstore. If False, we just look at the
835-
VideoBlock fields and do not query the contentstore. One reason
836-
we might do this is to avoid slamming contentstore() with queries
837-
when trying to make a listing of videos and their languages.
838-
839-
Defaults to `not FALLBACK_TO_ENGLISH_TRANSCRIPTS`.
840-
841-
transcripts (dict): A dict with all transcripts and a sub.
842-
include_val_transcripts(boolean): If True, adds the edx-val transcript languages as well.
843-
"""
844-
translations = []
845-
if verify_assets is None:
846-
verify_assets = not settings.FEATURES.get('FALLBACK_TO_ENGLISH_TRANSCRIPTS')
847-
848-
sub, other_langs = transcripts["sub"], transcripts["transcripts"]
849-
850-
if verify_assets:
851-
all_langs = dict(**other_langs)
852-
if sub:
853-
all_langs.update({'en': sub})
854-
855-
for language, filename in all_langs.items():
856-
try:
857-
# for bumper videos, transcripts are stored in content store only
858-
if is_bumper:
859-
get_transcript_for_video(self.location, filename, filename, language)
860-
else:
861-
get_transcript(self, language)
862-
except NotFoundError:
863-
continue
864-
865-
translations.append(language)
866-
else:
867-
# If we're not verifying the assets, we just trust our field values
868-
translations = list(other_langs)
869-
if not translations or sub:
870-
translations += ['en']
871-
872-
# to clean redundant language codes.
873-
return list(set(translations))
874-
875828
def get_default_transcript_language(self, transcripts, dest_lang=None):
876829
"""
877830
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
@@ -403,7 +403,11 @@ def transcript(self, request, dispatch):
403403
mimetype
404404
)
405405
elif dispatch.startswith('available_translations'):
406-
available_translations = self.available_translations(
406+
video_config_service = self.runtime.service(self, 'video_config')
407+
if not video_config_service:
408+
return Response(status=404)
409+
available_translations = video_config_service.available_translations(
410+
self,
407411
transcripts,
408412
verify_assets=True,
409413
is_bumper=is_bumper
@@ -476,7 +480,14 @@ def validate_transcript_upload_data(self, data):
476480

477481
# Get available transcript languages.
478482
transcripts = self.get_transcripts_info()
479-
available_translations = self.available_translations(transcripts, verify_assets=True)
483+
video_config_service = self.runtime.service(self, 'video_config')
484+
if not video_config_service:
485+
return error
486+
available_translations = video_config_service.available_translations(
487+
self,
488+
transcripts,
489+
verify_assets=True
490+
)
480491

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

0 commit comments

Comments
 (0)