Opened 7 years ago

Last modified 23 months ago

#7018 new New feature

Make ModelForm multiple inheritance possible

Reported by: bear330 Owned by: nobody
Component: Forms Version: master
Severity: Normal Keywords:
Cc: robillard.etienne@…, mmitar@…, carbonXT, ciantic@…, djsnickles@…, semente+django@…, bouke, cyen0312+django@…, maa@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If I have two ModelForm:

class AForm(ModelForm):
    class Meta:
        model = A

class BForm(ModelForm):
    class Meta:
        model = B

class CForm(AForm, BForm):
    pass

This doesn't work because of the code:

        declared_fields = get_declared_fields(bases, attrs, False)

in ModelFormMetaclass.

I can to this to make it work:

# Because ModelFormMetaclass will call get_declared_fields method with
# with_base_fields=False, we modify it with True.
from django.newforms import models as nmodels
gdf = nmodels.get_declared_fields
nmodels.get_declared_fields = \
    lambda bases, attrs, with_base_fields: gdf(bases, attrs, True)

But it will be nice if this behavior is default behavior.

Thanks.

Attachments (1)

metaforms.py (5.7 KB) - added by mitar 5 years ago.
Metaclass and mixin which allows multiple ModelForm inheritance

Download all attachments as: .zip

Change History (23)

comment:1 follow-up: Changed 7 years ago by bear330

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

A reference about form multiple inheritance:

http://www.djangosnippets.org/snippets/703/

comment:2 Changed 7 years ago by erob

  • Cc robillard.etienne@… added

comment:3 Changed 7 years ago by garcia_marc

  • milestone set to post-1.0

comment:4 in reply to: ↑ 1 Changed 7 years ago by bear330

After [7846], that this way to make ModelForm be able to multiple inheritance is invalid.

New way to make it work is:

def getMixupFormMetaClass():
    """
    Get metaclass for form class that inherits multiple forms, especially the
    base classes of form mix up with Form and ModelForm (metaclass mix up with
    DeclarativeFieldsMetaclass, ModelFormMetaclass).
    See http://code.djangoproject.com/ticket/7018 for related details.
    @return Function that will generate class that metaclass will do.
    """
    from django.newforms import models as nmodels
    factory = classmaker()
    def new(name, bases, attrs):
        # get_declared_fields can only be called once, the result will be wrong
        # if you call it again. We call it first to hold the correct result.
        fields = nmodels.get_declared_fields(bases, attrs, True)
        newclass = factory(name, bases, attrs)
        # Restore the correct result.
        newclass.base_fields = fields
        return newclass
    return new

comment:5 Changed 7 years ago by ericholscher

  • Triage Stage changed from Unreviewed to Design decision needed

comment:6 Changed 7 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:7 Changed 6 years ago by Wernight

It may be nice to have also Form and ModelForm multiple inheritance support. Once case I can think of is having the a user profile ModelForm couple with the Auth UserCreationForm. Illustated below:

class UserProfile(models.Model):
    # This is requred fields for AUTH_PROFILE_MODULE
    user = models.ForeignKey(User, unique=True)

    # User profile information
    age = models.PositiveSmallIntegerField(_("Age"))
    gender = models.CharField(_("Gender"), max_length=1, choices=(
        ('M', _('Male')),
        ('F', _('Female')),
    ))

class MyRegistrationForm(UserCreationForm, forms.ModelForm):
    class Meta:
        model = UserProfile

Given that Django provides a good support user profiles, it may be a good idea to support those profiles' creation in an easy way for fields that a project see as being required for all its users.

comment:8 Changed 5 years ago by mitar

  • Cc mmitar@… added

comment:9 Changed 5 years ago by mitar

  • Summary changed from Make ModelForm multiple inheritance is possible. to Make ModelForm multiple inheritance possible

I made a metaclass which allows multiple ModelForm forms to be combined. I also made a mixin which then allows resulting form to be used on multiple instances (for each parent ModelForm one) which are then used to populate form fields, validation and also they are all updated by simply calling form.save(). All this is heavily hackish but it works and it proves this is doable and could go also into official version (where it would be much less hackish). I am attaching the code.

The problem is that it is hard to reason about such forms and code and even decide which behavior is proper (for example: how to order fields in a form with multiple parents and even with some manually declared fields? what if fields definitions overlap?). (It is useful to use FieldsetFormMixin described bellow to remedy this.)

All this can be used with my mixin for fieldsets so that every parent form can be put into its own fieldset. Then it is possible to do such form which allows user and profile changes at the same time:

class UserAndProfileChangeForm(metaforms.FieldsetFormMixin, metaforms.ParentsIncludedModelFormMixin, UserChangeForm, UserProfileChangeForm):
  error_css_class = 'error'
  required_css_class = 'required'
  fieldset = UserChangeForm.fieldset + [(
    utils_text.capfirst(UserProfileChangeForm.Meta.model._meta.verbose_name), {
      'fields': UserProfileChangeForm.base_fields.keys(),
    })]

  __metaclass__ = metaforms.ParentsIncludedModelFormMetaclass

And save on such form validates and saves both objects. Ah, one more thing, form initialization changes somewhat. instance attribute gets a list of instances:

forms.UserAndProfileChangeForm(request.POST, instance=[request.user, request.user.get_profile()])

But be careful if you pass objects directly from request, those instances are changed by form.is_valid method even if form does not validate which could allow user to alter some context (post)processors behavior. So it is necessary to do something like:

if request.method == 'POST':
  stored_user = copy.copy(request.user)
  form = forms.UserAndProfileChangeForm(request.POST, instance=[request.user, request.user.get_profile()])
  if form.is_valid():
    objs = form.save()
    return shortcuts.redirect(objs[-1]) # The last element is user profile object
  else:
    # Restore user request object as it is changed by form.is_valid
    request.user = stored_user
    if hasattr(request.user, '_profile_cache'):
      # Invalidates profile cache
      delattr(request.user, '_profile_cache')
else:
  form = forms.UserAndProfileChangeForm(instance=[request.user, request.user.get_profile()])

Also user registration form (for use with django-registration) is doable:

class UserRegistrationForm(metaforms.FieldsetFormMixin, metaforms.ParentsIncludedModelFormMixin, UserCreationForm, UserProfileChangeForm):
  error_css_class = 'error'
  required_css_class = 'required'
  fieldset = UserCreationForm.fieldset + [(
    utils_text.capfirst(UserProfileChangeForm.Meta.model._meta.verbose_name), {
      'fields': UserProfileChangeForm.base_fields.keys(),
    })]
  
  def save(self, commit=True):
    # We disable save method as registration backend module should take care of user and user
    # profile objects creation and we do not use this form for changing data
    assert False
    return None
  
  __metaclass__ = metaforms.ParentsIncludedModelFormMetaclass

With django-registration backend having such register method:

def register(self, request, **kwargs):
  user = super(ProfileBackend, self).register(request, **kwargs)
  profile, created = utils.get_profile_model().objects.get_or_create(user=user)
  
  # lambda-object to the rescue
  form = lambda: None
  form.cleaned_data = kwargs
  
  # First name, last name and e-mail address are stored in user object
  forms_models.construct_instance(form, user)
  user.save()
  
  # Other fields are stored in user profile object
  forms_models.construct_instance(form, profile)
  profile.save()
  
  return user

Changed 5 years ago by mitar

Metaclass and mixin which allows multiple ModelForm inheritance

comment:10 Changed 5 years ago by carbonXT

  • Cc carbonXT added

comment:11 Changed 4 years ago by Ciantic

  • Cc ciantic@… added

This is especially needed as the class based views does not support views with multiple forms.

I have models and forms that are joined together with ForeignKey or OneToOne, e.g. I have models: User, UserProfile, TwitterProfile, FacebookProfile, but there is no easy way to write views, it would be handy if I could just simply combine the forms as one form.

There is also requirement to combine formset and form in one view, e.g. billing view where InvoiceForm and InvoiceRowFormset should be joined. They do implement rather similar interface (save, clean, etc) but creating good class based views that support multiple forms in addition to formsets in one view needs more thinking.

comment:12 Changed 4 years ago by mitar

Have you tried my mixin above? It works for me for combining user, UserProfile and my UserSettings models into one form.

comment:13 Changed 4 years ago by julien

  • Severity set to Normal
  • Type set to New feature

comment:14 Changed 4 years ago by tehfink <djsnickles@…>

  • Cc djsnickles@… added

comment:15 Changed 4 years ago by semente

  • Cc semente+django@… added
  • Easy pickings unset
  • UI/UX unset

comment:16 Changed 4 years ago by Alex

  • Triage Stage changed from Design decision needed to Accepted

comment:17 Changed 3 years ago by bouke

  • Cc bouke added

comment:18 Changed 2 years ago by Lance0312

  • Cc cyen0312+django@… added

comment:19 Changed 23 months ago by maa@…

  • Cc maa@… added

comment:20 follow-up: Changed 23 months ago by mjtamlyn

Worth noting here that you can now combine Form and ModelForm together. Combining the multiple models is a much more complex issue, and one I'm personally not convinced should be included. This has implications on validation and saving which could have a myriad of edge cases. Personally I think some code which renders and validates two forms would be superior, handled effectively as a nonhomogeneous Form set.

comment:21 in reply to: ↑ 20 Changed 23 months ago by anonymous

Replying to mjtamlyn:

Worth noting here that you can now combine Form and ModelForm together. Combining the multiple models is a much more complex issue, and one I'm personally not convinced should be included. This has implications on validation and saving which could have a myriad of edge cases. Personally I think some code which renders and validates two forms would be superior, handled effectively as a nonhomogeneous Form set.

This is closer to what I want to do (combine multiple model forms in one form)
Where can I find info on this ?
thanks,

comment:22 Changed 23 months ago by mjtamlyn

Sounds like you want something along the lines of https://github.com/bmispelon/django-multiform

You'd nerd to write the connection logic yourself each time but I think that's fine. Personally I'm not convinced this is code which needs to live in core. Baptiste may have a different opinion.

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