Opened 3 years ago

Closed 2 years ago

Last modified 19 months ago

#27991 closed Cleanup/optimization (fixed)

Add 'obj' kwarg to InlineModelAdmin.has_add_permission()

Reported by: Olivier Dalang Owned by: Vladimir Ivanov
Component: contrib.admin Version: 1.10
Severity: Normal Keywords:
Cc: Manel Clos Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi !

InlineModelAdmin has the following methods :

def has_add_permission(self, request):
    ...
def has_change_permission(self, request, obj=None):
    ...
def has_delete_permission(self, request, obj=None):
    ...

In those methods, the obj parameter represents the parent instance. So, by overriding ModelInline.has_change_permission you can have some specific logic to define whether an user can change inlines objects depending on the parent object.

The thing is that the has_add_permission misses this argument. It probably came from a confusion between the parent instance (the obj argument) and the child instance (not accessible).

Still, exactly as it makes sense overriding ModelInline.has_change_permission, we may need to have some specific logic to define whether an user can add inlines objects depending on the parent object.

I suggest we add the obj argument to ModelInline.has_add_permission, and make it more clear (both in the doc and as comments in the code) that obj refers to the parent instance, and not the inline instance.

It would be awesome if those methods also took the inline instance as a parameter (eg. def has_change_permission(self, request, obj=None, childObj=None): to keep it backwards compatible ) but that would probably be for another ticket.

Thanks !

Olivier

Change History (18)

comment:1 Changed 3 years ago by Tim Graham

Component: Uncategorizedcontrib.admin
Summary: InlineModelAdmin's has_add_permission method doesn't accept instance argumentAdd 'obj' kwarg to InlineModelAdmin's.has_add_permission()
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

Seems okay at first glance. For backwards compatibility, each has_add_permission()call that starts passing obj needs to check if the has_add_permission() method accepts an obj parameter and give a deprecation warning if not; see a7c256cb5491bf2a77abdff01638239db5bfd9d5 for a similar deprecation.

comment:2 Changed 3 years ago by Tim Graham

Summary: Add 'obj' kwarg to InlineModelAdmin's.has_add_permission()Add 'obj' kwarg to InlineModelAdmin.has_add_permission()

comment:3 Changed 3 years ago by Vladimir Ivanov

Has patch: set
Owner: changed from nobody to Vladimir Ivanov
Status: newassigned

comment:4 Changed 3 years ago by Tim Graham

Patch needs improvement: set

When updating, please also check your patch using the PatchReviewChecklist, then uncheck "Patch needs improvement" on this ticket.

comment:5 Changed 3 years ago by Vladimir Ivanov

Patch needs improvement: unset

comment:6 Changed 3 years ago by felixxm

Patch needs improvement: set

comment:7 Changed 3 years ago by Vladimir Ivanov

Is there anything I can improve else? Why is it in 'Patch needs improvement'?

comment:8 Changed 3 years ago by Manel Clos

Cc: Manel Clos added

comment:9 Changed 2 years ago by Jon Dufresne

Patch needs improvement: unset

comment:10 Changed 2 years ago by Carlton Gibson

Patch needs improvement: set

comment:11 Changed 2 years ago by Jon Dufresne

Patch needs improvement: unset

Addressed feedback.

comment:12 Changed 2 years ago by Carlton Gibson

Triage Stage: AcceptedReady for checkin

comment:13 Changed 2 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In be6ca893:

Fixed #27991 -- Added obj arg to InlineModelAdmin.has_add_permission().

Thanks Vladimir Ivanov for the initial patch.

comment:14 Changed 2 years ago by Carlton Gibson <carlton.gibson@…>

In fd8a7a5:

Fixed #29723 -- Fixed crash if InlineModelAdmin.has_add_permission() doesn't accept the obj argument.

  • Refs #27991 -- Added testing for ModelAdmin.get_inline_instances() if the inline's has_add_permission() doesn't accept 'obj'.
  • Fixed #29723 -- Fixed crash if InlineModelAdmin.has_add_permission() doesn't accept the obj argument.

comment:15 Changed 2 years ago by Carlton Gibson <carlton.gibson@…>

In 152b1d78:

[2.1.x] Fixed #29723 -- Fixed crash if InlineModelAdmin.has_add_permission() doesn't accept the obj argument.

  • Refs #27991 -- Added testing for ModelAdmin.get_inline_instances() if the inline's has_add_permission() doesn't accept 'obj'.
  • Fixed #29723 -- Fixed crash if InlineModelAdmin.has_add_permission() doesn't accept the obj argument.

Backport of fd8a7a5313f5e223212085b2e470e43c0047e066 from master

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

In 3c01fe30:

Fixed #30097 -- Made 'obj' arg of InlineModelAdmin.has_add_permission() optional.

Restored backwards compatibility after refs #27991.
Regression in be6ca89396c031619947921c81b8795d816e3285.

comment:17 Changed 19 months ago by Tim Graham <timograham@…>

In 3df13847:

[2.1.x] Fixed #30097 -- Made 'obj' arg of InlineModelAdmin.has_add_permission() optional.

Restored backwards compatibility after refs #27991.
Regression in be6ca89396c031619947921c81b8795d816e3285.

Backport of 3c01fe30f3dd4dc1c8bb4fec816bd277d1ae5fa6 from master.

comment:18 Changed 19 months ago by Tim Graham <timograham@…>

In 6079ed82:

Refs #27991 -- Made obj a required argument of InlineModelAdmin.has_add_permission().

Per deprecation timeline.

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