Opened 4 years ago

Closed 3 years ago

Last modified 21 months ago

#19617 closed New feature (fixed)

mixins on ModelForms

Reported by: Daniele Procida Owned by: loic84
Component: Forms Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

At present, this sort of thing is not possible:

# define a reusable mixin for ModelForms
 InputURLMixin(object):
    input_url = forms.CharField(max_length=255, required = False,
        help_text=u"<strong>External URL</strong> not found above? Enter a new one.", 
        )
  
# the form
class LinkItemForm(InputURLMixin, forms.ModelForm):
    [...]
 
# the admin class
class PluginInlineLink(admin.StackedInline):
    form = LinkItemForm
    [...]
    fieldsets = # if fieldsets contains "input_url": "Unknown field(s) (input_url) specified"

In other words, it's not possible for a ModelForm to inherit from a Mixin(object) or indeed Mixin(Form).

In the former case, the Mixin(object) fails the test in forms.ModelFormMetaclass.__new__() that would allow it to be treated as a parent worth inheriting from, so the attributes set on it are ignored when the subclass is created.

In the latter case, the metaclasses conflict in ways I don't fully understand. The docs https://docs.djangoproject.com/en/dev//topics/forms/modelforms/#form-inheritance simply remark "For technical reasons, a subclass cannot inherit from both a ModelForm and a Form simultaneously".

It doesn't seem possible to give the putative mixin class the metaclasses it needs manually to solve this (e.g. __metaclass__ = DeclarativeFieldsMetaclass or __metaclass__ = ModelFormMetaclass.

One solution might be to provide a FormMixin base class for mixins to inherit from.

Another might be to make forms.ModelFormMetaclass.__new__() less discriminating, and allow get_declared_fields() to look at attributes of classes that are not only ModelForms.

Change History (11)

comment:1 Changed 4 years ago by anonymous

Component: UncategorizedForms
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedDesign decision needed
Type: UncategorizedNew feature

comment:2 Changed 4 years ago by Aymeric Augustin

Triage Stage: Design decision neededAccepted

comment:3 Changed 3 years ago by loic84

Has patch: set
Owner: changed from nobody to loic84
Status: newassigned
Version: 1.4master

comment:4 Changed 3 years ago by Tim Graham <timograham@…>

In 54cd930baf49c45e3e91338a9f4a56d19090ff25:

Clarfied the ModelForm docs with respect to generated vs. declared fields.

The ModelForm docs suggested that fields defined declaratively override
default fields generated from the form Meta.

This is conceptually wrong, especially with inheritance in mind. Meta is
usually defined on the topmost ModelForm subclass, while fields can come
from anywhere in the MRO, especially base classes; therefore we suggested
that something defined in a base class override something from a subclass.

This patch rephrases the docs around the idea that Meta is used to generate
*missing* fields.

Refs #8620, #19617.

Thanks @mjtamlyn and @timgraham for the review.

comment:5 Changed 3 years ago by Tim Graham <timograham@…>

In 8222a48253e61f4d3c1d30156a5f9e0d59a7c56c:

[1.6.x] Clarfied the ModelForm docs with respect to generated vs. declared fields.

The ModelForm docs suggested that fields defined declaratively override
default fields generated from the form Meta.

This is conceptually wrong, especially with inheritance in mind. Meta is
usually defined on the topmost ModelForm subclass, while fields can come
from anywhere in the MRO, especially base classes; therefore we suggested
that something defined in a base class override something from a subclass.

This patch rephrases the docs around the idea that Meta is used to generate
*missing* fields.

Refs #8620, #19617.

Thanks @mjtamlyn and @timgraham for the review.

Backport of 54cd930baf from master

comment:6 Changed 3 years ago by Loic Bistuer <loic.bistuer@…>

Resolution: fixed
Status: assignedclosed

In ac5ec7b8bcc5623cc05d4df900c39e194f5affcf:

Fixed #19617 -- Refactored Form metaclasses to support more inheritance scenarios.

Thanks apollo13, funkybob and mjtamlyn for the reviews.

comment:7 Changed 3 years ago by Marc Tamlyn <marc.tamlyn@…>

In ce823d37109208644ec8a9acf2dfa915c73c42eb:

Merge pull request #1382 from loic/ticket19617

Fixed #19617 -- Refactored form metaclasses to support more inheritance scenarios.

comment:8 Changed 21 months ago by Tim Graham <timograham@…>

In 9704b0a82e1f1c6ed0118f948a56652594f0a43b:

Removed forms.forms.get_declared_fields() per deprecation timeline; refs #19617.

comment:9 Changed 21 months ago by Tim Graham <timograham@…>

In 89e9f81601f7a343690e1153e70fd56091246d0b:

Clarified deprecation of forms.forms.get_declared_fields(); refs #19617.

comment:10 Changed 21 months ago by Tim Graham <timograham@…>

In af523573fc523cf5afba24b178f4b6bded6f1def:

[1.7.x] Clarified deprecation of forms.forms.get_declared_fields(); refs #19617.

Backport of 89e9f81601f7a343690e1153e70fd56091246d0b from master

comment:11 Changed 21 months ago by Tim Graham <timograham@…>

In f31ed5c934e7e664b912c13bd854cc9445f34ec5:

[1.8.x] Clarified deprecation of forms.forms.get_declared_fields(); refs #19617.

Backport of 89e9f81601f7a343690e1153e70fd56091246d0b from master

Note: See TracTickets for help on using tickets.
Back to Top