Opened 15 years ago

Closed 14 years ago

Last modified 14 years ago

#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


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:

Attachments (3)

redirect_to_with_query_string.diff (6.5 KB ) - added by steingrd 15 years ago.
redirect_to_with_query_string2.diff (5.8 KB ) - added by steingrd 15 years ago.
adds a query_string optional keyword arg to redirect_to
redirect_to_with_query_string_r11688.diff (6.0 KB ) - added by steingrd 15 years ago.
Patch that applies cleanly to r11688

Download all attachments as: .zip

Change History (13)

comment:1 by anonymous, 15 years ago

Cc: fcorreia@… added

comment:2 by Jacob, 15 years ago

milestone: 1.1
Triage Stage: UnreviewedAccepted

I'd expect the query string to *always* be passed through, actually; no need for an option.

by steingrd, 15 years ago

comment:3 by steingrd, 15 years ago

Cc: steingrd@… added
Has patch: set
Owner: changed from nobody to steingrd
Status: newassigned

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/ The feature broke a few tests in regressiontests/test_client_regress/, these have been updated as well.

I haven't updated the documentation as English is not my first language.

comment:4 by Malcolm Tredinnick, 15 years ago

Triage Stage: AcceptedDesign 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.

by steingrd, 15 years ago

adds a query_string optional keyword arg to redirect_to

comment:5 by steingrd, 15 years ago

Owner: changed from steingrd to nobody
Status: assignednew

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 by Jacob, 15 years ago

milestone: 1.11.2
Triage Stage: Design decision neededAccepted

by steingrd, 15 years ago

Patch that applies cleanly to r11688

comment:7 by Eric Holscher, 14 years ago

Triage Stage: AcceptedReady for checkin

Patch looks good to me.

comment:8 by James Bennett, 14 years ago

milestone: 1.2

1.2 is feature-frozen, moving this feature request off the milestone.

comment:9 by Malcolm Tredinnick, 14 years ago

Resolution: fixed
Status: newclosed

(In [13746]) Add option to redirect_to view to allow passing along the query string
from the original request. Default is current behaviour, which is not to
pass the query string (it often won't be appropriate to do so).

Thanks to steingrd@… for the patch and tests. Fixed #9966.

comment:10 by Claude Paroz, 14 years ago

Shouldn't the doc mention that this was added in dev version?

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