Reverse and resolve issues using includes with namespaces in flagged_path()

[mkeener]

Django version: 2.2
Django-flags version: 5.0.9

We have a mutli-module django application with several of the modules gated behind feature flags. In our deployment urls.py we use a combination of standard path() definitions with separate flagged_paths. Our flagged paths look like:

flagged_path(
    'FLAG_NAME',
    'module-path/',
    include(('path.to.module.urls',
                  'path.to.module.app_config'),
                  namespace='app_namespace'),
    state=True,
    fallback=FLAG_DISABLED_VIEW
)

The contents of the module urls file looks something like:

urlpatterns = [
    path('', NewIndexView.as_view(), name='index'),
    path('legacy/', IndexView.as_view(), name='index-legacy'),
    path('legacy/create/', CreateView.as_view(), name='index-legacy-create'),
    path('legacy/<uuid:pk>/', DetailView.as_view(), name='index-legacy-detail'),
]

When verifying the view name with reverse() we get an error that there is no reverse match like this:

django.urls.exceptions.NoReverseMatch: Reverse for 'index-legacy' with no arguments not found. 1 pattern(s) tried: ['module-path/$legacy/$']

I'm trying to figure out why there is an additional $ in the pattern, and if that has anything to do with why the paths fail to match. We're not using any regex path matching via re_path or old url(r'^module-name/$') style path matching.

If I switch from using flagged_path() to flagged_re_path it appears to resolve correctly. Is there something I'm misunderstanding about the usage of flagged_path()? All the instances I've used flagged_path() have been drop-in replacements for path() usage. Any direction would be appreciated!

[willbarton]

@mkeener Hmm, your assumptions about flagged_path should be correct. This sounds like a bug, and is not intentional. I'll look into it!

[willbarton]

@mkeener I believe I've found the problem — and it's a pretty simple one:

We have a _flagged_path function that mirrors Django's _path function. The intention is to decorate the views attached to URL patterns, so that the flag check will happen when the view is run — that's how the URL flagging works, it's just another way to add the flag_check() decorator around a view, except that it can work for include()s as well.

The problem is when creating the new pattern for an include, we set is_endpoint to True. This should be False, because it's not an endpoint.

Django's RegexPattern doesn't only rely on that is_endpoint variable, it also checks for trailing $ in the pattern. So, it works. RoutePattern (which path() creates) does no such checking, it just converts to a regex (which adds a termination match if is_endpoint is True).

So... the fix here is for us to set is_endpoint to False when creating the flagged patterns for includes().

This is a long way to saying... it's a simple, one-line change, and I'll have a PR to fix it momentarily.

[mkeener]

This is great, thanks for the explanation! I was really wondering where the extra $ was coming from and knew it was likely the culprit, but couldn't tell you why. Thanks again for your help and quick response!