fix active NavItem check for urls with query params (#114)

Author: joshuadavidthomas

CHANGELOG.md

@@ -18,6 +18,10 @@ and this project attempts to adhere to [Semantic Versioning](https://semver.org/
 
 ## [Unreleased]
 
+### Changed
+
+- Updated `NavItem.get_active` to allow for using URLs that contain query strings.
+
 ## [0.8.0]
 
 ### Changed

src/django_simple_nav/nav.py

@@ -5,6 +5,7 @@
 from dataclasses import field
 from typing import Callable
 from typing import cast
+from urllib.parse import parse_qs
 from urllib.parse import urlparse
 from urllib.parse import urlunparse
 
@@ -136,10 +137,20 @@ def get_active(self, request: HttpRequest) -> bool:
         if not url:
             return False
 
-        if settings.APPEND_SLASH and not request.path.endswith("/"):  # pyright: ignore[reportAny]
-            request.path += "/"
+        parsed_url = urlparse(url)
+        parsed_request = urlparse(request.build_absolute_uri())
 
-        return request.path == url
+        url_path = parsed_url.path
+        request_path = parsed_request.path
+
+        if settings.APPEND_SLASH:
+            url_path = url_path.rstrip("/") + "/"
+            request_path = request_path.rstrip("/") + "/"
+
+        url_query = parse_qs(parsed_url.query)
+        request_query = parse_qs(parsed_request.query)
+
+        return url_path == request_path and url_query == request_query
 
     def get_items(self, request: HttpRequest) -> list[NavGroup | NavItem] | None:
         # this needs to be set to shadow the built-in `items()` of the dict

tests/conftest.py

@@ -38,4 +38,9 @@ def pytest_configure(config):
 
 @pytest.fixture
 def req():
-    return HttpRequest()
+    # adding a HTTP_HOST header for now to fix tests, but
+    # we really should switch to using a RequestFactory
+    # instead of this fixture
+    request = HttpRequest()
+    request.META = {"HTTP_HOST": "test"}
+    return request

tests/test_navitem.py

@@ -120,20 +120,23 @@ def get_url(self):
 
 
 @pytest.mark.parametrize(
-    "url,req_path,expected",
+    "url,req_path,req_params,expected",
     [
-        ("/test/", "/test/", True),
-        ("/test/", "/other/", False),
-        ("fake-view", "/fake-view/", True),
-        ("/test", "/test/", True),
-        ("/test/", "/test", True),
-        ("/test/nested/", "/test/", False),
+        ("/test/", "/test/", None, True),
+        ("/test/", "/other/", None, False),
+        ("fake-view", "/fake-view/", None, True),
+        ("/test", "/test/", None, True),
+        ("/test/", "/test", None, True),
+        ("/test/nested/", "/test/", None, False),
+        ("/test/?query=param", "/test/", {"query": "param"}, True),
+        ("/test/?query=param", "/test/", None, False),
     ],
 )
-def test_active(url, req_path, expected, req):
+def test_active(url, req_path, req_params, expected, rf):
     item = NavItem(title=..., url=url)
 
-    req.path = req_path
+    req = rf.get(req_path, req_params)
+    print(f"{req.path=}")
 
     assert item.get_active(req) == expected