Opened 11 years ago

Closed 10 years ago

Last modified 9 years ago

#19617 closed New feature (fixed)

mixins on ModelForms

Reported by: Daniele Procida Owned by: loic84
Component: Forms Version: dev
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 by anonymous, 11 years ago

Component: UncategorizedForms
Triage Stage: UnreviewedDesign decision needed
Type: UncategorizedNew feature

comment:2 by Aymeric Augustin, 11 years ago

Triage Stage: Design decision neededAccepted

comment:3 by loic84, 11 years ago

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

comment:4 by Tim Graham <timograham@…>, 10 years ago

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 by Tim Graham <timograham@…>, 10 years ago

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 by Loic Bistuer <loic.bistuer@…>, 10 years ago

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 by Marc Tamlyn <marc.tamlyn@…>, 10 years ago

In ce823d37109208644ec8a9acf2dfa915c73c42eb:

Merge pull request #1382 from loic/ticket19617

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

comment:8 by Tim Graham <timograham@…>, 9 years ago

In 9704b0a82e1f1c6ed0118f948a56652594f0a43b:

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

comment:9 by Tim Graham <timograham@…>, 9 years ago

In 89e9f81601f7a343690e1153e70fd56091246d0b:

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

comment:10 by Tim Graham <timograham@…>, 9 years ago

In af523573fc523cf5afba24b178f4b6bded6f1def:

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

Backport of 89e9f81601f7a343690e1153e70fd56091246d0b from master

comment:11 by Tim Graham <timograham@…>, 9 years ago

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