From 00e42a42b9d40384c4876d96e5637ddf220e4e37 Mon Sep 17 00:00:00 2001
From: Sunil Mohan Adapa <sunil@medhas.org>
Date: Mon, 15 Nov 2021 12:51:12 -0800
Subject: [PATCH] menu: Avoid reversing URL during Menu component construction

This allows app initialization to happen without Django being configured.

Tests:

- Update unit tests.

- Visit /app, /system and /help pages. All the icons are listed properly.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>
---
 plinth/menu.py            | 29 ++++++++++++-----------
 plinth/tests/test_menu.py | 49 +++++++--------------------------------
 2 files changed, 23 insertions(+), 55 deletions(-)

diff --git a/plinth/menu.py b/plinth/menu.py
index 97c811c7e..cee9bcfc6 100644
--- a/plinth/menu.py
+++ b/plinth/menu.py
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
 
-from django.urls import reverse, reverse_lazy
+from django.urls import reverse_lazy
 
 from plinth import app
 
@@ -8,7 +8,7 @@ from plinth import app
 class Menu(app.FollowerComponent):
     """Component to manage a single menu item."""
 
-    _all_menus = {}
+    _all_menus = set()
 
     def __init__(self, component_id, name=None, short_description=None,
                  icon=None, url_name=None, url_args=None, url_kwargs=None,
@@ -57,21 +57,22 @@ class Menu(app.FollowerComponent):
         self.url = url
         self.order = order
         self.advanced = advanced
-        self.items = []
 
-        # Add self to parent menu item
-        if parent_url_name:
-            parent_menu = self.get(parent_url_name)
-            parent_menu.items.append(self)
+        self.url_name = url_name
+        self.url_args = url_args
+        self.url_kwargs = url_kwargs
+        self.parent_url_name = parent_url_name
 
-        # Add self to global list of menu items
-        self._all_menus[url] = self
+        # Add self to global list of menu items.
+        self._all_menus.add(self)
 
-    @classmethod
-    def get(cls, urlname, url_args=None, url_kwargs=None):
-        """Return a menu item with given URL name."""
-        url = reverse(urlname, args=url_args, kwargs=url_kwargs)
-        return cls._all_menus[url]
+    @property
+    def items(self):
+        """Return the list of children for this menu item."""
+        return [
+            item for item in self._all_menus
+            if item.parent_url_name == self.url_name
+        ]
 
     def sorted_items(self):
         """Return menu items in sorted order according to current locale."""
diff --git a/plinth/tests/test_menu.py b/plinth/tests/test_menu.py
index bc953f87c..6c6e48997 100644
--- a/plinth/tests/test_menu.py
+++ b/plinth/tests/test_menu.py
@@ -47,20 +47,20 @@ def build_menu(size=5):
 @pytest.fixture(name='empty_menus', autouse=True)
 def fixture_empty_menus():
     """Remove all menu entries before starting a test."""
-    Menu._all_menus = {}
+    Menu._all_menus = set()
 
 
-def test_init():
+def test_init(rf):
     """Verify that main_menu and essential items are created."""
     menu_module.init()
     main_menu = menu_module.main_menu
     assert isinstance(main_menu, Menu)
 
-    apps_menu = main_menu.get('apps')
+    apps_menu = main_menu.active_item(rf.get('/apps/foo/'))
     assert apps_menu.icon == 'fa-download'
     assert str(apps_menu.url) == '/apps/'
 
-    system_menu = main_menu.get('system')
+    system_menu = main_menu.active_item(rf.get('/sys/bar/'))
     assert system_menu.icon == 'fa-cog'
     assert str(system_menu.url) == '/sys/'
 
@@ -95,6 +95,7 @@ def test_menu_creation_with_arguments():
                 expected_icon, url_name, url_kwargs=url_kwargs,
                 parent_url_name='index', order=expected_order, advanced=True)
 
+    assert menu.parent_url_name == 'index'
     assert len(parent_menu.items) == 1
     assert parent_menu.items[0] == menu
     assert expected_name == menu.name
@@ -103,46 +104,12 @@ def test_menu_creation_with_arguments():
     assert expected_url == menu.url
     assert expected_order == menu.order
     assert menu.advanced
+    assert url_name == menu.url_name
+    assert menu.url_args is None
+    assert url_kwargs == menu.url_kwargs
     assert not menu.items
 
 
-def test_get():
-    """Verify that a menu item can be correctly retrieved."""
-    expected_name = 'Name2'
-    expected_short_description = 'ShortDescription2'
-    expected_icon = 'Icon2'
-    expected_url = 'index'
-    url_name = 'index'
-    reversed_url = reverse(url_name)
-    expected_order = 2
-    menu = Menu('menu-test', expected_name, expected_short_description,
-                expected_icon, url_name, order=expected_order, advanced=True)
-    actual_item = menu.get(expected_url)
-
-    assert actual_item is not None
-    assert expected_name == actual_item.name
-    assert expected_short_description == actual_item.short_description
-    assert expected_icon == actual_item.icon
-    assert reversed_url == actual_item.url
-    assert expected_order == actual_item.order
-    assert actual_item.advanced
-    assert not actual_item.items
-
-
-def test_get_with_item_not_found():
-    """Verify that a KeyError is raised if a menu item is not found."""
-    expected_name = 'Name3'
-    expected_short_description = 'ShortDescription3'
-    expected_icon = 'Icon3'
-    url_name = 'index'
-    expected_order = 3
-    menu = Menu('menu-test', expected_name, expected_short_description,
-                expected_icon, url_name, order=expected_order)
-
-    with pytest.raises(KeyError):
-        menu.get('apps')
-
-
 def test_sort_items():
     """Verify that menu items are sorted correctly."""
     size = 1000