Refactor how logged user navbar is constructed (#1732)

* Refactor tests to use django testing tools instead of using mock

* Create context processor to dinamically build user's navbar

* Refactor base html to use backend data to organize the navbar

pydotorg/context_processors.py

@@ -1,5 +1,5 @@
 from django.conf import settings
-from django.urls import resolve, Resolver404
+from django.urls import resolve, Resolver404, reverse
 
 
 def site_info(request):
@@ -28,3 +28,38 @@ def blog_url(request):
     return {
         'BLOG_URL': settings.PYTHON_BLOG_URL,
     }
+
+
+def user_nav_bar_links(request):
+    nav = {}
+    if request.user.is_authenticated:
+        user = request.user.username
+        nav = {
+            "account": {
+                "label": "Your Account",
+                "urls": [
+                    {"url": reverse("users:user_detail", args=[user]), "label": "View profile"},
+                    {"url": reverse("users:user_profile_edit"), "label": "Edit profile"},
+                    {"url": reverse("account_change_password"), "label": "Change password"},
+                ],
+            },
+            "psf_membership": {
+                "label": "Membership",
+                "urls": [
+                    {"url": reverse("users:user_nominations_view"), "label": "Nominations"},
+                ],
+            }
+        }
+
+        if request.user.has_membership:
+            nav["psf_membership"]['urls'].append({
+                "url": reverse("users:user_membership_edit"),
+                "label": "Edit PSF membership"
+            })
+        else:
+            nav["psf_membership"]['urls'].append({
+                "url": reverse("users:user_membership_create"),
+                "label": "Become a PSF member"
+            })
+
+    return {"USER_NAV_BAR": nav}

pydotorg/settings/base.py

@@ -103,6 +103,7 @@
                 'pydotorg.context_processors.url_name',
                 'pydotorg.context_processors.get_host_with_scheme',
                 'pydotorg.context_processors.blog_url',
+                'pydotorg.context_processors.user_nav_bar_links',
             ],
         },
     },

pydotorg/tests/test_context_processors.py

@@ -1,32 +1,93 @@
+from model_bakery import baker
+
+from django.urls import reverse
 from django.conf import settings
 from pydotorg import context_processors
-from django.test import TestCase
-
-
-class MockRequest:
-    def __init__(self, path):
-        self.path = path
-        super().__init__()
+from django.test import TestCase, RequestFactory
+from django.contrib.auth.models import AnonymousUser
 
 
 class TemplateProcessorsTestCase(TestCase):
+    def setUp(self):
+        self.factory = RequestFactory()
+
     def test_url_name(self):
-        mock_request = MockRequest(path='/inner/')
-        self.assertEqual({'URL_NAMESPACE': '', 'URL_NAME': 'inner'}, context_processors.url_name(mock_request))
+        request = self.factory.get('/inner/')
+        self.assertEqual({'URL_NAMESPACE': '', 'URL_NAME': 'inner'}, context_processors.url_name(request))
 
-        mock_request = MockRequest(path='/events/calendars/')
-        self.assertEqual({'URL_NAMESPACE': 'events', 'URL_NAME': 'events:calendar_list'}, context_processors.url_name(mock_request))
+        request = self.factory.get('/events/calendars/')
+        self.assertEqual({'URL_NAMESPACE': 'events', 'URL_NAME': 'events:calendar_list'}, context_processors.url_name(request))
 
-        mock_request = MockRequest(path='/getit-404/releases/3.3.3/not-an-actual-thing/')
-        self.assertEqual({}, context_processors.url_name(mock_request))
+        request = self.factory.get('/getit-404/releases/3.3.3/not-an-actual-thing/')
+        self.assertEqual({}, context_processors.url_name(request))
 
-        mock_request = MockRequest(path='/getit-404/releases/3.3.3/\r\n/')
-        self.assertEqual({}, context_processors.url_name(mock_request))
+        request = self.factory.get('/getit-404/releases/3.3.3/\r\n/')
+        self.assertEqual({}, context_processors.url_name(request))
 
-        mock_request = MockRequest(path='/nothing/here/')
-        self.assertEqual({}, context_processors.url_name(mock_request))
+        request = self.factory.get('/nothing/here/')
+        self.assertEqual({}, context_processors.url_name(request))
 
     def test_blog_url(self):
-        mock_request = MockRequest(path='/about/')
-        self.assertEqual({'BLOG_URL': settings.PYTHON_BLOG_URL}, context_processors.blog_url(mock_request))
+        request = self.factory.get('/about/')
+        self.assertEqual({'BLOG_URL': settings.PYTHON_BLOG_URL}, context_processors.blog_url(request))
+
+    def test_user_nav_bar_links_for_non_psf_members(self):
+        request = self.factory.get('/about/')
+        request.user = baker.make(settings.AUTH_USER_MODEL, username='foo')
+
+        expected_nav = {
+            "account": {
+                "label": "Your Account",
+                "urls": [
+                    {"url": reverse("users:user_detail", args=['foo']), "label": "View profile"},
+                    {"url": reverse("users:user_profile_edit"), "label": "Edit profile"},
+                    {"url": reverse("account_change_password"), "label": "Change password"},
+                ],
+            },
+            "psf_membership": {
+                "label": "Membership",
+                "urls": [
+                    {"url": reverse("users:user_nominations_view"), "label": "Nominations"},
+                    {"url": reverse("users:user_membership_create"), "label": "Become a PSF member"},
+                ],
+            }
+        }
+
+        self.assertEqual(
+            {"USER_NAV_BAR": expected_nav},
+            context_processors.user_nav_bar_links(request)
+        )
+
+    def test_user_nav_bar_links_for_psf_members(self):
+        request = self.factory.get('/about/')
+        request.user = baker.make(settings.AUTH_USER_MODEL, username='foo')
+        baker.make('users.Membership', creator=request.user)
+
+        expected_nav = {
+            "account": {
+                "label": "Your Account",
+                "urls": [
+                    {"url": reverse("users:user_detail", args=['foo']), "label": "View profile"},
+                    {"url": reverse("users:user_profile_edit"), "label": "Edit profile"},
+                    {"url": reverse("account_change_password"), "label": "Change password"},
+                ],
+            },
+            "psf_membership": {
+                "label": "Membership",
+                "urls": [
+                    {"url": reverse("users:user_nominations_view"), "label": "Nominations"},
+                    {"url": reverse("users:user_membership_edit"), "label": "Edit PSF membership"},
+                ],
+            }
+        }
+
+        self.assertEqual(
+            {"USER_NAV_BAR": expected_nav},
+            context_processors.user_nav_bar_links(request)
+        )
+
+    def test_user_nav_bar_links_for_anonymous_user(self):
+        request = self.factory.get('/about/')
+        request.user = AnonymousUser()
 
+        self.assertEqual({"USER_NAV_BAR": {}}, context_processors.user_nav_bar_links(request))

templates/users/base.html

@@ -14,23 +14,18 @@
 
 
 {% block content %}
-
-    {% if request.user.is_authenticated %}
+  {% if USER_NAV_BAR %}
     <div class="user-profile-controls">
         <ul class="menu">
-            <li>Manage your Information: </li>
-            <li><a href="{% url 'users:user_detail' slug=request.user.username %}">View profile</a></li>
-            <li><a href="{% url 'users:user_nominations_view' %}">Nominations</a></li>
-            <li><a href="{% url 'users:user_profile_edit' %}">Edit profile</a></li>
-            {% if request.user.has_membership %}
-            <li><a href="{% url 'users:user_membership_edit' %}">Edit PSF membership</a></li>
-            {% else %}
-            <li><a href="{% url 'users:user_membership_create' %}">Become a PSF member</a></li>
-            {% endif %}
-            <li><a href="{% url 'account_change_password' %}">Change Password</a></li>
+          <li>Manage your Information: </li>
+          {% for section in USER_NAV_BAR.values %}
+            {% for url in section.urls %}
+            <li><a href="{{ url.url }}">{{ url.label }}</a></li>
+            {% endfor %}
+          {% endfor %}
         </ul>
     </div>
-    {% endif %}
     {% block user_content %}{% endblock %}
+  {% endif %}
 
 {% endblock %}