Opened 5 years ago

Closed 5 years ago

#13929 closed (invalid)

ModelForm save in version 1.2.1 not backward compatible due to construct and exclude args

Reported by: margieroginski Owned by: nobody
Component: Uncategorized Version: 1.2
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


I feel like the model validation code has not left a path that allows the modelForm save() to be easily backward compatible with the old 1.1 behavior.

I would like to avoid doing model validation for my model form, by overriding _post_clean() to be a pass. I am able to avoid the model validation just fine by doing this, but when I call the modelForm's save() method, it does not save the model form data to the model due to the fact that save() calls save_instance() with the construct argument set to False.

I understand that the save() code is assuming that I have already constructed by instance, and for that reason it is calling save_instance with construct set to False. But I think I should be able to override _post_clean with a pass and then have save() still work the way it used to. It seems like ModelForm's save() method could figure out itself whether construct_instance has already been called, and if it has not been called, called save_instance() with construct set to True. Additionally, for this to be backward compatible, we'd need to pass the exclude argument to save_instance, as we used to in 1.1.

Currently in order for me to bypass the new model validation, I need to both override _post_clean() with a pass (which I'm happy with), and also override save() such that it looks like this:

    def save(self, commit=True):

        if is None:
            fail_message = 'created'
            fail_message = 'changed'
        return save_instance(self, self.instance, self._meta.fields,
                             fail_message, commit, construct=True, exclude=self._meta.exclude)

    save.alters_data = True

It seems like this override of save() could be avoided, allowing 1.2.1 to be backward compatible with 1.1 by just overriding _post_clean() with a pass.


Change History (2)

comment:1 Changed 5 years ago by margieroginski

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from ModelForm save in version 1.2.1 not backward compatible due to not passing self._meta.exclude to ModelForm save in version 1.2.1 not backward compatible due to construct and exclude args

Changing the Summary to make more sense

comment:2 Changed 5 years ago by carljm

  • Resolution set to invalid
  • Status changed from new to closed

The fact that you can break a ModelForm by overriding undocumented methods (explicitly marked as private with the leading underscore) is, almost by definition, not a bug.

Some validation that used to be performed directly by ModelForm fields was moved to model fields in 1.2. So the manner in which validation happens changed, but the intent was that the overall effect of ModelForm validation should still be backwards-compatible. If there's a backwards-incompatibility in the resulting behavior, that may be a bug. But this report doesn't provide any information about what actual backwards-incompatibility you might be seeing when you call save() on a (unmodified) ModelForm. Please reopen if you can provide detailed information about the actual incompatible behavior.

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