Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Video editor supports transcripts [FC-0076] #36058

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
9cdf45f
feat: Updates to support save video with transcript in library home
ChrisChV Dec 25, 2024
3885dd1
style: Fix lint
ChrisChV Dec 25, 2024
a723e96
feat: Updated code to get english transcript from filename
ChrisChV Jan 15, 2025
1c2a575
style: Fix lint
ChrisChV Jan 15, 2025
74f027f
feat: Upload transcript file as static asset in Learning Core in libr…
ChrisChV Jan 17, 2025
2adacee
style: Fix lint
ChrisChV Jan 20, 2025
0393829
feat: Updates to support download transcripts from youtube in vlibrar…
ChrisChV Jan 22, 2025
e27221e
style: Fix lint
ChrisChV Jan 22, 2025
05067f6
test: Add test for download youtube transcripts in library content
ChrisChV Jan 22, 2025
2f53a75
style: Fix lint
ChrisChV Jan 22, 2025
5c6819a
feat: Support copy transcripts from a library
ChrisChV Jan 22, 2025
95b51e8
refactor: Allow use edx_video_id (edxval) in new runtime
ChrisChV Jan 23, 2025
c42e223
refactor: Adds new edx_video_id when copy to course
ChrisChV Jan 24, 2025
4bf3af3
refactor: Remove unnevessary code
ChrisChV Jan 24, 2025
1de1d4e
feat: Support delete transcripts in library
ChrisChV Jan 25, 2025
a59898d
style: Fix lint
ChrisChV Jan 27, 2025
9173ed8
Merge branch 'master' into chris/FAL-3989-video-transcripts
ChrisChV Jan 29, 2025
590a5cd
style: Nits on the code
ChrisChV Jan 30, 2025
0e54450
refactor: Update replace_transcript to verify transcript
ChrisChV Jan 30, 2025
5685f16
refactor: Verify LibraryLocatorV2 in manage_video_subtitles_save to a…
ChrisChV Jan 30, 2025
e4f7c72
style: Add comment in transcripts_ajax.py
ChrisChV Jan 30, 2025
b478ac5
refactor: _get_item to avoid return isLibraryContent
ChrisChV Jan 30, 2025
173a0af
refactor: studio_transcript to separate methods in separated functions
ChrisChV Jan 30, 2025
4532a2a
refactor: Update code in video_handlers to avoid "static/None"
ChrisChV Jan 30, 2025
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
38 changes: 35 additions & 3 deletions cms/djangoapps/contentstore/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import re
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm seeing a bug when I sync a LibraryBlock video with transcripts from an upstream video.

Steps to reproduce:

  1. Create a library video with transcripts (here, I imported them from the example youtube video).
  2. Publish the library video.
  3. Copy it to the clipboard.
  4. Paste into a course.
    Note that the transcripts are displaying fine here.
  5. Re-edit the library video, and replace a transcript. (Here, I replaced the English one, I don't know if replacing others causes the same issue).
  6. Return to the course LibraryBlock, and refresh to see the "updates available" button. Click it.
    Note that the upstream video preview shows its transcripts fine, but the downstream (course) video preview doesn't show its transcripts anymore.
  7. Accept changes.
    Note that the course video no longer shows its transcripts, but if you edit it, you can see they're still there.
Syncing.upstream.video.breaks.transcripts.mp4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is related to openedx/modular-learning#246

Copy link
Contributor

Choose a reason for hiding this comment

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

@ChrisChV That could very well be.. however I don't think it's resolved by @DanielVZ96 's #36173, but it's also possible that I didn't merge conflicts accurately. cf my merged branch.


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 _
Expand All @@ -23,6 +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 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
Expand Down Expand Up @@ -299,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
Expand Down Expand Up @@ -504,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],
Expand Down Expand Up @@ -540,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,
Expand All @@ -566,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,
Expand All @@ -583,8 +597,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:
Expand Down Expand Up @@ -616,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
pomegranited marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
20 changes: 20 additions & 0 deletions cms/djangoapps/contentstore/views/tests/test_transcripts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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')
pomegranited marked this conversation as resolved.
Show resolved Hide resolved

def test_replace_transcript_fails_without_data(self):
"""
Verify that replace transcript fails if we do not provide video data in request.
Expand Down
Loading
Loading