[Django] #18558: Supply `url` property to `HttpResponseRedirect` and `HttpResponsePermanentRedirect`
Django
`HttpResponsePermanentRedirect`
-------------------------------+--------------------
Reporter: coolRR | Owner: nobody
Type: Uncategorized | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
Currently the way to get the URL that a `HttpResponseRedirect` is
redirecting to requires doing `response['Location']`. This is not so
intuitive. There's no reason to remember the HTML header name when dealing
with a redirect response.
Instead I propose this property:
url = property(lambda self: self['Location'])
--
Ticket URL: <https://code.djangoproject.com/ticket/18558>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
`HttpResponsePermanentRedirect`
Type: Uncategorized | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by coolRR):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Sorry for the butchered Python, I meant:
{{{#!python
url = property(lambda self: self['Location'])
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18558#comment:1>
Django
`HttpResponsePermanentRedirect`
Reporter: coolRR | Owner: nobody
Type: New feature | Status: new
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Changes (by jezdez):
* type: Uncategorized => New feature
* stage: Unreviewed => Accepted
Old description:
> Currently the way to get the URL that a `HttpResponseRedirect` is
> redirecting to requires doing `response['Location']`. This is not so
> intuitive. There's no reason to remember the HTML header name when
> dealing with a redirect response.
>
> Instead I propose this property:
>
> url = property(lambda self: self['Location'])
New description:
Currently the way to get the URL that a `HttpResponseRedirect` is
redirecting to requires doing `response['Location']`. This is not so
intuitive. There's no reason to remember the HTML header name when dealing
with a redirect response.
Instead I propose this property:
{{{
url = property(lambda self: self['Location'])
}}}
--
Comment:
Formatting updates.
--
Ticket URL: <https://code.djangoproject.com/ticket/18558#comment:2>
Django
`HttpResponsePermanentRedirect`
Reporter: coolRR | Owner: nobody
Type: New feature | Status: new
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by coolRR):
What is needed now to add this to Django?
--
Ticket URL: <https://code.djangoproject.com/ticket/18558#comment:3>
Django
`HttpResponsePermanentRedirect`
Reporter: coolRR | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by hirokiky):
* cc: hirokiky@… (added)
Comment:
https://github.com/django/django/pull/657
I opened a pull-request. Fixed the implementation and tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/18558#comment:4>
Django
`HttpResponsePermanentRedirect`
Reporter: coolRR | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: 0 => 1
* needs_docs: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/18558#comment:5>
Django
`HttpResponsePermanentRedirect`
Reporter: coolRR | Owner: hirokiky
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Changes (by hirokiky):
* status: new => assigned
* owner: nobody => hirokiky
--
Ticket URL: <https://code.djangoproject.com/ticket/18558#comment:6>
Django
`HttpResponsePermanentRedirect`
Reporter: coolRR | Owner: hirokiky
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by anonymous):
* needs_better_patch: 1 => 0
* needs_docs: 1 => 0
Comment:
I added the documentaiton and tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/18558#comment:7>
Django
`HttpResponsePermanentRedirect`
Reporter: coolRR | Owner: claudep
Type: New feature | Status: assigned
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by claudep):
* owner: hirokiky => claudep
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/18558#comment:8>
Django
`HttpResponsePermanentRedirect`
Reporter: coolRR | Owner: claudep
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e94f405d9499d310ef58b7409a98759a5f5512b0"]:
{{{
#!CommitTicketReference repository=""
revision="e94f405d9499d310ef58b7409a98759a5f5512b0"
Fixed #18558 -- Added url property to HttpResponseRedirect*
Thanks coolRR for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18558#comment:9>
Django
`HttpResponsePermanentRedirect`
Reporter: coolRR | Owner: claudep
Type: New feature | Status: closed
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by hirokiky):
Thanks coolRR and claudep.
I am glad to contribute to Django at first time.
--
Ticket URL: <https://code.djangoproject.com/ticket/18558#comment:10>
Django
`HttpResponsePermanentRedirect`
Reporter: coolRR | Owner: claudep
Type: New feature | Status: closed
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by coolRR):
Thank you hirokiky and claudep!
--
Ticket URL: <https://code.djangoproject.com/ticket/18558#comment:11>
Django
`HttpResponsePermanentRedirect`
Reporter: coolRR | Owner: claudep
Type: New feature | Status: closed
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by tevans):
Hi, I think this change is incorrect.
It makes HttpResponse(status=302, content='Foo') behave differently to
HttpResponseRedirect(content='Foo').
This can be seen by using the test client, and asking it to follow
redirects. It will attempt to access the url property on the response, but
because the response is a redirect that is not derived from
HttpResponseRedirectBase, this will fail.
--
Ticket URL: <https://code.djangoproject.com/ticket/18558#comment:12>
Django
`HttpResponsePermanentRedirect`
Reporter: coolRR | Owner: claudep
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: fixed =>
--
Ticket URL: <https://code.djangoproject.com/ticket/18558#comment:13>
Django
`HttpResponsePermanentRedirect`
Reporter: coolRR | Owner: claudep
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
`HttpResponseRedirect(content='Foo')` is not valid, the `redirect_to`
parameter being mandatory. I think that the proper answer would be not to
use a plain `HttpResponse` for an HTTP redirection. If you have a valid
use case for using an `HttpResponse(status=302)` instead of an
`HttpResponseRedirect`, please open a new ticket (where you can reference
this one).
--
Ticket URL: <https://code.djangoproject.com/ticket/18558#comment:14>
Django
`HttpResponsePermanentRedirect`
Reporter: coolRR | Owner: claudep
Type: New feature | Status: closed
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by tevans):
If creating a redirect manually from a HttpResponse is incorrect, why is
it possible to create a HttpResponse with status 301, 302, 303 or 307?
The problem is that the test client uses both the API of HttpResponse to
check whether the response is a redirect or not, and then it uses the API
of HttpResponseRedirectBase to determine the redirection location. You
think that is not a bug?
It only comes around because someone has decided that knowing the URL for
a redirect is in the Location header is needless knowledge, it is better
to instead learn a distinct API just for this framework. Also, lets make
it so that that API prevents previously working and still currently
documented as working code from being treated correctly, and ignore bug
reports on it because pseudo code didn't have all the arguments specified.
Yay progress!
--
Ticket URL: <https://code.djangoproject.com/ticket/18558#comment:15>