Add nodes validation to the admin site (#1818)

misago/admin/site.py

@@ -92,12 +92,18 @@ def is_root(self):
         return False
 
 
+class AdminSiteInvalidNodeError(Exception):
+    pass
+
+
 class AdminSite:
     def __init__(self):
         self.nodes_record = []
         self.nodes_dict = {}
 
     def build_nodes_dict(self):
+        self.validate_nodes()
+
         nodes_dict = {"misago:admin": Node(link="misago:admin:index")}
 
         iterations = 0
@@ -108,7 +114,7 @@ def build_nodes_dict(self):
                     "Misago Admin hierarchy is invalid or too complex to resolve. "
                     "Nodes left: %s"
                 )
-                raise ValueError(message % self.nodes_record)
+                raise AdminSiteInvalidNodeError(message % self.nodes_record)
 
             for index, node in enumerate(self.nodes_record):
                 if node["parent"] in nodes_dict:
@@ -208,6 +214,53 @@ def visible_branches(self, request):
 
         return branches
 
+    def validate_nodes(self):
+        self.validate_nodes_parents()
+        self.validate_nodes_after()
+        self.validate_nodes_before()
+
+    def validate_nodes_parents(self):
+        parents: set[str] = {"misago:admin"}
+
+        for node in self.nodes_record:
+            parents.add(node["namespace"])
+
+        for node in self.nodes_record:
+            if node["parent"] not in parents:
+                raise AdminSiteInvalidNodeError(
+                    f"Misago Admin node '{node['link']}' has an invalid parent "
+                    f"'{node["parent"]}'."
+                )
+
+    def validate_nodes_after(self):
+        self.validate_nodes_ordering("after")
+
+    def validate_nodes_before(self):
+        self.validate_nodes_ordering("before")
+
+    def validate_nodes_ordering(self, node_attr: str):
+        for node in self.nodes_record:
+            self.validate_node_ordering(node, node_attr)
+
+    def validate_node_ordering(self, node: dict, node_attr: str):
+        if node[node_attr] is None:
+            return
+
+        node_siblings: set[str] = set()
+
+        for other_node in self.nodes_record:
+            if other_node == node:
+                continue
+
+            if other_node["parent"] == node["parent"]:
+                node_siblings.add(other_node["link"])
+
+        if node[node_attr] not in node_siblings:
+            raise AdminSiteInvalidNodeError(
+                f"Misago Admin node '{node['link']}' has an invalid {node_attr} "
+                f"node '{node[node_attr]}'."
+            )
+
 
 def join_namespace(*args):
     parts = list(filter(None, args))

misago/admin/tests/test_admin_site.py

@@ -1,41 +1,124 @@
-from ..site import Node
+import pytest
+
+from ..site import AdminSite, AdminSiteInvalidNodeError, Node
+
+
+def test_admin_site_validate_nodes_raises_error_if_node_has_invalid_parent():
+    site = AdminSite()
+
+    site.add_node(
+        name="Test",
+        icon="test",
+        namespace="test",
+        parent="invalid",
+    )
+
+    with pytest.raises(AdminSiteInvalidNodeError) as exc_info:
+        site.validate_nodes()
+
+    assert str(exc_info.value) == (
+        "Misago Admin node 'misago:admin:invalid:test:index' has "
+        "an invalid parent 'misago:admin:invalid'."
+    )
+
+
+def test_admin_site_validate_nodes_raises_error_if_node_has_invalid_after():
+    site = AdminSite()
+
+    site.add_node(
+        name="Test",
+        icon="test",
+        namespace="parent",
+    )
+    site.add_node(
+        name="Test",
+        icon="test",
+        parent="parent",
+        namespace="second",
+        after="invalid",
+    )
+    site.add_node(
+        name="Test",
+        icon="test",
+        parent="parent",
+        namespace="first",
+    )
+
+    with pytest.raises(AdminSiteInvalidNodeError) as exc_info:
+        site.validate_nodes()
+
+    assert str(exc_info.value) == (
+        "Misago Admin node 'misago:admin:parent:second:index' has "
+        "an invalid after node 'misago:admin:parent:invalid'."
+    )
+
+
+def test_admin_site_validate_nodes_raises_error_if_node_has_invalid_before():
+    site = AdminSite()
+
+    site.add_node(
+        name="Test",
+        icon="test",
+        namespace="parent",
+    )
+    site.add_node(
+        name="Test",
+        icon="test",
+        parent="parent",
+        namespace="second",
+        before="invalid",
+    )
+    site.add_node(
+        name="Test",
+        icon="test",
+        parent="parent",
+        namespace="first",
+    )
+
+    with pytest.raises(AdminSiteInvalidNodeError) as exc_info:
+        site.validate_nodes()
+
+    assert str(exc_info.value) == (
+        "Misago Admin node 'misago:admin:parent:second:index' has "
+        "an invalid before node 'misago:admin:parent:invalid'."
+    )
 
 
 def test_node_is_added_at_end_of_parent_children():
-    master = Node(name="Apples", link="misago:index")
+    parent = Node(name="Apples", link="misago:index")
     child = Node(name="Oranges", link="misago:index")
-    master.add_node(child)
+    parent.add_node(child)
 
-    assert master.children()[-1].name == child.name
+    assert parent.children()[-1].name == child.name
 
 
 def test_add_node_after():
     """add_node added node after specific node"""
-    master = Node(name="Apples", link="misago:index")
+    parent = Node(name="Apples", link="misago:index")
 
     child = Node(name="Oranges", link="misago:index")
-    master.add_node(child)
+    parent.add_node(child)
 
     test = Node(name="Potatoes", link="misago:index")
-    master.add_node(test, after="misago:index")
+    parent.add_node(test, after="misago:index")
 
-    all_nodes = master.children()
+    all_nodes = parent.children()
     for i, node in enumerate(all_nodes):
         if node.name == test.name:
             assert all_nodes[i - 1].name == child.name
 
 
 def test_add_node_before():
     """add_node added node  before specific node"""
-    master = Node(name="Apples", link="misago:index")
+    parent = Node(name="Apples", link="misago:index")
 
     child = Node(name="Oranges", link="misago:index")
-    master.add_node(child)
+    parent.add_node(child)
 
     test = Node(name="Potatoes", link="misago:index")
-    master.add_node(test, before="misago:index")
+    parent.add_node(test, before="misago:index")
 
-    all_nodes = master.children()
+    all_nodes = parent.children()
     for i, node in enumerate(all_nodes):
         if node.name == test.name:
             assert all_nodes[i + 1].name == child.name

misago/categories/management/commands/healcategorytrees.py

@@ -9,7 +9,7 @@
 class Command(BaseCommand):
     """
     This command rebuilds the category trees in the database.
-    
+
     It's useful when the MPTT data of one or more categories becomes invalid,
     either due to a bug or manual database manipulation.
     """