refactor: Drop the unused legacy video upload page.#37582
refactor: Drop the unused legacy video upload page.#37582kdmccormick merged 2 commits intomasterfrom
Conversation
fdd2b61 to
0e3689c
Compare
fb7625b to
d6dd652
Compare
d6dd652 to
58979c3
Compare
58979c3 to
a9724da
Compare
|
Sandbox deployment failed 💥 |
|
Sandbox deployment failed 💥 |
|
Wow, net -7000, nice 🔥 I didn't even realize there was a legacy video uploads page! Is it this? https://edx.readthedocs.io/projects/edx-partner-course-staff/en/latest/video/upload_video.html Regarding the
I remember us discussing in standup (and hinted in that thread) that someone could maybe work on a version of the MFE page which didn't include any of the edX-specific stuff, and exposed plugin slots which edX could plug their stuff into. But I don't know what that would look like and I'm not sure who would do it. I think our options for the purpose of the legacy studio removal are either |
You're saying keep the cleanup but leave the waffle flag in place so that the new MFE page remains off by default since it's actually useless for non edx.org operators at the moment? If we do that, we have to answer the question of what does the backend do when the waffle flag is set to false and there is no old page to display. I think this means we update the code to not provide links to the video page in this case and so we'll need to update other older templates to put conditional around those references. Does that sound right to you @kdmccormick ? |
Good question, the thing is I'm not actually sure how one gets to the legacy video page, up until now I didn't know it even existed. Do you know how it's accessed today? Is it possible it's already behind some flag? If the new uploads page is fully edX-specific, then I'd guess that the legacy page is edX-specific too. |
a9724da to
3e590ec
Compare
|
Sandbox deployment failed 💥 |
The legacy page is not accessible accessible at all today. It's only accessible from the legacy studio header and the authoring MFE either does or doesn't show the new videos page but has no ability to go to the old videos page. There's also another bug in the MFE where it will show a link to the videos page if the old MFE is enabled, but then only adds the page to the route if a frontend setting is set. So I'm thinking we should remove the old page which is not accessible, but leave the waffle flag for now and leave it at false by default so that the community doesn't get the new videos page which they still can't use. What do you think? |
In the interest of getting legacy studio cleaned up, fully agree Here's an issue where we can track follow-up: #37972 |
60d2128 to
0c5605a
Compare
| 'js/views/video/transcripts/utils', 'js/views/video/transcripts/message_manager', | ||
| 'js/views/video/transcripts/file_uploader', 'sinon', | ||
| 'xmodule' | ||
| 'xmodule', 'accessibility' |
There was a problem hiding this comment.
Note for reviewers: This was added to get the edx object to load correctly. That object was previously being loaded implicitly by some of the other JS being loaded on the same test page but with a bunch of the other video related code removed, this previous side-effect was exposed. Adding this module as an explicit dependency fixed the issue in this test without needing to add back a bunch of the other JS code.
|
fyi, if everything works, a sandbox should get created and linked in the output of the |
|
sandbox failed, but the build logs are actually helpful now. |
|
Weird that the CI build of assets did not catch this. Looks like it doesn't run the production require js optimizer which is what caught the issue. I'll fix the issue but also see if I can update the asset check workflow to fix this. |
0c5605a to
a7d9a20
Compare
a7d9a20 to
ea89278
Compare
| # .. toggle_description: This flag enables the use of the new studio video uploads page MFE. | ||
| # Note: This page only works on edx.org or other sites that have reverse-engineered | ||
| # the edX video pipeline. It is off by default for the community. | ||
| # .. toggle_use_cases: opt_in | ||
| # .. toggle_creation_date: 2023-05-15 | ||
| # .. toggle_tickets: https://github.com/openedx/openedx-platform/issues/37972 |
|
Smoke tested on the sandbox. Confirmed that, by default, there's no videos page. With the waffle flag enabled, the broken (empty) video page is available in Studio under "Content". This is the same as what happened on master before. |
5659fa9 to
167c169
Compare
167c169 to
4b6011e
Compare
The legacy video uploads page in Studio has been replaced with a new view in the Authoring MFE. The legacy page has not been available for some time, so it's all dead code. This PR removes it. Please note that there's a waffle flag which enables the MFE version of the video uploads page: `contentstore.new_studio_mfe.use_new_video_uploads_page`. Unlike the other Studio MFE waffles, we're NOT going to remove this one now, because the video uploads page has always been broken for sites other than edx.org (or sites that have reverse-engineered their video pipeline) so we'd like to keep the flag until it's either fixed for the community or removed (#37972). This work is part of #36108 Co-Authored-By: Kyle McCormick <kyle@axim.org>
4b6011e to
69ddfb2
Compare
|
Reworded the commit since we're not longer removing the waffle flag |
The static-assets-check workflow was setting DJANGO_SETTINGS_MODULE=lms.envs.production globally, but then running both LMS and CMS collectstatic. Because manage.py doesn't override DJANGO_SETTINGS_MODULE when it's already set (and --settings isn't passed), CMS was running with LMS settings instead of CMS settings. This meant CMS-specific RequireJS builds (cms/static/cms/js/build.js) were never being tested, allowing issues like missing modules to slip through to sandbox deployments. Fix by setting DJANGO_SETTINGS_MODULE explicitly for each service. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
69ddfb2 to
341ff6c
Compare

The legacy video uploads page in Studio has been replaced with a new
view in the Authoring MFE. This change removes the now unused
JS/CSS/HTML/Python related to the old video page.
This work is part of #36108
BREAKING CHANGE: The `contentstore.new_studio_mfe.use_new_video_uploads_pagewaffle flag is no longer respected. The code operates as if this is set to True.
EDIT: As discussed below, both the new and old video uploads page only worked on edx.org. So, we are keeping the
contentstore.new_studio_mfe.use_new_video_uploads_pagein place in order to keep the new version flagged-off, and just removing the old version of the page entirely (which has been inaccessible for a while). - Kyle