[Django] #35542: BoundField's label and help_text and renderer should be properties not class members

5 views
Skip to first unread message

Django

unread,
Jun 20, 2024, 8:39:16 AMJun 20
to django-...@googlegroups.com
#35542: BoundField's label and help_text and renderer should be properties not
class members
-------------------------------------+-------------------------------------
Reporter: | Owner: Christophe Henry
Christophe Henry |
Type: | Status: assigned
Uncategorized |
Component: Forms | Version: 5.0
Severity: Normal | Keywords:
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
`BoundField` exposes
[https://github.com/django/django/blob/main/django/forms/boundfield.py#L23
label]
[https://github.com/django/django/blob/main/django/forms/boundfield.py#L27
help_text] and
[https://github.com/django/django/blob/main/django/forms/boundfield.py#L28
renderer], all of them are copies of underlying members of `Field` and
`Form`.

This is a bit misleanding since you can modify `Field`s member in `Form`'s
`__init__` [https://docs.djangoproject.com/en/5.0/ref/forms/fields
/#fields-which-handle-relationships like documented here] but any
modification of `help_text`, `label` or `renderer` after
`super().__init__` won't be reflected on the bound which can leand to
incomprehensible behavior in templates
--
Ticket URL: <https://code.djangoproject.com/ticket/35542>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jun 21, 2024, 3:47:36 PMJun 21
to django-...@googlegroups.com
#35542: BoundField's label and help_text and renderer should be properties not
class members
-------------------------------------+-------------------------------------
Reporter: Christophe Henry | Owner:
Type: | Christophe Henry
Cleanup/optimization | Status: closed
Component: Forms | Version: dev
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Natalia Bidart):

* resolution: => needsinfo
* status: assigned => closed
* type: Uncategorized => Cleanup/optimization
* version: 5.0 => dev

Comment:

Hello again Christophe Henry! Thank you for your report.

We would need more information to understand your use case. The
documentation you linked about "Fields which handle relationships" refers
to redefining a field's queryset, not the label/help_text/renderer/etc.

With the information provided so far, I cannot determine whether this is a
very specific issue arising from a niche use case or something that
applies to the broader ecosystem. Django is a framework designed to
provide robust and reliable solutions for common scenarios. Therefore,
before accepting any changes to well-established and mature code, it is
crucial to demonstrate clear benefits from the proposed modification and
ensure there are no risks of breaking existing functionality.

Specifically, we would need a small Django test project or a failing test
case that shows the code path or use case that needs fixing. I'll close as
`needsinfo` in the meantime.
--
Ticket URL: <https://code.djangoproject.com/ticket/35542#comment:1>

Django

unread,
Jun 27, 2024, 7:04:12 AM (8 days ago) Jun 27
to django-...@googlegroups.com
#35542: BoundField's label and help_text and renderer should be properties not
class members
-------------------------------------+-------------------------------------
Reporter: Christophe Henry | Owner:
Type: | Christophe Henry
Cleanup/optimization | Status: closed
Component: Forms | Version: dev
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Christophe Henry):

Replying to [comment:1 Natalia Bidart]:

Hello Natalia!

Well the documentation I linked features this as the first example:

{{{#!python
class FooMultipleChoiceForm(forms.Form):
foo_select = forms.ModelMultipleChoiceField(queryset=None)

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.fields["foo_select"].queryset = ...
}}}

The same way, you can do this, for instance:

{{{#!python
class FooMultipleChoiceForm(forms.Form):
foo_select = forms.CharField()

def __init__(self, user: User, *args, **kwargs):
super().__init__(*args, **kwargs)
self.fields["foo_select"].help_text = f"Indicate
{user.first_name}'s pet name"
}}}

But if you do this, you won't get the result you expect when rendering
form with `{{ form }}` since what will be rendered is
`self["foo_select"].help_text.` (the associated `BoundField`) and not
`self.fields["foo_select"].help_text`. In order for the help text to
correctly be rendered, you need to write:

{{{#!python
class FooMultipleChoiceForm(forms.Form):
foo_select = forms.CharField()

def __init__(self, user: User, *args, **kwargs):
super().__init__(*args, **kwargs)
# Not self.field
self["foo_select"].help_text = f"Indicate {user.first_name}'s pet
name"
}}}

This is confusing and is inconsistant with the `queryset` example of the
documentation for anyone who's not very familiar with Django's `Form` API
and looks very much like a bug.

My proposed solution does not introduce any breaking changes and can even
be easily reverted to its original behavior with the patch I also proposed
for #35542.
--
Ticket URL: <https://code.djangoproject.com/ticket/35542#comment:2>

Django

unread,
Jul 3, 2024, 6:00:32 AM (3 days ago) Jul 3
to django-...@googlegroups.com
#35542: BoundField's label and help_text and renderer should be properties not
class members
-------------------------------------+-------------------------------------
Reporter: Christophe Henry | Owner:
Type: | Christophe Henry
Cleanup/optimization | Status: new
Component: Forms | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Christophe Henry):

* resolution: needsinfo =>
* status: closed => new

--
Ticket URL: <https://code.djangoproject.com/ticket/35542#comment:3>

Django

unread,
1:13 PM (5 hours ago) 1:13 PM
to django-...@googlegroups.com
#35542: BoundField's label and help_text and renderer should be properties not
class members
-------------------------------------+-------------------------------------
Reporter: Christophe Henry | Owner:
Type: | Christophe Henry
Cleanup/optimization | Status: closed
Component: Forms | Version: dev
Severity: Normal | Resolution:
| worksforme
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Natalia Bidart):

* resolution: => worksforme
* status: new => closed

Comment:

Replying to [comment:2 Christophe Henry]:

Hello Christophe, thank you for the extra details. In this sentence below:

> But if you do this, you won't get the result you expect when rendering
form with `{{ form }}`

I would need you to describe in detail what is the expected result from
your view, because what I would usually expect is what I get when doing a
local test.
For reference, this is my form and view, inspired by your code:

{{{#!python
from django import forms
from django.contrib import messages
from django.core.exceptions import ValidationError
from django.http import HttpResponseRedirect
from django.shortcuts import render


class FooForm(forms.Form):
foo_select = forms.CharField()

def __init__(self, user, *args, **kwargs):
super().__init__(*args, **kwargs)
self.fields["foo_select"].help_text = f"Indicate
{user.first_name}'s pet name"

def clean_foo_select(self):
value = self.cleaned_data.get("foo_select")
if value != "fido":
raise ValidationError("foo_select should be fido")


def foo_view(request):
form = FooForm(user=request.user)
if request.method == "POST":
form = FooForm(user=request.user, data=request.POST)
if form.is_valid():
messages.success(request, f"Valid {form.cleaned_data=}! :-)")
return HttpResponseRedirect(".")

messages.error(request, f"INVALID {form.data=}! :-(")

return render(request, "foo.html", {"form": form})
}}}

And in my template, the correct help text is shown at all times (on GET,
on form submission with errors and on form submission without errors).

Because of the above, I'll close as `worksforme`, but please fell free to
comment with further details. Ideally you would provide a minimal sample
Django project showcasing the issue and describing what you expect to get.

Thanks again!
Natalia.
--
Ticket URL: <https://code.djangoproject.com/ticket/35542#comment:4>
Reply all
Reply to author
Forward
0 new messages