Merge pull request #548 from olivierdalang/fix_466

Fix #466 (Unique constraint failure when moving nodes). Thanks @matthiask and @olivierdalang

mptt/managers.py

@@ -616,6 +616,7 @@ def move_node(self, node, target, position='last-child'):
         move the node yourself by setting node.parent.
         """
         self._move_node(node, target, position=position)
+        node.save()
         node_moved.send(sender=node.__class__, instance=node,
                         target=target, position=position)
 
@@ -817,20 +818,13 @@ def _inter_tree_move_and_close_gap(
                     THEN %(right)s - %%s
                 WHEN %(right)s > %%s
                     THEN %(right)s - %%s
-                ELSE %(right)s END,
-            %(parent)s = CASE
-                WHEN %(pk)s = %%s
-                    THEN %(new_parent)s
-                ELSE %(parent)s END
+                ELSE %(right)s END
         WHERE %(tree_id)s = %%s""" % {
             'table': qn(self.tree_model._meta.db_table),
             'level': qn(opts.get_field(self.level_attr).column),
             'left': qn(opts.get_field(self.left_attr).column),
             'tree_id': qn(opts.get_field(self.tree_id_attr).column),
             'right': qn(opts.get_field(self.right_attr).column),
-            'parent': qn(opts.get_field(self.parent_attr).column),
-            'pk': qn(opts.pk.column),
-            'new_parent': parent_pk is None and 'NULL' or '%s',
         }
 
         left = getattr(node, self.left_attr)
@@ -844,11 +838,8 @@ def _inter_tree_move_and_close_gap(
             gap_target_left, gap_size,
             left, right, left_right_change,
             gap_target_left, gap_size,
-            node._meta.pk.get_db_prep_value(node.pk, connection),
             getattr(node, self.tree_id_attr)
         ]
-        if parent_pk is not None:
-            params.insert(-1, parent_pk)
 
         cursor = connection.cursor()
         cursor.execute(inter_tree_move_query, params)
@@ -1146,18 +1137,12 @@ def _move_child_within_tree(self, node, target, position):
                   THEN %(right)s + %%s
                 WHEN %(right)s >= %%s AND %(right)s <= %%s
                   THEN %(right)s + %%s
-                ELSE %(right)s END,
-            %(parent)s = CASE
-                WHEN %(pk)s = %%s
-                  THEN %%s
-                ELSE %(parent)s END
+                ELSE %(right)s END
         WHERE %(tree_id)s = %%s""" % {
             'table': qn(self.tree_model._meta.db_table),
             'level': qn(opts.get_field(self.level_attr).column),
             'left': qn(opts.get_field(self.left_attr).column),
             'right': qn(opts.get_field(self.right_attr).column),
-            'parent': qn(opts.get_field(self.parent_attr).column),
-            'pk': qn(opts.pk.column),
             'tree_id': qn(opts.get_field(self.tree_id_attr).column),
         }
 
@@ -1168,8 +1153,6 @@ def _move_child_within_tree(self, node, target, position):
             left_boundary, right_boundary, gap_size,
             left, right, left_right_change,
             left_boundary, right_boundary, gap_size,
-            node._meta.get_field(node._meta.pk.name).get_db_prep_value(node.pk, connection),
-            parent._meta.get_field(parent._meta.pk.name).get_db_prep_value(parent.pk, connection),
             tree_id])
 
         # Update the node to be consistent with the updated
@@ -1217,28 +1200,20 @@ def _move_root_node(self, node, target, position):
         SET %(level)s = %(level)s - %%s,
             %(left)s = %(left)s - %%s,
             %(right)s = %(right)s - %%s,
-            %(tree_id)s = %%s,
-            %(parent)s = CASE
-                WHEN %(pk)s = %%s
-                    THEN %%s
-                ELSE %(parent)s END
+            %(tree_id)s = %%s
         WHERE %(left)s >= %%s AND %(left)s <= %%s
           AND %(tree_id)s = %%s""" % {
             'table': qn(self.tree_model._meta.db_table),
             'level': qn(opts.get_field(self.level_attr).column),
             'left': qn(opts.get_field(self.left_attr).column),
             'right': qn(opts.get_field(self.right_attr).column),
             'tree_id': qn(opts.get_field(self.tree_id_attr).column),
-            'parent': qn(opts.get_field(self.parent_attr).column),
-            'pk': qn(opts.pk.column),
         }
 
         cursor = connection.cursor()
         cursor.execute(move_tree_query, [
             level_change, left_right_change, left_right_change,
             new_tree_id,
-            node._meta.pk.get_db_prep_value(node.pk, connection),
-            parent._meta.pk.get_db_prep_value(parent.pk, connection),
             left, right, tree_id])
 
         # Update the former root node to be consistent with the updated

mptt/models.py

@@ -811,7 +811,7 @@ def _get_user_field_names(self):
         field_names = []
         internal_fields = (
             self._mptt_meta.left_attr, self._mptt_meta.right_attr, self._mptt_meta.tree_id_attr,
-            self._mptt_meta.level_attr, self._mptt_meta.parent_attr)
+            self._mptt_meta.level_attr)
         for field in self._meta.fields:
             if (field.name not in internal_fields) and (not isinstance(field, AutoField)) and (not field.primary_key):  # noqa
                 field_names.append(field.name)

tests/myapp/models.py

@@ -334,6 +334,13 @@ class Book(MPTTModel):
     m2m = TreeManyToManyField(Category, blank=True, related_name='books_m2m')
 
 
+class UniqueTogetherModel( MPTTModel ):
+    class Meta:
+        unique_together = (('parent','code',),)
+    parent = TreeForeignKey('self', null=True)
+    code = models.CharField(max_length=10)
+
+
 class NullableOrderedInsertionModel(MPTTModel):
     name = models.CharField(max_length=50, null=True)
     parent = TreeForeignKey(
@@ -358,3 +365,4 @@ class MPTTMeta:
 
     def __str__(self):
         return self.name
+

tests/myapp/tests.py

@@ -37,7 +37,7 @@
     Category, Item, Genre, CustomPKName, SingleProxyModel, DoubleProxyModel,
     ConcreteModel, OrderedInsertion, AutoNowDateFieldModel, Person,
     CustomTreeQueryset, Node, CustomTreeManager, Book, UUIDNode, Student,
-    MultipleManagerModel, NullableOrderedInsertionModel, NullableDescOrderedInsertionModel)
+    MultipleManagerModel, UniqueTogetherModel, NullableOrderedInsertionModel, NullableDescOrderedInsertionModel)
 
 
 def get_tree_details(nodes):
@@ -2298,6 +2298,46 @@ def test_assignment(self):
         n1.save()
 
 
+class MovingNodeWithUniqueConstraint(TreeTestCase):
+    def test_unique_together_move_to_same_parent_change_code(self):
+        """Regression test for #466 1"""
+
+        UniqueTogetherModel.objects.all().delete()
+
+        a = UniqueTogetherModel.objects.create(code='a', parent=None)
+        b = UniqueTogetherModel.objects.create(code='b', parent=None)
+        a1 = UniqueTogetherModel.objects.create(code='1', parent=a)
+        b1 = UniqueTogetherModel.objects.create(code='1', parent=b)
+        b1.parent, b1.code = a, '2'  # b1 -> a2
+        b1.save()
+
+        self.assertTreeEqual(UniqueTogetherModel.objects.all(), """
+            1 - 1 0 1 6
+            3 1 1 1 2 3
+            4 1 1 1 4 5
+            2 - 2 0 1 2
+        """)
+
+    def test_unique_together_move_to_same_code_change_parent(self):
+        """Regression test for #466 1"""
+
+        UniqueTogetherModel.objects.all().delete()
+
+        a = UniqueTogetherModel.objects.create(code='a', parent=None)
+        b = UniqueTogetherModel.objects.create(code='b', parent=None)
+        a1 = UniqueTogetherModel.objects.create(code='1', parent=a)
+        a2 = UniqueTogetherModel.objects.create(code='2', parent=a)
+        a2.parent, a2.code = b, '1'  # a2 -> b1
+        a2.save()
+
+        self.assertTreeEqual(UniqueTogetherModel.objects.all(), """
+            1 - 1 0 1 4
+            3 1 1 1 2 3
+            2 - 2 0 1 4
+            4 2 2 1 2 3
+        """)
+
+
 class NullableOrderedInsertion(TreeTestCase):
     def test_nullable_ordered_insertion(self):