prevent duplicate items in search registry

froide/document/apps.py

@@ -25,7 +25,7 @@ def ready(self):
             PageViewSet,
         )
 
-        search_registry.register(add_search)
+        search_registry.register("document", add_search)
 
         api_router.register(r"document", DocumentViewSet, basename="document")
         api_router.register(
@@ -42,6 +42,5 @@ def ready(self):
 def add_search(request):
     return {
         "title": _("Documents"),
-        "name": "document",
         "url": reverse("document-search"),
     }

froide/foirequest/apps.py

@@ -50,7 +50,7 @@ def ready(self):
         account_merged.connect(merge_user)
         account_made_private.connect(make_account_private)
         registry.register(export_user_data)
-        search_registry.register(add_search)
+        search_registry.register("foirequest", add_search)
         comment_will_be_posted.connect(signals.pre_comment_foimessage)
         team_changed.connect(keep_foiproject_teams_synced_with_requests)
         account_confirmed.connect(send_request_when_account_confirmed)
@@ -63,7 +63,6 @@ def ready(self):
 def add_search(request):
     return {
         "title": _("Requests"),
-        "name": "foirequest",
         "url": reverse("foirequest-list"),
     }
 

froide/helper/search/registry.py

@@ -1,19 +1,41 @@
+from typing import Callable, NotRequired, TypedDict
+
+from django.http import HttpRequest
+
+from django_stubs_ext import StrOrPromise
+
+
+class SearchItem(TypedDict):
+    name: NotRequired[str]
+    title: StrOrPromise
+    url: StrOrPromise
+    menu_title: NotRequired[StrOrPromise]
+    order: NotRequired[int]
+
+
+type SearchResponse = SearchItem | None
+type SearchItemCallback = Callable[[HttpRequest], SearchResponse]
+
+
 class SearchRegistry(object):
     def __init__(self):
-        self.searches = []
+        self.searches: dict[str, SearchItemCallback] = {}
+
+    def register(self, name: str, func: SearchItemCallback):
+        if name in self.searches:
+            return
 
-    def register(self, func):
-        self.searches.append(func)
+        self.searches[name] = func
 
-    def get_searches(self, request):
-        sections = []
-        for callback in self.searches:
+    def get_searches(self, request: HttpRequest) -> list[SearchItem]:
+        sections: list[SearchItem] = []
+        for name, callback in self.searches.items():
             menu_item = callback(request)
             if menu_item is None:
                 continue
+            menu_item["name"] = name
             sections.append(menu_item)
-        sections = sorted(sections, key=lambda x: (x.get("order", 5), x["title"]))
-        return sections
+        return sorted(sections, key=lambda x: (x.get("order", 5), x["title"]))
 
 
 search_registry = SearchRegistry()

froide/helper/search/tests/test_search_registry.py

@@ -0,0 +1,40 @@
+from django.http import HttpRequest
+from django.urls import reverse
+from django.utils.translation import gettext_lazy as _
+
+import pytest
+
+from froide.helper.search.registry import SearchItem, SearchRegistry
+
+
+@pytest.mark.django_db
+def test_no_duplicates():
+    registry = SearchRegistry()
+
+    def add_search(request) -> SearchItem:
+        return {
+            "title": _("Requests"),
+            "url": reverse("foirequest-list"),
+        }
+
+    def add_empty_search(request):
+        return None
+
+    registry.register("foirequest", add_search)
+
+    r = HttpRequest()
+
+    assert len(registry.searches) == 1
+    assert len(registry.get_searches(r)) == 1
+
+    registry.register("foirequest", add_search)
+    assert len(registry.searches) == 1
+    assert len(registry.get_searches(r)) == 1
+
+    registry.register("document", add_search)
+    assert len(registry.searches) == 2
+    assert len(registry.get_searches(r)) == 2
+
+    registry.register("empty", add_empty_search)
+    assert len(registry.searches) == 3
+    assert len(registry.get_searches(r)) == 2

froide/publicbody/apps.py

@@ -22,7 +22,7 @@ def ready(self):
 
         registry.register(export_user_data)
         account_merged.connect(merge_user)
-        search_registry.register(add_search)
+        search_registry.register("publicbody", add_search)
 
         from froide.api import api_router
 
@@ -47,7 +47,6 @@ def ready(self):
 
 def add_search(request):
     return {
-        "name": "publicbody",
         "title": _("Public Bodies"),
         "url": reverse("publicbody-list"),
     }

pyproject.toml

@@ -69,7 +69,7 @@ dev = [
   "beautifulsoup4",
   "django-coverage-plugin",
   "django-extended-makemessages>=1.7.1",
-  "django-stubs",
+  "django-stubs<5.3",
   "factory-boy",
   "faker",
   "monkeytype",
@@ -93,6 +93,7 @@ dev = [
   "prek>=0.3.5",
   "pytest-xdist>=3.8.0",
   "pytest-cov>=7.1.0",
+  "django-stubs-ext<5.3",
 ]
 
 [build-system]