#9061 closed New feature (fixed)
formsets with can_delete=True shouldn't add delete field to extra forms
| Reported by: | Gabriel Farrell | Owned by: | David Smith |
|---|---|---|---|
| Component: | Forms | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Current behavior of formsets with can_delete=True is to add a delete field to every form. This behavior differs from that expected, however (why would one want a delete option on an "add" form?), as well as that of the builtin admin. I've included a patch on formsets.py, but haven't bothered with patching tests yet.
Attachments (6)
Change History (27)
by , 17 years ago
| Attachment: | no_delete_formsets_extra.diff added |
|---|
by , 17 years ago
| Attachment: | t9061-a.diff added |
|---|
newer diff with edit to forms.models to avoid error on save
comment:1 by , 17 years ago
| Needs tests: | set |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
I thought brosner already had done this. Seems a valid issue, anyway.
comment:2 by , 17 years ago
Having lived with this for a while, I can see the case where one might want a delete field on extra forms. I'd still argue that it shouldn't be the default, but it could be another option on formsets. Perhaps delete_extra or extra_delete?
comment:3 by , 17 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
I don't think this is the case on trunk anymore. I don't see delete checkboxes for "add" forms.
comment:4 by , 16 years ago
| Resolution: | fixed |
|---|---|
| Status: | closed → reopened |
The delete fields still appear here on all rows with the latest trunk, r11468.
from django import forms
class Form(forms.Form):
test = forms.CharField()
FormSet = forms.formsets.formset_factory(Form, can_delete=1)
formset = FormSet(initial=[{'test':'Some initial data for the first row'}])
The formset contains two rows, both of which have a delete checkbox.
comment:5 by , 15 years ago
| Severity: | → Normal |
|---|---|
| Type: | → New feature |
comment:8 by , 13 years ago
| Status: | reopened → new |
|---|
comment:9 by , 10 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
While I can understand how from a usability perspective this could be confusing, the ability to delete can also be a useful way to discard a new form entry rather than having to clear each populated field for the given form(s).
As a result, I propose adding a can_delete_extra option to formsets, which allow developers to decide whether they wish to omit deletion fields from their formsets without having to write any any additional logic in to their templates/views.
I'm about to attach the relevant patches. If accepted, I'm happy to provide a patch to public documentation for reference also.
by , 10 years ago
| Attachment: | potential-solution.patch added |
|---|
Potential solution by adding in 'can_delete_extra' option
by , 10 years ago
| Attachment: | after.patch added |
|---|
Tests introduced to ensure all is well following introduction of potential solution
comment:12 by , 10 years ago
Addressed points raised in PR (4772) and opened new PR (5323) as advised.
comment:13 by , 10 years ago
Thanks for the updating patch. After you do so, don't forget to uncheck "Patch needs improvement" so the ticket returns to the review queue. I'll leave it checked for now as we've hit the feature freeze for 1.9 and the patch will need to be updated for 1.10 once we cut the stable/1.9.x branch this week.
comment:14 by , 10 years ago
Thanks Tim, I've updated the version number in the associated documentation from 1.9 to 1.10 accordingly. I'll update the ticket once the automated builds have succeeded.
comment:15 by , 10 years ago
| Patch needs improvement: | unset |
|---|
comment:16 by , 10 years ago
| Patch needs improvement: | set |
|---|
The new option should be available on model formsets too.
comment:17 by , 5 years ago
| Owner: | changed from to |
|---|
comment:18 by , 5 years ago
| Patch needs improvement: | unset |
|---|
comment:19 by , 5 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
don't add delete field to formset extra forms