From d605907bbec007484f53202caa999eb427183ee9 Mon Sep 17 00:00:00 2001
From: Sunil Mohan Adapa <sunil@medhas.org>
Date: Tue, 15 Oct 2024 15:45:16 -0700
Subject: [PATCH] context_processors: Use active menu urls to decide what to
 highlight

- We are using submenu.url to check for specific URLs and then highlight a menu
item. This is somewhat incorrect due to string search and not generic enough. We
have another mechanism 'active_menu_urls' to perform this. Improve and use this
instead.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
---
 plinth/context_processors.py            |  4 +++-
 plinth/templates/base.html              |  8 ++++----
 plinth/tests/test_context_processors.py | 12 ++++++------
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/plinth/context_processors.py b/plinth/context_processors.py
index 01a4bbf8b..ddfc2af0f 100644
--- a/plinth/context_processors.py
+++ b/plinth/context_processors.py
@@ -27,7 +27,9 @@ def common(request):
         request, user=request.user)
 
     slash_indices = [match.start() for match in re.finditer('/', request.path)]
-    active_menu_urls = [request.path[:index + 1] for index in slash_indices]
+    active_menu_urls = [
+        request.path[:index + 1] for index in slash_indices[2:]
+    ]  # Ignore the first two slashes '/plinth/apps/'
     return {
         'cfg': cfg,
         'submenu': menu.main_menu.active_item(request),
diff --git a/plinth/templates/base.html b/plinth/templates/base.html
index eb91fc4b6..680d93184 100644
--- a/plinth/templates/base.html
+++ b/plinth/templates/base.html
@@ -83,7 +83,7 @@
 
         {% block mainmenu_left %}
           <a href="{% url 'index' %}"  class="navbar-brand
-                   {% if not submenu.url %} menu_link_active {% else %}
+                   {% if not active_menu_urls %} menu_link_active {% else %}
                    menu_link {% endif %}" title="{{ box_name }}">
             <i class="fa fa-freedombox fa-2x fa-inverse" aria-hidden="true"></i>
           </a>
@@ -110,7 +110,7 @@
                 <li class="nav-item">
                   {% url 'index' as index_url %}
                   <a href="{{ index_url }}" title='{% trans " Home" %}'
-                     class="nav-link {% if not submenu.url %}
+                     class="nav-link {% if not active_menu_urls %}
                      menu_link_active {% else %} menu_link {% endif %}">
                     {% trans "Home" %}
                   </a>
@@ -118,7 +118,7 @@
                 <li class="nav-item">
                   {% url 'apps' as apps_url %}
                   <a href="{{ apps_url }}" title='{% trans " Apps" %}'
-                     class="nav-link {% if apps_url == submenu.url %}
+                     class="nav-link {% if apps_url in active_menu_urls %}
                      menu_link_active {% else %} menu_link {% endif %}">
                     <span class="fa fa-th nav-icon"></span>
                     {% trans "Apps" %}
@@ -127,7 +127,7 @@
                 <li class="nav-item">
                   {% url 'system' as system_url %}
                   <a href="{{ system_url }}" title='{% trans " System" %}'
-                     class="nav-link {% if system_url == submenu.url %}
+                     class="nav-link {% if system_url in active_menu_urls %}
                      menu_link_active {% else %} menu_link {% endif %}">
                     <span class="fa fa-cog nav-icon"></span>
                     {% trans "System" %}
diff --git a/plinth/tests/test_context_processors.py b/plinth/tests/test_context_processors.py
index 168811274..50f7e9e13 100644
--- a/plinth/tests/test_context_processors.py
+++ b/plinth/tests/test_context_processors.py
@@ -23,7 +23,7 @@ def test_common(Notification, load_cfg):
     """Verify that the common() function returns the correct values."""
 
     request = HttpRequest()
-    request.path = '/aaa/bbb/ccc/'
+    request.path = '/plinth/aaa/bbb/ccc/'
     request.user = Mock()
     request.user.groups.filter().exists = Mock(return_value=True)
     request.session = MagicMock()
@@ -41,7 +41,7 @@ def test_common(Notification, load_cfg):
 
     urls = response['active_menu_urls']
     assert urls is not None
-    assert ['/', '/aaa/', '/aaa/bbb/', '/aaa/bbb/ccc/'] == urls
+    assert ['/plinth/aaa/', '/plinth/aaa/bbb/', '/plinth/aaa/bbb/ccc/'] == urls
 
     assert response['user_is_admin']
 
@@ -57,10 +57,10 @@ def test_common_border_conditions(Notification):
     response = cp.common(request)
     assert response['active_menu_urls'] == []
 
-    request.path = '/'
+    request.path = '/plinth/'
     response = cp.common(request)
-    assert response['active_menu_urls'] == ['/']
+    assert response['active_menu_urls'] == []
 
-    request.path = '/aaa/bbb'
+    request.path = '/plinth/aaa/bbb/ccc'
     response = cp.common(request)
-    assert response['active_menu_urls'] == ['/', '/aaa/']
+    assert response['active_menu_urls'] == ['/plinth/aaa/', '/plinth/aaa/bbb/']