feat!: remove multi-speed YouTube fields from VideoBlock [DEPR]#254
Open
farhan wants to merge 4 commits into
Open
feat!: remove multi-speed YouTube fields from VideoBlock [DEPR]#254farhan wants to merge 4 commits into
farhan wants to merge 4 commits into
Conversation
YouTube has supported native speed controls for years, making youtube_id_0_75, youtube_id_1_25, and youtube_id_1_5 obsolete. Removes these fields and simplifies create_youtube_string() to only use youtube_id_1_0. Closes openedx/openedx-platform#36888 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy multi-speed YouTube ID fields from the Video XBlock and updates YouTube serialization to only emit the 1.00 speed entry, aligning with modern YouTube playback behavior while updating affected tests.
Changes:
- Removed
youtube_id_0_75,youtube_id_1_25, andyoutube_id_1_5fromVideoFields. - Simplified
create_youtube_string()to only serializeyoutube_id_1_0as1.00:<id>. - Updated Video XBlock unit tests to reflect the removed fields and new export format.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
xblocks_contrib/video/video_xfields.py |
Drops the deprecated speed-specific YouTube ID fields from the block schema. |
xblocks_contrib/video/video_utils.py |
Updates YouTube stream string generation to only include the 1.00 speed. |
xblocks_contrib/video/tests/test_video.py |
Adjusts import/export and utility tests for the new single-speed YouTube behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Now that multi-speed fields are removed, _parse_youtube no longer needs to populate 0.75/1.25/1.50 keys. Simplify to only retain the 1.00 entry. Also update class docstring and fix the now-redundant test_create_youtube_string_missing to test the empty ID case. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When parsing XML like youtube="0.75:abc" (no 1.0 entry), _parse_youtube
was returning {'1.00': ''}, silently losing the video ID. Now falls back
to the first non-empty legacy speed so no ID is dropped on import.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The for loop and normalized_speed logic in parse_video_xml existed solely
to fan out youtube_id_* fields for 0.75/1.25/1.50 speeds. Now that
_parse_youtube always returns {'1.00': id}, replace the loop with a direct
assignment to youtube_id_1_0. Also fix the docstring example to use the
canonical 1.00 format, correct the fallback comment to say "lowest speed
entry", and update the test_parse_youtube_invalid docstring to match the
actual return value.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Comment on lines
+865
to
+866
| the source string (legacy OLX with only non-1.0 speeds), the first | ||
| non-empty speed ID is used as a fallback so no YouTube ID is lost. |
| def test_parse_youtube_legacy_fallback(self): | ||
| """ | ||
| Legacy OLX that only specifies non-1.0 speeds should not silently drop | ||
| the YouTube ID — the first non-empty speed should be promoted to 1.00. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes openedx/openedx-platform#36888
Summary
youtube_id_0_75,youtube_id_1_25, andyoutube_id_1_5fields fromVideoFields— YouTube has supported native speed controls for years, making these redundantcreate_youtube_string()invideo_utils.pyto only useyoutube_id_1_0_parse_youtube()to only initialize and return the1.00speed key — the0.75,1.25,1.50entries were dead code after field removaltest_create_youtube_string_missingto test the meaningful empty-ID case instead of duplicating the happy pathOld XML with multi-speed YouTube attributes (e.g.
youtube="0.75:...,1.00:...,1.25:...,1.5:...") will continue to parse without errors — the non-1.0 speeds are silently ignored, and YouTube's native player handles speed control natively.Test plan
xblocks_contrib/video/tests/test_video.pypassesyoutube_id_0_75,youtube_id_1_25, oryoutube_id_1_5references remain in video source filesyoutube_id_1_0set — confirm YouTube speed controls work via the native player🤖 Generated with Claude Code