Skip to content

Commit b982e44

Browse files
feanilclaude
andcommitted
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 #37053 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 0f57cf2 commit b982e44

9 files changed

Lines changed: 37 additions & 146 deletions

File tree

cms/envs/mock.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,6 @@ FEATURES:
426426
ENABLE_LTI_PROVIDER: true
427427
ENABLE_MAX_FAILED_LOGIN_ATTEMPTS: true
428428
ENABLE_MKTG_EMAIL_OPT_IN: true
429-
ENABLE_MKTG_SITE: true
430429
ENABLE_MOBILE_REST_API: true
431430
ENABLE_PASSWORD_RESET_FAILURE_EMAIL: true
432431
ENABLE_PROCTORED_EXAMS: true

common/djangoapps/edxmako/shortcuts.py

Lines changed: 7 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -20,33 +20,20 @@
2020
from django.core.validators import URLValidator
2121
from django.http import HttpResponse # pylint: disable=unused-import
2222
from django.template import engines
23-
from django.urls import NoReverseMatch, reverse
24-
from edx_django_utils.monitoring import set_custom_attribute
2523
from six.moves.urllib.parse import urljoin
2624

2725
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
28-
from xmodule.util.xmodule_django import get_current_request_hostname # pylint: disable=wrong-import-order
2926

3027
from . import Engines
3128

3229
log = logging.getLogger(__name__)
3330

3431

3532
def marketing_link(name):
36-
"""Returns the correct URL for a link to the marketing site
37-
depending on if the marketing site is enabled
33+
"""Returns the URL for a marketing site link from MKTG_URLS.
3834
39-
Since the marketing site is enabled by a setting, we have two
40-
possible URLs for certain links. This function is to decides
41-
which URL should be provided.
35+
MKTG_URL_OVERRIDES take priority. Falls back to '#' if the link is not configured.
4236
"""
43-
# link_map maps URLs from the marketing site to the old equivalent on
44-
# the Django site
45-
link_map = settings.MKTG_URL_LINK_MAP
46-
enable_mktg_site = configuration_helpers.get_value(
47-
'ENABLE_MKTG_SITE',
48-
settings.FEATURES.get('ENABLE_MKTG_SITE', False)
49-
)
5037
marketing_urls = configuration_helpers.get_value(
5138
'MKTG_URLS',
5239
settings.MKTG_URLS
@@ -66,7 +53,7 @@ def marketing_link(name):
6653
log.debug("Invalid link set for link %s: %s", name, err)
6754
return '#'
6855

69-
if enable_mktg_site and name in marketing_urls:
56+
if name in marketing_urls:
7057
# special case for when we only want the root marketing URL
7158
if name == 'ROOT':
7259
return marketing_urls.get('ROOT')
@@ -81,23 +68,9 @@ def marketing_link(name):
8168
# URLs in the MKTG_URLS setting
8269
# e.g. urljoin('https://marketing.com', 'https://open-edx.org/about') >>> 'https://open-edx.org/about'
8370
return urljoin(marketing_urls.get('ROOT'), marketing_urls.get(name))
84-
# only link to the old pages when the marketing site isn't on
85-
elif not enable_mktg_site and name in link_map:
86-
# don't try to reverse disabled marketing links
87-
if link_map[name] is not None:
88-
host_name = get_current_request_hostname() # pylint: disable=unused-variable # noqa: F841
89-
if link_map[name].startswith('http'):
90-
return link_map[name]
91-
else:
92-
try:
93-
return reverse(link_map[name])
94-
except NoReverseMatch:
95-
log.debug("Cannot find corresponding link for name: %s", name)
96-
set_custom_attribute('unresolved_marketing_link', name)
97-
return '#'
98-
else:
99-
log.debug("Cannot find corresponding link for name: %s", name)
100-
return '#'
71+
72+
log.debug("Cannot find corresponding link for name: %s", name)
73+
return '#'
10174

10275

10376
def is_any_marketing_link_set(names):
@@ -112,20 +85,11 @@ def is_marketing_link_set(name):
11285
"""
11386
Returns a boolean if a given named marketing link is configured.
11487
"""
115-
116-
enable_mktg_site = configuration_helpers.get_value(
117-
'ENABLE_MKTG_SITE',
118-
settings.FEATURES.get('ENABLE_MKTG_SITE', False)
119-
)
12088
marketing_urls = configuration_helpers.get_value(
12189
'MKTG_URLS',
12290
settings.MKTG_URLS
12391
)
124-
125-
if enable_mktg_site:
126-
return name in marketing_urls
127-
else:
128-
return name in settings.MKTG_URL_LINK_MAP
92+
return name in marketing_urls
12993

13094

13195
def marketing_link_context_processor(request):

common/djangoapps/edxmako/tests.py

Lines changed: 16 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,10 @@
33
from unittest.mock import Mock, patch
44

55
import ddt
6-
from django.conf import settings
76
from django.http import HttpResponse
87
from django.test import TestCase
98
from django.test.client import RequestFactory
109
from django.test.utils import override_settings
11-
from django.urls import reverse
1210
from edx_django_utils.cache import RequestCache
1311

1412
from common.djangoapps.edxmako import LOOKUP, add_lookup
@@ -33,92 +31,36 @@ class ShortcutsTests(UrlResetMixin, TestCase):
3331

3432
@override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'ABOUT': '/about-us'})
3533
def test_marketing_link(self):
36-
with override_settings(MKTG_URL_LINK_MAP={'ABOUT': self._get_test_url_name()}):
37-
# test marketing site on
38-
with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}):
39-
expected_link = 'https://dummy-root/about-us'
40-
link = marketing_link('ABOUT')
41-
assert link == expected_link
42-
# test marketing site off
43-
with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}):
44-
expected_link = reverse(self._get_test_url_name())
45-
link = marketing_link('ABOUT')
46-
assert link == expected_link
34+
expected_link = 'https://dummy-root/about-us'
35+
link = marketing_link('ABOUT')
36+
assert link == expected_link
37+
38+
@override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'ABOUT': '/about-us'})
39+
def test_marketing_link_unconfigured(self):
40+
assert marketing_link('NOT_CONFIGURED') == '#'
4741

4842
@override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'ABOUT': '/about-us'})
4943
def test_is_marketing_link_set(self):
50-
with override_settings(MKTG_URL_LINK_MAP={'ABOUT': self._get_test_url_name()}):
51-
# test marketing site on
52-
with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}):
53-
assert is_marketing_link_set('ABOUT')
54-
assert not is_marketing_link_set('NOT_CONFIGURED')
55-
# test marketing site off
56-
with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}):
57-
assert is_marketing_link_set('ABOUT')
58-
assert not is_marketing_link_set('NOT_CONFIGURED')
44+
assert is_marketing_link_set('ABOUT')
45+
assert not is_marketing_link_set('NOT_CONFIGURED')
5946

6047
@override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'ABOUT': '/about-us'})
6148
def test_is_any_marketing_link_set(self):
62-
with override_settings(MKTG_URL_LINK_MAP={'ABOUT': self._get_test_url_name()}):
63-
# test marketing site on
64-
with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}):
65-
assert is_any_marketing_link_set(['ABOUT'])
66-
assert is_any_marketing_link_set(['ABOUT', 'NOT_CONFIGURED'])
67-
assert not is_any_marketing_link_set(['NOT_CONFIGURED'])
68-
# test marketing site off
69-
with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}):
70-
assert is_any_marketing_link_set(['ABOUT'])
71-
assert is_any_marketing_link_set(['ABOUT', 'NOT_CONFIGURED'])
72-
assert not is_any_marketing_link_set(['NOT_CONFIGURED'])
73-
74-
def _get_test_url_name(self): # pylint: disable=missing-function-docstring
75-
if settings.ROOT_URLCONF == 'lms.urls':
76-
# return any lms url name
77-
return 'dashboard'
78-
else:
79-
# return any cms url name
80-
return 'organizations'
49+
assert is_any_marketing_link_set(['ABOUT'])
50+
assert is_any_marketing_link_set(['ABOUT', 'NOT_CONFIGURED'])
51+
assert not is_any_marketing_link_set(['NOT_CONFIGURED'])
8152

8253
@override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'TOS': '/tos'})
8354
@override_settings(MKTG_URL_OVERRIDES={'TOS': 'https://edx.org'})
8455
def test_override_marketing_link_valid(self):
85-
expected_link = 'https://edx.org'
86-
# test marketing site on
87-
with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}):
88-
link = marketing_link('TOS')
89-
assert link == expected_link
90-
# test marketing site off
91-
with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}):
92-
link = marketing_link('TOS')
93-
assert link == expected_link
56+
link = marketing_link('TOS')
57+
assert link == 'https://edx.org'
9458

9559
@override_settings(MKTG_URLS={'ROOT': 'https://dummy-root', 'TOS': '/tos'})
9660
@override_settings(MKTG_URL_OVERRIDES={'TOS': '123456'})
9761
def test_override_marketing_link_invalid(self):
98-
expected_link = '#'
99-
# test marketing site on
100-
with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': True}):
101-
link = marketing_link('TOS')
102-
assert link == expected_link
103-
# test marketing site off
104-
with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}):
105-
link = marketing_link('TOS')
106-
assert link == expected_link
107-
108-
@skip_unless_lms
109-
def test_link_map_url_reverse(self):
110-
url_link_map = {
111-
'ABOUT': 'dashboard',
112-
'BAD_URL': 'foobarbaz',
113-
}
114-
115-
with patch.dict('django.conf.settings.FEATURES', {'ENABLE_MKTG_SITE': False}):
116-
with override_settings(MKTG_URL_LINK_MAP=url_link_map):
117-
link = marketing_link('ABOUT')
118-
assert link == '/dashboard'
119-
120-
link = marketing_link('BAD_URL')
121-
assert link == '#'
62+
link = marketing_link('TOS')
63+
assert link == '#'
12264

12365

12466
class AddLookupTests(TestCase):

docs/docs_settings.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
# Settings that will fail if we enable them, and we don't need them for docs anyway.
3030
FEATURES["RUN_AS_ANALYTICS_SERVER_ENABLED"] = False # noqa: F405
3131
FEATURES["ENABLE_SOFTWARE_SECURE_FAKE"] = False # noqa: F405
32-
FEATURES["ENABLE_MKTG_SITE"] = False # noqa: F405
3332

3433
INSTALLED_APPS.extend( # noqa: F405
3534
[

lms/envs/common.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -401,8 +401,8 @@
401401
# .. toggle_name: settings.ENABLE_COURSE_HOME_REDIRECT
402402
# .. toggle_implementation: DjangoSetting
403403
# .. toggle_default: True
404-
# .. toggle_description: When enabled, along with the ENABLE_MKTG_SITE feature toggle, users who attempt to access a
405-
# course "about" page will be redirected to the course home url.
404+
# .. toggle_description: When enabled, users who attempt to access a course "about" page will be redirected to the
405+
# course home url.
406406
# .. toggle_use_cases: open_edx
407407
# .. toggle_creation_date: 2019-01-15
408408
# .. toggle_tickets: https://github.com/openedx/edx-platform/pull/19604

lms/envs/devstack.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,6 @@ def should_show_debug_toolbar(request): # pylint: disable=missing-function-docs
423423
# Toggle this off if you don't want anything to do with enterprise in devstack.
424424
ENABLE_ENTERPRISE_INTEGRATION = True
425425

426-
ENABLE_MKTG_SITE = os.environ.get('ENABLE_MARKETING_SITE', False) # noqa: F405
427426
MARKETING_SITE_ROOT = os.environ.get('MARKETING_SITE_ROOT', 'http://localhost:8080') # noqa: F405
428427

429428
MKTG_URLS = {

lms/envs/mock.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,6 @@ FEATURES:
582582
ENABLE_INTEGRITY_SIGNATURE: true
583583
ENABLE_MAX_FAILED_LOGIN_ATTEMPTS: true
584584
ENABLE_MKTG_EMAIL_OPT_IN: true
585-
ENABLE_MKTG_SITE: true
586585
ENABLE_MOBILE_REST_API: true
587586
ENABLE_NEW_BULK_EMAIL_EXPERIENCE: true
588587
ENABLE_ORA_MOBILE_SUPPORT: true

openedx/core/djangoapps/common_initialization/checks.py

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,18 @@ def validate_marketing_site_setting(app_configs, **kwargs): # pylint: disable=u
3333
Validates marketing site related settings.
3434
"""
3535
errors = []
36-
if settings.FEATURES.get('ENABLE_MKTG_SITE'):
37-
if not hasattr(settings, 'MKTG_URLS'):
38-
errors.append(
39-
Error(
40-
'ENABLE_MKTG_SITE is True, but MKTG_URLS is not defined.',
41-
id='common.djangoapps.common_initialization.E002',
42-
)
36+
if not hasattr(settings, 'MKTG_URLS'):
37+
errors.append(
38+
Error(
39+
'MKTG_URLS is not defined.',
40+
id='common.djangoapps.common_initialization.E002',
4341
)
44-
if not settings.MKTG_URLS.get('ROOT'):
45-
errors.append(
46-
Error(
47-
'There is no ROOT defined in MKTG_URLS.',
48-
id='common.djangoapps.common_initialization.E003',
49-
)
42+
)
43+
elif not settings.MKTG_URLS.get('ROOT'):
44+
errors.append(
45+
Error(
46+
'There is no ROOT defined in MKTG_URLS.',
47+
id='common.djangoapps.common_initialization.E003',
5048
)
49+
)
5150
return errors

openedx/envs/common.py

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,16 +1121,6 @@ def add_optional_apps(optional_apps, installed_apps):
11211121
# .. toggle_tickets: https://github.com/openedx/edx-platform/pull/2749
11221122
EMBARGO = False
11231123

1124-
# .. toggle_name: ENABLE_MKTG_SITE
1125-
# .. toggle_implementation: DjangoSetting
1126-
# .. toggle_default: False
1127-
# .. toggle_description: Toggle to enable alternate urls for marketing links.
1128-
# .. toggle_use_cases: open_edx
1129-
# .. toggle_creation_date: 2014-03-24
1130-
# .. toggle_warning: When this is enabled, the MKTG_URLS setting should be defined. The use case of this feature
1131-
# toggle is uncertain.
1132-
ENABLE_MKTG_SITE = False
1133-
11341124
# Expose Mobile REST API.
11351125
ENABLE_MOBILE_REST_API = False
11361126

0 commit comments

Comments
 (0)