Opened 21 months ago

Closed 21 months ago

Last modified 21 months ago

#20642 closed Cleanup/optimization (fixed)

Deprecate `Option.get_(add|change|delete)_permission`.

Reported by: charettes Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: 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

In the whole Clean Model Options process we should deprecate those methods in favor of contrib.auth alternatives.

Even if those methods are undocumented it's no secret they are used in the wild. Django exclusively use them in contrib.admin:

(django)simon@simon-laptop:~/workspace/django$ grep -E "get_(add|change|delete)_permission" django/ -Rn --include="*.py"
django/db/models/options.py:416:    def get_add_permission(self):
django/db/models/options.py:419:    def get_change_permission(self):
django/db/models/options.py:422:    def get_delete_permission(self):
django/contrib/admin/options.py:355:        return request.user.has_perm(opts.app_label + '.' + opts.get_add_permission())
django/contrib/admin/options.py:369:        return request.user.has_perm(opts.app_label + '.' + opts.get_change_permission())
django/contrib/admin/options.py:383:        return request.user.has_perm(opts.app_label + '.' + opts.get_delete_permission())
django/contrib/admin/options.py:1671:            self.opts.app_label + '.' + self.opts.get_add_permission())
django/contrib/admin/options.py:1683:            opts.app_label + '.' + opts.get_change_permission())
django/contrib/admin/options.py:1693:            self.opts.app_label + '.' + self.opts.get_delete_permission())
django/contrib/admin/util.py:122:                           opts.get_delete_permission())

Attachments (2)

0001-Fixed-20642-Deprecated-Option.get_-add-change-delete.patch (18.5 KB) - added by charettes 21 months ago.
Patch with doc
20642-tests.diff (3.9 KB) - added by timo 21 months ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 21 months ago by charettes

  • Has patch set

Added a patch that deprecates option permission methods and moves contrib.auth.management._get_permission_codename to contrib.auth.get_permission_codename and use it in contrib.admin.

Wondering if we should document get_permission_codename, it feels a bit odd to deprecate those methods while specifying no alternatives in the release notes.

I still pointed to it in the raised deprecation warning.

comment:2 Changed 21 months ago by loic84

  • Triage Stage changed from Unreviewed to Ready for checkin

+1 much needed cleanup.

Tested it on a project that uses permissions and ModelAdmin.has_x_permission overrides extensively and didn't notice any issue.

I'd go either ways for documenting get_permission_codename, the methods it replaces weren't documented, so people that used them know where to look.

Last edited 21 months ago by loic84 (previous) (diff)

Changed 21 months ago by charettes

Patch with doc

comment:3 Changed 21 months ago by charettes

Updated the RFC patch to use super in InlineAdmin instead of duplicating permission codename creation.

comment:4 Changed 21 months ago by Simon Charette <charette.s@…>

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

In b91787910c9d5a036674d46a73d1b48ca33123a3:

Fixed #20642 -- Deprecated Option.get_(add|change|delete)_permission.

Those methods were only used by contrib.admin internally and exclusively
related to contrib.auth. Since they were undocumented but used
in the wild the raised deprecation warning point to an also undocumented
alternative that lives in contrib.auth.

Also did some PEP8 and other cleanups in the affected modules.

comment:5 Changed 21 months ago by Simon Charette <charette.s@…>

In e1dd24d2f6dd3464ab50593320a7eb2325d6c196:

Added missing deprecation note for model permission methods.

refs #20642.

comment:6 Changed 21 months ago by Simon Charette <charette.s@…>

In e628753e7dd1611fb4cf770a78126e96a02ab1d7:

[1.6.x] Added missing deprecation note for model permission methods.

refs #20642.

Backport of e1dd24d2f from master.

Changed 21 months ago by timo

comment:7 Changed 21 months ago by Tim Graham <timograham@…>

In a6a905c619e48bb3db4a5fbb09e5e03abb7ed0f6:

Updated tests for deprecation of Option.get_(add|change|delete)_permission.

refs #20642.

comment:8 Changed 21 months ago by Tim Graham <timograham@…>

In 3c51962cabc9537221b86c667aac5ffaa1469660:

[1.6.x] Updated tests for deprecation of Option.get_(add|change|delete)_permission.

refs #20642.

Backport of a6a905c619 from master.

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