Skip to content

fix(AU-2128): Let 'en' be selected as transcript by default when youtube video#35116

Merged
rayzhou-bit merged 3 commits intomasterfrom
fix/AU-2128
Jul 15, 2024
Merged

fix(AU-2128): Let 'en' be selected as transcript by default when youtube video#35116
rayzhou-bit merged 3 commits intomasterfrom
fix/AU-2128

Conversation

@Rodra
Copy link
Copy Markdown
Contributor

@Rodra Rodra commented Jul 12, 2024

AU-2128

Description

Let english transcript be selected by default in video player despite transcript not available in transcripts["transcripts"] but available in transcripts["sub"].

Current Behavior
https://www.loom.com/share/a4e31efd83d8418f9577f3e43c9a5721?sid=34060663-e8dc-456a-a49d-ea163b650c4d

New Behavior
https://www.loom.com/share/c7cf07c1440942c893e7287496b83b7b?sid=bec454c3-7685-4dca-9970-daf341d64c43

Testing instructions

Deadline

Other information

Copy link
Copy Markdown
Contributor

@nsprenkle nsprenkle left a comment

Choose a reason for hiding this comment

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

@Rodra , could you add some comments here to explain the logic, I'm finding it hard to parse through.

@Rodra
Copy link
Copy Markdown
Contributor Author

Rodra commented Jul 15, 2024

@Rodra , could you add some comments here to explain the logic, I'm finding it hard to parse through.

Sure, this new condition when selecting the default transcript is the one that does the trick https://github.com/openedx/edx-platform/pull/35116/files#diff-a7e3c5a7f8c78b1241f3617e180bc1976ac83b45e804055e4a677eed0bca87e4R872.

Since at this point of the code https://github.com/openedx/edx-platform/pull/35116/files#diff-a7e3c5a7f8c78b1241f3617e180bc1976ac83b45e804055e4a677eed0bca87e4L867 other_langs is a list that doesn't have english (because is not part of that list if it's a youtube video), we were skipping the first condition of the if (https://github.com/openedx/edx-platform/pull/35116/files#diff-a7e3c5a7f8c78b1241f3617e180bc1976ac83b45e804055e4a677eed0bca87e4L867) and the transcript language selected ends up being the one that was set prior to triggering this action (https://github.com/openedx/edx-platform/pull/35116/files#diff-a7e3c5a7f8c78b1241f3617e180bc1976ac83b45e804055e4a677eed0bca87e4L871).

But I noticed that english IS being added to the transcript options of the video player, but a little bit later in here https://github.com/openedx/edx-platform/blob/master/xmodule/video_block/video_block.py#L191. So I just reused this condition to select transcript as default even if english is not inside of the transcript options list yet. Because I'm sure it will be added later.

Copy link
Copy Markdown
Contributor

@rayzhou-bit rayzhou-bit left a comment

Choose a reason for hiding this comment

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

lgtm!

@rayzhou-bit rayzhou-bit merged commit 69b4f69 into master Jul 15, 2024
@rayzhou-bit rayzhou-bit deleted the fix/AU-2128 branch July 15, 2024 17:00
@edx-pipeline-bot
Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

@edx-pipeline-bot
Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been rolled back from the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been rolled back from the edX production environment.

@edx-pipeline-bot
Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

mudassir-hafeez pushed a commit to mudassir-hafeez/edx-platform that referenced this pull request Jul 24, 2024
mudassir-hafeez pushed a commit to mudassir-hafeez/edx-platform that referenced this pull request Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants