From 9cdf45f59420b928e89a441a0e9e0cf497dcf980 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 25 Dec 2024 16:11:35 -0500 Subject: [PATCH 01/39] feat: Updates to support save video with transcript in library home * Add error handler on save video to avoid create sjson * Support transcripts without edx_video_id in definition_to_xml --- xmodule/tests/test_video.py | 42 ++++++++++++++++++++++++ xmodule/video_block/transcripts_utils.py | 2 ++ xmodule/video_block/video_block.py | 10 ++++-- 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/xmodule/tests/test_video.py b/xmodule/tests/test_video.py index 5e95f77082b1..9ae8fb7c5e88 100644 --- a/xmodule/tests/test_video.py +++ b/xmodule/tests/test_video.py @@ -741,6 +741,48 @@ def test_export_to_xml(self, mock_val_api): course_id=self.block.scope_ids.usage_id.context_key, ) + def test_export_to_xml_without_video_id(self): + """ + Test that we write the correct XML without video_id on export. + """ + self.block.youtube_id_0_75 = 'izygArpw-Qo' + self.block.youtube_id_1_0 = 'p2Q6BrNhdh8' + self.block.youtube_id_1_25 = '1EeWXzPdhSA' + self.block.youtube_id_1_5 = 'rABDYkeK0x8' + self.block.show_captions = False + self.block.start_time = datetime.timedelta(seconds=1.0) + self.block.end_time = datetime.timedelta(seconds=60) + self.block.track = 'http://www.example.com/track' + self.block.handout = 'http://www.example.com/handout' + self.block.download_track = True + self.block.html5_sources = ['http://www.example.com/source.mp4', 'http://www.example.com/source1.ogg'] + self.block.download_video = True + self.block.transcripts = {'ua': 'ukrainian_translation.srt', 'ge': 'german_translation.srt'} + + xml = self.block.definition_to_xml(self.file_system) + parser = etree.XMLParser(remove_blank_text=True) + xml_string = '''\ + + ''' + expected = etree.XML(xml_string, parser=parser) + self.assertXmlEqual(expected, xml) + @patch('xmodule.video_block.video_block.edxval_api') def test_export_to_xml_val_error(self, mock_val_api): # Export should succeed without VAL data if video does not exist diff --git a/xmodule/video_block/transcripts_utils.py b/xmodule/video_block/transcripts_utils.py index 866edf596812..849134860a1c 100644 --- a/xmodule/video_block/transcripts_utils.py +++ b/xmodule/video_block/transcripts_utils.py @@ -508,6 +508,8 @@ def manage_video_subtitles_save(item, user, old_metadata=None, generate_translat ) except TranscriptException: pass + except AttributeError: + pass if reraised_message: item.save_with_metadata(user) raise TranscriptException(reraised_message) diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index 84d7edcf7263..30af789506ed 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -855,11 +855,15 @@ def definition_to_xml(self, resource_fs): # lint-amnesty, pylint: disable=too-m if new_transcripts.get('en'): xml.set('sub', '') - # Update `transcripts` attribute in the xml - xml.set('transcripts', json.dumps(transcripts, sort_keys=True)) - except edxval_api.ValVideoNotFoundError: pass + else: + if transcripts.get('en'): + xml.set('sub', '') + + if transcripts: + # Update `transcripts` attribute in the xml + xml.set('transcripts', json.dumps(transcripts, sort_keys=True)) # Sorting transcripts for easy testing of resulting xml for transcript_language in sorted(transcripts.keys()): From 3885dd1aadc65277f610bde7202643106e744c75 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 25 Dec 2024 16:22:39 -0500 Subject: [PATCH 02/39] style: Fix lint --- xmodule/video_block/transcripts_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xmodule/video_block/transcripts_utils.py b/xmodule/video_block/transcripts_utils.py index 849134860a1c..bb97e331ef45 100644 --- a/xmodule/video_block/transcripts_utils.py +++ b/xmodule/video_block/transcripts_utils.py @@ -509,7 +509,7 @@ def manage_video_subtitles_save(item, user, old_metadata=None, generate_translat except TranscriptException: pass except AttributeError: - pass + pass if reraised_message: item.save_with_metadata(user) raise TranscriptException(reraised_message) From a723e9672af63f85f455ed335ff7c386d418eff6 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 15 Jan 2025 14:50:52 -0500 Subject: [PATCH 03/39] feat: Updated code to get english transcript from filename --- xmodule/video_block/transcripts_utils.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/xmodule/video_block/transcripts_utils.py b/xmodule/video_block/transcripts_utils.py index bb97e331ef45..75086d84ba79 100644 --- a/xmodule/video_block/transcripts_utils.py +++ b/xmodule/video_block/transcripts_utils.py @@ -1021,6 +1021,26 @@ def get_transcript_from_contentstore(video, language, output_format, transcripts except (KeyError, NotFoundError): continue + if transcript_content is None and language == 'en': + # `get_transcript_for_video`` can get the transcript using just the filename, + # but in the above loop the filename from 'en' is overwritten. + # + # If it doesn't yet have the transcription and the language is 'en', + # check again but this time using the original filename. + # + # The use case for which this has been implemented is when copying a video from + # a library and pasting it into a course. + # The asset is copied, but we only have the filename to obtain the content. + try: + input_format, base_name, transcript_content = get_transcript_for_video( + video.location, + subs_id=sub_id, + file_name=other_languages['en'], + language=language + ) + except (KeyError, NotFoundError): + pass + if transcript_content is None: raise NotFoundError('No transcript for `{lang}` language'.format( lang=language From 1c2a57547e5801f2dfb0d7ec6ad36d303a4587a2 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 15 Jan 2025 15:31:16 -0500 Subject: [PATCH 04/39] style: Fix lint --- xmodule/video_block/transcripts_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xmodule/video_block/transcripts_utils.py b/xmodule/video_block/transcripts_utils.py index 75086d84ba79..9fd8231b2af7 100644 --- a/xmodule/video_block/transcripts_utils.py +++ b/xmodule/video_block/transcripts_utils.py @@ -1034,7 +1034,7 @@ def get_transcript_from_contentstore(video, language, output_format, transcripts try: input_format, base_name, transcript_content = get_transcript_for_video( video.location, - subs_id=sub_id, + subs_id=None, file_name=other_languages['en'], language=language ) From 74f027f2cf82197a4f0df735720577c69fe83ba1 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 17 Jan 2025 18:51:35 -0500 Subject: [PATCH 05/39] feat: Upload transcript file as static asset in Learning Core in library components --- .../core/djangoapps/xblock/rest_api/views.py | 2 +- xmodule/video_block/video_handlers.py | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/openedx/core/djangoapps/xblock/rest_api/views.py b/openedx/core/djangoapps/xblock/rest_api/views.py index edcbf22e0d3d..05a16adb1a6f 100644 --- a/openedx/core/djangoapps/xblock/rest_api/views.py +++ b/openedx/core/djangoapps/xblock/rest_api/views.py @@ -175,7 +175,7 @@ def xblock_handler( """ # To support sandboxed XBlocks, custom frontends, and other use cases, we # authenticate requests using a secure token in the URL. see - # openedx.core.djangoapps.xblock.utils.get_secure_hash_for_xblock_handler + # openedx.core.djangoapps.xblock.utils.get_secure_token_for_xblock_handler # for details and rationale. if not validate_secure_token_for_xblock_handler(user_id, str(usage_key), secure_token): raise PermissionDenied("Invalid/expired auth token.") diff --git a/xmodule/video_block/video_handlers.py b/xmodule/video_block/video_handlers.py index b7857e881ece..076b073a8566 100644 --- a/xmodule/video_block/video_handlers.py +++ b/xmodule/video_block/video_handlers.py @@ -13,13 +13,15 @@ from django.core.files.base import ContentFile from django.utils.timezone import now from edxval.api import create_external_video, create_or_update_video_transcript, delete_video_transcript -from opaque_keys.edx.locator import CourseLocator +from opaque_keys.edx.locator import CourseLocator, LibraryLocatorV2 +from opaque_keys.edx.keys import UsageKeyV2 from webob import Response from xblock.core import XBlock from xblock.exceptions import JsonHandlerError from xmodule.exceptions import NotFoundError from xmodule.fields import RelativeTime +from openedx.core.djangoapps.content_libraries import api as lib_api from .transcripts_utils import ( Transcript, @@ -517,8 +519,9 @@ def studio_transcript(self, request, dispatch): try: # Convert SRT transcript into an SJSON format # and upload it to S3. + content = transcript_file.read() sjson_subs = Transcript.convert( - content=transcript_file.read().decode('utf-8'), + content=content.decode('utf-8'), input_format=Transcript.SRT, output_format=Transcript.SJSON ).encode() @@ -541,6 +544,17 @@ def studio_transcript(self, request, dispatch): self.transcripts.pop(language_code, None) self.transcripts[new_language_code] = f'{edx_video_id}-{new_language_code}.srt' response = Response(json.dumps(payload), status=201) + + if isinstance(self.scope_ids.usage_id, UsageKeyV2): + usage_key = self.scope_ids.usage_id + if isinstance(usage_key.context_key, LibraryLocatorV2): + # Save transcript as static asset in Learning Core if is a library component + filename = f"static/{self.transcripts[new_language_code]}" + lib_api.add_library_block_static_asset_file( + usage_key, + filename, + content, + ) except (TranscriptsGenerationException, UnicodeDecodeError): response = Response( json={ From 2adacee2f97eeb4a2581ed42c4299bde5c760dca Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 20 Jan 2025 14:30:18 -0500 Subject: [PATCH 06/39] style: Fix lint --- xmodule/video_block/video_handlers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/xmodule/video_block/video_handlers.py b/xmodule/video_block/video_handlers.py index 076b073a8566..b8f5f4bcc8ca 100644 --- a/xmodule/video_block/video_handlers.py +++ b/xmodule/video_block/video_handlers.py @@ -499,6 +499,7 @@ def studio_transcript(self, request, dispatch): """ _ = self.runtime.service(self, "i18n").ugettext + # pylint: disable=too-many-nested-blocks if dispatch.startswith('translation'): if request.method == 'POST': From 0393829aad0a4a65dc153eeecde0d9bc50192c41 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 22 Jan 2025 12:42:21 -0500 Subject: [PATCH 07/39] feat: Updates to support download transcripts from youtube in vlibrary videos --- .../contentstore/views/transcripts_ajax.py | 192 ++++++++++++------ xmodule/video_block/video_handlers.py | 19 +- 2 files changed, 137 insertions(+), 74 deletions(-) diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index 8cb7f455013b..fd3439dd78bb 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -19,7 +19,8 @@ from django.utils.translation import gettext as _ from edxval.api import create_external_video, create_or_update_video_transcript from opaque_keys import InvalidKeyError -from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.keys import UsageKey, UsageKeyV2 +from opaque_keys.edx.locator import LibraryLocatorV2 from cms.djangoapps.contentstore.video_storage_handlers import TranscriptProvider from common.djangoapps.student.auth import has_course_author_access @@ -43,6 +44,8 @@ get_transcript_link_from_youtube, get_transcript_links_from_youtube, ) +from openedx.core.djangoapps.content_libraries import api as lib_api, permissions +from openedx.core.djangoapps.xblock import api as xblock_api __all__ = [ 'upload_transcripts', @@ -81,13 +84,17 @@ def link_video_to_component(video_component, user): edx_video_id = clean_video_id(video_component.edx_video_id) if not edx_video_id: edx_video_id = create_external_video(display_name='external video') + + if isinstance(video_component.usage_key, UsageKeyV2): + return edx_video_id + video_component.edx_video_id = edx_video_id video_component.save_with_metadata(user) return edx_video_id -def save_video_transcript(edx_video_id, input_format, transcript_content, language_code): +def save_video_transcript(usage_key, edx_video_id, input_format, transcript_content, language_code): """ Saves a video transcript to the VAL and its content to the configured django storage(DS). @@ -118,6 +125,21 @@ def save_video_transcript(edx_video_id, input_format, transcript_content, langua }, file_data=ContentFile(sjson_subs), ) + if isinstance(usage_key.context_key, LibraryLocatorV2): + # Save transcript as static asset in Learning Core if is a library component + srt_content = Transcript.convert( + content=sjson_subs, + input_format=Transcript.SJSON, + output_format=Transcript.SRT + ).encode() + + filename = f"static/{edx_video_id}-{language_code}.srt" + lib_api.add_library_block_static_asset_file( + usage_key, + filename, + srt_content, + ) + result = True except (TranscriptsGenerationException, UnicodeDecodeError): result = False @@ -142,9 +164,10 @@ def validate_video_block(request, locator): """ error, item = None, None try: - item = _get_item(request, {'locator': locator}) + item, _ = _get_item(request, {'locator': locator}) if item.category != 'video': - error = _('Transcripts are supported only for "video" blocks.') + raise TranscriptsRequestValidationException(_('Transcripts are supported only for "video" blocks.')) + except (InvalidKeyError, ItemNotFoundError): error = _('Cannot find item by locator.') @@ -308,7 +331,7 @@ def check_transcripts(request): # lint-amnesty, pylint: disable=too-many-statem } try: - __, videos, item = _validate_transcripts_data(request) + __, videos, item, isLibraryContent = _validate_transcripts_data(request) except TranscriptsRequestValidationException as e: return error_response(transcripts_presence, str(e)) @@ -319,61 +342,39 @@ def check_transcripts(request): # lint-amnesty, pylint: disable=too-many-statem get_transcript_from_val(edx_video_id=edx_video_id, lang='en') command = 'found' except NotFoundError: - filename = f'subs_{item.sub}.srt.sjson' - content_location = StaticContent.compute_location(item.location.course_key, filename) - try: - local_transcripts = contentstore().find(content_location).data.decode('utf-8') - transcripts_presence['current_item_subs'] = item.sub - except NotFoundError: - pass - # Check for youtube transcripts presence youtube_id = videos.get('youtube', None) if youtube_id: - transcripts_presence['is_youtube_mode'] = True + _check_youtube_transcripts( + transcripts_presence, + youtube_id, + item, + isLibraryContent, + ) - # youtube local - filename = f'subs_{youtube_id}.srt.sjson' + if not isLibraryContent: + filename = f'subs_{item.sub}.srt.sjson' content_location = StaticContent.compute_location(item.location.course_key, filename) try: - local_transcripts = contentstore().find(content_location).data.decode('utf-8') - transcripts_presence['youtube_local'] = True + contentstore().find(content_location).data.decode('utf-8') + transcripts_presence['current_item_subs'] = item.sub except NotFoundError: - log.debug("Can't find transcripts in storage for youtube id: %s", youtube_id) + pass - if get_transcript_link_from_youtube(youtube_id): - transcripts_presence['youtube_server'] = True - #check youtube local and server transcripts for equality - if transcripts_presence['youtube_server'] and transcripts_presence['youtube_local']: + # Check for html5 local transcripts presence + html5_subs = [] + for html5_id in videos['html5']: + filename = f'subs_{html5_id}.srt.sjson' + content_location = StaticContent.compute_location(item.location.course_key, filename) try: - transcript_links = get_transcript_links_from_youtube( - youtube_id, - settings, - item.runtime.service(item, "i18n") + html5_subs.append(contentstore().find(content_location).data) + transcripts_presence['html5_local'].append(html5_id) + except NotFoundError: + log.debug("Can't find transcripts in storage for non-youtube video_id: %s", html5_id) + if len(html5_subs) == 2: # check html5 transcripts for equality + transcripts_presence['html5_equal'] = ( + json.loads(html5_subs[0].decode('utf-8')) == json.loads(html5_subs[1].decode('utf-8')) ) - for (_, link) in transcript_links.items(): - youtube_server_subs = get_transcript_from_youtube( - link, youtube_id, item.runtime.service(item, "i18n") - ) - if json.loads(local_transcripts) == youtube_server_subs: # check transcripts for equality - transcripts_presence['youtube_diff'] = False - except GetTranscriptsFromYouTubeException: - pass - - # Check for html5 local transcripts presence - html5_subs = [] - for html5_id in videos['html5']: - filename = f'subs_{html5_id}.srt.sjson' - content_location = StaticContent.compute_location(item.location.course_key, filename) - try: - html5_subs.append(contentstore().find(content_location).data) - transcripts_presence['html5_local'].append(html5_id) - except NotFoundError: - log.debug("Can't find transcripts in storage for non-youtube video_id: %s", html5_id) - if len(html5_subs) == 2: # check html5 transcripts for equality - transcripts_presence['html5_equal'] = ( - json.loads(html5_subs[0].decode('utf-8')) == json.loads(html5_subs[1].decode('utf-8')) - ) command, __ = _transcripts_logic(transcripts_presence, videos) @@ -381,6 +382,43 @@ def check_transcripts(request): # lint-amnesty, pylint: disable=too-many-statem return JsonResponse(transcripts_presence) +def _check_youtube_transcripts(transcripts_presence, youtube_id, item, isLibraryContent): + """ + Check for youtube transcripts presence + """ + transcripts_presence['is_youtube_mode'] = True + + if get_transcript_link_from_youtube(youtube_id): + transcripts_presence['youtube_server'] = True + + if not isLibraryContent: + # youtube local + filename = f'subs_{youtube_id}.srt.sjson' + content_location = StaticContent.compute_location(item.location.course_key, filename) + try: + local_transcripts = contentstore().find(content_location).data.decode('utf-8') + transcripts_presence['youtube_local'] = True + except NotFoundError: + log.debug("Can't find transcripts in storage for youtube id: %s", youtube_id) + + #check youtube local and server transcripts for equality + if transcripts_presence['youtube_server'] and transcripts_presence['youtube_local']: + try: + transcript_links = get_transcript_links_from_youtube( + youtube_id, + settings, + item.runtime.service(item, "i18n") + ) + for (_, link) in transcript_links.items(): + youtube_server_subs = get_transcript_from_youtube( + link, youtube_id, item.runtime.service(item, "i18n") + ) + if json.loads(local_transcripts) == youtube_server_subs: # check transcripts for equality + transcripts_presence['youtube_diff'] = False + except GetTranscriptsFromYouTubeException: + pass + + def _transcripts_logic(transcripts_presence, videos): """ By `transcripts_presence` content, figure what show to user: @@ -447,7 +485,8 @@ def _validate_transcripts_data(request): data: dict, loaded json from request, videos: parsed `data` to useful format, - item: video item from storage + item: video item from storage or library + isLibraryContent: `True` if the item is a library content Raises `TranscriptsRequestValidationException` if validation is unsuccessful or `PermissionDenied` if user has no access. @@ -456,7 +495,7 @@ def _validate_transcripts_data(request): if not data: raise TranscriptsRequestValidationException(_('Incoming video data is empty.')) try: - item = _get_item(request, data) + item, isLibraryContent = _get_item(request, data) except (InvalidKeyError, ItemNotFoundError): raise TranscriptsRequestValidationException(_("Can't find item by locator.")) # lint-amnesty, pylint: disable=raise-missing-from @@ -475,7 +514,7 @@ def _validate_transcripts_data(request): if videos['html5'].get('video') != video_data['video']: videos['html5'][video_data['video']] = video_data['mode'] - return data, videos, item + return data, videos, item, isLibraryContent def validate_transcripts_request(request, include_yt=False, include_html5=False): @@ -549,7 +588,13 @@ def choose_transcripts(request): edx_video_id = link_video_to_component(video, request.user) # 3. Upload the retrieved transcript to DS for the linked video ID. - success = save_video_transcript(edx_video_id, input_format, transcript_content, language_code='en') + success = save_video_transcript( + video.usage_key, + edx_video_id, + input_format, + transcript_content, + language_code='en', + ) if success: response = JsonResponse({'edx_video_id': edx_video_id, 'status': 'Success'}, status=200) else: @@ -588,7 +633,13 @@ def rename_transcripts(request): edx_video_id = link_video_to_component(video, request.user) # 3. Upload the retrieved transcript to DS for the linked video ID. - success = save_video_transcript(edx_video_id, input_format, transcript_content, language_code='en') + success = save_video_transcript( + video.usage_key, + edx_video_id, + input_format, + transcript_content, + language_code='en', + ) if success: response = JsonResponse({'edx_video_id': edx_video_id, 'status': 'Success'}, status=200) else: @@ -631,7 +682,13 @@ def replace_transcripts(request): success = True for transcript in transcript_content: [language_code, json_content] = transcript - success = save_video_transcript(edx_video_id, Transcript.SJSON, json_content, language_code) + success = save_video_transcript( + video.usage_key, + edx_video_id, + Transcript.SJSON, + json_content, + language_code, + ) if success: response = JsonResponse({'edx_video_id': edx_video_id, 'status': 'Success'}, status=200) else: @@ -643,21 +700,30 @@ def replace_transcripts(request): def _get_item(request, data): """ Obtains from 'data' the locator for an item. - Next, gets that item from the modulestore (allowing any errors to raise up). + Next, gets that item from the modulestore (allowing any errors to raise up) + or from library API if is a library content. Finally, verifies that the user has access to the item. - Returns the item. + Returns the item and a boolean if is a library content. """ usage_key = UsageKey.from_string(data.get('locator')) - if not usage_key.context_key.is_course: - # TODO: implement transcript support for learning core / content libraries. - raise TranscriptsRequestValidationException(_('Transcripts are not yet supported in content libraries.')) + + context_key = usage_key.context_key + if not context_key.is_course: + if isinstance(context_key, LibraryLocatorV2): + lib_api.require_permission_for_library_key( + context_key, + request.user, + permissions.CAN_EDIT_THIS_CONTENT_LIBRARY + ) + return xblock_api.load_block(usage_key, request.user), True + raise TranscriptsRequestValidationException(_('Transcripts are not yet supported for this type of block')) # This is placed before has_course_author_access() to validate the location, - # because has_course_author_access() raises r if location is invalid. + # because has_course_author_access() raises error if location is invalid. item = modulestore().get_item(usage_key) # use the item's course_key, because the usage_key might not have the run if not has_course_author_access(request.user, item.location.course_key): raise PermissionDenied() - return item + return item, False diff --git a/xmodule/video_block/video_handlers.py b/xmodule/video_block/video_handlers.py index b8f5f4bcc8ca..0da2791edf9f 100644 --- a/xmodule/video_block/video_handlers.py +++ b/xmodule/video_block/video_handlers.py @@ -14,7 +14,6 @@ from django.utils.timezone import now from edxval.api import create_external_video, create_or_update_video_transcript, delete_video_transcript from opaque_keys.edx.locator import CourseLocator, LibraryLocatorV2 -from opaque_keys.edx.keys import UsageKeyV2 from webob import Response from xblock.core import XBlock from xblock.exceptions import JsonHandlerError @@ -546,16 +545,14 @@ def studio_transcript(self, request, dispatch): self.transcripts[new_language_code] = f'{edx_video_id}-{new_language_code}.srt' response = Response(json.dumps(payload), status=201) - if isinstance(self.scope_ids.usage_id, UsageKeyV2): - usage_key = self.scope_ids.usage_id - if isinstance(usage_key.context_key, LibraryLocatorV2): - # Save transcript as static asset in Learning Core if is a library component - filename = f"static/{self.transcripts[new_language_code]}" - lib_api.add_library_block_static_asset_file( - usage_key, - filename, - content, - ) + if isinstance(self.scope_ids.usage_id.context_key, LibraryLocatorV2): + # Save transcript as static asset in Learning Core if is a library component + filename = f"static/{self.transcripts[new_language_code]}" + lib_api.add_library_block_static_asset_file( + self.scope_ids.usage_id, + filename, + content, + ) except (TranscriptsGenerationException, UnicodeDecodeError): response = Response( json={ From e27221e54ba96387eba0b7fe794eefb22cf6b922 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 22 Jan 2025 12:58:58 -0500 Subject: [PATCH 08/39] style: Fix lint --- cms/djangoapps/contentstore/views/transcripts_ajax.py | 4 ++-- openedx/core/djangoapps/content_libraries/api.py | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index fd3439dd78bb..ec15f6b26e05 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -44,7 +44,7 @@ get_transcript_link_from_youtube, get_transcript_links_from_youtube, ) -from openedx.core.djangoapps.content_libraries import api as lib_api, permissions +from openedx.core.djangoapps.content_libraries import api as lib_api from openedx.core.djangoapps.xblock import api as xblock_api __all__ = [ @@ -714,7 +714,7 @@ def _get_item(request, data): lib_api.require_permission_for_library_key( context_key, request.user, - permissions.CAN_EDIT_THIS_CONTENT_LIBRARY + lib_api.permissions.CAN_EDIT_THIS_CONTENT_LIBRARY ) return xblock_api.load_block(usage_key, request.user), True raise TranscriptsRequestValidationException(_('Transcripts are not yet supported for this type of block')) diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index c51c707fc470..4300bdea0569 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -1952,3 +1952,6 @@ def import_blocks_create_task(library_key, course_key, use_course_key_as_block_i log.info(f"Import block task created: import_task={import_task} " f"celery_task={result.id}") return import_task + +# To enable use content library permissions as public API +permissions = permissions From 05067f64bcc85064206107cc3245a571664a3732 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 22 Jan 2025 13:47:44 -0500 Subject: [PATCH 09/39] test: Add test for download youtube transcripts in library content --- .../views/tests/test_transcripts.py | 20 +++++++++++++++++++ .../contentstore/views/transcripts_ajax.py | 4 ++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_transcripts.py b/cms/djangoapps/contentstore/views/tests/test_transcripts.py index 95fbaccbab7c..4c55da7ff563 100644 --- a/cms/djangoapps/contentstore/views/tests/test_transcripts.py +++ b/cms/djangoapps/contentstore/views/tests/test_transcripts.py @@ -15,9 +15,11 @@ from django.urls import reverse from edxval.api import create_video from opaque_keys.edx.keys import UsageKey +from organizations.tests.factories import OrganizationFactory from cms.djangoapps.contentstore.tests.utils import CourseTestCase, setup_caption_responses from openedx.core.djangoapps.contentserver.caching import del_cached_content +from openedx.core.djangoapps.content_libraries import api as lib_api from xmodule.contentstore.content import StaticContent # lint-amnesty, pylint: disable=wrong-import-order from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.exceptions import NotFoundError # lint-amnesty, pylint: disable=wrong-import-order @@ -92,6 +94,17 @@ def setUp(self): resp = self.client.ajax_post('/xblock/', data) self.assertEqual(resp.status_code, 200) + self.library = lib_api.create_library( + org=OrganizationFactory.create(short_name="org1"), + slug="lib", + title="Library", + ) + self.library_block = lib_api.create_library_block( + self.library.key, + "video", + "video-transcript", + ) + self.video_usage_key = self._get_usage_key(resp) self.item = modulestore().get_item(self.video_usage_key) # hI10vDNYz4M - valid Youtube ID with transcripts. @@ -702,6 +715,13 @@ def test_replace_transcript_success(self, edx_video_id): expected_sjson_content = json.loads(SJSON_TRANSCRIPT_CONTENT) self.assertDictEqual(actual_sjson_content, expected_sjson_content) + def test_replace_transcript_library_content_success(self): + # Make call to replace transcripts from youtube + response = self.replace_transcript(self.library_block.usage_key, self.youtube_id) + + # Verify the response + self.assert_response(response, expected_status_code=200, expected_message='Success') + def test_replace_transcript_fails_without_data(self): """ Verify that replace transcript fails if we do not provide video data in request. diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index ec15f6b26e05..47702771b451 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -164,9 +164,9 @@ def validate_video_block(request, locator): """ error, item = None, None try: - item, _ = _get_item(request, {'locator': locator}) + item, _isLibraryContent = _get_item(request, {'locator': locator}) if item.category != 'video': - raise TranscriptsRequestValidationException(_('Transcripts are supported only for "video" blocks.')) + error = _('Transcripts are supported only for "video" blocks.') except (InvalidKeyError, ItemNotFoundError): error = _('Cannot find item by locator.') From 2f53a75adc68d1c4f26da5da62b3c034d7310a05 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 22 Jan 2025 14:54:48 -0500 Subject: [PATCH 10/39] style: Fix lint --- cms/djangoapps/contentstore/views/transcripts_ajax.py | 2 +- openedx/core/djangoapps/content_libraries/api.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index 47702771b451..7add5abfb2b9 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -714,7 +714,7 @@ def _get_item(request, data): lib_api.require_permission_for_library_key( context_key, request.user, - lib_api.permissions.CAN_EDIT_THIS_CONTENT_LIBRARY + lib_api.lib_permissions.CAN_EDIT_THIS_CONTENT_LIBRARY ) return xblock_api.load_block(usage_key, request.user), True raise TranscriptsRequestValidationException(_('Transcripts are not yet supported for this type of block')) diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 4300bdea0569..2f6d43277fc1 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -1954,4 +1954,4 @@ def import_blocks_create_task(library_key, course_key, use_course_key_as_block_i return import_task # To enable use content library permissions as public API -permissions = permissions +lib_permissions = permissions From 5c6819a1c4258170806b19e163de42793484215f Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 22 Jan 2025 18:06:43 -0500 Subject: [PATCH 11/39] feat: Support copy transcripts from a library --- cms/djangoapps/contentstore/helpers.py | 5 ++-- xmodule/video_block/transcripts_utils.py | 29 +++++++++++++++++------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index e40eddb6c99e..8e2745444bf1 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -23,6 +23,7 @@ from xmodule.exceptions import NotFoundError from xmodule.modulestore.django import modulestore from xmodule.xml_block import XmlMixin +from xmodule.video_block.transcripts_utils import build_components_import_path from cms.djangoapps.models.settings.course_grading import CourseGradingModel from cms.lib.xblock.upstream_sync import UpstreamLink, UpstreamLinkException, fetch_customizable_fields @@ -583,8 +584,8 @@ def _import_file_into_course( # we're not going to attempt to change. if clipboard_file_path.startswith('static/'): # If it's in this form, it came from a library and assumes component-local assets - file_path = clipboard_file_path.lstrip('static/') - import_path = f"components/{usage_key.block_type}/{usage_key.block_id}/{file_path}" + file_path = clipboard_file_path.removeprefix('static/') + import_path = build_components_import_path(usage_key, file_path) filename = pathlib.Path(file_path).name new_key = course_key.make_asset_key("asset", import_path.replace("/", "_")) else: diff --git a/xmodule/video_block/transcripts_utils.py b/xmodule/video_block/transcripts_utils.py index 9fd8231b2af7..81e6130c1f13 100644 --- a/xmodule/video_block/transcripts_utils.py +++ b/xmodule/video_block/transcripts_utils.py @@ -1021,12 +1021,8 @@ def get_transcript_from_contentstore(video, language, output_format, transcripts except (KeyError, NotFoundError): continue - if transcript_content is None and language == 'en': - # `get_transcript_for_video`` can get the transcript using just the filename, - # but in the above loop the filename from 'en' is overwritten. - # - # If it doesn't yet have the transcription and the language is 'en', - # check again but this time using the original filename. + if transcript_content is None: + # `get_transcript_for_video` can get the transcript using just the filename. # # The use case for which this has been implemented is when copying a video from # a library and pasting it into a course. @@ -1035,11 +1031,22 @@ def get_transcript_from_contentstore(video, language, output_format, transcripts input_format, base_name, transcript_content = get_transcript_for_video( video.location, subs_id=None, - file_name=other_languages['en'], + file_name=other_languages[language], language=language ) except (KeyError, NotFoundError): - pass + # If the video is copied from a library, the component import path is used, + # so we also need to try this use case. + try: + file_name = build_components_import_path(video.location, other_languages[language]) + input_format, base_name, transcript_content = get_transcript_for_video( + video.location, + subs_id=None, + file_name=file_name, + language=language + ) + except (KeyError, NotFoundError): + pass if transcript_content is None: raise NotFoundError('No transcript for `{lang}` language'.format( @@ -1061,6 +1068,12 @@ def get_transcript_from_contentstore(video, language, output_format, transcripts return transcript_content, transcript_name, Transcript.mime_types[output_format] +def build_components_import_path(usage_key, file_path): + """ + Build components import path + """ + return f"components/{usage_key.block_type}/{usage_key.block_id}/{file_path}" + def get_transcript_from_learning_core(video_block, language, output_format, transcripts_info): """ From 95b51e844158d62c450b543e7872a7822eca69ec Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 23 Jan 2025 17:28:56 -0500 Subject: [PATCH 12/39] refactor: Allow use edx_video_id (edxval) in new runtime This is used to be retroactive in copy-paste videos from Library to Course and Course to Library --- xmodule/video_block/video_block.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index 30af789506ed..039433f6e954 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -732,8 +732,6 @@ def parse_xml_new_runtime(cls, node, runtime, keys): if key not in cls.fields: # lint-amnesty, pylint: disable=unsupported-membership-test continue # parse_video_xml returns some old non-fields like 'source' setattr(video_block, key, cls.fields[key].from_json(val)) # lint-amnesty, pylint: disable=unsubscriptable-object - # Don't use VAL in the new runtime: - video_block.edx_video_id = None return video_block @classmethod From c42e223a1a852d9ce7a0a4e61cdc8befca590eb7 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 24 Jan 2025 17:32:01 -0500 Subject: [PATCH 13/39] refactor: Adds new edx_video_id when copy to course --- cms/djangoapps/contentstore/helpers.py | 35 ++++++++++++++++++++++++-- xmodule/video_block/video_block.py | 2 ++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index 8e2745444bf1..9067fad08ac3 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -10,6 +10,7 @@ import re from attrs import frozen, Factory +from django.core.files.base import ContentFile from django.conf import settings from django.contrib.auth import get_user_model from django.utils.translation import gettext as _ @@ -23,7 +24,8 @@ from xmodule.exceptions import NotFoundError from xmodule.modulestore.django import modulestore from xmodule.xml_block import XmlMixin -from xmodule.video_block.transcripts_utils import build_components_import_path +from xmodule.video_block.transcripts_utils import Transcript, build_components_import_path +from edxval.api import create_external_video, create_or_update_video_transcript from cms.djangoapps.models.settings.course_grading import CourseGradingModel from cms.lib.xblock.upstream_sync import UpstreamLink, UpstreamLinkException, fetch_customizable_fields @@ -300,13 +302,21 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> tags=user_clipboard.content.tags, ) + usage_key = new_xblock.scope_ids.usage_id + if usage_key.block_type == 'video': + # The edx_video_id must always be new so as not + # to interfere with the data of the copied block + new_xblock.edx_video_id = create_external_video(display_name='external video') + store.update_item(new_xblock, request.user.id) + # Now handle static files that need to go into Files & Uploads. static_files = content_staging_api.get_staged_content_static_files(user_clipboard.content.id) notices, substitutions = _import_files_into_course( + block=new_xblock, course_key=parent_key.context_key, staged_content_id=user_clipboard.content.id, static_files=static_files, - usage_key=new_xblock.scope_ids.usage_id, + usage_key=usage_key, ) # Rewrite the OLX's static asset references to point to the new @@ -505,6 +515,7 @@ def _import_xml_node_to_parent( def _import_files_into_course( + block: XBlock, course_key: CourseKey, staged_content_id: int, static_files: list[content_staging_api.StagedContentFileData], @@ -541,6 +552,7 @@ def _import_files_into_course( # At this point, we know this is a "Files & Uploads" asset that we may need to copy into the course: try: result, substitution_for_file = _import_file_into_course( + block, course_key, staged_content_id, file_data_obj, @@ -567,6 +579,7 @@ def _import_files_into_course( def _import_file_into_course( + block: XBlock, course_key: CourseKey, staged_content_id: int, file_data_obj: content_staging_api.StagedContentFileData, @@ -617,6 +630,24 @@ def _import_file_into_course( if thumbnail_content is not None: content.thumbnail_location = thumbnail_location contentstore().save(content) + if usage_key.block_type == 'video': + # Adding transcripts to VAL using the nex edx_video_id + language_code = next((k for k, v in block.transcripts.items() if v == filename), None) + if language_code: + sjson_subs = Transcript.convert( + content=data, + input_format=Transcript.SRT, + output_format=Transcript.SJSON + ).encode() + create_or_update_video_transcript( + video_id=block.edx_video_id, + language_code=language_code, + metadata={ + 'file_format': Transcript.SJSON, + 'language_code': language_code + }, + file_data=ContentFile(sjson_subs), + ) return True, {clipboard_file_path: f"static/{import_path}"} elif current_file.content_digest == file_data_obj.md5_hash: # The file already exists and matches exactly, so no action is needed diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index 039433f6e954..30af789506ed 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -732,6 +732,8 @@ def parse_xml_new_runtime(cls, node, runtime, keys): if key not in cls.fields: # lint-amnesty, pylint: disable=unsupported-membership-test continue # parse_video_xml returns some old non-fields like 'source' setattr(video_block, key, cls.fields[key].from_json(val)) # lint-amnesty, pylint: disable=unsubscriptable-object + # Don't use VAL in the new runtime: + video_block.edx_video_id = None return video_block @classmethod From 4bf3af34157f7951209d76a0470b5d36f42aa9a5 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 24 Jan 2025 17:37:05 -0500 Subject: [PATCH 14/39] refactor: Remove unnevessary code --- xmodule/video_block/transcripts_utils.py | 27 ------------------------ 1 file changed, 27 deletions(-) diff --git a/xmodule/video_block/transcripts_utils.py b/xmodule/video_block/transcripts_utils.py index 81e6130c1f13..fbfa14826c8d 100644 --- a/xmodule/video_block/transcripts_utils.py +++ b/xmodule/video_block/transcripts_utils.py @@ -1021,33 +1021,6 @@ def get_transcript_from_contentstore(video, language, output_format, transcripts except (KeyError, NotFoundError): continue - if transcript_content is None: - # `get_transcript_for_video` can get the transcript using just the filename. - # - # The use case for which this has been implemented is when copying a video from - # a library and pasting it into a course. - # The asset is copied, but we only have the filename to obtain the content. - try: - input_format, base_name, transcript_content = get_transcript_for_video( - video.location, - subs_id=None, - file_name=other_languages[language], - language=language - ) - except (KeyError, NotFoundError): - # If the video is copied from a library, the component import path is used, - # so we also need to try this use case. - try: - file_name = build_components_import_path(video.location, other_languages[language]) - input_format, base_name, transcript_content = get_transcript_for_video( - video.location, - subs_id=None, - file_name=file_name, - language=language - ) - except (KeyError, NotFoundError): - pass - if transcript_content is None: raise NotFoundError('No transcript for `{lang}` language'.format( lang=language From 1de1d4ee62d1b127fdc2cdd7b71b5ca690d013c9 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 24 Jan 2025 19:00:00 -0500 Subject: [PATCH 15/39] feat: Support delete transcripts in library --- xmodule/video_block/video_handlers.py | 41 +++++++++++++++++---------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/xmodule/video_block/video_handlers.py b/xmodule/video_block/video_handlers.py index 0da2791edf9f..dfd4ae1ff155 100644 --- a/xmodule/video_block/video_handlers.py +++ b/xmodule/video_block/video_handlers.py @@ -574,22 +574,33 @@ def studio_transcript(self, request, dispatch): if edx_video_id: delete_video_transcript(video_id=edx_video_id, language_code=language) - if language == 'en': - # remove any transcript file from content store for the video ids - possible_sub_ids = [ - self.sub, # pylint: disable=access-member-before-definition - self.youtube_id_1_0 - ] + get_html5_ids(self.html5_sources) - for sub_id in possible_sub_ids: - remove_subs_from_store(sub_id, self, language) - - # update metadata as `en` can also be present in `transcripts` field - remove_subs_from_store(self.transcripts.pop(language, None), self, language) - - # also empty `sub` field - self.sub = '' # pylint: disable=attribute-defined-outside-init + if isinstance(self.scope_ids.usage_id.context_key, LibraryLocatorV2): + transcript_file_path = f"static/{self.transcripts.pop(language, None)}" + lib_api.delete_library_block_static_asset_file(self.scope_ids.usage_id, transcript_file_path) + field = self.fields['transcripts'] + if self.transcripts: + transcripts_copy = self.transcripts.copy() + field.delete_from(self) + field.write_to(self, transcripts_copy) + else: + field.delete_from(self) else: - remove_subs_from_store(self.transcripts.pop(language, None), self, language) + if language == 'en': + # remove any transcript file from content store for the video ids + possible_sub_ids = [ + self.sub, # pylint: disable=access-member-before-definition + self.youtube_id_1_0 + ] + get_html5_ids(self.html5_sources) + for sub_id in possible_sub_ids: + remove_subs_from_store(sub_id, self, language) + + # update metadata as `en` can also be present in `transcripts` field + remove_subs_from_store(self.transcripts.pop(language, None), self, language) + + # also empty `sub` field + self.sub = '' # pylint: disable=attribute-defined-outside-init + else: + remove_subs_from_store(self.transcripts.pop(language, None), self, language) return Response(status=200) From a59898decb1dde6a228b519bd568a9931bd6052e Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 27 Jan 2025 10:28:28 -0500 Subject: [PATCH 16/39] style: Fix lint --- cms/djangoapps/contentstore/helpers.py | 2 +- xmodule/video_block/transcripts_utils.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index 9067fad08ac3..a7f2e2beadf4 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -306,7 +306,7 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> if usage_key.block_type == 'video': # The edx_video_id must always be new so as not # to interfere with the data of the copied block - new_xblock.edx_video_id = create_external_video(display_name='external video') + new_xblock.edx_video_id = create_external_video(display_name='external video') store.update_item(new_xblock, request.user.id) # Now handle static files that need to go into Files & Uploads. diff --git a/xmodule/video_block/transcripts_utils.py b/xmodule/video_block/transcripts_utils.py index fbfa14826c8d..aa5bb8cb17fc 100644 --- a/xmodule/video_block/transcripts_utils.py +++ b/xmodule/video_block/transcripts_utils.py @@ -1041,6 +1041,7 @@ def get_transcript_from_contentstore(video, language, output_format, transcripts return transcript_content, transcript_name, Transcript.mime_types[output_format] + def build_components_import_path(usage_key, file_path): """ Build components import path From 590a5cda91f566801234e9f7f96b5f00dc553633 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 29 Jan 2025 19:30:58 -0500 Subject: [PATCH 17/39] style: Nits on the code --- cms/djangoapps/contentstore/helpers.py | 2 +- cms/djangoapps/contentstore/views/transcripts_ajax.py | 2 +- openedx/core/djangoapps/content_libraries/api.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index a7f2e2beadf4..571fbae39e51 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -631,7 +631,7 @@ def _import_file_into_course( content.thumbnail_location = thumbnail_location contentstore().save(content) if usage_key.block_type == 'video': - # Adding transcripts to VAL using the nex edx_video_id + # Adding transcripts to VAL using the new edx_video_id language_code = next((k for k, v in block.transcripts.items() if v == filename), None) if language_code: sjson_subs = Transcript.convert( diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index 7add5abfb2b9..9ee85a28b632 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -401,7 +401,7 @@ def _check_youtube_transcripts(transcripts_presence, youtube_id, item, isLibrary except NotFoundError: log.debug("Can't find transcripts in storage for youtube id: %s", youtube_id) - #check youtube local and server transcripts for equality + # check youtube local and server transcripts for equality if transcripts_presence['youtube_server'] and transcripts_presence['youtube_local']: try: transcript_links = get_transcript_links_from_youtube( diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 2f6d43277fc1..1e0c2578ea31 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -1953,5 +1953,5 @@ def import_blocks_create_task(library_key, course_key, use_course_key_as_block_i f"celery_task={result.id}") return import_task -# To enable use content library permissions as public API +# Allow content library permissions to be used in the public API lib_permissions = permissions From 0e5445055c64fc152eed35980c72365a21bd6e3b Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 29 Jan 2025 20:32:31 -0500 Subject: [PATCH 18/39] refactor: Update replace_transcript to verify transcript --- .../views/tests/test_transcripts.py | 24 ++++++++++++++++--- .../contentstore/views/transcripts_ajax.py | 4 ++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_transcripts.py b/cms/djangoapps/contentstore/views/tests/test_transcripts.py index 4c55da7ff563..61c0bd81b9c8 100644 --- a/cms/djangoapps/contentstore/views/tests/test_transcripts.py +++ b/cms/djangoapps/contentstore/views/tests/test_transcripts.py @@ -29,8 +29,10 @@ GetTranscriptsFromYouTubeException, Transcript, get_video_transcript_content, - remove_subs_from_store + get_transcript, + remove_subs_from_store, ) +from openedx.core.djangoapps.xblock import api as xblock_api TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE) TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'] = 'test_xcontent_%s' % uuid4().hex @@ -99,11 +101,15 @@ def setUp(self): slug="lib", title="Library", ) - self.library_block = lib_api.create_library_block( + self.library_block_metadata = lib_api.create_library_block( self.library.key, "video", "video-transcript", ) + self.library_block = xblock_api.load_block( + self.library_block_metadata.usage_key, + self.user, + ) self.video_usage_key = self._get_usage_key(resp) self.item = modulestore().get_item(self.video_usage_key) @@ -717,11 +723,23 @@ def test_replace_transcript_success(self, edx_video_id): def test_replace_transcript_library_content_success(self): # Make call to replace transcripts from youtube - response = self.replace_transcript(self.library_block.usage_key, self.youtube_id) + response = self.replace_transcript(self.library_block_metadata.usage_key, self.youtube_id) # Verify the response self.assert_response(response, expected_status_code=200, expected_message='Success') + # Obtain updated block + updated_block = xblock_api.load_block( + self.library_block_metadata.usage_key, + self.user, + ) + + # Verify transcript content + transcript = get_transcript(updated_block, 'en', Transcript.SJSON) + actual_sjson_content = json.loads(transcript[0]) + expected_sjson_content = json.loads(SJSON_TRANSCRIPT_CONTENT) + self.assertDictEqual(actual_sjson_content, expected_sjson_content) + def test_replace_transcript_fails_without_data(self): """ Verify that replace transcript fails if we do not provide video data in request. diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index 9ee85a28b632..eeef324ba028 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -689,7 +689,11 @@ def replace_transcripts(request): json_content, language_code, ) + if not success: + break + video.transcripts[language_code] = f"{edx_video_id}-{language_code}.srt" if success: + video.save() response = JsonResponse({'edx_video_id': edx_video_id, 'status': 'Success'}, status=200) else: response = error_response({}, _('There is a problem with the YouTube transcript file.')) From 5685f16389de256e1a4f5cb7010a7783ff7a96fd Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 29 Jan 2025 20:45:27 -0500 Subject: [PATCH 19/39] refactor: Verify LibraryLocatorV2 in manage_video_subtitles_save to avoid raise AttributeError --- xmodule/video_block/transcripts_utils.py | 25 ++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/xmodule/video_block/transcripts_utils.py b/xmodule/video_block/transcripts_utils.py index aa5bb8cb17fc..1d4aba80f289 100644 --- a/xmodule/video_block/transcripts_utils.py +++ b/xmodule/video_block/transcripts_utils.py @@ -20,12 +20,14 @@ from opaque_keys.edx.keys import UsageKeyV2 from pysrt import SubRipFile, SubRipItem, SubRipTime from pysrt.srtexc import Error +from opaque_keys.edx.locator import LibraryLocatorV2 from openedx.core.djangoapps.xblock.api import get_component_from_usage_key from xmodule.contentstore.content import StaticContent from xmodule.contentstore.django import contentstore from xmodule.exceptions import NotFoundError + from .bumper_utils import get_bumper_settings try: @@ -498,18 +500,17 @@ def manage_video_subtitles_save(item, user, old_metadata=None, generate_translat remove_subs_from_store(video_id, item, lang) reraised_message = '' - for lang in new_langs: # 3b - try: - generate_sjson_for_all_speeds( - item, - item.transcripts[lang], - {speed: subs_id for subs_id, speed in youtube_speed_dict(item).items()}, - lang, - ) - except TranscriptException: - pass - except AttributeError: - pass + if not isinstance(item.usage_key.context_key, LibraryLocatorV2): + for lang in new_langs: # 3b + try: + generate_sjson_for_all_speeds( + item, + item.transcripts[lang], + {speed: subs_id for subs_id, speed in youtube_speed_dict(item).items()}, + lang, + ) + except TranscriptException: + pass if reraised_message: item.save_with_metadata(user) raise TranscriptException(reraised_message) From e4f7c72496355e7eab29a5d0a7f9830808891c7b Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 29 Jan 2025 20:57:34 -0500 Subject: [PATCH 20/39] style: Add comment in transcripts_ajax.py --- cms/djangoapps/contentstore/views/transcripts_ajax.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index eeef324ba028..a1afff3018a9 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -86,6 +86,9 @@ def link_video_to_component(video_component, user): edx_video_id = create_external_video(display_name='external video') if isinstance(video_component.usage_key, UsageKeyV2): + # `edx_video_id` is not used in the new runtime, + # also saving it generates errors. + # See `parse_xml_new_runtime()` in `video_block.py` return edx_video_id video_component.edx_video_id = edx_video_id From b478ac52927193e65559535e5b9e603adfca9a54 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 29 Jan 2025 21:10:27 -0500 Subject: [PATCH 21/39] refactor: _get_item to avoid return isLibraryContent --- .../contentstore/views/transcripts_ajax.py | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index a1afff3018a9..5fa7d91380ef 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -167,7 +167,7 @@ def validate_video_block(request, locator): """ error, item = None, None try: - item, _isLibraryContent = _get_item(request, {'locator': locator}) + item = _get_item(request, {'locator': locator}) if item.category != 'video': error = _('Transcripts are supported only for "video" blocks.') @@ -334,7 +334,7 @@ def check_transcripts(request): # lint-amnesty, pylint: disable=too-many-statem } try: - __, videos, item, isLibraryContent = _validate_transcripts_data(request) + __, videos, item = _validate_transcripts_data(request) except TranscriptsRequestValidationException as e: return error_response(transcripts_presence, str(e)) @@ -352,10 +352,9 @@ def check_transcripts(request): # lint-amnesty, pylint: disable=too-many-statem transcripts_presence, youtube_id, item, - isLibraryContent, ) - if not isLibraryContent: + if not isinstance(item.usage_key, UsageKeyV2): filename = f'subs_{item.sub}.srt.sjson' content_location = StaticContent.compute_location(item.location.course_key, filename) try: @@ -385,7 +384,7 @@ def check_transcripts(request): # lint-amnesty, pylint: disable=too-many-statem return JsonResponse(transcripts_presence) -def _check_youtube_transcripts(transcripts_presence, youtube_id, item, isLibraryContent): +def _check_youtube_transcripts(transcripts_presence, youtube_id, item): """ Check for youtube transcripts presence """ @@ -394,7 +393,7 @@ def _check_youtube_transcripts(transcripts_presence, youtube_id, item, isLibrary if get_transcript_link_from_youtube(youtube_id): transcripts_presence['youtube_server'] = True - if not isLibraryContent: + if not isinstance(item.usage_key, UsageKeyV2): # youtube local filename = f'subs_{youtube_id}.srt.sjson' content_location = StaticContent.compute_location(item.location.course_key, filename) @@ -489,7 +488,6 @@ def _validate_transcripts_data(request): data: dict, loaded json from request, videos: parsed `data` to useful format, item: video item from storage or library - isLibraryContent: `True` if the item is a library content Raises `TranscriptsRequestValidationException` if validation is unsuccessful or `PermissionDenied` if user has no access. @@ -498,7 +496,7 @@ def _validate_transcripts_data(request): if not data: raise TranscriptsRequestValidationException(_('Incoming video data is empty.')) try: - item, isLibraryContent = _get_item(request, data) + item = _get_item(request, data) except (InvalidKeyError, ItemNotFoundError): raise TranscriptsRequestValidationException(_("Can't find item by locator.")) # lint-amnesty, pylint: disable=raise-missing-from @@ -517,7 +515,7 @@ def _validate_transcripts_data(request): if videos['html5'].get('video') != video_data['video']: videos['html5'][video_data['video']] = video_data['mode'] - return data, videos, item, isLibraryContent + return data, videos, item def validate_transcripts_request(request, include_yt=False, include_html5=False): @@ -723,7 +721,7 @@ def _get_item(request, data): request.user, lib_api.lib_permissions.CAN_EDIT_THIS_CONTENT_LIBRARY ) - return xblock_api.load_block(usage_key, request.user), True + return xblock_api.load_block(usage_key, request.user) raise TranscriptsRequestValidationException(_('Transcripts are not yet supported for this type of block')) # This is placed before has_course_author_access() to validate the location, # because has_course_author_access() raises error if location is invalid. @@ -733,4 +731,4 @@ def _get_item(request, data): if not has_course_author_access(request.user, item.location.course_key): raise PermissionDenied() - return item, False + return item From 173a0afa08248c78b8a5e226352ecc1903e5b4e0 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 29 Jan 2025 21:24:49 -0500 Subject: [PATCH 22/39] refactor: studio_transcript to separate methods in separated functions --- xmodule/video_block/video_handlers.py | 258 ++++++++++++++------------ 1 file changed, 137 insertions(+), 121 deletions(-) diff --git a/xmodule/video_block/video_handlers.py b/xmodule/video_block/video_handlers.py index dfd4ae1ff155..0911f2f08256 100644 --- a/xmodule/video_block/video_handlers.py +++ b/xmodule/video_block/video_handlers.py @@ -468,7 +468,6 @@ def validate_transcript_upload_data(self, data): return error - # pylint: disable=too-many-statements @XBlock.handler def studio_transcript(self, request, dispatch): """ @@ -496,140 +495,157 @@ def studio_transcript(self, request, dispatch): no SRT extension or not parse-able by PySRT UnicodeDecodeError: non-UTF8 uploaded file content encoding. """ - _ = self.runtime.service(self, "i18n").ugettext - - # pylint: disable=too-many-nested-blocks if dispatch.startswith('translation'): if request.method == 'POST': - error = self.validate_transcript_upload_data(data=request.POST) - if error: - response = Response(json={'error': error}, status=400) - else: - edx_video_id = clean_video_id(request.POST['edx_video_id']) - language_code = request.POST['language_code'] - new_language_code = request.POST['new_language_code'] - transcript_file = request.POST['file'].file - - if not edx_video_id: - # Back-populate the video ID for an external video. - # pylint: disable=attribute-defined-outside-init - self.edx_video_id = edx_video_id = create_external_video(display_name='external video') - - try: - # Convert SRT transcript into an SJSON format - # and upload it to S3. - content = transcript_file.read() - sjson_subs = Transcript.convert( - content=content.decode('utf-8'), - input_format=Transcript.SRT, - output_format=Transcript.SJSON - ).encode() - create_or_update_video_transcript( - video_id=edx_video_id, - language_code=language_code, - metadata={ - 'file_format': Transcript.SJSON, - 'language_code': new_language_code - }, - file_data=ContentFile(sjson_subs), - ) - payload = { - 'edx_video_id': edx_video_id, - 'language_code': new_language_code - } - # If a new transcript is added, then both new_language_code and - # language_code fields will have the same value. - if language_code != new_language_code: - self.transcripts.pop(language_code, None) - self.transcripts[new_language_code] = f'{edx_video_id}-{new_language_code}.srt' - response = Response(json.dumps(payload), status=201) - - if isinstance(self.scope_ids.usage_id.context_key, LibraryLocatorV2): - # Save transcript as static asset in Learning Core if is a library component - filename = f"static/{self.transcripts[new_language_code]}" - lib_api.add_library_block_static_asset_file( - self.scope_ids.usage_id, - filename, - content, - ) - except (TranscriptsGenerationException, UnicodeDecodeError): - response = Response( - json={ - 'error': _( - 'There is a problem with this transcript file. Try to upload a different file.' - ) - }, - status=400 - ) + response = self._studio_transcript_upload(request) elif request.method == 'DELETE': - request_data = request.json + response = self._studio_transcript_delete(request) + elif request.method == 'GET': + response = self._studio_transcript_get(request) + else: + # Any other HTTP method is not allowed. + response = Response(status=404) - if 'lang' not in request_data or 'edx_video_id' not in request_data: - return Response(status=400) + else: # unknown dispatch + log.debug("Dispatch is not allowed") + response = Response(status=404) - language = request_data['lang'] - edx_video_id = clean_video_id(request_data['edx_video_id']) + return response - if edx_video_id: - delete_video_transcript(video_id=edx_video_id, language_code=language) + def _studio_transcript_upload(self, request): + """ + Upload transcript. Usedn in "POST" method in `studio_transcript` + """ + _ = self.runtime.service(self, "i18n").ugettext + error = self.validate_transcript_upload_data(data=request.POST) + if error: + response = Response(json={'error': error}, status=400) + else: + edx_video_id = clean_video_id(request.POST['edx_video_id']) + language_code = request.POST['language_code'] + new_language_code = request.POST['new_language_code'] + transcript_file = request.POST['file'].file + + if not edx_video_id: + # Back-populate the video ID for an external video. + # pylint: disable=attribute-defined-outside-init + self.edx_video_id = edx_video_id = create_external_video(display_name='external video') + + try: + # Convert SRT transcript into an SJSON format + # and upload it to S3. + content = transcript_file.read() + sjson_subs = Transcript.convert( + content=content.decode('utf-8'), + input_format=Transcript.SRT, + output_format=Transcript.SJSON + ).encode() + create_or_update_video_transcript( + video_id=edx_video_id, + language_code=language_code, + metadata={ + 'file_format': Transcript.SJSON, + 'language_code': new_language_code + }, + file_data=ContentFile(sjson_subs), + ) + payload = { + 'edx_video_id': edx_video_id, + 'language_code': new_language_code + } + # If a new transcript is added, then both new_language_code and + # language_code fields will have the same value. + if language_code != new_language_code: + self.transcripts.pop(language_code, None) + self.transcripts[new_language_code] = f'{edx_video_id}-{new_language_code}.srt' + response = Response(json.dumps(payload), status=201) if isinstance(self.scope_ids.usage_id.context_key, LibraryLocatorV2): - transcript_file_path = f"static/{self.transcripts.pop(language, None)}" - lib_api.delete_library_block_static_asset_file(self.scope_ids.usage_id, transcript_file_path) - field = self.fields['transcripts'] - if self.transcripts: - transcripts_copy = self.transcripts.copy() - field.delete_from(self) - field.write_to(self, transcripts_copy) - else: - field.delete_from(self) - else: - if language == 'en': - # remove any transcript file from content store for the video ids - possible_sub_ids = [ - self.sub, # pylint: disable=access-member-before-definition - self.youtube_id_1_0 - ] + get_html5_ids(self.html5_sources) - for sub_id in possible_sub_ids: - remove_subs_from_store(sub_id, self, language) - - # update metadata as `en` can also be present in `transcripts` field - remove_subs_from_store(self.transcripts.pop(language, None), self, language) - - # also empty `sub` field - self.sub = '' # pylint: disable=attribute-defined-outside-init - else: - remove_subs_from_store(self.transcripts.pop(language, None), self, language) + # Save transcript as static asset in Learning Core if is a library component + filename = f"static/{self.transcripts[new_language_code]}" + lib_api.add_library_block_static_asset_file( + self.scope_ids.usage_id, + filename, + content, + ) + except (TranscriptsGenerationException, UnicodeDecodeError): + response = Response( + json={ + 'error': _( + 'There is a problem with this transcript file. Try to upload a different file.' + ) + }, + status=400 + ) + return response - return Response(status=200) + def _studio_transcript_delete(self, request): + """ + Delete transcript. Usedn in "DELETE" method in `studio_transcript` + """ + request_data = request.json - elif request.method == 'GET': - language = request.GET.get('language_code') - if not language: - return Response(json={'error': _('Language is required.')}, status=400) + if 'lang' not in request_data or 'edx_video_id' not in request_data: + return Response(status=400) - try: - transcript_content, transcript_name, mime_type = get_transcript( - video=self, lang=language, output_format=Transcript.SRT - ) - response = Response(transcript_content, headerlist=[ - ( - 'Content-Disposition', - f'attachment; filename="{transcript_name}"' - ), - ('Content-Language', language), - ('Content-Type', mime_type) - ]) - except (UnicodeDecodeError, TranscriptsGenerationException, NotFoundError): - response = Response(status=404) + language = request_data['lang'] + edx_video_id = clean_video_id(request_data['edx_video_id']) + + if edx_video_id: + delete_video_transcript(video_id=edx_video_id, language_code=language) + if isinstance(self.scope_ids.usage_id.context_key, LibraryLocatorV2): + transcript_file_path = f"static/{self.transcripts.pop(language, None)}" + lib_api.delete_library_block_static_asset_file(self.scope_ids.usage_id, transcript_file_path) + field = self.fields['transcripts'] + if self.transcripts: + transcripts_copy = self.transcripts.copy() + field.delete_from(self) + field.write_to(self, transcripts_copy) else: - # Any other HTTP method is not allowed. - response = Response(status=404) + field.delete_from(self) + else: + if language == 'en': + # remove any transcript file from content store for the video ids + possible_sub_ids = [ + self.sub, # pylint: disable=access-member-before-definition + self.youtube_id_1_0 + ] + get_html5_ids(self.html5_sources) + for sub_id in possible_sub_ids: + remove_subs_from_store(sub_id, self, language) + + # update metadata as `en` can also be present in `transcripts` field + remove_subs_from_store(self.transcripts.pop(language, None), self, language) + + # also empty `sub` field + self.sub = '' # pylint: disable=attribute-defined-outside-init + else: + remove_subs_from_store(self.transcripts.pop(language, None), self, language) - else: # unknown dispatch - log.debug("Dispatch is not allowed") - response = Response(status=404) + return Response(status=200) + + def _studio_transcript_get(self, request): + """ + Get transcript. Usedn in "GET" method in `studio_transcript` + """ + _ = self.runtime.service(self, "i18n").ugettext + language = request.GET.get('language_code') + if not language: + return Response(json={'error': _('Language is required.')}, status=400) + try: + transcript_content, transcript_name, mime_type = get_transcript( + video=self, lang=language, output_format=Transcript.SRT + ) + response = Response(transcript_content, headerlist=[ + ( + 'Content-Disposition', + f'attachment; filename="{transcript_name}"' + ), + ('Content-Language', language), + ('Content-Type', mime_type) + ]) + except (UnicodeDecodeError, TranscriptsGenerationException, NotFoundError): + response = Response(status=404) return response From 4532a2ae41af434503964bcde762c8e196936de5 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 29 Jan 2025 21:30:35 -0500 Subject: [PATCH 23/39] refactor: Update code in video_handlers to avoid "static/None" --- xmodule/video_block/video_handlers.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/xmodule/video_block/video_handlers.py b/xmodule/video_block/video_handlers.py index 0911f2f08256..6bf545c5e6a3 100644 --- a/xmodule/video_block/video_handlers.py +++ b/xmodule/video_block/video_handlers.py @@ -596,15 +596,19 @@ def _studio_transcript_delete(self, request): delete_video_transcript(video_id=edx_video_id, language_code=language) if isinstance(self.scope_ids.usage_id.context_key, LibraryLocatorV2): - transcript_file_path = f"static/{self.transcripts.pop(language, None)}" - lib_api.delete_library_block_static_asset_file(self.scope_ids.usage_id, transcript_file_path) - field = self.fields['transcripts'] - if self.transcripts: - transcripts_copy = self.transcripts.copy() - field.delete_from(self) - field.write_to(self, transcripts_copy) - else: - field.delete_from(self) + transcript_name = self.transcripts.pop(language, None) + if transcript_name: + lib_api.delete_library_block_static_asset_file( + self.scope_ids.usage_id, + f"static/{transcript_name}", + ) + field = self.fields['transcripts'] + if self.transcripts: + transcripts_copy = self.transcripts.copy() + field.delete_from(self) + field.write_to(self, transcripts_copy) + else: + field.delete_from(self) else: if language == 'en': # remove any transcript file from content store for the video ids From bb10d188ac9bc6d6856f6e227332f33719dc80a3 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 30 Jan 2025 12:28:47 -0500 Subject: [PATCH 24/39] style: Fix nit --- xmodule/video_block/transcripts_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/xmodule/video_block/transcripts_utils.py b/xmodule/video_block/transcripts_utils.py index 1d4aba80f289..4dda003b1f47 100644 --- a/xmodule/video_block/transcripts_utils.py +++ b/xmodule/video_block/transcripts_utils.py @@ -27,7 +27,6 @@ from xmodule.contentstore.django import contentstore from xmodule.exceptions import NotFoundError - from .bumper_utils import get_bumper_settings try: From a254fe4a5672e796baf46c3caa40d569a0203133 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 12 Feb 2025 18:17:42 -0500 Subject: [PATCH 25/39] refactor: Use usage_key in some places of the code --- xmodule/video_block/video_handlers.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/xmodule/video_block/video_handlers.py b/xmodule/video_block/video_handlers.py index 6bf545c5e6a3..d28808181d6e 100644 --- a/xmodule/video_block/video_handlers.py +++ b/xmodule/video_block/video_handlers.py @@ -561,11 +561,11 @@ def _studio_transcript_upload(self, request): self.transcripts[new_language_code] = f'{edx_video_id}-{new_language_code}.srt' response = Response(json.dumps(payload), status=201) - if isinstance(self.scope_ids.usage_id.context_key, LibraryLocatorV2): + if isinstance(self.usage_key.context_key, LibraryLocatorV2): # Save transcript as static asset in Learning Core if is a library component filename = f"static/{self.transcripts[new_language_code]}" lib_api.add_library_block_static_asset_file( - self.scope_ids.usage_id, + self.usage_key, filename, content, ) @@ -595,11 +595,11 @@ def _studio_transcript_delete(self, request): if edx_video_id: delete_video_transcript(video_id=edx_video_id, language_code=language) - if isinstance(self.scope_ids.usage_id.context_key, LibraryLocatorV2): + if isinstance(self.usage_key.context_key, LibraryLocatorV2): transcript_name = self.transcripts.pop(language, None) if transcript_name: lib_api.delete_library_block_static_asset_file( - self.scope_ids.usage_id, + self.usage_key, f"static/{transcript_name}", ) field = self.fields['transcripts'] From 34e5cbc493bf116a18d3e22ed139b3866e230161 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 12 Feb 2025 18:55:46 -0500 Subject: [PATCH 26/39] refactor: _import_transcripts created in helpers.py --- cms/djangoapps/contentstore/helpers.py | 74 ++++++++++++++++++-------- 1 file changed, 51 insertions(+), 23 deletions(-) diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index 031d704bf04f..e1d2048fbcd6 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -274,12 +274,17 @@ def _insert_static_files_into_downstream_xblock( """ static_files = content_staging_api.get_staged_content_static_files(staged_content_id) notices, substitutions = _import_files_into_course( - block=downstream_xblock, course_key=downstream_xblock.context_key, staged_content_id=staged_content_id, static_files=static_files, - usage_key=downstream_xblock.scope_ids.usage_id, + usage_key=downstream_xblock.usage_key, ) + if downstream_xblock.usage_key.block_type == 'video': + _import_transcripts( + downstream_xblock, + staged_content_id=staged_content_id, + static_files=static_files, + ) # Rewrite the OLX's static asset references to point to the new # locations for those assets. See _import_files_into_course for more @@ -559,7 +564,6 @@ def _import_xml_node_to_parent( def _import_files_into_course( - block: XBlock, course_key: CourseKey, staged_content_id: int, static_files: list[content_staging_api.StagedContentFileData], @@ -596,7 +600,6 @@ def _import_files_into_course( # At this point, we know this is a "Files & Uploads" asset that we may need to copy into the course: try: result, substitution_for_file = _import_file_into_course( - block, course_key, staged_content_id, file_data_obj, @@ -626,7 +629,6 @@ def _import_files_into_course( def _import_file_into_course( - block: XBlock, course_key: CourseKey, staged_content_id: int, file_data_obj: content_staging_api.StagedContentFileData, @@ -677,24 +679,6 @@ def _import_file_into_course( if thumbnail_content is not None: content.thumbnail_location = thumbnail_location contentstore().save(content) - if usage_key.block_type == 'video': - # Adding transcripts to VAL using the new edx_video_id - language_code = next((k for k, v in block.transcripts.items() if v == filename), None) - if language_code: - sjson_subs = Transcript.convert( - content=data, - input_format=Transcript.SRT, - output_format=Transcript.SJSON - ).encode() - create_or_update_video_transcript( - video_id=block.edx_video_id, - language_code=language_code, - metadata={ - 'file_format': Transcript.SJSON, - 'language_code': language_code - }, - file_data=ContentFile(sjson_subs), - ) return True, {clipboard_file_path: f"static/{import_path}"} elif current_file.content_digest == file_data_obj.md5_hash: # The file already exists and matches exactly, so no action is needed except substitutions @@ -704,6 +688,50 @@ def _import_file_into_course( return False, {} +def _import_transcripts( + block: XBlock, + staged_content_id: int, + static_files: list[content_staging_api.StagedContentFileData], +): + """ + Adds transcripts to VAL using the new edx_video_id. + """ + for file_data_obj in static_files: + clipboard_file_path = file_data_obj.filename + data = content_staging_api.get_staged_content_static_file_data( + staged_content_id, + clipboard_file_path + ) + if data is None: + raise NotFoundError(file_data_obj.source_key) + + if clipboard_file_path.startswith('static/'): + # If it's in this form, it came from a library and assumes component-local assets + file_path = clipboard_file_path.removeprefix('static/') + else: + # Otherwise it came from a course... + file_path = clipboard_file_path + + filename = pathlib.Path(file_path).name + + language_code = next((k for k, v in block.transcripts.items() if v == filename), None) + if language_code: + sjson_subs = Transcript.convert( + content=data, + input_format=Transcript.SRT, + output_format=Transcript.SJSON + ).encode() + create_or_update_video_transcript( + video_id=block.edx_video_id, + language_code=language_code, + metadata={ + 'file_format': Transcript.SJSON, + 'language_code': language_code + }, + file_data=ContentFile(sjson_subs), + ) + + def is_item_in_course_tree(item): """ Check that the item is in the course tree. From c73f82cb7276f40a1ec78a42c9427bea9f290e04 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 12 Feb 2025 19:36:31 -0500 Subject: [PATCH 27/39] refactor: save_video_transcript_in_learning_core created save_video_transcript_in_learning_core created in transcript_ajax to avoid create an edx_video_id for library videos --- .../contentstore/views/transcripts_ajax.py | 149 ++++++++++++------ 1 file changed, 99 insertions(+), 50 deletions(-) diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index 5fa7d91380ef..d728449d5077 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -10,6 +10,7 @@ import json import logging import os +from uuid import uuid4 from django.conf import settings from django.contrib.auth.decorators import login_required @@ -84,20 +85,55 @@ def link_video_to_component(video_component, user): edx_video_id = clean_video_id(video_component.edx_video_id) if not edx_video_id: edx_video_id = create_external_video(display_name='external video') - - if isinstance(video_component.usage_key, UsageKeyV2): - # `edx_video_id` is not used in the new runtime, - # also saving it generates errors. - # See `parse_xml_new_runtime()` in `video_block.py` - return edx_video_id - video_component.edx_video_id = edx_video_id video_component.save_with_metadata(user) return edx_video_id -def save_video_transcript(usage_key, edx_video_id, input_format, transcript_content, language_code): +def save_video_transcript_in_learning_core( + usage_key, + input_format, + transcript_content, + language_code +): + """ + Saves a video transcript to the learning core + + Arguments: + usage_key: UsageKey of the block + input_format: Input transcript format for content being passed. + transcript_content: Content of the transcript file + language_code: transcript language code + + Returns: + result: A boolean indicating whether the transcript was saved or not. + video_key: Key used in video filename + """ + video_key = None + try: + srt_content = Transcript.convert( + content=transcript_content, + input_format=input_format, + output_format=Transcript.SRT + ).encode() + + video_key = uuid4() + + filename = f"static/{video_key}-{language_code}.srt" + lib_api.add_library_block_static_asset_file( + usage_key, + filename, + srt_content, + ) + result = True + except (TranscriptsGenerationException, UnicodeDecodeError): + result = False + + return result, video_key + + +def save_video_transcript(edx_video_id, input_format, transcript_content, language_code): """ Saves a video transcript to the VAL and its content to the configured django storage(DS). @@ -128,20 +164,6 @@ def save_video_transcript(usage_key, edx_video_id, input_format, transcript_cont }, file_data=ContentFile(sjson_subs), ) - if isinstance(usage_key.context_key, LibraryLocatorV2): - # Save transcript as static asset in Learning Core if is a library component - srt_content = Transcript.convert( - content=sjson_subs, - input_format=Transcript.SJSON, - output_format=Transcript.SRT - ).encode() - - filename = f"static/{edx_video_id}-{language_code}.srt" - lib_api.add_library_block_static_asset_file( - usage_key, - filename, - srt_content, - ) result = True except (TranscriptsGenerationException, UnicodeDecodeError): @@ -586,18 +608,27 @@ def choose_transcripts(request): return error_response({}, _('No such transcript.')) # 2. Link a video to video component if its not already linked to one. - edx_video_id = link_video_to_component(video, request.user) + if not isinstance(video.usage_key.context_key, LibraryLocatorV2): + edx_video_id = link_video_to_component(video, request.user) + video_key = edx_video_id # 3. Upload the retrieved transcript to DS for the linked video ID. - success = save_video_transcript( - video.usage_key, - edx_video_id, - input_format, - transcript_content, - language_code='en', - ) + if isinstance(video.usage_key.context_key, LibraryLocatorV2): + success, video_key = save_video_transcript_in_learning_core( + video.usage_key, + input_format, + transcript_content, + language_code='en', + ) + else: + success = save_video_transcript( + edx_video_id, + input_format, + transcript_content, + language_code='en', + ) if success: - response = JsonResponse({'edx_video_id': edx_video_id, 'status': 'Success'}, status=200) + response = JsonResponse({'edx_video_id': video_key, 'status': 'Success'}, status=200) else: response = error_response({}, _('There is a problem with the chosen transcript file.')) @@ -631,18 +662,27 @@ def rename_transcripts(request): return error_response({}, _('No such transcript.')) # 2. Link a video to video component if its not already linked to one. - edx_video_id = link_video_to_component(video, request.user) + if not isinstance(video.usage_key.context_key, LibraryLocatorV2): + edx_video_id = link_video_to_component(video, request.user) + video_key = edx_video_id # 3. Upload the retrieved transcript to DS for the linked video ID. - success = save_video_transcript( - video.usage_key, - edx_video_id, - input_format, - transcript_content, - language_code='en', - ) + if isinstance(video.usage_key.context_key, LibraryLocatorV2): + success, video_key = save_video_transcript_in_learning_core( + video.usage_key, + input_format, + transcript_content, + language_code='en', + ) + else: + success = save_video_transcript( + edx_video_id, + input_format, + transcript_content, + language_code='en', + ) if success: - response = JsonResponse({'edx_video_id': edx_video_id, 'status': 'Success'}, status=200) + response = JsonResponse({'edx_video_id': video_key, 'status': 'Success'}, status=200) else: response = error_response( {}, _('There is a problem with the existing transcript file. Please upload a different file.') @@ -675,7 +715,9 @@ def replace_transcripts(request): return error_response({}, str(e)) # 2. Link a video to video component if its not already linked to one. - edx_video_id = link_video_to_component(video, request.user) + if not isinstance(video.usage_key.context_key, LibraryLocatorV2): + edx_video_id = link_video_to_component(video, request.user) + video_key = edx_video_id # for transcript in transcript_links: @@ -683,19 +725,26 @@ def replace_transcripts(request): success = True for transcript in transcript_content: [language_code, json_content] = transcript - success = save_video_transcript( - video.usage_key, - edx_video_id, - Transcript.SJSON, - json_content, - language_code, - ) + if isinstance(video.usage_key.context_key, LibraryLocatorV2): + success, video_key = save_video_transcript_in_learning_core( + video.usage_key, + Transcript.SJSON, + json_content, + language_code, + ) + else: + success = save_video_transcript( + edx_video_id, + Transcript.SJSON, + json_content, + language_code, + ) if not success: break - video.transcripts[language_code] = f"{edx_video_id}-{language_code}.srt" + video.transcripts[language_code] = f"{video_key}-{language_code}.srt" if success: video.save() - response = JsonResponse({'edx_video_id': edx_video_id, 'status': 'Success'}, status=200) + response = JsonResponse({'edx_video_id': video_key, 'status': 'Success'}, status=200) else: response = error_response({}, _('There is a problem with the YouTube transcript file.')) From ddb6953c39d5fe772523db6b533266db12f57ff7 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 13 Feb 2025 14:53:26 -0500 Subject: [PATCH 28/39] refactor: _studio_transcript_upload in video_handlers * _save_transcript_field created to save the `transcripts` Field * _studio_transcript_upload updated to avoid create edx_video_id for libraries * _studio_transcript_upload updated to avoid add transcripts to VAL for libraries --- xmodule/video_block/video_handlers.py | 73 ++++++++++++++++----------- 1 file changed, 44 insertions(+), 29 deletions(-) diff --git a/xmodule/video_block/video_handlers.py b/xmodule/video_block/video_handlers.py index d28808181d6e..97b08dfd4706 100644 --- a/xmodule/video_block/video_handlers.py +++ b/xmodule/video_block/video_handlers.py @@ -513,6 +513,20 @@ def studio_transcript(self, request, dispatch): return response + def _save_transcript_field(self): + """ + Save `transcripts` block field. + """ + field = self.fields['transcripts'] + if self.transcripts: + transcripts_copy = self.transcripts.copy() + # Need to delete to overwrite, it's weird behavior, + # but it only works like this. + field.delete_from(self) + field.write_to(self, transcripts_copy) + else: + field.delete_from(self) + def _studio_transcript_upload(self, request): """ Upload transcript. Usedn in "POST" method in `studio_transcript` @@ -527,29 +541,21 @@ def _studio_transcript_upload(self, request): new_language_code = request.POST['new_language_code'] transcript_file = request.POST['file'].file - if not edx_video_id: - # Back-populate the video ID for an external video. - # pylint: disable=attribute-defined-outside-init - self.edx_video_id = edx_video_id = create_external_video(display_name='external video') + isLibrary = isinstance(self.usage_key.context_key, LibraryLocatorV2) + + if isLibrary: + filename = f'transcript-{new_language_code}.srt' + else: + if not edx_video_id: + # Back-populate the video ID for an external video. + # pylint: disable=attribute-defined-outside-init + self.edx_video_id = edx_video_id = create_external_video(display_name='external video') + filename = f'{edx_video_id}-{new_language_code}.srt' try: # Convert SRT transcript into an SJSON format # and upload it to S3. content = transcript_file.read() - sjson_subs = Transcript.convert( - content=content.decode('utf-8'), - input_format=Transcript.SRT, - output_format=Transcript.SJSON - ).encode() - create_or_update_video_transcript( - video_id=edx_video_id, - language_code=language_code, - metadata={ - 'file_format': Transcript.SJSON, - 'language_code': new_language_code - }, - file_data=ContentFile(sjson_subs), - ) payload = { 'edx_video_id': edx_video_id, 'language_code': new_language_code @@ -558,10 +564,8 @@ def _studio_transcript_upload(self, request): # language_code fields will have the same value. if language_code != new_language_code: self.transcripts.pop(language_code, None) - self.transcripts[new_language_code] = f'{edx_video_id}-{new_language_code}.srt' - response = Response(json.dumps(payload), status=201) - - if isinstance(self.usage_key.context_key, LibraryLocatorV2): + self.transcripts[new_language_code] = filename + if isLibrary: # Save transcript as static asset in Learning Core if is a library component filename = f"static/{self.transcripts[new_language_code]}" lib_api.add_library_block_static_asset_file( @@ -569,6 +573,23 @@ def _studio_transcript_upload(self, request): filename, content, ) + self._save_transcript_field() + else: + sjson_subs = Transcript.convert( + content=content.decode('utf-8'), + input_format=Transcript.SRT, + output_format=Transcript.SJSON + ).encode() + create_or_update_video_transcript( + video_id=edx_video_id, + language_code=language_code, + metadata={ + 'file_format': Transcript.SJSON, + 'language_code': new_language_code + }, + file_data=ContentFile(sjson_subs), + ) + response = Response(json.dumps(payload), status=201) except (TranscriptsGenerationException, UnicodeDecodeError): response = Response( json={ @@ -602,13 +623,7 @@ def _studio_transcript_delete(self, request): self.usage_key, f"static/{transcript_name}", ) - field = self.fields['transcripts'] - if self.transcripts: - transcripts_copy = self.transcripts.copy() - field.delete_from(self) - field.write_to(self, transcripts_copy) - else: - field.delete_from(self) + self._save_transcript_field() else: if language == 'en': # remove any transcript file from content store for the video ids From 46aa8ca7afe16896f37858c9290ea117adae8cdc Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 13 Feb 2025 15:14:25 -0500 Subject: [PATCH 29/39] refactor: transcripts_ajax to avoid use edx_video_id in filenames in libraries --- .../contentstore/views/transcripts_ajax.py | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index d728449d5077..f1903ae89768 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -10,7 +10,6 @@ import json import logging import os -from uuid import uuid4 from django.conf import settings from django.contrib.auth.decorators import login_required @@ -110,7 +109,6 @@ def save_video_transcript_in_learning_core( result: A boolean indicating whether the transcript was saved or not. video_key: Key used in video filename """ - video_key = None try: srt_content = Transcript.convert( content=transcript_content, @@ -118,9 +116,7 @@ def save_video_transcript_in_learning_core( output_format=Transcript.SRT ).encode() - video_key = uuid4() - - filename = f"static/{video_key}-{language_code}.srt" + filename = f"static/transcript-{language_code}.srt" lib_api.add_library_block_static_asset_file( usage_key, filename, @@ -130,7 +126,7 @@ def save_video_transcript_in_learning_core( except (TranscriptsGenerationException, UnicodeDecodeError): result = False - return result, video_key + return result def save_video_transcript(edx_video_id, input_format, transcript_content, language_code): @@ -591,6 +587,7 @@ def choose_transcripts(request): Or error in case of validation failures. """ error, validated_data = validate_transcripts_request(request, include_html5=True) + edx_video_id = None if error: response = error_response({}, error) else: @@ -610,11 +607,10 @@ def choose_transcripts(request): # 2. Link a video to video component if its not already linked to one. if not isinstance(video.usage_key.context_key, LibraryLocatorV2): edx_video_id = link_video_to_component(video, request.user) - video_key = edx_video_id # 3. Upload the retrieved transcript to DS for the linked video ID. if isinstance(video.usage_key.context_key, LibraryLocatorV2): - success, video_key = save_video_transcript_in_learning_core( + success = save_video_transcript_in_learning_core( video.usage_key, input_format, transcript_content, @@ -628,7 +624,7 @@ def choose_transcripts(request): language_code='en', ) if success: - response = JsonResponse({'edx_video_id': video_key, 'status': 'Success'}, status=200) + response = JsonResponse({'edx_video_id': edx_video_id, 'status': 'Success'}, status=200) else: response = error_response({}, _('There is a problem with the chosen transcript file.')) @@ -646,6 +642,7 @@ def rename_transcripts(request): Or error in case of validation failures. """ error, validated_data = validate_transcripts_request(request) + edx_video_id = None if error: response = error_response({}, error) else: @@ -664,11 +661,10 @@ def rename_transcripts(request): # 2. Link a video to video component if its not already linked to one. if not isinstance(video.usage_key.context_key, LibraryLocatorV2): edx_video_id = link_video_to_component(video, request.user) - video_key = edx_video_id # 3. Upload the retrieved transcript to DS for the linked video ID. if isinstance(video.usage_key.context_key, LibraryLocatorV2): - success, video_key = save_video_transcript_in_learning_core( + success = save_video_transcript_in_learning_core( video.usage_key, input_format, transcript_content, @@ -682,7 +678,7 @@ def rename_transcripts(request): language_code='en', ) if success: - response = JsonResponse({'edx_video_id': video_key, 'status': 'Success'}, status=200) + response = JsonResponse({'edx_video_id': edx_video_id, 'status': 'Success'}, status=200) else: response = error_response( {}, _('There is a problem with the existing transcript file. Please upload a different file.') @@ -702,6 +698,7 @@ def replace_transcripts(request): """ error, validated_data = validate_transcripts_request(request, include_yt=True) youtube_id = validated_data['youtube'] + edx_video_id = None if error: response = error_response({}, error) elif not youtube_id: @@ -717,21 +714,19 @@ def replace_transcripts(request): # 2. Link a video to video component if its not already linked to one. if not isinstance(video.usage_key.context_key, LibraryLocatorV2): edx_video_id = link_video_to_component(video, request.user) - video_key = edx_video_id - - # for transcript in transcript_links: # 3. Upload YT transcript to DS for the linked video ID. success = True for transcript in transcript_content: [language_code, json_content] = transcript if isinstance(video.usage_key.context_key, LibraryLocatorV2): - success, video_key = save_video_transcript_in_learning_core( + success = save_video_transcript_in_learning_core( video.usage_key, Transcript.SJSON, json_content, language_code, ) + filename = f"transcript-{language_code}.srt" else: success = save_video_transcript( edx_video_id, @@ -739,12 +734,13 @@ def replace_transcripts(request): json_content, language_code, ) + filename = f"{edx_video_id}-{language_code}.srt" if not success: break - video.transcripts[language_code] = f"{video_key}-{language_code}.srt" + video.transcripts[language_code] = filename if success: video.save() - response = JsonResponse({'edx_video_id': video_key, 'status': 'Success'}, status=200) + response = JsonResponse({'edx_video_id': edx_video_id, 'status': 'Success'}, status=200) else: response = error_response({}, _('There is a problem with the YouTube transcript file.')) From 5bb426f505c1f0ba3b862f992a62dd2ac78091bd Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 14 Feb 2025 12:48:28 -0500 Subject: [PATCH 30/39] refactor: Update _get_item in transcripts_ajax to use check_permission of load_block --- cms/djangoapps/contentstore/views/transcripts_ajax.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index f1903ae89768..858a4956c0cf 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -46,6 +46,7 @@ ) from openedx.core.djangoapps.content_libraries import api as lib_api from openedx.core.djangoapps.xblock import api as xblock_api +from openedx.core.djangoapps.xblock.data import CheckPerm __all__ = [ 'upload_transcripts', @@ -761,12 +762,11 @@ def _get_item(request, data): context_key = usage_key.context_key if not context_key.is_course: if isinstance(context_key, LibraryLocatorV2): - lib_api.require_permission_for_library_key( - context_key, + return xblock_api.load_block( + usage_key, request.user, - lib_api.lib_permissions.CAN_EDIT_THIS_CONTENT_LIBRARY + check_permission=CheckPerm.CAN_EDIT, ) - return xblock_api.load_block(usage_key, request.user) raise TranscriptsRequestValidationException(_('Transcripts are not yet supported for this type of block')) # This is placed before has_course_author_access() to validate the location, # because has_course_author_access() raises error if location is invalid. From 5dda777a2fce34cf9c4869024a5da210b5a7e3ed Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 14 Feb 2025 12:52:30 -0500 Subject: [PATCH 31/39] refactor: Delete lib_permissions from content_libraries/api.py --- openedx/core/djangoapps/content_libraries/api.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/api.py b/openedx/core/djangoapps/content_libraries/api.py index 1e0c2578ea31..c51c707fc470 100644 --- a/openedx/core/djangoapps/content_libraries/api.py +++ b/openedx/core/djangoapps/content_libraries/api.py @@ -1952,6 +1952,3 @@ def import_blocks_create_task(library_key, course_key, use_course_key_as_block_i log.info(f"Import block task created: import_task={import_task} " f"celery_task={result.id}") return import_task - -# Allow content library permissions to be used in the public API -lib_permissions = permissions From 53f42fa8c72333217d99f1c9917eaeed2ca8af54 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 14 Feb 2025 12:55:51 -0500 Subject: [PATCH 32/39] style: Fix typos in comments in video_handlers.py --- xmodule/video_block/video_handlers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/xmodule/video_block/video_handlers.py b/xmodule/video_block/video_handlers.py index 97b08dfd4706..ecaa9908bbce 100644 --- a/xmodule/video_block/video_handlers.py +++ b/xmodule/video_block/video_handlers.py @@ -529,7 +529,7 @@ def _save_transcript_field(self): def _studio_transcript_upload(self, request): """ - Upload transcript. Usedn in "POST" method in `studio_transcript` + Upload transcript. Used in "POST" method in `studio_transcript` """ _ = self.runtime.service(self, "i18n").ugettext error = self.validate_transcript_upload_data(data=request.POST) @@ -603,7 +603,7 @@ def _studio_transcript_upload(self, request): def _studio_transcript_delete(self, request): """ - Delete transcript. Usedn in "DELETE" method in `studio_transcript` + Delete transcript. Used in "DELETE" method in `studio_transcript` """ request_data = request.json @@ -646,7 +646,7 @@ def _studio_transcript_delete(self, request): def _studio_transcript_get(self, request): """ - Get transcript. Usedn in "GET" method in `studio_transcript` + Get transcript. Used in "GET" method in `studio_transcript` """ _ = self.runtime.service(self, "i18n").ugettext language = request.GET.get('language_code') From 9c7e4e82d361987b978963ab0d823040ed263a6f Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 14 Feb 2025 13:41:48 -0500 Subject: [PATCH 33/39] style: Update comment of test_export_to_xml_without_video_id --- xmodule/tests/test_video.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xmodule/tests/test_video.py b/xmodule/tests/test_video.py index 9ae8fb7c5e88..e98c78244db5 100644 --- a/xmodule/tests/test_video.py +++ b/xmodule/tests/test_video.py @@ -743,7 +743,7 @@ def test_export_to_xml(self, mock_val_api): def test_export_to_xml_without_video_id(self): """ - Test that we write the correct XML without video_id on export. + Test that we write the correct XML on export of a video without edx_video_id. """ self.block.youtube_id_0_75 = 'izygArpw-Qo' self.block.youtube_id_1_0 = 'p2Q6BrNhdh8' From 8c6a3ddf27fed81155f3d7cc9e7802959d9f5cd2 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 14 Feb 2025 15:10:21 -0500 Subject: [PATCH 34/39] fix: Fix tests and lint --- .../contentstore/views/transcripts_ajax.py | 2 +- xmodule/video_block/video_handlers.py | 17 ++++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index 858a4956c0cf..f98457a008f6 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -105,7 +105,7 @@ def save_video_transcript_in_learning_core( input_format: Input transcript format for content being passed. transcript_content: Content of the transcript file language_code: transcript language code - + Returns: result: A boolean indicating whether the transcript was saved or not. video_key: Key used in video filename diff --git a/xmodule/video_block/video_handlers.py b/xmodule/video_block/video_handlers.py index ecaa9908bbce..6d136b8b6869 100644 --- a/xmodule/video_block/video_handlers.py +++ b/xmodule/video_block/video_handlers.py @@ -560,20 +560,14 @@ def _studio_transcript_upload(self, request): 'edx_video_id': edx_video_id, 'language_code': new_language_code } - # If a new transcript is added, then both new_language_code and - # language_code fields will have the same value. - if language_code != new_language_code: - self.transcripts.pop(language_code, None) - self.transcripts[new_language_code] = filename if isLibrary: # Save transcript as static asset in Learning Core if is a library component - filename = f"static/{self.transcripts[new_language_code]}" + filename = f"static/{filename}" lib_api.add_library_block_static_asset_file( self.usage_key, filename, content, ) - self._save_transcript_field() else: sjson_subs = Transcript.convert( content=content.decode('utf-8'), @@ -589,6 +583,15 @@ def _studio_transcript_upload(self, request): }, file_data=ContentFile(sjson_subs), ) + + # If a new transcript is added, then both new_language_code and + # language_code fields will have the same value. + if language_code != new_language_code: + self.transcripts.pop(language_code, None) + self.transcripts[new_language_code] = filename + + if isLibrary: + self._save_transcript_field() response = Response(json.dumps(payload), status=201) except (TranscriptsGenerationException, UnicodeDecodeError): response = Response( From 0745f0917a5e76c3578777e9cb9416885d47aea2 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 14 Feb 2025 18:01:02 -0500 Subject: [PATCH 35/39] fix: Sync transcripts in video blocks from library to course --- cms/djangoapps/contentstore/helpers.py | 18 +++++++++++++++++- .../rest_api/v2/views/downstreams.py | 7 ++++++- cms/lib/xblock/upstream_sync.py | 4 ++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index e1d2048fbcd6..32626f263dec 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -25,7 +25,11 @@ from xmodule.modulestore.django import modulestore from xmodule.xml_block import XmlMixin from xmodule.video_block.transcripts_utils import Transcript, build_components_import_path -from edxval.api import create_external_video, create_or_update_video_transcript +from edxval.api import ( + create_external_video, + create_or_update_video_transcript, + delete_video_transcript, +) from cms.djangoapps.models.settings.course_grading import CourseGradingModel from cms.lib.xblock.upstream_sync import UpstreamLink, UpstreamLinkException, fetch_customizable_fields @@ -732,6 +736,18 @@ def _import_transcripts( ) +def clear_transcripts(block: XBlock): + """ + Deletes all transcripts of a video block + """ + for language_code in block.transcripts.keys(): + delete_video_transcript( + video_id=block.edx_video_id, + language_code=language_code, + ) + block.transcripts = {} + + def is_item_in_course_tree(item): """ Check that the item is in the course tree. diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py index 46e67e87ea0c..739f33d014f1 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py @@ -73,7 +73,9 @@ UpstreamLink, UpstreamLinkException, NoUpstream, BadUpstream, BadDownstream, fetch_customizable_fields, sync_from_upstream, decline_sync, sever_upstream_link ) -from cms.djangoapps.contentstore.helpers import import_static_assets_for_library_sync +from cms.djangoapps.contentstore.helpers import ( + import_static_assets_for_library_sync, clear_transcripts +) from common.djangoapps.student.auth import has_studio_write_access, has_studio_read_access from openedx.core.lib.api.view_utils import ( DeveloperErrorViewMixin, @@ -198,6 +200,9 @@ def post(self, request: _AuthenticatedRequest, usage_key_string: str) -> Respons """ downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=True) try: + if downstream.usage_key.block_type == "video": + # Delete all transcripts so we can copy new ones from upstream + clear_transcripts(downstream) upstream = sync_from_upstream(downstream, request.user) static_file_notices = import_static_assets_for_library_sync(downstream, upstream, request) except UpstreamLinkException as exc: diff --git a/cms/lib/xblock/upstream_sync.py b/cms/lib/xblock/upstream_sync.py index 8a12ea8fc045..22cad3c6d36a 100644 --- a/cms/lib/xblock/upstream_sync.py +++ b/cms/lib/xblock/upstream_sync.py @@ -297,7 +297,11 @@ def _update_non_customizable_fields(*, upstream: XBlock, downstream: XBlock) -> """ syncable_fields = _get_synchronizable_fields(upstream, downstream) customizable_fields = set(downstream.get_customizable_fields().keys()) + isVideoBlock = downstream.usage_key.block_type == "video" for field_name in syncable_fields - customizable_fields: + if isVideoBlock and field_name == 'edx_video_id': + # Avoid overwriting edx_video_id between blocks + continue new_upstream_value = getattr(upstream, field_name) setattr(downstream, field_name, new_upstream_value) From eff9d0408771675643b8f8f0c33419a0a48e8789 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 17 Feb 2025 17:00:42 -0500 Subject: [PATCH 36/39] test: Tests for sync video block --- cms/djangoapps/contentstore/helpers.py | 13 --------- .../rest_api/v2/views/downstreams.py | 5 ++-- .../v2/views/tests/test_downstreams.py | 4 ++- cms/lib/xblock/test/test_upstream_sync.py | 29 +++++++++++++++++++ xmodule/video_block/transcripts_utils.py | 12 ++++++++ 5 files changed, 46 insertions(+), 17 deletions(-) diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index 32626f263dec..59bd6b56277d 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -28,7 +28,6 @@ from edxval.api import ( create_external_video, create_or_update_video_transcript, - delete_video_transcript, ) from cms.djangoapps.models.settings.course_grading import CourseGradingModel @@ -736,18 +735,6 @@ def _import_transcripts( ) -def clear_transcripts(block: XBlock): - """ - Deletes all transcripts of a video block - """ - for language_code in block.transcripts.keys(): - delete_video_transcript( - video_id=block.edx_video_id, - language_code=language_code, - ) - block.transcripts = {} - - def is_item_in_course_tree(item): """ Check that the item is in the course tree. diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py index 739f33d014f1..38d313d1eae1 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py @@ -73,14 +73,13 @@ UpstreamLink, UpstreamLinkException, NoUpstream, BadUpstream, BadDownstream, fetch_customizable_fields, sync_from_upstream, decline_sync, sever_upstream_link ) -from cms.djangoapps.contentstore.helpers import ( - import_static_assets_for_library_sync, clear_transcripts -) +from cms.djangoapps.contentstore.helpers import import_static_assets_for_library_sync from common.djangoapps.student.auth import has_studio_write_access, has_studio_read_access from openedx.core.lib.api.view_utils import ( DeveloperErrorViewMixin, view_auth_classes, ) +from xmodule.video_block.transcripts_utils import clear_transcripts from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py index 31877b9153d5..9195e381bc7e 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py @@ -249,7 +249,8 @@ def call_api(self, usage_key_string): @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable) @patch.object(downstreams_views, "sync_from_upstream") @patch.object(downstreams_views, "import_static_assets_for_library_sync", return_value=StaticFileNotices()) - def test_200(self, mock_sync_from_upstream, mock_import_staged_content): + @patch.object(downstreams_views, "clear_transcripts") + def test_200(self, mock_sync_from_upstream, mock_import_staged_content, mock_clear_transcripts): """ Does the happy path work? """ @@ -258,6 +259,7 @@ def test_200(self, mock_sync_from_upstream, mock_import_staged_content): assert response.status_code == 200 assert mock_sync_from_upstream.call_count == 1 assert mock_import_staged_content.call_count == 1 + assert mock_clear_transcripts.call_count == 1 class DeleteDownstreamSyncViewtest(_DownstreamSyncViewTestMixin, SharedModuleStoreTestCase): diff --git a/cms/lib/xblock/test/test_upstream_sync.py b/cms/lib/xblock/test/test_upstream_sync.py index 2f8f77ab6560..4d4d1fba1273 100644 --- a/cms/lib/xblock/test/test_upstream_sync.py +++ b/cms/lib/xblock/test/test_upstream_sync.py @@ -71,6 +71,21 @@ def setUp(self): '/>\n' )) + self.upstream_video_key = libs.create_library_block(self.library.key, "video", "video-upstream").usage_key + libs.set_library_block_olx(self.upstream_video_key, ( + '' + ' ' + '' + )) + libs.publish_changes(self.library.key, self.user.id) self.taxonomy_all_org = tagging_api.create_taxonomy( @@ -539,3 +554,17 @@ def test_sync_library_block_tags(self): assert len(object_tags) == len(new_upstream_tags) for object_tag in object_tags: assert object_tag.value in new_upstream_tags + + def test_sync_video_block(self): + downstream = BlockFactory.create(category='video', parent=self.unit, upstream=str(self.upstream_video_key)) + downstream.edx_video_id = "test_video_id" + + # Sync + sync_from_upstream(downstream, self.user) + assert downstream.upstream_version == 2 + assert downstream.upstream_display_name == "Video Test" + assert downstream.display_name == "Video Test" + + # `edx_video_id` doesn't change + assert downstream.edx_video_id == "test_video_id" + \ No newline at end of file diff --git a/xmodule/video_block/transcripts_utils.py b/xmodule/video_block/transcripts_utils.py index 4dda003b1f47..c08f9e4696bc 100644 --- a/xmodule/video_block/transcripts_utils.py +++ b/xmodule/video_block/transcripts_utils.py @@ -686,6 +686,18 @@ def convert_video_transcript(file_name, content, output_format): return dict(filename=filename, content=converted_transcript) +def clear_transcripts(block): + """ + Deletes all transcripts of a video block from VAL + """ + for language_code in block.transcripts.keys(): + edxval_api.delete_video_transcript( + video_id=block.edx_video_id, + language_code=language_code, + ) + block.transcripts = {} + + class Transcript: """ Container for transcript methods. From 4973037b4bcdcaa27f44ff1f1e5f6bc8d663a2fc Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 17 Feb 2025 17:53:10 -0500 Subject: [PATCH 37/39] style: Fix lint --- cms/lib/xblock/test/test_upstream_sync.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cms/lib/xblock/test/test_upstream_sync.py b/cms/lib/xblock/test/test_upstream_sync.py index 4d4d1fba1273..71fa7d51bb9d 100644 --- a/cms/lib/xblock/test/test_upstream_sync.py +++ b/cms/lib/xblock/test/test_upstream_sync.py @@ -567,4 +567,3 @@ def test_sync_video_block(self): # `edx_video_id` doesn't change assert downstream.edx_video_id == "test_video_id" - \ No newline at end of file From 6903be2b674410d06f07572f7acc6c37b3f23482 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 19 Feb 2025 17:34:24 -0500 Subject: [PATCH 38/39] style: Nits and comments in the code --- cms/djangoapps/contentstore/views/transcripts_ajax.py | 6 +++++- xmodule/video_block/video_handlers.py | 11 +++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index f98457a008f6..912d5b075e2e 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -98,7 +98,11 @@ def save_video_transcript_in_learning_core( language_code ): """ - Saves a video transcript to the learning core + Saves a video transcript to the learning core. + + Learning Core uses the standard `.srt` format for subtitles. + Note: SJSON is an edx-specific format that we're trying to move away from, + so for all new stuff related to Learning Core should only use `.srt`. Arguments: usage_key: UsageKey of the block diff --git a/xmodule/video_block/video_handlers.py b/xmodule/video_block/video_handlers.py index 6d136b8b6869..97f40ab2776d 100644 --- a/xmodule/video_block/video_handlers.py +++ b/xmodule/video_block/video_handlers.py @@ -541,9 +541,9 @@ def _studio_transcript_upload(self, request): new_language_code = request.POST['new_language_code'] transcript_file = request.POST['file'].file - isLibrary = isinstance(self.usage_key.context_key, LibraryLocatorV2) + is_library = isinstance(self.usage_key.context_key, LibraryLocatorV2) - if isLibrary: + if is_library: filename = f'transcript-{new_language_code}.srt' else: if not edx_video_id: @@ -560,7 +560,7 @@ def _studio_transcript_upload(self, request): 'edx_video_id': edx_video_id, 'language_code': new_language_code } - if isLibrary: + if is_library: # Save transcript as static asset in Learning Core if is a library component filename = f"static/{filename}" lib_api.add_library_block_static_asset_file( @@ -590,7 +590,7 @@ def _studio_transcript_upload(self, request): self.transcripts.pop(language_code, None) self.transcripts[new_language_code] = filename - if isLibrary: + if is_library: self._save_transcript_field() response = Response(json.dumps(payload), status=201) except (TranscriptsGenerationException, UnicodeDecodeError): @@ -622,6 +622,9 @@ def _studio_transcript_delete(self, request): if isinstance(self.usage_key.context_key, LibraryLocatorV2): transcript_name = self.transcripts.pop(language, None) if transcript_name: + # TODO: In the future, we need a proper XBlock API + # like `self.static_assets.delete(...)` instead of coding + # these runtime-specific/library-specific APIs. lib_api.delete_library_block_static_asset_file( self.usage_key, f"static/{transcript_name}", From e6ae3adfe1166aa9d904d58ac543b62975635e02 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 19 Feb 2025 17:42:03 -0500 Subject: [PATCH 39/39] style: Fix lint --- cms/djangoapps/contentstore/views/transcripts_ajax.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index 912d5b075e2e..9b667d9d67fa 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -99,7 +99,7 @@ def save_video_transcript_in_learning_core( ): """ Saves a video transcript to the learning core. - + Learning Core uses the standard `.srt` format for subtitles. Note: SJSON is an edx-specific format that we're trying to move away from, so for all new stuff related to Learning Core should only use `.srt`.