Opened 3 years ago

Closed 23 months ago

Last modified 7 months ago

#19617 closed New feature (fixed)

mixins on ModelForms

Reported by: EvilDMP 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 3 years ago by anonymous

  • Component changed from Uncategorized to Forms
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed
  • Type changed from Uncategorized to New feature

comment:2 Changed 2 years ago by aaugustin

  • Triage Stage changed from Design decision needed to Accepted

comment:3 Changed 2 years ago by loic84

  • Has patch set
  • Owner changed from nobody to loic84
  • Status changed from new to assigned
  • Version changed from 1.4 to master

comment:4 Changed 23 months 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 23 months 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 23 months ago by Loic Bistuer <loic.bistuer@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In ac5ec7b8bcc5623cc05d4df900c39e194f5affcf:

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

Thanks apollo13, funkybob and mjtamlyn for the reviews.

comment:7 Changed 23 months 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 7 months ago by Tim Graham <timograham@…>

In 9704b0a82e1f1c6ed0118f948a56652594f0a43b:

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

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

In 89e9f81601f7a343690e1153e70fd56091246d0b:

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

comment:10 Changed 7 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 7 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