Fix (potentially) unreacheable article.save() bug inside FormWizard.s…

…ave() (#474)

aldryn_newsblog/cms_wizards.py

@@ -93,38 +93,28 @@ def __init__(self, **kwargs):
 
     def save(self, commit=True):
         article = super(CreateNewsBlogArticleForm, self).save(commit=False)
-
-        # Set owner to current user
         article.owner = self.user
 
-        # If 'content' field has value, create a TextPlugin with same and add
-        # it to the PlaceholderField
-        content = clean_html(self.cleaned_data.get('content', ''), False)
-        if content and permissions.has_plugin_permission(
-                self.user, 'TextPlugin', 'add'):
-            # If the article has not been saved, then there will be no
-            # Placeholder set-up for this article yet, so, ensure we have saved
-            # first.
-            if not article.pk:
-                article.save()
-
-            if article and article.content:
-                add_plugin(
-                    placeholder=article.content,
-                    plugin_type='TextPlugin',
-                    language=self.language_code,
-                    body=content,
-                )
-
         if ENABLE_REVERSION:
             from reversion.revisions import revision_context_manager
             with transaction.atomic():
                 with revision_context_manager.create_revision():
                     article.save()
-                    if self.user:
-                        revision_context_manager.set_user(self.user)
-                    revision_context_manager.set_comment(
-                        ugettext("Initial version."))
+                    revision_context_manager.set_user(self.user)
+                    revision_context_manager.set_comment(ugettext("Initial version."))
+        else:
+            article.save()
+
+        # If 'content' field has value, create a TextPlugin with same and add it to the PlaceholderField
+        content = clean_html(self.cleaned_data.get('content', ''), False)
+        if content and permissions.has_plugin_permission(self.user, 'TextPlugin', 'add'):
+            add_plugin(
+                placeholder=article.content,
+                plugin_type='TextPlugin',
+                language=self.language_code,
+                body=content,
+            )
+
         return article
 
 

aldryn_newsblog/tests/__init__.py

@@ -78,10 +78,11 @@ def rand_str(cls, prefix=u'', length=23, chars=string.ascii_letters):
         return prefix + u''.join(random.choice(chars) for _ in range(length))
 
     @classmethod
-    def create_user(cls):
-        return User.objects.create(
-            username=cls.rand_str(), first_name=cls.rand_str(),
-            last_name=cls.rand_str())
+    def create_user(cls, **kwargs):
+        kwargs.setdefault('username', cls.rand_str())
+        kwargs.setdefault('first_name', cls.rand_str())
+        kwargs.setdefault('last_name', cls.rand_str())
+        return User.objects.create(**kwargs)
 
     def create_person(self):
         return Person.objects.create(

aldryn_newsblog/tests/test_cms_wizards.py

@@ -0,0 +1,48 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from aldryn_newsblog.tests import NewsBlogTestCase
+from aldryn_newsblog.cms_wizards import CreateNewsBlogArticleForm
+
+
+class CreateNewsBlogArticleFormTestCase(NewsBlogTestCase):
+    def get_form(self, has_content, has_permission):
+        data = {'title': 'My super title', 'app_config': self.app_config.id}
+        if has_content:
+            data['content'] = 'My super content'
+
+        form = CreateNewsBlogArticleForm(wizard_language='en', data=data)
+        form.user = self.create_user(is_staff=has_permission, is_superuser=has_permission)
+        self.assertTrue(form.is_valid())
+        return form
+
+    def test_article_is_saved_with_content_user_with_plugin_permission(self):
+        form = self.get_form(has_content=True, has_permission=True)
+
+        article = form.save()
+        self.assertTrue(article.__class__.objects.filter(id=article.id).exists())
+        self.assertEquals(article.content.get_plugins('en').count(), 1)
+        plugin = article.content.get_plugins('en').get()
+        self.assertEquals(plugin.plugin_type, 'TextPlugin')
+        self.assertEquals(plugin.djangocms_text_ckeditor_text.body, 'My super content')
+
+    def test_article_is_saved_without_content_with_plugin_permission(self):
+        form = self.get_form(has_content=False, has_permission=True)
+
+        article = form.save()
+        self.assertTrue(article.__class__.objects.filter(id=article.id).exists())
+        self.assertFalse(article.content.get_plugins('en').exists())
+
+    def test_article_is_saved_with_content_without_plugin_permission(self):
+        form = self.get_form(has_content=True, has_permission=False)
+
+        article = form.save()
+        self.assertTrue(article.__class__.objects.filter(id=article.id).exists())
+        self.assertFalse(article.content.get_plugins('en').exists())
+
+    def test_article_is_saved_without_content_without_plugin_permission(self):
+        form = self.get_form(has_content=False, has_permission=False)
+
+        article = form.save()
+        self.assertTrue(article.__class__.objects.filter(id=article.id).exists())
+        self.assertFalse(article.content.get_plugins('en').exists())

test_requirements/base.txt

@@ -9,6 +9,7 @@ djangocms-helper
 # djangocms_text_ckeditor
 docopt
 flake8
+tox>=2.9.1
 html5lib<0.99999999
 pyflakes
 selenium