Skip to content

Commit 7c9f468

Browse files
feanilkdmccormick
andcommitted
refactor: Drop the unused legacy video upload page.
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 <[email protected]>
1 parent 0a9f789 commit 7c9f468

45 files changed

Lines changed: 31 additions & 5982 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,12 @@ def get_use_new_files_uploads_page(self, obj):
122122

123123
def get_use_new_video_uploads_page(self, obj):
124124
"""
125-
Method to get the use_new_video_uploads_page switch
125+
Method to get the use_new_video_uploads_page switch.
126+
127+
This is off by default because the video uploads page requires the edX
128+
video pipeline which is not available to the open source community.
129+
130+
See https://github.com/openedx/openedx-platform/issues/37972
126131
"""
127132
course_key = self.get_course_key()
128133
return toggles.use_new_video_uploads_page(course_key)

cms/djangoapps/contentstore/toggles.py

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -259,19 +259,22 @@ def use_new_export_page(course_key):
259259
# .. toggle_name: contentstore.new_studio_mfe.use_new_video_uploads_page
260260
# .. toggle_implementation: CourseWaffleFlag
261261
# .. toggle_default: False
262-
# .. toggle_description: This flag enables the use of the new studio video uploads page mfe
263-
# .. toggle_use_cases: temporary
264-
# .. toggle_creation_date: 2023-5-15
265-
# .. toggle_target_removal_date: 2023-8-31
266-
# .. toggle_tickets: TNL-10619
267-
# .. toggle_warning:
262+
# .. toggle_description: This flag enables the use of the new studio video uploads page MFE.
263+
# Note: This page only works on edx.org or other sites that have reverse-engineered
264+
# the edX video pipeline. It is off by default for the community.
265+
# .. toggle_use_cases: opt_in
266+
# .. toggle_creation_date: 2023-05-15
267+
# .. toggle_tickets: https://github.com/openedx/openedx-platform/issues/37972
268268
ENABLE_NEW_STUDIO_VIDEO_UPLOADS_PAGE = CourseWaffleFlag(
269269
f'{CONTENTSTORE_NAMESPACE}.new_studio_mfe.use_new_video_uploads_page', __name__)
270270

271271

272272
def use_new_video_uploads_page(course_key):
273273
"""
274-
Returns a boolean if new studio video uploads mfe is enabled
274+
Returns a boolean if new studio video uploads MFE is enabled.
275+
276+
This is off by default because the video uploads page requires the edX
277+
video pipeline which is not available to the open source community.
275278
"""
276279
return ENABLE_NEW_STUDIO_VIDEO_UPLOADS_PAGE.is_enabled(course_key)
277280

cms/djangoapps/contentstore/utils.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@
5151
use_new_import_page,
5252
use_new_schedule_details_page,
5353
use_new_unit_page,
54-
use_new_video_uploads_page,
5554
)
5655
from cms.djangoapps.models.settings.course_grading import CourseGradingModel
5756
from cms.djangoapps.models.settings.course_metadata import CourseMetadata
@@ -425,11 +424,10 @@ def get_video_uploads_url(course_locator) -> str:
425424
Gets course authoring microfrontend URL for files and uploads page view.
426425
"""
427426
video_uploads_url = None
428-
if use_new_video_uploads_page(course_locator):
429-
mfe_base_url = get_course_authoring_url(course_locator)
430-
course_mfe_url = f'{mfe_base_url}/course/{course_locator}/videos/'
431-
if mfe_base_url:
432-
video_uploads_url = course_mfe_url
427+
mfe_base_url = get_course_authoring_url(course_locator)
428+
course_mfe_url = f'{mfe_base_url}/course/{course_locator}/videos/'
429+
if mfe_base_url:
430+
video_uploads_url = course_mfe_url
433431
return video_uploads_url
434432

435433

cms/djangoapps/contentstore/video_storage_handlers.py

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050
from tempfile import NamedTemporaryFile, mkdtemp
5151
from wsgiref.util import FileWrapper
5252

53-
from common.djangoapps.edxmako.shortcuts import render_to_response
5453
from common.djangoapps.util.json_request import JsonResponse
5554
from openedx.core.djangoapps.video_config.models import VideoTranscriptEnabledFlag
5655
from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE
@@ -62,8 +61,8 @@
6261
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
6362

6463
from .models import VideoUploadConfig
65-
from .toggles import use_new_video_uploads_page, use_mock_video_uploads
66-
from .utils import get_video_uploads_url, get_course_videos_context
64+
from .toggles import use_mock_video_uploads
65+
from .utils import get_video_uploads_url
6766
from .video_utils import validate_video_image
6867
from .views.course import get_course_and_check_access
6968

@@ -740,13 +739,7 @@ def videos_index_html(course, pagination_conf=None):
740739
"""
741740
Returns an HTML page to display previous video uploads and allow new ones
742741
"""
743-
if use_new_video_uploads_page(course.id):
744-
return redirect(get_video_uploads_url(course.id))
745-
context = get_course_videos_context(
746-
course,
747-
pagination_conf,
748-
)
749-
return render_to_response('videos_index.html', context)
742+
return redirect(get_video_uploads_url(course.id))
750743

751744

752745
def videos_index_json(course):

cms/djangoapps/contentstore/views/tests/test_videos.py

Lines changed: 5 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,10 @@
4343
from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order
4444

4545
from ..videos import (
46-
ENABLE_VIDEO_UPLOAD_PAGINATION,
4746
KEY_EXPIRATION_IN_SECONDS,
4847
VIDEO_IMAGE_UPLOAD_ENABLED,
4948
)
5049
from cms.djangoapps.contentstore.video_storage_handlers import (
51-
_get_default_video_image_url,
5250
TranscriptProvider,
5351
StatusDisplayStrings,
5452
convert_video_status,
@@ -478,24 +476,14 @@ def test_get_json_transcripts(self, expected_video_keys, uploaded_transcripts, e
478476
if response_video['edx_video_id'] == self.previous_uploads[0]['edx_video_id']:
479477
self.assertEqual(response_video.get('transcripts', []), expected_transcripts)
480478

481-
def test_get_html(self):
482-
response = self.client.get(self.url)
483-
self.assertEqual(response.status_code, 200)
484-
self.assertRegex(response["Content-Type"], "^text/html(;.*)?$")
485-
self.assertContains(response, _get_default_video_image_url())
486-
# Crude check for presence of data in returned HTML
487-
for video in self.previous_uploads:
488-
self.assertContains(response, video["edx_video_id"])
489-
self.assertNotContains(response, 'video_upload_pagination')
490-
491-
@override_waffle_flag(ENABLE_VIDEO_UPLOAD_PAGINATION, active=True)
492-
def test_get_html_paginated(self):
479+
def test_get_redirects_to_video_uploads_url(self):
493480
"""
494-
Tests that response is paginated.
481+
Test that GET requests redirect to the MFE video uploads page.
495482
"""
483+
from cms.djangoapps.contentstore.utils import get_video_uploads_url
496484
response = self.client.get(self.url)
497-
self.assertEqual(response.status_code, 200)
498-
self.assertContains(response, 'video_upload_pagination')
485+
self.assertEqual(response.status_code, 302)
486+
self.assertEqual(response.url, get_video_uploads_url(self.course.id))
499487

500488
@override_settings(AWS_ACCESS_KEY_ID="test_key_id", AWS_SECRET_ACCESS_KEY="test_secret")
501489
@patch("cms.djangoapps.contentstore.video_storage_handlers.boto3.resource")
@@ -869,23 +857,6 @@ def test_video_transcript_status_conversion(self, course_video_upload_token, exp
869857

870858
self.assert_video_status(url, edx_video_id, expected_video_status_text)
871859

872-
@ddt.data(True, False)
873-
@patch('openedx.core.djangoapps.video_config.models.VideoTranscriptEnabledFlag.feature_enabled')
874-
def test_video_index_transcript_feature_enablement(self, is_video_transcript_enabled, video_transcript_feature):
875-
"""
876-
Test that when video transcript is enabled/disabled, correct response is rendered.
877-
"""
878-
video_transcript_feature.return_value = is_video_transcript_enabled
879-
response = self.client.get(self.url)
880-
self.assertEqual(response.status_code, 200)
881-
882-
# Verify that course video button is present in the response if videos transcript feature is enabled.
883-
button_html = '<button class="button course-video-settings-button">'
884-
if is_video_transcript_enabled:
885-
self.assertContains(response, button_html)
886-
else:
887-
self.assertNotContains(response, button_html)
888-
889860

890861
@ddt.ddt
891862
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_VIDEO_UPLOAD_PIPELINE": True})

cms/static/cms/js/build.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@
2828
'js/factories/outline',
2929
'js/factories/settings',
3030
'js/factories/settings_advanced',
31-
'js/factories/settings_graders',
32-
'js/factories/videos_index'
31+
'js/factories/settings_graders'
3332
]),
3433
/**
3534
* By default all the configuration for optimization happens from the command

cms/static/cms/js/spec/main.js

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -248,12 +248,6 @@
248248
'js/spec/utils/drag_and_drop_spec',
249249
'js/spec/utils/handle_iframe_binding_spec',
250250
'js/spec/utils/module_spec',
251-
'js/spec/views/active_video_upload_list_spec',
252-
'js/spec/views/previous_video_upload_spec',
253-
'js/spec/views/video_thumbnail_spec',
254-
'js/spec/views/course_video_settings_spec',
255-
'js/spec/views/video_transcripts_spec',
256-
'js/spec/views/previous_video_upload_list_spec',
257251
'js/spec/views/assets_spec',
258252
'js/spec/views/baseview_spec',
259253
'js/spec/views/paged_container_spec',

cms/static/js/factories/videos_index.js

Lines changed: 0 additions & 81 deletions
This file was deleted.

cms/static/js/models/active_video_upload.js

Lines changed: 0 additions & 38 deletions
This file was deleted.

cms/static/js/spec/video/transcripts/message_manager_spec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ define(
33
'jquery', 'underscore', 'backbone',
44
'js/views/video/transcripts/utils', 'js/views/video/transcripts/message_manager',
55
'js/views/video/transcripts/file_uploader', 'sinon',
6-
'xmodule'
6+
'xmodule', 'accessibility'
77
],
88
function($, _, Backbone, Utils, MessageManager, FileUploader, sinon) {
99
'use strict';

0 commit comments

Comments
 (0)