[Django] #31721: The function modelform_factory does not consider the formfield_callback of the form that it recieves as argument
Django
the form that it recieves as argument
--------------------------------------+-----------------------------------
Reporter: vhaso | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.2
Severity: Normal | Keywords: modelform_factory
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------+-----------------------------------
The function django.forms.modelform_factory returns a form class based on
the class it recieves as form argument. As an additional argument it
accepts a formfield_callback function. When no callback is provided the
class uses no callback instead of the formfield_callback of the base form
provided.
Example:
{{{
from django import forms, models
class MyModel(forms.Model):
active = BooleanField()
name = CharField(max_length=64, blank=True, null=True)
def all_required(field, **kwargs):
formfield = field.formfield(**kwargs)
formfield.required = True
return formfield
class MyForm(models.ModelForm):
formfield_callback = all_required
class Meta:
model = MyModel
formfield_callback = all_required
FactoryForm = forms.modelform_factory(MyModel, form=MyForm)
}}}
The expected behavior would be that the FactoryForm uses the
formfield_callback specified in the Meta attribute of MyForm and that
therefore the name-field would be required in both the FactoryForm and
MyForm. However, under the current behavior of modelform_factory the
formfield_callback is overwritten (with the default argument None) before
the new class is constructed and in FactoryForm the name-field is not
required.
I believe this is a bug, because this behavior has been observed before in
Ticket [https://code.djangoproject.com/ticket/18573 #18573] in Django 1.3.
The test that was proposed there was incorrect, because under the expected
behavior the callback should have been called four times not two times as
was asserted. (I believe this test has been removed from version 2,
because I find no equivalent test in tests/model_formsets_regress.)
--
Ticket URL: <https://code.djangoproject.com/ticket/31721>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
the form that it recieves as argument
Reporter: Klaas-Jan Gorter | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.2
Keywords: modelform_factory | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Klaas-Jan Gorter:
Old description:
New description:
The function django.forms.modelform_factory returns a form class based on
the class it recieves as form argument. As an additional argument it
accepts a formfield_callback function. When no callback is provided the
class uses no callback instead of the formfield_callback of the base form
provided.
Example:
{{{
from django import forms
form django.db import models
--
--
Ticket URL: <https://code.djangoproject.com/ticket/31721#comment:1>
Django
the form that it recieves as argument
Reporter: Klaas-Jan Gorter | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.2
Keywords: modelform_factory | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Klaas-Jan Gorter:
Old description:
> The function django.forms.modelform_factory returns a form class based on
> the class it recieves as form argument. As an additional argument it
> accepts a formfield_callback function. When no callback is provided the
> class uses no callback instead of the formfield_callback of the base form
> provided.
>
> Example:
> {{{
> from django import forms
> form django.db import models
New description:
The function django.forms.modelform_factory returns a form class based on
the class it recieves as form argument. As an additional argument it
accepts a formfield_callback function. When no callback is provided the
class uses no callback instead of the formfield_callback of the base form
provided.
Example:
{{{
from django import forms
form django.db import models
class MyModel(forms.Model):
active = models.BooleanField()
name = models.CharField(max_length=64, blank=True, null=True)
def all_required(field, **kwargs):
formfield = field.formfield(**kwargs)
formfield.required = True
return formfield
class MyForm(forms.ModelForm):
formfield_callback = all_required
class Meta:
model = MyModel
formfield_callback = all_required
FactoryForm = forms.modelform_factory(MyModel, form=MyForm)
}}}
The expected behavior would be that the FactoryForm uses the
formfield_callback specified in the Meta attribute of MyForm and that
therefore the name-field would be required in both the FactoryForm and
MyForm. However, under the current behavior of modelform_factory the
formfield_callback is overwritten (with the default argument None) before
the new class is constructed and in FactoryForm the name-field is not
required.
I believe this is a bug, because this behavior has been observed before in
Ticket [https://code.djangoproject.com/ticket/18573 #18573] in Django 1.3.
The test that was proposed there was incorrect, because under the expected
behavior the callback should have been called four times not two times as
was asserted. (I believe this test has been removed from version 2,
because I find no equivalent test in tests/model_formsets_regress.)
--
--
Ticket URL: <https://code.djangoproject.com/ticket/31721#comment:2>
Django
the form that it recieves as argument
Reporter: Klaas-Jan Gorter | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.2
Keywords: modelform_factory | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Klaas-Jan Gorter:
Old description:
> The function django.forms.modelform_factory returns a form class based on
> the class it recieves as form argument. As an additional argument it
> accepts a formfield_callback function. When no callback is provided the
> class uses no callback instead of the formfield_callback of the base form
> provided.
>
> Example:
> {{{
> from django import forms
> form django.db import models
>
> class MyModel(forms.Model):
> active = models.BooleanField()
> name = models.CharField(max_length=64, blank=True, null=True)
>
> def all_required(field, **kwargs):
> formfield = field.formfield(**kwargs)
> formfield.required = True
> return formfield
>
> class MyForm(forms.ModelForm):
> formfield_callback = all_required
>
> class Meta:
> model = MyModel
> formfield_callback = all_required
>
> FactoryForm = forms.modelform_factory(MyModel, form=MyForm)
> }}}
> The expected behavior would be that the FactoryForm uses the
> formfield_callback specified in the Meta attribute of MyForm and that
> therefore the name-field would be required in both the FactoryForm and
> MyForm. However, under the current behavior of modelform_factory the
> formfield_callback is overwritten (with the default argument None) before
> the new class is constructed and in FactoryForm the name-field is not
> required.
>
> I believe this is a bug, because this behavior has been observed before
> in Ticket [https://code.djangoproject.com/ticket/18573 #18573] in Django
> 1.3. The test that was proposed there was incorrect, because under the
> expected behavior the callback should have been called four times not two
> times as was asserted. (I believe this test has been removed from version
> 2, because I find no equivalent test in tests/model_formsets_regress.)
New description:
The function django.forms.modelform_factory returns a form class based on
the class it recieves as form argument. As an additional argument it
accepts a formfield_callback function. When no callback is provided the
class uses no callback instead of the formfield_callback of the base form
provided.
Example:
{{{
from django import forms
form django.db import models
class MyModel(forms.Model):
active = models.BooleanField()
name = models.CharField(max_length=64, blank=True, null=True)
def all_required(field, **kwargs):
formfield = field.formfield(**kwargs)
formfield.required = True
return formfield
class MyForm(forms.ModelForm):
formfield_callback = all_required
class Meta:
model = MyModel
formfield_callback = all_required
fields = '__all__'
FactoryForm = forms.modelform_factory(MyModel, form=MyForm)
}}}
The expected behavior would be that the FactoryForm uses the
formfield_callback specified in the Meta attribute of MyForm and that
therefore the name-field would be required in both the FactoryForm and
MyForm. However, under the current behavior of modelform_factory the
formfield_callback is overwritten (with the default argument None) before
the new class is constructed and in FactoryForm the name-field is not
required.
I believe this is a bug, because this behavior has been observed before in
Ticket [https://code.djangoproject.com/ticket/18573 #18573] in Django 1.3.
The test that was proposed there was incorrect, because under the expected
behavior the callback should have been called four times not two times as
was asserted. (I believe this test has been removed from version 2,
because I find no equivalent test in tests/model_formsets_regress.)
--
--
Ticket URL: <https://code.djangoproject.com/ticket/31721#comment:3>
Django
the form that it recieves as argument
Reporter: Klaas-Jan Gorter | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 2.2
Keywords: modelform_factory | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Klaas-Jan Gorter:
Old description:
> The function django.forms.modelform_factory returns a form class based on
> the class it recieves as form argument. As an additional argument it
> accepts a formfield_callback function. When no callback is provided the
> class uses no callback instead of the formfield_callback of the base form
> provided.
>
> Example:
> {{{
> from django import forms
> form django.db import models
>
> class MyModel(forms.Model):
> active = models.BooleanField()
> name = models.CharField(max_length=64, blank=True, null=True)
>
> def all_required(field, **kwargs):
> formfield = field.formfield(**kwargs)
> formfield.required = True
> return formfield
>
> class MyForm(forms.ModelForm):
> formfield_callback = all_required
>
> class Meta:
> model = MyModel
> formfield_callback = all_required
> fields = '__all__'
>
> FactoryForm = forms.modelform_factory(MyModel, form=MyForm)
> }}}
> The expected behavior would be that the FactoryForm uses the
> formfield_callback specified in the Meta attribute of MyForm and that
> therefore the name-field would be required in both the FactoryForm and
> MyForm. However, under the current behavior of modelform_factory the
> formfield_callback is overwritten (with the default argument None) before
> the new class is constructed and in FactoryForm the name-field is not
> required.
>
> I believe this is a bug, because this behavior has been observed before
> in Ticket [https://code.djangoproject.com/ticket/18573 #18573] in Django
> 1.3. The test that was proposed there was incorrect, because under the
> expected behavior the callback should have been called four times not two
> times as was asserted. (I believe this test has been removed from version
> 2, because I find no equivalent test in tests/model_formsets_regress.)
New description:
The function django.forms.modelform_factory returns a form class based on
the class it recieves as form argument. As an additional argument it
accepts a formfield_callback function. When no callback is provided the
class uses no callback instead of the formfield_callback of the base form
provided.
Example:
{{{
from django import forms
form django.db import models
class MyModel(forms.Model):
active = models.BooleanField()
name = models.CharField(max_length=64, blank=True, null=True)
def all_required(field, **kwargs):
formfield = field.formfield(**kwargs)
formfield.required = True
return formfield
class MyForm(forms.ModelForm):
formfield_callback = all_required
class Meta:
model = MyModel
formfield_callback = all_required
fields = ['active', 'name']
FactoryForm = forms.modelform_factory(MyModel, form=MyForm)
}}}
The expected behavior would be that the FactoryForm uses the
formfield_callback specified in the Meta attribute of MyForm and that
therefore the fields would be required in both the FactoryForm and MyForm.
However, under the current behavior of modelform_factory the
formfield_callback is overwritten (with the default argument None) before
the new class is constructed and in FactoryForm the fields are not
required.
I believe this is a bug, because this behavior has been observed before in
Ticket [https://code.djangoproject.com/ticket/18573 #18573] in Django 1.3.
The test that was proposed there was incorrect, because under the expected
behavior the callback should have been called four times not two times as
was asserted. (I believe this test has been removed from version 2,
because I find no equivalent test in tests/model_formsets_regress.)
--
--
Ticket URL: <https://code.djangoproject.com/ticket/31721#comment:4>
Django
the form that it recieves as argument
Reporter: Klaas-Jan Gorter | Owner: nobody
Component: Forms | Version: 2.2
Severity: Normal | Resolution:
Keywords: modelform_factory | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* cc: Claude Paroz (added)
* type: Bug => New feature
Comment:
I was always the impression that `formfield_callback` was solely an
allowed kwarg of the `modelform_factory` function and its friends.
> I believe this test has been removed from version 2, because I find no
equivalent test in tests/model_formsets_regress.
From what I can see the patch from #18573 was never committed and thus
nothing was actually removed over the years.
Given
[https://github.com/django/django/blob/3b1cb78063466e996cbb042e44aadfac30df73fa/tests/model_forms/tests.py#L2820
no tests exists for this behavior] and that
[https://docs.djangoproject.com/en/3.0/search/?q=formfield_callback it's
not documented] I'll change this bug report to a feature request. That
seems like a really niche use case but since I don't use Django forms much
myself nowadays I'll let other folks chime in.
--
Ticket URL: <https://code.djangoproject.com/ticket/31721#comment:5>
Django
-----------------------------------+------------------------------------
Reporter: Klaas-Jan Gorter | Owner: nobody
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: modelform_factory | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* version: 2.2 => master
* stage: Unreviewed => Accepted
Comment:
I agree with Simon that it's really niche and
`ModelForm.Meta.formfield_callback` has never been a documented and
[https://github.com/django/django/blob/f386454d1302b66d0eb331ed0ae9e4811e2f3a15/django/forms/models.py#L195-L205
supported feature]. Nevertheless, we should support it for consistency
with other `Meta` options (this can also simplify fix for #24974)
--
Ticket URL: <https://code.djangoproject.com/ticket/31721#comment:6>
Django
Reporter: Klaas-Jan Gorter | Owner: Pat Garcia
Type: New feature | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: modelform_factory | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Pat Garcia):
* owner: nobody => Pat Garcia
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/31721#comment:7>
Django
Reporter: Klaas-Jan Gorter | Owner: (none)
Type: New feature | Status: new
Component: Forms | Version: dev
Severity: Normal | Resolution:
Keywords: modelform_factory | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* owner: Pat Garcia => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/31721#comment:8>
Django
Reporter: Klaas-Jan Gorter | Owner: Kamil Turek
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: modelform_factory | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Kamil Turek):
* owner: nobody => Kamil Turek
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/31721#comment:7>
Django
-----------------------------------+---------------------------------------
Reporter: Klaas-Jan Gorter | Owner: Kamil Turek
Type: New feature | Status: assigned
Component: Forms | Version: dev
Severity: Normal | Resolution:
Keywords: modelform_factory | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/15916 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/31721#comment:8>
Django
Reporter: Klaas-Jan Gorter | Owner: Kamil
| Turek
Type: New feature | Status: assigned
Component: Forms | Version: dev
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/31721#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Klaas-Jan Gorter | Owner: Kamil
| Turek
Component: Forms | Version: dev
Severity: Normal | Resolution: fixed
Keywords: modelform_factory | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e03cdf76e78ea992763df4d3e16217d298929301" e03cdf7]:
{{{
#!CommitTicketReference repository=""
revision="e03cdf76e78ea992763df4d3e16217d298929301"
Fixed #31721 -- Allowed ModelForm meta to specify form fields.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31721#comment:10>