[Django] #27991: InlineModelAdmin's has_add_permission method doesn't accept instance argument
Django
argument
-----------------------------------------+------------------------
Reporter: Olivier | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.10
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
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
--
Ticket URL: <https://code.djangoproject.com/ticket/27991>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
--------------------------------------+------------------------------------
Reporter: Olivier | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* component: Uncategorized => contrib.admin
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:1>
Django
--------------------------------------+------------------------------------
Reporter: Olivier | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:2>
Django
Reporter: Olivier | Owner: Vladimir
Type: | Ivanov
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Vladimir Ivanov):
* owner: nobody => Vladimir Ivanov
* status: new => assigned
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/8304
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Olivier | Owner: Vladimir
Type: | Ivanov
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
Comment:
When updating, please also check your patch using the
PatchReviewChecklist, then uncheck "Patch needs improvement" on this
ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Olivier | Owner: Vladimir
Type: | Ivanov
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Vladimir Ivanov):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Olivier | Owner: Vladimir
Type: | Ivanov
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Olivier | Owner: Vladimir
Type: | Ivanov
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Vladimir Ivanov):
Is there anything I can improve else? Why is it in 'Patch needs
improvement'?
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Olivier | Owner: Vladimir
Type: | Ivanov
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Manel Clos):
* cc: Manel Clos (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Olivier | Owner: Vladimir
Type: | Ivanov
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Dufresne):
* needs_better_patch: 1 => 0
Comment:
[https://github.com/django/django/pull/9721 New PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Olivier | Owner: Vladimir
Type: | Ivanov
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Olivier | Owner: Vladimir
Type: | Ivanov
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Dufresne):
* needs_better_patch: 1 => 0
Comment:
Addressed feedback.
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Olivier | Owner: Vladimir
Type: | Ivanov
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: Olivier | Owner: Vladimir
Type: | Ivanov
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"be6ca89396c031619947921c81b8795d816e3285" be6ca893]:
{{{
#!CommitTicketReference repository=""
revision="be6ca89396c031619947921c81b8795d816e3285"
Fixed #27991 -- Added obj arg to InlineModelAdmin.has_add_permission().
Thanks Vladimir Ivanov for the initial patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: Olivier | Owner: Vladimir
Type: | Ivanov
Cleanup/optimization | Status: closed
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson <carlton.gibson@…>):
In [changeset:"fd8a7a5313f5e223212085b2e470e43c0047e066" fd8a7a5]:
{{{
#!CommitTicketReference repository=""
revision="fd8a7a5313f5e223212085b2e470e43c0047e066"
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: Olivier | Owner: Vladimir
Type: | Ivanov
Cleanup/optimization | Status: closed
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson <carlton.gibson@…>):
In [changeset:"152b1d783bfcf1bab8a038999e1f848a1b4384ef" 152b1d78]:
{{{
#!CommitTicketReference repository=""
revision="152b1d783bfcf1bab8a038999e1f848a1b4384ef"
[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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: Olivier | Owner: Vladimir
Type: | Ivanov
Cleanup/optimization | Status: closed
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"3c01fe30f3dd4dc1c8bb4fec816bd277d1ae5fa6" 3c01fe30]:
{{{
#!CommitTicketReference repository=""
revision="3c01fe30f3dd4dc1c8bb4fec816bd277d1ae5fa6"
Fixed #30097 -- Made 'obj' arg of InlineModelAdmin.has_add_permission()
optional.
Restored backwards compatibility after refs #27991.
Regression in be6ca89396c031619947921c81b8795d816e3285.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: Olivier | Owner: Vladimir
Type: | Ivanov
Cleanup/optimization | Status: closed
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"3df13847d583f0e439d03dd85e4d8d2a36e6de73" 3df13847]:
{{{
#!CommitTicketReference repository=""
revision="3df13847d583f0e439d03dd85e4d8d2a36e6de73"
[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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: Olivier | Owner: Vladimir
Type: | Ivanov
Cleanup/optimization | Status: closed
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"6079ed82f43d8cc4b2c07eb36bc14efa0a1a5c17" 6079ed82]:
{{{
#!CommitTicketReference repository=""
revision="6079ed82f43d8cc4b2c07eb36bc14efa0a1a5c17"
Refs #27991 -- Made obj a required argument of
InlineModelAdmin.has_add_permission().
Per deprecation timeline.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27991#comment:18>