Skip to content

Commit b27c671

Browse files
committed
refactor: Move upload/delete transcript int video_config service
This moves edx-platform-specific logic out of the VideoBlock, in preparation for the VideoBlock extraction: #36282
1 parent 41acf0e commit b27c671

File tree

4 files changed

+182
-137
lines changed

4 files changed

+182
-137
lines changed

openedx/core/djangoapps/video_config/services.py

Lines changed: 136 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,33 @@
88

99
import logging
1010

11-
from opaque_keys.edx.keys import CourseKey, UsageKey
12-
13-
from openedx.core.djangoapps.video_config import sharing
11+
from django.core.files import File
12+
from django.core.files.base import ContentFile
13+
from edxval.api import create_external_video, create_or_update_video_transcript, delete_video_transcript
14+
from opaque_keys.edx.keys import (
15+
CourseKey,
16+
UsageKey,
17+
)
18+
from opaque_keys.edx.locator import LibraryLocatorV2
19+
from xblocks_contrib.video import VideoBlock
1420
from organizations.api import get_course_organization
21+
from openedx.core.djangoapps.video_config import sharing
1522
from openedx.core.djangoapps.video_config.models import (
1623
CourseYoutubeBlockedFlag,
1724
HLSPlaybackEnabledFlag,
1825
)
1926
from openedx.core.djangoapps.video_config.toggles import TRANSCRIPT_FEEDBACK
2027
from openedx.core.djangoapps.video_pipeline.config.waffle import DEPRECATE_YOUTUBE
28+
from openedx.core.djangoapps.content_libraries.api import (
29+
add_library_block_static_asset_file,
30+
delete_library_block_static_asset_file,
31+
)
32+
from openedx.core.djangoapps.video_config.transcripts_utils import (
33+
Transcript,
34+
clean_video_id,
35+
get_html5_ids,
36+
remove_subs_from_store,
37+
)
2138

2239
log = logging.getLogger(__name__)
2340

@@ -29,6 +46,9 @@ class VideoConfigService:
2946
This service abstracts away edx-platform specific functionality
3047
that the Video XBlock needs, allowing the Video XBlock to be
3148
extracted to a separate repository.
49+
50+
TODO: This service could be improved in a few ways:
51+
https://github.com/openedx/edx-platform/issues/37656
3252
"""
3353

3454
def get_public_video_url(self, usage_id: UsageKey) -> str:
@@ -37,7 +57,7 @@ def get_public_video_url(self, usage_id: UsageKey) -> str:
3757
"""
3858
return sharing.get_public_video_url(usage_id)
3959

40-
def get_public_sharing_context(self, video_block, course_key: CourseKey) -> dict:
60+
def get_public_sharing_context(self, video_block: VideoBlock, course_key: CourseKey) -> dict:
4161
"""
4262
Get the complete public sharing context for a video.
4363
@@ -91,3 +111,115 @@ def is_hls_playback_enabled(self, course_id: CourseKey) -> bool:
91111
Check if HLS playback is enabled for the course.
92112
"""
93113
return HLSPlaybackEnabledFlag.feature_enabled(course_id)
114+
115+
def upload_transcript(
116+
self,
117+
*,
118+
video_block: VideoBlock,
119+
edx_video_id: str | None,
120+
transcript_file: File,
121+
language_code: str,
122+
new_language_code: str,
123+
) -> None:
124+
"""
125+
Store a transcript, however the runtime prefers to.
126+
127+
Can raise:
128+
* UnicodeDecodeError
129+
* TranscriptsGenerationException
130+
"""
131+
edx_video_id = clean_video_id(edx_video_id)
132+
is_library = isinstance(video_block.usage_key.context_key, LibraryLocatorV2)
133+
if is_library:
134+
filename = f'transcript-{new_language_code}.srt'
135+
else:
136+
if not edx_video_id:
137+
# Back-populate the video ID for an external video.
138+
# pylint: disable=attribute-defined-outside-init
139+
video_block.edx_video_id = edx_video_id = create_external_video(display_name='external video')
140+
filename = f'{edx_video_id}-{new_language_code}.srt'
141+
142+
content = transcript_file.read()
143+
if is_library:
144+
# Save transcript as static asset in Learning Core if is a library component
145+
add_library_block_static_asset_file(video_block.usage_key, f"static/{filename}", content)
146+
else:
147+
# Convert SRT transcript into an SJSON format and upload it to S3 if a course component
148+
sjson_subs = Transcript.convert(
149+
content=content.decode('utf-8'),
150+
input_format=Transcript.SRT,
151+
output_format=Transcript.SJSON
152+
).encode()
153+
create_or_update_video_transcript(
154+
video_id=edx_video_id,
155+
language_code=language_code,
156+
metadata={
157+
'file_format': Transcript.SJSON,
158+
'language_code': new_language_code
159+
},
160+
file_data=ContentFile(sjson_subs),
161+
)
162+
163+
# If a new transcript is added, then both new_language_code and
164+
# language_code fields will have the same value.
165+
if language_code != new_language_code:
166+
video_block.transcripts.pop(language_code, None)
167+
video_block.transcripts[new_language_code] = filename
168+
169+
if is_library:
170+
_save_transcript_field(video_block)
171+
172+
def delete_transcript(
173+
self,
174+
*,
175+
video_block: VideoBlock,
176+
edx_video_id: str | None,
177+
language_code: str,
178+
) -> None:
179+
"""
180+
Delete a transcript from the runtime's storage.
181+
"""
182+
edx_video_id = clean_video_id(edx_video_id)
183+
if edx_video_id:
184+
delete_video_transcript(video_id=edx_video_id, language_code=language_code)
185+
if isinstance(video_block.context_key, LibraryLocatorV2):
186+
transcript_name = video_block.transcripts.pop(language_code, None)
187+
if transcript_name:
188+
delete_library_block_static_asset_file(video_block.usage_key, f"static/{transcript_name}")
189+
_save_transcript_field(video_block)
190+
else:
191+
if language_code == 'en':
192+
# remove any transcript file from content store for the video ids
193+
possible_sub_ids = [
194+
video_block.sub, # pylint: disable=access-member-before-definition
195+
video_block.youtube_id_1_0
196+
] + get_html5_ids(video_block.html5_sources)
197+
for sub_id in possible_sub_ids:
198+
remove_subs_from_store(sub_id, video_block, language_code)
199+
# update metadata as `en` can also be present in `transcripts` field
200+
remove_subs_from_store(
201+
video_block.transcripts.pop(language_code, None), video_block, language_code
202+
)
203+
# also empty `sub` field
204+
video_block.sub = '' # pylint: disable=attribute-defined-outside-init
205+
else:
206+
remove_subs_from_store(
207+
video_block.transcripts.pop(language_code, None), video_block, language_code
208+
)
209+
210+
211+
def _save_transcript_field(video_block: VideoBlock):
212+
"""
213+
Hacky workaround to ensure that transcript field is saved for Learning Core video blocks.
214+
215+
It's not clear why this is necessary.
216+
"""
217+
field = video_block.fields['transcripts']
218+
if video_block.transcripts:
219+
transcripts_copy = video_block.transcripts.copy()
220+
# Need to delete to overwrite, it's weird behavior,
221+
# but it only works like this.
222+
field.delete_from(video_block)
223+
field.write_to(video_block, transcripts_copy)
224+
else:
225+
field.delete_from(video_block)

openedx/core/djangoapps/video_config/transcripts_utils.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@
2828
from xmodule.contentstore.django import contentstore
2929
from xmodule.exceptions import NotFoundError
3030

31-
from xmodule.video_block.bumper_utils import get_bumper_settings
32-
3331
try:
3432
from edxval import api as edxval_api
3533
except ImportError:
@@ -911,6 +909,11 @@ def get_transcripts_info(self, is_bumper=False):
911909
is_bumper(bool): If True, the request is for the bumper transcripts
912910
include_val_transcripts(bool): If True, include edx-val transcripts as well
913911
"""
912+
# TODO: This causes a circular import when imported at the top-level.
913+
# This import will be removed as part of the VideoBlock extraction.
914+
# https://github.com/openedx/edx-platform/issues/36282
915+
from xmodule.video_block.bumper_utils import get_bumper_settings
916+
914917
if is_bumper:
915918
transcripts = copy.deepcopy(get_bumper_settings(self).get('transcripts', {}))
916919
sub = transcripts.pop("en", "")

setup.cfg

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,6 @@ ignore_imports =
106106
# -> openedx.core.djangoapps.course_groups.partition_scheme
107107
# -> lms.djangoapps.courseware.masquerade
108108
openedx.core.djangoapps.course_groups.partition_scheme -> lms.djangoapps.courseware.masquerade
109-
# cms.djangoapps.contentstore.[various]
110-
# -> openedx.core.djangoapps.content.course_overviews.models
111-
# -> lms.djangoapps.ccx.utils
112-
# & lms.djangoapps.certificates.api
113-
# & lms.djangoapps.discussion.django_comment_client
114-
openedx.core.djangoapps.content.course_overviews.models -> lms.djangoapps.*.*
115109
# cms.djangoapps.export_course_metadata.tasks
116110
# -> openedx.core.djangoapps.schedules.content_highlights
117111
# -> lms.djangoapps.courseware.block_render & lms.djangoapps.courseware.model_data
@@ -169,6 +163,9 @@ ignore_imports =
169163
# https://github.com/openedx/edx-platform/issues/33428
170164
openedx.core.djangoapps.content_libraries.permissions -> cms.djangoapps.course_creators.views
171165
openedx.core.djangoapps.content_libraries.tasks -> cms.djangoapps.contentstore.storage
166+
# Outstanding arch issue: course_overviews is tangled up with LMS
167+
# https://github.com/openedx/edx-platform/issues/37658
168+
openedx.core.djangoapps.content.course_overviews.models -> lms.djangoapps.**
172169

173170
[importlinter:contract:2]
174171
name = Do not depend on non-public API of isolated apps.
@@ -232,3 +229,5 @@ ignore_imports =
232229
# Content libraries imports contentstore.helpers which imports upstream_sync
233230
openedx.core.djangoapps.content_libraries.api.blocks -> cms.djangoapps.contentstore.helpers
234231
openedx.core.djangoapps.content_libraries.api.libraries -> cms.djangoapps.contentstore.helpers
232+
# Outstanding arch issue: course_overviews is tangled up with LMS
233+
openedx.core.djangoapps.content.course_overviews.models -> lms.djangoapps.**

0 commit comments

Comments
 (0)