Opened 8 years ago

Last modified 7 weeks ago

#25887 new Cleanup/optimization

Clarify support for ForeignKey and form fields other than ModelChoiceField

Reported by: Miriam Gershenson Owned by: nobody
Component: Documentation Version: dev
Severity: Normal Keywords: ModelForm, ForeignKey
Cc: benjaminjkraft@…, Ülgen Sarıkavak Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Miriam Gershenson)

I'm maintaining a project which has a similar situation to #23795: we have an ajax autocomplete form field for ForeignKeys, which in our case inherits from forms.IntegerField. We also have a custom model field which subclasses ForeignKey in order to override formfield() and specify our custom form field as the default.

When we updated from Django 1.6 to 1.8, ModelForms for all models using this field started erroring on import because of the limit_choices_to keyword argument getting inserted by RelatedField.formfield(), not being handled by our form field (which has no support for limit_choices_to because we haven't needed it), and eventually being passed to the Field constructor through some super() calls.

Here is a simple example of the error:

class CustomForeignKey(models.ForeignKey):
    def formfield(self, **kwargs):
        defaults = {'form_class': forms.IntegerField} # or a subclass of forms.IntegerField
        defaults.update(kwargs)
        return super(CustomForeignKey, self).formfield(**defaults)

class Target(models.Model):
    pass

class NormalFKModel(models.Model):
    fk = models.ForeignKey(Target)

class CustomFKModel(models.Model):
    fk = CustomForeignKey(Target)

# This will work fine until I try to submit the form
class NormalFKModelForm(forms.ModelForm):
    fk = forms.IntegerField()
    class Meta:
        model = NormalFKModel
        fields = '__all__'

# This will error on import on all Django versions
class CustomFKModelForm(forms.ModelForm):
    class Meta:
        model = CustomFKModel
        fields = '__all__'

A similar error happens even on older Django, or when I add a workaround for limit_choices_to only; the code I had which worked on 1.6 already included some workarounds for a few other similar issues, which I hadn't been aware of.

The documentation on overriding formfield() doesn't mention this pitfall, and neither does the form field documentation, or any other relevant documentation I could find.

Ideally, both of the ModelForm examples above would display without erroring, and would work correctly if the forms.IntegerField is replaced by a subclass which normalizes to model instances rather than integers, especially considering that one of them already meets those conditions. Alternatively, I would also consider this fixed if some documentation were added on special requirements for custom form fields that will be used for ForeignKeys.

Change History (7)

comment:1 by Ben Kraft, 8 years ago

Cc: benjaminjkraft@… added

comment:2 by Miriam Gershenson, 8 years ago

Description: modified (diff)

comment:3 by Tim Graham, 8 years ago

Is there a reason you need a custom form field instead of simply a custom widget?

comment:4 by Miriam Gershenson, 8 years ago

Yes, we have some custom logic in the form field's __init__() and clean() as well. In any case, this error happens in almost the same situation as in #23795, but with the addition of a custom model field which exists only to set the custom form field as the default, and if that use case is supported, it makes sense for this one to be as well. The issue with limit_choices_to was a regression since 1.6, and the documentation currently has no indication that what we're trying to do might not be supported.

comment:5 by Tim Graham, 8 years ago

I haven't fully investigated but I'm not sure Django can do anything about it other than document that your Field must pop off any unexpected kwargs. Do you have a proposal?

comment:6 by Tim Graham, 8 years ago

Component: FormsDocumentation
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

Accepting as a documentation issue pending further investigation.

comment:7 by Ülgen Sarıkavak, 7 weeks ago

Cc: Ülgen Sarıkavak added
Note: See TracTickets for help on using tickets.
Back to Top