Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#13521 closed (fixed)

admin inline max_num option doesn't work as intended

Reported by: Rolando Espinoza La fuente Owned by: nobody
Component: contrib.admin Version: master
Severity: Keywords: max_num inlines
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Hi, first of all sorry for my english :)

I just tested with inline example in django docs:
http://docs.djangoproject.com/en/dev/ref/contrib/admin/#inlinemodeladmin-objects
(using django's git mirror svn13230)

class BookInline(admin.TabularInline):
    model = Book
    extra = 1
    max_num = 10

class AuthorAdmin(admin.ModelAdmin):
    inlines = [
        BookInline,
    ]

But on admin interface, the "Add another book" link disappear on the first click, showing only 2 inline forms.

I believe that's because this line on inlines.js script:

if ((maxForms.val() != '') && (maxForms.val() <= totalForms.val())) {

val() function returns string, resulting "10" <= "2" which is True in javascript.

I've attached a little patch that uses same approach as showAddButton logic.

PD: If you apply the patch for test purpose, don't forget to overwrite/update inlines.min.js!

Attachments (1)

django1.2_inlines_js_max_num_comparison.patch (995 bytes) - added by Rolando Espinoza La fuente 6 years ago.

Download all attachments as: .zip

Change History (7)

Changed 6 years ago by Rolando Espinoza La fuente

comment:1 Changed 6 years ago by Rolando Espinoza La fuente

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

Here is a ready to run project to reproduce the bug: http://github.com/darkrho/django_inline_example

comment:2 Changed 6 years ago by Gabriel Hurley

I haven't verified the bug myself (about to walk out the door), but looking at the patch, wouldn't it be better to do a parseInt() call around maxForms.val() and totalForms.val()? That would make the type conversion explicit rather than relying on Javascript's (masochistically) dynamic typing for the subtraction operator as in the patch.

comment:3 in reply to:  2 Changed 6 years ago by Rolando Espinoza La fuente

Replying to gabrielhurley:

I haven't verified the bug myself (about to walk out the door), but looking at the patch, wouldn't it be better to do a parseInt() call around maxForms.val() and totalForms.val()? That would make the type conversion explicit rather than relying on Javascript's (masochistically) dynamic typing for the subtraction operator as in the patch.

I was using parseInt() at first time, but there a few more places there the comparison is done using substraction. Also that lines should be changed to use parseInt(). But parseInt() is slower, that's why I choose substract operation as is already used there.

comment:4 Changed 6 years ago by Russell Keith-Magee

Triage Stage: UnreviewedAccepted

comment:5 Changed 6 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

(In [13234]) Fixed #13521 -- Corrected javascript comparisons determining when to show and hide the 'add another' inline button. Thanks to darkrho for the report and patch.

comment:6 Changed 5 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

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