Opened 7 years ago

Last modified 8 weeks ago

#9061 assigned New feature

formsets with can_delete=True shouldn't add delete field to extra forms

Reported by: gsf Owned by: danielward
Component: Forms Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
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 gsf 7 years ago.
don't add delete field to formset extra forms
t9061-a.diff (1.3 KB) - added by gsf 7 years ago.
newer diff with edit to forms.models to avoid error on save
formsets.py.diff (202 bytes) - added by oban 4 years ago.
updated patch for 1.3.1
before.patch (1.6 KB) - added by danielward 3 months ago.
Latest 'before' test to confirm behaviour
potential-solution.patch (2.0 KB) - added by danielward 3 months ago.
Potential solution by adding in 'can_delete_extra' option
after.patch (2.5 KB) - added by danielward 3 months ago.
Tests introduced to ensure all is well following introduction of potential solution

Download all attachments as: .zip

Change History (17)

Changed 7 years ago by gsf

don't add delete field to formset extra forms

Changed 7 years ago by gsf

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

comment:1 Changed 7 years ago by SmileyChris

  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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

comment:2 Changed 7 years ago by gsf

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 Changed 6 years ago by jkocherhans

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

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

comment:4 Changed 6 years ago by Matthew

  • Resolution fixed deleted
  • Status changed from closed to 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 Changed 4 years ago by lukeplant

  • Severity set to Normal
  • Type set to New feature

Changed 4 years ago by oban

updated patch for 1.3.1

comment:6 Changed 4 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:7 Changed 4 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:8 Changed 2 years ago by aaugustin

  • Status changed from reopened to new

comment:9 Changed 3 months ago by danielward

  • Owner changed from nobody to danielward
  • Status changed from new to 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.

Changed 3 months ago by danielward

Latest 'before' test to confirm behaviour

Changed 3 months ago by danielward

Potential solution by adding in 'can_delete_extra' option

Changed 3 months ago by danielward

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

comment:10 Changed 2 months ago by timgraham

  • Needs tests unset

comment:11 Changed 8 weeks ago by timgraham

  • Patch needs improvement set

Left comments for improvement on the PR.

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