Merge pull request #3826 from yakky/feature/fix_page_move

Fix page move

cms/admin/pageadmin.py

@@ -182,17 +182,13 @@ def save_model(self, request, obj, form, change):
             obj.numchild = 0
             obj.depth = 0
             if parent:
-                parent.add_child(instance=obj)
+                saved_obj = parent.add_child(instance=obj)
             else:
-                obj.add_root(instance=obj)
-            new_pk = obj.pk
-            saved_obj = Page.objects.get(pk=new_pk)
-            obj.pk = pk
-            obj.path = saved_obj.path
-            obj.numchild = saved_obj.numchild
-            obj.depth = saved_obj.depth
-            saved_obj.delete()
-            obj.save(no_signals=True)
+                saved_obj = obj.add_root(instance=obj)
+            tmp_pk = saved_obj.pk
+            saved_obj.pk = pk
+            Page.objects.get(pk=tmp_pk).delete()
+            saved_obj.save(no_signals=True)
         else:
             if 'history' in request.path_info:
                 old_obj = Page.objects.get(pk=obj.pk)
@@ -219,7 +215,7 @@ def save_model(self, request, obj, form, change):
                 else:
                     obj.parent_id = target.parent_id
                 obj.save()
-                obj.move(target, pos=position)
+                obj = obj.move(target, pos=position)
         page_type_id = form.cleaned_data.get('page_type')
         copy_target_id = request.GET.get('copy_target')
         if copy_target_id or page_type_id:

cms/admin/placeholderadmin.py

@@ -433,13 +433,12 @@ def move_plugin(self, request):
                     return HttpResponseBadRequest(force_text('parent must be in the same language as plugin_language'))
                 plugin.parent_id = parent.pk
                 plugin.save()
-                plugin.move(parent, pos='last-child')
+                plugin = plugin.move(parent, pos='last-child')
         else:
             sibling = CMSPlugin.get_last_root_node()
             plugin.parent_id = None
             plugin.save()
-            plugin.move(sibling, pos='right')
-        plugin = CMSPlugin.objects.get(pk=plugin.pk)
+            plugin = plugin.move(sibling, pos='right')
         for child in [plugin] + list(plugin.get_descendants()):
             child.placeholder = placeholder
             child.language = language

cms/api.py

@@ -220,12 +220,10 @@ def create_page(title, template, language, menu_title=None, slug=None,
         limit_visibility_in_menu=limit_visibility_in_menu,
         xframe_options=xframe_options,
     )
-    page.add_root(instance=page)
-    page = page.reload()
+    page = page.add_root(instance=page)
 
     if parent:
-        page.move(target=parent, pos=position)
-        page = page.reload()
+        page = page.move(target=parent, pos=position)
 
     create_title(
         language=language,
@@ -345,11 +343,10 @@ def add_plugin(placeholder, plugin_type, language, position='last-child',
         parent_id=parent_id,
     )
 
-    plugin_base.add_root(instance=plugin_base)
+    plugin_base = plugin_base.add_root(instance=plugin_base)
 
     if target:
-        plugin_base.move(target, pos=position)
-        plugin_base = CMSPlugin.objects.get(pk=plugin_base.pk)
+        plugin_base = plugin_base.move(target, pos=position)
     plugin = plugin_model(**data)
     plugin_base.set_base_attr(plugin)
     plugin.save()

cms/management/commands/subcommands/tree.py

@@ -23,7 +23,7 @@ def handle_noargs(self, **options):
                 page.move(target=last, pos='right')
             elif first and first.pk != page.pk:
                 page.move(target=first, pos='left')
-            last = page
+            last = page.reload()
         for page in Page.objects.filter(publisher_is_draft=False, parent__isnull=True).order_by('publisher_public__path'):
             page = page.reload()
             public = page.publisher_public

cms/models/pagemodel.py

@@ -185,14 +185,12 @@ def move_page(self, target, position='first-child'):
         else:
             self.parent_id = target.parent_id
         self.save()
-        self.move(target, pos=position)
+        moved_page = self.move(target, pos=position)
 
         # fire signal
         import cms.signals as cms_signals
+        cms_signals.page_moved.send(sender=Page, instance=moved_page)
 
-        cms_signals.page_moved.send(sender=Page, instance=self)
-        moved_page = Page.objects.get(pk=self.pk)
-        #self.save()  # always save the page after move, because of publisher
         # check the slugs
         page_utils.check_title_slugs(moved_page)
         ## Make sure to update the slug and path of the target page.
@@ -441,16 +439,13 @@ def save(self, no_signals=False, commit=True, **kwargs):
         if created:
             self.created_by = self.changed_by
         if commit:
-            if no_signals:  # ugly hack because of mptt
-                self.save_base(**kwargs)
-            else:
-                if not self.depth:
-                    if self.parent_id:
-                        self.parent.add_child(instance=self)
-                    else:
-                        self.add_root(instance=self)
-                    return #add_root and add_child save as well
-                super(Page, self).save(**kwargs)
+            if not self.depth:
+                if self.parent_id:
+                    self.parent.add_child(instance=self)
+                else:
+                    self.add_root(instance=self)
+                return #add_root and add_child save as well
+            super(Page, self).save(**kwargs)
 
     def save_base(self, *args, **kwargs):
         """Overridden save_base. If an instance is draft, and was changed, mark
@@ -1155,10 +1150,8 @@ def _publisher_save_public(self, obj):
                     if public_parent:
                         obj.parent_id = public_parent.pk
                         obj.parent = public_parent
-                        obj.add_root(instance=obj)
-                        #public_parent = public_parent.reload()
-                        obj = obj.reload()
-                        obj.move(target=public_parent, pos='first-child')
+                        obj = obj.add_root(instance=obj)
+                        obj = obj.move(target=public_parent, pos='first-child')
         else:
             # check if object was moved / structural tree change
             prev_public_sibling = obj.get_previous_filtered_sibling()
@@ -1168,22 +1161,26 @@ def _publisher_save_public(self, obj):
                 if public_prev_sib:
                     obj.parent_id = public_prev_sib.parent_id
                     obj.save()
-                    obj.move(public_prev_sib, pos="right")
+                    obj = obj.move(public_prev_sib, pos="right")
                 elif public_parent:
                     # move as a first child to parent
                     obj.parent_id = public_parent.pk
                     obj.save()
-                    obj.move(target=public_parent, pos='first-child')
+                    obj = obj.move(target=public_parent, pos='first-child')
                 else:
                     # it is a move from the right side or just save
                     next_sibling = self.get_next_filtered_sibling(**filters)
                     if next_sibling and next_sibling.publisher_public_id:
                         obj.parent_id = next_sibling.parent_id
                         obj.save()
-                        obj.move(next_sibling.publisher_public, pos="left")
+                        obj = obj.move(next_sibling.publisher_public, pos="left")
             else:
                 obj.save()
 
+    def move(self, target, pos=None):
+        super(Page, self).move(target, pos)
+        return self.reload()
+
     def rescan_placeholders(self):
         """
         Rescan and if necessary create placeholders in the current template.

cms/models/pluginmodel.py

@@ -240,6 +240,13 @@ def save(self, no_signals=False, *args, **kwargs):
                 return
             super(CMSPlugin, self).save()
 
+    def reload(self):
+        return CMSPlugin.objects.get(pk=self.pk)
+
+    def move(self, target, pos=None):
+        super(CMSPlugin, self).move(target, pos)
+        return self.reload()
+
     def set_base_attr(self, plugin):
         for attr in ['parent_id', 'placeholder', 'language', 'plugin_type', 'creation_date', 'depth', 'path',
                      'numchild', 'pk', 'position']:

cms/test_utils/testcases.py

@@ -188,7 +188,6 @@ def get_new_page_data(self, parent_id=''):
         self.counter += 1
         return page_data
 
-
     def get_new_page_data_dbfields(self, parent=None, site=None,
                                    language=None,
                                    template='nav_playground.html', ):

cms/tests/nested_plugins.py

@@ -231,7 +231,7 @@ def test_plugin_deep_nesting_and_copying(self):
         plugin_5 = add_plugin(placeholder, u"TextPlugin", u"en", body=u"05")
         left = CMSPlugin.objects.filter(parent__isnull=True).order_by('path')[0]
         plugin_5 = self.reload(plugin_5)
-        plugin_5.move(left, pos='right')
+        plugin_5 = plugin_5.move(left, pos='right')
         self.reorder_positions(plugin_5)
         self.reorder_positions(plugin_2)
 
@@ -379,7 +379,7 @@ def test_plugin_deep_nesting_and_copying(self):
         old_parent = plugin_2.parent
         plugin_2.parent_id = plugin_1.parent_id
         plugin_2.save()
-        plugin_2.move(target=plugin_1, pos="left")
+        plugin_2 = plugin_2.move(target=plugin_1, pos="left")
         self.reorder_positions(parent=old_parent)
         self.reorder_positions(plugin_2)
         self.copy_placeholders_and_check_results([placeholder])
@@ -390,7 +390,7 @@ def test_plugin_deep_nesting_and_copying(self):
         old_parent = plugin_6.parent
         plugin_6.parent_id = plugin_7.parent_id
         plugin_6.save()
-        plugin_6.move(target=plugin_7, pos="right")
+        plugin_6 = plugin_6.move(target=plugin_7, pos="right")
         self.reorder_positions(parent=old_parent)
         self.reorder_positions(plugin_6)
         self.copy_placeholders_and_check_results([placeholder])
@@ -401,7 +401,7 @@ def test_plugin_deep_nesting_and_copying(self):
         old_parent = plugin_3.parent
         plugin_3.parent_id = plugin_2.parent_id
         plugin_3.save()
-        plugin_3.move(target=plugin_2, pos="left")
+        plugin_3 = plugin_3.move(target=plugin_2, pos="left")
         self.reorder_positions(parent=old_parent)
         self.reorder_positions(plugin_3)
         self.copy_placeholders_and_check_results([placeholder])
@@ -412,7 +412,7 @@ def test_plugin_deep_nesting_and_copying(self):
         old_parent = plugin_3.parent
         plugin_3.parent_id = plugin_2.pk
         plugin_3.save()
-        plugin_3.move(target=plugin_2, pos="first-child")
+        plugin_3 = plugin_3.move(target=plugin_2, pos="first-child")
         self.reorder_positions(CMSPlugin.objects.filter(placeholder_id=plugin_3.placeholder_id, language=plugin_3.language, depth=1)[0])
         self.reorder_positions(plugin_3)
         self.copy_placeholders_and_check_results([placeholder])
@@ -423,7 +423,7 @@ def test_plugin_deep_nesting_and_copying(self):
         old_parent = plugin_7.parent
         plugin_7.parent_id = plugin_3.parent_id
         plugin_7.save()
-        plugin_7.move(target=plugin_3, pos="right")
+        plugin_7 = plugin_7.move(target=plugin_3, pos="right")
         self.reorder_positions(parent=old_parent)
         self.reorder_positions(plugin_7)
         self.copy_placeholders_and_check_results([placeholder, ])

cms/tests/page.py

@@ -98,6 +98,34 @@ def test_create_page_admin(self):
             title = Title.objects.drafts().get(slug=page_data['slug'])
             title = Title.objects.public().get(slug=page_data['slug'])
 
+    def test_create_tree_admin(self):
+        """
+        Test that a tree can be created via the admin
+        """
+        page_1 = self.get_new_page_data()
+
+        superuser = self.get_superuser()
+        with self.login_user_context(superuser):
+            # create home and auto publish
+            response = self.client.post(URL_CMS_PAGE_ADD, page_1)
+            self.assertRedirects(response, URL_CMS_PAGE)
+
+            title_home = Title.objects.drafts().get(slug=page_1['slug'])
+
+            page_2 = self.get_new_page_data(parent_id=title_home.page.pk)
+            page_3 = self.get_new_page_data(parent_id=title_home.page.pk)
+            page_4 = self.get_new_page_data(parent_id=title_home.page.pk)
+
+            response = self.client.post(URL_CMS_PAGE_ADD, page_2)
+            self.assertRedirects(response, URL_CMS_PAGE)
+            response = self.client.post(URL_CMS_PAGE_ADD, page_3)
+            self.assertRedirects(response, URL_CMS_PAGE)
+
+            title_left = Title.objects.drafts().get(slug=page_2['slug'])
+
+            response = self.client.post(URL_CMS_PAGE_ADD + '?target=%s&position=right' % title_left.page.pk, page_4)
+            self.assertRedirects(response, URL_CMS_PAGE)
+
     def test_create_page_api(self):
         page_data = {
             'title': 'root',
@@ -649,7 +677,7 @@ def test_delete_with_plugins(self):
             position=1,
             language=settings.LANGUAGES[0][0]
         )
-        plugin_base.add_root(instance=plugin_base)
+        plugin_base = plugin_base.add_root(instance=plugin_base)
 
         plugin = Text(body='')
         plugin_base.set_base_attr(plugin)

cms/tests/plugins.py

@@ -786,8 +786,7 @@ def test_copy_textplugin(self):
             placeholder=placeholder,
             position=0,
             language=self.FIRST_LANG)
-        plugin_base.add_root(instance=plugin_base)
-        plugin_base = CMSPlugin.objects.get(pk=plugin_base.pk)
+        plugin_base = plugin_base.add_root(instance=plugin_base)
         plugin = Text(body='')
         plugin_base.set_base_attr(plugin)
         plugin.save()
@@ -797,15 +796,13 @@ def test_copy_textplugin(self):
             placeholder=placeholder,
             position=0,
             language=self.FIRST_LANG)
-        plugin_base.add_child(instance=plugin_ref_1_base)
-        plugin_ref_1_base = CMSPlugin.objects.get(pk=plugin_ref_1_base.pk)
+        plugin_ref_1_base = plugin_base.add_child(instance=plugin_ref_1_base)
         plugin_ref_2_base = CMSPlugin(
             plugin_type='TextPlugin',
             placeholder=placeholder,
             position=1,
             language=self.FIRST_LANG)
-        plugin_base.add_child(instance=plugin_ref_2_base)
-        plugin_ref_2_base = CMSPlugin.objects.get(pk=plugin_ref_2_base.pk)
+        plugin_ref_2_base = plugin_base.add_child(instance=plugin_ref_2_base)
         plugin_ref_2 = Text(body='')
         plugin_ref_2_base.set_base_attr(plugin_ref_2)
 

cms/utils/copy_plugins.py

@@ -20,8 +20,7 @@ def copy_plugins_to(old_plugins, to_placeholder,
         from cms.models import CMSPlugin
         new_plugins[0].parent_id = parent_plugin_id
         new_plugins[0].save()
-        new_plugins[0].move(CMSPlugin.objects.get(pk=parent_plugin_id), pos='last-child')
-        new_plugins[0] = CMSPlugin.objects.get(pk=new_plugins[0].pk)
+        new_plugins[0] = new_plugins[0].move(CMSPlugin.objects.get(pk=parent_plugin_id), pos='last-child')
     plugins_ziplist = list(zip(new_plugins, old_plugins))
 
     # this magic is needed for advanced plugins like Text Plugins that can have