Fix linking for localsite URLs and make the usage more seamless.
Review Request #2007 — Created Dec. 16, 2010 and submitted
Fix linking for localsite URLs and make the usage more seamless.
This implements some tricks to get seamless, easy linking of localsite URLs
when inside a localsite.
First, the URLs have been registered by creating a new urlpatterns entry
that we both add to the main URLs list and add with a s/(name)/ prefix (using
an include). Django, it turns out, allows multiple URLs with the same name,
and it will try each URL with the regular expression and parameters given.
Going this route, we don't trip up the URL parsers by encasing the "s/" in the
optional regex.
That's the first half of the equation. The second is URL resolving.
URL resolving for templates is now very easy and transparent to work with.
We replace/wrap Django's own {% url %} tag. We do this by adding the
localsite app's templatetags as a built-in set of tags, which will actually
override any of Django's own built-in tags.
The new tag wraps Django's tag and attempts to resolve two versions of the
URL: one with the local site name and one without. We get the local_site_name
from the context, which in most cases (when using RequestContext) is provided
by the new localsite context processor, which gets it from the new
LocalSiteMiddleware. LocalSiteMiddleware processes every view and stores
the local_site_name in the request for the context processor to access,
if it's provided in the URL.
This is still missing a couple components. First, reverse() still hasn't
been replaced. The plan there is to provide a new wrapper version that works
like the new {% url %} tag, and then use that everywhere else. Second,
the datagrid code doesn't provide context when linking to an object from a
cell, so that needs to be added to Djblets. These will be done in a
subsequent change.
Added new unit tests that check the new template tag. Unit tests all pass. Did some tests by hand. Things seem to work, except for when reverse() or datagrid object linking comes into play.
- Change Summary:
-
Completely reworked this change. See the description for updates.
- Summary:
-
Add proper linking to items within local sites (in most places).Fix linking for localsite URLs and make the usage more seamless.
- Description:
-
~ Add proper linking to items within local sites (in most places).
~ Fix linking for localsite URLs and make the usage more seamless.
~ This adds the ability to properly link to localsite-specific URLs when
~ inside of a localsite. It introduces middleware and a context processor that ~ This implements some tricks to get seamless, easy linking of localsite URLs
~ when inside a localsite. - figures out the localsite's name and makes it available to templates. The - templates can then use a new {% local_site_url %} template tag, which works - exactly like {% url %} but can prefix the proper localsite name (if the - middleware determined we're in one). ~ There isn't yet a replacement for reverse() that can handle localsites.
~ That should be added next. Until that's done, many URLs will still be ~ invalid, such as those inside entries in the datagrid. ~ First, the URLs have been registered by creating a new urlpatterns entry
~ that we both add to the main URLs list and add with a s/(name)/ prefix (using ~ an include). Django, it turns out, allows multiple URLs with the same name, + and it will try each URL with the regular expression and parameters given. + Going this route, we don't trip up the URL parsers by encasing the "s/" in the + optional regex. + + That's the first half of the equation. The second is URL resolving.
+ + URL resolving for templates is now very easy and transparent to work with.
+ We replace/wrap Django's own {% url %} tag. We do this by adding the + localsite app's templatetags as a built-in set of tags, which will actually + override any of Django's own built-in tags. + + The new tag wraps Django's tag and attempts to resolve two versions of the
+ URL: one with the local site name and one without. We get the local_site_name + from the context, which in most cases (when using RequestContext) is provided + by the new localsite context processor, which gets it from the new + LocalSiteMiddleware. LocalSiteMiddleware processes every view and stores + the local_site_name in the request for the context processor to access, + if it's provided in the URL. + + This is still missing a couple components. First, reverse() still hasn't
+ been replaced. The plan there is to provide a new wrapper version that works + like the new {% url %} tag, and then use that everywhere else. Second, + the datagrid code doesn't provide context when linking to an object from a + cell, so that needs to be added to Djblets. These will be done in a + subsequent change. - Testing Done:
-
Added new unit tests that check the new template tag.
~ Went through the dashboard and checked that various links worked when inside a localsite.
~ Also saw that several don't, due to the lack of a working reverse(). ~ Unit tests all pass.
~ + Did some tests by hand. Things seem to work, except for when reverse() or datagrid object linking comes into play.
- Diff:
-
Revision 2 (+139 -31)