Bugs lurking in your urls.py
Tags: django, webdev, codequality, python
Published: 2020-11-16 19:51:49

`urls.py` can get unwieldy as the codebase grows. There are strategies to improve the maintainability, but sometimes that's a moot point like when joining a new team or inheriting a brownfield codebase, so it's important to know how cure as well as prevent. Can you see the problem with this `urls.py` from a mature codebase?

```py
from django.urls import path, include

import views


router = DefaultRouter()
router.register("application", views.AppView, basename="app")
router.register("licence", views.LicenceView, basename="licence")
router.register("good", views.GoodView, basename="good")
router.register("file-version", views.FileView, basename="file")


urlpatterns = router.urls

urlpatterns += [
    path("healthcheck/", include("health_check.urls")),
    path("callback/", views.CallbackView.as_view(), name="login"),
    path("applications/", include("api.applications.urls")),
    path("audit-trail/", include("api.audit_trail.urls")),
    path("cases/", include("api.cases.urls")),
    path("compliance/", include("api.compliance.urls")),
    path("goods/", include("api.goods.urls")),
    path("goods-types/", include("api.goodstype.urls")),
    path("picklist/", include("api.picklists.urls")),
    path("documents/", include("api.documents.urls")),
    path("queries/", include("api.queries.urls")),
    path("routing-rules/", include("api.workflow.urls")),
    path("licences/", include("api.licences.urls")),
    path("signup/", views.Signup.as_view(), name="login")),
    path("licences/", include("api.licences.urls")),
    path("data-workspace/", include("api.data_workspace.urls")),
] + static(settings.STATIC_URL, document_root=settings.STATIC_ROOT)

if settings.FEATURE_API_ENABLED:
    urlpatterns.append(path("search/", include("api.search.urls")))

if settings.FEATURE_ADMIN_ENABLED:
    urlpatterns.append(path("admin/", admin.site.urls))

    if settings.FEATURE_STAFF_SSO_ENABLED:
        urlpatterns = [
            path("admin/login/", api.core.views.LoginView.as_view()),
            path("auth/", include("authbroker_client.urls")),
        ] + urlpatterns
```

You saw it right? I did because [I'm a code review bot](https://django.doctor/) and am good at these kind of things. Here's a clue: what url will `reverse("login")` or `{% url "login" %}` return? Check again. That's right - `/callback/` and `/login/` have the same name!

```py
path('callback/', views.CallbackView.as_view(), name='login'),
...
path('signup/', views.Signup.as_view(), name='login'),
```

How did this happen? Well it's easily missed when reading the code. Certainly possible to miss during code review as humans are fallible. [W Edwards Deming](https://en.wikipedia.org/wiki/W._Edwards_Deming) pointed out that _do more manual QA_ will not necessarily lead to improved quality: when there are multiple people involved in QA, each can rely on the other to spot a defect, so as a factory manager adds more humans manually looking for defects, the more defects would be missed because Frank assumed Tim would catch it, and Tim was relying on Bill, and Bill knows Francis was good and Francis knew Kim never drops the ball etc. Is it their fault for missing the bugs? No, management should have provided then the right tools and processes to account for this phenomenon: _do better QA_ rather than _do more manual QA_.

What's the impact? This code getting into the `master` branch will mean half of the time templates will be linking to the wrong place: if a template tries to link to the login view via `{% url "login" %}` can you be sure it's going to the right place? No.

### Is your codebase hiding non-unique URLS?

Over time it's easy for tech debt to slip into your codebase. I can check that for you at [django.doctor](https://django.doctor), or can [review your GitHub PRs](https://github.com/marketplace/django-doctor/):

![Alt Text](https://dev-to-uploads.s3.amazonaws.com/i/d10epulva9d373jgldhu.png)

Or try out [Django refactor challenges](https://django.doctor/challenge).