Fix undo stack does not correctly roll back geometry edits

in a granular manner Fixes #17658
qgis · Feb 13, 2018 · 70644f1 · 70644f1
1 parent 66b9b43
commit 70644f1
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 5 deletions.
diff --git a/src/core/qgsvectorlayerundocommand.cpp b/src/core/qgsvectorlayerundocommand.cpp
@@ -118,9 +118,7 @@ QgsVectorLayerUndoCommandChangeGeometry::QgsVectorLayerUndoCommandChangeGeometry
   }
   else
   {
-    bool changedAlready = mBuffer->mChangedGeometries.contains( mFid );
-    QgsGeometry geom;
-    mOldGeom = changedAlready ? geom : QgsGeometry();
+    mOldGeom = mBuffer->mChangedGeometries.value( mFid, QgsGeometry() );
   }
 }
 

diff --git a/tests/src/python/test_qgsvectorlayereditbuffer.py b/tests/src/python/test_qgsvectorlayereditbuffer.py
@@ -278,23 +278,72 @@ def testChangeGeometry(self):
         self.assertFalse(layer.editBuffer().isFeatureGeometryChanged(1))
         self.assertFalse(layer.editBuffer().isFeatureGeometryChanged(2))
 
-        # change attribute values
+        # change geometry
         layer.changeGeometry(1, QgsGeometry.fromPointXY(QgsPointXY(10, 20)))
 
         # test contents of buffer
         self.assertEqual(list(layer.editBuffer().changedGeometries().keys()), [1])
         self.assertEqual(layer.editBuffer().changedGeometries()[1].constGet().x(), 10)
         self.assertTrue(layer.editBuffer().isFeatureGeometryChanged(1))
         self.assertFalse(layer.editBuffer().isFeatureGeometryChanged(2))
+        self.assertEqual(layer.undoStack().count(), 1)
+
+        self.assertEqual(layer.getFeature(1).geometry().constGet().x(), 10)
+        self.assertEqual(layer.getFeature(2).geometry().constGet().x(), 2)
+
+        # apply second change to same feature
+        layer.beginEditCommand('second change') # need to use an edit command to avoid the two geometry changes being merged
+        layer.changeGeometry(1, QgsGeometry.fromPointXY(QgsPointXY(100, 200)))
+        layer.endEditCommand()
+
+        # test contents of buffer
+        self.assertEqual(list(layer.editBuffer().changedGeometries().keys()), [1])
+        self.assertEqual(layer.editBuffer().changedGeometries()[1].constGet().x(), 100)
+        self.assertTrue(layer.editBuffer().isFeatureGeometryChanged(1))
+        self.assertFalse(layer.editBuffer().isFeatureGeometryChanged(2))
+        self.assertEqual(layer.undoStack().count(), 2)
+
+        self.assertEqual(layer.getFeature(1).geometry().constGet().x(), 100)
+        self.assertEqual(layer.getFeature(2).geometry().constGet().x(), 2)
 
         layer.changeGeometry(2, QgsGeometry.fromPointXY(QgsPointXY(20, 40)))
 
         # test contents of buffer
         self.assertEqual(set(layer.editBuffer().changedGeometries().keys()), set([1, 2]))
-        self.assertEqual(layer.editBuffer().changedGeometries()[1].constGet().x(), 10)
+        self.assertEqual(layer.editBuffer().changedGeometries()[1].constGet().x(), 100)
         self.assertEqual(layer.editBuffer().changedGeometries()[2].constGet().x(), 20)
         self.assertTrue(layer.editBuffer().isFeatureGeometryChanged(1))
         self.assertTrue(layer.editBuffer().isFeatureGeometryChanged(2))
+        self.assertEqual(layer.undoStack().count(), 3)
+
+        self.assertEqual(layer.getFeature(1).geometry().constGet().x(), 100)
+        self.assertEqual(layer.getFeature(2).geometry().constGet().x(), 20)
+
+        layer.undoStack().undo()
+
+        self.assertEqual(list(layer.editBuffer().changedGeometries().keys()), [1])
+        self.assertEqual(layer.editBuffer().changedGeometries()[1].constGet().x(), 100)
+        self.assertTrue(layer.editBuffer().isFeatureGeometryChanged(1))
+        self.assertFalse(layer.editBuffer().isFeatureGeometryChanged(2))
+
+        self.assertEqual(layer.getFeature(1).geometry().constGet().x(), 100)
+        self.assertEqual(layer.getFeature(2).geometry().constGet().x(), 2)
+
+        layer.undoStack().undo()
+        self.assertEqual(list(layer.editBuffer().changedGeometries().keys()), [1])
+        self.assertTrue(layer.editBuffer().isFeatureGeometryChanged(1))
+        self.assertFalse(layer.editBuffer().isFeatureGeometryChanged(2))
+
+        self.assertEqual(layer.getFeature(1).geometry().constGet().x(), 10)
+        self.assertEqual(layer.getFeature(2).geometry().constGet().x(), 2)
+
+        layer.undoStack().undo()
+        self.assertEqual(list(layer.editBuffer().changedGeometries().keys()), [])
+        self.assertFalse(layer.editBuffer().isFeatureGeometryChanged(1))
+        self.assertFalse(layer.editBuffer().isFeatureGeometryChanged(2))
+
+        self.assertEqual(layer.getFeature(1).geometry().constGet().x(), 1)
+        self.assertEqual(layer.getFeature(2).geometry().constGet().x(), 2)
 
     def testDeleteAttribute(self):
         # test deleting attributes from an edit buffer