From 54361366097ea4fbed38d344d88a7d1c269f43cd Mon Sep 17 00:00:00 2001 From: Jansen Kantor Date: Thu, 16 Feb 2023 13:18:45 -0500 Subject: [PATCH] feat: public video metadata + embed (#31753) * feat: public video metadata + embed * refactor: alphebetize template context * feat: don't default show transcript when embed * fix: rename var * fix: remove padding in embed view * style: newline * test: add tests --- lms/djangoapps/courseware/tests/test_views.py | 57 ++++++--- lms/djangoapps/courseware/toggles.py | 11 ++ lms/djangoapps/courseware/views/views.py | 65 +++++++--- lms/templates/public_video.html | 18 +++ lms/templates/public_video_share_embed.html | 116 ++++++++++++++++++ lms/urls.py | 7 ++ xmodule/video_block/video_block.py | 53 ++++---- 7 files changed, 276 insertions(+), 51 deletions(-) create mode 100644 lms/templates/public_video.html create mode 100644 lms/templates/public_video_share_embed.html diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index f8456349fd..188a73f2ef 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -27,6 +27,7 @@ from edx_toggles.toggles.testutils import override_waffle_flag from freezegun import freeze_time from opaque_keys.edx.keys import CourseKey, UsageKey from pytz import UTC +from openedx.core.djangoapps.waffle_utils.models import WaffleFlagCourseOverrideModel from rest_framework import status from web_fragments.fragment import Fragment from xblock.core import XBlock @@ -74,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 +from lms.djangoapps.courseware.toggles import COURSEWARE_OPTIMIZED_RENDER_XBLOCK, PUBLIC_VIDEO_SHARE from lms.djangoapps.courseware.user_state_client import DjangoXBlockUserStateClient from lms.djangoapps.instructor.access import allow_access from lms.djangoapps.verify_student.services import IDVerificationService @@ -2926,11 +2927,13 @@ class TestRenderXBlock(RenderXBlockTestMixin, ModuleStoreTestCase, CompletionWaf self.assertNotContains(response, banner_text, html=True) +@ddt.ddt class TestRenderPublicVideoXBlock(ModuleStoreTestCase): """ Tests for the courseware.render_public_video_xblock endpoint. """ - def setup_course(self): + + def setup_course(self, enable_waffle=True): """ Helper method to create the course. """ @@ -2958,45 +2961,69 @@ class TestRenderPublicVideoXBlock(ModuleStoreTestCase): category='video', display_name='Video with private access' ) + WaffleFlagCourseOverrideModel.objects.create( + waffle_flag=PUBLIC_VIDEO_SHARE.name, + course_id=course.id, + enabled=enable_waffle, + ) CourseOverview.load_from_module_store(course.id) - def get_response(self, usage_key): + def get_response(self, usage_key, is_embed): """ Overridable method to get the response from the endpoint that is being tested. """ - url = reverse('render_public_video_xblock', kwargs={'usage_key_string': str(usage_key)}) + view_name = 'render_public_video_xblock' + if is_embed: + view_name += '_embed' + url = reverse(view_name, kwargs={'usage_key_string': str(usage_key)}) return self.client.get(url) - def test_render_xblock_with_invalid_usage_key(self): + @ddt.data(True, False) + def test_render_xblock_with_invalid_usage_key(self, is_embed): """ Verify that endpoint returns expected response with invalid usage key """ - response = self.get_response(usage_key='some_invalid_usage_key') + response = self.get_response(usage_key='some_invalid_usage_key', is_embed=is_embed) self.assertContains(response, 'Page not found', status_code=404) - def test_render_xblock_with_non_video_usage_key(self): + @ddt.data(True, False) + def test_render_xblock_with_non_video_usage_key(self, is_embed): """ Verify that endpoint returns expected response if usage key block type is not `video` """ self.setup_course() - response = self.get_response(usage_key=self.html_block.location) + response = self.get_response(usage_key=self.html_block.location, is_embed=is_embed) self.assertContains(response, 'Page not found', status_code=404) - def test_render_xblock_with_video_usage_key_with_public_access(self): + @ddt.data(True, False) + def test_render_xblock_with_video_usage_key_with_public_access(self, is_embed): """ - Verify that endpoint returns expected response if usage key block type is `video` and video has public access + Verify that endpoint returns expected response if usage key block type is `video` + and video doesn't have 'public access' set as True """ self.setup_course() - response = self.get_response(usage_key=self.video_block_public.location) + response = self.get_response(usage_key=self.video_block_public.location, is_embed=is_embed) self.assertContains(response, 'Play video', status_code=200) - def test_render_xblock_with_video_usage_key_with_non_public_access(self): + @ddt.data(True, False) + def test_render_xblock_with_video_usage_key_with_non_public_access(self, is_embed): """ - Verify that endpoint returns expected response if usage key block type is `video` and video has private access + Verify that endpoint returns expected response if usage key block type is `video` + and video doesn't have 'public access' set as False """ self.setup_course() - response = self.get_response(usage_key=self.video_block_not_public.location) - self.assertContains(response, 'Page not found', status_code=404) + response = self.get_response(usage_key=self.video_block_not_public.location, is_embed=is_embed) + self.assertContains(response, 'Play video', status_code=200) + + @ddt.data(True, False) + def test_render_xblock_with_video_waffle_not_enabled(self, is_embed): + """ + Verify that endpoint returns expected response if waffle is not enabled for course. + """ + self.setup_course(enable_waffle=False) + for block in (self.video_block_public, self.video_block_not_public): + response = self.get_response(usage_key=block.location, is_embed=is_embed) + self.assertContains(response, 'Page not found', status_code=404) class TestRenderXBlockSelfPaced(TestRenderXBlock): # lint-amnesty, pylint: disable=test-inherits-tests diff --git a/lms/djangoapps/courseware/toggles.py b/lms/djangoapps/courseware/toggles.py index 01d9128bf8..aa2977d414 100644 --- a/lms/djangoapps/courseware/toggles.py +++ b/lms/djangoapps/courseware/toggles.py @@ -96,6 +96,17 @@ 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: Enables public viewing / sharing of all course videos. +# .. 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 dee8c854a6..cb29efb8e4 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -8,7 +8,7 @@ import logging import urllib from collections import OrderedDict, namedtuple from datetime import datetime -from urllib.parse import quote_plus +from urllib.parse import quote_plus, urljoin import bleach import requests @@ -86,7 +86,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 +from lms.djangoapps.courseware.toggles import course_is_invitation_only, PUBLIC_VIDEO_SHARE from lms.djangoapps.courseware.user_state_client import DjangoXBlockUserStateClient from lms.djangoapps.courseware.utils import ( _use_new_financial_assistance_flow, @@ -1613,21 +1613,23 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True): return render_to_response('courseware/courseware-chromeless.html', context) -@require_http_methods(["GET"]) -@ensure_valid_usage_key -@xframe_options_exempt -@transaction.non_atomic_requests -def render_public_video_xblock(request, usage_key_string): +def _render_public_video_xblock(request, usage_key_string, is_embed=False): """ - Returns an HttpResponse with HTML content for the Video xBlock with the given usage_key. - The returned HTML is a chromeless rendering of the Video xBlock (excluding content of the containing courseware). + Look up a given usage key and render the "public" view or the "embed" view """ view = 'public_view' + if is_embed: + template = 'public_video_share_embed.html' + else: + template = 'public_video.html' usage_key = UsageKey.from_string(usage_key_string) usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key)) course_key = usage_key.course_key + if not PUBLIC_VIDEO_SHARE.is_enabled(course_key): + raise Http404("Video not found.") + # usage key block type must be `video` else raise 404 if usage_key.block_type != 'video': raise Http404("Video not found.") @@ -1644,15 +1646,26 @@ def render_public_video_xblock(request, usage_key_string): will_recheck_access=False ) - # video must be public (`Public Access` field set to True) by course author in studio in video advanced settings - if not block.public_access: - raise Http404("Video not found.") + fragment = block.render(view, context={ + 'public_video_embed': is_embed, + }) - fragment = block.render(view, context={}) + video_description = f"Watch a video from the course {course.display_name} " + if course.display_organization is not None: + video_description += f"by {course.display_organization} " + video_description += "on edX.org" context = { 'fragment': fragment, 'course': course, + 'video_title': block.display_name_with_default, + 'video_description': video_description, + 'video_thumbnail': "https://www.edx.org/images/logos/edx-logo-elm.svg", + # 'video_thumbnail': "https://i.ytimg.com/vi/Kauv7MVPcsA/maxresdefault.jpg", + 'video_embed_url': urljoin( + settings.LMS_ROOT_URL, + reverse('render_public_video_xblock_embed', kwargs={'usage_key_string': str(usage_key)}) + ), 'disable_accordion': False, 'allow_iframing': True, 'disable_header': False, @@ -1662,7 +1675,31 @@ def render_public_video_xblock(request, usage_key_string): 'is_learning_mfe': True, 'is_mobile_app': False, } - return render_to_response('courseware/courseware-chromeless.html', context) + return render_to_response(template, context) + + +@require_http_methods(["GET"]) +@ensure_valid_usage_key +@xframe_options_exempt +@transaction.non_atomic_requests +def render_public_video_xblock_embed(request, usage_key_string): + """ + Returns an HttpResponse with HTML content for the Video xBlock with the given usage_key. + The returned HTML consists of nothing but the Video xBlock content for use in social media embedding. + """ + return _render_public_video_xblock(request, usage_key_string, is_embed=True) + + +@require_http_methods(["GET"]) +@ensure_valid_usage_key +@xframe_options_exempt +@transaction.non_atomic_requests +def render_public_video_xblock(request, usage_key_string): + """ + Returns an HttpResponse with HTML content for the Video xBlock with the given usage_key. + The returned HTML is a chromeless rendering of the Video xBlock (excluding content of the containing courseware). + """ + return _render_public_video_xblock(request, usage_key_string, is_embed=False) def get_optimization_flags_for_content(block, fragment): diff --git a/lms/templates/public_video.html b/lms/templates/public_video.html new file mode 100644 index 0000000000..6c7691a9dc --- /dev/null +++ b/lms/templates/public_video.html @@ -0,0 +1,18 @@ +<%page expression_filter="h"/> +<%inherit file="courseware/courseware-chromeless.html"/> + +<%block name="head_extra"> + + + + + + + + + + + + + + diff --git a/lms/templates/public_video_share_embed.html b/lms/templates/public_video_share_embed.html new file mode 100644 index 0000000000..f134c30a33 --- /dev/null +++ b/lms/templates/public_video_share_embed.html @@ -0,0 +1,116 @@ +## coding=utf-8 + +<%page expression_filter="h"/> +<%! main_css = "style-main-v1" %> + +<%namespace name='static' file='static_content.html'/> +<%! +import six +from lms.djangoapps.branding import api as branding_api +from django.utils.translation import gettext as _ +from django.utils.translation import get_language_bidi +from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +from openedx.core.djangolib.js_utils import js_escaped_string +from openedx.core.release import RELEASE_LINE +from openedx.core.djangolib.markup import HTML +%> + +<%def name="course_name()"> + <% return _("{course_number} Courseware").format(course_number=course.display_number_with_default) %> + + + + + + + + + + + <%def name="pagetitle()" /> + ${static.get_page_title_breadcrumbs(course_name())} + + <% + jsi18n_path = "js/i18n/{language}/djangojs.js".format(language=LANGUAGE_CODE) + ie11_fix_path = "js/ie11_find_array.js" + %> + + + <% favicon_url = branding_api.get_favicon_url() %> + + + <%static:css group='style-vendor'/> + % if '/' in self.attr.main_css: + % if get_language_bidi(): + <% + rtl_css_file = self.attr.main_css.replace('.css', '-rtl.css') + %> + + % else: + + % endif + % else: + <%static:css group='${self.attr.main_css}'/> + % endif + + <%static:js group='main_vendor'/> + <%static:js group='application'/> + + <%static:webpack entry="commons"/> + + <%static:css group='style-course-vendor'/> + <%static:css group='style-course'/> + + <%include file="widgets/segment-io.html" /> + + + <% google_site_verification_id = configuration_helpers.get_value('GOOGLE_SITE_VERIFICATION_ID', settings.GOOGLE_SITE_VERIFICATION_ID) %> + % if google_site_verification_id: + + % endif + + + +<% ga_acct = static.get_value("GOOGLE_ANALYTICS_ACCOUNT", settings.GOOGLE_ANALYTICS_ACCOUNT) %> +% if ga_acct: + +% endif + +<% branch_key = static.get_value("BRANCH_IO_KEY", settings.BRANCH_IO_KEY) %> +% if branch_key and not is_from_mobile_app: + +% endif + + + + + +
+ ${HTML(fragment.body_html())} +
+ + <%static:js group='courseware'/> + ${HTML(fragment.foot_html())} + + diff --git a/lms/urls.py b/lms/urls.py index 3a4b7d3e02..3bebbe3c2c 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -54,6 +54,7 @@ from openedx.features.enterprise_support.api import enterprise_enabled RESET_COURSE_DEADLINES_NAME = 'reset_course_deadlines' RENDER_XBLOCK_NAME = 'render_xblock' RENDER_VIDEO_XBLOCK_NAME = 'render_public_video_xblock' +RENDER_VIDEO_XBLOCK_EMBED_NAME = 'render_public_video_xblock_embed' COURSE_PROGRESS_NAME = 'progress' if settings.DEBUG or settings.FEATURES.get('ENABLE_DJANGO_ADMIN_SITE'): @@ -330,6 +331,12 @@ urlpatterns += [ courseware_views.render_public_video_xblock, name=RENDER_VIDEO_XBLOCK_NAME, ), + re_path( + fr'^videos/{settings.USAGE_KEY_PATTERN}/embed$', + courseware_views.render_public_video_xblock_embed, + name=RENDER_VIDEO_XBLOCK_EMBED_NAME, + ), + # xblock Resource URL re_path( diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index 4ef6545322..969ea92dac 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -260,17 +260,19 @@ class VideoBlock( """ Returns a fragment that contains the html for the public view """ - if getattr(self.runtime, 'suppports_state_for_anonymous_users', False): + is_embed = context.get("public_video_embed") + + if not is_embed and getattr(self.runtime, 'suppports_state_for_anonymous_users', False): # The new runtime can support anonymous users as fully as regular users: return self.student_view(context) - fragment = Fragment(self.get_html(view=PUBLIC_VIEW)) + fragment = Fragment(self.get_html(view=PUBLIC_VIEW, context=context)) add_webpack_to_fragment(fragment, 'VideoBlockPreview') shim_xmodule_js(fragment, 'Video') return fragment - def get_html(self, view=STUDENT_VIEW): # lint-amnesty, pylint: disable=arguments-differ, too-many-statements - + def get_html(self, view=STUDENT_VIEW, context=None): # lint-amnesty, pylint: disable=arguments-differ, too-many-statements + context = context or {} track_status = (self.download_track and self.track) transcript_download_format = self.transcript_download_format if not track_status else None sources = [source for source in self.html5_sources if source] @@ -371,13 +373,7 @@ class VideoBlock( settings_service = self.runtime.service(self, 'settings') # lint-amnesty, pylint: disable=unused-variable - poster = None - if edxval_api and self.edx_video_id: - poster = edxval_api.get_course_video_image_url( - course_id=self.scope_ids.usage_id.context_key.for_branch(None), - edx_video_id=self.edx_video_id.strip() - ) - + poster = self._poster() completion_service = self.runtime.service(self, 'completion') if completion_service: completion_enabled = completion_service.completion_tracking_enabled() @@ -396,7 +392,7 @@ class VideoBlock( # true, but now staff or admin have hidden the autoadvance button and the student won't be able to disable # it anymore; therefore we force-disable it in this case (when controls aren't visible). autoadvance_this_video = self.auto_advance and autoadvance_enabled - + is_embed = context.get('public_video_embed', False) metadata = { 'autoAdvance': autoadvance_this_video, # For now, the option "data-autohide-html5" is hard coded. This option @@ -431,7 +427,9 @@ class VideoBlock( 'savedVideoPosition': self.saved_video_position.total_seconds(), # pylint: disable=no-member 'saveStateEnabled': view != PUBLIC_VIEW, 'saveStateUrl': self.ajax_url + '/save_user_state', - 'showCaptions': json.dumps(self.show_captions), + # Despite the setting on the block, don't show transcript by default + # if the video is embedded in social media + 'showCaptions': json.dumps(self.show_captions and not is_embed), 'sources': sources, 'speed': self.speed, 'start': self.start_time.total_seconds(), # pylint: disable=no-member @@ -457,24 +455,24 @@ class VideoBlock( bumperize(self) - context = { + template_context = { 'autoadvance_enabled': autoadvance_enabled, - 'bumper_metadata': json.dumps(self.bumper['metadata']), # pylint: disable=E1101 - 'metadata': json.dumps(OrderedDict(metadata)), - 'poster': json.dumps(get_poster(self)), 'branding_info': branding_info, + 'bumper_metadata': json.dumps(self.bumper['metadata']), # pylint: disable=E1101 'cdn_eval': cdn_eval, 'cdn_exp_group': cdn_exp_group, - 'id': self.location.html_id(), - 'display_name': self.display_name_with_default, - 'handout': self.handout, + 'display_name': None if is_embed else self.display_name_with_default, 'download_video_link': download_video_link, + 'handout': self.handout, + 'id': self.location.html_id(), + 'license': getattr(self, "license", None), + 'metadata': json.dumps(OrderedDict(metadata)), + 'poster': json.dumps(get_poster(self)), 'track': track_url, 'transcript_download_format': transcript_download_format, 'transcript_download_formats_list': self.fields['transcript_download_format'].values, # lint-amnesty, pylint: disable=unsubscriptable-object - 'license': getattr(self, "license", None), } - return self.runtime.service(self, 'mako').render_template('video.html', context) + return self.runtime.service(self, 'mako').render_template('video.html', template_context) def validate(self): """ @@ -1150,3 +1148,14 @@ class VideoBlock( "encoded_videos": encoded_videos, "all_sources": all_sources, } + + def _poster(self): + """ + Helper to get poster info from edxval + """ + if edxval_api and self.edx_video_id: + return edxval_api.get_course_video_image_url( + course_id=self.scope_ids.usage_id.context_key.for_branch(None), + edx_video_id=self.edx_video_id.strip() + ) + return None