Update selected feature ids before emitting featuresDeleted

Fixes #44921
qgis · Sep 20, 2021 · c79c79a · c79c79a
1 parent b7a3ed0
commit c79c79a
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 3 deletions.
diff --git a/src/core/vector/qgsvectorlayer.cpp b/src/core/vector/qgsvectorlayer.cpp
@@ -1193,8 +1193,7 @@ bool QgsVectorLayer::deleteSelectedFeatures( int *deletedCount, QgsVectorLayer::
   int count = mSelectedFeatureIds.size();
   // Make a copy since deleteFeature modifies mSelectedFeatureIds
   QgsFeatureIds selectedFeatures( mSelectedFeatureIds );
-  const auto constSelectedFeatures = selectedFeatures;
-  for ( QgsFeatureId fid : constSelectedFeatures )
+  for ( QgsFeatureId fid : std::as_const( selectedFeatures ) )
   {
     deleted += deleteFeature( fid, context );  // removes from selection
   }
@@ -3384,7 +3383,6 @@ bool QgsVectorLayer::deleteFeature( QgsFeatureId fid, QgsVectorLayer::DeleteCont
 
   if ( res )
   {
-    mSelectedFeatureIds.remove( fid ); // remove it from selection
     updateExtents();
   }
 
@@ -3821,6 +3819,13 @@ void QgsVectorLayer::endEditCommand()
   mEditCommandActive = false;
   if ( !mDeletedFids.isEmpty() )
   {
+    if ( selectedFeatureCount() > 0 )
+    {
+      for ( const auto deletedFid : std::as_const( mDeletedFids ) )
+      {
+        mSelectedFeatureIds.remove( deletedFid );
+      }
+    }
     emit featuresDeleted( mDeletedFids );
     mDeletedFids.clear();
   }
@@ -5276,9 +5281,14 @@ void QgsVectorLayer::onJoinedFieldsChanged()
 void QgsVectorLayer::onFeatureDeleted( QgsFeatureId fid )
 {
   if ( mEditCommandActive )
+  {
     mDeletedFids << fid;
+  }
   else
+  {
+    mSelectedFeatureIds.remove( fid );
     emit featuresDeleted( QgsFeatureIds() << fid );
+  }
 
   emit featureDeleted( fid );
 }

diff --git a/tests/src/python/test_qgsvectorlayer.py b/tests/src/python/test_qgsvectorlayer.py
@@ -3632,6 +3632,35 @@ def testTransformContextIsSyncedFromProject(self):
         self.assertTrue(p.transformContext().hasTransform(QgsCoordinateReferenceSystem.fromEpsgId(4326), QgsCoordinateReferenceSystem.fromEpsgId(3857)))
         self.assertTrue(vl.transformContext().hasTransform(QgsCoordinateReferenceSystem.fromEpsgId(4326), QgsCoordinateReferenceSystem.fromEpsgId(3857)))
 
+    def testDeletedFeaturesAreNotSelected(self):
+        """Test that when features are deleted are also removed from selected before
+           featuresDeleted is emitted"""
+
+        layer = QgsVectorLayer("point?crs=epsg:4326&field=id:integer", "Scratch point layer", "memory")
+        layer.startEditing()
+        layer.addFeature(QgsFeature(layer.fields()))
+        layer.commitChanges()
+
+        self.assertEqual(layer.featureCount(), 1)
+
+        test_errors = []
+
+        def onFeaturesDeleted(deleted_fids):
+            selected = layer.selectedFeatureIds()
+            for fid in selected:
+                test_errors.append(f'Feature with id {fid} was deleted but is still selected')
+
+        layer.featuresDeleted.connect(onFeaturesDeleted)
+
+        layer.startEditing()
+        layer.selectAll()
+        layer.deleteSelectedFeatures()
+        layer.commitChanges()
+
+        self.assertEqual(test_errors, [], test_errors)
+        self.assertEqual(layer.featureCount(), 0)
+        self.assertEqual(layer.selectedFeatureIds(), [])
+
     def testSubsetStringInvalidLayer(self):
         """
         Test that subset strings can be set on invalid layers, and retrieved later...