Opened 11 years ago

Closed 11 years ago

#19425 closed Bug (fixed)

Inline Admin does not allow extra to be a property

Reported by: dave@… Owned by: nobody
Component: contrib.admin Version: 1.4
Severity: Normal Keywords: inline dynamic extras
Cc: crodjer@…, areski@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

this line

if not isinstance( cls.extra, int ):

in

django.contrib.admin.validation

causes this to fail

class MyInline( admin.StackedInline ):
    @property
    def extra( self ):
        if condition:
            return 3
        else:
            self.max_num = 0
            return 0

and should be updated to this accordingly

if not isinstance( cls.extra, int ) and not isinstance( cls.extra, property ):

Attachments (1)

patch_ticket_19425_b.txt (4.0 KB ) - added by Areski Belaid 11 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 by Simon Charette, 11 years ago

Actually I think we should add a get_extra method instead.

comment:2 by Rohan Jain, 11 years ago

Quickly compiled a patch for it, taking charettes' suggestion in consideration: https://github.com/django/django/pull/576

comment:3 by Simon Charette, 11 years ago

The pull request is looking quite good.

This would also need documentation and a 1.6 release note but before going forward with this I think we should wait for this feature to be accepted by a core developer.

comment:4 by Russell Keith-Magee, 11 years ago

Triage Stage: UnreviewedAccepted

comment:5 by Nick Sandford, 11 years ago

Needs documentation: set

comment:6 by anonymous, 11 years ago

Added documentation in the corresponding pull request.

in reply to:  6 comment:7 by Rohan Jain, 11 years ago

Replying to anonymous:

Added documentation in the corresponding pull request.

This comment was by me.

comment:8 by Simon Charette, 11 years ago

Reviewed PR.

comment:9 by Rohan Jain, 11 years ago

Incorporated the suggested changes in PR

comment:10 by anonymous, 11 years ago

I've checked the PR and seems good to me. Documentations is clear enough and complete.

Also I checked all tests are passing, so I think this is ready for checking.

comment:11 by woodm, 11 years ago

Triage Stage: AcceptedReady for checkin

I also checked the PR and it seems good to me as well.

comment:12 by Areski Belaid, 11 years ago

Great patch, I tested it during the sprint djangocon eu and it worked for me.
when I rebase it I add a small conflict at the import django.core.exceptions. https://github.com/areski/django/commit/f97139866517d9fe2731f76e27d80bf7ea3aa93d#L0R16

by Areski Belaid, 11 years ago

Attachment: patch_ticket_19425_b.txt added

comment:13 by Areski Belaid, 11 years ago

Related also to ticket #18388

comment:14 by Rohan Jain, 11 years ago

Cc: crodjer@… added

Updated the pull request with the suggestions in discussion.

Re-referencing: https://github.com/django/django/pull/576

comment:15 by Areski Belaid, 11 years ago

Cc: areski@… added

comment:16 by Tim Graham <timograham@…>, 11 years ago

Resolution: fixed
Status: newclosed

In 36aecb12b850aeed173a8e524cacb3444f807785:

Fixed #19425 - Added InlineModelAdmin.get_extra hook.

Thanks dave@ for the suggestion and Rohan Jain for the patch.

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