Decorators make invalid assumptions about the target URL pattern when redirecting

[hheimbuerger]

Abstract

I think the redirect_to parameter of the decorators (e.g. @waffle_flag) make invalid assumptions about the destination view, namely that they have the same set of arguments as the decorated view.

Background

The @waffle_flag decorator supports supplying a redirect_to argument, which can contain a URL pattern to redirect to if the flag isn't set. To determine the URL to redirect to, waffle uses the following function:

def get_response_to_redirect(view, *args, **kwargs):
    try:
        return redirect(reverse(view, args=args, kwargs=kwargs)) if view else None
    except NoReverseMatch:
        return None

This basically makes the assumption that the URL structure of the decorated view closely matches the URL structure of the redirected to view. It does that by trying to reverse the redirection URL pattern using the args and kwargs of the decorated view.

Where does the assumption that these two match come from? Why should or must these two match?

Example case

Here's a simple counter example.

urls.py:

url(r'^show/(?P<entity>.+)/$', views.show, name='show'),
url(r'^failure/', views.failure, name='failure'),

The show view is our main view. When the feature flag isn't enabled, we want to redirect to failure.

And here's the corresponding views.py:

@waffle_flag('feature1', redirect_to='failure')
def show(request, entity):
    return HttpResponse('success: ' + str(entity))

def failure(request):
    return HttpResponse('failure')

I think this is really straightforward and should work. I would expect that If the flag is set, and /show/foo/ is opened, it displays success: foo. If the flag is unset, it should redirect to /failure/.

However, because show expects a parameter entity, waffle assumes that failure will take the same parameter, which it doesn't. So the @waffle_flag fails to find a view with name failure and a keyword argument entity (there's only one view with the name failure, which doesn't take any keyword arguments, and therefore it isn't a match!) and instead generates a 404.

Do you agree that this behavior is unexpected?

[clintonb]

I agree that is confusing. I would expect the passing of args and kwargs to be optional, controlled via boolean toggle. I'm happy to review a PR fixing this.

[hheimbuerger]

What is the use case for the submitting the arguments from one view to another? I don't quite see a reason to ever want this, and therefore why there should be a boolean toggle.

[clintonb]

Backward-compatibility. We don't know who is relying on this functionality. What may be "wrong" to us may be a feature to someone else.

[hheimbuerger]

@clintonb Ah, so you're saying you would also let this toggle default to the old behavior then, I assume?

[clintonb]

That’s correct.