Opened 4 years ago

Last modified 9 months ago

#15910 new New feature

show delete links for all admin inline formset rows

Reported by: Arthur de Jong <arthur@…> Owned by: nobody
Component: contrib.admin Version: 1.3
Severity: Normal Keywords:
Cc: arthur@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: yes

Description

When adding a form to an inline formset in the admin interface it has a nice "Remove" link added automatically. These links are however missing from empty rows that have been created as a result from the "extra" option. They are also removed when the form is saved and validation of the form as a whole fails.

I think each row in the formset should have either a delete checkbox (signifying that a database entry will be removed) or have a delete link that removes the row using JavaScript.

In our project I've worked around this by adding the remove link to all rows matching
$(rows).not(".has_original").not(".empty-form").not(".add-row")
in tabular.html but I ended up copying code from inlines.js because there is no easy way to get at the options and other code and data available in inlines.js.

I'm willing to have a look at extending inlines.js to implement this. The problem is that in inlines.js the remove link is only added when the "add another..." link is clicked. This should be changed to have this link always added.

Attachments (4)

models.py (223 bytes) - added by Arthur de Jong <arthur@…> 4 years ago.
example models.py file
admin.py (276 bytes) - added by Arthur de Jong <arthur@…> 4 years ago.
example admin.py file
Screenshot-Change top model.png (28.9 KB) - added by Arthur de Jong <arthur@…> 4 years ago.
screen shot demonstrating problem
15910-delete-links-on-all-inline-formset-items.patch (5.4 KB) - added by Arthur de Jong <arthur@…> 4 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 4 years ago by aaugustin

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

Attaching an example of models.py + admin.py (+ screenshot?) would make it easier to understand the problem and review your patch, when it's ready.

Changed 4 years ago by Arthur de Jong <arthur@…>

example models.py file

Changed 4 years ago by Arthur de Jong <arthur@…>

example admin.py file

Changed 4 years ago by Arthur de Jong <arthur@…>

screen shot demonstrating problem

comment:2 Changed 4 years ago by Arthur de Jong <arthur@…>

The screen shot was generated from an otherwise vanilla app by saving an instance of TopModel with one InlineModel instance, editing the object removing the name from the topmodel instance and clicking "Add another" once (the validation error makes problem slightly more apparent).

The second and third items from the inline model cannot be easily removed. This can only be done by resetting all the fields to their default values (which may be hard if there are multiple fields with complex defaults).

I'll start working on a patch.

Changed 4 years ago by Arthur de Jong <arthur@…>

comment:3 Changed 4 years ago by Arthur de Jong <arthur@…>

  • Has patch set

Just added a patch that implements the described functionality. It moves
the code that adds the delete link from the "add another" link to a loop
over all inline objects that don't already have an original object (they
should have a delete checkbox).

The delete link is also added to the hidden empty row so automatically
ends up in any dynamically added rows.

To determine which rows should get a delete link it checks the row for an
ID of the original inline object. My initial attempt checked for the
presense of the delete checkbox but that didn't work correctly when the
formset has the can_delete property set to False.

It would be a lot simpler by just doing

$(this).not(".has_original").each(

but this class is only assigned to the TabularInline
formset. An alternative approach would be to add the has_original class
in [source:django/trunk/django/contrib/admin/templates/admin/edit_inline/stacked.html stacked.html] also. The downside of that is that there may be third-party
inline templates that may not have this class.

The patch is tested with both TabularInline and StackedInline with
can_delete set to True and False on the following browsers:

  • Chromium 11.0.696.65 on Debian
  • Iceweasel 3.6.13 on Debian
  • Internet Explorer 8.0.6001.18702 on Windows XP
  • Firefox 3.6.17 on Windows XP

Also, a few lines down from my modifications I notice that a line is
indented with spaces instead of tabs. Do you also want me to include the
tabs in the patch?

Note that the patch doesn't include the minified version.

comment:4 Changed 4 years ago by julien

  • UI/UX set

comment:5 Changed 4 years ago by Arthur de Jong <arthur@…>

  • Cc arthur@… added

Hi, is there anything else I can do to get this into Django?

comment:6 Changed 2 years ago by anonymous

Just tried Arthurs patch and with it seems to work nice with Django 1.5 too (just some minor row adjustments). Would be nice to have this included in next Django release.

comment:7 Changed 9 months ago by timo

  • Needs tests set
  • Patch needs improvement set

Current patch likely needs to be updated to apply clealy and also could use some selenium tests.

comment:8 Changed 9 months ago by Arthur de Jong <arthur@…>

I'm more than happy to invest some more time in this to get the patch in shape if there is a desire to get this fixed. Please let me know.

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