From 4ac2c5e92b34904d5a54f3e76993a3ac646e09bb Mon Sep 17 00:00:00 2001
From: Feanil Patel <feanil@edx.org>
Date: Thu, 9 Jul 2020 13:35:23 -0400
Subject: [PATCH] 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.
---
 common/djangoapps/edxmako/shortcuts.py |  7 +++++--
 common/djangoapps/edxmako/tests.py     | 15 +++++++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/common/djangoapps/edxmako/shortcuts.py b/common/djangoapps/edxmako/shortcuts.py
index 08a7c6c98c..8c37739594 100644
--- a/common/djangoapps/edxmako/shortcuts.py
+++ b/common/djangoapps/edxmako/shortcuts.py
@@ -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 '#'
diff --git a/common/djangoapps/edxmako/tests.py b/common/djangoapps/edxmako/tests.py
index 1ef7b2449a..e3f630c349 100644
--- a/common/djangoapps/edxmako/tests.py
+++ b/common/djangoapps/edxmako/tests.py
@@ -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):
     """