Merge a8fb376 into 06f5290

cms/tests/test_nested_plugins.py

@@ -929,3 +929,55 @@ def test_add_child_plugin(self):
         link_plugin = CMSPlugin.objects.get(parent_id=text_plugin_en.pk)
         self.assertEqual(link_plugin.parent_id, text_plugin_en.pk)
         self.assertEqual(link_plugin.position, text_plugin_en.position + 1)
+
+    def test_plugin_deep_nesting_and_copying_issue_position_parent_child_discrepency(self):
+        """
+        Captures an edge case issue where plugins have been seen to have a higher
+        position than their parent. When the placeholder is
+        copied the parent defaults to None because the plugin is not yet created / remapped.
+
+        Plugins first created in this order:
+
+            Plugin 1 (pk1, position 1)
+            Plugin 2 (pk2, position 2)
+            Plugin 3 (pk3, position 3)
+
+        Then a top level plugin is made a child of another.
+        The result is a child with a lower id and higher position that it's parent.
+
+            Plugin 1 (pk1, position 1)
+            Plugin 3 has children (pk3, position 3)
+                Plugin 2 (pk2, position 2)
+        """
+        placeholder = Placeholder(slot="some_slot")
+        placeholder.save()
+        # plugins in placeholder
+        plugin_1 = add_plugin(placeholder, "TextPlugin", "en", body="01")
+        plugin_2 = add_plugin(placeholder, "TextPlugin", "en", body="02")
+        plugin_3 = add_plugin(placeholder, "TextPlugin", "en", body="03")
+
+        expected_tree = [
+            (plugin_1.pk, None),
+            (plugin_2.pk, None),
+            (plugin_3.pk, None),
+        ]
+
+        self.copy_placeholders_and_check_results([placeholder])
+        self.compare_plugin_tree(expected_tree, placeholder)
+
+        plugin_2.parent = plugin_3
+        plugin_2.save()
+
+        self.reload(plugin_2)
+        self.reload(plugin_3)
+
+        expected_tree = [
+            (plugin_1.pk, None),
+            (plugin_2.pk, plugin_3.pk),
+            (plugin_3.pk, None),
+        ]
+
+        placeholder._recalculate_plugin_positions("en")
+
+        self.copy_placeholders_and_check_results([placeholder])
+        self.compare_plugin_tree(expected_tree, placeholder)

cms/utils/plugins.py

@@ -121,12 +121,30 @@ def get_plugin_restrictions(plugin, page=None, restrictions_cache=None):
     return (child_classes, parent_classes)
 
 
+def _reunite_orphaned_placeholder_plugin_children(root_plugin, orphaned_plugin_list, plugins_by_id):
+    """
+    Handle plugins where the parent hasn't yet been copied (child seen before the parent)
+
+    CAVEAT: The only reason this exists is because the plugin position is not
+           sequential through children when the user nests plugins.
+           It's now too late as content already has this issue, it would be a very expensive
+           calculation to recalculate every placeholders positions, needs to be handled gracefully
+           so that it doesn't actually matter :-).
+    """
+    for old_plugin_parent_id, new_plugin in orphaned_plugin_list:
+        new_parent = plugins_by_id.get(old_plugin_parent_id, root_plugin)
+        if new_parent:
+            new_plugin.parent = new_parent
+            new_plugin.save()
+
+
 def copy_plugins_to_placeholder(plugins, placeholder, language=None,
                                 root_plugin=None, start_positions=None):
     plugin_pairs = []
     plugins_by_id = OrderedDict()
     # Keeps track of the next available position per language.
     positions_by_language = {}
+    orphaned_plugin_list = []
 
     if start_positions:
         positions_by_language.update(start_positions)
@@ -180,6 +198,16 @@ def copy_plugins_to_placeholder(plugins, placeholder, language=None,
             plugin_pairs.append((new_plugin, source_plugin))
         plugins_by_id[source_plugin.pk] = new_plugin
 
+        # Rescue any orphaned plugins
+        if not parent and source_plugin.parent_id:
+            orphaned_plugin_list.append(
+                (source_plugin.parent_id, new_plugin)
+            )
+
+    # Reunite any orphaned plugins with the parent
+    if orphaned_plugin_list:
+        _reunite_orphaned_placeholder_plugin_children(root_plugin, orphaned_plugin_list, plugins_by_id)
+
     # Backwards compatibility
     # This magic is needed for advanced plugins like Text Plugins that can have
     # nested plugins and need to update their content based on the new plugins.
@@ -189,6 +217,7 @@ def copy_plugins_to_placeholder(plugins, placeholder, language=None,
 
     for language in positions_by_language:
         placeholder._recalculate_plugin_positions(language)
+
     return list(plugins_by_id.values())