Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions cms/djangoapps/contentstore/views/transcripts_ajax.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)

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

Expand Down
4 changes: 3 additions & 1 deletion lms/djangoapps/courseware/tests/test_video_mongo.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
56 changes: 56 additions & 0 deletions openedx/core/djangoapps/video_config/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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):
"""
Expand Down
4 changes: 2 additions & 2 deletions openedx/core/djangoapps/video_config/transcripts_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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)


Expand Down
60 changes: 7 additions & 53 deletions xmodule/video_block/video_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
Loading