From da1982f432103f09cc8d8a641132f6c54b0150a7 Mon Sep 17 00:00:00 2001 From: Jansen Kantor Date: Tue, 28 Mar 2023 16:02:36 -0400 Subject: [PATCH] fix: get marketing url from overview, with override (#31998) * fix: get marketing url from overview, with override * style: quality --- lms/djangoapps/courseware/tests/test_views.py | 77 ++++++++++++++++++- lms/djangoapps/courseware/views/views.py | 33 ++++++-- 2 files changed, 98 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 451c310488..c012047a25 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -2,6 +2,7 @@ Tests courseware views.py """ +from contextlib import contextmanager import html import itertools import json @@ -27,6 +28,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.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.waffle_utils.models import WaffleFlagCourseOverrideModel from rest_framework import status from web_fragments.fragment import Fragment @@ -3536,6 +3538,16 @@ class TestPublicVideoXBlockView(TestBasePublicVideoXBlock): request = RequestFactory().get('/?utm_source=edx.org&utm_medium=referral&utm_campaign=video') base_block = PublicVideoXBlockView(request=request) + @contextmanager + def mock_get_learn_more_url(self, **kwargs): + """ Helper for mocking get_learn_more_button_url """ + with patch.object( + PublicVideoXBlockView, + 'get_learn_more_button_url', + **kwargs + ) as mock_get_learn_more_url: + yield mock_get_learn_more_url + def test_get_template_and_context(self): """ Get template and context. @@ -3543,10 +3555,11 @@ class TestPublicVideoXBlockView(TestBasePublicVideoXBlock): self.setup_course(enable_waffle=True) fragment = MagicMock() with patch.object(self.video_block_public, "render", return_value=fragment): - template, context = self.base_block.get_template_and_context(self.course, self.video_block_public) - assert template == 'public_video.html' - assert context['fragment'] == fragment - assert context['course'] == self.course + with self.mock_get_learn_more_url(): + template, context = self.base_block.get_template_and_context(self.course, self.video_block_public) + assert template == 'public_video.html' + assert context['fragment'] == fragment + assert context['course'] == self.course @ddt.data("poster", None) def test_get_social_sharing_metadata(self, poster_url): @@ -3588,6 +3601,62 @@ class TestPublicVideoXBlockView(TestBasePublicVideoXBlock): url = self.base_block.build_url(base_url, params, utm_params) assert url == 'http://test.server?param1=value1¶m2=value2&utm_source=edx.org' + @patch.dict(settings.FEATURES, {"ENABLE_MKTG_SITE": False}) + def test_get_learn_more_button_url__marketing_not_enabled(self): + """ + Test that when the marketing site isn't enabled, the learn more button + goes to the 'about_course' view + """ + self.setup_course() + CourseOverviewFactory.create( + id=self.course.id, + ) + url = self.base_block.get_learn_more_button_url( + self.course, {} + ) + self.assertEqual( + url, + reverse('about_course', kwargs={'course_id': str(self.course.id)}) + ) + + @ddt.unpack + @ddt.data( + (True, False), + (False, True), + (True, True), + (True, False) + ) + @patch.dict(settings.FEATURES, {"ENABLE_MKTG_SITE": True}) + def test_get_learn_more_button_url(self, has_marketing_url, has_override): + """ + Test for learn more button url behavior when the marketing site is + enabled but considering overrides and missing marketing urls. + """ + self.setup_course() + utm_params = {'utm_source': 'askjeeves'} + overview = CourseOverviewFactory.create(id=self.course.id) + if has_marketing_url: + overview.marketing_url = 'mysite.com/this_course' + overview.save() + + url_overrides = {'someothercourse': 'someotherurl'} + overridden_url = "goto.this/site/instead" + if has_override: + url_overrides[str(self.course.id)] = overridden_url + + with override_settings(MKTG_URL_OVERRIDES=url_overrides): + url = self.base_block.get_learn_more_button_url(self.course, utm_params) + + if has_override: + expected_url = overridden_url + elif has_marketing_url: + expected_url = overview.marketing_url + else: + expected_url = reverse('about_course', kwargs={'course_id': str(self.course.id)}) + + self.assertIn(expected_url, url) + self.assertIn(urlencode(utm_params), url) + class TestPublicVideoXBlockEmbedView(TestBasePublicVideoXBlock): """Test Public Video XBlock Embed View""" diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index fd2e1f6c8a..8b713a038c 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -1787,7 +1787,7 @@ class PublicVideoXBlockView(BasePublicVideoXBlockView): fragment = video_block.render('public_view', context={ 'public_video_embed': False, }) - course_about_page_url, enroll_url = self.get_public_video_cta_button_urls(course) + learn_more_url, enroll_url = self.get_public_video_cta_button_urls(course) social_sharing_metadata = self.get_social_sharing_metadata(course, video_block) org_logo = self.get_organization_logo_from_course(course) context = { @@ -1795,7 +1795,7 @@ class PublicVideoXBlockView(BasePublicVideoXBlockView): 'course': course, 'org_logo': org_logo, 'social_sharing_metadata': social_sharing_metadata, - 'learn_more_url': course_about_page_url, + 'learn_more_url': learn_more_url, 'enroll_url': enroll_url, 'allow_iframing': True, 'disable_window_wrap': True, @@ -1836,25 +1836,42 @@ class PublicVideoXBlockView(BasePublicVideoXBlockView): ) } + def get_learn_more_button_url(self, course, utm_params): + """ + If the marketing site is enabled and a course has a marketing page, use that URL. + If not, point to the `about_course` view. + Override all with the MKTG_URL_OVERRIDES setting. + """ + course_key = str(course.id) + course_overview = CourseOverview.get_from_id(course.id) + if course_overview.has_marketing_url(): + base_url = course_overview.marketing_url + else: + base_url = reverse('about_course', kwargs={'course_id': course_key}) + + marketing_url_overrides = configuration_helpers.get_value( + 'MKTG_URL_OVERRIDES', + settings.MKTG_URL_OVERRIDES + ) + base_url = marketing_url_overrides.get(course_key, base_url) + return self.build_url(base_url, {}, utm_params) + def get_public_video_cta_button_urls(self, course): """ Get the links for the 'enroll' and 'learn more' buttons on the public video page """ - course_key = str(course.id) utm_params = self.get_utm_params() - course_about_page_url = self.build_url( - reverse('about_course', kwargs={'course_id': course_key}), {}, utm_params - ) + learn_more_url = self.get_learn_more_button_url(course, utm_params) enroll_url = self.build_url( reverse('register_user'), { - 'course_id': course_key, + 'course_id': str(course.id), 'enrollment_action': 'enroll', 'email_opt_in': False, }, utm_params ) - return course_about_page_url, enroll_url + return learn_more_url, enroll_url def get_utm_params(self): """