Opened 2 years ago

Closed 22 months 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 23 months ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 2 years ago by charettes

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Actually I think we should add a get_extra method instead.

comment:2 Changed 2 years ago by crodjer

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

comment:3 Changed 2 years ago by charettes

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

  • Triage Stage changed from Unreviewed to Accepted

comment:5 Changed 2 years ago by slurms

  • Needs documentation set

comment:6 follow-up: Changed 2 years ago by anonymous

Added documentation in the corresponding pull request.

comment:7 in reply to: ↑ 6 Changed 2 years ago by crodjer

Replying to anonymous:

Added documentation in the corresponding pull request.

This comment was by me.

comment:8 Changed 2 years ago by charettes

Reviewed PR.

comment:9 Changed 2 years ago by crodjer

Incorporated the suggested changes in PR

comment:10 Changed 2 years ago by anonymous

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 Changed 2 years ago by woodm

  • Triage Stage changed from Accepted to Ready for checkin

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

comment:12 Changed 23 months ago by areski

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

Changed 23 months ago by areski

comment:13 Changed 23 months ago by areski

Related also to ticket #18388

comment:14 Changed 23 months ago by crodjer

  • Cc crodjer@… added

Updated the pull request with the suggestions in discussion.

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

comment:15 Changed 23 months ago by areski

  • Cc areski@… added

comment:16 Changed 22 months ago by Tim Graham <timograham@…>

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

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