refactor: move editor_saved to VideoConfigService (#37829)

refactor: move editor_saved to VideoConfigService (#37829)
* This moves edx-platform-specific logic out of the VideoBlock,
in preparation for the VideoBlock extraction
This commit is contained in:
Muhammad Farhan Khan
2026-01-08 11:50:08 +05:00
committed by GitHub
parent d29b0473f4
commit 0b628353ef
5 changed files with 71 additions and 59 deletions

View File

@@ -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)

View File

@@ -1541,7 +1541,9 @@ class TestEditorSavedMethod(BaseTestVideoXBlock):
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

View File

@@ -33,12 +33,21 @@ from openedx.core.djangoapps.video_config.transcripts_utils import (
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 @@ class VideoConfigService:
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):
"""

View File

@@ -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)

View File

@@ -38,14 +38,13 @@ from openedx.core.lib.license import LicenseMixin
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 @@ class _BuiltInVideoBlock(
"""
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 @@ class _BuiltInVideoBlock(
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)
video_config_service = self.runtime.service(self, 'video_config')
if video_config_service:
video_config_service.handle_editor_saved(self, user.id, old_metadata)
# 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
)
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):