Opened 7 years ago

Closed 4 years ago

Last modified 4 years ago

#29087 closed Bug (fixed)

Impossible to delete pending new inline in admin when invalid (delete button missing)

Reported by: Owen Heisler Owned by: frnhr
Component: contrib.admin Version: 2.0
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: yes

Description (last modified by frnhr)

When adding a new inline record in the Django Admin, it is impossible to delete the record if it has had a validation error. This is the most evident to the user if he/she accidentally attempts to use the wrong inline for some data. For example, if a ContactAdmin has both PhoneInline and EmailInline, the user may accidentally enter a phone number in an EmailInline record. When attempting to submit the changes, a validation error is raised. Seeing the problem, the user would now like to just delete the new record that is pending. However, at this point there is no Delete button. Thus the user is forced to either (a) reload the change page, discarding any other unrelated pending changes on the page; or (b) change the data to make it valid, click Save and continue editing, and then delete the offending record before finally saving the changes again.

Test code

Consider the following code, where the Django Admin provides access to a Groups page with a Persons inline.

models.py:

from django.db import models

class Group(models.Model):
    name = models.CharField(max_length=30)

class Person(models.Model):
    group = models.ForeignKey(Group, on_delete=models.CASCADE)
    first_name = models.CharField(max_length=30)
    last_name = models.CharField(max_length=30)

admin.py:

from django.contrib import admin
from .models import *

class PersonInline(admin.StackedInline):
    model = Person
    extra = 0

@admin.register(Group)
class GroupAdmin(admin.ModelAdmin):
    inlines = [PersonInline]

Steps to reproduce

  1. Go to the Group add page (for example, <http://localhost:8000/admin/myapp/group/add/>). Enter a Group name, add a Person with first_name and last_name, and click Save and continue editing.
  2. Change the Group name. Add a second Person for the group, but enter a first_name only. A delete button (X icon) is available to discard the pending record: https://s13.postimg.org/6wv1azzyv/image.png
  3. Click SAVE. There will be a validation error, as expected, because last_name is not provided. However, the Delete button is missing: https://s13.postimg.org/3q0hrduyf/image.png
  4. The user is now forced to either (a) fix the validation error or (b) reload the page, discarding all unrelated changes on the page. It is not possible for the user to fix the validation error by simply canceling/deleting the pending new record.

It is actually worse than that…

Another cause of validation errors can be uniqueness constraints. These errors can be difficult to fix and sometimes actually cannot be fixed. So it is totally possible that because of a validation error the user is forced to reload the page, there is no other choice.

Building on the models above, here is an example of a validation error that can be unsalvageable for users without the create permission for groups. New Council model is added, a foreign key from Person to Council is created and also a constraint with unique_together = ('council', 'group') is set.

https://i.ibb.co/K2ndRGT/Screenshot-2019-08-30-at-21-08-13.png

Change History (17)

comment:1 by Tim Graham, 7 years ago

Triage Stage: UnreviewedAccepted

comment:4 by frnhr, 5 years ago

Owner: set to frnhr
Status: newassigned

comment:5 by frnhr, 5 years ago

Has patch: set
Last edited 5 years ago by frnhr (previous) (diff)

comment:6 by frnhr, 5 years ago

Description: modified (diff)

comment:7 by frnhr, 5 years ago

Description: modified (diff)

comment:8 by Carlton Gibson, 5 years ago

Patch needs improvement: set

Fix should address the CSS TabularInline too. (This has the same issue.) Also, the PR includes a significant refactoring, which should be in a separate commit.
But other than that, looks goods. (Works 🙂)

Last edited 5 years ago by Carlton Gibson (previous) (diff)

comment:9 by frnhr, 5 years ago

Patch needs improvement: unset

PR edited

comment:10 by Carlton Gibson, 5 years ago

Triage Stage: AcceptedReady for checkin

PR is ready to go.

comment:11 by Carlton Gibson <carlton@…>, 5 years ago

In 6ea3aad:

Refs #29087 -- Refactored admin inlines.js.

Split logic into separate functions to clarify and allow reuse.

comment:12 by Carlton Gibson <carlton@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In 24e540f:

Fixed #29087 -- Added delete buttons for unsaved admin inlines on validation error.

comment:13 by Carlton Gibson <carlton@…>, 4 years ago

In f4272d00:

Fixed #32348, Refs #29087 -- Corrected tutorial for updated deleting inlines UI.

Updated tutorial to match change in 24e540fbd71bd2b0843e751bde61ad0052a811b3
allowing deletion of original extra inlines.

comment:14 by Carlton Gibson <carlton.gibson@…>, 4 years ago

In 4dbbe37:

[3.2.x] Fixed #32348, Refs #29087 -- Corrected tutorial for updated deleting inlines UI.

Updated tutorial to match change in 24e540fbd71bd2b0843e751bde61ad0052a811b3
allowing deletion of original extra inlines.

Backport of f4272d000af598018247fe9687dac0fd02a29a7c from master

comment:15 by Carlton Gibson <carlton.gibson@…>, 4 years ago

In fa203f17:

[3.1.x] Fixed #32348, Refs #29087 -- Corrected tutorial for updated deleting inlines UI.

Updated tutorial to match change in 24e540fbd71bd2b0843e751bde61ad0052a811b3
allowing deletion of original extra inlines.

Backport of f4272d000af598018247fe9687dac0fd02a29a7c from master

comment:16 by Backit, 4 years ago

Resolution: fixed
Status: closednew

This bug still exists in Django 2.22

comment:17 by Carlton Gibson, 4 years ago

Resolution: fixed
Status: newclosed

Please don't reopen closed tickets.

This fix was made as part of Django 3.1 (see 24e540fbd71bd2b0843e751bde61ad0052a811b3)
You'll need to update to that or beyond.

in reply to:  17 ; comment:18 by Backit, 4 years ago

Replying to Carlton Gibson:

Please don't reopen closed tickets.

This fix was made as part of Django 3.1 (see 24e540fbd71bd2b0843e751bde61ad0052a811b3)
You'll need to update to that or beyond.

But 2.2 is still supported, so should receive bugfixes, isn'it?

in reply to:  18 comment:19 by Mariusz Felisiak, 4 years ago

Replying to Backit:

But 2.2 is still supported, so should receive bugfixes, isn'it?

Django 2.2. is in extended support so it receives only security patches.

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