diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py
index c012047a25..45c969b7b1 100644
--- a/lms/djangoapps/courseware/tests/test_views.py
+++ b/lms/djangoapps/courseware/tests/test_views.py
@@ -28,7 +28,6 @@ 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
@@ -3098,41 +3097,6 @@ class TestRenderPublicVideoXBlock(TestBasePublicVideoXBlock):
self.assertEqual(expected_status_code, response.status_code)
self.assertEqual(expected_status_code, embed_response.status_code)
- def test_get_org_logo_none(self):
- # Given a course with no organizational logo
- self.setup_course()
- target_video = self.video_block_public
-
- # When I render the page
- response = self.get_response(usage_key=target_video.location, is_embed=False)
- content = response.content.decode('utf-8')
-
- # Then the page does not render an org logo
- org_logo = re.search('
', content)
- self.assertIsNone(org_logo)
-
- @patch('lms.djangoapps.courseware.views.views.get_course_organization')
- def test_get_org_logo(self, mock_get_org):
- # Given a course with an organizational logo
- self.setup_course()
- target_video = self.video_block_public
-
- mock_org_logo_url = "/assets/foo"
- mock_org_logo = MagicMock()
- mock_org_logo.url = mock_org_logo_url
-
- mock_get_org.return_value = {
- "logo": mock_org_logo
- }
-
- # When I render the page
- response = self.get_response(usage_key=target_video.location, is_embed=False)
- content = response.content.decode('utf-8')
-
- # Then the page does render an org logo
- org_logo = re.search(f'
', content)
- self.assertIsNotNone(org_logo)
-
class TestRenderXBlockSelfPaced(TestRenderXBlock): # lint-amnesty, pylint: disable=test-inherits-tests
"""
@@ -3537,6 +3501,7 @@ class TestPublicVideoXBlockView(TestBasePublicVideoXBlock):
"""Test Public Video XBlock View"""
request = RequestFactory().get('/?utm_source=edx.org&utm_medium=referral&utm_campaign=video')
base_block = PublicVideoXBlockView(request=request)
+ default_utm_params = {'utm_source': 'edx.org', 'utm_medium': 'referral', 'utm_campaign': 'video'}
@contextmanager
def mock_get_learn_more_url(self, **kwargs):
@@ -3545,8 +3510,18 @@ class TestPublicVideoXBlockView(TestBasePublicVideoXBlock):
PublicVideoXBlockView,
'get_learn_more_button_url',
**kwargs
- ) as mock_get_learn_more_url:
- yield mock_get_learn_more_url
+ ) as mock_get_url:
+ yield mock_get_url
+
+ @contextmanager
+ def mock_get_catalog_course_data(self, **kwargs):
+ """ Helper for mocking get_catalog_course_data """
+ with patch.object(
+ PublicVideoXBlockView,
+ 'get_catalog_course_data',
+ **kwargs
+ ) as mock_get_data:
+ yield mock_get_data
def test_get_template_and_context(self):
"""
@@ -3556,11 +3531,46 @@ class TestPublicVideoXBlockView(TestBasePublicVideoXBlock):
fragment = MagicMock()
with patch.object(self.video_block_public, "render", return_value=fragment):
with self.mock_get_learn_more_url():
- template, context = self.base_block.get_template_and_context(self.course, self.video_block_public)
+ with self.mock_get_catalog_course_data():
+ 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.unpack
+ @ddt.data(
+ (None, None, {}),
+ ('uuid', None, {}),
+ ('uuid', {}, {'org_logo': None, 'marketing_url': None}),
+ )
+ def test_get_catalog_course_data(self, mock_get_uuid, mock_get_data, expected_response):
+ self.setup_course()
+ with patch('lms.djangoapps.courseware.views.views.get_course_uuid_for_course', return_value=mock_get_uuid):
+ with patch('lms.djangoapps.courseware.views.views.get_course_data', return_value=mock_get_data):
+ assert self.base_block.get_catalog_course_data(self.course) == expected_response
+
+ @ddt.unpack
+ @ddt.data(
+ ({}, None),
+ ({'marketing_url': 'www.somesite.com/this'}, 'www.somesite.com/this'),
+ ({'marketing_url': 'www.somesite.com/this?utm_source=jansen'}, 'www.somesite.com/this'),
+ )
+ def test_get_catalog_course_marketing_url(self, input_data, expected_url):
+ url = self.base_block._get_catalog_course_marketing_url(input_data)
+ assert url == expected_url
+
+ @ddt.unpack
+ @ddt.data(
+ ({}, None),
+ ({'owners': []}, None),
+ ({'owners': [{}]}, None),
+ ({'owners': [{'logo_image_url': 'somesite.org/image'}]}, 'somesite.org/image'),
+ ({'owners': [{'logo_image_url': 'firsturl'}, {'logo_image_url': 'secondurl'}]}, 'firsturl'),
+ )
+ def test_get_catalog_course_owner_logo(self, input_data, expected_url):
+ url = self.base_block._get_catalog_course_owner_logo(input_data)
+ assert url == expected_url
+
@ddt.data("poster", None)
def test_get_social_sharing_metadata(self, poster_url):
"""
@@ -3601,61 +3611,25 @@ 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):
+ def assert_url_with_params(self, url, base_url, params):
+ if params:
+ assert url == base_url + '?' + urlencode(params)
+ else:
+ assert url == base_url
+
+ @ddt.data({}, {'marketing_url': 'some_url'})
+ def test_get_learn_more_button_url(self, catalog_course_info):
"""
- Test that when the marketing site isn't enabled, the learn more button
- goes to the 'about_course' view
+ If we have a marketing url from the catalog service, use that. Otherwise
+ use the courseware about_course
"""
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
+ url = self.base_block.get_learn_more_button_url(self.course, catalog_course_info, self.default_utm_params)
+ if 'marketing_url' in catalog_course_info:
+ expected_url = catalog_course_info['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)
+ self.assert_url_with_params(url, expected_url, self.default_utm_params)
class TestPublicVideoXBlockEmbedView(TestBasePublicVideoXBlock):
diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py
index 8b713a038c..c711165044 100644
--- a/lms/djangoapps/courseware/views/views.py
+++ b/lms/djangoapps/courseware/views/views.py
@@ -38,7 +38,6 @@ from markupsafe import escape
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey, UsageKey
from openedx_filters.learning.filters import CourseAboutRenderStarted
-from organizations.api import get_course_organization
from pytz import UTC
from requests.exceptions import ConnectionError, Timeout # pylint: disable=redefined-builtin
from rest_framework import status
@@ -102,7 +101,12 @@ from lms.djangoapps.instructor.enrollment import uses_shib
from lms.djangoapps.instructor.views.api import require_global_staff
from lms.djangoapps.survey import views as survey_views
from lms.djangoapps.verify_student.services import IDVerificationService
-from openedx.core.djangoapps.catalog.utils import get_programs, get_programs_with_type
+from openedx.core.djangoapps.catalog.utils import (
+ get_course_data,
+ get_course_uuid_for_course,
+ get_programs,
+ get_programs_with_type
+)
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.credit.api import (
get_credit_requirement_status,
@@ -1787,13 +1791,13 @@ class PublicVideoXBlockView(BasePublicVideoXBlockView):
fragment = video_block.render('public_view', context={
'public_video_embed': False,
})
- learn_more_url, enroll_url = self.get_public_video_cta_button_urls(course)
+ catalog_course_data = self.get_catalog_course_data(course)
+ learn_more_url, enroll_url = self.get_public_video_cta_button_urls(course, catalog_course_data)
social_sharing_metadata = self.get_social_sharing_metadata(course, video_block)
- org_logo = self.get_organization_logo_from_course(course)
context = {
'fragment': fragment,
'course': course,
- 'org_logo': org_logo,
+ 'org_logo': catalog_course_data.get('org_logo'),
'social_sharing_metadata': social_sharing_metadata,
'learn_more_url': learn_more_url,
'enroll_url': enroll_url,
@@ -1806,15 +1810,44 @@ class PublicVideoXBlockView(BasePublicVideoXBlockView):
}
return 'public_video.html', context
- def get_organization_logo_from_course(self, course):
+ def get_catalog_course_data(self, course):
"""
- Get organization logo for this course
+ Get information from the catalog service for this course
"""
- course_org = get_course_organization(course.id)
+ course_uuid = get_course_uuid_for_course(course.id)
+ if course_uuid is None:
+ return {}
+ catalog_course_data = get_course_data(
+ course_uuid,
+ ['owner', 'url_slug'],
+ )
+ if catalog_course_data is None:
+ return {}
- if course_org and course_org['logo']:
- return course_org['logo'].url
- return None
+ return {
+ 'org_logo': self._get_catalog_course_owner_logo(catalog_course_data),
+ 'marketing_url': self._get_catalog_course_marketing_url(catalog_course_data),
+ }
+
+ def _get_catalog_course_marketing_url(self, catalog_course_data):
+ """
+ Helper to extract url and remove any potential utm queries.
+ The discovery API includes UTM info unless you request it to not be included.
+ The request for the UUIDs will cache the response within the LMS so we need
+ to strip it here.
+ """
+ marketing_url = catalog_course_data.get('marketing_url')
+ if marketing_url is None:
+ return marketing_url
+ url_parts = urlparse(marketing_url)
+ return self._replace_url_query(url_parts, {})
+
+ def _get_catalog_course_owner_logo(self, catalog_course_data):
+ """ Helper to safely extract the course owner image url from the catalog course """
+ owners_data = catalog_course_data.get('owners', [])
+ if len(owners_data) == 0:
+ return None
+ return owners_data[0].get('logo_image_url', None)
def get_social_sharing_metadata(self, course, video_block):
"""
@@ -1836,32 +1869,23 @@ class PublicVideoXBlockView(BasePublicVideoXBlockView):
)
}
- def get_learn_more_button_url(self, course, utm_params):
+ def get_learn_more_button_url(self, course, catalog_course_data, 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)
+ base_url = catalog_course_data.get('marketing_url', None)
+ if base_url is None:
+ base_url = reverse('about_course', kwargs={'course_id': str(course.id)})
return self.build_url(base_url, {}, utm_params)
- def get_public_video_cta_button_urls(self, course):
+ def get_public_video_cta_button_urls(self, course, catalog_course_data):
"""
Get the links for the 'enroll' and 'learn more' buttons on the public video page
"""
utm_params = self.get_utm_params()
- learn_more_url = self.get_learn_more_button_url(course, utm_params)
+ learn_more_url = self.get_learn_more_button_url(course, catalog_course_data, utm_params)
enroll_url = self.build_url(
reverse('register_user'),
{
@@ -1889,15 +1913,18 @@ class PublicVideoXBlockView(BasePublicVideoXBlockView):
"""
if not params and not utm_params:
return base_url
- url_parts = urlparse(base_url)
+ parsed_url = urlparse(base_url)
full_params = {**params, **utm_params}
+ return self._replace_url_query(parsed_url, full_params)
+
+ def _replace_url_query(self, parsed_url, query):
return urlunparse((
- url_parts.scheme,
- url_parts.netloc,
- url_parts.path,
- url_parts.params,
- urlencode(full_params),
- url_parts.fragment
+ parsed_url.scheme,
+ parsed_url.netloc,
+ parsed_url.path,
+ parsed_url.params,
+ urlencode(query) if query else '',
+ parsed_url.fragment
))