diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index 903bf2dd796a..3fa0cba95182 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -86,7 +86,7 @@ def link_video_to_component(video_component, user): if not edx_video_id: edx_video_id = create_external_video(display_name='external video') video_component.edx_video_id = edx_video_id - video_component.save_with_metadata(user) + video_component.save_with_metadata(user.id) return edx_video_id @@ -258,7 +258,7 @@ def upload_transcripts(request): if not edx_video_id: edx_video_id = create_external_video(display_name='external video') video.edx_video_id = edx_video_id - video.save_with_metadata(request.user) + video.save_with_metadata(request.user.id) response = JsonResponse({'edx_video_id': edx_video_id, 'status': 'Success'}, status=200) @@ -282,7 +282,7 @@ def upload_transcripts(request): ) video.transcripts['en'] = f"{edx_video_id}-en.srt" - video.save_with_metadata(request.user) + video.save_with_metadata(request.user.id) if transcript_created is None: response = JsonResponse({'status': 'Invalid Video ID'}, status=400) diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index f01bfad49432..f4d2847cff0f 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -1541,7 +1541,9 @@ def test_editor_saved_when_youtube_and_html5_subs_exist(self): assert isinstance(Transcript.get_asset(item.location, self.file_name), StaticContent) assert isinstance(Transcript.get_asset(item.location, 'subs_video.srt.sjson'), StaticContent) old_metadata = own_metadata(item) - with patch('xmodule.video_block.video_block.manage_video_subtitles_save') as manage_video_subtitles_save: + with patch( + 'openedx.core.djangoapps.video_config.services.manage_video_subtitles_save' + ) as manage_video_subtitles_save: item.editor_saved(self.user, old_metadata, None) assert not manage_video_subtitles_save.called diff --git a/openedx/core/djangoapps/video_config/services.py b/openedx/core/djangoapps/video_config/services.py index e46d575e9542..5f2ca454eaa6 100644 --- a/openedx/core/djangoapps/video_config/services.py +++ b/openedx/core/djangoapps/video_config/services.py @@ -33,12 +33,21 @@ Transcript, clean_video_id, get_html5_ids, + manage_video_subtitles_save, remove_subs_from_store, get_transcript_for_video, get_transcript, ) from xmodule.exceptions import NotFoundError +from xmodule.modulestore.inheritance import own_metadata + +# The following import/except block for edxval is temporary measure until +# edxval is a proper XBlock Runtime Service. +try: + import edxval.api as edxval_api +except ImportError: + edxval_api = None log = logging.getLogger(__name__) @@ -296,6 +305,53 @@ def delete_transcript( video_block.transcripts.pop(language_code, None), video_block, language_code ) + def handle_editor_saved( + self, + video_block, + user_id: int, + old_metadata: dict | None + ): + """ + Handle video block editor save operations. + Used to update video values during save method from CMS. + """ + metadata_was_changed_by_user = old_metadata != own_metadata(video_block) + + # There is an edge case when old_metadata and own_metadata are same and we are importing transcript from youtube + # then there is a syncing issue where html5_subs are not syncing with youtube sub, We can make sync better by + # checking if transcript is present for the video and if any html5_ids transcript is not present then trigger + # the manage_video_subtitles_save to create the missing transcript with particular html5_id. + if not metadata_was_changed_by_user and video_block.sub and hasattr(video_block, 'html5_sources'): + html5_ids = get_html5_ids(video_block.html5_sources) + for subs_id in html5_ids: + try: + Transcript.asset(video_block.location, subs_id) + except NotFoundError: + # If a transcript does not not exist with particular html5_id then there is no need to check other + # html5_ids because we have to create a new transcript with this missing html5_id by turning on + # metadata_was_changed_by_user flag. + metadata_was_changed_by_user = True + break + + if metadata_was_changed_by_user: + video_block.edx_video_id = video_block.edx_video_id and video_block.edx_video_id.strip() + + # We want to override `youtube_id_1_0` with val youtube profile in the first place when someone adds/edits + # an `edx_video_id` or its underlying YT val profile. Without this, override will only happen when a user + # saves the video second time. This is because of the syncing of basic and advanced video settings which + # also syncs val youtube id from basic tab's `Video Url` to advanced tab's `Youtube ID`. + if video_block.edx_video_id and edxval_api: + val_youtube_id = edxval_api.get_url_for_profile(video_block.edx_video_id, 'youtube') + if val_youtube_id and video_block.youtube_id_1_0 != val_youtube_id: + video_block.youtube_id_1_0 = val_youtube_id + + manage_video_subtitles_save( + video_block, + user_id, + old_metadata if old_metadata else None, + generate_translation=True + ) + def _save_transcript_field(video_block): """ diff --git a/openedx/core/djangoapps/video_config/transcripts_utils.py b/openedx/core/djangoapps/video_config/transcripts_utils.py index 15cae30a2f62..7714680a3192 100644 --- a/openedx/core/djangoapps/video_config/transcripts_utils.py +++ b/openedx/core/djangoapps/video_config/transcripts_utils.py @@ -403,7 +403,7 @@ def get_html5_ids(html5_sources): return html5_ids -def manage_video_subtitles_save(item, user, old_metadata=None, generate_translation=False): +def manage_video_subtitles_save(item, user_id, old_metadata=None, generate_translation=False): """ Does some specific things, that can be done only on save. @@ -459,7 +459,7 @@ def manage_video_subtitles_save(item, user, old_metadata=None, generate_translat except TranscriptException: pass if reraised_message: - item.save_with_metadata(user) + item.save_with_metadata(user_id) raise TranscriptException(reraised_message) diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index 34e2b2d8776e..4cf58420b69e 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -38,14 +38,13 @@ from xmodule.contentstore.content import StaticContent from xmodule.editing_block import EditingMixin from xmodule.exceptions import NotFoundError -from xmodule.modulestore.inheritance import InheritanceKeyValueStore, own_metadata +from xmodule.modulestore.inheritance import InheritanceKeyValueStore from xmodule.raw_block import EmptyDataRawMixin from xmodule.util.builtin_assets import add_css_to_fragment, add_webpack_js_to_fragment from xmodule.validation import StudioValidation, StudioValidationMessage -from xmodule.video_block import manage_video_subtitles_save from xmodule.x_module import ( PUBLIC_VIEW, STUDENT_VIEW, - ResourceTemplates, shim_xmodule_js, + ResourceTemplates, XModuleMixin, XModuleToXBlockMixin, ) from xmodule.xml_block import XmlMixin, deserialize_field, is_pointer_tag, name_to_pathname @@ -254,18 +253,6 @@ def author_view(self, context): """ return self.student_view(context) - def studio_view(self, _context): - """ - Return the studio view. - """ - fragment = Fragment( - self.runtime.service(self, 'mako').render_cms_template(self.mako_template, self.get_context()) - ) - add_css_to_fragment(fragment, 'VideoBlockEditor.css') - add_webpack_js_to_fragment(fragment, 'VideoBlockEditor') - shim_xmodule_js(fragment, 'TabsEditingDescriptor') - return fragment - def public_view(self, context): """ Returns a fragment that contains the html for the public view @@ -573,49 +560,16 @@ def editor_saved(self, user, old_metadata, old_content): # lint-amnesty, pylint That means that html5_sources are always in list of fields that were changed (`metadata` param in save_item). This should be fixed too. """ - metadata_was_changed_by_user = old_metadata != own_metadata(self) - - # There is an edge case when old_metadata and own_metadata are same and we are importing transcript from youtube - # then there is a syncing issue where html5_subs are not syncing with youtube sub, We can make sync better by - # checking if transcript is present for the video and if any html5_ids transcript is not present then trigger - # the manage_video_subtitles_save to create the missing transcript with particular html5_id. - if not metadata_was_changed_by_user and self.sub and hasattr(self, 'html5_sources'): - html5_ids = get_html5_ids(self.html5_sources) - for subs_id in html5_ids: - try: - Transcript.asset(self.location, subs_id) - except NotFoundError: - # If a transcript does not not exist with particular html5_id then there is no need to check other - # html5_ids because we have to create a new transcript with this missing html5_id by turning on - # metadata_was_changed_by_user flag. - metadata_was_changed_by_user = True - break - - if metadata_was_changed_by_user: - self.edx_video_id = self.edx_video_id and self.edx_video_id.strip() - - # We want to override `youtube_id_1_0` with val youtube profile in the first place when someone adds/edits - # an `edx_video_id` or its underlying YT val profile. Without this, override will only happen when a user - # saves the video second time. This is because of the syncing of basic and advanced video settings which - # also syncs val youtube id from basic tab's `Video Url` to advanced tab's `Youtube ID`. - if self.edx_video_id and edxval_api: - val_youtube_id = edxval_api.get_url_for_profile(self.edx_video_id, 'youtube') - if val_youtube_id and self.youtube_id_1_0 != val_youtube_id: - self.youtube_id_1_0 = val_youtube_id - - manage_video_subtitles_save( - self, - user, - old_metadata if old_metadata else None, - generate_translation=True - ) + video_config_service = self.runtime.service(self, 'video_config') + if video_config_service: + video_config_service.handle_editor_saved(self, user.id, old_metadata) - def save_with_metadata(self, user): + def save_with_metadata(self, user_id): """ Save block with updated metadata to database." """ self.save() - self.runtime.modulestore.update_item(self, user.id) + self.runtime.modulestore.update_item(self, user_id) @property def editable_metadata_fields(self):