Opened 7 years ago

Closed 3 years ago

#15180 closed New feature (duplicate)

reverse doesn't check default_args

Reported by: olaf Owned by: nobody
Component: Core (URLs) Version: 1.3-beta
Severity: Normal Keywords: resolve default_args
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no


Lets say we have the following urls:

    (r'^photos/?$', 'views.photo_index', {'lang':'en'}, "photo_index"),
    (r'^fotos/?$', 'views.photo_index', {'lang':'de'}, "photo_index"),
    (r'^photo/(?P<photo_id>[\d]+)$', '', {"lang":"en"} ,"photo"),
    (r'^foto/(?P<photo_id>[\d]+)$', '', {"lang":"de"} ,"photo"),

When I want to get the specific URL for a German visitor I wan't to just to get the reverse for photo an add a keyword argument {'lang':'de'}. It is very much writing to write for each language a different wrapper function. You also would need to write a bunch of if-else statements to select the correct one.

I saw that also some else expected this and asked a question at stackoverflow about this, but he got only workaround as answers. I added a few lines to correct behavior, and I hope this is useful for someone. I am not sure, if I broke something so it would be nice someone else could test this code also.

Attachments (3)

urlresolvers.diff (1.9 KB) - added by olaf 7 years ago.
urlresolvers-v2.diff (3.4 KB) - added by olaf 7 years ago.
urlresolvers-v2.2.diff (3.4 KB) - added by olaf 7 years ago.

Download all attachments as: .zip

Change History (12)

Changed 7 years ago by olaf

Attachment: urlresolvers.diff added

comment:1 Changed 7 years ago by Russell Keith-Magee

Needs tests: set
Triage Stage: UnreviewedAccepted

comment:2 Changed 7 years ago by Russell Keith-Magee

I have a background suspicion that this will cause all sorts of havoc with existing URL lookups -- for example, existing views with default arguments that *aren't* using the default arguments to differentiate between views.

However, since the patch doesn't have any tests, I can't verify this quickly.

Marking this as a qualified accepted, the qualification being that the implementation *must* be backwards compatible with existing uses, and be tested as such. If that can't be achieved, then we'll need to close wontfix.

comment:3 Changed 7 years ago by olaf

Ok, I changed my patch a little bit. I added a new optional parameter to the reverse function. If check_default_args is not set(=None), reverse tries to find a match the standard way, if it will not succeed it tries one time again including the default_args. If it is set to False, it behaves just like the current reverse function. If it is set to true, it will check them from the beginning.

I would like to write those test cases, but I don't know how to use different url configs. Could you point me to an Howto or give me a small example, and I will write them.


Changed 7 years ago by olaf

Attachment: urlresolvers-v2.diff added

Changed 7 years ago by olaf

Attachment: urlresolvers-v2.2.diff added

comment:4 Changed 7 years ago by Julien Phalip

milestone: 1.3

This can't make it into 1.3 unless comprehensive regression tests are provided. Olaf, if you're interested in writing some tests, take a look at the guide available at
Don't hesitate to ask questions on the django-users list too if need be.

comment:5 Changed 7 years ago by Łukasz Rekucki

Severity: Normal
Type: New feature

comment:6 Changed 6 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:7 Changed 6 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:8 Changed 5 years ago by Aymeric Augustin

Component: Core (Other)Core (URLs)

comment:9 Changed 3 years ago by Bas Peschier

Resolution: duplicate
Status: newclosed

This was fixed in #13154

Note: See TracTickets for help on using tickets.
Back to Top