context_processors: Use breadcrumbs to highlight current section

- We were using hacky logic of assuming that if a page using the URL
/plinth/sys/..., then it belongs to 'system' section based on the URL match.
This won't work when the URL does not follow this pattern for any reason.

- Instead use the breadcrumbs mechanism which uses menu items and URL names to
determine the section a page belongs to.

Tests:

- Visit page, apps page, system page, help pages, an app page in apps sections,
an app page in system section, backups -> create backup page and notice that the
correct section is highlighted.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>

plinth/context_processors.py

@@ -3,12 +3,10 @@
 Django context processors to provide common data to templates.
 """
 
-import re
-
 from django.utils.translation import gettext as _
 from django.utils.translation import gettext_noop
 
-from plinth import cfg, web_server
+from plinth import cfg, views, web_server
 from plinth.utils import is_user_admin
 
 
@@ -26,13 +24,15 @@ def common(request):
     notifications_context = Notification.get_display_context(
         request, user=request.user)
 
-    slash_indices = [match.start() for match in re.finditer('/', request.path)]
+    breadcrumbs = views.get_breadcrumbs(request)
-    active_menu_urls = [
+    active_section_url = [
-        request.path[:index + 1] for index in slash_indices[2:]
+        key for key, value in breadcrumbs.items()
-    ]  # Ignore the first two slashes '/plinth/apps/'
+        if value.get('is_active_section')
+    ][0]
     return {
         'cfg': cfg,
-        'active_menu_urls': active_menu_urls,
+        'breadcrumbs': breadcrumbs,
+        'active_section_url': active_section_url,
         'box_name': _(cfg.box_name),
         'user_is_admin': is_user_admin(request, True),
         'user_css': web_server.get_user_css(),

plinth/templates/base.html

@@ -88,8 +88,9 @@
       <nav class="navbar navbar-expand-md">
 
         {% block mainmenu_left %}
-          <a href="{% url 'index' %}"  class="navbar-brand
+          {% url 'index' as index_url %}
-                   {% if not active_menu_urls %}active{% endif %}"
+          <a href="{{ index_url }}"  class="navbar-brand
+                   {% if index_url == active_section_url %}active{% endif %}"
              title="{{ box_name }}">
             <i class="fa fa-freedombox fa-2x fa-inverse" aria-hidden="true"></i>
           </a>
@@ -116,7 +117,7 @@
                 <li class="nav-item">
                   {% url 'index' as index_url %}
                   <a href="{{ index_url }}" title='{% trans " Home" %}'
-                     class="nav-link {% if not active_menu_urls %}
+                     class="nav-link {% if index_url == active_section_url %}
                      active{% endif %}">
                     {% trans "Home" %}
                   </a>
@@ -124,7 +125,7 @@
                 <li class="nav-item">
                   {% url 'apps' as apps_url %}
                   <a href="{{ apps_url }}" title='{% trans " Apps" %}'
-                     class="nav-link {% if apps_url in active_menu_urls %}
+                     class="nav-link {% if apps_url == active_section_url %}
                      active{% endif %}">
                     <span class="fa fa-th nav-icon"></span>
                     {% trans "Apps" %}
@@ -133,7 +134,7 @@
                 <li class="nav-item">
                   {% url 'system' as system_url %}
                   <a href="{{ system_url }}" title='{% trans " System" %}'
-                     class="nav-link {% if system_url in active_menu_urls %}
+                     class="nav-link {% if system_url == active_section_url %}
                      active{% endif %}">
                     <span class="fa fa-cog nav-icon"></span>
                     {% trans "System" %}

plinth/tests/test_context_processors.py

@@ -6,7 +6,7 @@ Test module for custom context processors.
 from unittest.mock import MagicMock, Mock, patch
 
 import pytest
-from django.http import HttpRequest
+from django.urls import resolve
 
 from plinth import context_processors as cp
 from plinth import menu as menu_module
@@ -15,15 +15,21 @@ from plinth import menu as menu_module
 @pytest.fixture(name='menu', autouse=True)
 def fixture_menu():
     """Initialized menu module."""
+    menu_module.Menu._all_menus = set()
     menu_module.init()
+    menu_module.Menu('home-id', name='Home', url_name='index')
+    menu_module.Menu('apps-id', name='Apps', url_name='apps',
+                     parent_url_name='index')
+    menu_module.Menu('testapp-id', name='Test App', url_name='testapp:index',
+                     parent_url_name='apps')
 
 
 @patch('plinth.notification.Notification')
-def test_common(Notification, load_cfg):
+def test_common(Notification, load_cfg, rf):
     """Verify that the common() function returns the correct values."""
-
+    url = '/apps/testapp/create/'
-    request = HttpRequest()
+    request = rf.get(url)
-    request.path = '/plinth/aaa/bbb/ccc/'
+    request.resolver_match = resolve(url)
     request.user = Mock()
     request.user.groups.filter().exists = Mock(return_value=True)
     request.session = MagicMock()
@@ -33,31 +39,7 @@ def test_common(Notification, load_cfg):
     config = response['cfg']
     assert config is not None
     assert config.box_name == 'FreedomBox'
-
     assert response['box_name'] == 'FreedomBox'
-
+    assert len(response['breadcrumbs']) == 4
-    urls = response['active_menu_urls']
+    assert response['active_section_url'] == '/apps/'
-    assert urls is not None
-    assert ['/plinth/aaa/', '/plinth/aaa/bbb/', '/plinth/aaa/bbb/ccc/'] == urls
-
     assert response['user_is_admin']
-
-
-@patch('plinth.notification.Notification')
-def test_common_border_conditions(Notification):
-    """Verify that the common() function works for border conditions."""
-    request = HttpRequest()
-    request.path = ''
-    request.user = Mock()
-    request.user.groups.filter().exists = Mock(return_value=True)
-    request.session = MagicMock()
-    response = cp.common(request)
-    assert response['active_menu_urls'] == []
-
-    request.path = '/plinth/'
-    response = cp.common(request)
-    assert response['active_menu_urls'] == []
-
-    request.path = '/plinth/aaa/bbb/ccc'
-    response = cp.common(request)
-    assert response['active_menu_urls'] == ['/plinth/aaa/', '/plinth/aaa/bbb/']