Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#13521 closed (fixed)

admin inline max_num option doesn't work as intended

Reported by: darkrho 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 darkrho 5 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 5 years ago by darkrho

  • 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 follow-up: Changed 5 years ago by 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.

comment:3 in reply to: ↑ 2 Changed 5 years ago by darkrho

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 5 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

comment:5 Changed 5 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

(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 4 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

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