Code

Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#9587 closed (fixed)

formset with can_delete shouldn't require deleted forms to validate

Reported by: Daniel Pope <dan@…> Owned by: nobody
Component: Forms Version: 1.0
Severity: Keywords: formset can_delete
Cc: vbmendes Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

If I check the can_delete checkbox for a FormSet form, I would expect the form to be deleted regardless of whether there are validation errors in that particular form. I'm getting "This field is required" even for forms I'm trying to delete.

I'm using inlineformset_factory.

Attachments (1)

formset_validation-r9699.diff (2.7 KB) - added by kratorius 5 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 5 years ago by kratorius

  • Keywords formset can_delete added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I see two possible solutions here:

  1. we don't validate the single form
  2. we validate the form, but the formset's will be valid even if the field that is going to be deleted is invalid

I'm more inclined to go with the second option since if the user checks the deletion checkbox but other fields are invalid, we should redisplay the whole formset and show eventual errors.

Anyway I can't see a clean way of fixing this issue. I wrote a patch, but I don't like the solution mainly because the only way I have to know if the form has to be deleted is to check for the DELETE string in the initial data (I can't access form.cleaned_data since if the form has errors cleaned_data gets deleted).

Changed 5 years ago by kratorius

comment:2 Changed 5 years ago by kratorius

  • Has patch set
  • Patch needs improvement set

comment:3 Changed 5 years ago by vbmendes

  • Cc vbmendes added

I think the first solution is better. If you want to delete the form, it doesn't matter if it validates or not.

comment:4 Changed 5 years ago by jacob

  • milestone set to 1.1
  • Triage Stage changed from Unreviewed to Accepted

comment:5 Changed 5 years ago by jkocherhans

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

(In [10206]) Fixed #9587. Formset.is_valid() now returns True if an invalid form is marked for deletion. Thanks for the test and intial patch, kratorius.
Note that this leaves the form and formset errors alone. Those forms still have errors, it's just that it doesn't matter that they're invalid in the context of the formset and deletion.
Also fixed #9665 while I was in there. Thanks, mark_hildreth.

comment:6 Changed 5 years ago by jkocherhans

(In [10219]) [1.0.X] Fixed #9587. Formset.is_valid() now returns True if an invalid form is marked for deletion. Backport of r10206 from trunk.

comment:7 Changed 5 years ago by jsamsa

Can we improve this to read the delete field value from cleaned_data so that validation has a chance to raise validation errors on delete? I have a case where I don't want to allow delete on inlines after the parent model reaches a certain state. I'll try writing a patch.

comment:8 Changed 4 years ago by danstadler

I would agree with jsamsa's comment on 06/08/09. I have a case where I don't want to delete the last child object, if the parent object is in a particular state. I'd like to be able to throw a ValidationError on delete, but that doesn't seem allowed. BTW we are on Django 1.0.3.

comment:9 Changed 3 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.