Editing buffer passthrough

python/core/auto_generated/vector/qgsvectorlayereditbuffer.sip.in

@@ -208,6 +208,7 @@ Returns ``True`` if the specified feature ID has been deleted but not committed.
 %End
 
 
+
   protected slots:
     void undoIndexChanged( int index );
 
@@ -269,8 +270,6 @@ Emitted after committing an attribute rename
 Constructor for QgsVectorLayerEditBuffer
 %End
 
-    void updateFields( QgsFields &fields );
-
     void updateFeatureGeometry( QgsFeature &f );
 %Docstring
 Update feature with uncommitted geometry updates

src/core/qgstransaction.cpp

@@ -218,7 +218,10 @@ QString QgsTransaction::createSavepoint( QString &error SIP_OUT )
     return QString();
 
   if ( !mLastSavePointIsDirty && !mSavepoints.isEmpty() )
+  {
+    qDebug() << "Returning TOP >>>>> " << mSavepoints.top();
     return mSavepoints.top();
+  }
 
   const QString name( QStringLiteral( "qgis" ) + ( QUuid::createUuid().toString().mid( 1, 24 ).replace( '-', QString() ) ) );
 

src/core/vector/qgsvectorlayer.cpp

@@ -874,7 +874,7 @@ QgsRectangle QgsVectorLayer::extent() const
   }
 
   if ( !mEditBuffer ||
-       ( mEditBuffer->mDeletedFeatureIds.isEmpty() && mEditBuffer->mChangedGeometries.isEmpty() ) ||
+       ( !mDataProvider->transaction() && ( mEditBuffer->deletedFeatureIds().isEmpty() && mEditBuffer->changedGeometries().isEmpty() ) ) ||
        QgsDataSourceUri( mDataProvider->dataSourceUri() ).useEstimatedMetadata() )
   {
     mDataProvider->updateExtents();
@@ -887,9 +887,10 @@ QgsRectangle QgsVectorLayer::extent() const
       rect.combineExtentWith( r );
     }
 
-    if ( mEditBuffer )
+    if ( mEditBuffer && !mDataProvider->transaction() )
     {
-      for ( QgsFeatureMap::const_iterator it = mEditBuffer->mAddedFeatures.constBegin(); it != mEditBuffer->mAddedFeatures.constEnd(); ++it )
+      const auto addedFeatures = mEditBuffer->addedFeatures();
+      for ( QgsFeatureMap::const_iterator it = addedFeatures.constBegin(); it != addedFeatures.constEnd(); ++it )
       {
         if ( it->hasGeometry() )
         {
@@ -3363,13 +3364,13 @@ long QgsVectorLayer::featureCount() const
   if ( ! mDataProvider )
     return -1;
   return mDataProvider->featureCount() +
-         ( mEditBuffer ? mEditBuffer->mAddedFeatures.size() - mEditBuffer->mDeletedFeatureIds.size() : 0 );
+         ( mEditBuffer && ! mDataProvider->transaction() ? mEditBuffer->addedFeatures().size() - mEditBuffer->deletedFeatureIds().size() : 0 );
 }
 
 QgsFeatureSource::FeatureAvailability QgsVectorLayer::hasFeatures() const
 {
-  const QgsFeatureIds deletedFeatures( mEditBuffer ? mEditBuffer->deletedFeatureIds() : QgsFeatureIds() );
+  const QgsFeatureIds deletedFeatures( mEditBuffer && ! mDataProvider->transaction() ? mEditBuffer->deletedFeatureIds() : QgsFeatureIds() );
-  const QgsFeatureMap addedFeatures( mEditBuffer ? mEditBuffer->addedFeatures() : QgsFeatureMap() );
+  const QgsFeatureMap addedFeatures( mEditBuffer && ! mDataProvider->transaction() ? mEditBuffer->addedFeatures() : QgsFeatureMap() );
 
   if ( mEditBuffer && !deletedFeatures.empty() )
   {
@@ -3453,9 +3454,9 @@ bool QgsVectorLayer::rollBack( bool deleteBuffer )
     return false;
   }
 
-  bool rollbackExtent = !mEditBuffer->mDeletedFeatureIds.isEmpty() ||
+  bool rollbackExtent = !mDataProvider->transaction() && ( !mEditBuffer->deletedFeatureIds().isEmpty() ||
-                        !mEditBuffer->mAddedFeatures.isEmpty() ||
+                        !mEditBuffer->addedFeatures().isEmpty() ||
-                        !mEditBuffer->mChangedGeometries.isEmpty();
+                        !mEditBuffer->changedGeometries().isEmpty() );
 
   emit beforeRollBack();
 
@@ -4040,7 +4041,7 @@ QSet<QVariant> QgsVectorLayer::uniqueValues( int index, int limit ) const
     {
       uniqueValues = mDataProvider->uniqueValues( index, limit );
 
-      if ( mEditBuffer )
+      if ( mEditBuffer && ! mDataProvider->transaction() )
       {
         QSet<QString> vals;
         const auto constUniqueValues = uniqueValues;
@@ -4088,10 +4089,11 @@ QSet<QVariant> QgsVectorLayer::uniqueValues( int index, int limit ) const
 
     case QgsFields::OriginEdit:
       // the layer is editable, but in certain cases it can still be avoided going through all features
-      if ( mEditBuffer->mDeletedFeatureIds.isEmpty() &&
+      if ( mDataProvider->transaction() || (
-           mEditBuffer->mAddedFeatures.isEmpty() &&
+             mEditBuffer->deletedFeatureIds().isEmpty() &&
-           !mEditBuffer->mDeletedAttributeIds.contains( index ) &&
+             mEditBuffer->addedFeatures().isEmpty() &&
-           mEditBuffer->mChangedAttributeValues.isEmpty() )
+             !mEditBuffer->deletedAttributeIds().contains( index ) &&
+             mEditBuffer->changedAttributeValues().isEmpty() ) )
       {
         uniqueValues = mDataProvider->uniqueValues( index, limit );
         return uniqueValues;
@@ -4147,7 +4149,7 @@ QStringList QgsVectorLayer::uniqueStringsMatching( int index, const QString &sub
     {
       results = mDataProvider->uniqueStringsMatching( index, substring, limit, feedback );
 
-      if ( mEditBuffer )
+      if ( mEditBuffer && ! mDataProvider->transaction() )
       {
         QgsFeatureMap added = mEditBuffer->addedFeatures();
         QMapIterator< QgsFeatureId, QgsFeature > addedIt( added );
@@ -4186,10 +4188,10 @@ QStringList QgsVectorLayer::uniqueStringsMatching( int index, const QString &sub
 
     case QgsFields::OriginEdit:
       // the layer is editable, but in certain cases it can still be avoided going through all features
-      if ( mEditBuffer->mDeletedFeatureIds.isEmpty() &&
+      if ( mDataProvider->transaction() || ( mEditBuffer->deletedFeatureIds().isEmpty() &&
-           mEditBuffer->mAddedFeatures.isEmpty() &&
+                                             mEditBuffer->addedFeatures().isEmpty() &&
-           !mEditBuffer->mDeletedAttributeIds.contains( index ) &&
+                                             !mEditBuffer->deletedAttributeIds().contains( index ) &&
-           mEditBuffer->mChangedAttributeValues.isEmpty() )
+                                             mEditBuffer->changedAttributeValues().isEmpty() ) )
       {
         return mDataProvider->uniqueStringsMatching( index, substring, limit, feedback );
       }
@@ -4257,7 +4259,7 @@ QVariant QgsVectorLayer::minimumOrMaximumValue( int index, bool minimum ) const
     case QgsFields::OriginProvider: //a provider field
     {
       QVariant val = minimum ? mDataProvider->minimumValue( index ) : mDataProvider->maximumValue( index );
-      if ( mEditBuffer )
+      if ( mEditBuffer && ! mDataProvider->transaction() )
       {
         QgsFeatureMap added = mEditBuffer->addedFeatures();
         QMapIterator< QgsFeatureId, QgsFeature > addedIt( added );
@@ -4290,10 +4292,10 @@ QVariant QgsVectorLayer::minimumOrMaximumValue( int index, bool minimum ) const
     case QgsFields::OriginEdit:
     {
       // the layer is editable, but in certain cases it can still be avoided going through all features
-      if ( mEditBuffer->mDeletedFeatureIds.isEmpty() &&
+      if ( mDataProvider->transaction() || ( mEditBuffer->deletedFeatureIds().isEmpty() &&
-           mEditBuffer->mAddedFeatures.isEmpty() &&
+                                             mEditBuffer->addedFeatures().isEmpty() &&
-           !mEditBuffer->mDeletedAttributeIds.contains( index ) &&
+                                             !mEditBuffer->deletedAttributeIds().contains( index ) &&
-           mEditBuffer->mChangedAttributeValues.isEmpty() )
+                                             mEditBuffer->changedAttributeValues().isEmpty() ) )
       {
         return minimum ? mDataProvider->minimumValue( index ) : mDataProvider->maximumValue( index );
       }

src/core/vector/qgsvectorlayereditbuffer.h

@@ -184,6 +184,13 @@ class CORE_EXPORT QgsVectorLayerEditBuffer : public QObject
      */
     bool isFeatureDeleted( QgsFeatureId id ) const { return mDeletedFeatureIds.contains( id ); }
 
+    /**
+     * Updates \a fields
+     * \note Not available in Python bindings
+     * \since QGIS 3.18
+     */
+    void updateFields( QgsFields &fields ) SIP_SKIP;
+
     //QString dumpEditBuffer();
 
   protected slots:
@@ -236,8 +243,6 @@ class CORE_EXPORT QgsVectorLayerEditBuffer : public QObject
     //! Constructor for QgsVectorLayerEditBuffer
     QgsVectorLayerEditBuffer() = default;
 
-    void updateFields( QgsFields &fields );
-
     //! Update feature with uncommitted geometry updates
     void updateFeatureGeometry( QgsFeature &f );
 
@@ -257,7 +262,6 @@ class CORE_EXPORT QgsVectorLayerEditBuffer : public QObject
 
   protected:
     QgsVectorLayer *L = nullptr;
-    friend class QgsVectorLayer;
 
     friend class QgsVectorLayerUndoCommand;
     friend class QgsVectorLayerUndoCommandAddFeature;

src/core/vector/qgsvectorlayereditpassthrough.cpp

@@ -32,7 +32,7 @@ bool QgsVectorLayerEditPassthrough::isModified() const
 
 bool QgsVectorLayerEditPassthrough::modify( QgsVectorLayerUndoPassthroughCommand *cmd )
 {
-  L->undoStack()->push( cmd ); // push takes owneship -> no need for cmd to be a smart ptr
+  L->undoStack()->push( cmd ); // push takes ownership -> no need for cmd to be a smart ptr
   if ( cmd->hasError() )
     return false;
 
@@ -109,6 +109,11 @@ bool QgsVectorLayerEditPassthrough::renameAttribute( int attr, const QString &ne
 bool QgsVectorLayerEditPassthrough::commitChanges( QStringList & /*commitErrors*/ )
 {
   mModified = false;
+  for ( int i = 0; i < L->undoStack()->count(); ++i )
+  {
+    const QgsVectorLayerUndoPassthroughCommand *cmdPrt = static_cast<const QgsVectorLayerUndoPassthroughCommand *>( L->undoStack()->command( i ) );
+    qDebug() << cmdPrt->text();
+  }
   return true;
 }
 

src/core/vector/qgsvectorlayerfeatureiterator.cpp

@@ -71,12 +71,16 @@ QgsVectorLayerFeatureSource::QgsVectorLayerFeatureSource( const QgsVectorLayer *
     else
     {
 #endif
-      mAddedFeatures = QgsFeatureMap( layer->editBuffer()->addedFeatures() );
+      // If we are inside a transaction the iterator "sees" the current status
-      mChangedGeometries = QgsGeometryMap( layer->editBuffer()->changedGeometries() );
+      if ( layer->dataProvider() && ! layer->dataProvider()->transaction() )
-      mDeletedFeatureIds = QgsFeatureIds( layer->editBuffer()->deletedFeatureIds() );
+      {
-      mChangedAttributeValues = QgsChangedAttributesMap( layer->editBuffer()->changedAttributeValues() );
+        mAddedFeatures = QgsFeatureMap( layer->editBuffer()->addedFeatures() );
-      mAddedAttributes = QList<QgsField>( layer->editBuffer()->addedAttributes() );
+        mChangedGeometries = QgsGeometryMap( layer->editBuffer()->changedGeometries() );
-      mDeletedAttributeIds = QgsAttributeList( layer->editBuffer()->deletedAttributeIds() );
+        mDeletedFeatureIds = QgsFeatureIds( layer->editBuffer()->deletedFeatureIds() );
+        mChangedAttributeValues = QgsChangedAttributesMap( layer->editBuffer()->changedAttributeValues() );
+        mAddedAttributes = QList<QgsField>( layer->editBuffer()->addedAttributes() );
+        mDeletedAttributeIds = QgsAttributeList( layer->editBuffer()->deletedAttributeIds() );
+      }
 #if 0
     }
 #endif

src/core/vector/qgsvectorlayerundopassthroughcommand.cpp

@@ -98,6 +98,10 @@ bool QgsVectorLayerUndoPassthroughCommand::rollBackToSavePoint()
     {
       setError();
     }
+    else
+    {
+      mBuffer->L->dataProvider()->transaction()->dirtyLastSavePoint();
+    }
   }
   return !hasError();
 }
@@ -115,6 +119,7 @@ QgsVectorLayerUndoPassthroughCommandAddFeatures::QgsVectorLayerUndoPassthroughCo
     mInitialFeatures.last().setId( sAddedIdLowWaterMark );
   }
   mFeatures = mInitialFeatures;
+
 }
 
 void QgsVectorLayerUndoPassthroughCommandAddFeatures::undo()
@@ -123,6 +128,7 @@ void QgsVectorLayerUndoPassthroughCommandAddFeatures::undo()
   {
     for ( const QgsFeature &f : qgis::as_const( mFeatures ) )
     {
+      mBuffer->mAddedFeatures.remove( f.id() );
       emit mBuffer->featureDeleted( f.id() );
     }
     mFeatures = mInitialFeatures;
@@ -132,10 +138,12 @@ void QgsVectorLayerUndoPassthroughCommandAddFeatures::undo()
 void QgsVectorLayerUndoPassthroughCommandAddFeatures::redo()
 {
   mFeatures = mInitialFeatures;
-  if ( setSavePoint() && mBuffer->L->dataProvider()->addFeatures( mFeatures ) )
+  mBuffer->L->dataProvider()->clearErrors();
+  if ( setSavePoint() && mBuffer->L->dataProvider()->addFeatures( mFeatures ) && ! mBuffer->L->dataProvider()->hasErrors() )
   {
     for ( const QgsFeature &f : qgis::as_const( mFeatures ) )
     {
+      mBuffer->mAddedFeatures.insert( f.id(), f );
       emit mBuffer->featureAdded( f.id() );
     }
   }
@@ -164,7 +172,8 @@ void QgsVectorLayerUndoPassthroughCommandDeleteFeatures::undo()
 
 void QgsVectorLayerUndoPassthroughCommandDeleteFeatures::redo()
 {
-  if ( setSavePoint() && mBuffer->L->dataProvider()->deleteFeatures( mFids ) )
+  mBuffer->L->dataProvider()->clearErrors();
+  if ( setSavePoint() && mBuffer->L->dataProvider()->deleteFeatures( mFids ) && ! mBuffer->L->dataProvider()->hasErrors() )
   {
     for ( const QgsFeatureId &id : mFids )
     {
@@ -187,9 +196,18 @@ QgsVectorLayerUndoPassthroughCommandChangeGeometry::QgsVectorLayerUndoPassthroug
 
 void QgsVectorLayerUndoPassthroughCommandChangeGeometry::undo()
 {
+
   if ( rollBackToSavePoint() )
   {
-    emit mBuffer->geometryChanged( mFid, mOldGeom );
+    if ( mBuffer->mAddedFeatures.contains( mFid ) )
+    {
+      mBuffer->mAddedFeatures[ mFid ].setGeometry( mOldGeom );
+    }
+    else
+    {
+      // TODO: unless first change?
+      mBuffer->mChangedGeometries[mFid] = mOldGeom;
+    }
   }
 }
 
@@ -197,8 +215,17 @@ void QgsVectorLayerUndoPassthroughCommandChangeGeometry::redo()
 {
   QgsGeometryMap geomMap;
   geomMap.insert( mFid, mNewGeom );
-  if ( setSavePoint() && mBuffer->L->dataProvider()->changeGeometryValues( geomMap ) )
+  mBuffer->L->dataProvider()->clearErrors();
+  if ( setSavePoint() && mBuffer->L->dataProvider()->changeGeometryValues( geomMap ) && ! mBuffer->L->dataProvider()->hasErrors() )
   {
+    if ( mBuffer->mAddedFeatures.contains( mFid ) )
+    {
+      mBuffer->mAddedFeatures[ mFid ].setGeometry( mNewGeom );
+    }
+    else
+    {
+      mBuffer->mChangedGeometries[ mFid ] = mNewGeom;
+    }
     emit mBuffer->geometryChanged( mFid, mNewGeom );
   }
   else
@@ -210,29 +237,98 @@ void QgsVectorLayerUndoPassthroughCommandChangeGeometry::redo()
 QgsVectorLayerUndoPassthroughCommandChangeAttribute::QgsVectorLayerUndoPassthroughCommandChangeAttribute( QgsVectorLayerEditBuffer *buffer, QgsFeatureId fid, int field, const QVariant &newValue )
   : QgsVectorLayerUndoPassthroughCommand( buffer, QObject::tr( "change attribute value" ) )
   , mFid( fid )
-  , mField( field )
+  , mFieldIndex( field )
   , mNewValue( newValue )
   , mOldValue( mBuffer->L->getFeature( mFid ).attribute( field ) )
+  , mFirstChange( true )
 {
+
+  if ( mBuffer->mAddedFeatures.contains( mFid ) )
+  {
+    // work with added feature
+    QgsFeatureMap::const_iterator it = mBuffer->mAddedFeatures.constFind( mFid );
+    Q_ASSERT( it != mBuffer->mAddedFeatures.constEnd() );
+    if ( it.value().attribute( mFieldIndex ).isValid() )
+    {
+      mOldValue = it.value().attribute( mFieldIndex );
+      mFirstChange = false;
+    }
+  }
+  else if ( mBuffer->mChangedAttributeValues.contains( mFid ) && mBuffer->mChangedAttributeValues[mFid].contains( mFieldIndex ) )
+  {
+    mOldValue = mBuffer->mChangedAttributeValues[mFid][mFieldIndex];
+    mFirstChange = false;
+  }
 }
 
 void QgsVectorLayerUndoPassthroughCommandChangeAttribute::undo()
 {
   if ( rollBackToSavePoint() )
   {
-    emit mBuffer->attributeValueChanged( mFid, mField, mOldValue );
+    QVariant original = mOldValue;
+
+    if ( mBuffer->mAddedFeatures.contains( mFid ) )
+    {
+      // added feature
+      QgsFeatureMap::iterator it = mBuffer->mAddedFeatures.find( mFid );
+      Q_ASSERT( it != mBuffer->mAddedFeatures.end() );
+      it.value().setAttribute( mFieldIndex, mOldValue );
+    }
+    else if ( mFirstChange )
+    {
+      // existing feature
+      mBuffer->mChangedAttributeValues[mFid].remove( mFieldIndex );
+      if ( mBuffer->mChangedAttributeValues[mFid].isEmpty() )
+        mBuffer->mChangedAttributeValues.remove( mFid );
+
+      if ( !mOldValue.isValid() )
+      {
+        // get old value from provider
+        QgsFeature tmp;
+        QgsFeatureRequest request;
+        request.setFilterFid( mFid );
+        request.setFlags( QgsFeatureRequest::NoGeometry );
+        request.setSubsetOfAttributes( QgsAttributeList() << mFieldIndex );
+        std::unique_ptr<QgsVectorLayer> layerClone( layer()->clone() );
+        QgsFeatureIterator fi = layerClone->getFeatures( request );
+        if ( fi.nextFeature( tmp ) )
+          original = tmp.attribute( mFieldIndex );
+      }
+    }
+    else
+    {
+      mBuffer->mChangedAttributeValues[mFid][mFieldIndex] = mOldValue;
+    }
+    emit mBuffer->attributeValueChanged( mFid, mFieldIndex, original );
   }
 }
 
 void QgsVectorLayerUndoPassthroughCommandChangeAttribute::redo()
 {
   QgsAttributeMap map;
-  map.insert( mField, mNewValue );
+  map.insert( mFieldIndex, mNewValue );
   QgsChangedAttributesMap attribMap;
   attribMap.insert( mFid, map );
-  if ( setSavePoint() && mBuffer->L->dataProvider()->changeAttributeValues( attribMap ) )
+  mBuffer->L->dataProvider()->clearErrors();
+  if ( setSavePoint() && mBuffer->L->dataProvider()->changeAttributeValues( attribMap ) && ! mBuffer->L->dataProvider()->hasErrors() )
   {
-    emit mBuffer->attributeValueChanged( mFid, mField, mNewValue );
+    // Update existing feature
+    QgsFeatureMap::iterator it = mBuffer->mAddedFeatures.find( mFid );
+    if ( it != mBuffer->mAddedFeatures.end() )
+    {
+      it.value().setAttribute( mFieldIndex, mNewValue );
+    }
+    else
+    {
+      // changed attribute of existing feature
+      if ( !mBuffer->mChangedAttributeValues.contains( mFid ) )
+      {
+        mBuffer->mChangedAttributeValues.insert( mFid, QgsAttributeMap() );
+      }
+
+      mBuffer->mChangedAttributeValues[mFid].insert( mFieldIndex, mNewValue );
+    }
+    emit mBuffer->attributeValueChanged( mFid, mFieldIndex, mNewValue );
   }
   else
   {
@@ -251,7 +347,8 @@ void QgsVectorLayerUndoPassthroughCommandAddAttribute::undo()
   // note that the deleteAttribute here is only necessary to inform the provider that
   // an attribute is removed after the rollBackToSavePoint
   const int attr = mBuffer->L->dataProvider()->fieldNameIndex( mField.name() );
-  if ( mBuffer->L->dataProvider()->deleteAttributes( QgsAttributeIds() << attr ) && rollBackToSavePoint() )
+  mBuffer->L->dataProvider()->clearErrors();
+  if ( mBuffer->L->dataProvider()->deleteAttributes( QgsAttributeIds() << attr ) && rollBackToSavePoint() && ! mBuffer->L->dataProvider()->hasErrors() )
   {
     mBuffer->updateLayerFields();
     emit mBuffer->attributeDeleted( attr );
@@ -264,7 +361,8 @@ void QgsVectorLayerUndoPassthroughCommandAddAttribute::undo()
 
 void QgsVectorLayerUndoPassthroughCommandAddAttribute::redo()
 {
-  if ( setSavePoint() && mBuffer->L->dataProvider()->addAttributes( QList<QgsField>() << mField ) )
+  mBuffer->L->dataProvider()->clearErrors();
+  if ( setSavePoint() && mBuffer->L->dataProvider()->addAttributes( QList<QgsField>() << mField ) && ! mBuffer->L->dataProvider()->hasErrors() )
   {
     mBuffer->updateLayerFields();
     const int attr = mBuffer->L->dataProvider()->fieldNameIndex( mField.name() );
@@ -286,7 +384,8 @@ void QgsVectorLayerUndoPassthroughCommandDeleteAttribute::undo()
 {
   // note that the addAttributes here is only necessary to inform the provider that
   // an attribute is added back after the rollBackToSavePoint
-  if ( mBuffer->L->dataProvider()->addAttributes( QList<QgsField>() << mField )  && rollBackToSavePoint() )
+  mBuffer->L->dataProvider()->clearErrors();
+  if ( mBuffer->L->dataProvider()->addAttributes( QList<QgsField>() << mField )  && rollBackToSavePoint() && ! mBuffer->L->dataProvider()->hasErrors() )
   {
     mBuffer->updateLayerFields();
     const int attr = mBuffer->L->dataProvider()->fieldNameIndex( mField.name() );
@@ -301,7 +400,8 @@ void QgsVectorLayerUndoPassthroughCommandDeleteAttribute::undo()
 void QgsVectorLayerUndoPassthroughCommandDeleteAttribute::redo()
 {
   const int attr = mBuffer->L->dataProvider()->fieldNameIndex( mField.name() );
-  if ( setSavePoint() && mBuffer->L->dataProvider()->deleteAttributes( QgsAttributeIds() << attr ) )
+  mBuffer->L->dataProvider()->clearErrors();
+  if ( setSavePoint() && mBuffer->L->dataProvider()->deleteAttributes( QgsAttributeIds() << attr ) && ! mBuffer->L->dataProvider()->hasErrors() )
   {
     mBuffer->updateLayerFields();
     emit mBuffer->attributeDeleted( attr );
@@ -326,7 +426,8 @@ void QgsVectorLayerUndoPassthroughCommandRenameAttribute::undo()
   // an attribute is renamed after the rollBackToSavePoint
   QgsFieldNameMap map;
   map[ mAttr ] = mOldName;
-  if ( mBuffer->L->dataProvider()->renameAttributes( map ) && rollBackToSavePoint() )
+  mBuffer->L->dataProvider()->clearErrors();
+  if ( mBuffer->L->dataProvider()->renameAttributes( map ) && rollBackToSavePoint() && ! mBuffer->L->dataProvider()->hasErrors() )
   {
     mBuffer->updateLayerFields();
     emit mBuffer->attributeRenamed( mAttr, mOldName );
@@ -341,7 +442,8 @@ void QgsVectorLayerUndoPassthroughCommandRenameAttribute::redo()
 {
   QgsFieldNameMap map;
   map[ mAttr ] = mNewName;
-  if ( setSavePoint() && mBuffer->L->dataProvider()->renameAttributes( map ) )
+  mBuffer->L->dataProvider()->clearErrors();
+  if ( setSavePoint() && mBuffer->L->dataProvider()->renameAttributes( map ) && ! mBuffer->L->dataProvider()->hasErrors() )
   {
     mBuffer->updateLayerFields();
     emit mBuffer->attributeRenamed( mAttr, mNewName );
@@ -427,7 +529,8 @@ void QgsVectorLayerUndoPassthroughCommandChangeAttributes::redo()
 {
   QgsChangedAttributesMap attribMap;
   attribMap.insert( mFid, mNewValues );
-  if ( setSavePoint() && mBuffer->L->dataProvider()->changeAttributeValues( attribMap ) )
+  mBuffer->L->dataProvider()->clearErrors();
+  if ( setSavePoint() && mBuffer->L->dataProvider()->changeAttributeValues( attribMap ) && ! mBuffer->L->dataProvider()->hasErrors() )
   {
     for ( auto it = mNewValues.constBegin(); it != mNewValues.constEnd(); ++it )
     {

src/core/vector/qgsvectorlayerundopassthroughcommand.h

@@ -200,9 +200,10 @@ class CORE_EXPORT QgsVectorLayerUndoPassthroughCommandChangeAttribute: public Qg
 
   private:
     QgsFeatureId mFid;
-    const int mField;
+    const int mFieldIndex;
     const QVariant mNewValue;
-    const QVariant mOldValue;
+    QVariant mOldValue;
+    bool mFirstChange;
 };
 
 /**

tests/src/python/test_qgsvectorlayereditbuffer.py

@@ -10,15 +10,18 @@ __author__ = 'Nyall Dawson'
 __date__ = '15/07/2016'
 __copyright__ = 'Copyright 2016, The QGIS Project'
 
+import os
 import qgis  # NOQA
-
+from qgis.PyQt.QtCore import QVariant, QTemporaryDir
-from qgis.PyQt.QtCore import QVariant
 
 from qgis.core import (QgsVectorLayer,
                        QgsFeature,
+                       QgsProject,
                        QgsGeometry,
                        QgsPointXY,
-                       QgsField)
+                       QgsField,
+                       QgsVectorFileWriter,
+                       QgsCoordinateTransformContext)
 from qgis.testing import start_app, unittest
 
 start_app()
@@ -391,6 +394,56 @@ class TestQgsVectorLayerEditBuffer(unittest.TestCase):
         self.assertEqual(layer.editBuffer().addedAttributes()[0].name(), 'new1')
         self.assertEqual(layer.editBuffer().addedAttributes()[1].name(), 'new2')
 
+    def testTransactionGroup(self):
+        """Test that the buffer works as expected when in transaction"""
+
+        ml = QgsVectorLayer('Point?field=int:integer&crs=epsg:4326', 'test', 'memory')
+        self.assertTrue(ml.isValid())
+
+        d = QTemporaryDir()
+        options = QgsVectorFileWriter.SaveVectorOptions()
+        options.driverName = 'GPKG'
+        options.layerName = 'layer_a'
+        err, _ = QgsVectorFileWriter.writeAsVectorFormatV2(ml, os.path.join(d.path(), 'transaction_test.gpkg'), QgsCoordinateTransformContext(), options)
+
+        self.assertEqual(err, QgsVectorFileWriter.NoError)
+        self.assertTrue(os.path.isfile(os.path.join(d.path(), 'transaction_test.gpkg')))
+
+        options.layerName = 'layer_b'
+        options.actionOnExistingFile = QgsVectorFileWriter.CreateOrOverwriteLayer
+        err, _ = QgsVectorFileWriter.writeAsVectorFormatV2(ml, os.path.join(d.path(), 'transaction_test.gpkg'), QgsCoordinateTransformContext(), options)
+
+        def _test(autoTransaction):
+
+            layer_a = QgsVectorLayer(os.path.join(d.path(), 'transaction_test.gpkg|layername=layer_a'))
+            layer_b = QgsVectorLayer(os.path.join(d.path(), 'transaction_test.gpkg|layername=layer_b'))
+
+            self.assertTrue(layer_a.isValid())
+            self.assertTrue(layer_b.isValid())
+
+            project = QgsProject()
+            project.setAutoTransaction(autoTransaction)
+            project.addMapLayers([layer_a, layer_b])
+
+            self.assertFalse(layer_b.isEditable())
+            self.assertTrue(layer_a.startEditing())
+
+            if not autoTransaction:
+                self.assertTrue(layer_b.startEditing())
+
+            self.assertTrue(layer_b.isEditable())
+
+            f = QgsFeature(layer_a.fields())
+            f.setGeometry(QgsGeometry.fromWkt('point(7 45)'))
+
+            self.assertTrue(layer_a.addFeatures([f]))
+
+            buffer = layer_a.editBuffer()
+            self.assertTrue(len(buffer.addedFeatures()) > 0)
+
+        _test(False)
+        _test(True)
+
 
 if __name__ == '__main__':
     unittest.main()