VideoBlock code Optimization/Correction - #38012
Conversation
- 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
added
the
create-sandbox
| @@ -265,7 +265,7 @@ def public_view(self, context): | |||
|
|
|||
There was a problem hiding this comment.
@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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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:
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:
|
@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
left a comment
salman2013
left a comment
There was a problem hiding this comment.
Based on the provided test results and previous change logs, it looks good to me.
github-project-automation
Bot
moved this from 🏗 In progress
to ✅ Done
in Aximprovements Team
- 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.
- 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.
…kMain-to-VideoBlockDisplay-ulmo.3 chore: optimize/correct the VideoBlock code (openedx#38012)
2 participants
We are going to do these changes for the extracted Video Block resides in
xblocks-contribin 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:
i18nservice declaration fromwantstoneeds, since the runtime must provide it for the block to function correctly.public_viewwebpack JS reference fromVideoBlockMaintoVideoBlockDisplay, as all VideoBlock JS files are bundled intoVideoBlockDisplayandVideoBlockMainis 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.