diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py index 83ee03294790..de047a6a66ae 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py @@ -47,7 +47,7 @@ def test_course_settings_response(self): "is_prerequisite_courses_enabled": False, "language_options": settings.ALL_LANGUAGES, "lms_link_for_about_page": get_link_for_about_page(self.course), - "marketing_enabled": False, + "marketing_enabled": True, "mfe_proctored_exam_settings_url": get_proctored_exam_settings_url( self.course.id ), diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 4c284fb07efb..a87bb5667d10 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -1386,11 +1386,7 @@ def get_course_settings(request, course_key, course_block): 'ENABLE_PUBLISHER', settings.FEATURES.get('ENABLE_PUBLISHER', False) ) - marketing_enabled = configuration_helpers.get_value_for_org( - course_block.location.org, - 'ENABLE_MKTG_SITE', - settings.FEATURES.get('ENABLE_MKTG_SITE', False) - ) + marketing_enabled = True enable_extended_course_details = configuration_helpers.get_value_for_org( course_block.location.org, 'ENABLE_EXTENDED_COURSE_DETAILS', diff --git a/cms/envs/mock.yml b/cms/envs/mock.yml index 5eaf422bd442..3b219071ddf1 100644 --- a/cms/envs/mock.yml +++ b/cms/envs/mock.yml @@ -426,7 +426,6 @@ FEATURES: ENABLE_LTI_PROVIDER: true ENABLE_MAX_FAILED_LOGIN_ATTEMPTS: true ENABLE_MKTG_EMAIL_OPT_IN: true - ENABLE_MKTG_SITE: true ENABLE_MOBILE_REST_API: true ENABLE_PASSWORD_RESET_FAILURE_EMAIL: true ENABLE_PROCTORED_EXAMS: true diff --git a/cms/envs/production.py b/cms/envs/production.py index f88df2086b76..604d2753bccd 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -84,7 +84,6 @@ def get_env_setting(setting): 'EVENT_TRACKING_BACKENDS', 'JWT_AUTH', 'CELERY_QUEUES', - 'MKTG_URL_LINK_MAP', 'REST_FRAMEWORK', 'EVENT_BUS_PRODUCER_CONFIG', 'DEFAULT_FILE_STORAGE', @@ -149,8 +148,6 @@ def get_env_setting(setting): CACHES['staticfiles']['KEY_PREFIX'] = EDX_PLATFORM_REVISION # noqa: F405 -MKTG_URL_LINK_MAP.update(_YAML_TOKENS.get('MKTG_URL_LINK_MAP', {})) # noqa: F405 - #Timezone overrides TIME_ZONE = CELERY_TIMEZONE # noqa: F405 diff --git a/common/djangoapps/edxmako/shortcuts.py b/common/djangoapps/edxmako/shortcuts.py index e075d413fd11..6387f5dbccd3 100644 --- a/common/djangoapps/edxmako/shortcuts.py +++ b/common/djangoapps/edxmako/shortcuts.py @@ -20,12 +20,9 @@ from django.core.validators import URLValidator from django.http import HttpResponse # pylint: disable=unused-import from django.template import engines -from django.urls import NoReverseMatch, reverse -from edx_django_utils.monitoring import set_custom_attribute from six.moves.urllib.parse import urljoin from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers -from xmodule.util.xmodule_django import get_current_request_hostname # pylint: disable=wrong-import-order from . import Engines @@ -33,20 +30,11 @@ def marketing_link(name): - """Returns the correct URL for a link to the marketing site - depending on if the marketing site is enabled + """Returns the URL for a marketing site link from MKTG_URLS. - Since the marketing site is enabled by a setting, we have two - possible URLs for certain links. This function is to decides - which URL should be provided. + MKTG_URL_OVERRIDES take priority. 'ROOT' falls back to the local landing page + ('/'); any other unconfigured link falls back to '#'. """ - # link_map maps URLs from the marketing site to the old equivalent on - # the Django site - link_map = settings.MKTG_URL_LINK_MAP - enable_mktg_site = configuration_helpers.get_value( - 'ENABLE_MKTG_SITE', - settings.FEATURES.get('ENABLE_MKTG_SITE', False) - ) marketing_urls = configuration_helpers.get_value( 'MKTG_URLS', settings.MKTG_URLS @@ -66,7 +54,7 @@ def marketing_link(name): log.debug("Invalid link set for link %s: %s", name, err) return '#' - if enable_mktg_site and name in marketing_urls: + if name in marketing_urls: # special case for when we only want the root marketing URL if name == 'ROOT': return marketing_urls.get('ROOT') @@ -81,23 +69,14 @@ def marketing_link(name): # URLs in the MKTG_URLS setting # e.g. urljoin('https://marketing.com', 'https://open-edx.org/about') >>> 'https://open-edx.org/about' return urljoin(marketing_urls.get('ROOT'), marketing_urls.get(name)) - # only link to the old pages when the marketing site isn't on - elif not enable_mktg_site and name in link_map: - # don't try to reverse disabled marketing links - if link_map[name] is not None: - host_name = get_current_request_hostname() # pylint: disable=unused-variable # noqa: F841 - if link_map[name].startswith('http'): - return link_map[name] - else: - try: - return reverse(link_map[name]) - except NoReverseMatch: - log.debug("Cannot find corresponding link for name: %s", name) - set_custom_attribute('unresolved_marketing_link', name) - return '#' - else: - log.debug("Cannot find corresponding link for name: %s", name) - return '#' + + # ROOT is the live site root (not a legacy marketing page), so default it to + # the local landing page when no marketing site ROOT is configured. + if name == 'ROOT': + return '/' + + log.debug("Cannot find corresponding link for name: %s", name) + return '#' def is_any_marketing_link_set(names): @@ -112,20 +91,11 @@ def is_marketing_link_set(name): """ Returns a boolean if a given named marketing link is configured. """ - - enable_mktg_site = configuration_helpers.get_value( - 'ENABLE_MKTG_SITE', - settings.FEATURES.get('ENABLE_MKTG_SITE', False) - ) marketing_urls = configuration_helpers.get_value( 'MKTG_URLS', settings.MKTG_URLS ) - - if enable_mktg_site: - return name in marketing_urls - else: - return name in settings.MKTG_URL_LINK_MAP + return name in marketing_urls def marketing_link_context_processor(request): @@ -144,9 +114,7 @@ def marketing_link_context_processor(request): return { "MKTG_URL_" + k: marketing_link(k) - for k in ( - settings.MKTG_URL_LINK_MAP.keys() | marketing_urls.keys() - ) + for k in marketing_urls.keys() } diff --git a/common/djangoapps/edxmako/tests.py b/common/djangoapps/edxmako/tests.py index e81ea883910a..6bc1282cb583 100644 --- a/common/djangoapps/edxmako/tests.py +++ b/common/djangoapps/edxmako/tests.py @@ -3,12 +3,10 @@ from unittest.mock import Mock, patch import ddt -from django.conf import settings from django.http import HttpResponse from django.test import TestCase from django.test.client import RequestFactory from django.test.utils import override_settings -from django.urls import reverse from edx_django_utils.cache import RequestCache from common.djangoapps.edxmako import LOOKUP, add_lookup @@ -33,92 +31,36 @@ class ShortcutsTests(UrlResetMixin, TestCase): @override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'ABOUT': '/about-us'}) def test_marketing_link(self): - with override_settings(MKTG_URL_LINK_MAP={'ABOUT': self._get_test_url_name()}): - # test marketing site on - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): - expected_link = 'https://dummy-root/about-us' - link = marketing_link('ABOUT') - assert link == expected_link - # test marketing site off - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}): - expected_link = reverse(self._get_test_url_name()) - link = marketing_link('ABOUT') - assert link == expected_link + expected_link = 'https://dummy-root/about-us' + link = marketing_link('ABOUT') + assert link == expected_link + + @override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'ABOUT': '/about-us'}) + def test_marketing_link_unconfigured(self): + assert marketing_link('NOT_CONFIGURED') == '#' @override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'ABOUT': '/about-us'}) def test_is_marketing_link_set(self): - with override_settings(MKTG_URL_LINK_MAP={'ABOUT': self._get_test_url_name()}): - # test marketing site on - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): - assert is_marketing_link_set('ABOUT') - assert not is_marketing_link_set('NOT_CONFIGURED') - # test marketing site off - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}): - assert is_marketing_link_set('ABOUT') - assert not is_marketing_link_set('NOT_CONFIGURED') + assert is_marketing_link_set('ABOUT') + assert not is_marketing_link_set('NOT_CONFIGURED') @override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'ABOUT': '/about-us'}) def test_is_any_marketing_link_set(self): - with override_settings(MKTG_URL_LINK_MAP={'ABOUT': self._get_test_url_name()}): - # test marketing site on - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): - assert is_any_marketing_link_set(['ABOUT']) - assert is_any_marketing_link_set(['ABOUT', 'NOT_CONFIGURED']) - assert not is_any_marketing_link_set(['NOT_CONFIGURED']) - # test marketing site off - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}): - assert is_any_marketing_link_set(['ABOUT']) - assert is_any_marketing_link_set(['ABOUT', 'NOT_CONFIGURED']) - assert not is_any_marketing_link_set(['NOT_CONFIGURED']) - - def _get_test_url_name(self): # pylint: disable=missing-function-docstring - if settings.ROOT_URLCONF == 'lms.urls': - # return any lms url name - return 'dashboard' - else: - # return any cms url name - return 'organizations' + assert is_any_marketing_link_set(['ABOUT']) + assert is_any_marketing_link_set(['ABOUT', 'NOT_CONFIGURED']) + assert not is_any_marketing_link_set(['NOT_CONFIGURED']) @override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'TOS': '/tos'}) @override_settings(MKTG_URL_OVERRIDES={'TOS': 'https://edx.org'}) def test_override_marketing_link_valid(self): - expected_link = 'https://edx.org' - # test marketing site on - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): - link = marketing_link('TOS') - assert link == expected_link - # test marketing site off - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}): - link = marketing_link('TOS') - assert link == expected_link + link = marketing_link('TOS') + assert link == 'https://edx.org' @override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'TOS': '/tos'}) @override_settings(MKTG_URL_OVERRIDES={'TOS': '123456'}) def test_override_marketing_link_invalid(self): - expected_link = '#' - # test marketing site on - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): - link = marketing_link('TOS') - assert link == expected_link - # test marketing site off - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}): - link = marketing_link('TOS') - assert link == expected_link - - @skip_unless_lms - def test_link_map_url_reverse(self): - url_link_map = { - 'ABOUT': 'dashboard', - 'BAD_URL': 'foobarbaz', - } - - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}): - with override_settings(MKTG_URL_LINK_MAP=url_link_map): - link = marketing_link('ABOUT') - assert link == '/dashboard' - - link = marketing_link('BAD_URL') - assert link == '#' + link = marketing_link('TOS') + assert link == '#' class AddLookupTests(TestCase): diff --git a/common/djangoapps/student/tests/test_email.py b/common/djangoapps/student/tests/test_email.py index 467eae934e56..06e89f21234c 100644 --- a/common/djangoapps/student/tests/test_email.py +++ b/common/djangoapps/student/tests/test_email.py @@ -531,15 +531,7 @@ def assertChangeEmailSent(self, test_body_type): # Must have two items in outbox: one for old email, another for new email assert len(mail.outbox) == 2 - use_https = self.request.is_secure() - if settings.FEATURES['ENABLE_MKTG_SITE']: - contact_link = marketing_link('CONTACT') - else: - contact_link = '{protocol}://{site}{link}'.format( - protocol='https' if use_https else 'http', - site=settings.SITE_NAME, - link=reverse('contact'), - ) + contact_link = marketing_link('CONTACT') # Verifying contents for msg in mail.outbox: @@ -590,17 +582,10 @@ def test_new_email_fails(self, ace_mail): @skip_unless_lms @override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'CONTACT': '/help/contact-us'}) @patch('common.djangoapps.student.signals.signals.USER_EMAIL_CHANGED.send') - @ddt.data( - ('plain_text', False), - ('plain_text', True), - ('html', False), - ('html', True) - ) - @ddt.unpack - def test_successful_email_change(self, test_body_type, test_marketing_enabled, mock_email_change_signal): - with patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': test_marketing_enabled}): - self.assertChangeEmailSent(test_body_type) - assert mock_email_change_signal.called + @ddt.data('plain_text', 'html') + def test_successful_email_change(self, test_body_type, mock_email_change_signal): + self.assertChangeEmailSent(test_body_type) + assert mock_email_change_signal.called meta = json.loads(UserProfile.objects.get(user=self.user).meta) assert 'old_emails' in meta diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index ac724d6f847b..b9fc0a2636b7 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -194,7 +194,6 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, MOCK_SETTINGS = { 'FEATURES': { 'DISABLE_START_DATES': False, - 'ENABLE_MKTG_SITE': True, 'DISABLE_SET_JWT_COOKIES_FOR_TESTS': True, }, 'SOCIAL_SHARING_SETTINGS': { @@ -1087,13 +1086,13 @@ def test_user_with_unacknowledged_notice(self, mock_notices): Verifies that we will redirect the learner to the URL returned from the `check_for_unacknowledged_notices` function. """ - mock_notices.return_value = reverse("about") + mock_notices.return_value = reverse("root") with override_settings(FEATURES={**settings.FEATURES, 'ENABLE_NOTICES': True}): response = self.client.get(self.path) assert response.status_code == 302 - assert response.url == "/about" + assert response.url == "/" mock_notices.assert_called_once() @patch('common.djangoapps.student.views.dashboard.check_for_unacknowledged_notices') diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 5609afd49ddc..eaa83292fe0e 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -924,15 +924,7 @@ def confirm_email_change(request, key): transaction.set_rollback(True) return response - use_https = request.is_secure() - if settings.FEATURES['ENABLE_MKTG_SITE']: - contact_link = marketing_link('CONTACT') - else: - contact_link = '{protocol}://{site}{link}'.format( - protocol='https' if use_https else 'http', - site=configuration_helpers.get_value('SITE_NAME', settings.SITE_NAME), - link=reverse('contact'), - ) + contact_link = marketing_link('CONTACT') site = Site.objects.get_current() message_context = get_base_template_context(site) diff --git a/common/djangoapps/util/course.py b/common/djangoapps/util/course.py index 6eeaec0966f6..53a4d692b68c 100644 --- a/common/djangoapps/util/course.py +++ b/common/djangoapps/util/course.py @@ -59,7 +59,7 @@ def get_link_for_about_page(course): if is_social_sharing_enabled and course.social_sharing_url: course_about_url = course.social_sharing_url - elif settings.FEATURES.get('ENABLE_MKTG_SITE') and getattr(course, 'marketing_url', None): + elif getattr(course, 'marketing_url', None): course_about_url = course.marketing_url else: course_about_url = f'{about_base_url}/courses/{course.id}/about' diff --git a/common/djangoapps/util/tests/test_course.py b/common/djangoapps/util/tests/test_course.py index 880d64735ad7..a430f479c32a 100644 --- a/common/djangoapps/util/tests/test_course.py +++ b/common/djangoapps/util/tests/test_course.py @@ -38,13 +38,12 @@ def setUp(self): self.course_overview.marketing_url = 'test_marketing_url' self.course_overview.save() - def get_course_sharing_link(self, enable_social_sharing, enable_mktg_site, use_overview=True): + def get_course_sharing_link(self, enable_social_sharing, use_overview=True): """ Get course sharing link. Arguments: enable_social_sharing(Boolean): To indicate whether social sharing is enabled. - enable_mktg_site(Boolean): A feature flag to decide activation of marketing site. Keyword Arguments: use_overview: indicates whether course overview or course block should get @@ -53,9 +52,6 @@ def get_course_sharing_link(self, enable_social_sharing, enable_mktg_site, use_o Returns course sharing url. """ mock_settings = { - 'FEATURES': { - 'ENABLE_MKTG_SITE': enable_mktg_site, - }, 'SOCIAL_SHARING_SETTINGS': { 'CUSTOM_COURSE_URLS': enable_social_sharing } @@ -70,19 +66,17 @@ def get_course_sharing_link(self, enable_social_sharing, enable_mktg_site, use_o return course_sharing_link @ddt.data( - (True, True, 'test_social_sharing_url'), - (False, True, 'test_marketing_url'), - (True, False, 'test_social_sharing_url'), - (False, False, f'{settings.LMS_ROOT_URL}/courses/course-v1:test_org+test_number+test_run/about'), + (True, 'test_social_sharing_url'), + (False, 'test_marketing_url'), ) @ddt.unpack - def test_sharing_link_with_settings(self, enable_social_sharing, enable_mktg_site, expected_course_sharing_link): + def test_sharing_link_with_settings(self, enable_social_sharing, expected_course_sharing_link): """ - Verify the method gives correct course sharing url on settings manipulations. + Verify the method gives correct course sharing url: social sharing URL takes priority + over marketing URL, which takes priority over the LMS about URL. """ actual_course_sharing_link = self.get_course_sharing_link( enable_social_sharing=enable_social_sharing, - enable_mktg_site=enable_mktg_site, ) assert actual_course_sharing_link == expected_course_sharing_link @@ -107,7 +101,6 @@ def test_sharing_link_with_course_overview_attrs(self, overview_attrs, expected_ actual_course_sharing_link = self.get_course_sharing_link( enable_social_sharing=True, - enable_mktg_site=True, ) assert actual_course_sharing_link == expected_course_sharing_link @@ -122,11 +115,10 @@ def test_sharing_link_with_course_overview_attrs(self, overview_attrs, expected_ def test_sharing_link_with_course_block(self, enable_social_sharing, expected_course_sharing_link): """ Verify the method gives correct course sharing url on passing - course block as a parameter. + course block as a parameter (course blocks have no marketing_url attribute). """ actual_course_sharing_link = self.get_course_sharing_link( enable_social_sharing=enable_social_sharing, - enable_mktg_site=True, use_overview=False, ) assert actual_course_sharing_link == expected_course_sharing_link @@ -146,8 +138,11 @@ def test_sharing_link_with_new_course_about_page( self, catalog_mfe_enabled, expected_course_sharing_link ): """ - Verify the method gives correct course sharing url when new course about page is used. + Verify the correct about URL base is used when neither social sharing URL nor + marketing URL is set (catalog MFE URL vs LMS root URL). """ + self.course_overview.marketing_url = None + self.course_overview.save() with override_settings(ENABLE_CATALOG_MICROFRONTEND=catalog_mfe_enabled): actual_course_sharing_link = get_link_for_about_page(self.course_overview) assert actual_course_sharing_link == expected_course_sharing_link diff --git a/docs/docs_settings.py b/docs/docs_settings.py index 9fe06e935676..6b08a744193d 100644 --- a/docs/docs_settings.py +++ b/docs/docs_settings.py @@ -29,7 +29,6 @@ # Settings that will fail if we enable them, and we don't need them for docs anyway. FEATURES["RUN_AS_ANALYTICS_SERVER_ENABLED"] = False # noqa: F405 FEATURES["ENABLE_SOFTWARE_SECURE_FAKE"] = False # noqa: F405 -FEATURES["ENABLE_MKTG_SITE"] = False # noqa: F405 INSTALLED_APPS.extend( # noqa: F405 [ diff --git a/lms/djangoapps/branding/tests/test_api.py b/lms/djangoapps/branding/tests/test_api.py index 71819a98d940..14cf3c9c9d48 100644 --- a/lms/djangoapps/branding/tests/test_api.py +++ b/lms/djangoapps/branding/tests/test_api.py @@ -46,7 +46,6 @@ class TestFooter(TestCase): """Test retrieving the footer. """ maxDiff = None - @mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}) @mock.patch.dict('django.conf.settings.MKTG_URLS', { "ROOT": "https://edx.org", "ENTERPRISE": "/enterprise" @@ -62,7 +61,6 @@ def test_footer_business_links_no_marketing_query_params(self): business_links = _footer_business_links() assert business_links[0]['url'] == 'https://edx.org/enterprise' - @mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}) @mock.patch.dict('django.conf.settings.MKTG_URLS', { "ROOT": "https://edx.org", "ABOUT": "/about-us", diff --git a/lms/djangoapps/branding/tests/test_views.py b/lms/djangoapps/branding/tests/test_views.py index 672a61aea6c4..a9eff025f76f 100644 --- a/lms/djangoapps/branding/tests/test_views.py +++ b/lms/djangoapps/branding/tests/test_views.py @@ -268,14 +268,12 @@ def test_index_does_not_redirect_without_site_override(self): response = self.client.get(reverse("root")) assert response.status_code == 200 - @override_settings(ENABLE_MKTG_SITE=True) @override_settings(MKTG_URLS={'ROOT': 'https://foo.bar/'}) @override_settings(LMS_ROOT_URL='https://foo.bar/') def test_index_wont_redirect_to_marketing_root_if_it_matches_lms_root(self): response = self.client.get(reverse("root")) assert response.status_code == 200 - @override_settings(ENABLE_MKTG_SITE=True) @override_settings(MKTG_URLS={'ROOT': 'https://home.foo.bar/'}) @override_settings(LMS_ROOT_URL='https://foo.bar/') def test_index_will_redirect_to_new_root_if_mktg_site_is_enabled(self): diff --git a/lms/djangoapps/branding/views.py b/lms/djangoapps/branding/views.py index 19842d4ae504..54a7f0ce2d05 100644 --- a/lms/djangoapps/branding/views.py +++ b/lms/djangoapps/branding/views.py @@ -48,19 +48,13 @@ def index(request): if use_catalog_mfe(): return redirect(f'{settings.CATALOG_MICROFRONTEND_URL}/', permanent=True) - enable_mktg_site = configuration_helpers.get_value( - 'ENABLE_MKTG_SITE', - getattr(settings, 'ENABLE_MKTG_SITE', False) + marketing_urls = configuration_helpers.get_value( + 'MKTG_URLS', + settings.MKTG_URLS ) - - if enable_mktg_site: - marketing_urls = configuration_helpers.get_value( - 'MKTG_URLS', - settings.MKTG_URLS - ) - root_url = marketing_urls.get("ROOT") - if root_url != getattr(settings, "LMS_ROOT_URL", None): - return redirect(root_url) + root_url = marketing_urls.get("ROOT") + if root_url and root_url != getattr(settings, "LMS_ROOT_URL", None): + return redirect(root_url) domain = request.headers.get('Host') @@ -76,11 +70,10 @@ def index(request): return student_views.index(request, user=request.user) except NoReverseMatch: log.error( - f'https is not a registered namespace Request from {domain}', - f'request_site= {request.site.__dict__}', - f'enable_mktg_site= {enable_mktg_site}', - f'Auth Status= {request.user.is_authenticated}', - f'Request Meta= {request.META}' + f'NoReverseMatch on index view for domain {domain}; ' + f'request_site={getattr(request, "site", None)}; ' + f'Auth Status={request.user.is_authenticated}; ' + f'Request Meta={request.META}' ) raise @@ -89,26 +82,15 @@ def index(request): @cache_if_anonymous() def courses(request): """ - Render the "find courses" page. If the marketing site is enabled, redirect - to that. Otherwise, if subdomain branding is on, this is the university - profile page. Otherwise, it's the edX courseware.views.views.courses page + Serve the "find courses" page. Redirects to the catalog MFE or the marketing + site COURSES URL if configured; falls back to rendering the local courses page. """ if use_catalog_mfe(): return redirect(f'{settings.CATALOG_MICROFRONTEND_URL}/courses', permanent=True) - enable_mktg_site = configuration_helpers.get_value( - 'ENABLE_MKTG_SITE', - settings.FEATURES.get('ENABLE_MKTG_SITE', False) - ) - - if enable_mktg_site: - return redirect(marketing_link('COURSES'), permanent=True) - - if not settings.FEATURES.get('COURSES_ARE_BROWSABLE'): - raise Http404 - - # we do not expect this case to be reached in cases where - # marketing is enabled or the courses are not browsable + courses_url = marketing_link('COURSES') + if courses_url != '#': + return redirect(courses_url, permanent=True) return courseware_views.courses(request) diff --git a/lms/djangoapps/courseware/tests/test_about.py b/lms/djangoapps/courseware/tests/test_about.py index 35cd028621bf..8da4955cc3f0 100644 --- a/lms/djangoapps/courseware/tests/test_about.py +++ b/lms/djangoapps/courseware/tests/test_about.py @@ -113,15 +113,13 @@ def test_visible_about_page_settings(self): resp = self.client.get(url) self.assertRedirects(resp, reverse('dashboard'), fetch_redirect_response=False) - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) + @patch.dict(settings.FEATURES, {'ENABLE_COURSE_HOME_REDIRECT': True}) def test_logged_in_marketing(self): self.setup_user() url = reverse('about_course', args=[str(self.course.id)]) resp = self.client.get(url) self.assertRedirects(resp, course_home_url(self.course.id), fetch_redirect_response=False) - @patch.dict(settings.FEATURES, {'ENABLE_COURSE_HOME_REDIRECT': False}) - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) def test_logged_in_marketing_without_course_home_redirect(self): """ Verify user is not redirected to course home page when @@ -133,19 +131,6 @@ def test_logged_in_marketing_without_course_home_redirect(self): # should not be redirected self.assertContains(resp, "OOGIE BLOOGIE") - @patch.dict(settings.FEATURES, {'ENABLE_COURSE_HOME_REDIRECT': True}) - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': False}) - def test_logged_in_marketing_without_mktg_site(self): - """ - Verify user is not redirected to course home page when - ENABLE_MKTG_SITE is set to False - """ - self.setup_user() - url = reverse('about_course', args=[str(self.course.id)]) - resp = self.client.get(url) - # should not be redirected - self.assertContains(resp, "OOGIE BLOOGIE") - @patch.dict(settings.FEATURES, {'ENABLE_PREREQUISITE_COURSES': True}) def test_pre_requisite_course(self): pre_requisite_course = CourseFactory.create(org='edX', course='900', display_name='pre requisite course') diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 960be2fd7849..eea0333f0ba1 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -545,14 +545,11 @@ def get(self, request, course_id, tab_type, **kwargs): # pylint: disable=argume return CourseTabView.handle_exceptions(request, course_key, course, exception) @staticmethod - def url_to_enroll(course_key): + def url_to_enroll(course_key): # pylint: disable=unused-argument """ Returns the URL to use to enroll in the specified course. """ - url_to_enroll = reverse('about_course', args=[str(course_key)]) - if settings.FEATURES.get('ENABLE_MKTG_SITE'): - url_to_enroll = marketing_link('COURSES') - return url_to_enroll + return marketing_link('COURSES') @staticmethod def register_user_access_warning_messages(request, course): @@ -1214,16 +1211,11 @@ def credit_course_requirements(course_key, student): def _course_home_redirect_enabled(): """ - Return True value if user needs to be redirected to course home based on value of - `ENABLE_MKTG_SITE` and `ENABLE_COURSE_HOME_REDIRECT feature` flags - Returns: boolean True or False + Return True if users should be redirected from the course about page to course home. """ - if configuration_helpers.get_value( - 'ENABLE_MKTG_SITE', settings.FEATURES.get('ENABLE_MKTG_SITE', False) - ) and configuration_helpers.get_value( - 'ENABLE_COURSE_HOME_REDIRECT', settings.FEATURES.get('ENABLE_COURSE_HOME_REDIRECT', True) - ): - return True + return bool(configuration_helpers.get_value( + 'ENABLE_COURSE_HOME_REDIRECT', settings.FEATURES.get('ENABLE_COURSE_HOME_REDIRECT', False) + )) @login_required diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index 565eef116913..c39516e16036 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -35,6 +35,7 @@ get_event_transaction_id, set_event_transaction_type, ) +from common.djangoapps.util.course import get_link_for_about_page from lms.djangoapps.branding.api import get_logo_url_for_email from lms.djangoapps.courseware.models import StudentModule from lms.djangoapps.grades.api import constants as grades_constants @@ -494,14 +495,7 @@ def get_email_params(course, auto_enroll, secure=True, course_key=None, display_ path=reverse('course_root', kwargs={'course_id': course_key}) ) - # We can't get the url to the course's About page if the marketing site is enabled. - course_about_url = None - if not settings.FEATURES.get('ENABLE_MKTG_SITE', False): - course_about_url = '{proto}://{site}{path}'.format( - proto=protocol, - site=stripped_site_name, - path=reverse('about_course', kwargs={'course_id': course_key}) - ) + course_about_url = get_link_for_about_page(course) is_shib_course = uses_shib(course) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index d68633c819c6..cd291662319b 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -1316,50 +1316,7 @@ def test_enroll_with_email_not_registered(self, protocol): assert 'Once you have registered and activated your account,' in body - assert '{proto}://{site}{about_path}'.format( # noqa: UP032 - proto=protocol, - site=self.site_name, - about_path=self.about_path - ) in body - - assert 'This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org' in body - - @ddt.data('http', 'https') - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) - def test_enroll_email_not_registered_mktgsite(self, protocol): - url = reverse('students_update_enrollment', kwargs={'course_id': str(self.course.id)}) - params = {'identifiers': self.notregistered_email, 'action': 'enroll', 'email_students': True} - environ = {'wsgi.url_scheme': protocol} - response = self.client.post(url, params, **environ) - - manual_enrollments = ManualEnrollmentAudit.objects.all() - assert manual_enrollments.count() == 1 - assert manual_enrollments[0].state_transition == UNENROLLED_TO_ALLOWEDTOENROLL - assert response.status_code == 200 - - text_body = mail.outbox[0].body - html_body = mail.outbox[0].alternatives[0][0] - - assert text_body.startswith('Dear student,') - assert 'To finish your registration, please visit' in text_body - assert 'Please finish your registration and fill' in html_body - - for body in [text_body, html_body]: - assert 'You have been invited to join {display_name} at edx.org by a member of the course staff.'.format( # noqa: UP032 # pylint: disable=line-too-long - display_name=self.course.display_name - ) in body - - assert '{proto}://{site}/register'.format( # noqa: UP032 - proto=protocol, - site=self.site_name - ) in body - - assert ('fill out the registration form making sure to use ' - 'robot-not-an-email-yet@robot.org in the Email field') in body - - assert 'You can then enroll in {display_name}.'.format( # noqa: UP032 - display_name=self.course.display_name - ) in body + assert f'{settings.LMS_ROOT_URL}{self.about_path}' in body assert 'This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org' in body @@ -1587,11 +1544,7 @@ def test_enroll_with_email_not_registered_with_shib(self, protocol, mock_uses_sh text_body = mail.outbox[0].body html_body = mail.outbox[0].alternatives[0][0] - course_url = '{proto}://{site}{about_path}'.format( # noqa: UP032 - proto=protocol, - site=self.site_name, - about_path=self.about_path, - ) + course_url = f'{settings.LMS_ROOT_URL}{self.about_path}' assert text_body.startswith('Dear student,') assert 'To access this course visit {course_url} and register for this course.'.format( # noqa: UP032 course_url=course_url, @@ -1606,31 +1559,6 @@ def test_enroll_with_email_not_registered_with_shib(self, protocol, mock_uses_sh assert 'This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org' in body - @patch('lms.djangoapps.instructor.enrollment.uses_shib') - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) - def test_enroll_email_not_registered_shib_mktgsite(self, mock_uses_shib): - # Try with marketing site enabled and shib on - mock_uses_shib.return_value = True - - url = reverse('students_update_enrollment', kwargs={'course_id': str(self.course.id)}) - # Try with marketing site enabled - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): - response = self.client.post(url, {'identifiers': self.notregistered_email, 'action': 'enroll', - 'email_students': True}) - - assert response.status_code == 200 - - text_body = mail.outbox[0].body - html_body = mail.outbox[0].alternatives[0][0] - assert text_body.startswith('Dear student,') - - for body in [text_body, html_body]: - assert 'You have been invited to join {display_name} at edx.org by a member of the course staff.'.format( # noqa: UP032 # pylint: disable=line-too-long - display_name=self.course.display_name, - ) in body - - assert 'This email was automatically sent from edx.org to robot-not-an-email-yet@robot.org' in body - @ddt.data('http', 'https') @patch('lms.djangoapps.instructor.enrollment.uses_shib') def test_enroll_with_email_not_registered_with_shib_autoenroll(self, protocol, mock_uses_shib): @@ -2270,11 +2198,7 @@ def test_add_notenrolled_with_email(self, protocol): assert 'by a member of the course staff.' in body assert 'enroll in this course and begin the beta test' in body - assert '{proto}://{site}{about_path}'.format( # noqa: UP032 - proto=protocol, - site=self.site_name, - about_path=self.about_path, - ) in body + assert f'{settings.LMS_ROOT_URL}{self.about_path}' in body assert 'This email was automatically sent from edx.org to {student_email}'.format( # noqa: UP032 student_email=self.notenrolled_student.email, @@ -2332,31 +2256,6 @@ def test_add_notenrolled_with_email_autoenroll(self, protocol): student_email=self.notenrolled_student.email, ) in body - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) - def test_add_notenrolled_email_mktgsite(self): - # Try with marketing site enabled - url = reverse('bulk_beta_modify_access', kwargs={'course_id': str(self.course.id)}) - response = self.client.post(url, {'identifiers': self.notenrolled_student.email, 'action': 'add', 'email_students': True}) # pylint: disable=line-too-long - - assert response.status_code == 200 - - text_body = mail.outbox[0].body - html_body = mail.outbox[0].alternatives[0][0] - student_name = self.notenrolled_student.profile.name - assert text_body.startswith(f'Dear {student_name}') - - for body in [text_body, html_body]: - assert 'You have been invited to be a beta tester for {display_name} at edx.org'.format( # noqa: UP032 - display_name=self.course.display_name, - ) in body - - assert 'by a member of the course staff.' in body - assert 'Visit edx.org' in body - assert 'enroll in this course and begin the beta test' in body - assert 'This email was automatically sent from edx.org to {student_email}'.format( # noqa: UP032 - student_email=self.notenrolled_student.email, - ) in body - def test_enroll_with_email_not_registered(self): # User doesn't exist url = reverse('bulk_beta_modify_access', kwargs={'course_id': str(self.course.id)}) diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index f3742b035fa1..5cb2e6504f3e 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -886,6 +886,8 @@ class TestGetEmailParamsCCX(SharedModuleStoreTestCase): def setUpClass(cls): super().setUpClass() cls.course = CourseFactory.create() + # get_link_for_about_page uses course.id (parent course), not the CCX key + cls.course_about_url = f'{settings.LMS_ROOT_URL}/courses/{cls.course.id}/about' @patch.dict('django.conf.settings.FEATURES', {'CUSTOM_COURSES_EDX': True}) def setUp(self): @@ -902,7 +904,6 @@ def setUp(self): site, self.course_key ) - self.course_about_url = self.course_url + 'about' self.registration_url = f'https://{site}/register' @patch.dict('django.conf.settings.FEATURES', {'CUSTOM_COURSES_EDX': True}) @@ -939,13 +940,11 @@ def setUpClass(cls): site, str(cls.course.id) ) - cls.course_about_url = cls.course_url + 'about' cls.registration_url = f'https://{site}/register' cls.logo_url = get_logo_url_for_email() + cls.course_about_url = f'{settings.LMS_ROOT_URL}/courses/{cls.course.id}/about' def test_normal_params(self): - # For a normal site, what do we expect to get for the URLs? - # Also make sure `auto_enroll` is properly passed through. result = get_email_params(self.course, False) assert result['auto_enroll'] is False @@ -954,18 +953,11 @@ def test_normal_params(self): assert result['course_url'] == self.course_url assert result['logo_url'] == self.logo_url - def test_marketing_params(self): - # For a site with a marketing front end, what do we expect to get for the URLs? - # Also make sure `auto_enroll` is properly passed through. - with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}): - result = get_email_params(self.course, True) + def test_auto_enroll_params(self): + result = get_email_params(self.course, True) assert result['auto_enroll'] is True - # We should *not* get a course about url (LMS doesn't know what the marketing site URLs are) - assert result['course_about_url'] is None - assert result['registration_url'] == self.registration_url - assert result['course_url'] == self.course_url - assert result['logo_url'] == self.logo_url + assert result['course_about_url'] == self.course_about_url @patch('lms.djangoapps.instructor.enrollment.get_logo_url_for_email', return_value='https://www.logo.png') def test_logo_url_params(self, mock_get_logo_url_for_email): diff --git a/lms/djangoapps/learner_home/test_views.py b/lms/djangoapps/learner_home/test_views.py index 2328e9e92342..9984f063f4c7 100644 --- a/lms/djangoapps/learner_home/test_views.py +++ b/lms/djangoapps/learner_home/test_views.py @@ -76,12 +76,14 @@ def test_happy_path(self, mock_marketing_link): @ddt.data( (True, f'{settings.CATALOG_MICROFRONTEND_URL}/courses'), - (False, '/courses'), + (False, '#'), ) @ddt.unpack def test_link_with_new_catalog_page(self, catalog_mfe_enabled, expected_catalog_link): """ Test that the catalog link is constructed correctly based on the MFE flags. + When the catalog MFE is disabled, marketing_link('COURSES') is used; '#' is + returned when MKTG_URLS has no COURSES entry. """ with override_settings(ENABLE_CATALOG_MICROFRONTEND=catalog_mfe_enabled): assert get_platform_settings()["courseSearchUrl"] == expected_catalog_link diff --git a/lms/djangoapps/mobile_api/tests/test_milestones.py b/lms/djangoapps/mobile_api/tests/test_milestones.py index 622f86f507ca..fbdccbb1fdf1 100644 --- a/lms/djangoapps/mobile_api/tests/test_milestones.py +++ b/lms/djangoapps/mobile_api/tests/test_milestones.py @@ -31,17 +31,14 @@ class MobileAPIMilestonesMixin: ALLOW_ACCESS_TO_MILESTONE_COURSE = False - @patch.dict(settings.FEATURES, { - 'ENABLE_PREREQUISITE_COURSES': True, - 'ENABLE_MKTG_SITE': True, - }) + @patch.dict(settings.FEATURES, {'ENABLE_PREREQUISITE_COURSES': True}) def test_unfulfilled_prerequisite_course(self): """ Tests the case for an unfulfilled pre-requisite course """ self._add_prerequisite_course() self.init_course_access() self._verify_unfulfilled_milestone_response() - @patch.dict(settings.FEATURES, {'ENABLE_PREREQUISITE_COURSES': True, 'ENABLE_MKTG_SITE': True}) + @patch.dict(settings.FEATURES, {'ENABLE_PREREQUISITE_COURSES': True}) def test_unfulfilled_prerequisite_course_for_staff(self): self._add_prerequisite_course() self.user.is_staff = True @@ -49,7 +46,7 @@ def test_unfulfilled_prerequisite_course_for_staff(self): self.init_course_access() self.api_response() - @patch.dict(settings.FEATURES, {'ENABLE_PREREQUISITE_COURSES': True, 'ENABLE_MKTG_SITE': True}) + @patch.dict(settings.FEATURES, {'ENABLE_PREREQUISITE_COURSES': True}) def test_fulfilled_prerequisite_course(self): """ Tests the case when a user fulfills existing pre-requisite course @@ -61,7 +58,6 @@ def test_fulfilled_prerequisite_course(self): self.api_response() @override_settings(ENTRANCE_EXAMS=True) - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) def test_unpassed_entrance_exam(self): """ Tests the case where the user has not passed the entrance exam @@ -71,7 +67,6 @@ def test_unpassed_entrance_exam(self): self._verify_unfulfilled_milestone_response() @override_settings(ENTRANCE_EXAMS=True) - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) def test_unpassed_entrance_exam_for_staff(self): self._add_entrance_exam() self.user.is_staff = True @@ -80,7 +75,6 @@ def test_unpassed_entrance_exam_for_staff(self): self.api_response() @override_settings(ENTRANCE_EXAMS=True) - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) def test_passed_entrance_exam(self): """ Tests access when user has passed the entrance exam diff --git a/lms/djangoapps/mobile_api/testutils.py b/lms/djangoapps/mobile_api/testutils.py index ab4a029a0fa5..95352e7cfc86 100644 --- a/lms/djangoapps/mobile_api/testutils.py +++ b/lms/djangoapps/mobile_api/testutils.py @@ -17,7 +17,6 @@ import ddt import pytz -from django.conf import settings from django.urls import reverse from django.utils import timezone from opaque_keys.edx.keys import CourseKey @@ -164,7 +163,6 @@ def init_course_access(self, course_id=None): """Base implementation of initializing the user for each test.""" self.login_and_enroll(course_id) - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) def test_success(self): self.init_course_access() @@ -178,7 +176,7 @@ def test_course_not_found(self): response = self.api_response(expected_response_code=None, course_id=non_existent_course_id) self.verify_failure(response) # allow subclasses to override verification - @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False, 'ENABLE_MKTG_SITE': True}) + @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) def test_unreleased_course(self): # ensure the course always starts in the future self.course = CourseFactory.create(mobile_available=True, static_asset_path="needed_for_split") @@ -194,7 +192,6 @@ def test_unreleased_course(self): (None, False) ) @ddt.unpack - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) def test_non_mobile_available(self, role, should_succeed): """ Tests that the MobileAvailabilityError() is raised for certain user @@ -222,7 +219,6 @@ def test_unenrolled_user(self): (None, False) ) @ddt.unpack - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) def test_visible_to_staff_only_course(self, role, should_succeed): self.init_course_access() self.course.visible_to_staff_only = True diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 748c50c44325..4c8808b2acc0 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -152,7 +152,6 @@ def verify_failure(self, response, error_type=None): courses = response.data assert len(courses) == 0 - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) @ddt.data(API_V05, API_V1, API_V2) def test_sort_order(self, api_version): self.login() @@ -175,7 +174,6 @@ def test_sort_order(self, api_version): @patch.dict(settings.FEATURES, { 'ENABLE_PREREQUISITE_COURSES': True, 'DISABLE_START_DATES': False, - 'ENABLE_MKTG_SITE': True, }) def test_courseware_access(self, api_version): self.login() @@ -235,7 +233,7 @@ def test_courseware_access(self, api_version): ('default_start_date', None, None, "empty", API_V2), ) @ddt.unpack - @patch.dict(settings.FEATURES, {'DISABLE_START_DATES': False, 'ENABLE_MKTG_SITE': True}) + @patch.dict(settings.FEATURES, {'DISABLE_START_DATES': False}) def test_start_type_and_display(self, start, advertised_start, expected_display, expected_type, api_version): """ Tests that the correct start_type and start_display are returned in the @@ -251,7 +249,7 @@ def test_start_type_and_display(self, start, advertised_start, expected_display, assert courses[0]['course']['start_display'] == expected_display @ddt.data(API_V05, API_V1, API_V2) - @patch.dict(settings.FEATURES, {"ENABLE_DISCUSSION_SERVICE": True, 'ENABLE_MKTG_SITE': True}) + @patch.dict(settings.FEATURES, {"ENABLE_DISCUSSION_SERVICE": True}) def test_discussion_url(self, api_version): self.login_and_enroll() @@ -1081,7 +1079,6 @@ def verify_pdf_certificate(self): certificate_data = response.data[0]['certificate'] assert certificate_data['url'] == certificate_url - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) def test_no_certificate(self): self.login_and_enroll() @@ -1089,21 +1086,21 @@ def test_no_certificate(self): certificate_data = response.data[0]['certificate'] self.assertDictEqual(certificate_data, {}) # noqa: PT009 - @patch.dict(settings.FEATURES, {'CERTIFICATES_HTML_VIEW': False, 'ENABLE_MKTG_SITE': True}) + @patch.dict(settings.FEATURES, {'CERTIFICATES_HTML_VIEW': False}) def test_pdf_certificate_with_html_cert_disabled(self): """ Tests PDF certificates with CERTIFICATES_HTML_VIEW set to True. """ self.verify_pdf_certificate() - @patch.dict(settings.FEATURES, {'CERTIFICATES_HTML_VIEW': True, 'ENABLE_MKTG_SITE': True}) + @patch.dict(settings.FEATURES, {'CERTIFICATES_HTML_VIEW': True}) def test_pdf_certificate_with_html_cert_enabled(self): """ Tests PDF certificates with CERTIFICATES_HTML_VIEW set to True. """ self.verify_pdf_certificate() - @patch.dict(settings.FEATURES, {'CERTIFICATES_HTML_VIEW': True, 'ENABLE_MKTG_SITE': True}) + @patch.dict(settings.FEATURES, {'CERTIFICATES_HTML_VIEW': True}) def test_web_certificate(self): self.login_and_enroll() @@ -1303,7 +1300,6 @@ def test_invalid_date(self): @ddt.ddt -@patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) class TestCourseEnrollmentSerializer(MobileAPITestCase, MilestonesTestCaseMixin): """ diff --git a/lms/djangoapps/static_template_view/tests/test_views.py b/lms/djangoapps/static_template_view/tests/test_views.py index ae96ab4380ad..49a72d485d02 100644 --- a/lms/djangoapps/static_template_view/tests/test_views.py +++ b/lms/djangoapps/static_template_view/tests/test_views.py @@ -7,62 +7,10 @@ from django.test import TestCase from django.urls import reverse -from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration_context - class MarketingSiteViewTests(TestCase): """ Tests for the marketing site views """ - def _test_view(self, view_name, mimetype): - """ - Gets a view and tests that it exists. - """ - resp = self.client.get(reverse(view_name)) - assert resp.status_code == 200 - assert resp['Content-Type'] == mimetype - - def test_sitemap(self): - """ - Test the sitemap view - """ - self._test_view('sitemap_xml', 'application/xml') - - def test_about(self): - """ - Test the about view - """ - self._test_view('about', 'text/html') - - def test_about_with_site_configuration(self): - """ - Test the about view with the header and content set in SiteConfiguration. - """ - test_header = "Very Unique Test Header" - test_content = "Very Unique Test Content" - test_header_key = 'static_template_about_header' - test_content_key = 'static_template_about_content' - response = None - configuration = {test_header_key: test_header, test_content_key: test_content} - with with_site_configuration_context(configuration=configuration): - response = self.client.get(reverse("about")) - self.assertContains(response, test_header) - self.assertContains(response, test_content) - - def test_about_with_site_configuration_and_html(self): - """ - Test the about view with html in the header. - """ - test_header = "Very Unique Test Header" - test_content = "Very Unique Test Content" - test_header_key = 'static_template_about_header' - test_content_key = 'static_template_about_content' - response = None - configuration = {test_header_key: test_header, test_content_key: test_content} - with with_site_configuration_context(configuration=configuration): - response = self.client.get(reverse("about")) - self.assertContains(response, test_header) - self.assertContains(response, test_content) - def test_404(self): """ Test the 404 view. diff --git a/lms/djangoapps/static_template_view/urls.py b/lms/djangoapps/static_template_view/urls.py index 644149c6dfcc..d29e193efc97 100644 --- a/lms/djangoapps/static_template_view/urls.py +++ b/lms/djangoapps/static_template_view/urls.py @@ -3,45 +3,13 @@ """ -from django.conf import settings from django.urls import path, re_path from lms.djangoapps.static_template_view import views urlpatterns = [ - path('blog', views.render, {'template': 'blog.html'}, name="blog"), - path('contact', views.render, {'template': 'contact.html'}, name="contact"), - path('donate', views.render, {'template': 'donate.html'}, name="donate"), - path('faq', views.render, {'template': 'faq.html'}, name="faq"), - path('help', views.render, {'template': 'help.html'}, name="help_edx"), - path('jobs', views.render, {'template': 'jobs.html'}, name="jobs"), - path('news', views.render, {'template': 'news.html'}, name="news"), - path('press', views.render, {'template': 'press.html'}, name="press"), - path('media-kit', views.render, {'template': 'media-kit.html'}, name="media-kit"), path('copyright', views.render, {'template': 'copyright.html'}, name="copyright"), # Press releases re_path(r'^press/([_a-zA-Z0-9-]+)$', views.render_press_release, name='press_release'), ] - -# Only enable URLs for those marketing links actually enabled in the -# settings. Disable URLs by marking them as None. -for key, value in settings.MKTG_URL_LINK_MAP.items(): - # Skip disabled URLs - if value is None: - continue - - # These urls are enabled separately - if key == "ROOT" or key == "COURSES": # pylint: disable=consider-using-in - continue - - # The MKTG_URL_LINK_MAP key specifies the template filename - template = key.lower() - if '.' not in template: - # Append STATIC_TEMPLATE_VIEW_DEFAULT_FILE_EXTENSION if - # no file extension was specified in the key - template = f"{template}.{settings.STATIC_TEMPLATE_VIEW_DEFAULT_FILE_EXTENSION}" - - # Make the assumption that the URL we want is the lowercased - # version of the map key - urlpatterns.append(re_path(r'^%s$' % key.lower(), views.render, {'template': template}, name=value)) # noqa: UP031 diff --git a/lms/djangoapps/static_template_view/views.py b/lms/djangoapps/static_template_view/views.py index 719fe99b2474..ed547cde45f9 100644 --- a/lms/djangoapps/static_template_view/views.py +++ b/lms/djangoapps/static_template_view/views.py @@ -58,9 +58,6 @@ def render(request, template): try: context = {} - # This is necessary for the dialog presented with the TOS in /register - if template == 'honor.html': - context['allow_iframing'] = True # Format Examples: static_template_about_header configuration_base = 'static_template_' + template.replace('.html', '').replace('-', '_') page_header = configuration_helpers.get_value(configuration_base + '_header') diff --git a/lms/djangoapps/support/views/contact_us.py b/lms/djangoapps/support/views/contact_us.py index f1e6af75b969..acb1dc122e62 100644 --- a/lms/djangoapps/support/views/contact_us.py +++ b/lms/djangoapps/support/views/contact_us.py @@ -8,7 +8,7 @@ from django.shortcuts import redirect from django.views.generic import View -from common.djangoapps.edxmako.shortcuts import render_to_response +from common.djangoapps.edxmako.shortcuts import marketing_link, render_to_response from common.djangoapps.student.models import CourseEnrollment from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.features.enterprise_support import api as enterprise_api @@ -23,7 +23,8 @@ def get(self, request): # pylint: disable=missing-function-docstring # If ZENDESK_URL is not defined, then it will redirect to the static contact page # to avoid 500 error that arises due to missing Zendesk configuration if not settings.ZENDESK_URL: - return redirect('contact') + contact_url = marketing_link('CONTACT') + return redirect(contact_url if contact_url != '#' else '/') if not configuration_helpers.get_value('CONTACT_US_PAGE', True): raise Http404 diff --git a/lms/envs/common.py b/lms/envs/common.py index 828c7874152b..615e91c1cbe8 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -390,13 +390,14 @@ # .. toggle_name: settings.ENABLE_COURSE_HOME_REDIRECT # .. toggle_implementation: DjangoSetting -# .. toggle_default: True -# .. toggle_description: When enabled, along with the ENABLE_MKTG_SITE feature toggle, users who attempt to access a -# course "about" page will be redirected to the course home url. +# .. toggle_default: False +# .. toggle_description: When enabled, users who attempt to access a course "about" page will be redirected to the +# course home url. Previously this only took effect when ENABLE_MKTG_SITE was also True; now it is the sole gate. +# Operators who relied on ENABLE_MKTG_SITE=True to activate this redirect should set this to True explicitly. # .. toggle_use_cases: open_edx # .. toggle_creation_date: 2019-01-15 # .. toggle_tickets: https://github.com/openedx/edx-platform/pull/19604 -ENABLE_COURSE_HOME_REDIRECT = True +ENABLE_COURSE_HOME_REDIRECT = False # .. toggle_name: settings.ENABLE_COMBINED_LOGIN_REGISTRATION_FOOTER # .. toggle_implementation: DjangoSetting @@ -2125,27 +2126,6 @@ 'DEEP_LINKING': True, } -######################### MARKETING SITE ############################### - -MKTG_URL_LINK_MAP.update({ # noqa: F405 - 'ABOUT': 'about', - 'CONTACT': 'contact', - 'FAQ': 'help', - 'COURSES': 'courses', - 'ROOT': 'root', - 'TOS': 'tos', - 'HONOR': 'honor', # If your site does not have an honor code, simply delete this line. - 'TOS_AND_HONOR': 'edx-terms-service', - 'PRIVACY': 'privacy', - 'PRESS': 'press', - 'BLOG': 'blog', - 'DONATE': 'donate', - 'SITEMAP.XML': 'sitemap_xml', - - # Verified Certificates - 'WHAT_IS_VERIFIED_CERT': 'verified-certificate', -}) - STATIC_TEMPLATE_VIEW_DEFAULT_FILE_EXTENSION = 'html' SEND_ACTIVATION_EMAIL_URL = '' diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index 1043016a6b2b..49a718edf9ce 100644 --- a/lms/envs/devstack.py +++ b/lms/envs/devstack.py @@ -423,7 +423,6 @@ def should_show_debug_toolbar(request): # pylint: disable=missing-function-docs # Toggle this off if you don't want anything to do with enterprise in devstack. ENABLE_ENTERPRISE_INTEGRATION = True -ENABLE_MKTG_SITE = os.environ.get('ENABLE_MARKETING_SITE', False) # noqa: F405 MARKETING_SITE_ROOT = os.environ.get('MARKETING_SITE_ROOT', 'http://localhost:8080') # noqa: F405 MKTG_URLS = { diff --git a/lms/envs/minimal.yml b/lms/envs/minimal.yml index f4f9bd8a8a8a..e01c3a6433cd 100644 --- a/lms/envs/minimal.yml +++ b/lms/envs/minimal.yml @@ -15,7 +15,6 @@ TRACKING_BACKENDS: {} EVENT_TRACKING_BACKENDS: {} JWT_AUTH: {} CELERY_QUEUES: {} -MKTG_URL_LINK_MAP: {} MKTG_URL_OVERRIDES: {} REST_FRAMEWORK: {} diff --git a/lms/envs/mock.yml b/lms/envs/mock.yml index e26ce4351b28..47edb551cc16 100644 --- a/lms/envs/mock.yml +++ b/lms/envs/mock.yml @@ -582,7 +582,6 @@ FEATURES: ENABLE_INTEGRITY_SIGNATURE: true ENABLE_MAX_FAILED_LOGIN_ATTEMPTS: true ENABLE_MKTG_EMAIL_OPT_IN: true - ENABLE_MKTG_SITE: true ENABLE_MOBILE_REST_API: true ENABLE_NEW_BULK_EMAIL_EXPERIENCE: true ENABLE_ORA_MOBILE_SUPPORT: true @@ -828,7 +827,6 @@ MKTG_URLS: TOS_AND_HONOR: hello TRADEMARKS: hello WHAT_IS_VERIFIED_CERT: hello -MKTG_URL_LINK_MAP: {} MKTG_URL_OVERRIDES: course-v1:Org+Course+Run: hello MOBILE_STORE_URLS: diff --git a/lms/envs/production.py b/lms/envs/production.py index b954841d59bc..0fa97a3ece1a 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -80,7 +80,6 @@ def get_env_setting(setting): 'EVENT_TRACKING_BACKENDS', 'JWT_AUTH', 'CELERY_QUEUES', - 'MKTG_URL_LINK_MAP', 'REST_FRAMEWORK', 'EVENT_BUS_PRODUCER_CONFIG', 'DEFAULT_FILE_STORAGE', @@ -175,8 +174,6 @@ def get_env_setting(setting): } ) -MKTG_URL_LINK_MAP.update(_YAML_TOKENS.get('MKTG_URL_LINK_MAP', {})) # noqa: F405 - # Timezone overrides TIME_ZONE = CELERY_TIMEZONE # noqa: F405 diff --git a/lms/envs/test.py b/lms/envs/test.py index 236781831a2d..d9f529e86938 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -201,28 +201,6 @@ ENABLE_MOBILE_REST_API = True ENABLE_VIDEO_ABSTRACTION_LAYER_API = True -######################### MARKETING SITE ############################### - -MKTG_URL_LINK_MAP = { - 'ABOUT': 'about', - 'CONTACT': 'contact', - 'HELP_CENTER': 'help-center', - 'COURSES': 'courses', - 'ROOT': 'root', - 'TOS': 'tos', - 'HONOR': 'honor', - 'PRIVACY': 'privacy', - 'CAREERS': 'careers', - 'NEWS': 'news', - 'PRESS': 'press', - 'BLOG': 'blog', - 'DONATE': 'donate', - 'SITEMAP.XML': 'sitemap_xml', - - # Verified Certificates - 'WHAT_IS_VERIFIED_CERT': 'verified-certificate', -} - SUPPORT_SITE_LINK = 'https://example.support.edx.org' PASSWORD_RESET_SUPPORT_LINK = 'https://support.example.com/password-reset-help.html' ACTIVATION_EMAIL_SUPPORT_LINK = 'https://support.example.com/activation-email-help.html' diff --git a/lms/templates/header/navbar-not-authenticated.html b/lms/templates/header/navbar-not-authenticated.html index b50e535acda6..6881c090cc6a 100644 --- a/lms/templates/header/navbar-not-authenticated.html +++ b/lms/templates/header/navbar-not-authenticated.html @@ -14,7 +14,6 @@ %> <% - mktg_site_enabled = static.get_value('ENABLE_MKTG_SITE', settings.FEATURES.get('ENABLE_MKTG_SITE', False)) courses_are_browsable = settings.FEATURES.get('COURSES_ARE_BROWSABLE') allows_login = not settings.FEATURES['DISABLE_LOGIN_BUTTON'] and not combined_login_and_register can_discover_courses = settings.FEATURES.get('ENABLE_COURSE_DISCOVERY') @@ -23,7 +22,6 @@ %>