Merge pull request #5292 from czpython/fixes/3.1.x/issue-5291

fixes #5291 on 3.1.x

cms/models/pluginmodel.py

@@ -345,15 +345,15 @@ def fix_tree(cls, destructive=False):
                 order = CMSPlugin.objects.filter(
                         placeholder_id=placeholder.pk, language=language,
                         parent_id__isnull=True
-                    ).order_by('position').values_list('pk', flat=True)
+                    ).order_by('position', 'path').values_list('pk', flat=True)
                 reorder_plugins(placeholder, None, language, order)
 
                 for plugin in CMSPlugin.objects.filter(
                         placeholder_id=placeholder.pk,
                         language=language).order_by('depth', 'path'):
                     order = CMSPlugin.objects.filter(
                             parent_id=plugin.pk
-                        ).order_by('position').values_list('pk', flat=True)
+                        ).order_by('position', 'path').values_list('pk', flat=True)
                     reorder_plugins(placeholder, plugin.pk, language, order)
 
     def post_copy(self, old_instance, new_old_ziplist):

cms/tests/nested_plugins.py

@@ -223,13 +223,63 @@ def test_plugin_fix_tree(self):
                                body=u"10", target=plugin_4
         )
 
-        original_plugins = (placeholder.get_plugins().order_by('position', 'path'))
+        # We do two comparisons here.
+        # One is to compare plugin position values
+        # per plugin instance.
+        # To do this we get a dictionary mapping plugin
+        # ids to their respective position.
+        # The second comparison is to make sure that
+        # plugins retain their position/path ordering.
+
+        # The reason for the these comparisons
+        # is because of an obscure behavior with postgres
+        # where somehow items with the same value that are
+        # sorted by that value will be returned in different
+        # order based on the orm query construction.
+
+        # By comparing ids with positions, we make sure that
+        # each plugin has the correct position after the fix-tree.
+        # See ticket #5291
+        plugins = (
+            CMSPlugin
+            .objects
+            .filter(placeholder=placeholder)
+        )
+
+        # Maps plugin ids to positions
+        original_plugin_positions = dict(
+            plugins
+            .order_by('position')
+            .values_list('pk', 'position')
+        )
+
+        # List of plugin ids sorted by position and path
+        original_plugin_ids = list(
+            plugins
+            .order_by('position', 'path')
+            .values_list('pk', flat=True)
+        )
 
-        CMSPlugin.objects.update(position=None)
+        # We use 1 to effectively "break" the tree
+        # and as a way to test that fixing trees with
+        # equal position values retains the correct ordering.
+        CMSPlugin.objects.update(position=1)
         CMSPlugin.fix_tree()
 
-        new_plugins = list(placeholder.get_plugins().order_by('position', 'path'))
-        self.assertSequenceEqual(original_plugins, new_plugins)
+        new_plugin_positions = dict(
+            plugins
+            .order_by('position')
+            .values_list('pk', 'position')
+        )
+
+        new_plugin_ids = list(
+            plugins
+            .order_by('position', 'path')
+            .values_list('pk', flat=True)
+        )
+
+        self.assertDictEqual(original_plugin_positions, new_plugin_positions)
+        self.assertSequenceEqual(original_plugin_ids, new_plugin_ids)
 
         # Now, check to see if the correct order is restored, even if we
         # re-arrange the plugins so that their natural «pk» order is different