Fixes Moving a plugin to a new placeholder ignores CMS_PLACEHOLDER_CONF limits

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():

cms/admin/placeholderadmin.py

@@ -1,11 +1,13 @@
 # -*- 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
 from cms.models.pluginmodel import CMSPlugin
 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

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

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

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) {

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 ($) {
 
 	});
 
-});
+});

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

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'