Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#20642 closed Cleanup/optimization (fixed)

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

Reported by: Simon Charette Owned by: nobody
Component: Database layer (models, ORM) Version: dev
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 Simon Charette 11 years ago.
Patch with doc
20642-tests.diff (3.9 KB ) - added by Tim Graham 11 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 by Simon Charette, 12 years ago

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 by loic84, 11 years ago

Triage Stage: UnreviewedReady 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 11 years ago by loic84 (previous) (diff)

by Simon Charette, 11 years ago

Patch with doc

comment:3 by Simon Charette, 11 years ago

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

comment:4 by Simon Charette <charette.s@…>, 11 years ago

Resolution: fixed
Status: newclosed

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 by Simon Charette <charette.s@…>, 11 years ago

In e1dd24d2f6dd3464ab50593320a7eb2325d6c196:

Added missing deprecation note for model permission methods.

refs #20642.

comment:6 by Simon Charette <charette.s@…>, 11 years ago

In e628753e7dd1611fb4cf770a78126e96a02ab1d7:

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

refs #20642.

Backport of e1dd24d2f from master.

by Tim Graham, 11 years ago

Attachment: 20642-tests.diff added

comment:7 by Tim Graham <timograham@…>, 11 years ago

In a6a905c619e48bb3db4a5fbb09e5e03abb7ed0f6:

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

refs #20642.

comment:8 by Tim Graham <timograham@…>, 11 years ago

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