Correct logic on building marketing links.
The logic used to has a special case for edge in the hostname which didn't really make sense. So instead we just check to see if the given url starts with http and if it does we return it directly. If it doesn't, then we try to convert it to a valid url and return that. If that fails we return . Add a new custom metric `unresolved_marketing_link` to track when we run into this scenario.
This commit is contained in:
@@ -24,6 +24,7 @@ from six.moves.urllib.parse import urljoin
|
||||
from django.core.validators import URLValidator
|
||||
from django.core.exceptions import ValidationError
|
||||
|
||||
from edx_django_utils.monitoring import set_custom_metric
|
||||
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
|
||||
from openedx.core.djangoapps.theming.helpers import is_request_in_themed_site
|
||||
from xmodule.util.xmodule_django import get_current_request_hostname
|
||||
@@ -87,13 +88,15 @@ def marketing_link(name):
|
||||
# don't try to reverse disabled marketing links
|
||||
if link_map[name] is not None:
|
||||
host_name = get_current_request_hostname()
|
||||
if all([host_name and 'edge' in host_name, 'http' in link_map[name]]):
|
||||
if link_map[name].startswith('http'):
|
||||
return link_map[name]
|
||||
else:
|
||||
try:
|
||||
return reverse(link_map[name])
|
||||
except NoReverseMatch:
|
||||
raise Http404
|
||||
log.debug(u"Cannot find corresponding link for name: %s", name)
|
||||
set_custom_metric('unresolved_marketing_link', name)
|
||||
return '#'
|
||||
else:
|
||||
log.debug(u"Cannot find corresponding link for name: %s", name)
|
||||
return '#'
|
||||
|
||||
@@ -98,6 +98,21 @@ class ShortcutsTests(UrlResetMixin, TestCase):
|
||||
link = marketing_link('TOS')
|
||||
self.assertEqual(link, expected_link)
|
||||
|
||||
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in 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 == '#'
|
||||
|
||||
|
||||
class AddLookupTests(TestCase):
|
||||
"""
|
||||
|
||||
Reference in New Issue
Block a user