Opened 8 months ago

Closed 8 months ago

#27942 closed Bug (fixed)

admin's inlines.js indiscriminately adds "add" buttons to all tables in inlines

Reported by: James Pulec Owned by: Adonys Alea Boffill
Component: contrib.admin Version: master
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

It seems that there are issues using tabular inlines, if you're using a custom field that is rendered with a table.

For example, if you have some custom field like https://github.com/django-recurrence/django-recurrence uses and try to use it in a tabular inline, the result will be that there is an extra add button appended inside the custom field widget.

This seems to be caused by https://github.com/django/django/blob/master/django/contrib/admin/static/admin/js/inlines.js#L23 since it tries to fetch the parent by using the selector defined on by this, which simply selects ANY child table rows of the current inline form. It's this selector that is then later used to determine where to append the add buttons, and if $parent selector has multiple results, add buttons get appended to all of them.

Change History (7)

comment:1 Changed 8 months ago by Tim Graham

Summary: Admin Issue with Tables in Custom Field in Tabular Inlinesadmin's inlines.js indiscriminately adds "add" buttons to all tables in inlines
Triage Stage: UnreviewedAccepted

I haven't tried to reproduce this but the idea seems reasonable.

comment:2 Changed 8 months ago by Adonys Alea Boffill

Owner: changed from nobody to Adonys Alea Boffill
Status: newassigned

I reached to reproduce the issue, I'm working to propose a solution.

comment:3 Changed 8 months ago by Adonys Alea Boffill

I have a solution modifying the jQuery selector we can change:

$(inlineOptions.name + "-group .tabular.inline-related tbody tr")

by

$(inlineOptions.name + "-group .tabular.inline-related tbody:first > tr")

That solution works good, all test are passed.

But the interesting thing is... what is the best option to build tests to satisfy this fixed??? This is an scenario difficult to reproduce.

  • JavaScript tests
  • Selenium tests
  • Both

To build selenium tests I think that is necessary to implement some test widget in order to generate the HTML content necessary to reproduce the specific scenario.

some other idea before start to try to build the tests??

comment:4 Changed 8 months ago by Tim Graham

I think it'd be acceptable to merge this fix without a test.

comment:5 Changed 8 months ago by Adonys Alea Boffill

Thanks Tim!, I added the PR https://github.com/django/django/pull/8256

comment:6 Changed 8 months ago by Tim Graham

Has patch: set
Triage Stage: AcceptedReady for checkin

comment:7 Changed 8 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In a0d29a9:

Fixed #27942 -- Prevented admin from adding "add" buttons to all tables in inlines.

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