Skip to content

VideoBlock code Optimization/Correction - #38012

Merged
farhan merged 1 commit into
masterfrom
farhan/video-code-optimization
Feb 17, 2026
Merged

VideoBlock code Optimization/Correction#38012
farhan merged 1 commit into
masterfrom
farhan/video-code-optimization

Conversation

@farhan

@farhan farhan commented Feb 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

We are going to do these changes for the extracted Video Block resides in xblocks-contrib in this PR.

Aximprovements team has decided we should directly do the changes within edx-platform for the code correction and it will get early test/response by the community.

Relevant old PR of edx-platform

Details:

  • Change the i18n service declaration from wants to needs, since the runtime must provide it for the block to function correctly.
  • Update the public_view webpack JS reference from VideoBlockMain to VideoBlockDisplay, as all VideoBlock JS files are bundled into VideoBlockDisplay and VideoBlockMain is not referring to anything or no longer exists in the repository.

Testing/Screenshots:

I have verified on Preview/Live Version/LMS/Studio, video block in redering fine on sandbox and local setup.
Here are the screenshots.

Screenshot 2026-02-17 at 11 46 53 AM
screencapture-apps-local-openedx-io-2001-authoring-course-course-v1-axim-1-1-container-block-v1-axim-1-1-type-vertical-block-vertical3-block-v1-axim-1-1-type-sequential-block-e90f308c5b314389899787459ced3361-2026-02-17-12_08_14
screencapture-apps-local-openedx-io-2000-learning-course-course-v1-axim-1-1-block-v1-axim-1-1-type-sequential-block-e90f308c5b314389899787459ced3361-block-v1-axim-1-1-type-vertical-block-vertical3-2026-02-17-12_08_08

@farhan
chore: optimize VideoBlock code
645efce 
- Change the `i18n` service declaration from `wants` to `needs`, since the
  runtime must provide it for the block to function correctly.

- Update the `public_view` webpack JS reference from `VideoBlockMain` to
  `VideoBlockDisplay`, as all VideoBlock JS files are bundled into
  `VideoBlockDisplay` and `VideoBlockMain` is not referring to anything
   or no longer exists in the repository.
@farhan farhan added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Feb 16, 2026
@farhan
farhan marked this pull request as ready for review February 17, 2026 07:10
salman2013
salman2013 reviewed Feb 17, 2026
View reviewed changes
Comment thread xmodule/video_block/video_block.py
@@ -265,7 +265,7 @@ def public_view(self, context):

@salman2013 salman2013 Feb 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@farhan public_view actually return the view seen by users who aren't logged in or who aren't enrolled in the course.(anonymous users)

Can you figure out how to test this because the testing results you shared are for student_view

I found this doc how to enable public course
https://docs.openedx.org/en/latest/site_ops/how-tos/enable_public_course_content.html

@farhan farhan Feb 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tested this on both the sandbox and my local setup (tutor dev), but I wasn’t able to load the Video block in incognito mode as a logged-out user.

@kdmccormick, do you know whether the Video public view access feature is currently working?

@farhan farhan Feb 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Seems feature is breaking while shifting to the mfes.
I managed to fix it via vibe coding/cursor and created test PR with the generated code.

After the fix, here is the preview:

Screenshot

With vibed fixing code and reverting line from
add_webpack_js_to_fragment(fragment, 'VideoBlockDisplay')
to
add_webpack_js_to_fragment(fragment, 'VideoBlockMain')
for the testing of the issue, here is the screen shot:

Screenshot

@salman2013 salman2013 Feb 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@farhan I don't think we need this fix PR, as i found where this was changed previously Ref I believe VideoBlockMain did not exist earlier.

@farhan
farhan requested a review from salman2013 February 17, 2026 15:19
@farhan farhan moved this to 🏗 In progress in Aximprovements Team Feb 17, 2026
@salman2013

salman2013 commented Feb 17, 2026

Copy link
Copy Markdown
Contributor

@farhan I found where this was changed previously, It was updated in the PR Migrating from RequireJS to ES6 Modules https://github.com/openedx/openedx-platform/pull/36595/changes#diff-0c437f410aa8b98a76aa91ccd72e2b9c2da8c49f09168fa66ed63861529eebaf

salman2013
salman2013 approved these changes Feb 17, 2026
View reviewed changes

@salman2013 salman2013 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Based on the provided test results and previous change logs, it looks good to me.

@farhan
farhan merged commit 8dd99de into master Feb 17, 2026
70 checks passed
@github-project-automation github-project-automation Bot moved this from 🏗 In progress to ✅ Done in Aximprovements Team Feb 17, 2026
@farhan
farhan deleted the farhan/video-code-optimization branch February 17, 2026 16:28
michaelroytman pushed a commit to edx/edx-platform that referenced this pull request Jun 18, 2026
@farhan @michaelroytman
chore: optimize/correct the VideoBlock code (openedx#38012)
9e03ad7 
- Change the `i18n` service declaration from `wants` to `needs`, since the
  runtime must provide it for the block to function correctly.

- Update the `public_view` webpack JS reference from `VideoBlockMain` to
  `VideoBlockDisplay`, as all VideoBlock JS files are bundled into
  `VideoBlockDisplay` and `VideoBlockMain` is not referring to anything
   or no longer exists in the repository.
michaelroytman pushed a commit to edx/edx-platform that referenced this pull request Jun 18, 2026
@farhan @michaelroytman
chore: optimize/correct the VideoBlock code (openedx#38012)
9f9ac2b 
- Change the `i18n` service declaration from `wants` to `needs`, since the
  runtime must provide it for the block to function correctly.

- Update the `public_view` webpack JS reference from `VideoBlockMain` to
  `VideoBlockDisplay`, as all VideoBlock JS files are bundled into
  `VideoBlockDisplay` and `VideoBlockMain` is not referring to anything
   or no longer exists in the repository.
michaelroytman added a commit to edx/edx-platform that referenced this pull request Jun 23, 2026
@michaelroytman
Merge pull request #361 from edx/michaelroytman/cherry-pick-VideoBloc…
ca0ff6d 
…kMain-to-VideoBlockDisplay-ulmo.3

chore: optimize/correct the VideoBlock code (openedx#38012)
obscherler pushed a commit to epfl-cede/edx-platform that referenced this pull request Sep 23, 2026
@farhan @obscherler
chore: optimize/correct the VideoBlock code (openedx#38012)
dea1d85 
- Change the `i18n` service declaration from `wants` to `needs`, since the
  runtime must provide it for the block to function correctly.

- Update the `public_view` webpack JS reference from `VideoBlockMain` to
  `VideoBlockDisplay`, as all VideoBlock JS files are bundled into
  `VideoBlockDisplay` and `VideoBlockMain` is not referring to anything
   or no longer exists in the repository.
robrap pushed a commit to edx/edx-platform that referenced this pull request Oct 9, 2026
@farhan @robrap
chore: optimize/correct the VideoBlock code (openedx#38012)
e123921 
- Change the `i18n` service declaration from `wants` to `needs`, since the
  runtime must provide it for the block to function correctly.

- Update the `public_view` webpack JS reference from `VideoBlockMain` to
  `VideoBlockDisplay`, as all VideoBlock JS files are bundled into
  `VideoBlockDisplay` and `VideoBlockMain` is not referring to anything
   or no longer exists in the repository.

(cherry picked from commit 94963db)
vgulati-apphelix pushed a commit to edx/edx-platform that referenced this pull request Nov 2, 2026
@robrap @claude @farhan
feat: land ulmo.1 (Django 5.2) on release-ulmo, batch 1 [LP-1148] (#514)
8ece19b 
* docs: add LP-1148 ulmo upgrade plan (batch 1 = ulmo.1) and handoff

Move the LP-1148 plan and merge audit here from the superseded reference
PR #505 and rewrite them for the batched approach: batch 1 is ulmo.1, the
only upstream point that has run in prod, built as a revert of the
revert (3a8fdad) on the release-ulmo tip; later batches merge the
upstream ulmo tags straight into release-ulmo.

The plan records the deploy/revert history, the decisions, how to
cross-check against edx/ulmo.1, edx/ulmo.3 and #505, the risk list for
the post-ulmo.1 upstream changes, and the next step (the merge work), so
a fresh session can continue from the docs alone.

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: link batch-1 PR #514 and list pending housekeeping in LP-1148 plan

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: record what is verified and unverified in LP-1148 plan; explain soft freeze

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: record #505 closure and repointed references in LP-1148 plan

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: restructure LP-1148 plan around QA ticket, prod deploy evidence and next steps

Group review and testing notes into one QA section (LP-1351), add a Jira
tickets section, record that batch 2 scope is open, and drop the soft
freeze. Record what the edx-internal deploy history shows was in prod and
when, add a how-to for checking it, and note the unreconciled May 27
timeline. Next step is the migration review, before the branch work.

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: record that the database is Aurora MySQL in LP-1148 plan

Drop the open SELECT VERSION() check; the MariaDB UUID migrations are
no-ops.

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: record code-side migration review findings in LP-1148 plan

Add the openedx-authz/casbin and openedx-learning migrations that ulmo.1
brings through dependency upgrades, note that openedx_authz 0006 cannot be
reversed, extend the prod queries, and add a library-permissions test item.

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: add migration review SQL and v2 library finding to LP-1148 plan

The authz data migration only copies content_libraries (v2) permission
rows; record the prod queries that decide whether it has any data.

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: record prod and stage migration query results in LP-1148 plan

Prod kept the casbin/authz/submissions migrations from the May deploy and
lost the reversible ones; contentstore 0014 looks low-risk there. Note
that information_schema row estimates are unreliable.

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: record migration review conclusions in LP-1148 plan

Prod, edge and stage migration state is known and needs no manual steps;
the normal deploy applies what is missing. Replace the interim results
with a per-environment state table, note how migrations and rollbacks run,
the stage schema-ahead-of-code mismatch, and the openedx-authz version
check for the batch-1 audit.

Co-Authored-By: Claude <noreply@anthropic.com>

* revert: reapply ulmo.1 merge into release-ulmo (revert of 3a8fdad)

Batch 1 of the ulmo upgrade [LP-1148]: bring back upstream release/ulmo.1
(Django 5.2) by reverting the May 27 revert, on the current release-ulmo.

Conflict resolutions (13 files):
- .github/workflows/check-for-tutorial-prs.yml: keep deleted (#507).
- create_user_gdpr_testing.py: keep deleted (fork removal, ENT-11576).
- xmodule/js/src/video/02_html5_hls_video.js: accept upstream's move to
  xmodule/assets/video/public/js/; the fork's HLS retry change is ported
  in a separate commit.
- contentstore/views/course.py: keep #509's LP-1102 staff branch and toggle
  import, then upstream's set-based org logic; drop the removed
  use_new_home_page / use_new_course_outline_page imports.
- modulestore_migrator/models.py: keep the fork's `.. no_pii:` annotations
  (#352).
- test_content_libraries.py: import both CourseKey (#509) and
  LibraryCollectionLocator (ulmo.1).
- user_api accounts test_api.py: keep #517's timezone/pytz imports and
  ulmo.1's ZoneInfo.
- course_duration_limits test_access.py: keep the fork's RequestCache
  clearing, with ulmo.1's ZoneInfo.
- requirements/common_constraints.txt: keep the fork's pip<26.2.1 and
  social-auth constraints (supersede ulmo.1's pip<25.3).
- requirements/edx/*.txt: lti-consumer-xblock 9.14.3 (ulmo.1), ora2 6.17.2
  (fork; it no longer needs loremipsum).

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: port LP-1205 audio description flag removal to migrated video JS

#461 (353a5da) removed the enable_audio_description waffle flag and
the `audioDescriptionEnabled` player metadata, and dropped the matching
gating from xmodule/js/src/video/09_video_audio_description.js. Upstream
ulmo moved the video JS to xmodule/assets/video/public/js/, so on this
branch the JS change merged cleanly into the stale old-path copy while the
live copy kept gating on the now-missing `audioDescriptionEnabled`,
which would hide audio description entirely.

Apply the same change to the live file.

Co-Authored-By: Claude <noreply@anthropic.com>
(cherry picked from commit 6732d6f)

* fix: port HLS fragment retry limit to migrated video JS

Upstream ulmo moved the video JS from xmodule/js/src/video/ to
xmodule/assets/video/public/js/. The fork's HLS fragment retry limit
(fb8bc85, 43300f9) was on the old path. The merge commit
fef5916 describes this port, but the change was left out of that
commit; this adds it.

Also fix the merged spec test, which referenced `Hls` while the migrated
spec imports `HLS`.

Co-Authored-By: Claude <noreply@anthropic.com>
(cherry picked from commit 687065c)

* fix: port video language menu sizing to migrated video JS

The fork's #215 (8fb361b) capped the video language selection menu
height. Its CSS part merged into VideoBlockDisplay.css, but its JS part
was on xmodule/js/src/video/09_video_caption.js, which upstream ulmo
moved to xmodule/assets/video/public/js/. Apply it to the live file.

Co-Authored-By: Claude <noreply@anthropic.com>
(cherry picked from commit c56841c)

* fix: drop studio-frontend translations pull from Makefile

Upstream removed studio-frontend (28ab2ce), and this branch no longer
depends on it, but the merge kept the studio-frontend source in
`pull_translations`. Remove it; `$(ATLAS_EXTRA_SOURCES)` stays as the
continuation line.

Co-Authored-By: Claude <noreply@anthropic.com>
(cherry picked from commit 91b8e72)

* feat: Upgrade Python dependency Django to 5.2.18

Security release. Same six pin lines that the upgrade-one-python-dependency
workflow changed for #505 (d408f09); patch releases change no
dependencies. Kept as its own commit so it is easy to drop.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: add oel_publishing.PublishableEntityVersionDependency to PII safelist

openedx-learning 0.30.2 (in ulmo.1) adds this model, whose docstring has
'.. no_pii' without the colon, so pii_check coverage falls below 100%.
release-ulmo lists it only under the old openedx_content label. Same fix as
feac70e on #505.

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: add batch-1 merge audit and record merge results in LP-1148 plan

The revert of the revert conflicted in 13 files, not many. The PII safelist
fix is needed in batch 1 because openedx-learning 0.30.2 is in ulmo.1.
Record the audit and cross-check results, the hand-edited Django bump,
and that #507 has landed.

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: add LP-1102 QA item and merge-method note to LP-1148 plan

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: update commit SHAs in LP-1148 plan and audit after reword

The revert-of-revert commit was reworded to a conventional-commit subject,
which changed it and the commits above it.

Co-Authored-By: Claude <noreply@anthropic.com>

* chore: optimize/correct the VideoBlock code (openedx#38012)

- Change the `i18n` service declaration from `wants` to `needs`, since the
  runtime must provide it for the block to function correctly.

- Update the `public_view` webpack JS reference from `VideoBlockMain` to
  `VideoBlockDisplay`, as all VideoBlock JS files are bundled into
  `VideoBlockDisplay` and `VideoBlockMain` is not referring to anything
   or no longer exists in the repository.

(cherry picked from commit 94963db)

* docs: record ulmo.3 cross-check and video regression fix; drop stage-snapshot migration check

The edx/ulmo.3 cross-check found a ulmo.1 regression (video public_view
bundle) that upstream fixed only on master and verawood; batch 1 takes the
fix. The manual show_unapplied_migrations run against a stage snapshot is
redundant with the known code and database state; review the deploy's own
migration_plan.yml instead. Add a public-video QA item.

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: record ulmo.1-regression sweep and release-notes results in LP-1148 plan

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: defer content-libraries-v2 fixes to the playbook in LP-1148 plan

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: fix CI status heading in LP-1148 batch-1 audit

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: record LP-1148 completion, CI rerun and commitlint notes in plan

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: record 2026-10-14 release-ulmo sync in LP-1148 plan

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: track LP-1148 CI results in the sync log

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: record 2026-10-19 release-ulmo sync and requirements conflict guidance

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: note LP-1391 covers the forum advisory in the LP-1148 sync log

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: read CI live instead of recording it in the LP-1148 sync log

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: mention the GitHub UI for reading LP-1148 CI status

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: record 2026-10-30 release-ulmo sync and the LP-1308 rollback plan

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: make the LP-1148 playbook step a final step of every batch

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: link the LP-1308 deploy dashboard and its 5xx normal range

Co-Authored-By: Claude <noreply@anthropic.com>

* build: pin JS tests to ubuntu-24.04 until LP-1451

On ubuntu-26.04 (ubuntu-latest from 2026-10-19), 22 video player specs in
the xmodule webpack suite time out waiting for playback. The same suite
passes on ubuntu-24.04. Pin runs-on until LP-1451 fixes the specs or the
Firefox setup. matrix.os stays ubuntu-latest, because release-ulmo
requires the check named "JS (ubuntu-latest, 24, 3.11)".

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: record the LP-1308 no-maintenance-banner decision in the plan

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: note the old BOMS-621 dashboard is replaced in the LP-1148 plan

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: drop the old dashboard from the LP-1148 deploy plan and add Django 5.2.19

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: put the stage QA scope at the top of the LP-1148 deploy section

Co-Authored-By: Claude <noreply@anthropic.com>

* docs: leave signal details to the LP-1308 dashboard, keep the rollback procedure

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Muhammad Farhan Khan <farhan.khan@arbisoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@salman2013 salman2013 salman2013 approved these changes
@irtazaakram irtazaakram Awaiting requested review from irtazaakram irtazaakram is a code owner

Assignees

No one assigned

Labels

create-sandbox open-craft-grove should create a sandbox environment from this PR

Projects

Aximprovements Team
Status: ✅ Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@farhan @salman2013