View DiffRedo our URL scheme to use {% url %} tags
Submitted
Updated 7 months ago
| Christian Hammond | Reviewers | ||
| trunk | reviewboard | ||
| 458, 459 | |||
| None | Review Board SVN | ||
We've been using the old get_absolute_url() function in our templates, which works for the most part but has limitations. Using it on a User object doesn't work so well when installed in a subdirectory, requiring that we prepend SITE_ROOT and then slice the first "/". This gets ugly fast.
We've also been hard-coding other paths ("/account/login/", "/dashboard/", etc.), requiring prepending SITE_ROOT again.
Django's {% url %} tag provides a cleaner solution, allowing us to give certain pages names that we can refer to. Now something like {{SITE_ROOT}}{{submitter.get_absolute_url|slice:"1"}} turns into {% url user submitter %}.
This in theory could also give extensions in the future the ability to override certain pages if they wanted.
The result of this change is that we use get_absolute_url() a lot less for some things and never for users. We could move the rest of the get_absolute_url() calls in the future if we choose, though what we have works for now. We've also eliminated most references to SITE_ROOT in the templates.
There's some misc. fixes and cleanups in here too, such as using djblets's new user_displayname filter.
Made sure every page I tested worked properly in subdirectory installs. All the user links I could find now point to the correct location.
Can you run the unit tests?
-
/trunk/reviewboard/templates/iphone/review_request_detail.html (Diff revision 1) 23 <a href="{{group.get_absolute_url}}">{{group}}</a>{%if not forloop.last %}, {%endif %}
23 <a href="{% url iphone-group group %}">{{group}}</a>{%if not forloop.last %}, {%endif %}
24 {% endfor %}24 {% endfor %}25 {% if review_request_details.target_groups.count and review_request_details.target_people.count %}<br />{% endif %}25 {% if review_request_details.target_groups.count and review_request_details.target_people.count %}<br />{% endif %}26 {% for person in review_request_details.target_people.all %}26 {% for person in review_request_details.target_people.all %}27 <a href="{{person.get_absolute_url}}">{{person}}</a>{%if not forloop.last %}, {%endif %}
27 <a href="{% url iphone-user person %}">{{person}}</a>{%if not forloop.last %}, {%endif %}
-
Not your fault (this time), but {%endif %} is weird.
Also, it'd be nice if you could bump the diffviewer pagination up on this install. I did it in settings.py right now, so an update should be sufficient.