From b6ba32830a227b8c343a4a4c4dccbfdb72f5867d Mon Sep 17 00:00:00 2001 From: Jansen Kantor Date: Thu, 16 Mar 2023 09:51:05 -0400 Subject: [PATCH] feat: add endpoint to tell if video sharing feature is enabled for a course (#31931) * feat: add endpoint to tell if video sharing feature is enabled for a course * feat: create video setting endpoint * feat: move toggle out of lms * docs: update toggle location in comment * docs: fix toggle annotation --- .../contentstore/views/tests/test_videos.py | 88 +++++++++++++++++-- cms/djangoapps/contentstore/views/videos.py | 20 +++++ cms/urls.py | 5 ++ .../courseware/tests/test_video_mongo.py | 2 +- lms/djangoapps/courseware/tests/test_views.py | 3 +- lms/djangoapps/courseware/toggles.py | 11 --- lms/djangoapps/courseware/views/views.py | 5 +- .../core/djangoapps/video_config/toggles.py | 17 ++++ xmodule/video_block/video_block.py | 2 +- 9 files changed, 131 insertions(+), 22 deletions(-) create mode 100644 openedx/core/djangoapps/video_config/toggles.py diff --git a/cms/djangoapps/contentstore/views/tests/test_videos.py b/cms/djangoapps/contentstore/views/tests/test_videos.py index d07cdb56d3..7d02d667a6 100644 --- a/cms/djangoapps/contentstore/views/tests/test_videos.py +++ b/cms/djangoapps/contentstore/views/tests/test_videos.py @@ -42,6 +42,7 @@ from ..videos import ( ENABLE_VIDEO_UPLOAD_PAGINATION, KEY_EXPIRATION_IN_SECONDS, VIDEO_IMAGE_UPLOAD_ENABLED, + PUBLIC_VIDEO_SHARE, StatusDisplayStrings, TranscriptProvider, _get_default_video_image_url, @@ -159,11 +160,10 @@ class VideoUploadTestBase: ) -class VideoUploadTestMixin(VideoUploadTestBase): +class VideoStudioAccessTestsMixin: """ - Test cases for the video upload feature + Base Access tests for studio video views """ - def test_anon_user(self): self.client.logout() response = self.client.get(self.url) @@ -184,6 +184,11 @@ class VideoUploadTestMixin(VideoUploadTestBase): response = client.get(self.url) self.assertEqual(response.status_code, 403) + +class VideoPipelineStudioAccessTestsMixin: + """ + Access tests for video views that rely on the video pipeline + """ def test_video_pipeline_not_enabled(self): settings.FEATURES["ENABLE_VIDEO_UPLOAD_PIPELINE"] = False self.assertEqual(self.client.get(self.url).status_code, 404) @@ -329,7 +334,13 @@ class VideoUploadPostTestsMixin: @override_settings(VIDEO_UPLOAD_PIPELINE={ "VEM_S3_BUCKET": "vem_test_bucket", "BUCKET": "test_bucket", "ROOT_PATH": "test_root" }) -class VideosHandlerTestCase(VideoUploadTestMixin, VideoUploadPostTestsMixin, CourseTestCase): +class VideosHandlerTestCase( + VideoUploadTestBase, + VideoStudioAccessTestsMixin, + VideoPipelineStudioAccessTestsMixin, + VideoUploadPostTestsMixin, + CourseTestCase +): """Test cases for the main video upload endpoint""" VIEW_NAME = 'videos_handler' @@ -841,7 +852,11 @@ class VideosHandlerTestCase(VideoUploadTestMixin, VideoUploadPostTestsMixin, Cou @override_settings(VIDEO_UPLOAD_PIPELINE={ "VEM_S3_BUCKET": "vem_test_bucket", "BUCKET": "test_bucket", "ROOT_PATH": "test_root" }) -class GenerateVideoUploadLinkTestCase(VideoUploadTestBase, VideoUploadPostTestsMixin, CourseTestCase): +class GenerateVideoUploadLinkTestCase( + VideoUploadTestBase, + VideoUploadPostTestsMixin, + CourseTestCase +): """ Test cases for the main video upload endpoint """ @@ -1472,7 +1487,12 @@ class TranscriptPreferencesTestCase(VideoUploadTestBase, CourseTestCase): @patch.dict("django.conf.settings.FEATURES", {"ENABLE_VIDEO_UPLOAD_PIPELINE": True}) @override_settings(VIDEO_UPLOAD_PIPELINE={"BUCKET": "test_bucket", "ROOT_PATH": "test_root"}) -class VideoUrlsCsvTestCase(VideoUploadTestMixin, CourseTestCase): +class VideoUrlsCsvTestCase( + VideoUploadTestBase, + VideoStudioAccessTestsMixin, + VideoPipelineStudioAccessTestsMixin, + CourseTestCase +): """Test cases for the CSV download endpoint for video uploads""" VIEW_NAME = "video_encodings_download" @@ -1550,3 +1570,59 @@ class VideoUrlsCsvTestCase(VideoUploadTestMixin, CourseTestCase): response["Content-Disposition"], "attachment; filename*=utf-8''n%C3%B3n-%C3%A4scii_video_urls.csv" ) + + +@ddt.ddt +class GetVideoFeaturesTestCase( + VideoStudioAccessTestsMixin, + CourseTestCase +): + """Test cases for the get_video_features endpoint """ + def setUp(self): + super().setUp() + self.url = self.get_url_for_course_key() + + def get_url_for_course_key(self, course_id=None): + """ Helper to generate a url for a course key """ + course_id = course_id or str(self.course.id) + return reverse_course_url("video_features", course_id) + + def test_basic(self): + """ Test for expected return keys """ + response = self.client.get(self.get_url_for_course_key()) + self.assertEqual(response.status_code, 200) + self.assertEqual( + set(response.json().keys()), + { + 'videoSharingEnabled', + 'allowThumbnailUpload', + } + ) + + @ddt.data(True, False) + def test_video_share_enabled(self, is_enabled): + """ Test the public video share flag """ + self._test_video_feature( + PUBLIC_VIDEO_SHARE, + 'videoSharingEnabled', + override_waffle_flag, + is_enabled, + ) + + @ddt.data(True, False) + def test_video_image_upload_enabled(self, is_enabled): + """ Test the video image upload switch """ + self._test_video_feature( + VIDEO_IMAGE_UPLOAD_ENABLED, + 'allowThumbnailUpload', + override_waffle_switch, + is_enabled, + ) + + def _test_video_feature(self, flag, key, override_fn, is_enabled): + """ Test that setting a waffle flag or switch on or off will cause the expected result """ + with override_fn(flag, is_enabled): + response = self.client.get(self.get_url_for_course_key()) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()[key], is_enabled) diff --git a/cms/djangoapps/contentstore/views/videos.py b/cms/djangoapps/contentstore/views/videos.py index 3631ff20d3..3daa65b911 100644 --- a/cms/djangoapps/contentstore/views/videos.py +++ b/cms/djangoapps/contentstore/views/videos.py @@ -45,7 +45,9 @@ from rest_framework.response import Response from common.djangoapps.edxmako.shortcuts import render_to_response from common.djangoapps.util.json_request import JsonResponse, expect_json +from common.djangoapps.util.views import ensure_valid_course_key from openedx.core.djangoapps.video_config.models import VideoTranscriptEnabledFlag +from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE from openedx.core.djangoapps.video_pipeline.config.waffle import ( DEPRECATE_YOUTUBE, ENABLE_DEVSTACK_VIDEO_UPLOADS, @@ -64,6 +66,7 @@ __all__ = [ 'video_encodings_download', 'video_images_handler', 'video_images_upload_enabled', + 'get_video_features', 'transcript_preferences_handler', 'generate_video_upload_link_handler', ] @@ -275,6 +278,23 @@ def video_images_upload_enabled(request): return JsonResponse({'allowThumbnailUpload': True}) +@ensure_valid_course_key +@login_required +@require_GET +def get_video_features(request, course_key_string): + """ Return a dict with info about which video features are enabled """ + course_key = CourseKey.from_string(course_key_string) + course = get_course_and_check_access(course_key, request.user) + if not course: + return HttpResponseNotFound() + + features = { + 'allowThumbnailUpload': VIDEO_IMAGE_UPLOAD_ENABLED.is_enabled(), + 'videoSharingEnabled': PUBLIC_VIDEO_SHARE.is_enabled(course_key), + } + return JsonResponse(features) + + def validate_transcript_preferences(provider, cielo24_fidelity, cielo24_turnaround, three_play_turnaround, video_source_language, preferred_languages): """ diff --git a/cms/urls.py b/cms/urls.py index af851e21d4..e96fdc8c87 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -164,6 +164,11 @@ urlpatterns = oauth2_urlpatterns + [ contentstore_views.video_images_handler, name='video_images_handler'), path('video_images_upload_enabled', contentstore_views.video_images_upload_enabled, name='video_images_upload_enabled'), + re_path( + fr'^video_features/{settings.COURSE_KEY_PATTERN}', + contentstore_views.get_video_features, + name='video_features' + ), re_path(fr'^transcript_preferences/{settings.COURSE_KEY_PATTERN}$', contentstore_views.transcript_preferences_handler, name='transcript_preferences_handler'), re_path(fr'^transcript_credentials/{settings.COURSE_KEY_PATTERN}$', diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index 9abdf022a1..46487197a4 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -48,7 +48,7 @@ from xmodule.x_module import PUBLIC_VIEW, STUDENT_VIEW from common.djangoapps.xblock_django.constants import ATTR_KEY_REQUEST_COUNTRY_CODE from lms.djangoapps.courseware.tests.helpers import get_context_dict_from_string -from lms.djangoapps.courseware.toggles import PUBLIC_VIDEO_SHARE +from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE from openedx.core.djangoapps.video_pipeline.config.waffle import DEPRECATE_YOUTUBE from openedx.core.djangoapps.waffle_utils.models import WaffleFlagCourseOverrideModel from openedx.core.djangolib.testing.utils import CacheIsolationTestCase diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 94105a3f64..943f4e3287 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -75,7 +75,7 @@ from lms.djangoapps.courseware.block_render import get_block, handle_xblock_call from lms.djangoapps.courseware.tests.factories import StudentModuleFactory from lms.djangoapps.courseware.tests.helpers import MasqueradeMixin, get_expiration_banner_text, set_preview_mode from lms.djangoapps.courseware.testutils import RenderXBlockTestMixin -from lms.djangoapps.courseware.toggles import COURSEWARE_OPTIMIZED_RENDER_XBLOCK, PUBLIC_VIDEO_SHARE +from lms.djangoapps.courseware.toggles import COURSEWARE_OPTIMIZED_RENDER_XBLOCK from lms.djangoapps.courseware.user_state_client import DjangoXBlockUserStateClient from lms.djangoapps.courseware.views.views import ( BasePublicVideoXBlockView, @@ -92,6 +92,7 @@ from openedx.core.djangoapps.credit.api import set_credit_requirements from openedx.core.djangoapps.credit.models import CreditCourse, CreditProvider from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES from openedx.core.djangolib.testing.utils import get_mock_request +from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE from openedx.core.lib.url_utils import quote_slashes from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.course_duration_limits.models import CourseDurationLimitConfig diff --git a/lms/djangoapps/courseware/toggles.py b/lms/djangoapps/courseware/toggles.py index b8e097d517..a20b88c373 100644 --- a/lms/djangoapps/courseware/toggles.py +++ b/lms/djangoapps/courseware/toggles.py @@ -96,17 +96,6 @@ COURSEWARE_OPTIMIZED_RENDER_XBLOCK = CourseWaffleFlag( # .. toggle_status: unsupported COURSES_INVITE_ONLY = SettingToggle('COURSES_INVITE_ONLY', default=False) -# .. toggle_name: courseware.public_video_share -# .. toggle_implementation: CourseWaffleFlag -# .. toggle_default: False -# .. toggle_description: Gates access to public videos. This flag must be enabled, and individual -# videos must be marked as "public_access" -# .. toggle_use_cases: temporary -# .. toggle_creation_date: 2023-02-02 -# .. toggle_target_removal_date: None -PUBLIC_VIDEO_SHARE = CourseWaffleFlag( - f'{WAFFLE_FLAG_NAMESPACE}.public_video_share', __name__ -) ENABLE_OPTIMIZELY_IN_COURSEWARE = WaffleSwitch( # lint-amnesty, pylint: disable=toggle-missing-annotation 'RET.enable_optimizely_in_courseware', __name__ diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index ad179548bc..1bf598282a 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -87,7 +87,7 @@ from lms.djangoapps.courseware.masquerade import is_masquerading_as_specific_stu from lms.djangoapps.courseware.model_data import FieldDataCache from lms.djangoapps.courseware.models import BaseStudentModuleHistory, StudentModule from lms.djangoapps.courseware.permissions import MASQUERADE_AS_STUDENT, VIEW_COURSE_HOME, VIEW_COURSEWARE -from lms.djangoapps.courseware.toggles import course_is_invitation_only, PUBLIC_VIDEO_SHARE +from lms.djangoapps.courseware.toggles import course_is_invitation_only from lms.djangoapps.courseware.user_state_client import DjangoXBlockUserStateClient from lms.djangoapps.courseware.utils import ( _use_new_financial_assistance_flow, @@ -115,6 +115,7 @@ from openedx.core.djangoapps.plugin_api.views import EdxFragmentView from openedx.core.djangoapps.programs.utils import ProgramMarketingDataExtender from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.util.user_messages import PageLevelMessages +from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE from openedx.core.djangoapps.zendesk_proxy.utils import create_zendesk_ticket from openedx.core.djangolib.markup import HTML, Text from openedx.core.lib.courses import get_course_by_id @@ -1738,7 +1739,7 @@ class BasePublicVideoXBlockView(View): """ Load course and video from modulestore. Raises 404 if: - - courseware.public_video_share waffle flag is not enabled for this course + - video_config.public_video_share waffle flag is not enabled for this course - block is not video - block is not marked as "public_access" """ diff --git a/openedx/core/djangoapps/video_config/toggles.py b/openedx/core/djangoapps/video_config/toggles.py new file mode 100644 index 0000000000..6552f73664 --- /dev/null +++ b/openedx/core/djangoapps/video_config/toggles.py @@ -0,0 +1,17 @@ +""" +Video config toggles +""" +from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag + +WAFFLE_FLAG_NAMESPACE = 'video_config' + +# .. toggle_name: video_config.public_video_share +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: Gates access to the public social sharing video feature. +# .. toggle_use_cases: temporary, opt_in +# .. toggle_creation_date: 2023-02-02 +# .. toggle_target_removal_date: None +PUBLIC_VIDEO_SHARE = CourseWaffleFlag( + f'{WAFFLE_FLAG_NAMESPACE}.public_video_share', __name__ +) diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index 2d2a4300f9..5ed0824e00 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -495,7 +495,7 @@ class VideoBlock( Returns the public video url """ if self.public_access and self._is_lms_platform(): - from lms.djangoapps.courseware.toggles import PUBLIC_VIDEO_SHARE + from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE if PUBLIC_VIDEO_SHARE.is_enabled(self.location.course_key): return urljoin( settings.LMS_ROOT_URL,