Suppressed template errors in admin
Ole Laursen
I have a project where setting TEMPLATE_STRING_IF_INVALID has been
invaluable in finding problems (for various reasons). The caveat
mentioned in the docs don't kick in here because no templates are
inherited from elsewhere. Except the admin which unfortunately breaks
down in some places.
I realize there's been a big discussion before about the merits of
error suppression. No need to repeat that.
But is it really not possible to fix admin in the few places it's a
problem? Those I've seen so far have one-line fixes where you add an
empty variable to the context. Seems harmless to me. I realize
cluttering the template itself with a {% if var %} is ugly. No need to
go there.
I don't mind writing a patch for the cases that bother me, but won't
do it unless somebody is willing to apply it?
Ole
Jonas H.
+1
This little ugly hack is incredibly useful during development:
# Hack
class RaiseMissingVariable(object):
def __mod__(self, val):
raise AttributeError("Missing %r" % val)
def __contains__(self, what):
return True
TEMPLATE_STRING_IF_INVALID = RaiseMissingVariable()
Russell Keith-Magee
On principle, I have no objection to the idea of making the admin
templates more robust in the presence of TEMPLATE_STRING_IF_INVALID;
adding dummy values in the context sounds like a reasonable approach
-- *if* doing this doesn't undermine broader error handling in the
templates.
Also -- dummy values may not be the only option. There's also |default
and |default_if_none filters.
However, in order to evaluate the details, I'd need to see them --
which unfortunately means a patch (or, at least, an indicative start
of a patch).
I fully appreciate your desire not to waste your time, though --
there's no point making a patch if it will be ignored. I would suggest
the best approach here is to attack this in stages. Provide a patch
that fixes a small number of uses in a limited number of key
templates, and poke around on django-dev or on IRC to ask for a
review. Once that initial patch is approved and/or applied, expand the
patch until it covers all problematic uses.
Russ %-)
h3
great idea.
I work on two projects[1] where I would lose a useful debugging
information and
it would be the same lost to anyone working on the admin or extending
the admin.
Of course we could use a settings to toggle on/off the admin error
suppression,
but that would mean another setting.. which is to be avoided.
Maybe a better approach would be to simply mark the fallback string as
safe.
Then it would be possible to suppress them like so:
TEMPLATE_STRING_IF_INVALID = "<!-- Invalid var -->"
This would have the benefit to be more flexible:
TEMPLATE_STRING_IF_INVALID = "!! Invalid var !!" # settings_dev.py
TEMPLATE_STRING_IF_INVALID = "<!-- Invalid var -->" # settings_prod.py
But this would have side effects on the non-admin templates too.. so
it's not ideal.
Maybe something like this:
TEMPLATE_STRING_IF_INVALID = {'default: '!! Invalid var !!',
'django.contrib.admin': ''}
I think it would pretty much solve the problem, preserve backward
compatibility/behavior and
even allow to suppress errors for any other apps.
[1] https://github.com/sehmaschine/django-grappelli
http://jqmobile-sandbox.motion-m.ca/
--
Maxime Haineault
On Aug 24, 7:39 pm, Russell Keith-Magee <russ...@keith-magee.com>
wrote:
Ole Laursen
wrote:
> templates more robust in the presence of TEMPLATE_STRING_IF_INVALID;
> adding dummy values in the context sounds like a reasonable approach
> -- *if* doing this doesn't undermine broader error handling in the
> templates.
tops. Thanks for considering this! I'm checking out the SVN release
right now, but actually the one that I've been bitten by so far has a
patch here (patch against current trunk below):
https://code.djangoproject.com/ticket/12136
Note that this was merged into a monster bug that had worse patches
(as far as I can tell), adding if-tags in the templates, and
depressingly never went anywhere.
Ole
Index: django/contrib/admin/templatetags/admin_list.py
===================================================================
--- django/contrib/admin/templatetags/admin_list.py (revision 16696)
+++ django/contrib/admin/templatetags/admin_list.py (working copy)
@@ -102,7 +102,10 @@
admin_order_field = getattr(attr, "admin_order_field",
None)
if not admin_order_field:
# Not sortable
- yield {"text": text}
+ yield {
+ "text": text,
+ "class_attrib": ""
+ }
continue
# OK, it is sortable if we got this far
Ole Laursen
> I'm not sure suppressing templates errors for the admin is such a
> great idea.
is sloppy and doesn't include all the variables it uses in the
context. Normally you don't see this because this kind of error is
suppressed by Django. However, if you turn on
TEMPLATE_STRING_IF_INVALID, they show up and break the markup, e.g.
the column headers in the overview table.
Ole
Luke Plant
> On principle, I have no objection to the idea of making the admin
> templates more robust in the presence of TEMPLATE_STRING_IF_INVALID;
> adding dummy values in the context sounds like a reasonable approach
> -- *if* doing this doesn't undermine broader error handling in the
> templates.
If we call this a bug, and agree to fix it, does it mean that from now
on for any changes to the admin template we have to be careful to test
with TEMPLATE_STRING_IF_INVALID != '' ?
That sounds kind of tedious, and I would be against making a promise
never to break this again in the future. If we can fix it now with
relatively little incovenience, fine, but I don't want that to turn into
a promise.
Regards,
Luke
--
"Doubt: In the battle between you and the world, bet on the world."
(despair.com)
Luke Plant || http://lukeplant.me.uk/
Yuri Baburov
On Sat, Sep 3, 2011 at 9:13 PM, Luke Plant <L.Pla...@cantab.net> wrote:
> On 25/08/11 00:39, Russell Keith-Magee wrote:
>
>> On principle, I have no objection to the idea of making the admin
>> templates more robust in the presence of TEMPLATE_STRING_IF_INVALID;
>> adding dummy values in the context sounds like a reasonable approach
>> -- *if* doing this doesn't undermine broader error handling in the
>> templates.
>
> If we call this a bug, and agree to fix it, does it mean that from now
> on for any changes to the admin template we have to be careful to test
> with TEMPLATE_STRING_IF_INVALID != '' ?
We could switch admin test suite to always run with proposed
TEMPLATE_STRING_IF_INVALID = RaiseMissingVariable() and check if there
are more mistakes, fix mistakes (if any), and make final patch!
Any missing {{ variable }} could be easily replaced with {% firstof
variable "" %} -- it works, and I remember |default and
|default_if_none didn't few years ago. Not sure if it's still the
same.
At least there are
https://code.djangoproject.com/browser/django/trunk/tests/regressiontests/admin_views/tests.py
and
https://code.djangoproject.com/browser/django/trunk/tests/regressiontests/admin_inlines/tests.py
which render admin templates.
> That sounds kind of tedious, and I would be against making a promise
> never to break this again in the future. If we can fix it now with
> relatively little incovenience, fine, but I don't want that to turn into
> a promise.
Then we could generously ask to show us a test which breaks admin
output when TEMPLATE_STRING_IF_INVALID = RaiseMissingVariable()
I don't believe there will be much work, because current patch
proposes to fix exactly same wart that were there 4 years ago, just
with different, more elegant, piece of code -- check my
https://code.djangoproject.com/ticket/3579 ( some part has been fixed
later at https://code.djangoproject.com/ticket/4497 ).
--
Best regards, Yuri V. Baburov, Skype: yuri.baburov, MSN: bu...@live.com
Thomas Guettler
Am 03.09.2011 16:13, schrieb Luke Plant:
> On 25/08/11 00:39, Russell Keith-Magee wrote:
>
>> On principle, I have no objection to the idea of making the admin
>> templates more robust in the presence of TEMPLATE_STRING_IF_INVALID;
>> adding dummy values in the context sounds like a reasonable approach
>> -- *if* doing this doesn't undermine broader error handling in the
>> templates.
>
> If we call this a bug, and agree to fix it, does it mean that from now
> on for any changes to the admin template we have to be careful to test
> with TEMPLATE_STRING_IF_INVALID != '' ?
>
> That sounds kind of tedious, and I would be against making a promise
> never to break this again in the future. If we can fix it now with
> relatively little incovenience, fine, but I don't want that to turn into
> a promise.
If the django core makes a promise that TEMPLATE_STRING_IF_INVALID=RaiseMissingVariable() is
supported in admin (and other contrib apps), I will help to get it done.
Up to now, that's why I don't use the template language in my apps.
Zen of Python: Errors should never pass silently.
Ole, can you please set up a a branch at github or bitbucket?
Thomas
--
Thomas Guettler, http://www.thomas-guettler.de/
E-Mail: guettli (*) thomas-guettler + de
Russell Keith-Magee
>
>
> Am 03.09.2011 16:13, schrieb Luke Plant:
>> On 25/08/11 00:39, Russell Keith-Magee wrote:
>>
>>> On principle, I have no objection to the idea of making the admin
>>> templates more robust in the presence of TEMPLATE_STRING_IF_INVALID;
>>> adding dummy values in the context sounds like a reasonable approach
>>> -- *if* doing this doesn't undermine broader error handling in the
>>> templates.
>>
>> If we call this a bug, and agree to fix it, does it mean that from now
>> on for any changes to the admin template we have to be careful to test
>> with TEMPLATE_STRING_IF_INVALID != '' ?
>>
>> That sounds kind of tedious, and I would be against making a promise
>> never to break this again in the future. If we can fix it now with
>> relatively little incovenience, fine, but I don't want that to turn into
>> a promise.
>
> If the django core makes a promise that TEMPLATE_STRING_IF_INVALID=RaiseMissingVariable() is
> supported in admin (and other contrib apps), I will help to get it done.
>
> Up to now, that's why I don't use the template language in my apps.
>
> Zen of Python: Errors should never pass silently.
This particular argument fails because we're not talking about Python,
here. We're talking about a template language that renders to a public
facing output. Errors on user-facing code isn't cool.
Yours,
Russ Magee %-)
Ole Laursen
> Ole, can you please set up a a branch at github or bitbucket?
I don't think discussing this is worth anyone's time. No need to
promise anything or elevate this to a Big Decision as long as it's
about trivial non-intrusive fixes.
I'm still hoping someone (Russ?) will just take the patch and apply
it, and I can go back to being the happy camper. :)
Ole
Jens Diemer
> But this would have side effects on the non-admin templates too.. so
> it's not ideal.
>
> Maybe something like this:
>
> TEMPLATE_STRING_IF_INVALID = {'default: '!! Invalid var !!',
> 'django.contrib.admin': ''}
I do a hack in my admin.py:
-----------------------------------------------------------------------------
# some django admin stuff is broken if TEMPLATE_STRING_IF_INVALID != ""
# http://code.djangoproject.com/ticket/3579
if settings.TEMPLATE_STRING_IF_INVALID != "":
# Patch the render_to_response function ;)
from django.contrib.auth import admin as auth_admin
from django.contrib.admin import options
def patched_render_to_response(*args, **kwargs):
old = settings.TEMPLATE_STRING_IF_INVALID
settings.TEMPLATE_STRING_IF_INVALID = ""
result = render_to_response(*args, **kwargs)
settings.TEMPLATE_STRING_IF_INVALID = old
return result
options.render_to_response = patched_render_to_response
auth_admin.render_to_response = patched_render_to_response
-----------------------------------------------------------------------------
So you can use TEMPLATE_STRING_IF_INVALID, but it's excluded from django admin ;)
--
Mfg.
Jens Diemer