#9966 closed (fixed)
Generic view "redirect_to" to carry existing query string
Reported by: | FilipeCorreia | Owned by: | nobody |
---|---|---|---|
Component: | Generic views | Version: | 1.0 |
Severity: | Keywords: | ||
Cc: | fcorreia@…, steingrd@… | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The "redirect_to" generic view currently does not pass along any query string that may exist in the original request.
An extra parameter could be added to "redirect_to", specifying if the original query string should be appended or not. See the following thread:
http://groups.google.com/group/django-users/browse_frm/thread/4566f4040cef518a/8bfcd6ddd4ebb9f5
Attachments (3)
Change History (13)
comment:1 Changed 14 years ago by
Cc: | fcorreia@… added |
---|
comment:2 Changed 14 years ago by
milestone: | → 1.1 |
---|---|
Triage Stage: | Unreviewed → Accepted |
Changed 14 years ago by
Attachment: | redirect_to_with_query_string.diff added |
---|
comment:3 Changed 14 years ago by
Cc: | steingrd@… added |
---|---|
Has patch: | set |
Owner: | changed from nobody to steingrd |
Status: | new → assigned |
I added a patch that passes through query strings on redirect in django.views.generic.simple.redirect_to.
I've also written a test case for the redirect_to view, unfortunately it shows up as a file named /dev/null in my patch, the filename is tests/regressiontests/views/tests/generic/simple.py. The feature broke a few tests in regressiontests/test_client_regress/models.py, these have been updated as well.
I haven't updated the documentation as English is not my first language.
comment:4 Changed 14 years ago by
Triage Stage: | Accepted → Design decision needed |
---|
Have to say, I disagree with Jacob's comment:2. Inthe general case, it's not going to be correct to pass through the query string. That string is specific to the particular resource requested, not all resources that the site serves and redirect is redirecting to a different resource. It's appropriate to pass on the query string when, say, redirecting to a canonical URL form (e.g. appending a slash to the URL), but, in general, no, it's not.
I'd be -1 on making this behave this way always. As an option, though, it's absolutely a good idea.
Changed 14 years ago by
Attachment: | redirect_to_with_query_string2.diff added |
---|
adds a query_string optional keyword arg to redirect_to
comment:5 Changed 14 years ago by
Owner: | changed from steingrd to nobody |
---|---|
Status: | assigned → new |
I actually agree with Malcolm, the query string should only follow when configured to so. I have created a new patch that adds a query_string keyword argument to the redirect_to function.
I've added the option to the generic views reference documentation and updated all tests.
comment:6 Changed 14 years ago by
milestone: | 1.1 → 1.2 |
---|---|
Triage Stage: | Design decision needed → Accepted |
Changed 14 years ago by
Attachment: | redirect_to_with_query_string_r11688.diff added |
---|
Patch that applies cleanly to r11688
comment:7 Changed 13 years ago by
Triage Stage: | Accepted → Ready for checkin |
---|
Patch looks good to me.
comment:8 Changed 13 years ago by
milestone: | 1.2 |
---|
1.2 is feature-frozen, moving this feature request off the milestone.
comment:9 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
I'd expect the query string to *always* be passed through, actually; no need for an option.