[SE-4161] Add Download Transcripts button #6
Conversation
There was a problem hiding this comment.
@gabor-boros LGTM 👍
- I tested this: verified the download transcripts button works as expected
- I read through the code
-
I checked for accessibilityN/A - Includes documentation
There was a problem hiding this comment.
Thanks Gabor for adding this nice feature! I've checked the PR and there's no blockers for it, except for one improvement that would be awesome to make which is the use of pytest in combination with removal of run_tests.py.
run_tests.py can add maintenance burden that is unnecessary so I added few suggestions that I've used in other XBlocks to manage such issue in reusing workbench settings when testing XBlocks.
Please let me know what do you think.
|
Hello @OmarIthawi, |
Co-authored-by: Omar Al-Ithawi <i@omardo.com>
Co-authored-by: Omar Al-Ithawi <i@omardo.com>
|
@OmarIthawi This is now ready for your review. Please note, that I added a The related build: https://travis-ci.com/github/gabor-boros/xblock-wistia/builds/220349595 |
There was a problem hiding this comment.
LGTM! Thanks @gabor-boros for the pytest refactoring and going for the extra mile!
|
@gabor-boros you can now use the tag to install this PR: https://github.com/appsembler/xblock-wistia/releases/tag/v0.3 We don't have it published on pypi yet. |
|
Thank you @OmarIthawi for merging! 🚀 |
The "download transcripts" button already exists, though it actually downloads the captions not transcripts. Hence that button -- and the related code -- is renamed to use the term "captions". Because the original intention was to add a download transcripts button in appsembler#6, this commit adds the originally required button and functionality as well. Besides that, it worth mentioning the transcripts are different in a way compared to captions in a way that it is not bind to any language.
The "download transcripts" button already exists, though it actually downloads the captions not transcripts. Hence that button -- and the related code -- is renamed to use the term "captions". Because the original intention was to add a download transcripts button in appsembler#6, this commit adds the originally required button and functionality as well. Besides that, it worth mentioning the transcripts are different in a way compared to captions in a way that it is not bind to any language.
The "download transcripts" button already exists, though it actually downloads the captions not transcripts. Hence that button -- and the related code -- is renamed to use the term "captions". Because the original intention was to add a download transcripts button in appsembler#6, this commit adds the originally required button and functionality as well. Besides that, it worth mentioning the transcripts are different in a way compared to captions in a way that it is not bind to any language.
This PR adds a new feature to download available transcripts for a Wistia video. In case no transcripts are available, the download button won't be shown. Furthermore, the transcripts are downloaded as a Zip archive containing all available transcripts.
The srt files' content is retrieved from the Wistia API and stored in a temporary directory such as the Zip archive to save memory. The Zip archive transferred as an HTTP response, not served as a static file.
Dependencies: None
Screenshots:
Sandbox URL: N/A
Merge deadline: ASAP
Testing instructions:
Author notes and concerns:
N/A
Resources
.txtsuffixReviewers