Re: [Django] #15910: show delete links for all admin inline formset rows
Django
-------------------------------------+-------------------------------------
Reporter: Arthur de | Owner: nobody
Jong <arthur@…> | Status: new
Type: New | Component: contrib.admin
feature | Severity: Normal
Milestone: | Keywords:
Version: 1.3 | Has patch: 1
Resolution: | Needs tests: 0
Triage Stage: Accepted | Easy pickings: 0
Needs documentation: 0 |
Patch needs improvement: 0 |
-------------------------------------+-------------------------------------
Changes (by Arthur de Jong <arthur@…>):
* has_patch: 0 => 1
Comment:
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
{{{#!js
$(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.
--
Ticket URL: <https://code.djangoproject.com/ticket/15910#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Arthur de | Owner: nobody
Jong <arthur@…> | Status: new
Type: New | Component: contrib.admin
feature | Severity: Normal
Milestone: | Keywords:
Version: 1.3 | Has patch: 1
Resolution: | Needs tests: 0
Triage Stage: Accepted | Easy pickings: 0
Needs documentation: 0 |
Patch needs improvement: 0 |
Changes (by Arthur de Jong <arthur@…>):
* cc: arthur@… (added)
Comment:
Hi, is there anything else I can do to get this into Django?
--
Ticket URL: <https://code.djangoproject.com/ticket/15910#comment:5>
Django
-------------------------------------+-------------------------------------
<arthur@…> | Status: new
Type: New feature | Version: 1.3
Component: contrib.admin | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 1
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/15910#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Arthur de Jong | Owner: nobody
<arthur@…> | Status: new
Type: New feature | Version: 1.3
Component: contrib.admin | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Needs tests: 1 | UI/UX: 1
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timo):
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
Current patch likely needs to be updated to apply clealy and also could
use some selenium tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/15910#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Arthur de Jong | Owner: nobody
<arthur@…> | Status: new
Type: New feature | Version: 1.3
Component: contrib.admin | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 1
Needs tests: 1 | UI/UX: 1
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/15910#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Arthur de Jong | Owner: nobody
<arthur@…> |
Component: contrib.admin | Version: 1.3
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by arthurdejong):
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/
--
Ticket URL: <https://code.djangoproject.com/ticket/15910#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Arthur de Jong | Owner: nobody
<arthur@…> |
Type: New feature | Status: new
Component: contrib.admin | Version: 1.3
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
Comment:
[https://github.com/django/django/pull/5594 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/15910#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Arthur de Jong | Owner: nobody
<arthur@…> |
Type: New feature | Status: new
Component: contrib.admin | Version: 1.3
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Changes (by timgraham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/15910#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Arthur de Jong | Owner: nobody
<arthur@…> |
Type: New feature | Status: new
Component: contrib.admin | Version: 1.3
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/15910#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: Arthur de Jong | Owner: nobody
<arthur@…> |
Type: New feature | Status: new
Component: contrib.admin | Version: 1.3
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/15910#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: Arthur de Jong | Owner: nobody
<arthur@…> |
Type: New feature | Status: new
Component: contrib.admin | Version: 1.3
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by djpretzel):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/15910#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: Arthur de Jong | Owner: nobody
<arthur@…> |
Component: contrib.admin | Version: 1.3
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* type: New feature => Bug
--
Ticket URL: <https://code.djangoproject.com/ticket/15910#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: Arthur de Jong | Owner: nobody
<arthur@…> |
Type: Bug | Status: new
Component: contrib.admin | Version: 1.3
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by frnhr):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/15910#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: Arthur de Jong | Owner: nobody
<arthur@…> |
Component: contrib.admin | Version: 1.3
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => duplicate
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/15910#comment:17>