From 9679dc696886923e93a350367672d1890f368e35 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 3 Jun 2026 15:45:47 -0400 Subject: [PATCH 01/10] feat: remove ENABLE_MKTG_SITE setting and simplify marketing URL resolution ENABLE_MKTG_SITE was a feature flag that selected between two URL resolution strategies: MKTG_URLS (external marketing site) vs MKTG_URL_LINK_MAP (local Django URL reversal). The local-template path is now legacy; all operators are expected to use MKTG_URLS. This commit removes the flag and its conditional branches, keeping the MKTG_URLS-based behaviour unconditionally: - Drop the setting definition and toggle annotation from openedx/envs/common.py - Remove the override from lms/envs/devstack.py and both mock.yml files - Simplify marketing_link() and is_marketing_link_set() in edxmako/shortcuts.py to always consult MKTG_URLS; drop the MKTG_URL_LINK_MAP fallback branches and the now-unused NoReverseMatch/reverse/set_custom_attribute/get_current_request_hostname imports - Update validate_marketing_site_setting() in checks.py to always validate MKTG_URLS rather than gating validation on ENABLE_MKTG_SITE - Trim edxmako/tests.py: remove False-path test variants and the MKTG_URL_LINK_MAP link-reversal test; each remaining test now exercises a single, unconditional code path Part of https://github.com/openedx/openedx-platform/issues/37053 Co-Authored-By: Claude Sonnet 4.6 --- cms/envs/mock.yml | 1 - common/djangoapps/edxmako/shortcuts.py | 50 ++--------- common/djangoapps/edxmako/tests.py | 90 ++++--------------- docs/docs_settings.py | 1 - lms/envs/common.py | 4 +- lms/envs/devstack.py | 1 - lms/envs/mock.yml | 1 - .../common_initialization/checks.py | 25 +++--- openedx/envs/common.py | 10 --- 9 files changed, 37 insertions(+), 146 deletions(-) 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/common/djangoapps/edxmako/shortcuts.py b/common/djangoapps/edxmako/shortcuts.py index e075d413fd11..ff85e7a4ba00 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,10 @@ 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. Falls back to '#' if the link is not configured. """ - # 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 +53,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 +68,9 @@ 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 '#' + + log.debug("Cannot find corresponding link for name: %s", name) + return '#' def is_any_marketing_link_set(names): @@ -112,20 +85,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): 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/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/envs/common.py b/lms/envs/common.py index e3b79d5bbca2..c388d9940542 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -401,8 +401,8 @@ # .. 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_description: When enabled, users who attempt to access a course "about" page will be redirected to the +# course home url. # .. toggle_use_cases: open_edx # .. toggle_creation_date: 2019-01-15 # .. toggle_tickets: https://github.com/openedx/edx-platform/pull/19604 diff --git a/lms/envs/devstack.py b/lms/envs/devstack.py index 4983260f03dd..e32b29df149f 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/mock.yml b/lms/envs/mock.yml index e26ce4351b28..5046975e52d3 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 diff --git a/openedx/core/djangoapps/common_initialization/checks.py b/openedx/core/djangoapps/common_initialization/checks.py index 3eb41ce48518..a3276dd17a9b 100644 --- a/openedx/core/djangoapps/common_initialization/checks.py +++ b/openedx/core/djangoapps/common_initialization/checks.py @@ -33,19 +33,18 @@ def validate_marketing_site_setting(app_configs, **kwargs): # pylint: disable=u Validates marketing site related settings. """ errors = [] - if settings.FEATURES.get('ENABLE_MKTG_SITE'): - if not hasattr(settings, 'MKTG_URLS'): - errors.append( - Error( - 'ENABLE_MKTG_SITE is True, but MKTG_URLS is not defined.', - id='common.djangoapps.common_initialization.E002', - ) + if not hasattr(settings, 'MKTG_URLS'): + errors.append( + Error( + 'MKTG_URLS is not defined.', + id='common.djangoapps.common_initialization.E002', ) - if not settings.MKTG_URLS.get('ROOT'): - errors.append( - Error( - 'There is no ROOT defined in MKTG_URLS.', - id='common.djangoapps.common_initialization.E003', - ) + ) + elif not settings.MKTG_URLS.get('ROOT'): + errors.append( + Error( + 'There is no ROOT defined in MKTG_URLS.', + id='common.djangoapps.common_initialization.E003', ) + ) return errors diff --git a/openedx/envs/common.py b/openedx/envs/common.py index f0b53db8a5a6..6e05412da9a8 100644 --- a/openedx/envs/common.py +++ b/openedx/envs/common.py @@ -1121,16 +1121,6 @@ def add_optional_apps(optional_apps, installed_apps): # .. toggle_tickets: https://github.com/openedx/edx-platform/pull/2749 EMBARGO = False -# .. toggle_name: ENABLE_MKTG_SITE -# .. toggle_implementation: DjangoSetting -# .. toggle_default: False -# .. toggle_description: Toggle to enable alternate urls for marketing links. -# .. toggle_use_cases: open_edx -# .. toggle_creation_date: 2014-03-24 -# .. toggle_warning: When this is enabled, the MKTG_URLS setting should be defined. The use case of this feature -# toggle is uncertain. -ENABLE_MKTG_SITE = False - # Expose Mobile REST API. ENABLE_MOBILE_REST_API = False From 9aef4bc82163cd01fb74557f8a2ddf05b8f1b804 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 4 Jun 2026 12:08:22 -0400 Subject: [PATCH 02/10] feat: remove ENABLE_MKTG_SITE from LMS branding views index() (served at /) now unconditionally checks MKTG_URLS['ROOT'] and redirects when it differs from LMS_ROOT_URL, matching the previous ENABLE_MKTG_SITE=True behaviour. The catalog MFE redirect (ENABLE_CATALOG_MICROFRONTEND) still fires first and is unaffected. Also fixes a pre-existing bug where log.error() in the NoReverseMatch handler was passed multiple f-strings as positional args, causing a TypeError when that branch was reached. courses() (served at /courses) now resolves in priority order: 1. Redirect to the catalog MFE if ENABLE_CATALOG_MICROFRONTEND is set 2. Permanent redirect to marketing_link('COURSES') from MKTG_URLS 3. Render the local course list via courseware.views.views.courses() The old ENABLE_MKTG_SITE gate and COURSES_ARE_BROWSABLE guard are removed. The local course list fallback (case 3) preserves behaviour for operators who have neither the catalog MFE nor MKTG_URLS['COURSES'] configured. The stanford-style theme footer is updated to use marketing_link() instead of reverse() for the 'about' and 'tos' URL routes, which are removed in a later commit. Tests drop their @override_settings(ENABLE_MKTG_SITE=True) decorators; the test names and assertions are otherwise unchanged. Co-Authored-By: Claude Sonnet 4.6 --- lms/djangoapps/branding/tests/test_api.py | 2 - lms/djangoapps/branding/tests/test_views.py | 2 - lms/djangoapps/branding/views.py | 48 ++++++------------- .../stanford-style/lms/templates/footer.html | 16 +++---- 4 files changed, 23 insertions(+), 45 deletions(-) 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/themes/stanford-style/lms/templates/footer.html b/themes/stanford-style/lms/templates/footer.html index afd4dfd4e41b..1a0986a1c48c 100644 --- a/themes/stanford-style/lms/templates/footer.html +++ b/themes/stanford-style/lms/templates/footer.html @@ -1,9 +1,9 @@ ## mako <%! from datetime import date - from django.urls import reverse from django.utils.translation import gettext as _ from openedx.core.djangoapps.lang_pref.api import footer_language_selector_is_enabled + from common.djangoapps.edxmako.shortcuts import marketing_link %> <%namespace name='static' file='static_content.html'/> @@ -12,13 +12,13 @@
From 01a53008df9079ce0697318afe7a26f9fd986a8c Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 4 Jun 2026 13:51:34 -0400 Subject: [PATCH 03/10] feat: remove ENABLE_MKTG_SITE from enrollment, courseware, and mobile API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - course_about_url in enrollment emails now uses get_link_for_about_page(), resolving the marketing URL, social sharing URL, or LMS about URL as a fallback - get_link_for_about_page() uses course.marketing_url unconditionally - has_marketing_url() returns bool(self.marketing_url) directly - url_to_enroll() always returns marketing_link('COURSES') - _course_home_redirect_enabled() only checks ENABLE_COURSE_HOME_REDIRECT; default changed from True to False because the flag previously had no effect when ENABLE_MKTG_SITE=False (the old default) — operators who relied on ENABLE_MKTG_SITE=True to activate the course-about redirect must now set ENABLE_COURSE_HOME_REDIRECT=True explicitly - Remove ENABLE_MKTG_SITE patches from courseware, instructor, and mobile API tests; merge redundant test cases where applicable BREAKING CHANGE: ENABLE_COURSE_HOME_REDIRECT now defaults to False. Operators who previously had ENABLE_MKTG_SITE=True and want the course about page to redirect to the learning MFE must explicitly set ENABLE_COURSE_HOME_REDIRECT=True in their configuration. Co-Authored-By: Claude Sonnet 4.6 --- common/djangoapps/util/course.py | 2 +- common/djangoapps/util/tests/test_course.py | 27 ++--- lms/djangoapps/courseware/tests/test_about.py | 17 +-- lms/djangoapps/courseware/views/views.py | 20 +--- lms/djangoapps/instructor/enrollment.py | 10 +- lms/djangoapps/instructor/tests/test_api.py | 107 +----------------- .../instructor/tests/test_enrollment.py | 20 +--- .../mobile_api/tests/test_milestones.py | 12 +- lms/djangoapps/mobile_api/testutils.py | 6 +- lms/djangoapps/mobile_api/users/tests.py | 10 +- lms/envs/common.py | 7 +- .../content/course_overviews/models.py | 2 +- 12 files changed, 40 insertions(+), 200 deletions(-) diff --git a/common/djangoapps/util/course.py b/common/djangoapps/util/course.py index 4e58997c684a..fa0b04d76651 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/lms/djangoapps/courseware/tests/test_about.py b/lms/djangoapps/courseware/tests/test_about.py index b34c29a7cb45..c91d07a29416 100644 --- a/lms/djangoapps/courseware/tests/test_about.py +++ b/lms/djangoapps/courseware/tests/test_about.py @@ -115,15 +115,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 @@ -135,19 +133,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 8ae58850920d..82a340ec1af5 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 c4c1a6c0b050..a9a6eee0d979 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 515e3f4bb02f..090195302717 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -887,6 +887,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' @override_settings(CUSTOM_COURSES_EDX=True) def setUp(self): @@ -903,7 +905,6 @@ def setUp(self): site, self.course_key ) - self.course_about_url = self.course_url + 'about' self.registration_url = f'https://{site}/register' @override_settings(CUSTOM_COURSES_EDX=True) @@ -940,13 +941,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 @@ -955,18 +954,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/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 1599cf1007e2..438d2ffaf628 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -148,7 +148,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() @@ -171,7 +170,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() @@ -231,7 +229,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 @@ -248,7 +246,6 @@ def test_start_type_and_display(self, start, advertised_start, expected_display, @ddt.data(API_V05, API_V1, API_V2) @override_settings(ENABLE_DISCUSSION_SERVICE=True) - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) def test_discussion_url(self, api_version): self.login_and_enroll() @@ -1078,7 +1075,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() @@ -1087,7 +1083,6 @@ def test_no_certificate(self): self.assertDictEqual(certificate_data, {}) # noqa: PT009 @override_settings(CERTIFICATES_HTML_VIEW=False) - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) def test_pdf_certificate_with_html_cert_disabled(self): """ Tests PDF certificates with CERTIFICATES_HTML_VIEW set to True. @@ -1095,7 +1090,6 @@ def test_pdf_certificate_with_html_cert_disabled(self): self.verify_pdf_certificate() @override_settings(CERTIFICATES_HTML_VIEW=True) - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) def test_pdf_certificate_with_html_cert_enabled(self): """ Tests PDF certificates with CERTIFICATES_HTML_VIEW set to True. @@ -1103,7 +1097,6 @@ def test_pdf_certificate_with_html_cert_enabled(self): self.verify_pdf_certificate() @override_settings(CERTIFICATES_HTML_VIEW=True) - @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True}) def test_web_certificate(self): self.login_and_enroll() @@ -1303,7 +1296,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/envs/common.py b/lms/envs/common.py index c388d9940542..b3ba17c129a4 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -400,13 +400,14 @@ # .. toggle_name: settings.ENABLE_COURSE_HOME_REDIRECT # .. toggle_implementation: DjangoSetting -# .. toggle_default: True +# .. toggle_default: False # .. toggle_description: When enabled, users who attempt to access a course "about" page will be redirected to the -# course home url. +# 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 diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index 3115999a87fb..c9aa305bb9b1 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/openedx/core/djangoapps/content/course_overviews/models.py @@ -567,7 +567,7 @@ def has_marketing_url(self): """ Returns whether the course has marketing url. """ - return settings.FEATURES.get('ENABLE_MKTG_SITE') and bool(self.marketing_url) + return bool(self.marketing_url) def has_social_sharing_url(self): """ From bb5e58873420a0c7df287a73222b203239e0aab0 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 4 Jun 2026 13:53:20 -0400 Subject: [PATCH 04/10] feat: remove ENABLE_MKTG_SITE from student email view Email change confirmation contact link now always comes from marketing_link('CONTACT') via MKTG_URLS. Remove the fallback that constructed a local URL when the flag was False. Co-Authored-By: Claude Sonnet 4.6 --- common/djangoapps/student/tests/test_email.py | 25 ++++--------------- common/djangoapps/student/tests/test_views.py | 1 - common/djangoapps/student/views/management.py | 10 +------- 3 files changed, 6 insertions(+), 30 deletions(-) diff --git a/common/djangoapps/student/tests/test_email.py b/common/djangoapps/student/tests/test_email.py index 8c4788a432ed..7ba4887dd91f 100644 --- a/common/djangoapps/student/tests/test_email.py +++ b/common/djangoapps/student/tests/test_email.py @@ -530,15 +530,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: @@ -589,17 +581,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..941baff49117 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': { diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 77483dd99c4e..a21a17de08c5 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) From b11341b5ba49ba202f3e23c239ede59e66c87aea Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 4 Jun 2026 14:42:40 -0400 Subject: [PATCH 05/10] feat: remove ENABLE_MKTG_SITE from CMS, registration tests, and docs - contentstore utils: marketing_enabled is always True - registration tests: remove ENABLE_MKTG_SITE=False test cases that tested now-dead local URL generation for honor code and TOS links; remove ENABLE_MKTG_SITE=True decorators from the remaining tests - site_configuration test mixin: remove ENABLE_MKTG_SITE example key - schedules README: remove ENABLE_MKTG_SITE from example config Co-Authored-By: Claude Sonnet 4.6 --- .../rest_api/v1/views/tests/test_settings.py | 2 +- cms/djangoapps/contentstore/utils.py | 6 +- .../core/djangoapps/schedules/docs/README.rst | 3 - .../site_configuration/tests/mixins.py | 1 - .../user_authn/views/tests/test_register.py | 81 ------------------- 5 files changed, 2 insertions(+), 91 deletions(-) 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 14aa7a6858a3..dbc5377dd40a 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -1387,11 +1387,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/openedx/core/djangoapps/schedules/docs/README.rst b/openedx/core/djangoapps/schedules/docs/README.rst index 3004704c9c6e..7670ee71d85c 100644 --- a/openedx/core/djangoapps/schedules/docs/README.rst +++ b/openedx/core/djangoapps/schedules/docs/README.rst @@ -208,9 +208,6 @@ add/change the following: .. code:: python - FEATURES = { - 'ENABLE_MKTG_SITE': True, - } MKTG_URLS = { 'ROOT': '', } diff --git a/openedx/core/djangoapps/site_configuration/tests/mixins.py b/openedx/core/djangoapps/site_configuration/tests/mixins.py index 82a74fe3a6d0..a2a5254767ec 100644 --- a/openedx/core/djangoapps/site_configuration/tests/mixins.py +++ b/openedx/core/djangoapps/site_configuration/tests/mixins.py @@ -33,7 +33,6 @@ def setUp(self): "SITE_NAME": self.site_other.domain, "SESSION_COOKIE_DOMAIN": self.site_other.domain, "course_org_filter": "fakeOtherX", - "ENABLE_MKTG_SITE": True, "SHOW_ECOMMERCE_REPORTS": True, "MKTG_URLS": { "ROOT": "https://marketing.fakeother", diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_register.py b/openedx/core/djangoapps/user_authn/views/tests/test_register.py index fb2ff9ef30a9..ed95fbc39342 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_register.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_register.py @@ -1127,7 +1127,6 @@ def test_registration_form_confirm_email(self): @override_settings( MKTG_URLS={"ROOT": "https://www.test.com/", "HONOR": "honor"}, ) - @mock.patch.dict(settings.FEATURES, {"ENABLE_MKTG_SITE": True}) def test_registration_honor_code_mktg_site_enabled(self): link_template = "{link_label}" link_template2 = "{link_label}" @@ -1159,44 +1158,11 @@ def test_registration_honor_code_mktg_site_enabled(self): } ) - @override_settings(MKTG_URLS_LINK_MAP={"HONOR": "honor"}) - @mock.patch.dict(settings.FEATURES, {"ENABLE_MKTG_SITE": False}) - def test_registration_honor_code_mktg_site_disabled(self): - link_template = "{link_label}" - link_label = "Terms of Service and Honor Code" - link_label2 = "Privacy Policy" - self._assert_reg_field( - {"honor_code": "required"}, - { - "label": ("By creating an account, you agree to the {spacing}" - "{link_label} {spacing}" - "and you acknowledge that {platform_name} and each Member process your " - "personal data in accordance {spacing}" - "with the {link_label2}.").format( - platform_name=settings.PLATFORM_NAME, - link_label=self.link_template.format(link_label=link_label), - link_label2=link_template.format(link_label=link_label2), - spacing=' ' * 18 - ), - "name": "honor_code", - "defaultValue": False, - "type": "plaintext", - "required": True, - "errorMessages": { - "required": "You must agree to the {platform_name} {link_label}".format( # noqa: UP032 - platform_name=settings.PLATFORM_NAME, - link_label=link_label - ) - } - } - ) - @override_settings(MKTG_URLS={ "ROOT": "https://www.test.com/", "HONOR": "honor", "TOS": "tos", }) - @mock.patch.dict(settings.FEATURES, {"ENABLE_MKTG_SITE": True}) def test_registration_separate_terms_of_service_mktg_site_enabled(self): # Honor code field should say ONLY honor code, # not "terms of service and honor code" @@ -1245,53 +1211,6 @@ def test_registration_separate_terms_of_service_mktg_site_enabled(self): } ) - @override_settings(MKTG_URLS_LINK_MAP={"HONOR": "honor", "TOS": "tos"}) - @mock.patch.dict(settings.FEATURES, {"ENABLE_MKTG_SITE": False}) - def test_registration_separate_terms_of_service_mktg_site_disabled(self): - # Honor code field should say ONLY honor code, - # not "terms of service and honor code" - link_label = 'Honor Code' - self._assert_reg_field( - {"honor_code": "required", "terms_of_service": "required"}, - { - "label": "I agree to the {platform_name} {link_label}".format( # noqa: UP032 - platform_name=settings.PLATFORM_NAME, - link_label=self.link_template.format(link_label=link_label) - ), - "name": "honor_code", - "defaultValue": False, - "type": "checkbox", - "required": True, - "errorMessages": { - "required": "You must agree to the {platform_name} Honor Code".format( # noqa: UP032 - platform_name=settings.PLATFORM_NAME - ) - } - } - ) - - link_label = 'Terms of Service' - # Terms of service field should also be present - link_template = "{link_label}" - self._assert_reg_field( - {"honor_code": "required", "terms_of_service": "required"}, - { - "label": "I agree to the {platform_name} {link_label}".format( # noqa: UP032 - platform_name=settings.PLATFORM_NAME, - link_label=link_template.format(link_label=link_label) - ), - "name": "terms_of_service", - "defaultValue": False, - "type": "checkbox", - "required": True, - "errorMessages": { - "required": "You must agree to the {platform_name} Terms of Service".format( # noqa: UP032 - platform_name=settings.PLATFORM_NAME - ) - } - } - ) - @override_settings( REGISTRATION_EXTRA_FIELDS={ "level_of_education": "optional", From 5df2302891606c8f9aa7e5373418442f72219e1a Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 4 Jun 2026 14:44:12 -0400 Subject: [PATCH 06/10] feat: remove ENABLE_MKTG_SITE guards from navbar templates Marketing navigation links (HOW_IT_WORKS, COURSES, SCHOOLS) are now always rendered in the unauthenticated nav bars. marketing_link() falls back to '#' when a URL is not configured in MKTG_URLS. Co-Authored-By: Claude Sonnet 4.6 --- .../header/navbar-not-authenticated.html | 3 --- .../navigation/navbar-not-authenticated.html | 20 +++++++++---------- 2 files changed, 9 insertions(+), 14 deletions(-) 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 @@ %>