Opened 8 months ago

Last modified 7 months ago

#27991 assigned Cleanup/optimization

Add 'obj' kwarg to InlineModelAdmin.has_add_permission()

Reported by: Olivier Owned by: Vladimir Ivanov
Component: contrib.admin Version: 1.10
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
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 (7)

comment:1 Changed 8 months 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 8 months 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 8 months ago by Vladimir Ivanov

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

comment:4 Changed 8 months 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 8 months ago by Vladimir Ivanov

Patch needs improvement: unset

comment:6 Changed 7 months ago by felixxm

Patch needs improvement: set

comment:7 Changed 7 months ago by Vladimir Ivanov

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

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