Opened 14 years ago

Closed 14 years ago

Last modified 12 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: dev
Severity: Keywords: max_num inlines
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

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 14 years ago.

Download all attachments as: .zip

Change History (7)

by Rolando Espinoza La fuente, 14 years ago

comment:1 by Rolando Espinoza La fuente, 14 years ago

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

comment:2 by Gabriel Hurley, 14 years ago

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.

in reply to:  2 comment:3 by Rolando Espinoza La fuente, 14 years ago

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 by Russell Keith-Magee, 14 years ago

Triage Stage: UnreviewedAccepted

comment:5 by Russell Keith-Magee, 14 years ago

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 by Jacob, 12 years ago

milestone: 1.2

Milestone 1.2 deleted

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