Skip to content

New feature: Add webm file format to Studio#2973

Merged
bjester merged 13 commits intolearningequality:unstablefrom
sairina:webm
May 4, 2021
Merged

New feature: Add webm file format to Studio#2973
bjester merged 13 commits intolearningequality:unstablefrom
sairina:webm

Conversation

@sairina
Copy link
Copy Markdown
Contributor

@sairina sairina commented Feb 19, 2021

Description

We added the video format webm for use in Studio both in the Studio code as well as pulled in the updated le-utils dependency.

Steps to Test

Ricecooker-created channel

  1. Create a new sushichef script using ricecooker.
  2. Add at least one webm file to a video node (via URL or upload from your computer)
  3. Once the script has been run successfully, log in to Studio
  4. You should see the new ricecooker-made channel appear in your channel list
  5. Click on it, then click on the link "Updated resources are ready for review"
  6. Navigate to your webm video and check to see that it plays

Manually-created channel

  1. Log in
  2. Create new channel (click New channel)
  3. In the new channel, upload a file that has the .webm extension (click on Add > Upload files)
  4. Check to see that it appears in the channel
  5. Click on it to check that it plays

Implementation Notes (optional)

At a high level, how did you implement this?

Added a new file format in le-utils, pulled that into Studio, updated associated files, tests, language strings to include webm files, and updated DB to include webm as an option for one of the fields.

Checklist

Delete any items that don't apply

  • Is the code clean and well-commented?
  • Has the docs label been added if this introduces a change that needs to be updated in the user docs?
  • Has the CHANGELOG label been added to this pull request? Items with this label will be added to the CHANGELOG at a later time
  • Are there tests for this change?
  • Are all user-facing strings translated properly (if applicable)?
  • Are views organized into pages, components, and layouts directories as described in the docs?
  • Are users' storage used being recalculated properly on any changes to their main tree files?
  • Are there any new interactions that need to be added to the QA Sheet?
  • Are there opportunities for using Google Analytics here (if applicable)?
  • If any Python requirements have changed, are the updated requirements.txt files also included in this PR?
  • Are the migrations safe for a large db (if applicable)?

Comments

Any additional notes you'd like to add

Reviewers

If you are looking to assign a reviewer, here are some options:

  • Jordan jayoshih (full stack)
  • Aron aronasorman (back end, devops)
  • Micah micahscopes (full stack)
  • Kevin kollivier (back end)
  • Ivan ivanistheone (Ricecooker)
  • Richard rtibbles (full stack, Kolibri)
  • Radina @radinamatic (documentation)

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 19, 2021

Codecov Report

Merging #2973 (1fb9dbc) into unstable (7558536) will decrease coverage by 0.40%.
The diff coverage is 100.00%.

❗ Current head 1fb9dbc differs from pull request most recent head 07fb3ca. Consider uploading reports for the commit 07fb3ca to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##           unstable    #2973      +/-   ##
============================================
- Coverage     86.13%   85.72%   -0.41%     
============================================
  Files           305      300       -5     
  Lines         16455    15891     -564     
============================================
- Hits          14173    13623     -550     
+ Misses         2282     2268      -14     
Impacted Files Coverage Δ
...tentcuration/migrations/0121_auto_20210219_2314.py 100.00% <100.00%> (ø)
...ration/contentcuration/tests/test_createchannel.py 100.00% <100.00%> (ø)
...ibri_content/migrations/0010_auto_20210202_0604.py 100.00% <100.00%> (ø)
contentcuration/contentcuration/utils/cache.py 31.70% <0.00%> (-33.64%) ⬇️
contentcuration/contentcuration/viewsets/task.py 75.00% <0.00%> (-19.92%) ⬇️
contentcuration/contentcuration/utils/nodes.py 77.77% <0.00%> (-6.04%) ⬇️
contentcuration/contentcuration/viewsets/file.py 72.22% <0.00%> (-2.78%) ⬇️
contentcuration/contentcuration/models.py 84.41% <0.00%> (-2.15%) ⬇️
...tcuration/contentcuration/db/models/expressions.py 93.33% <0.00%> (-1.91%) ⬇️
...ion/contentcuration/tests/viewsets/test_channel.py 92.91% <0.00%> (-1.27%) ⬇️
... and 39 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7be4bd...07fb3ca. Read the comment docs.

@radinamatic
Copy link
Copy Markdown
Member

radinamatic commented Feb 19, 2021

This feature will presumably need to be added to the user documentation and have proper steps for testing added in the Gherkin scenarios, so I've added the labels 🙂

@sairina sairina added the changelog Needs to be added to the changelog (remove once added) label Feb 20, 2021
@sairina sairina marked this pull request as ready for review February 20, 2021 03:39
@rtibbles
Copy link
Copy Markdown
Member

Looking through the code this looks like we have all the right changes in place. This change feels simultaneously slightly risky, and also not risky at all. Wondering if we might want to just have it in its own encapsulated release for focal testing.

@sairina
Copy link
Copy Markdown
Contributor Author

sairina commented Feb 25, 2021

Since there needs to be changes made to documentation, Gherkin, etc. per @radinamatic , perhaps its own release would be best?

@rtibbles rtibbles changed the base branch from hotfixes to unstable April 15, 2021 20:49
Copy link
Copy Markdown
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

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

Looks good 🎉

@rtibbles
Copy link
Copy Markdown
Member

rtibbles commented May 4, 2021

Let's get this into the next batch to hotfixes and check how this flies with Kolibri!

@bjester
Copy link
Copy Markdown
Member

bjester commented May 4, 2021

@rtibbles Do migrations get applied prior to the new migrations check (sh -c './contentcuration/manage.py makemigrations --check')?

@rtibbles
Copy link
Copy Markdown
Member

rtibbles commented May 4, 2021

No, but they shouldn't need to, as Django bases its migrations checking off code, not the DB.

@bjester bjester merged commit f164515 into learningequality:unstable May 4, 2021
@pcenov
Copy link
Copy Markdown
Member

pcenov commented May 13, 2021

Tested at https://hotfixes.studio.learningequality.org/ - all looks good. Noticed that WebM videos cannot be uploaded or played in my Safari 13.1.3 on MacOS Catalina 10.15.7 but read online that with the release of macOS Big Sur 11.3 Safari can play WebM videos.

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

Labels

changelog Needs to be added to the changelog (remove once added) TODO: needs gherkin update TODO: needs user docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants