From b91787910c9d5a036674d46a73d1b48ca33123a3 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Sat, 22 Jun 2013 21:48:09 -0400 Subject: [PATCH] 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. --- django/contrib/admin/options.py | 100 +++++++++++---------- django/contrib/auth/__init__.py | 15 +++- django/contrib/auth/management/__init__.py | 12 +-- django/db/models/options.py | 24 +++++ docs/releases/1.6.txt | 7 ++ 5 files changed, 106 insertions(+), 52 deletions(-) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index ce10cf72ba..fd516cb512 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -4,18 +4,15 @@ from functools import partial, reduce, update_wrapper from django import forms from django.conf import settings -from django.forms.formsets import all_valid, DELETION_FIELD_NAME -from django.forms.models import (modelform_factory, modelformset_factory, - inlineformset_factory, BaseInlineFormSet, modelform_defines_fields) -from django.contrib.contenttypes.models import ContentType +from django.contrib import messages from django.contrib.admin import widgets, helpers from django.contrib.admin.util import (unquote, flatten_fieldsets, get_deleted_objects, model_format_dict, NestedObjects, lookup_needs_distinct) from django.contrib.admin import validation from django.contrib.admin.templatetags.admin_static import static from django.contrib.admin.templatetags.admin_urls import add_preserved_filters -from django.contrib import messages -from django.views.decorators.csrf import csrf_protect +from django.contrib.auth import get_permission_codename +from django.contrib.contenttypes.models import ContentType from django.core.exceptions import PermissionDenied, ValidationError, FieldError from django.core.paginator import Paginator from django.core.urlresolvers import reverse @@ -24,6 +21,9 @@ from django.db.models.constants import LOOKUP_SEP from django.db.models.related import RelatedObject from django.db.models.fields import BLANK_CHOICE_DASH, FieldDoesNotExist from django.db.models.sql.constants import QUERY_TERMS +from django.forms.formsets import all_valid, DELETION_FIELD_NAME +from django.forms.models import (modelform_factory, modelformset_factory, + inlineformset_factory, BaseInlineFormSet, modelform_defines_fields) from django.http import Http404, HttpResponseRedirect from django.http.response import HttpResponseBase from django.shortcuts import get_object_or_404 @@ -39,6 +39,8 @@ from django.utils.text import capfirst, get_text_list from django.utils.translation import ugettext as _ from django.utils.translation import ungettext from django.utils.encoding import force_text +from django.views.decorators.csrf import csrf_protect + IS_POPUP_VAR = '_popup' @@ -58,15 +60,15 @@ FORMFIELD_FOR_DBFIELD_DEFAULTS = { 'form_class': forms.SplitDateTimeField, 'widget': widgets.AdminSplitDateTime }, - models.DateField: {'widget': widgets.AdminDateWidget}, - models.TimeField: {'widget': widgets.AdminTimeWidget}, - models.TextField: {'widget': widgets.AdminTextareaWidget}, - models.URLField: {'widget': widgets.AdminURLFieldWidget}, - models.IntegerField: {'widget': widgets.AdminIntegerFieldWidget}, + models.DateField: {'widget': widgets.AdminDateWidget}, + models.TimeField: {'widget': widgets.AdminTimeWidget}, + models.TextField: {'widget': widgets.AdminTextareaWidget}, + models.URLField: {'widget': widgets.AdminURLFieldWidget}, + models.IntegerField: {'widget': widgets.AdminIntegerFieldWidget}, models.BigIntegerField: {'widget': widgets.AdminBigIntegerFieldWidget}, - models.CharField: {'widget': widgets.AdminTextInputWidget}, - models.ImageField: {'widget': widgets.AdminFileWidget}, - models.FileField: {'widget': widgets.AdminFileWidget}, + models.CharField: {'widget': widgets.AdminTextInputWidget}, + models.ImageField: {'widget': widgets.AdminFileWidget}, + models.FileField: {'widget': widgets.AdminFileWidget}, } csrf_protect_m = method_decorator(csrf_protect) @@ -352,7 +354,8 @@ class BaseModelAdmin(six.with_metaclass(RenameBaseModelAdminMethods)): Can be overridden by the user in subclasses. """ opts = self.opts - return request.user.has_perm(opts.app_label + '.' + opts.get_add_permission()) + codename = get_permission_codename('add', opts) + return request.user.has_perm("%s.%s" % (opts.app_label, codename)) def has_change_permission(self, request, obj=None): """ @@ -366,7 +369,8 @@ class BaseModelAdmin(six.with_metaclass(RenameBaseModelAdminMethods)): request has permission to change *any* object of the given type. """ opts = self.opts - return request.user.has_perm(opts.app_label + '.' + opts.get_change_permission()) + codename = get_permission_codename('change', opts) + return request.user.has_perm("%s.%s" % (opts.app_label, codename)) def has_delete_permission(self, request, obj=None): """ @@ -380,7 +384,9 @@ class BaseModelAdmin(six.with_metaclass(RenameBaseModelAdminMethods)): request has permission to delete *any* object of the given type. """ opts = self.opts - return request.user.has_perm(opts.app_label + '.' + opts.get_delete_permission()) + codename = get_permission_codename('delete', opts) + return request.user.has_perm("%s.%s" % (opts.app_label, codename)) + class ModelAdmin(BaseModelAdmin): "Encapsulates all admin options and functionality for a given model." @@ -608,11 +614,11 @@ class ModelAdmin(BaseModelAdmin): """ from django.contrib.admin.models import LogEntry, ADDITION LogEntry.objects.log_action( - user_id = request.user.pk, - content_type_id = ContentType.objects.get_for_model(object).pk, - object_id = object.pk, - object_repr = force_text(object), - action_flag = ADDITION + user_id=request.user.pk, + content_type_id=ContentType.objects.get_for_model(object).pk, + object_id=object.pk, + object_repr=force_text(object), + action_flag=ADDITION ) def log_change(self, request, object, message): @@ -623,12 +629,12 @@ class ModelAdmin(BaseModelAdmin): """ from django.contrib.admin.models import LogEntry, CHANGE LogEntry.objects.log_action( - user_id = request.user.pk, - content_type_id = ContentType.objects.get_for_model(object).pk, - object_id = object.pk, - object_repr = force_text(object), - action_flag = CHANGE, - change_message = message + user_id=request.user.pk, + content_type_id=ContentType.objects.get_for_model(object).pk, + object_id=object.pk, + object_repr=force_text(object), + action_flag=CHANGE, + change_message=message ) def log_deletion(self, request, object, object_repr): @@ -640,11 +646,11 @@ class ModelAdmin(BaseModelAdmin): """ from django.contrib.admin.models import LogEntry, DELETION LogEntry.objects.log_action( - user_id = request.user.pk, - content_type_id = ContentType.objects.get_for_model(self.model).pk, - object_id = object.pk, - object_repr = object_repr, - action_flag = DELETION + user_id=request.user.pk, + content_type_id=ContentType.objects.get_for_model(self.model).pk, + object_id=object.pk, + object_repr=object_repr, + action_flag=DELETION ) def action_checkbox(self, obj): @@ -880,7 +886,7 @@ class ModelAdmin(BaseModelAdmin): 'has_add_permission': self.has_add_permission(request), 'has_change_permission': self.has_change_permission(request, obj), 'has_delete_permission': self.has_delete_permission(request, obj), - 'has_file_field': True, # FIXME - this should check if form or formsets have a FileField, + 'has_file_field': True, # FIXME - this should check if form or formsets have a FileField, 'has_absolute_url': hasattr(self.model, 'get_absolute_url'), 'form_url': form_url, 'opts': opts, @@ -1050,7 +1056,7 @@ class ModelAdmin(BaseModelAdmin): if action_form.is_valid(): action = action_form.cleaned_data['action'] select_across = action_form.cleaned_data['select_across'] - func, name, description = self.get_actions(request)[action] + func = self.get_actions(request)[action][0] # Get the list of selected PKs. If nothing's selected, we can't # perform an action on it, so bail. Except we want to perform @@ -1281,7 +1287,7 @@ class ModelAdmin(BaseModelAdmin): actions = self.get_actions(request) if actions: # Add the action checkboxes if there are any actions available. - list_display = ['action_checkbox'] + list(list_display) + list_display = ['action_checkbox'] + list(list_display) ChangeList = self.get_changelist(request) try: @@ -1430,7 +1436,10 @@ class ModelAdmin(BaseModelAdmin): raise PermissionDenied if obj is None: - raise Http404(_('%(name)s object with primary key %(key)r does not exist.') % {'name': force_text(opts.verbose_name), 'key': escape(object_id)}) + raise Http404( + _('%(name)s object with primary key %(key)r does not exist.') % + {'name': force_text(opts.verbose_name), 'key': escape(object_id)} + ) using = router.db_for_write(self.model) @@ -1439,7 +1448,7 @@ class ModelAdmin(BaseModelAdmin): (deleted_objects, perms_needed, protected) = get_deleted_objects( [obj], opts, request.user, self.admin_site, using) - if request.POST: # The user has already confirmed the deletion. + if request.POST: # The user has already confirmed the deletion. if perms_needed: raise PermissionDenied obj_display = force_text(obj) @@ -1457,7 +1466,9 @@ class ModelAdmin(BaseModelAdmin): (opts.app_label, opts.model_name), current_app=self.admin_site.name) preserved_filters = self.get_preserved_filters(request) - post_url = add_preserved_filters({'preserved_filters': preserved_filters, 'opts': opts}, post_url) + post_url = add_preserved_filters( + {'preserved_filters': preserved_filters, 'opts': opts}, post_url + ) else: post_url = reverse('admin:index', current_app=self.admin_site.name) @@ -1523,6 +1534,7 @@ class ModelAdmin(BaseModelAdmin): "admin/object_history.html" ], context, current_app=self.admin_site.name) + class InlineModelAdmin(BaseModelAdmin): """ Options for inline editing of ``model`` instances. @@ -1666,8 +1678,7 @@ class InlineModelAdmin(BaseModelAdmin): # to have the change permission for the related model in order to # be able to do anything with the intermediate model. return self.has_change_permission(request) - return request.user.has_perm( - self.opts.app_label + '.' + self.opts.get_add_permission()) + return super(InlineModelAdmin, self).has_add_permission(request) def has_change_permission(self, request, obj=None): opts = self.opts @@ -1678,8 +1689,8 @@ class InlineModelAdmin(BaseModelAdmin): if field.rel and field.rel.to != self.parent_model: opts = field.rel.to._meta break - return request.user.has_perm( - opts.app_label + '.' + opts.get_change_permission()) + codename = get_permission_codename('change', opts) + return request.user.has_perm("%s.%s" % (opts.app_label, codename)) def has_delete_permission(self, request, obj=None): if self.opts.auto_created: @@ -1688,8 +1699,7 @@ class InlineModelAdmin(BaseModelAdmin): # to have the change permission for the related model in order to # be able to do anything with the intermediate model. return self.has_change_permission(request, obj) - return request.user.has_perm( - self.opts.app_label + '.' + self.opts.get_delete_permission()) + return super(InlineModelAdmin, self).has_delete_permission(request, obj) class StackedInline(InlineModelAdmin): diff --git a/django/contrib/auth/__init__.py b/django/contrib/auth/__init__.py index 029193d582..2f620a34fe 100644 --- a/django/contrib/auth/__init__.py +++ b/django/contrib/auth/__init__.py @@ -108,7 +108,9 @@ def logout(request): def get_user_model(): - "Return the User model that is active in this project" + """ + Returns the User model that is active in this project. + """ from django.db.models import get_model try: @@ -122,6 +124,10 @@ def get_user_model(): def get_user(request): + """ + Returns the user model instance associated with the given request session. + If no user is retrieved an instance of `AnonymousUser` is returned. + """ from .models import AnonymousUser try: user_id = request.session[SESSION_KEY] @@ -132,3 +138,10 @@ def get_user(request): except (KeyError, AssertionError): user = AnonymousUser() return user + + +def get_permission_codename(action, opts): + """ + Returns the codename of the permission for the specified action. + """ + return '%s_%s' % (action, opts.model_name) diff --git a/django/contrib/auth/management/__init__.py b/django/contrib/auth/management/__init__.py index 5c1bfbc515..1f338469f8 100644 --- a/django/contrib/auth/management/__init__.py +++ b/django/contrib/auth/management/__init__.py @@ -6,7 +6,8 @@ from __future__ import unicode_literals import getpass import unicodedata -from django.contrib.auth import models as auth_app, get_user_model +from django.contrib.auth import (models as auth_app, get_permission_codename, + get_user_model) from django.core import exceptions from django.core.management.base import CommandError from django.db import DEFAULT_DB_ALIAS, router @@ -16,10 +17,6 @@ from django.utils import six from django.utils.six.moves import input -def _get_permission_codename(action, opts): - return '%s_%s' % (action, opts.model_name) - - def _get_all_permissions(opts, ctype): """ Returns (codename, name) for all permissions in the given opts. @@ -29,16 +26,18 @@ def _get_all_permissions(opts, ctype): _check_permission_clashing(custom, builtin, ctype) return builtin + custom + def _get_builtin_permissions(opts): """ Returns (codename, name) for all autogenerated permissions. """ perms = [] for action in ('add', 'change', 'delete'): - perms.append((_get_permission_codename(action, opts), + perms.append((get_permission_codename(action, opts), 'Can %s %s' % (action, opts.verbose_name_raw))) return perms + def _check_permission_clashing(custom, builtin, ctype): """ Check that permissions for a model do not clash. Raises CommandError if @@ -58,6 +57,7 @@ def _check_permission_clashing(custom, builtin, ctype): (codename, ctype.app_label, ctype.model_class().__name__)) pool.add(codename) + def create_permissions(app, created_models, verbosity, db=DEFAULT_DB_ALIAS, **kwargs): try: get_model('auth', 'Permission') diff --git a/django/db/models/options.py b/django/db/models/options.py index b8a79023e8..ad25de4a3e 100644 --- a/django/db/models/options.py +++ b/django/db/models/options.py @@ -414,12 +414,36 @@ class Options(object): return cache def get_add_permission(self): + """ + This method has been deprecated in favor of + `django.contrib.auth.get_permission_codename`. refs #20642 + """ + warnings.warn( + "`Options.get_add_permission` has been deprecated in favor " + "of `django.contrib.auth.get_permission_codename`.", + PendingDeprecationWarning, stacklevel=2) return 'add_%s' % self.model_name def get_change_permission(self): + """ + This method has been deprecated in favor of + `django.contrib.auth.get_permission_codename`. refs #20642 + """ + warnings.warn( + "`Options.get_change_permission` has been deprecated in favor " + "of `django.contrib.auth.get_permission_codename`.", + PendingDeprecationWarning, stacklevel=2) return 'change_%s' % self.model_name def get_delete_permission(self): + """ + This method has been deprecated in favor of + `django.contrib.auth.get_permission_codename`. refs #20642 + """ + warnings.warn( + "`Options.get_delete_permission` has been deprecated in favor " + "of `django.contrib.auth.get_permission_codename`.", + PendingDeprecationWarning, stacklevel=2) return 'delete_%s' % self.model_name def get_all_related_objects(self, local_only=False, include_hidden=False, diff --git a/docs/releases/1.6.txt b/docs/releases/1.6.txt index 5d1a37ac53..3d59ce771b 100644 --- a/docs/releases/1.6.txt +++ b/docs/releases/1.6.txt @@ -855,6 +855,13 @@ on a widget, you should now define this method on the form field itself. ``Model._meta.module_name`` was renamed to ``model_name``. Despite being a private API, it will go through a regular deprecation path. +``get_(add|change|delete)_permission`` model _meta methods +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +``Model._meta.get_(add|change|delete)_permission`` methods were deprecated. +Even if they were not part of the public API they'll also go through +a regular deprecation path. + ``get_query_set`` and similar methods renamed to ``get_queryset`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~