Opened 16 years ago

Closed 4 years ago

Last modified 4 years ago

#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)

no_delete_formsets_extra.diff (673 bytes ) - added by Gabriel Farrell 16 years ago.
don't add delete field to formset extra forms
t9061-a.diff (1.3 KB ) - added by Gabriel Farrell 16 years ago.
newer diff with edit to forms.models to avoid error on save
formsets.py.diff (202 bytes ) - added by oban 13 years ago.
updated patch for 1.3.1
before.patch (1.6 KB ) - added by Daniel Ward 9 years ago.
Latest 'before' test to confirm behaviour
potential-solution.patch (2.0 KB ) - added by Daniel Ward 9 years ago.
Potential solution by adding in 'can_delete_extra' option
after.patch (2.5 KB ) - added by Daniel Ward 9 years ago.
Tests introduced to ensure all is well following introduction of potential solution

Download all attachments as: .zip

Change History (27)

by Gabriel Farrell, 16 years ago

don't add delete field to formset extra forms

by Gabriel Farrell, 16 years ago

Attachment: t9061-a.diff added

newer diff with edit to forms.models to avoid error on save

comment:1 by Chris Beaven, 16 years ago

Needs tests: set
Triage Stage: UnreviewedAccepted

I thought brosner already had done this. Seems a valid issue, anyway.

comment:2 by Gabriel Farrell, 16 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 jkocherhans, 16 years ago

Resolution: fixed
Status: newclosed

I don't think this is the case on trunk anymore. I don't see delete checkboxes for "add" forms.

comment:4 by Matthew, 15 years ago

Resolution: fixed
Status: closedreopened

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 Luke Plant, 14 years ago

Severity: Normal
Type: New feature

by oban, 13 years ago

Attachment: formsets.py.diff added

updated patch for 1.3.1

comment:6 by Aymeric Augustin, 13 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:7 by Aymeric Augustin, 13 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:8 by Aymeric Augustin, 12 years ago

Status: reopenednew

comment:9 by Daniel Ward, 9 years ago

Owner: changed from nobody to Daniel Ward
Status: newassigned

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 Daniel Ward, 9 years ago

Attachment: before.patch added

Latest 'before' test to confirm behaviour

by Daniel Ward, 9 years ago

Attachment: potential-solution.patch added

Potential solution by adding in 'can_delete_extra' option

by Daniel Ward, 9 years ago

Attachment: after.patch added

Tests introduced to ensure all is well following introduction of potential solution

comment:10 by Tim Graham, 9 years ago

Needs tests: unset

comment:11 by Tim Graham, 9 years ago

Patch needs improvement: set

Left comments for improvement on the PR.

comment:12 by Daniel Ward, 9 years ago

Addressed points raised in PR (4772) and opened new PR (5323) as advised.

comment:13 by Tim Graham, 9 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 Daniel Ward, 9 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 Daniel Ward, 9 years ago

Patch needs improvement: unset

comment:16 by Tim Graham, 9 years ago

Patch needs improvement: set

The new option should be available on model formsets too.

comment:17 by David Smith, 4 years ago

Owner: changed from Daniel Ward to David Smith

comment:18 by David Smith, 4 years ago

Patch needs improvement: unset

comment:19 by Carlton Gibson, 4 years ago

Triage Stage: AcceptedReady for checkin

comment:20 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In 162765d6:

Fixed #9061 -- Allowed FormSets to disable deleting extra forms.

Thanks to Dan Ward for the initial patch.

comment:21 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In 35b03788:

Refs #9061 -- Allowed GenericInlineFormSet to disable deleting extra forms.

Follow up to 162765d6c3182e36095d29543e21b44b908625fc.

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