Repository: vitalik/django-ninja
Pull request #638 (2022-12-22 12:50:20)

Allow reversing URLs with multiple operations.
A pretty glaring shortcoming is that it is impossible to reverse URLs that have been registered with same path, but different operations.

Using the same path for different operations is a pretty common pattern. It is even used throughout Ninja's documentation.

    @router.get("/{identifier}")
    def detail(request, identifier: str):
        return "ok"

    @router.put("/{identifier}")
    def update(request, identifier: str, data: Policy):
        return "ok"

The above is perfectly valid, but reversing `update` using `django.urls.reverse()` will result in a `NoReverseMatch` error.

This commit is an attempt at solving the issue. N.B. I am not familiar with the internals of Ninja. This is just an experiment.


Patch:
diff --git a/ninja/operation.py b/ninja/operation.py
index 4737843..74590d2 100644
--- a/ninja/operation.py
+++ b/ninja/operation.py
@@ -50,12 +50,15 @@ class Operation:
         exclude_defaults: bool = False,
         exclude_none: bool = False,
         include_in_schema: bool = True,
+        url_name: str = None,
     ) -> None:
         self.is_async = False
         self.path: str = path
         self.methods: List[str] = methods
         self.view_func: Callable = view_func
         self.api: "NinjaAPI" = cast("NinjaAPI", None)
+        if url_name is not None:
+            self.url_name = url_name
 
         self.auth_param: Optional[Union[Sequence[Callable], Callable, object]] = auth
         self.auth_callbacks: Sequence[Callable] = []
@@ -177,7 +180,8 @@ class Operation:
             response_model = self.response_models[Ellipsis]
         else:
             raise ConfigError(
-                f"Schema for status {status} is not set in response {self.response_models.keys()}"
+                f"Schema for status {status} is not set in response"
+                f" {self.response_models.keys()}"
             )
 
         temporal_response.status_code = status
@@ -312,6 +316,7 @@ class PathView:
             exclude_defaults=exclude_defaults,
             exclude_none=exclude_none,
             include_in_schema=include_in_schema,
+            url_name=url_name,
         )
 
         self.operations.append(operation)
diff --git a/ninja/router.py b/ninja/router.py
index 2bfaa11..a933a72 100644
--- a/ninja/router.py
+++ b/ninja/router.py
@@ -308,19 +308,18 @@ class Router:
     def urls_paths(self, prefix: str) -> Iterator[URLPattern]:
         prefix = replace_path_param_notation(prefix)
         for path, path_view in self.path_operations.items():
-            path = replace_path_param_notation(path)
-            route = "/".join([i for i in (prefix, path) if i])
-            # to skip lot of checks we simply treat double slash as a mistake:
-            route = normalize_path(route)
-            route = route.lstrip("/")
+            for operation in path_view.operations:
+                path = replace_path_param_notation(path)
+                route = "/".join([i for i in (prefix, path) if i])
+                # to skip lot of checks we simply treat double slash as a mistake:
+                route = normalize_path(route)
+                route = route.lstrip("/")
 
-            url_name = path_view.url_name or ""
-            if not url_name and self.api:
-                url_name = self.api.get_operation_url_name(
-                    path_view.operations[0], router=self
-                )
+                url_name = getattr(operation, "url_name", "")
+                if not url_name and self.api:
+                    url_name = self.api.get_operation_url_name(operation, router=self)
 
-            yield django_path(route, path_view.get_view(), name=url_name)
+                yield django_path(route, path_view.get_view(), name=url_name)
 
     def add_router(
         self,
@@ -349,7 +348,8 @@ class Router:
 
             if not debug_server_url_reimport():
                 raise ConfigError(
-                    f"Router@'{prefix}' has already been attached to API {self.api.title}:{self.api.version} "
+                    f"Router@'{prefix}' has already been attached to API"
+                    f" {self.api.title}:{self.api.version} "
                 )
         internal_routes = []
         for inter_prefix, inter_router in self._routers:


Test patch:
diff --git a/tests/demo_project/demo/urls.py b/tests/demo_project/demo/urls.py
index 7de2d73..0e4b249 100644
--- a/tests/demo_project/demo/urls.py
+++ b/tests/demo_project/demo/urls.py
@@ -31,6 +31,16 @@ def foobar(request):
     return "foobar"
 
 
+@api_v3.post("foobar")
+def post_foobar(request):
+    return "foobar"
+
+
+@api_v3.put("foobar", url_name="foobar_put")
+def put_foobar(request):
+    return "foobar"
+
+
 api_multi_param = NinjaAPI(version="1.0.1")
 api_multi_param.add_router("", multi_param)
 
diff --git a/tests/test_reverse.py b/tests/test_reverse.py
new file mode 100644
index 0000000..1821779
--- /dev/null
+++ b/tests/test_reverse.py
@@ -0,0 +1,14 @@
+import pytest
+from django.urls import reverse
+
+
+@pytest.mark.parametrize(
+    "view_name, path",
+    [
+        ("foobar", "/api/v3/foobar"),
+        ("post_foobar", "/api/v3/foobar"),
+        ("foobar_put", "/api/v3/foobar"),
+    ],
+)
+def test_reverse(view_name, path):
+    assert reverse(f"api-3.0.0:{view_name}") == path