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
1 change: 1 addition & 0 deletions openedx/core/djangoapps/schedules/content_highlights.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ def _get_course_block(course_descriptor, user):
""" Gets course block that takes into account user state and permissions """
# Adding courseware imports here to insulate other apps (e.g. schedules) to
# avoid import errors.
# TODO remove the LMS dependency https://github.com/openedx/edx-platform/issues/37659
from lms.djangoapps.courseware.model_data import FieldDataCache
from lms.djangoapps.courseware.block_render import get_block_for_descriptor

Expand Down
142 changes: 138 additions & 4 deletions openedx/core/djangoapps/video_config/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,33 @@

import logging

from opaque_keys.edx.keys import CourseKey, UsageKey

from openedx.core.djangoapps.video_config import sharing
from django.core.files import File
from django.core.files.base import ContentFile
from edxval.api import create_external_video, create_or_update_video_transcript, delete_video_transcript
from opaque_keys.edx.keys import (
CourseKey,
UsageKey,
)
from opaque_keys.edx.locator import LibraryLocatorV2
from xblocks_contrib.video import VideoBlock
Copy link
Contributor

@farhan farhan Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we import it from the xmodule.

I am also having import VideoBlock in this PR, will be great if we could skip its annotation until we are done with extraction work.

from organizations.api import get_course_organization
from openedx.core.djangoapps.video_config import sharing
from openedx.core.djangoapps.video_config.models import (
CourseYoutubeBlockedFlag,
HLSPlaybackEnabledFlag,
)
from openedx.core.djangoapps.video_config.toggles import TRANSCRIPT_FEEDBACK
from openedx.core.djangoapps.video_pipeline.config.waffle import DEPRECATE_YOUTUBE
from openedx.core.djangoapps.content_libraries.api import (
add_library_block_static_asset_file,
delete_library_block_static_asset_file,
)
from openedx.core.djangoapps.video_config.transcripts_utils import (
Transcript,
clean_video_id,
get_html5_ids,
remove_subs_from_store,
)

log = logging.getLogger(__name__)

Expand All @@ -29,6 +46,9 @@ class VideoConfigService:
This service abstracts away edx-platform specific functionality
that the Video XBlock needs, allowing the Video XBlock to be
extracted to a separate repository.

TODO: This service could be improved in a few ways:
https://github.com/openedx/edx-platform/issues/37656
"""

def get_public_video_url(self, usage_id: UsageKey) -> str:
Expand All @@ -37,7 +57,7 @@ def get_public_video_url(self, usage_id: UsageKey) -> str:
"""
return sharing.get_public_video_url(usage_id)

def get_public_sharing_context(self, video_block, course_key: CourseKey) -> dict:
def get_public_sharing_context(self, video_block: VideoBlock, course_key: CourseKey) -> dict:
"""
Get the complete public sharing context for a video.

Expand Down Expand Up @@ -91,3 +111,117 @@ def is_hls_playback_enabled(self, course_id: CourseKey) -> bool:
Check if HLS playback is enabled for the course.
"""
return HLSPlaybackEnabledFlag.feature_enabled(course_id)

def upload_transcript(
self,
*,
video_block: VideoBlock,
language_code: str,
new_language_code: str | None,
transcript_file: File,
edx_video_id: str | None,
) -> None:
"""
Store a transcript, however the runtime prefers to.

Mutates:
* video_block.transcripts
* video_block.edx_video_id, iff a new video is created in edx-val.

Can raise:
* UnicodeDecodeError
* TranscriptsGenerationException
Copy link
Contributor

@farhan farhan Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we throw platform specific exceptions, for example TranscriptsGenerationException here? 🤔

Even if we throw them we can't catch the specific exceptions within the external XBlock.

So if we can only catch the general exception in the external XBlock. Shouldn't we only throw general exceptions?

"""
is_library = isinstance(video_block.usage_key.context_key, LibraryLocatorV2)
content: bytes = transcript_file.read()
if is_library:
# Save transcript as static asset in Learning Core if is a library component
filename = f'transcript-{new_language_code}.srt'
add_library_block_static_asset_file(video_block.usage_key, f"static/{filename}", content)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are appending static and passing in the param here unlike we are setting it in the transcripts field of video block in old code here

Which will possibly not update the field while saving here

else:
edx_video_id = clean_video_id(edx_video_id)
if not edx_video_id:
# Back-populate the video ID for an external video.
# pylint: disable=attribute-defined-outside-init
edx_video_id = create_external_video(display_name='external video')
video_block.edx_video_id = edx_video_id
filename = f'{edx_video_id}-{new_language_code}.srt'
# Convert SRT transcript into an SJSON format and upload it to S3 if a course component
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),
)

# 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:
video_block.transcripts.pop(language_code, None)
video_block.transcripts[new_language_code] = filename

if is_library:
_save_transcript_field(video_block)

def delete_transcript(
self,
*,
video_block: VideoBlock,
edx_video_id: str | None,
language_code: str,
) -> None:
"""
Delete a transcript from the runtime's storage.
"""
edx_video_id = clean_video_id(edx_video_id)
if edx_video_id:
delete_video_transcript(video_id=edx_video_id, language_code=language_code)
if isinstance(video_block.context_key, LibraryLocatorV2):
transcript_name = video_block.transcripts.pop(language_code, None)
if transcript_name:
delete_library_block_static_asset_file(video_block.usage_key, f"static/{transcript_name}")
_save_transcript_field(video_block)
else:
if language_code == 'en':
# remove any transcript file from content store for the video ids
possible_sub_ids = [
video_block.sub, # pylint: disable=access-member-before-definition
video_block.youtube_id_1_0
] + get_html5_ids(video_block.html5_sources)
for sub_id in possible_sub_ids:
remove_subs_from_store(sub_id, video_block, language_code)
# update metadata as `en` can also be present in `transcripts` field
remove_subs_from_store(
video_block.transcripts.pop(language_code, None), video_block, language_code
)
# also empty `sub` field
video_block.sub = '' # pylint: disable=attribute-defined-outside-init
else:
remove_subs_from_store(
video_block.transcripts.pop(language_code, None), video_block, language_code
)


def _save_transcript_field(video_block: VideoBlock):
"""
Hacky workaround to ensure that transcript field is saved for Learning Core video blocks.

It's not clear why this is necessary.
"""
field = video_block.fields['transcripts']
if video_block.transcripts:
transcripts_copy = video_block.transcripts.copy()
# Need to delete to overwrite, it's weird behavior,
# but it only works like this.
field.delete_from(video_block)
field.write_to(video_block, transcripts_copy)
else:
field.delete_from(video_block)
7 changes: 5 additions & 2 deletions openedx/core/djangoapps/video_config/transcripts_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@
from xmodule.contentstore.django import contentstore
from xmodule.exceptions import NotFoundError

from xmodule.video_block.bumper_utils import get_bumper_settings

try:
from edxval import api as edxval_api
except ImportError:
Expand Down Expand Up @@ -911,6 +909,11 @@ def get_transcripts_info(self, is_bumper=False):
is_bumper(bool): If True, the request is for the bumper transcripts
include_val_transcripts(bool): If True, include edx-val transcripts as well
"""
# TODO: This causes a circular import when imported at the top-level.
# This import will be removed as part of the VideoBlock extraction.
# https://github.com/openedx/edx-platform/issues/36282
from xmodule.video_block.bumper_utils import get_bumper_settings

if is_bumper:
transcripts = copy.deepcopy(get_bumper_settings(self).get('transcripts', {}))
sub = transcripts.pop("en", "")
Expand Down
2 changes: 2 additions & 0 deletions openedx/core/djangoapps/xblock/runtime/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ def handle_grade_event(self, block: XBlock, event: dict):
Submit a grade for the block.
"""
if self.user and not self.user.is_anonymous:
# TODO: we shouldn't be using an LMS API here.
# https://github.com/openedx/edx-platform/issues/37660
grades_signals.SCORE_PUBLISHED.send(
sender=None,
block=block,
Expand Down
21 changes: 15 additions & 6 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,6 @@ ignore_imports =
# -> openedx.core.djangoapps.course_groups.partition_scheme
# -> lms.djangoapps.courseware.masquerade
openedx.core.djangoapps.course_groups.partition_scheme -> lms.djangoapps.courseware.masquerade
# cms.djangoapps.contentstore.[various]
# -> openedx.core.djangoapps.content.course_overviews.models
# -> lms.djangoapps.ccx.utils
# & lms.djangoapps.certificates.api
# & lms.djangoapps.discussion.django_comment_client
openedx.core.djangoapps.content.course_overviews.models -> lms.djangoapps.*.*
# cms.djangoapps.export_course_metadata.tasks
# -> openedx.core.djangoapps.schedules.content_highlights
# -> lms.djangoapps.courseware.block_render & lms.djangoapps.courseware.model_data
Expand Down Expand Up @@ -169,6 +163,12 @@ ignore_imports =
# https://github.com/openedx/edx-platform/issues/33428
openedx.core.djangoapps.content_libraries.permissions -> cms.djangoapps.course_creators.views
openedx.core.djangoapps.content_libraries.tasks -> cms.djangoapps.contentstore.storage
# Outstanding arch issue: course_overviews is tangled up with LMS
# https://github.com/openedx/edx-platform/issues/37658
openedx.core.djangoapps.content.course_overviews.models -> lms.djangoapps.**
# Outstanding arch issue: content_highlights uses courseware block_render
# https://github.com/openedx/edx-platform/issues/37659
openedx.core.djangoapps.schedules.content_highlights -> lms.djangoapps.courseware.block_render

[importlinter:contract:2]
name = Do not depend on non-public API of isolated apps.
Expand Down Expand Up @@ -232,3 +232,12 @@ ignore_imports =
# Content libraries imports contentstore.helpers which imports upstream_sync
openedx.core.djangoapps.content_libraries.api.blocks -> cms.djangoapps.contentstore.helpers
openedx.core.djangoapps.content_libraries.api.libraries -> cms.djangoapps.contentstore.helpers
# Outstanding arch issue: course_overviews is tangled up with LMS
# https://github.com/openedx/edx-platform/issues/37658
openedx.core.djangoapps.content.course_overviews.models -> lms.djangoapps.**
# Outstanding arch issue: content_highlights uses courseware block_render
# https://github.com/openedx/edx-platform/issues/37659
openedx.core.djangoapps.schedules.content_highlights -> lms.djangoapps.courseware.block_render
# This import happens only because grading signals are defined in LMS rather than openedx-events
# https://github.com/openedx/edx-platform/issues/37660
openedx.core.djangoapps.xblock.runtime.runtime -> lms.djangoapps.grades.api
Loading
Loading