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 23994,ModelFormsets do not check pk fields of deleted forms in validation,Raphael Gaschignard,nobody," If you have a Formset, there's a way to mark certain forms as deleted through the {{{can_delete}}} parameter In {{{BaseFormset.is_valid}}}: {{{ for i in range(0, self.total_form_count()): form = self.forms[i] if self.can_delete: if self._should_delete_form(form): # This form is going to be deleted so any of its errors # should not cause the entire formset to be invalid. continue forms_valid &= form.is_valid() }}} So forms not marked for deleted will not trigger validation errors, which sounds good to me. Among other things it will not validate the primary key field of a model form. ModelFormsets give us a save method to deal with saving insteances created with formset There's a helper function to deal with saving forms created from existing objects. We decide which forms represent ""existing objects"" by the presence of a pk field in the form data In {{{BaseModelFormset.save_existing_objects}}}: {{{ for form in self.initial_forms: pk_name = self._pk_field.name raw_pk_value = form._raw_value(pk_name) # clean() for different types of PK fields can sometimes return # the model instance, and sometimes the PK. Handle either. pk_value = form.fields[pk_name].clean(raw_pk_value) pk_value = getattr(pk_value, 'pk', pk_value) }}} Here we are assuming that the pk field clean is valid! When it's not then this throws a ValidationError. so you can do {{{ if modelformset.is_valid(): modelformset.save() }}} and have a validation error on a call to {{{save}}}. To my knowledge this is counter to the general pattern of ""Validation Errors should be caught by {{{is_valid}}}"" I think this could be fixed in two ways: First way would be for the clean of modelformsets to check the pk values on deleted forms , which would trigger validation errors on the {{{is_valid}}}. A solution I would prefer is for us to ignore deleted forms for pk values that don't exist. So a try/catch in the {{{save_existing_objects}}} method that would skip over deleted forms with no found instance in the queryset. This would prevent issues such as concurrent edition (load same form twice, delete same item on both, submit the request twice), where formsets blow up. ",Uncategorized,closed,Forms,1.6,Normal,duplicate,formset,,Unreviewed,0,0,0,0,0,0