[ogr][transactions] Fix transaction group

Fix #58845

src/core/providers/ogr/qgsogrprovidermetadata.cpp

@@ -660,7 +660,7 @@ QgsTransaction *QgsOgrProviderMetadata::createTransaction( const QString &connSt
   auto ds = QgsOgrProviderUtils::getAlreadyOpenedDataset( connString );
   if ( !ds )
   {
-    QgsMessageLog::logMessage( QObject::tr( "Cannot open transaction on %1, since it is is not currently opened" ).arg( connString ),
+    QgsMessageLog::logMessage( QObject::tr( "Cannot open transaction on %1, since it is not currently opened" ).arg( connString ),
                                QObject::tr( "OGR" ), Qgis::MessageLevel::Critical );
     return nullptr;
   }

src/core/providers/ogr/qgsogrproviderutils.cpp

@@ -2276,7 +2276,7 @@ QgsOgrLayerUniquePtr QgsOgrProviderUtils::getLayer( const QString &dsName,
 
   if ( checkModificationDateAgainstCache && !canUseOpenedDatasets( dsName ) )
   {
-    QgsDebugMsgLevel( QStringLiteral( "Cannot reuse existing opened dataset(s) on %1 since it has been modified" ).arg( dsName ), 2 );
+    QgsDebugMsgLevel( QStringLiteral( "Cannot reuse existing opened dataset(s) %1 on %2 since it has been modified" ).arg( layerName, dsName ), 2 );
     invalidateCachedDatasets( dsName );
   }
 

src/core/qgstransactiongroup.cpp

@@ -81,13 +81,22 @@ void QgsTransactionGroup::onEditingStarted()
   QString errorMsg;
   mTransaction->begin( errorMsg );
 
+  const auto triggeringLayer = qobject_cast<QgsVectorLayer *>( sender() );
+
   const auto constMLayers = mLayers;
   for ( QgsVectorLayer *layer : constMLayers )
   {
     mTransaction->addLayer( layer, true );
-    layer->startEditing();
+
+    // Do not start editing the triggering layer, it will be started by the caller
+    if ( layer != triggeringLayer )
+    {
+      layer->startEditing();
+    }
+
     connect( layer, &QgsVectorLayer::beforeCommitChanges, this, &QgsTransactionGroup::onBeforeCommitChanges );
     connect( layer, &QgsVectorLayer::beforeRollBack, this, &QgsTransactionGroup::onRollback );
+
   }
 }
 

src/core/vector/qgsvectorlayer.cpp

@@ -3764,7 +3764,11 @@ bool QgsVectorLayer::commitChanges( bool stopEditing )
   updateFields();
 
   mDataProvider->updateExtents();
-  mDataProvider->leaveUpdateMode();
+
+  if ( stopEditing )
+  {
+    mDataProvider->leaveUpdateMode();
+  }
 
   // This second call is required because OGR provider with JSON
   // driver might have changed fields order after the call to

src/core/vector/qgsvectorlayereditbuffergroup.cpp

@@ -455,7 +455,11 @@ void QgsVectorLayerEditBufferGroup::editingFinished( bool stopEditing )
     layer->updateFields();
 
     layer->dataProvider()->updateExtents();
-    layer->dataProvider()->leaveUpdateMode();
+
+    if ( stopEditing )
+    {
+      layer->dataProvider()->leaveUpdateMode();
+    }
 
     // This second call is required because OGR provider with JSON
     // driver might have changed fields order after the call to

src/providers/spatialite/qgsspatialiteprovider.cpp

@@ -6549,7 +6549,7 @@ QgsTransaction *QgsSpatiaLiteProviderMetadata::createTransaction( const QString
   QgsSqliteHandle *ds { QgsSqliteHandle::openDb( dsUri.database() ) };
   if ( !ds )
   {
-    QgsMessageLog::logMessage( QObject::tr( "Cannot open transaction on %1, since it is is not currently opened" ).arg( connString ),
+    QgsMessageLog::logMessage( QObject::tr( "Cannot open transaction on %1, since it is not currently opened" ).arg( connString ),
                                QObject::tr( "spatialite" ), Qgis::MessageLevel::Critical );
     return nullptr;
   }

tests/src/python/test_provider_ogr_gpkg.py

@@ -39,6 +39,8 @@
     QgsCoordinateReferenceSystem,
     QgsDataProvider,
     QgsFeature,
+    QgsRelation,
+    QgsRelationContext,
     QgsFeatureRequest,
     QgsFeatureSink,
     QgsField,
@@ -1746,6 +1748,59 @@ def testTransaction(self):
         self.assertEqual(len([f for f in vl2_external.getFeatures(QgsFeatureRequest())]), 1)
         del vl2_external
 
+    def testTransactionGroupAutomatic(self):
+        """Test for issue #58845"""
+
+        temp_dir = QTemporaryDir()
+        temp_path = temp_dir.path()
+        tmpfile = os.path.join(temp_path, 'testTransactionGroupAutomatic.gpkg')
+        ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile)
+        lyr = ds.CreateLayer('types', geom_type=ogr.wkbNone)
+        lyr.CreateField(ogr.FieldDefn('name', ogr.OFTString))
+        lyr = ds.CreateLayer('shops', geom_type=ogr.wkbPoint)
+        lyr.CreateField(ogr.FieldDefn('name', ogr.OFTString))
+        lyr.CreateField(ogr.FieldDefn('type_fk', ogr.OFTString))
+        ds = None
+
+        type_layer = QgsVectorLayer(f'{tmpfile}' + "|layername=" + "types", 'types', 'ogr')
+        self.assertTrue(type_layer.isValid())
+        shops_layer = QgsVectorLayer(f'{tmpfile}' + "|layername=" + "shops", 'shops', 'ogr')
+        self.assertTrue(shops_layer.isValid())
+
+        # prepare a project with transactions enabled
+        p = QgsProject()
+        p.setTransactionMode(Qgis.TransactionMode.AutomaticGroups)
+        p.addMapLayers([type_layer, shops_layer])
+
+        # Add one to many relation
+        relation = QgsRelation(QgsRelationContext(p))
+        relation.setName('shops_types')
+        relation.setId('shops_types')
+        relation.setReferencingLayer(shops_layer.id())
+        relation.setReferencedLayer(type_layer.id())
+        relation.addFieldPair('type_fk', 'name')
+        self.assertTrue(relation.isValid(), relation.validationError())
+
+        p.relationManager().addRelation(relation)
+
+        self.assertTrue(shops_layer.startEditing())
+        self.assertTrue(shops_layer.isEditable())
+        self.assertTrue(type_layer.isEditable())
+        f = QgsFeature(shops_layer.fields())
+        f['name'] = 'shop1'
+        self.assertTrue(shops_layer.addFeature(f))
+
+        self.assertTrue(p.commitChanges(True, shops_layer))
+        self.assertFalse(shops_layer.isEditable())
+        self.assertFalse(type_layer.isEditable())
+        self.assertTrue(shops_layer.startEditing())
+        self.assertTrue(shops_layer.isEditable())
+        self.assertTrue(type_layer.isEditable())
+
+        # Verify stored data
+        self.assertEqual(len([f for f in shops_layer.getFeatures()]), 1)
+        self.assertTrue(p.rollBack(True, shops_layer))
+
     def testJson(self):
         tmpfile = os.path.join(self.basetestpath, 'test_json.gpkg')
         testdata_path = unitTestDataPath('provider')
@@ -2250,6 +2305,8 @@ def testTransactionGroup(self):
         self.assertFalse(vl1_1.isEditable())
         self.assertFalse(vl1_2.isEditable())
 
+        self.assertTrue(vl2_1.rollBack())
+
     def testTransactionGroupIterator(self):
         """Test issue GH #39178: the bug is that this test hangs
         forever in an endless loop"""
@@ -2317,6 +2374,7 @@ def testTransactionGroupCrash(self):
         # Now add another one
         feature.setAttributes([None, 'three'])
         self.assertTrue(vl.addFeature(feature))
+        self.assertTrue(vl.commitChanges(True))
 
     def _testVectorLayerExporterDeferredSpatialIndex(self, layerOptions, expectSpatialIndex):
         """ Internal method """