Skip to content

Loading…

Fixes Moving a plugin to a new placeholder ignores CMS_PLACEHOLDER_CONF limits #1670

Merged
merged 4 commits into from

2 participants

@czpython
Divio AG member

This is a patch for issue #1646.

Basically I modified the Javascript to display a message when a user attempts to move or add a plugin either in the front or backend and the action was not successful. I also modified the move_plugin to check for the placeholder limits and moved the checking functionality to a function (has_reached_plugin_limit) in separate module, which raises an exception (PluginLimitReached) . Also created a new test case for PageAdmin methods which allows us to test methods like add_plugin and move_plugin. And last but not least I fixed a NameError occurring under certain conditions in testcases.py

@ojii ojii merged commit 251236c into divio:develop

1 check passed

Details default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
View
25 cms/admin/pageadmin.py
@@ -31,7 +31,7 @@
PagePermissionInlineAdmin, ViewRestrictionInlineAdmin)
from cms.admin.views import revert_plugins
from cms.apphook_pool import apphook_pool
-from cms.exceptions import NoPermissionsException
+from cms.exceptions import NoPermissionsException, PluginLimitReached
from cms.forms.widgets import PluginEditor
from cms.models import (Page, Title, CMSPlugin, PagePermission,
PageModeratorState, EmptyTitle, GlobalPagePermission, titlemodels)
@@ -48,6 +48,7 @@
from cms.utils.permissions import has_global_page_permission
from cms.utils.plugins import current_site
+from cms.plugins.utils import has_reached_plugin_limit
from menus.menu_pool import menu_pool
DJANGO_1_3 = LooseVersion(django.get_version()) < LooseVersion('1.4')
@@ -1141,19 +1142,13 @@ def add_plugin(self, request):
parent = None
# page add-plugin
if page:
+ # this only runs when both page and placeholder are not empty.
language = request.POST['language'] or get_language_from_request(request)
position = CMSPlugin.objects.filter(language=language, placeholder=placeholder).count()
- limits = placeholder_utils.get_placeholder_conf("limits", placeholder.slot, page.get_template())
- if limits:
- global_limit = limits.get("global")
- type_limit = limits.get(plugin_type)
- if global_limit and position >= global_limit:
- return HttpResponseBadRequest(_("This placeholder already has the maximum number of plugins"))
- elif type_limit:
- type_count = CMSPlugin.objects.filter(language=language, placeholder=placeholder, plugin_type=plugin_type).count()
- if type_count >= type_limit:
- plugin_name = unicode(plugin_pool.get_plugin(plugin_type).name)
- return HttpResponseBadRequest(_("This placeholder already has the maximum number allowed of %s plugins.") % plugin_name)
+ try:
+ has_reached_plugin_limit(placeholder, plugin_type, language, template=page.get_template())
+ except PluginLimitReached, e:
+ return HttpResponseBadRequest(str(e))
# in-plugin add-plugin
elif parent_id:
parent = get_object_or_404(CMSPlugin, pk=parent_id)
@@ -1376,9 +1371,13 @@ def move_plugin(self, request):
if not placeholder_slot in placeholders:
return HttpResponseBadRequest(str("error"))
placeholder = page.placeholders.get(slot=placeholder_slot)
- plugin.placeholder = placeholder
+ try:
+ has_reached_plugin_limit(placeholder, plugin.plugin_type, plugin.language, template=page.get_template())
+ except PluginLimitReached, e:
+ return HttpResponseBadRequest(str(e))
# plugin positions are 0 based, so just using count here should give us 'last_position + 1'
position = CMSPlugin.objects.filter(placeholder=placeholder).count()
+ plugin.placeholder = placeholder
plugin.position = position
# update the placeholder on all descendant plugins as well
for child in plugin.get_descendants():
View
33 cms/admin/placeholderadmin.py
@@ -1,4 +1,5 @@
# -*- coding: utf-8 -*-
+from cms.exceptions import PluginLimitReached
from cms.forms.fields import PlaceholderFormField
from cms.models.fields import PlaceholderField
from cms.models.placeholdermodel import Placeholder
@@ -6,6 +7,7 @@
from cms.plugin_pool import plugin_pool
from cms.utils import get_language_from_request, cms_static_url, get_cms_setting
from cms.utils.permissions import has_plugin_permission
+from cms.plugins.utils import has_reached_plugin_limit
from copy import deepcopy
from django.conf import settings
from django.contrib.admin import ModelAdmin
@@ -149,27 +151,11 @@ def add_plugin(self, request):
# check add permissions on placeholder
if not placeholder.has_add_permission(request):
return HttpResponseForbidden(_("You don't have permission to add content here."))
-
- # check the limits defined in CMS_PLACEHOLDER_CONF for this placeholder
- limits = get_cms_setting('PLACEHOLDER_CONF').get(placeholder.slot, {}).get('limits', None)
- if limits:
- count = placeholder.cmsplugin_set.count()
- global_limit = limits.get("global", None)
- type_limit = limits.get(plugin_type, None)
- # check the global limit first
- if global_limit and count >= global_limit:
- return HttpResponseBadRequest(
- "This placeholder already has the maximum number of plugins."
- )
- elif type_limit: # then check the type specific limit
- type_count = CMSPlugin.objects.filter(
- language=language, placeholder=placeholder, plugin_type=plugin_type
- ).count()
- if type_count >= type_limit:
- return HttpResponseBadRequest(
- "This placeholder already has the maximum number (%s) "
- "of %s plugins." % (type_limit, plugin_type)
- )
+
+ try:
+ has_reached_plugin_limit(placeholder, plugin_type, language)
+ except PluginLimitReached, e:
+ return HttpResponseBadRequest(str(e))
# actually add the plugin
plugin = CMSPlugin(language=language, plugin_type=plugin_type,
@@ -273,6 +259,11 @@ def move_plugin(self, request):
if not placeholder.has_change_permission(request):
raise Http404
+ try:
+ has_reached_plugin_limit(placeholder, plugin.plugin_type, plugin.language)
+ except PluginLimitReached, e:
+ return HttpResponseBadRequest(str(e))
+
# plugin positions are 0 based, so just using count here should give us 'last_position + 1'
position = CMSPlugin.objects.filter(placeholder=placeholder).count()
plugin.placeholder = placeholder
View
9 cms/exceptions.py
@@ -7,6 +7,13 @@ class PluginNotRegistered(Exception):
pass
+class PluginLimitReached(Exception):
+ """
+ Gets triggered when a placeholder has reached it's plugin limit.
+ """
+ pass
+
+
class AppAlreadyRegistered(Exception):
pass
@@ -54,4 +61,4 @@ class DontUsePageAttributeWarning(Warning): pass
class CMSDeprecationWarning(Warning): pass
-class LanguageError(Exception): pass
+class LanguageError(Exception): pass
View
30 cms/plugins/utils.py
@@ -2,10 +2,15 @@
import operator
from itertools import groupby
+from django.utils.translation import ugettext as _
+
+from cms.exceptions import PluginLimitReached
from cms.plugin_pool import plugin_pool
from cms.utils import get_language_from_request
from cms.utils.i18n import get_redirect_on_fallback, get_fallback_languages
from cms.utils.moderator import get_cmsplugin_queryset
+from cms.utils.placeholder import get_placeholder_conf
+
def get_plugins(request, placeholder, lang=None):
if not placeholder:
@@ -97,3 +102,28 @@ def get_plugins_for_page(request, page, lang=None):
placeholder__page=page, placeholder__slot__in=slots, language=lang, parent__isnull=True
).order_by('placeholder', 'position').select_related())
return getattr(page, '_%s_plugins_cache' % lang)
+
+
+def has_reached_plugin_limit(placeholder, plugin_type, language, template=None):
+ """
+ Checks if placeholder has reached it's global plugin limit,
+ if not then it checks if it has reached it's plugin_type limit.
+ """
+ limits = get_placeholder_conf("limits", placeholder.slot, template)
+ if limits:
+ global_limit = limits.get("global")
+ type_limit = limits.get(plugin_type)
+ # total plugin count
+ count = placeholder.cmsplugin_set.filter(language=language).count()
+ if global_limit and count >= global_limit:
+ raise PluginLimitReached(_("This placeholder already has the maximum number of plugins (%s)." % count))
+ elif type_limit:
+ # total plugin type count
+ type_count = placeholder.cmsplugin_set.filter(
+ language=language,
+ plugin_type=plugin_type,
+ ).count()
+ if type_count >= type_limit:
+ plugin_name = unicode(plugin_pool.get_plugin(plugin_type).name)
+ raise PluginLimitReached(_("This placeholder already has the maximum number (%s) of allowed %s plugins.") % (type_limit, plugin_name))
+ return False
View
17 cms/static/cms/js/plugin_editor.js
@@ -103,12 +103,17 @@
var plugin_id = ui.item.attr('id').split('plugin_')[1];
var slot_name = ui.item.parent().parent().data('name');
var placeholder_id = ui.item.parent().parent().data('id');
- $.post("move-plugin/", {
- placeholder: slot_name,
- placeholder_id: placeholder_id,
- plugin_id: plugin_id,
- ids: d
- }, function(){}, "json");
+ $.ajax({
+ url: "move-plugin/", dataType: "html", type: "POST",
+ data: ({ placeholder:slot_name, placeholder_id:placeholder_id, plugin_id:plugin_id, ids:d }),
+ error: function(xhr) {
+ if (xhr.status < 500) {
+ alert(xhr.responseText);
+ }
+ // Cancel the drop
+ ui.sender.sortable("cancel");
+ }
+ });
} else {
// moved in placeholder
if (d) {
View
29 cms/static/cms/js/plugins/cms.placeholders.js
@@ -84,8 +84,13 @@ CMS.$(document).ready(function ($) {
// we get the id back
that.editPlugin.call(that, values.placeholder_id, response, editUrl);
},
- 'error': function () {
- throw new Error('CMS.Placeholders was unable to perform this ajax request. Try again or contact the developers.');
+ 'error': function(xhr) {
+ if (xhr.status < 500) {
+ alert(xhr.responseText);
+ }
+ else{
+ throw new Error('CMS.Placeholders was unable to perform this ajax request. Try again or contact the developers.');
+ };
}
});
},
@@ -235,8 +240,13 @@ CMS.$(document).ready(function ($) {
'url': url,
'data': { 'ids': array.join('_') },
'success': refreshPluginPosition,
- 'error': function () {
- throw new Error('CMS.Placeholders was unable to perform this ajax request. Try again or contact the developers.');
+ 'error': function(xhr) {
+ if (xhr.status < 500) {
+ alert(xhr.responseText);
+ }
+ else{
+ throw new Error('CMS.Placeholders was unable to perform this ajax request. Try again or contact the developers.');
+ };
}
});
@@ -333,8 +343,13 @@ CMS.$(document).ready(function ($) {
'success': function () {
refreshPluginPosition(slot);
},
- 'error': function () {
- throw new Error('CMS.Placeholders was unable to perform this ajax request. Try again or contact the developers.');
+ 'error': function(xhr) {
+ if (xhr.status < 500) {
+ alert(xhr.responseText);
+ }
+ else{
+ throw new Error('CMS.Placeholders was unable to perform this ajax request. Try again or contact the developers.');
+ };
}
});
});
@@ -660,4 +675,4 @@ CMS.$(document).ready(function ($) {
});
-});
+});
View
1 cms/test_utils/testcases.py
@@ -4,6 +4,7 @@
SettingsOverride)
from django.conf import settings
from django.contrib.auth.models import User, AnonymousUser
+from django.contrib.sites.models import Site
from django.core.exceptions import ObjectDoesNotExist
from django.core.urlresolvers import reverse
from django.template.context import Context
View
93 cms/tests/page.py
@@ -6,11 +6,13 @@
from django.conf import settings
from django.contrib.sites.models import Site
+from django.contrib import admin
from django.core.exceptions import ValidationError
from django.core.urlresolvers import reverse
from django.http import HttpRequest, HttpResponse, HttpResponseNotFound
from cms.admin.forms import PageForm
+from cms.admin.pageadmin import PageAdmin
from cms.api import create_page, add_plugin
from cms.models import Page, Title
from cms.models.placeholdermodel import Placeholder
@@ -23,7 +25,8 @@
from cms.test_utils.testcases import (CMSTestCase, URL_CMS_PAGE,
URL_CMS_PAGE_ADD)
from cms.test_utils.util.context_managers import (LanguageOverride,
- SettingsOverride)
+ SettingsOverride,
+ UserLoginContext)
from cms.utils.page_resolver import get_page_from_request, is_valid_url
from cms.utils import timezone, get_cms_setting
from cms.utils.page import is_valid_page_slug
@@ -774,6 +777,94 @@ def test_plugin_loading_queries(self):
self.assertIn('link-%d-%d' % (i, j), content)
+class PageAdminTestBase(CMSTestCase):
+ """
+ The purpose of this class is to provide some basic functionality
+ to test methods of the Page admin.
+ """
+ placeholderconf = {'body': {
+ 'limits': {
+ 'global': 2,
+ 'TextPlugin': 1,
+ }
+ }
+ }
+
+ def get_page(self, parent=None, site=None,
+ language=None, template='nav_playground.html'):
+ page_data = {
+ 'title': 'test page %d' % self.counter,
+ 'slug': 'test-page-%d' % self.counter,
+ 'language': settings.LANGUAGES[0][0] if not language else language,
+ 'template': template,
+ 'parent': parent if parent else None,
+ 'site': site if site else Site.objects.get_current(),
+ }
+ page_data = self.get_new_page_data_dbfields()
+ return create_page(**page_data)
+
+ def get_admin(self):
+ """
+ Returns a PageAdmin instance.
+ """
+ return PageAdmin(Page, admin.site)
+
+ def get_post_request(self, data):
+ return self.get_request(post_data=data)
+
+
+class PageAdminTest(PageAdminTestBase):
+ def test_global_limit_on_plugin_move(self):
+ admin = self.get_admin()
+ superuser = self.get_superuser()
+ cms_page = self.get_page()
+ source_placeholder = cms_page.placeholders.get(slot='right-column')
+ target_placeholder = cms_page.placeholders.get(slot='body')
+ data = {
+ 'placeholder': source_placeholder,
+ 'plugin_type': 'LinkPlugin',
+ 'language': 'en',
+ }
+ plugin_1 = add_plugin(**data)
+ plugin_2 = add_plugin(**data)
+ plugin_3 = add_plugin(**data)
+ with UserLoginContext(self, superuser):
+ with SettingsOverride(CMS_PLACEHOLDER_CONF=self.placeholderconf):
+ request = self.get_post_request({'placeholder': target_placeholder.slot, 'plugin_id': plugin_1.pk})
+ response = admin.move_plugin(request) # first
+ self.assertEqual(response.status_code, 200)
+ request = self.get_post_request({'placeholder': target_placeholder.slot, 'plugin_id': plugin_2.pk})
+ response = admin.move_plugin(request) # second
+ self.assertEqual(response.status_code, 200)
+ request = self.get_post_request({'placeholder': target_placeholder.slot, 'plugin_id': plugin_3.pk})
+ response = admin.move_plugin(request) # third
+ self.assertEqual(response.status_code, 400)
+ self.assertEqual(response.content, "This placeholder already has the maximum number of plugins (2).")
+
+ def test_type_limit_on_plugin_move(self):
+ admin = self.get_admin()
+ superuser = self.get_superuser()
+ cms_page = self.get_page()
+ source_placeholder = cms_page.placeholders.get(slot='right-column')
+ target_placeholder = cms_page.placeholders.get(slot='body')
+ data = {
+ 'placeholder': source_placeholder,
+ 'plugin_type': 'TextPlugin',
+ 'language': 'en',
+ }
+ plugin_1 = add_plugin(**data)
+ plugin_2 = add_plugin(**data)
+ with UserLoginContext(self, superuser):
+ with SettingsOverride(CMS_PLACEHOLDER_CONF=self.placeholderconf):
+ request = self.get_post_request({'placeholder': target_placeholder.slot, 'plugin_id': plugin_1.pk})
+ response = admin.move_plugin(request) # first
+ self.assertEqual(response.status_code, 200)
+ request = self.get_post_request({'placeholder': target_placeholder.slot, 'plugin_id': plugin_2.pk})
+ response = admin.move_plugin(request) # second
+ self.assertEqual(response.status_code, 400)
+ self.assertEqual(response.content, "This placeholder already has the maximum number (1) of allowed Text plugins.")
+
+
class NoAdminPageTests(CMSTestCase):
urls = 'cms.test_utils.project.noadmin_urls'
View
52 cms/tests/placeholder.py
@@ -451,7 +451,7 @@ def test_global_limit(self):
self.assertEqual(response.status_code, 200)
response = admin.add_plugin(request) # third
self.assertEqual(response.status_code, 400)
- self.assertEqual(response.content, "This placeholder already has the maximum number of plugins.")
+ self.assertEqual(response.content, "This placeholder already has the maximum number of plugins (2).")
def test_type_limit(self):
placeholder = self.get_placeholder()
@@ -469,7 +469,55 @@ def test_type_limit(self):
self.assertEqual(response.status_code, 200)
response = admin.add_plugin(request) # second
self.assertEqual(response.status_code, 400)
- self.assertEqual(response.content, "This placeholder already has the maximum number (1) of TextPlugin plugins.")
+ self.assertEqual(response.content, "This placeholder already has the maximum number (1) of allowed Text plugins.")
+
+ def test_global_limit_on_plugin_move(self):
+ admin = self.get_admin()
+ superuser = self.get_superuser()
+ source_placeholder = Placeholder.objects.create(slot='source')
+ target_placeholder = self.get_placeholder()
+ data = {
+ 'placeholder': source_placeholder,
+ 'plugin_type': 'LinkPlugin',
+ 'language': 'en',
+ }
+ plugin_1 = add_plugin(**data)
+ plugin_2 = add_plugin(**data)
+ plugin_3 = add_plugin(**data)
+ with UserLoginContext(self, superuser):
+ with SettingsOverride(CMS_PLACEHOLDER_CONF=self.placeholderconf):
+ request = self.get_post_request({'placeholder_id': target_placeholder.pk, 'plugin_id': plugin_1.pk})
+ response = admin.move_plugin(request) # first
+ self.assertEqual(response.status_code, 200)
+ request = self.get_post_request({'placeholder_id': target_placeholder.pk, 'plugin_id': plugin_2.pk})
+ response = admin.move_plugin(request) # second
+ self.assertEqual(response.status_code, 200)
+ request = self.get_post_request({'placeholder_id': target_placeholder.pk, 'plugin_id': plugin_3.pk})
+ response = admin.move_plugin(request) # third
+ self.assertEqual(response.status_code, 400)
+ self.assertEqual(response.content, "This placeholder already has the maximum number of plugins (2).")
+
+ def test_type_limit_on_plugin_move(self):
+ admin = self.get_admin()
+ superuser = self.get_superuser()
+ source_placeholder = Placeholder.objects.create(slot='source')
+ target_placeholder = self.get_placeholder()
+ data = {
+ 'placeholder': source_placeholder,
+ 'plugin_type': 'TextPlugin',
+ 'language': 'en',
+ }
+ plugin_1 = add_plugin(**data)
+ plugin_2 = add_plugin(**data)
+ with UserLoginContext(self, superuser):
+ with SettingsOverride(CMS_PLACEHOLDER_CONF=self.placeholderconf):
+ request = self.get_post_request({'placeholder_id': target_placeholder.pk, 'plugin_id': plugin_1.pk})
+ response = admin.move_plugin(request) # first
+ self.assertEqual(response.status_code, 200)
+ request = self.get_post_request({'placeholder_id': target_placeholder.pk, 'plugin_id': plugin_2.pk})
+ response = admin.move_plugin(request) # second
+ self.assertEqual(response.status_code, 400)
+ self.assertEqual(response.content, "This placeholder already has the maximum number (1) of allowed Text plugins.")
def test_edit_plugin_and_cancel(self):
placeholder = self.get_placeholder()
Something went wrong with that request. Please try again.