Opened 13 years ago

Closed 4 years ago

#15910 closed Bug (duplicate)

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

Download all attachments as: .zip

Change History (21)

comment:1 by Aymeric Augustin, 13 years ago

Triage Stage: UnreviewedAccepted

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.

by Arthur de Jong <arthur@…>, 13 years ago

Attachment: models.py added

example models.py file

by Arthur de Jong <arthur@…>, 13 years ago

Attachment: admin.py added

example admin.py file

by Arthur de Jong <arthur@…>, 13 years ago

screen shot demonstrating problem

comment:2 by Arthur de Jong <arthur@…>, 13 years ago

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.

by Arthur de Jong <arthur@…>, 13 years ago

comment:3 by Arthur de Jong <arthur@…>, 13 years ago

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 by Julien Phalip, 13 years ago

UI/UX: set

comment:5 by Arthur de Jong <arthur@…>, 13 years ago

Cc: arthur@… added

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

comment:6 by anonymous, 11 years ago

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 by Tim Graham, 10 years ago

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 by Arthur de Jong <arthur@…>, 10 years ago

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.

comment:9 by Arthur de Jong, 8 years ago

I have updated my patch and it is available in the ticket_15910 branch of my Github repo:

https://github.com/arthurdejong/django/tree/ticket_15910

The branch includes a Selenium tests that demonstrates the original problem.

All the tests (including the Selenium ones) pass. The branch also contains a fix in a related test that would be more quickly exposed with the made changes.

The commit also updates the minified file but does not include any updated documentation. I don't think any documentation updates are required because the delete links don't seem to be documented on
https://docs.djangoproject.com/en/1.8/ref/contrib/admin/

comment:10 by Tim Graham, 8 years ago

Needs tests: unset
Patch needs improvement: unset

comment:11 by Tim Graham, 8 years ago

Patch needs improvement: set

comment:12 by Arthur de Jong, 8 years ago

Patch needs improvement: unset

comment:13 by Tim Graham, 8 years ago

Patch needs improvement: set

comment:14 by David W. Lloyd, 6 years ago

This has been broken for seven years, and partially fixed for two. As someone new to Django who is trying to use inlines, it's frustrating that right out of the box, if validation happens to fail, the user is left with a bunch of added inline entries they have zero means of deleting.

This isn't a "New Feature", it's a bug. If the intention behind the ability to add AND remove extra inlines prior to form submission is for usability, then taking away that ability after validation fails isn't consistent with that design goal.

Use case:

  1. I spend 10-15 minutes working on a complex form entry.
  2. I add a dozen associated inline addresses/relationships/whatevers via the "add another" option.
  3. I submit the form and hold my breath.
  4. There's a validation error on one of the dozen addresses/relationships/whatevers that I entered.
  5. I realize I only needed 11, not 12, addresses/relationships/whatevers
  6. My entire form submission is now fudged, since I can't remove the single problem inline.... unless I am comfortable entering a bogus record and deleting it afterwards, if I even have that ability.
  7. Sad face.

Would love to see this fixed. It's one of a number of problems surrounding inlines in the admin.

comment:15 by David W. Lloyd, 6 years ago

Type: New featureBug

comment:16 by frnhr, 5 years ago

I think this is a duplicate of https://code.djangoproject.com/ticket/29087 Or the other way around? I have submitted a PR for 29087.

comment:17 by Nicolas Pantel, 4 years ago

Resolution: duplicate
Status: newclosed

I confirm, #29087 is a duplicate of this one.
As you proposed a PR in the other ticket, and it has a better description, I close this one as duplicate.

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