Opened 16 years ago

Last modified 3 years ago

#7018 new New feature

Make ModelForm multiple inheritance possible

Reported by: bear330 Owned by: nobody
Component: Forms Version: dev
Severity: Normal Keywords:
Cc: robillard.etienne@…, mmitar@…, ciantic@…, djsnickles@…, semente+django@…, Bouke Haarsma, 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 14 years ago.
Metaclass and mixin which allows multiple ModelForm inheritance

Download all attachments as: .zip

Change History (25)

comment:1 by bear330, 16 years ago

A reference about form multiple inheritance:

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

comment:2 by Etienne Robillard, 16 years ago

Cc: robillard.etienne@… added

comment:3 by Marc Garcia, 16 years ago

milestone: post-1.0

in reply to:  1 comment:4 by bear330, 16 years ago

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 by Eric Holscher, 16 years ago

Triage Stage: UnreviewedDesign decision needed

comment:6 by (none), 15 years ago

milestone: post-1.0

Milestone post-1.0 deleted

comment:7 by Werner Beroux, 14 years ago

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 by Mitar, 14 years ago

Cc: mmitar@… added

comment:9 by Mitar, 14 years ago

Summary: Make ModelForm multiple inheritance is possible.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

by Mitar, 14 years ago

Attachment: metaforms.py added

Metaclass and mixin which allows multiple ModelForm inheritance

comment:10 by Mike Fogel, 13 years ago

Cc: Mike Fogel added

comment:11 by Jari Pennanen, 13 years ago

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 by Mitar, 13 years ago

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

comment:13 by Julien Phalip, 13 years ago

Severity: Normal
Type: New feature

comment:14 by tehfink <djsnickles@…>, 13 years ago

Cc: djsnickles@… added

comment:15 by Guilherme Gondim, 13 years ago

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

comment:16 by Alex Gaynor, 13 years ago

Triage Stage: Design decision neededAccepted

comment:17 by Bouke Haarsma, 11 years ago

Cc: Bouke Haarsma added

comment:18 by Lance Chen, 11 years ago

Cc: cyen0312+django@… added

comment:19 by maa@…, 10 years ago

Cc: maa@… added

comment:20 by Marc Tamlyn, 10 years ago

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.

in reply to:  20 comment:21 by anonymous, 10 years ago

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 by Marc Tamlyn, 10 years ago

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.

comment:23 by Mitar, 8 years ago

Now with Django 1.9, the approaches mentioned in this ticket do not work anymore, because get_declared_fields is not available anymore. What would be a new approach?

comment:24 by Mike Fogel, 3 years ago

Cc: Mike Fogel removed
Note: See TracTickets for help on using tickets.
Back to Top