Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMS_PERMISSION: optionally use raw id user lookups #1481

Merged
merged 9 commits into from Apr 15, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 43 additions & 4 deletions cms/admin/forms.py
Expand Up @@ -10,7 +10,7 @@
from cms.utils.page import is_valid_page_slug from cms.utils.page import is_valid_page_slug
from cms.utils.page_resolver import get_page_from_path, is_valid_url from cms.utils.page_resolver import get_page_from_path, is_valid_url
from cms.utils.permissions import (get_current_user, get_subordinate_users, from cms.utils.permissions import (get_current_user, get_subordinate_users,
get_subordinate_groups) get_subordinate_groups, get_user_permission_level)
from cms.utils.urlutils import any_path_re from cms.utils.urlutils import any_path_re
from django import forms from django import forms
from django.contrib.auth.forms import UserCreationForm from django.contrib.auth.forms import UserCreationForm
Expand Down Expand Up @@ -196,14 +196,53 @@ class PagePermissionInlineAdminForm(forms.ModelForm):
level or under him in choosen page tree, and users which were created by him, level or under him in choosen page tree, and users which were created by him,
but aren't assigned to higher page level than current user. but aren't assigned to higher page level than current user.
""" """
user = forms.ModelChoiceField('user', label=_('user'), widget=UserSelectAdminWidget, required=False)
page = forms.ModelChoiceField(Page, label=_('user'), widget=HiddenInput(), required=True) page = forms.ModelChoiceField(Page, label=_('user'), widget=HiddenInput(), required=True)


def __init__(self, *args, **kwargs): def __init__(self, *args, **kwargs):
super(PagePermissionInlineAdminForm, self).__init__(*args, **kwargs) super(PagePermissionInlineAdminForm, self).__init__(*args, **kwargs)
user = get_current_user() # current user from threadlocals user = get_current_user() # current user from threadlocals
self.fields['user'].queryset = get_subordinate_users(user) sub_users = get_subordinate_users(user)
self.fields['user'].widget.user = user # assign current user
limit_choices = True
use_raw_id = False

# Unfortunately, if there are > 500 users in the system, non-superusers
# won't see any benefit here because if we ask Django to put all the
# user PKs in limit_choices_to in the query string of the popup we're
# in danger of causing 414 errors so we fall back to the normal input
# widget.
if get_cms_setting('RAW_ID_USERS'):
if sub_users.count() < 500:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this where it should use the variable setting?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The threshold check to decide whether we should try to use a raw ID field is applied in cms/admin/permissionadmin.py. The "< 500" check applied here is only to prevent the raw ID blowing up with a 414 error, and there's little point making this configurable, IMO (though 500 is a little arbitrary).

# If there aren't too many users, proceed as normal and use a
# raw id field with limit_choices_to
limit_choices = True
use_raw_id = True
elif get_user_permission_level(user) == 0:
# If there are enough choices to possibly cause a 414 request
# URI too large error, we only proceed with the raw id field if
# the user is a superuser & thus can legitimately circumvent
# the limit_choices_to condition.
limit_choices = False
use_raw_id = True

# We don't use the fancy custom widget if the admin form wants to use a
# raw id field for the user
if use_raw_id:
from django.contrib.admin.widgets import ForeignKeyRawIdWidget
# This check will be False if the number of users in the system
# is less than the threshold set by the RAW_ID_USERS setting.
if isinstance(self.fields['user'].widget, ForeignKeyRawIdWidget):
# We can't set a queryset on a raw id lookup, but we can use
# the fact that it respects the limit_choices_to parameter.
if limit_choices:
self.fields['user'].widget.rel.limit_choices_to = dict(
id__in=list(sub_users.values_list('pk', flat=True))
)
else:
self.fields['user'].widget = UserSelectAdminWidget()
self.fields['user'].queryset = sub_users
self.fields['user'].widget.user = user # assign current user

self.fields['group'].queryset = get_subordinate_groups(user) self.fields['group'].queryset = get_subordinate_groups(user)


def clean(self): def clean(self):
Expand Down
13 changes: 11 additions & 2 deletions cms/admin/permissionadmin.py
Expand Up @@ -4,10 +4,11 @@
from cms.exceptions import NoPermissionsException from cms.exceptions import NoPermissionsException
from cms.models import Page, PagePermission, GlobalPagePermission, PageUser from cms.models import Page, PagePermission, GlobalPagePermission, PageUser
from cms.utils.conf import get_cms_setting from cms.utils.conf import get_cms_setting
from cms.utils.helpers import classproperty
from cms.utils.permissions import get_user_permission_level from cms.utils.permissions import get_user_permission_level
from copy import deepcopy from copy import deepcopy
from django.conf import settings
from django.contrib import admin from django.contrib import admin
from django.contrib.auth.models import User
from django.template.defaultfilters import title from django.template.defaultfilters import title
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _


Expand All @@ -26,7 +27,15 @@ class PagePermissionInlineAdmin(TabularInline):
classes = ['collapse', 'collapsed'] classes = ['collapse', 'collapsed']
exclude = ['can_view'] exclude = ['can_view']
extra = 0 # edit page load time boost extra = 0 # edit page load time boost


@classproperty
def raw_id_fields(cls):
# Dynamically set raw_id_fields based on settings
threshold = get_cms_setting('RAW_ID_USERS')
if threshold and User.objects.count() > threshold:
return ['user']
return []

def queryset(self, request): def queryset(self, request):
""" """
Queryset change, so user with global change permissions can see Queryset change, so user with global change permissions can see
Expand Down
2 changes: 2 additions & 0 deletions cms/utils/conf.py
Expand Up @@ -32,6 +32,8 @@ def wrapper():
'TEMPLATE_INHERITANCE': True, 'TEMPLATE_INHERITANCE': True,
'PLACEHOLDER_CONF': {}, 'PLACEHOLDER_CONF': {},
'PERMISSION': False, 'PERMISSION': False,
# Whether to use raw ID lookups for users when PERMISSION is True
'RAW_ID_USERS': False,
'PUBLIC_FOR': 'all', 'PUBLIC_FOR': 'all',
'CONTENT_CACHE_DURATION': 60, 'CONTENT_CACHE_DURATION': 60,
'SHOW_START_DATE': False, 'SHOW_START_DATE': False,
Expand Down
33 changes: 32 additions & 1 deletion cms/utils/helpers.py
Expand Up @@ -66,4 +66,35 @@ def make_revision_with_plugins(obj, user=None, message=None):
revision_context.add_to_context(revision_manager, plugin, bpadapter.get_version_data(plugin, VERSION_CHANGE)) revision_context.add_to_context(revision_manager, plugin, bpadapter.get_version_data(plugin, VERSION_CHANGE))


def find_placeholder_relation(obj): def find_placeholder_relation(obj):
return 'page' return 'page'


class classproperty(object):
"""Like @property, but for classes, not just instances.

Example usage:

>>> from cms.utils.helpers import classproperty
>>> class A(object):
... @classproperty
... def x(cls):
... return 'x'
... @property
... def y(self):
... return 'y'
...
>>> A.x
'x'
>>> A().x
'x'
>>> A.y
<property object at 0x2939628>
>>> A().y
'y'

"""
def __init__(self, fget):
self.fget = fget

def __get__(self, owner_self, owner_cls):
return self.fget(owner_cls)
27 changes: 27 additions & 0 deletions docs/getting_started/configuration.rst
Expand Up @@ -530,6 +530,33 @@ a certain page all users he creates can, in turn, only edit this page. Naturally
he can limit the rights of the users he creates even further, allowing them to see he can limit the rights of the users he creates even further, allowing them to see
only a subset of the pages to which he is allowed access. only a subset of the pages to which he is allowed access.


.. setting:: CMS_RAW_ID_USERS

CMS_RAW_ID_USERS
================

Default: ``False``

This setting only applies if :setting:`CMS_PERMISSION` is ``True``

The "view restrictions" and "page permissions" inlines on the
:class:`cms.models.Page` admin change forms can cause performance problems
where there are many thousands of users being put into simple select boxes. If
set to a positive integer, this setting forces the inlines on that page to use
standard Django admin raw ID widgets rather than select boxes if the number of
users in the system is greater than that number, dramatically improving
performance.

.. note:: Using raw ID fields in combination with ``limit_choices_to`` causes
errors due to excessively long URLs if you have many thousands of
users (the PKs are all included in the URL of the popup window). For
this reason, we only apply this limit if the number of users is
relatively small (fewer than 500). If the number of users we need to
limit to is greater than that, we use the usual input field instead
unless the user is a CMS superuser, in which case we bypass the
limit. Unfortunately, this means that non-superusers won't see any
benefit from this setting.

.. setting:: CMS_PUBLIC_FOR .. setting:: CMS_PUBLIC_FOR


CMS_PUBLIC_FOR CMS_PUBLIC_FOR
Expand Down