Skip to content

Update the get_transcript util with html5_sources #17976

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.

Sign up for GitHub

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

Jump to bottom
Merged
mushtaqak merged 1 commit into from
Apr 25, 2018
Merged

Update the get_transcript util with html5_sources #17976

mushtaqak merged 1 commit into from
Apr 25, 2018

Conversation

@mushtaqak
Copy link
Contributor

@mushtaqak mushtaqak commented Apr 17, 2018 •
edited
Loading

Update the get_transcript util with html5_sources

EDUCATOR-2746

@mushtaqak mushtaqak changed the base branch from master to ammar/EDUCATOR-1759-delete-transcript-from-advance-tab April 17, 2018 12:55
@edx-status-bot
Copy link

edx-status-bot commented Apr 17, 2018

Your PR has finished running tests.

@mushtaqak mushtaqak force-pushed the mushtaq/html5_sources_transcript branch from 81d69e3 to ec00c22 Compare April 17, 2018 13:05
@edx-status-bot
Copy link

edx-status-bot commented Apr 17, 2018

Your PR has finished running tests.

1 similar comment
@edx-status-bot
Copy link

edx-status-bot commented Apr 20, 2018

Your PR has finished running tests.

@muhammad-ammar muhammad-ammar force-pushed the ammar/EDUCATOR-1759-delete-transcript-from-advance-tab branch 2 times, most recently from f2b499e to 82d0968 Compare April 23, 2018 13:45
@mushtaqak mushtaqak force-pushed the mushtaq/html5_sources_transcript branch from c07d57a to f5f2b19 Compare April 23, 2018 14:11
@edx-status-bot
Copy link

edx-status-bot commented Apr 23, 2018

Your PR has finished running tests.

1 similar comment
@edx-status-bot
Copy link

edx-status-bot commented Apr 23, 2018

Your PR has finished running tests.

@mushtaqak mushtaqak force-pushed the mushtaq/html5_sources_transcript branch from a24eb23 to a28b9bb Compare April 24, 2018 07:00
@edx-status-bot
Copy link

edx-status-bot commented Apr 24, 2018

Your PR has finished running tests.

@muhammad-ammar muhammad-ammar force-pushed the ammar/EDUCATOR-1759-delete-transcript-from-advance-tab branch from 82d0968 to d5758bd Compare April 24, 2018 11:50
@mushtaqak mushtaqak force-pushed the mushtaq/html5_sources_transcript branch from a28b9bb to e0cbe78 Compare April 24, 2018 14:03
@edx-status-bot
Copy link

edx-status-bot commented Apr 24, 2018

Your PR has finished running tests.

@mushtaqak
Copy link
Contributor Author

mushtaqak commented Apr 25, 2018

jenkins run all

@edx-status-bot
Copy link

edx-status-bot commented Apr 25, 2018

Your PR has finished running tests.

muhammad-ammar
muhammad-ammar approved these changes Apr 25, 2018
View reviewed changes
Copy link
Contributor

@muhammad-ammar muhammad-ammar left a comment

Choose a reason for hiding this comment

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

LGTM 👍 just one minor comment

common/lib/xmodule/xmodule/video_module/transcripts_utils.py
except KeyError:
raise NotFoundError
possible_sub_ids = [youtube_id, sub, video.youtube_id_1_0] + get_html5_ids(video.html5_sources)
for sub_id in possible_sub_ids:
Copy link
Contributor

@muhammad-ammar muhammad-ammar Apr 25, 2018

Choose a reason for hiding this comment

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

what about for sub_id in set(possible_sub_ids): to avoid duplicates?

Copy link
Contributor Author

@mushtaqak mushtaqak Apr 25, 2018

Choose a reason for hiding this comment

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

set reorders the element in ascending order which is what we don't want.

@mushtaqak mushtaqak force-pushed the mushtaq/html5_sources_transcript branch from e0cbe78 to b181e6b Compare April 25, 2018 12:43
@mushtaqak mushtaqak changed the base branch from ammar/EDUCATOR-1759-delete-transcript-from-advance-tab to transcripts-phase-2 April 25, 2018 12:43
@mushtaqak
Copy link
Contributor Author

mushtaqak commented Apr 25, 2018

jenkins run all

@edx-status-bot
Copy link

edx-status-bot commented Apr 25, 2018

Your PR has finished running tests.

1 similar comment
@edx-status-bot
Copy link

edx-status-bot commented Apr 25, 2018

Your PR has finished running tests.

@mushtaqak
Copy link
Contributor Author

mushtaqak commented Apr 25, 2018

jenkins run all

@edx-status-bot
Copy link

edx-status-bot commented Apr 25, 2018

Your PR has finished running tests.

1 similar comment
@edx-status-bot
Copy link

edx-status-bot commented Apr 25, 2018

Your PR has finished running tests.

@mushtaqak
Copy link
Contributor Author

mushtaqak commented Apr 25, 2018

jenkins run bokchoy

@mushtaqak
Copy link
Contributor Author

mushtaqak commented Apr 25, 2018

jenkins run quality

@mushtaqak
Copy link
Contributor Author

mushtaqak commented Apr 25, 2018

jenkins run python

@edx-status-bot
Copy link

edx-status-bot commented Apr 25, 2018

Your PR has finished running tests.

@mushtaqak
Copy link
Contributor Author

mushtaqak commented Apr 25, 2018

jenkins run python

@edx-status-bot
Copy link

edx-status-bot commented Apr 25, 2018

Your PR has finished running tests.

2 similar comments
@edx-status-bot
Copy link

edx-status-bot commented Apr 25, 2018

Your PR has finished running tests.

@edx-status-bot
Copy link

edx-status-bot commented Apr 25, 2018

Your PR has finished running tests.

@mushtaqak
Copy link
Contributor Author

mushtaqak commented Apr 25, 2018

jenkins run bokchoy

@mushtaqak
Copy link
Contributor Author

mushtaqak commented Apr 25, 2018

jenkins run python

1 similar comment
@mushtaqak
Copy link
Contributor Author

mushtaqak commented Apr 25, 2018

jenkins run python

@mushtaqak
Copy link
Contributor Author

mushtaqak commented Apr 25, 2018

jenkins run bokchoy

@edx-status-bot
Copy link

edx-status-bot commented Apr 25, 2018

Your PR has finished running tests.

@muhammad-ammar
Copy link
Contributor

muhammad-ammar commented Apr 25, 2018

@mushtaqak build passed and green

@mushtaqak mushtaqak merged commit 1ba833f into transcripts-phase-2 Apr 25, 2018
@mushtaqak mushtaqak deleted the mushtaq/html5_sources_transcript branch April 25, 2018 16:41
@edx-status-bot
Copy link

edx-status-bot commented Apr 25, 2018

Your PR has finished running tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@muhammad-ammar muhammad-ammar muhammad-ammar approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mushtaqak @edx-status-bot @muhammad-ammar