Use path converters for URL type checking and conversion

Why are we doing this?

We have a number of views that raise an unhandled ValueError and 500 if a misformatted URL is passed. This is because they perform operations that require values of certain types but neither they nor the URL dispatcher do anything to ensure passed values are of those types or handle what happens if the values they are using are not. Normally users follow valid URLS we construct, but sometimes they enter them manually incorrectly as in the example below. They should get a 404 or other error in such case, not a 500. The user should not be able to easily trigger unhandled exceptions. This also makes Sentry noise.

For example:

• /builder/60b18755/search/lives/delete/ fails because the view is expecting the penultimate group to be an integer to match pattern: <hash>/search/<search_id>/<search_slug>/. This is a real error that a user hit, see this Sentry report. It seems they were trying to manually enter the URL to delete a search.
• /builder/h/ fails as the view is expecting the draft ID to be hex in pattern <hash>/. Local development example:

  File "/opencodelists/builder/decorators.py", line 15, in wrapped_view
    id = unhash(hash, "CodelistVersion")
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opencodelists/opencodelists/hash_utils.py", line 55, in unhash
    return int(h, 16) * inv_c % N
           ^^^^^^^^^^
ValueError: invalid literal for int() with base 16: 'h'

How will we know when it's done?

There is defensive code in place to handle such bad URLs being entered, either in the URL dispatcher, view code, or actions.py code, or more than one of these.

What are we doing?

The problem is URL patterns are matching URLs views can't handle and passing bad parameters to those views. We should use Django's in-built path converters feature in the URL dispatchers to avoid this. Then, URLs with the wrong types won't match the relevant patterns and the user will get a 404, instead of unhandled exceptions being raised and leading to a 500, and a Sentry event. We can write custom converters for things like the draft version identifiers which are hex. E.g., builder/urls.py could look something like:

class CodelistVersionHashConverter:
    regex = '[0-9a-fA-F]+'

    def to_python(self, value):
        return value  

    def to_url(self, value):
        return value

from django.urls import path, register_converter
register_converter(CodelistVersionHashConverter, 'hash')

urlpatterns = [
    path("<hash:hash>/", views.draft, name="draft"),
    path("<hash:hash>/search/<int:search_id>/<slug:search_slug>/", views.search, name="search"),
    path(
        "<hash:hash>/search/<int:search_id>/<slug:search_slug>/delete/",
        views.delete_search,
        name="delete-search",
    ),
    path("<hash:hash>/no-search-term/", views.no_search_term, name="no-search-term"),
    path("<hash:hash>/update/", views.update, name="update"),
    path("<hash:hash>/search/", views.new_search, name="new-search"),
]

We could handle some of the conversion currently done in builder/decorators.py in the converter, as well as validating that the value is hex. We should apply this approach systematically across all the URL dispatcher urls.py modules, to avoid as many such issues as possible, and so we have a consistent approach to build on. We may be able to remove per-view code that handles, for example, str to int conversion as the converter can handle it.

For belt-and-braces we could also make each view perform type checking before calling unsafe functions on values passed from parameters, or handle the resulting ValueError (or other errors) sensibly. The cost of that may be slightly less readable view code. EDIT: I'm leaning towards treating the view-and-URL-dispatcher as one unit to test with a simulated client, and not having type-checking/error handling within the view code. Probably view code is only ever called after URL dispatch so it's reasonable to treat that as the entrypoint for tests and slim down the view code.

There are a number of views that have relevant parameters that might benefit from path converters, probably we should split this issue into smaller tasks/PRs:

• The numerical search_id in builder views in the first example under "Why are we doing this" -- https://github.com/opensafely-core/opencodelists/pull/2638
• Path parameters called slugs in their variable names -- https://github.com/opensafely-core/opencodelists/pull/2641
• orgs/usernames in opencodelists -- also slugs. Same PR -- https://github.com/opensafely-core/opencodelists/pull/2641
• Builder hex hashes (second example under "Why are we doing this" and similar, code example above).
• tag_or_hash in codelists/urls.py. https://github.com/opensafely-core/opencodelists/pull/2651 Corresponds to CodelistVersion property tag_or_hash. Presumably the hash is hex characters, what can tag be? codelist/models.py doesn't explain what tags are so a little poking around required to see how it's used... turns out free text that's currently mostly an historical thing, Slack thread thinking about how that could change. Upshot for this, probably just a str: converter...

Defining delivery tasks guidance