id summary reporter owner description type status component version severity resolution keywords cc stage has_patch needs_docs needs_tests needs_better_patch easy ui_ux 13121 Backwards compatible patch for ModelForm to improve model validation. Oroku Saki nobody "Sometimes a model needs to create other model instances (to create relationships that are unimportant to the model's client) at the time of its own instantiation. This brings up problems with data integrity. If a model fails one of the validators, the related objects will be left as meaningless rows (which could have side effects such as unique values used up, etc). In order to conquer this problem Model.foo_method() level transactions need to be used. This cannot currently be done. The reason is that, although the docs say differently (this is not a documentation ticket), they are currently incorrect. Rather, the code is incorrect and the docs are correctly in tune with what Django had in mind when it designed Model validation. Currently you cannot do this because {{{ModelForm._post_clean()}}} re-implements {{{Model.full_clean()}}} rather than just calling it. Its implementation is only 1 line different than just calling {{{Model.full_clean()}}}. = Example Usage = {{{ @transaction.commit_on_success def full_clean(exclude=None): """""" Calls ``Model.clean()``, ``Model.clean_fields()``, and ``Model.validate_unique()`` """""" super(Widget, self).full_clean(exclude) def clean(self): """""" Creates self.leg (expecting this to be undone buy transaction mentioned above if self.save() never completes). The foreign relationship created here is purely for implementation reasons, and does not concern the client (ie, the view). It should function the same with any view regardless to whether a view (Admin, custom view, Django Piston JSON API, etc) enlists the use of transactions, etc. It is a means of separation of concerns, since a Widget's creator """""" try: self.leg = WidgetLeg.objects.create(friend=self, name=self.name) except IntegrityError: raise ValidationError('You did this or that wrong when saving ``Widget``.') }}} = Current Django Code = == Current Implementation of ModelForm._post_clean() == {{{ def _post_clean(self): exclude = self._get_validation_exclusions() opts = self._meta # Update the model instance with self.cleaned_data. self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude) # Clean the model instance's fields. try: self.instance.clean_fields(exclude=exclude) except ValidationError, e: self._update_errors(e.message_dict) # Call the model instance's clean method. try: self.instance.clean() except ValidationError, e: self._update_errors({NON_FIELD_ERRORS: e.messages}) # Validate uniqueness if needed. if self._validate_unique: self.validate_unique() }}} == Current Implementation of Model.full_clean() ( almost identical to ModelForm._post_clean() ) == {{{ def full_clean(self, exclude=None): """""" Calls clean_fields, clean, and validate_unique, on the model, and raises a ``ValidationError`` for any errors that occured. """""" errors = {} if exclude is None: exclude = [] try: self.clean_fields(exclude=exclude) except ValidationError, e: errors = e.update_error_dict(errors) # Form.clean() is run even if other validation fails, so do the # same with Model.clean() for consistency. try: self.clean() except ValidationError, e: errors = e.update_error_dict(errors) # Run unique checks, but only for fields that passed validation. for name in errors.keys(): if name != NON_FIELD_ERRORS and name not in exclude: exclude.append(name) try: self.validate_unique(exclude=exclude) except ValidationError, e: errors = e.update_error_dict(errors) if errors: raise ValidationError(errors) }}} == Current ``Model.clean()`` == {{{ def clean(self): self._validate_unique = True return self.cleaned_data }}} == And inside of ``ModelForm.__init__()`` on line 251 == {{{ self._validate_unique = False }}} = And... The Patch = == Change for ``ModelForm._post_clean()`` to use Model.full_clean() instead of reimplementing it == {{{ def _post_clean(self): exclude = self._get_validation_exclusions() opts = self._meta # Update the model instance with self.cleaned_data. self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude) # Perform all model validation by using the already implemented Model.full_clean() try: self.instance.full_clean(exclude=exclude) except ValidationError, e: self._update_errors(e.message_dict) }}} == Change for ``ModelForm.clean()`` to remove special ``self._validate_clean`` flag == {{{ def clean(self): # This was just setting the flag to ``True`` all the time anyway, which is an # easy placeholder to re-implement in ``Model.clean()`` if needed. return self.cleaned_data }}} == Change to ``ModelForm.__init__()`` to remove ``self._validate_clean`` flag == {{{ # Nothing, this same flagging behavior could be }}} = The result = With this, the implementation details of {{{Model.full_clean()}}} are now orthogonal to ModelForms, and ModelForms works purely on propagation. You can now use transactions and have full access to making any decorator you wish to wrap the entire validation level so that you can handle errors, or whatever other cool stuff the pros out there can think up, right from the warm comfort of the Model, which means no more repeating Model implementation steps in every single view for one model, or creating custom model forms just for the Admin. I really care a lot about this patch going into production, and if there are weaknesses that I may have overlooked, please let me know and I will update my code. This is why I've written a patch for this problem and opened a new ticket. I don't want to use a self-hacked Django. " closed Database layer (models, ORM) 1.2-beta duplicate Unreviewed 1 0 1 0 0 0