feat: Enable transcripts for video library [FC-0076] #1596

ChrisChV · 2025-01-17T23:54:16Z

Description

Get updateTranscriptHandlerUrl() and call it when is ready.
Enable LanguageNamesWidget in a library.
Enable add transcripts for libraries.
Enable delete transcripts for libraries.
Enable replace transcripts for libraries.
Enable download transcripts for libraries.
Enable download transcripts from YouTube
Which edX user roles will this change impact? "Course Author"

Supporting information

Github link: Video editor supports transcripts (minimal version) #1352
Depends on: feat: Video editor supports transcripts [FC-0076] edx-platform#36058
Internal ticket: FAL-3989

Testing instructions

Create a video in a course unit. You can use this video link
Add two or more transcripts. You can use this files
Copy the video block.
Go to a library in the library home and paste the video block.
Verify that you can see the transcripts in the preview and in the static assets.
Copy the block and paste it in the course. Verify that you can see the transcripts in the preview.
Copy the same block and paste it in the library again. Verify that you can see the transcripts in the preview. and in the static assets.
Create a video in a library. You can use this video link
Add two or more transcripts. You can use this files. Verify that you can see the transcripts in the preview. and in the static assets.
Copy the video block and paste it in a course. Verify that you can see the transcripts in the preview.
Copy the same video block from the course and paste it in a library. Verify that you can see the transcripts in the preview. and in the static assets.
Copy the same video block from the library and paste in a course. Verify that you can see the transcripts in the preview.
In a library, on the video editor, verify:
- You can delete a transcript.
- You can replace a transcript.
- You can download a transcript
- You can update the language of a transcript.
In a library, create a new video block and use a YouTube link. You can use this video
Save the block and open the editor again. Click on the Import transcript button in the port transcript from YouTube? box. Save.
Verify that you can see the transcripts in the preview. and in the static assets.
Copy the video block and paste it in a course. Verify that you can see the transcripts in the preview.

Other information

TODO: The transcripts are not copied when you add a library component in a course. This will be fixed in Copy static assets when using a library component in a course (via Problem Bank or Library Content) modular-learning#246
Merge feat: Video editor supports transcripts [FC-0076] edx-platform#36058 first

openedx-webhooks · 2025-01-17T23:54:20Z

Thanks for the pull request, @ChrisChV!

This repository is currently maintained by @openedx/2u-tnl.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

codecov · 2025-01-20T21:54:36Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.26%. Comparing base (b7241a1) to head (e23367e).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1596      +/-   ##
==========================================
+ Coverage   93.24%   93.26%   +0.02%     
==========================================
  Files        1100     1100              
  Lines       21803    21848      +45     
  Branches     4725     4639      -86     
==========================================
+ Hits        20330    20377      +47     
- Misses       1401     1406       +5     
+ Partials       72       65       -7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pomegranited

@ChrisChV Your code is working fine :) But I left some nits and questions, so would like to re-review once they're addressed.

Also: I found some bugs with the subtitles in the Course asset list. When I've copied a Library Video into a Course (creating a new LibraryBlock), it (correctly) notifies me that the subtitle files have been added to the course. But when I try to view their URLs, I get errors:

Subtitle.asset.bugs.mp4

pomegranited · 2025-01-29T06:46:22Z

src/editors/containers/VideoEditor/components/VideoSettingsModal/index.tsx

-    {!isLibrary && ( // Since content libraries v2 don't support static assets yet, we can't include transcripts.
-      <TranscriptWidget />
-    )}
+    <TranscriptWidget />


(out of scope for this issue) The Transcripts part of this VideoPreviewWidget is hard to use 😞

Can't type in the language I want in the dropdown; I have to scroll all the way down to the language to select it.

No spinners shown while transcript uploads / downloads from Youtube are happening -- it just sits there looking like it's doing nothing.
It also doesn't prevent me from clicking "Save", and so if an upload / replace / delete for a transcript fails, I don't know about it until I re-open the modal.

When I import transcripts from YouTube, only one of the imported transcripts (English) shows up in the preview widget.
I have to save, and the re-edit to see all languages for the imported transcripts.

The "select a language then launch file upload control" behavior is non-intuitive, and may not be accessible either? I think these two components should split in two, so I can select my language / upload a file in whatever order I want to.

The "Download" transcript menu item tries to send me to a new page. It should open in a new window (which closes after the download completes).

Do we have budget for a follow-up task to fix this?

Thanks @pomegranited for listing this issues. It will be very helpful!

Do we have budget for a follow-up task to fix this?

I think so, I have to evaluate it.

pomegranited · 2025-01-29T06:52:36Z

src/editors/data/redux/thunkActions/requests.js

-    }),
-    ...rest,
-  }));
+  const isLibrary = selectors.app.isLibrary(getState());


nit: Do we need to call getState() twice in each of these 4 functions below?

Updated here 9f74c07

pomegranited · 2025-01-29T07:11:47Z

src/editors/data/services/cms/urls.ts

@@ -80,6 +80,14 @@ export const downloadVideoTranscriptURL = (({ studioEndpointUrl, blockId, langua
  `${videoTranscripts({ studioEndpointUrl, blockId })}?language_code=${language}`
 )) satisfies UrlFunction;

+export const trascriptXblockV2 = (({ transcriptHandlerUrl }) => (


nit: typo

Suggested change

export const trascriptXblockV2 = (({ transcriptHandlerUrl }) => (

export const transcriptXblockV2 = (({ transcriptHandlerUrl }) => (

Updated here 4961e73

src/editors/data/redux/video/selectors.js

pomegranited · 2025-01-29T07:28:02Z

.../components/VideoSettingsModal/components/TranscriptWidget/__snapshots__/index.test.jsx.snap

@@ -242,6 +242,8 @@ exports[`TranscriptWidget component snapshots snapshot: renders ErrorAlert with
 </CollapsibleFormWidget>
 `;

+exports[`TranscriptWidget component snapshots snapshot: renders when isLibrary is true 1`] = `undefined`;


I don't understand this test? Isn't the TranscriptWidget rendered when isLibrary is true?

Updated here 91b4a9d

…ptDownloadUrl

ChrisChV · 2025-01-30T00:21:32Z

Also: I found some bugs with the subtitles in the Course asset list. When I've copied a Library Video into a Course (creating a new LibraryBlock), it (correctly) notifies me that the subtitle files have been added to the course. But when I try to view their URLs, I get errors:

I think it's an issue with all files in general. Using that screen, I added a new file, and it has the same problem. For now, it is out of the scope of this task.

pomegranited

👍 Thank you for making those changes @ChrisChV ! Works great.

I tested this using the PR testing instructions.
I read through the code
I checked for accessibility issues by using my keyboard to navigate
Includes documentation -- good code comments
~~User-facing strings are extracted for translation~~ N/A

bradenmacdonald · 2025-01-30T18:04:29Z

@pomegranited @ChrisChV We are fixing the issue with linking static files into a course in openedx/edx-platform#36173

DanielVZ96

👍 LGTM sorry for taking so long!

I tested this
I read through the code
I checked for accessibility issues

bradenmacdonald

nice work!

bradenmacdonald · 2025-02-11T23:16:20Z

...s/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/index.jsx

@@ -207,6 +213,7 @@ export const mapStateToProps = (state) => ({
  allowTranscriptImport: selectors.video.allowTranscriptImport(state),
  isUploadError: selectors.requests.isFailed(state, { requestKey: RequestKeys.uploadTranscript }),
  isDeleteError: selectors.requests.isFailed(state, { requestKey: RequestKeys.deleteTranscript }),
+  isLibrary: selectors.app.isLibrary(state),


If you need to access the redux store, please use a hook instead of a prop:

const isLibrary = useSelector(selectors.app.isLibrary);

feat: Enable transcripts for video library

b7fbac9

ChrisChV requested a review from a team as a code owner January 17, 2025 23:54

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jan 17, 2025

ChrisChV marked this pull request as draft January 17, 2025 23:54

test: Enable transcript in libraries

627b4af

test: Adding tests for updateTranscriptHandlerUrl

a82e796

ChrisChV force-pushed the chris/FAL-3989-video-transcripts branch from 51ac20d to a82e796 Compare January 21, 2025 20:47

mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Jan 22, 2025

ChrisChV added 2 commits January 27, 2025 10:29

feat: Suppor delete and replace transcripts in library

9c4a930

feat: enable download, replace and update transcripts in library

73c4bda

ChrisChV mentioned this pull request Jan 29, 2025

feat: Video editor supports transcripts [FC-0076] openedx/edx-platform#36058

Open

1 task

ChrisChV marked this pull request as ready for review January 29, 2025 02:31

pomegranited suggested changes Jan 29, 2025

View reviewed changes

ChrisChV added 5 commits January 29, 2025 15:06

Merge branch 'master' into chris/FAL-3989-video-transcripts

729780e

refactor: use getState once in requests.js functions

9f74c07

refactor: trascriptXblockV2 to transcriptXblockV2 in urls

4961e73

refactor: delete getTranscriptDownloadUrlV2 and add it to getTranscri…

91b4a9d

…ptDownloadUrl

style: Fix coverage

e23367e

pomegranited approved these changes Jan 30, 2025

View reviewed changes

DanielVZ96 approved these changes Feb 1, 2025

View reviewed changes

bradenmacdonald approved these changes Feb 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Enable transcripts for video library [FC-0076] #1596

feat: Enable transcripts for video library [FC-0076] #1596

ChrisChV commented Jan 17, 2025 •

edited

Loading

openedx-webhooks commented Jan 17, 2025 •

edited

Loading

codecov bot commented Jan 20, 2025 •

edited

Loading

pomegranited left a comment

pomegranited Jan 29, 2025 •

edited

Loading

ChrisChV Jan 30, 2025

pomegranited Jan 29, 2025

ChrisChV Jan 30, 2025

pomegranited Jan 29, 2025

ChrisChV Jan 30, 2025

pomegranited Jan 29, 2025

ChrisChV Jan 30, 2025

ChrisChV commented Jan 30, 2025

pomegranited left a comment

bradenmacdonald commented Jan 30, 2025

DanielVZ96 left a comment

bradenmacdonald left a comment

bradenmacdonald Feb 11, 2025

	export const trascriptXblockV2 = (({ transcriptHandlerUrl }) => (
	export const transcriptXblockV2 = (({ transcriptHandlerUrl }) => (

feat: Enable transcripts for video library [FC-0076] #1596

Are you sure you want to change the base?

feat: Enable transcripts for video library [FC-0076] #1596

Conversation

ChrisChV commented Jan 17, 2025 • edited Loading

Description

Supporting information

Testing instructions

Other information

openedx-webhooks commented Jan 17, 2025 • edited Loading

codecov bot commented Jan 20, 2025 • edited Loading

Codecov Report

pomegranited left a comment

Choose a reason for hiding this comment

pomegranited Jan 29, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrisChV commented Jan 30, 2025

pomegranited left a comment

Choose a reason for hiding this comment

bradenmacdonald commented Jan 30, 2025

DanielVZ96 left a comment

Choose a reason for hiding this comment

bradenmacdonald left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrisChV commented Jan 17, 2025 •

edited

Loading

openedx-webhooks commented Jan 17, 2025 •

edited

Loading

codecov bot commented Jan 20, 2025 •

edited

Loading

pomegranited Jan 29, 2025 •

edited

Loading