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?
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 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!
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 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, or can review your GitHub PRs:
Or try out Django refactor challenges.
Top comments (0)