[Django] #35716: #35189 brokes admin/[model]/add pages when application does not allow undefined variables on templates

6 views
Skip to first unread message

Django

unread,
Aug 28, 2024, 6:34:47 AM8/28/24
to django-...@googlegroups.com
#35716: #35189 brokes admin/[model]/add pages when application does not allow
undefined variables on templates
---------------------------------+-----------------------------------------
Reporter: Fábio Domingues | Type: Bug
Status: new | Component: contrib.admin
Version: 5.1 | Severity: Normal
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------
Applications that does not forgive undefined variables on templates, for
example using this:
{{{
# Raise an exception when accessing an undefined variable in templates
class InvalidString(str):
def __mod__(self, other):
from django.template.base import TemplateSyntaxError

raise TemplateSyntaxError(f"Undefined variable or unknown value
for: {other}")
}}}
raises an error "Undefined variable or unknown value for:
fieldset.formset.prefix" on line 1 of
django/contrib/admin/templates/admin/includes/fieldset.html

My sugestion is to use firstof tags instead of the with tag.
--
Ticket URL: <https://code.djangoproject.com/ticket/35716>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Aug 28, 2024, 9:02:25 AM8/28/24
to django-...@googlegroups.com
#35716: #35189 brokes admin/[model]/add pages when application does not allow
undefined variables on templates
--------------------------------------+------------------------------------
Reporter: Fábio Domingues | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Natalia Bidart):

* cc: Adam Johnson (added)
* stage: Unreviewed => Accepted
* type: Bug => Cleanup/optimization

Comment:

Hello Fábio Domingues, thank you for your ticket. There was a
[https://discord.com/channels/856567261900832808/859997770274045954/threads/1272493850086342738
previous conversation] about this in the (unofficial) Django Discord
server, I'll extract some fragments to provide context in this ticket for
future readers:

From Marijke:
> My changes to fieldsets.html have created an obscure issue. (PR:
https://github.com/django/django/pull/17910)
> I enabled `FAIL_INVALID_TEMPLATE_VARS` in `pytest-django`, which is
"''Fail tests that render templates which make use of invalid template
variables.''"
> I also ran tests on the availability of the admin model "changelist" and
"add" pages (just a simple `request.get()` to see if the pages return HTTP
200).
> These tests now fail on the following error: `Failed: Undefined template
variable 'fieldset.formset.prefix' in
'path/to/django/contrib/admin/templates/admin/includes/fieldset.html'`
> You don't notice this issue when using the admin, since Django doesn't
crash on missing references by default.
> Do I report this as a bug in Trac?

From David:
> While "Django doesn't crash on missing references by default" is 100% a
feature and some/many folk rely on it there are others who will want to
test for missing template variables. See blog post by Adam
https://adamj.eu/tech/2022/03/30/how-to-make-django-error-for-undefined-
template-variables/
> I'm not sure it's a bug if the Django admin is using Django's default
behaviour. Afterall 'the admin isn't your project' .
> Clearly some folk 'abuse' this though.
> I'm therefore a unsure if this is worth fixing (I'm slightly leaning
towards this tbh).

From me:
> while I agree that Django admin is using Django's default behaviour, do
we have a confirmation that formset is "correctly" undefined? Marijke do
you have more details?

We have some previous similar tickets being accepted such as #31865 and
#2688, but we also have one closed #28516 in favor of #28526.

Also, I have taken a quick look at your proposed fix
([https://github.com/django/django/pull/18521 PR]) and I'm not sure that
is the right fix. I would rather to design a way to be able to pass the
prefix from the call sites instead.

Because all of the above, I'm tentatively accepting this ticket to
evaluate how a solution would look like, but I also think that if we don't
find a clean and nice way to fix this issue in particular, we should close
as dupe of #28526.

(Adam, I'm adding you as cc since you added yourself to #28526.)
--
Ticket URL: <https://code.djangoproject.com/ticket/35716#comment:1>

Django

unread,
Aug 28, 2024, 9:04:06 AM8/28/24
to django-...@googlegroups.com
#35716: #35189 brokes admin/[model]/add pages when application does not allow
undefined variables on templates
--------------------------------------+------------------------------------
Reporter: Fábio Domingues | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 5.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Natalia Bidart):

* needs_better_patch: 0 => 1
* needs_tests: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35716#comment:2>
Reply all
Reply to author
Forward
0 new messages