From 1839bc01c6e48af4b53f4444c6244a8f6d21cf52 Mon Sep 17 00:00:00 2001 From: nsprenkle Date: Wed, 17 May 2023 11:30:20 -0400 Subject: [PATCH] feat: update text for sharing to Twitter Updated logic to include organization info, when available. Also refactored away some no-longer-relevant code and pulled Twitter handle from config. style: add missing newline fix: fix outdated signature in test refactor: make organization optional arg Required to fix some tests style: fix pylint issues chore: organization is a dict change the accessor + linting --- .../courseware/tests/test_sharing_sites.py | 2 +- xmodule/video_block/sharing_sites.py | 51 ++++++++++++++++--- xmodule/video_block/video_block.py | 8 ++- 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_sharing_sites.py b/lms/djangoapps/courseware/tests/test_sharing_sites.py index d2fb0b4554..15fcddd584 100644 --- a/lms/djangoapps/courseware/tests/test_sharing_sites.py +++ b/lms/djangoapps/courseware/tests/test_sharing_sites.py @@ -67,7 +67,7 @@ class TestSharingSites(TestCase): 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) + sharing_sites_info = sharing_sites_info_for_video(TEST_PUBLIC_URL, organization=None) for expected_config, actual_info in zip(sharing_site_configs, sharing_sites_info): self.assertDictEqual( actual_info, diff --git a/xmodule/video_block/sharing_sites.py b/xmodule/video_block/sharing_sites.py index 60265385b5..189c316760 100644 --- a/xmodule/video_block/sharing_sites.py +++ b/xmodule/video_block/sharing_sites.py @@ -2,11 +2,10 @@ Defines the sharing sites for different social media platforms """ from collections import namedtuple +from django.conf import settings 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', [ @@ -24,7 +23,6 @@ TWITTER = SharingSiteConfig( 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( @@ -48,7 +46,7 @@ ALL_SHARING_SITES = [ ] -def sharing_sites_info_for_video(video_public_url): +def sharing_sites_info_for_video(video_public_url, organization=None): """ Returns a list of dicts, each containing the name, fa_icon_name, and sharing_url """ @@ -59,14 +57,43 @@ def sharing_sites_info_for_video(video_public_url): 'fa_icon_name': sharing_site_config.fa_icon_name, 'sharing_url': sharing_url( video_public_url, - sharing_site_config - ) + sharing_site_config, + organization=organization + ), } result.append(sharing_site_info) return result -def sharing_url(video_public_url, sharing_site_config): +def get_share_text(social_account_handle, organization_name): + """ + Generate the text we will pre-populate when sharing a post to social media. + + NOTE: Most of the time, we will have all info of these, but have provided + reasonable fallbacks in case some are missing. + """ + if social_account_handle and organization_name: + return _( + "Here's a fun clip from a class I'm taking on {social_account_handle} from {organization_name}.\n\n" + ).format( + social_account_handle=social_account_handle, + organization_name=organization_name, + ) + elif social_account_handle: + return _( + "Here's a fun clip from a class I'm taking on {social_account_handle}.\n\n" + ).format(social_account_handle=social_account_handle) + elif organization_name: + return _( + "Here's a fun clip from a class I'm taking from {organization_name}.\n\n" + ).format(organization_name=organization_name) + else: + return _( + "Here's a fun clip from a class I'm taking on {platform_name}.\n\n" + ).format(platform_name=settings.PLATFORM_NAME) + + +def sharing_url(video_public_url, sharing_site_config, organization=None): """ Returns the sharing url with the appropriate parameters """ @@ -76,5 +103,13 @@ def sharing_url(video_public_url, sharing_site_config): 'utm_campaign': 'social-share-exp', sharing_site_config.url_param_name: video_public_url } - share_params.update(sharing_site_config.additional_site_params) + + # Special handling for Twitter, pre-populate share text + if sharing_site_config.name == "twitter": + twitter_handle = settings.PLATFORM_TWITTER_ACCOUNT + org_name = organization['name'] if organization else None + share_params.update({"text": get_share_text(twitter_handle, org_name)}) + else: + 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 ea0fe00ba0..aa83b5401c 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -23,6 +23,7 @@ from django.conf import settings from edx_django_utils.cache import RequestCache from lxml import etree from opaque_keys.edx.locator import AssetLocator +from organizations.api import get_course_organization from web_fragments.fragment import Fragment from xblock.completable import XBlockCompletionMode from xblock.core import XBlock @@ -480,11 +481,16 @@ 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) + organization = get_course_organization(self.course_id) + template_context['sharing_sites_info'] = sharing_sites_info_for_video( + public_video_url, + organization=organization + ) return self.runtime.service(self, 'mako').render_template('video.html', template_context)