From 4de07c25d032ed23e612563555c57135d4691f39 Mon Sep 17 00:00:00 2001 From: jansenk Date: Fri, 21 Apr 2023 16:25:53 -0400 Subject: [PATCH] refactor: video sharing js test: add testing for VideoSocialSharingHandler test: fix context tests refactor: move most link logic to django chore: update python test chore: update linting --- .../courseware/tests/test_sharing_sites.py | 82 +++++++++++++ .../courseware/tests/test_video_mongo.py | 30 +++++ lms/templates/video.html | 91 ++++---------- xmodule/js/fixtures/video_all.html | 18 +++ xmodule/js/karma_runner_webpack.js | 1 + xmodule/js/spec/video/social_share_spec.js | 47 ++++++++ .../js/src/video/036_video_social_sharing.js | 111 ++++++++++++++++++ xmodule/js/src/video/10_main.js | 7 +- xmodule/video_block/sharing_sites.py | 80 +++++++++++++ xmodule/video_block/video_block.py | 8 ++ 10 files changed, 402 insertions(+), 73 deletions(-) create mode 100644 lms/djangoapps/courseware/tests/test_sharing_sites.py create mode 100644 xmodule/js/spec/video/social_share_spec.js create mode 100644 xmodule/js/src/video/036_video_social_sharing.js create mode 100644 xmodule/video_block/sharing_sites.py diff --git a/lms/djangoapps/courseware/tests/test_sharing_sites.py b/lms/djangoapps/courseware/tests/test_sharing_sites.py new file mode 100644 index 0000000000..d2fb0b4554 --- /dev/null +++ b/lms/djangoapps/courseware/tests/test_sharing_sites.py @@ -0,0 +1,82 @@ +""" +tests for the sharing sites +""" + +import ddt +from unittest import TestCase +from unittest.mock import patch +from urllib.parse import parse_qsl +from xmodule.video_block.sharing_sites import ( + sharing_url, + sharing_sites_info_for_video, + SharingSiteConfig, +) + +TEST_SHARING_SITE_NAME = "test_site_name" +TEST_SHARING_SITE_ICON = "test-icon-name" +TEST_URL_PARAM_NAME = "this-url" +TEST_BASE_SHARE_URL = "www.mysite.org/videos" + +TEST_SHARING_SITE_CONFIG = SharingSiteConfig( + name=TEST_SHARING_SITE_NAME, + fa_icon_name=TEST_SHARING_SITE_ICON, + url_param_name=TEST_URL_PARAM_NAME, + base_share_url=TEST_BASE_SHARE_URL, +) + +TEST_SHARING_SITE_CONFIG_WITH_ADDITIONAL_PARAMS = SharingSiteConfig( + name=TEST_SHARING_SITE_NAME, + fa_icon_name=TEST_SHARING_SITE_ICON, + url_param_name=TEST_URL_PARAM_NAME, + base_share_url=TEST_BASE_SHARE_URL, + additional_site_params={'state': 'NY', 'color': 'red'} +) + +TEST_PUBLIC_URL = "http://www.openedx.org/videos/some_video_or_other" + + +@ddt.ddt +class TestSharingSites(TestCase): + """ + Tests for the sharing sites + """ + @ddt.data( + TEST_SHARING_SITE_CONFIG, + TEST_SHARING_SITE_CONFIG_WITH_ADDITIONAL_PARAMS + ) + def test_sharing_url(self, config): + """ + Test that the sharing url is built correctly + """ + base_url, params = sharing_url(TEST_PUBLIC_URL, config).split("?") + self.assertEqual(base_url, config.base_share_url) + decoded_params = dict(parse_qsl(params)) + self.assertEqual(decoded_params[config.url_param_name], TEST_PUBLIC_URL) + if getattr(config, 'additional_site_params', False): + # additional_site_params will be the subset of decoded_params + for key, expected_value in config.additional_site_params.items(): + assert decoded_params[key] == expected_value + self.assertNotIn('additional_site_params', decoded_params) + + def test_sharing_sites_info_for_video(self): + """ + Test that the sharing sites info is built correctly + """ + sharing_site_configs = [ + TEST_SHARING_SITE_CONFIG, + TEST_SHARING_SITE_CONFIG_WITH_ADDITIONAL_PARAMS, + ] + with patch('xmodule.video_block.sharing_sites.ALL_SHARING_SITES', new=sharing_site_configs): + sharing_sites_info = sharing_sites_info_for_video(TEST_PUBLIC_URL) + for expected_config, actual_info in zip(sharing_site_configs, sharing_sites_info): + self.assertDictEqual( + actual_info, + { + 'name': expected_config.name, + 'fa_icon_name': expected_config.fa_icon_name, + 'sharing_url': sharing_url( + TEST_PUBLIC_URL, + expected_config + ) + } + ) diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index 0e9008a099..719ca3f8c2 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -96,6 +96,8 @@ class TestVideoYouTube(TestVideo): # lint-amnesty, pylint: disable=missing-clas 'branding_info': None, 'license': None, 'bumper_metadata': 'null', + 'block_id': str(self.block.location), + 'course_id': str(self.block.location.course_key), 'cdn_eval': False, 'cdn_exp_group': None, 'display_name': 'A Name', @@ -179,6 +181,8 @@ class TestVideoNonYouTube(TestVideo): # pylint: disable=test-inherits-tests 'branding_info': None, 'license': None, 'bumper_metadata': 'null', + 'block_id': str(self.block.location), + 'course_id': str(self.block.location.course_key), 'cdn_eval': False, 'cdn_exp_group': None, 'display_name': 'A Name', @@ -454,6 +458,8 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): 'branding_info': None, 'license': None, 'bumper_metadata': 'null', + 'block_id': str(self.block.location), + 'course_id': str(self.block.location.course_key), 'cdn_eval': False, 'cdn_exp_group': None, 'display_name': 'A Name', @@ -503,6 +509,8 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): track_url if data['expected_track_url'] == 'a_sub_file.srt.sjson' else data['expected_track_url'] ), 'id': self.block.location.html_id(), + 'block_id': str(self.block.location), + 'course_id': str(self.block.location.course_key), 'metadata': json.dumps(metadata) }) @@ -574,6 +582,8 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): 'branding_info': None, 'license': None, 'bumper_metadata': 'null', + 'block_id': str(self.block.location), + 'course_id': str(self.block.location.course_key), 'cdn_eval': False, 'cdn_exp_group': None, 'display_name': 'A Name', @@ -612,6 +622,8 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): }) expected_context.update({ 'id': self.block.location.html_id(), + 'block_id': str(self.block.location), + 'course_id': str(self.block.location.course_key), 'download_video_link': data['result'].get('download_video_link'), 'metadata': json.dumps(expected_context['metadata']) }) @@ -701,6 +713,8 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): 'branding_info': None, 'license': None, 'bumper_metadata': 'null', + 'block_id': str(self.block.location), + 'course_id': str(self.block.location.course_key), 'cdn_eval': False, 'cdn_exp_group': None, 'display_name': 'A Name', @@ -754,6 +768,8 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): }) expected_context.update({ 'id': self.block.location.html_id(), + 'block_id': str(self.block.location), + 'course_id': str(self.block.location.course_key), 'download_video_link': data['result']['download_video_link'], 'metadata': json.dumps(expected_context['metadata']) }) @@ -874,6 +890,8 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): 'branding_info': None, 'license': None, 'bumper_metadata': 'null', + 'block_id': str(self.block.location), + 'course_id': str(self.block.location.course_key), 'cdn_eval': False, 'cdn_exp_group': None, 'display_name': 'A Name', @@ -914,6 +932,8 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): }) expected_context.update({ 'id': self.block.location.html_id(), + 'block_id': str(self.block.location), + 'course_id': str(self.block.location.course_key), 'download_video_link': data['result']['download_video_link'], 'metadata': json.dumps(expected_context['metadata']) }) @@ -987,6 +1007,8 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): }, 'license': None, 'bumper_metadata': 'null', + 'block_id': str(self.block.location), + 'course_id': str(self.block.location.course_key), 'cdn_eval': False, 'cdn_exp_group': None, 'display_name': 'A Name', @@ -1030,6 +1052,8 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): }) expected_context.update({ 'id': self.block.location.html_id(), + 'block_id': str(self.block.location), + 'course_id': str(self.block.location.course_key), 'download_video_link': data['result'].get('download_video_link'), 'metadata': json.dumps(expected_context['metadata']) }) @@ -1134,6 +1158,8 @@ class TestGetHtmlMethod(BaseTestVideoXBlock): }) expected_context.update({ 'id': self.block.location.html_id(), + 'block_id': str(self.block.location), + 'course_id': str(self.block.location.course_key), 'download_video_link': data['result'].get('download_video_link'), 'metadata': json.dumps(expected_context['metadata']) }) @@ -2339,6 +2365,8 @@ class TestVideoWithBumper(TestVideo): # pylint: disable=test-inherits-tests self.get_handler_url('publish_completion', ''), 'is_bumper', 1 ), })), + 'block_id': str(self.block.location), + 'course_id': str(self.block.location.course_key), 'cdn_eval': False, 'cdn_exp_group': None, 'display_name': 'A Name', @@ -2414,6 +2442,8 @@ class TestAutoAdvanceVideo(TestVideo): # lint-amnesty, pylint: disable=test-inh context = { 'autoadvance_enabled': autoadvanceenabled_flag, 'branding_info': None, + 'block_id': str(self.block.location), + 'course_id': str(self.block.location.course_key), 'license': None, 'cdn_eval': False, 'cdn_exp_group': None, diff --git a/lms/templates/video.html b/lms/templates/video.html index f4f4ec9a55..5684976d4a 100644 --- a/lms/templates/video.html +++ b/lms/templates/video.html @@ -17,6 +17,8 @@ from openedx.core.djangolib.js_utils import ( data-bumper-metadata='${bumper_metadata}' data-autoadvance-enabled="${autoadvance_enabled}" data-poster='${poster}' + data-block-id='${block_id}' + data-course-id='${course_id}' tabindex="-1" >
@@ -46,44 +48,29 @@ from openedx.core.djangolib.js_utils import (
- % if (download_video_link or track or handout or branding_info or public_video_url) and not hide_downloads: + % if (download_video_link or track or handout or branding_info or public_sharing_enabled) and not hide_downloads:

${_('Downloads and transcripts')}

- % if download_video_link or public_video_url: + % if download_video_link or public_sharing_enabled:

${_('Video')}

- % if public_video_url: - - ${_('Share on:')} - - - ${_('Share on Twitter')} - - - - ${_('Share on Facebook')} - - - - ${_('Share on LinkedIn')} - + % if sharing_sites_info: + % endif - % if download_video_link and public_video_url: + % if download_video_link and public_sharing_enabled:
% endif % if download_video_link: @@ -154,42 +141,4 @@ from openedx.core.djangolib.js_utils import ( initializeCDNExperiment(); } -% endif; -% if public_video_url: - % endif diff --git a/xmodule/js/fixtures/video_all.html b/xmodule/js/fixtures/video_all.html index 8b168ddbfb..bd16672058 100644 --- a/xmodule/js/fixtures/video_all.html +++ b/xmodule/js/fixtures/video_all.html @@ -5,6 +5,8 @@
@@ -35,6 +37,22 @@

Video

+ Download video file diff --git a/xmodule/js/karma_runner_webpack.js b/xmodule/js/karma_runner_webpack.js index 540a955bb0..c5c4290d60 100644 --- a/xmodule/js/karma_runner_webpack.js +++ b/xmodule/js/karma_runner_webpack.js @@ -68,6 +68,7 @@ import './spec/video/video_speed_control_spec.js'; import './spec/video/video_storage_spec.js'; import './spec/video/video_volume_control_spec.js'; import './spec/time_spec.js'; +import './spec/video/social_share_spec.js'; // overwrite the loaded method and manually start the karma after a delay // Somehow the code initialized in jQuery's onready doesn't get called before karma auto starts diff --git a/xmodule/js/spec/video/social_share_spec.js b/xmodule/js/spec/video/social_share_spec.js new file mode 100644 index 0000000000..e09011b9e1 --- /dev/null +++ b/xmodule/js/spec/video/social_share_spec.js @@ -0,0 +1,47 @@ +(function() { + 'use strict'; + describe('VideoSocialSharingHandler', function() { + var state, spyOpen; + + beforeEach(function() { + state = jasmine.initializePlayer('video_all.html'); + spyOpen = spyOn(window, 'open').and.returnValue(null); + window.analytics = jasmine.createSpyObj('analytics', ['track']) + }); + + afterAll(() => delete window.analytics); + + describe('clicking social share opens the correct URL', function() { + const testCases = [ + { + source: 'twitter', + url: "https://twitter.com/intent/tweet?text=Here's%20a%20fun%20clip%20from%20a%20class%20I'm%20taking%20on%20%40edXonline.%0A%0A&url=" + }, + { source: 'facebook', url: "https://www.facebook.com/sharer/sharer.php?u=" }, + { source: 'linkedin', url: 'https://www.linkedin.com/sharing/share-offsite/?url=' }, + ]; + _.each(testCases, ({ source, url }) => { + it(source, () => { + var siteShareButton = $(`.social-share-link[data-source="${source}"]`); + expect(siteShareButton.length).toEqual(1); + + siteShareButton.trigger('click'); + expect(spyOpen).toHaveBeenCalledWith( + url + `video-share-url%3Futm_source%3D${source}%26utm_medium%3Dsocial%26utm_campaign%3Dsocial-share-exp`, + 'targetWindow', + 'toolbar=no,location=0,status=no,menubar=no,scrollbars=yes,resizable=yes,width=600,height=400' + ); + + expect(window.analytics.track).toHaveBeenCalledWith( + 'edx.social.video.share_button.clicked', + { + source: source, + video_block_id: 'block-v1:coursekey+type@video+block@000000000000000000', + course_id: 'course-v1:someOrg+thisCOurse+runAway', + } + ); + }); + }); + }); + }); +}).call(this); \ No newline at end of file diff --git a/xmodule/js/src/video/036_video_social_sharing.js b/xmodule/js/src/video/036_video_social_sharing.js new file mode 100644 index 0000000000..d73aa768b2 --- /dev/null +++ b/xmodule/js/src/video/036_video_social_sharing.js @@ -0,0 +1,111 @@ +(function(define) { + 'use strict'; + // VideoSocialSharingHandler module. + define( + 'video/036_video_social_sharing.js', ['underscore', 'gettext'], + function(_, gettext) { + var VideoSocialSharingHandler, SocialSharingSite, SimpleSocialSharingSite, facebook, linkedin, twitter; + /** + * Video Social Sharing control module. + * @exports video/036_video_social_sharing.js + * @constructor + * @param {jquery Element} element + * @param {Object} options + */ + VideoSocialSharingHandler = function(element, options) { + if (!(this instanceof VideoSocialSharingHandler)) { + return new VideoSocialSharingHandler(element, options); + } + + _.bindAll(this, 'clickHandler'); + + this.container = element; + + if (this.container.find('.wrapper-downloads .wrapper-social-share')) { + this.initialize(); + } + + return false; + }; + + VideoSocialSharingHandler.prototype = { + // Initializes the module. + initialize: function() { + this.el = this.container.find('.wrapper-social-share'); + this.el.on('click', '.btn-link', this.clickHandler); + this.baseVideoUrl = this.el.data('url'); + this.course_id = this.container.data('courseId'); + this.block_id = this.container.data('blockId') + this.socialSharingSites = this.getSocialSharingSites() + }, + + clickHandler: function(event) { + var self = this; + event.preventDefault(); + var source = $(event.currentTarget).data('source') + var utmQuery = $.param({ + utm_source: source, + utm_medium: 'social', + utm_campaign: 'social-share-exp', + }); + var sharedVideoUrl = encodeURIComponent(self.baseVideoUrl + "?" + utmQuery); + var socialShareSite = self.socialSharingSites[source]; + var socialMediaShareLinkUrl = socialShareSite.generateShareUrl(sharedVideoUrl); + window.open( + socialMediaShareLinkUrl, + 'targetWindow', + 'toolbar=no,location=0,status=no,menubar=no,scrollbars=yes,resizable=yes,width=600,height=400' + ); + self.sendAnalyticsEvent(source); + }, + + getSocialSharingSites: function() { + var socialSharingSites = {}, + socialSharingSitesList = [ + twitter, facebook, linkedin + ]; + + _.each(socialSharingSitesList, function(socialSharingSite) { + socialSharingSites[socialSharingSite.name] = socialSharingSite; + }, this); + + return socialSharingSites; + }, + + sendAnalyticsEvent: function(source) { + window.analytics.track( + 'edx.social.video.share_button.clicked', + { + source: source, + video_block_id: this.container.data('blockId'), + course_id: this.container.data('courseId'), + } + ); + } + }; + + // Define the social sharing sites and how they generate + // a link to their share page. + SocialSharingSite = function(name, generateShareUrl) { + // A social sharing site with a name and a function to generate a share URL + this.name = name; + this.generateShareUrl = generateShareUrl; + }; + SimpleSocialSharingSite = function(name, baseShareUrl) { + // A social sharing site with a url that is a static string with the url appended + this.name = name; + this.generateShareUrl = (url) => baseShareUrl + url; + } + twitter = new SocialSharingSite( + 'twitter', + url => { + var tweetText = encodeURIComponent(gettext("Here's a fun clip from a class I'm taking on @edXonline.\n\n")); + return "https://twitter.com/intent/tweet?text=" + tweetText + "&url=" + url; + } + ); + facebook = new SimpleSocialSharingSite('facebook', 'https://www.facebook.com/sharer/sharer.php?u='); + linkedin = new SimpleSocialSharingSite('linkedin', 'https://www.linkedin.com/sharing/share-offsite/?url='); + + return VideoSocialSharingHandler; + }); +}(RequireJS.define)); diff --git a/xmodule/js/src/video/10_main.js b/xmodule/js/src/video/10_main.js index eacc1ee13c..e1c8753ed2 100644 --- a/xmodule/js/src/video/10_main.js +++ b/xmodule/js/src/video/10_main.js @@ -64,14 +64,15 @@ 'video/09_poster.js', 'video/09_completion.js', 'video/10_commands.js', - 'video/095_video_context_menu.js' + 'video/095_video_context_menu.js', + 'video/036_video_social_sharing.js' ], function( VideoStorage, initialize, FocusGrabber, VideoAccessibleMenu, VideoControl, VideoFullScreen, VideoQualityControl, VideoProgressSlider, VideoVolumeControl, VideoSpeedControl, VideoAutoAdvanceControl, VideoCaption, VideoPlayPlaceholder, VideoPlayPauseControl, VideoPlaySkipControl, VideoSkipControl, VideoBumper, VideoSaveStatePlugin, VideoEventsPlugin, VideoEventsBumperPlugin, VideoPoster, - VideoCompletionHandler, VideoCommands, VideoContextMenu + VideoCompletionHandler, VideoCommands, VideoContextMenu, VideoSocialSharing ) { /* RequireJS */ var youtubeXhr = null, @@ -135,6 +136,8 @@ saveStateUrl: state.metadata.saveStateUrl }); + VideoSocialSharing(el); + if (bumperMetadata) { VideoPoster(el, { poster: el.data('poster'), diff --git a/xmodule/video_block/sharing_sites.py b/xmodule/video_block/sharing_sites.py new file mode 100644 index 0000000000..60265385b5 --- /dev/null +++ b/xmodule/video_block/sharing_sites.py @@ -0,0 +1,80 @@ +""" +Defines the sharing sites for different social media platforms +""" +from collections import namedtuple +from django.utils.translation import gettext_lazy as _ +from urllib.parse import urlencode + +TWITTER_SHARE_MESSAGE = _("Here's a fun clip from a class I'm taking on @edXonline.\n\n") + +SharingSiteConfig = namedtuple( + 'SharingSiteConfig', + [ + 'name', + 'fa_icon_name', + 'url_param_name', + 'base_share_url', + 'additional_site_params' + ], + defaults=[{}] +) + +TWITTER = SharingSiteConfig( + name='twitter', + fa_icon_name='fa-twitter-square', + url_param_name='url', + base_share_url='https://twitter.com/intent/tweet', + additional_site_params={'text': TWITTER_SHARE_MESSAGE} +) + +FACEBOOK = SharingSiteConfig( + name='facebook', + fa_icon_name='fa-facebook-square', + url_param_name='u', + base_share_url='https://www.facebook.com/sharer/sharer.php' +) + +LINKEDIN = SharingSiteConfig( + name='linkedin', + fa_icon_name='fa-linkedin-square', + url_param_name='url', + base_share_url='https://www.linkedin.com/sharing/share-offsite/' +) + +ALL_SHARING_SITES = [ + TWITTER, + FACEBOOK, + LINKEDIN, +] + + +def sharing_sites_info_for_video(video_public_url): + """ + Returns a list of dicts, each containing the name, fa_icon_name, and sharing_url + """ + result = [] + for sharing_site_config in ALL_SHARING_SITES: + sharing_site_info = { + 'name': sharing_site_config.name, + 'fa_icon_name': sharing_site_config.fa_icon_name, + 'sharing_url': sharing_url( + video_public_url, + sharing_site_config + ) + } + result.append(sharing_site_info) + return result + + +def sharing_url(video_public_url, sharing_site_config): + """ + Returns the sharing url with the appropriate parameters + """ + share_params = { + 'utm_source': sharing_site_config.name, + 'utm_medium': 'social', + 'utm_campaign': 'social-share-exp', + sharing_site_config.url_param_name: video_public_url + } + share_params.update(sharing_site_config.additional_site_params) + return sharing_site_config.base_share_url + '?' + urlencode(share_params) diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index 4f3bdac56d..f0ad304d8f 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -59,6 +59,7 @@ from xmodule.x_module import ( from xmodule.xml_block import XmlMixin, deserialize_field, is_pointer_tag, name_to_pathname from .bumper_utils import bumperize +from .sharing_sites import sharing_sites_info_for_video from .transcripts_utils import ( Transcript, VideoTranscriptsMixin, @@ -471,6 +472,8 @@ class VideoBlock( 'handout': self.handout, 'hide_downloads': is_public_view or is_embed, 'id': self.location.html_id(), + 'block_id': str(self.location), + 'course_id': str(self.location.course_key), 'is_embed': is_embed, 'license': getattr(self, "license", None), 'metadata': json.dumps(OrderedDict(metadata)), @@ -479,6 +482,11 @@ class VideoBlock( 'transcript_download_format': transcript_download_format, 'transcript_download_formats_list': self.fields['transcript_download_format'].values, # lint-amnesty, pylint: disable=unsubscriptable-object } + if self.is_public_sharing_enabled(): + public_video_url = self.get_public_video_url() + template_context['public_sharing_enabled'] = True + template_context['public_video_url'] = public_video_url + template_context['sharing_sites_info'] = sharing_sites_info_for_video(public_video_url) # Public video previewing / social media sharing if self.is_public_sharing_enabled():