[Django] #20500: Flatpages catchall URLconf example that works with APPEND_SLASH
Django
--------------------------------------+-----------------------
Reporter: josh.23.french@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.4
Severity: Normal | Keywords: flatpages
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+-----------------------
In https://docs.djangoproject.com/en/dev/ref/contrib/flatpages/#using-the-
urlconf the catchall example doesn't include a slash anywhere, so it
catches all pages without a slash.
So we just need to add a slash. (inside the match parenthesis)
{{{
# Your other patterns here
urlpatterns += patterns('django.contrib.flatpages.views',
(r'^(?P<url>.*/)$', 'flatpage'),
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20500>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: josh.23.french@… | Owner: ByteMonk
Type: | Status: assigned
Cleanup/optimization | Version: 1.4
Component: Documentation | Resolution:
Severity: Normal | Triage Stage:
Keywords: flatpages | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by ByteMonk):
* owner: nobody => ByteMonk
* needs_better_patch: => 0
* status: new => assigned
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/20500#comment:1>
Django
Reporter: josh.23.french@… | Owner: ByteMonk
Type: | Status: assigned
Cleanup/optimization | Version: 1.4
Component: Documentation | Resolution:
Severity: Normal | Triage Stage:
Keywords: flatpages | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by claudep):
I'm not sure I follow your argumentation. Could you please add some
concrete example about the problem here?
--
Ticket URL: <https://code.djangoproject.com/ticket/20500#comment:2>
Django
Reporter: josh.23.french@… | Owner: ByteMonk
Type: | Status: assigned
Cleanup/optimization | Version: 1.4
Component: Documentation | Resolution:
Severity: Normal | Triage Stage:
Keywords: flatpages | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by josh.23.french@…):
Here's an example urlconf without the slash:
{{{
#!python
urlpatterns = patterns('',
url(r'^articles/$', 'news.views.archive'),
url(r'^articles/2003/$', 'news.views.special_case_2003'),
url(r'^articles/(\d{4})/$', 'news.views.year_archive'),
url(r'^articles/(\d{4})/(\d{2})/$', 'news.views.month_archive'),
url(r'^articles/(\d{4})/(\d{2})/(\d+)/$',
'news.views.article_detail'),
)
# Directly from the docs
urlpatterns += patterns('django.contrib.flatpages.views',
(r'^(?P<url>.*)$', 'flatpage'),
)
}}}
When `/articles` is asked for, the flatpages pattern matches it, and if
there is no `/articles/` flatpage, you get a 404.
What should happen is that `/articles` gets redirected to `/articles/` and
the correct view gets processed.
The APPEND_SLASH logic only works if the sans-slash url isn't valid and a
slashed one is valid. (django.middleware.common, line 71)
{{{
/articles <-- valid, no redirect
}}}
The current pattern will match either, so no redirect to a slashed url is
returned when flatpages sends a 404. (flatpages.views line 40)
By adding the slash in the urlpattern, we require that all flatpages have
slashes... those that don't will be redirected by the APPEND_SLASHES
logic. So `/articles` will redirect to `/articles/`, and `/flatpage` to
`/flatpage/`. (All flatpages are required by admin to have slashes, so
there is no problem of flatpages existing without slashes, unless
somebody's doing something weird.)
Does that make more sense? Or am I missing something here?
--
Ticket URL: <https://code.djangoproject.com/ticket/20500#comment:3>
Django
Reporter: josh.23.french@… | Owner: ByteMonk
Type: | Status: assigned
Cleanup/optimization | Version: 1.4
Component: Documentation | Resolution:
Severity: Normal | Triage Stage:
Keywords: flatpages | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by claudep):
Thanks for the useful explanation. I would consider this as a bug in the
flatpage view. In my opinion, the view should simply redirect after
testing `if not url.endswith('/') and settings.APPEND_SLASH:`. Thoughts?
--
Ticket URL: <https://code.djangoproject.com/ticket/20500#comment:4>
Django
Reporter: josh.23.french@… | Owner: ByteMonk
Type: | Status: assigned
Cleanup/optimization | Version: 1.4
Component: Documentation | Resolution:
Severity: Normal | Triage Stage:
Keywords: flatpages | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by josh.23.french@…):
If we skip right to the redirect, we'd end up redirecting pages to 404s,
which seems like something we want to keep.
([https://github.com/django/django/commit/196ac8f8b31302d026af6d79f764c8f146a8c78e
196ac8f8b3])
(If it's not, ignore everything that follows.)
[[BR]]
[[BR]]
''Keep in mind I'm completely new to Django's codebase(4 months), so the
following code might be inappropriate.''
This logic might be too heavy, but if we want to keep the no-redirect in
cases where it'll end up a 404, it's something to think about:
{{{
#!python
from django.core import urlresolvers
if not url.endswith('/') and settings.APPEND_SLASH:
url += '/'
# Check if new url will resolve, and get the view info if it does.
try:
found_url = urlresolvers.resolve(url)
except urlresolvers.Resolver404:
raise
# Check if the new url is a flatpage. If it is, skip the second half
of the OR and redirect.
if (FlatPage.objects.filter(url__exact=url,
sites__id__exact=site_id).exists()
# Check if the new url_name would bring us back here. (If it won't,
redirect.
# If it will, both sides of the OR are False and we raise the 404.)
or (found_url.url_name !=
"django.contrib.flatpages.views.flatpage")):
# Stole this from CommonMiddleware. We want the same warning...
and CommonMiddleware's warning will never get raise'd.
if settings.DEBUG and request.method == 'POST':
raise RuntimeError((""
"You called this URL via POST, but the URL doesn't end "
"in a slash and you have APPEND_SLASH set. Django can't "
"redirect to the slash URL while maintaining POST data. "
"Change your form to point to %s (note the trailing "
"slash), or set APPEND_SLASH=False in your Django "
"settings.") % url)
return HttpResponsePermanentRedirect(url)
else:
# No flatpage exists
raise
else:
raise
}}}
This is basically the APPEND_SLASH logic with a check for FlatPages
specifically.
--
Ticket URL: <https://code.djangoproject.com/ticket/20500#comment:5>
Django
Reporter: josh.23.french@… | Owner: ByteMonk
Type: | Status: assigned
Cleanup/optimization | Version: 1.4
Component: Documentation | Resolution:
Severity: Normal | Triage Stage:
Keywords: flatpages | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by josh.23.french):
The whole try/except around `found_url = urlresolvers.resolve(url)` is
excessive. Apparently, `Resolver404` is a subclass of `Http404`. (I
couldn't edit it-I assume because I didn't have an account, so I got one.)
--
Ticket URL: <https://code.djangoproject.com/ticket/20500#comment:6>
Django
Reporter: josh.23.french@… | Owner: ByteMonk
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: flatpages | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by claudep):
* version: 1.4 => master
* stage: Unreviewed => Accepted
Comment:
Replying to [comment:5 josh.23.french@…]:
> If we skip right to the redirect, we'd end up redirecting pages to 404s,
which seems like something we want to keep.
([https://github.com/django/django/commit/196ac8f8b31302d026af6d79f764c8f146a8c78e
196ac8f8b3])
> (If it's not, ignore everything that follows.)
Good point, you are right.
Maybe you were also right from the start, telling people to add the final
slash in the catch-all pattern when APPEND_SLASH is True might be the
better solution.
--
Ticket URL: <https://code.djangoproject.com/ticket/20500#comment:7>
Django
Reporter: josh.23.french@… | Owner: EvilDMP
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: flatpages | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by EvilDMP):
* owner: ByteMonk => EvilDMP
Comment:
I have marked this ticket as suitable for a first-time committer attending
a [https://dont-be-afraid-to-commit.readthedocs.org/ Don't be afraid to
commit workshop].
The next planned session will be hosted by
[http://www.cardiffdevworkshop.com Cardiff Dev Workshop] on Saturday 8th
June.
If you want to tackle this ticket before then, or at any time in fact,
please '''don't''' let the fact that it's assigned to me stop you. Feel
free to re-assign it to yourself and do whatever you like to it.
--
Ticket URL: <https://code.djangoproject.com/ticket/20500#comment:8>
Django
Reporter: josh.23.french@… | Owner: EvilDMP
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: flatpages | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timo):
* has_patch: 0 => 1
Comment:
Came across a pull request. Haven't followed the discussion to determine
if it's how we want to solve this.
https://github.com/django/django/pull/1221
--
Ticket URL: <https://code.djangoproject.com/ticket/20500#comment:9>
Django
Reporter: josh.23.french@… | Owner: EvilDMP
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: flatpages | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by josh.23.french):
If we do want to fix it this way, that patch also needs a warning (or
note) that the slash must be removed when `APPEND_SLASH = False`.
Something like:
{{{
You can also set it up as a "catchall" pattern. In this case, it is
important
to place the pattern at the end of the other urlpatterns::
# Your other patterns here
urlpatterns += patterns('django.contrib.flatpages.views',
(r'^(?P<url>.*/)$', 'flatpage'),
)
.. warning::
If you set APPEND_SLASH to False, you must remove the slash in the
catchall
pattern, or flatpages without a trailing slash will not be matched.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20500#comment:10>
Django
Reporter: josh.23.french@… | Owner: EvilDMP
Component: Documentation | Version: master
Severity: Normal | Resolution: fixed
Keywords: flatpages | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"536703abf029ac1b9c2ebcf6f82fb16da524bebe"]:
{{{
#!CommitTicketReference repository=""
revision="536703abf029ac1b9c2ebcf6f82fb16da524bebe"
Fixed #20500 - Updated flatpages URLconf example to work with
APPEND_SLASH.
Thanks josh.23.french@.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20500#comment:11>
Django
Reporter: josh.23.french@… | Owner: EvilDMP
Type: Cleanup/optimization | Status: closed
Component: Documentation | Version: master
Severity: Normal | Resolution: fixed
Keywords: flatpages | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"b75f1f3d27d06e5fe4fdcc8e74ec3e7caeff7704"]:
{{{
#!CommitTicketReference repository=""
revision="b75f1f3d27d06e5fe4fdcc8e74ec3e7caeff7704"
[1.5.x] Fixed #20500 - Updated flatpages URLconf example to work with
APPEND_SLASH.
Thanks josh.23.french@.
Backport of 536703abf0 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20500#comment:12>