fix #18954 add&rename vector fields (#8982)

fix #18954 add&rename vector fields at the same time

python/core/auto_generated/qgsfields.sip.in

@@ -54,22 +54,29 @@ Copy constructor
 
     void clear();
 %Docstring
-Remove all fields
+Removes all fields
 %End
 
     bool append( const QgsField &field, FieldOrigin origin = OriginProvider, int originIndex = -1 );
 %Docstring
-Append a field. The field must have unique name, otherwise it is rejected (returns false)
+Appends a field. The field must have unique name, otherwise it is rejected (returns false)
+%End
+
+    bool rename( int fieldIdx, const QString &name );
+%Docstring
+Renames a name of field. The field must have unique name, otherwise change is rejected (returns false)
+
+.. versionadded:: 3.6
 %End
 
     bool appendExpressionField( const QgsField &field, int originIndex );
 %Docstring
-Append an expression field. The field must have unique name, otherwise it is rejected (returns false)
+Appends an expression field. The field must have unique name, otherwise it is rejected (returns false)
 %End
 
     void remove( int fieldIdx );
 %Docstring
-Remove a field with the given index
+Removes a field with the given index
 %End
 %MethodCode
     if ( a0 < 0 || a0 >= sipCpp->count() )
@@ -85,12 +92,12 @@ Remove a field with the given index
 
     void extend( const QgsFields &other );
 %Docstring
-Extend with fields from another QgsFields container
+Extends with fields from another QgsFields container
 %End
 
     bool isEmpty() const;
 %Docstring
-Check whether the container is empty
+Checks whether the container is empty
 %End
 
     int count() const;
@@ -252,7 +259,7 @@ name of the field.
 
     int lookupField( const QString &fieldName ) const;
 %Docstring
-Look up field's index from the field name.
+Looks up field's index from the field name.
 This method matches in the following order:
 
 1. The exact field name taking case sensitivity into account

src/app/qgssourcefieldsproperties.cpp

@@ -145,13 +145,6 @@ void QgsSourceFieldsProperties::attributeAdded( int idx )
   setRow( row, idx, fields.at( idx ) );
   mFieldsList->setCurrentCell( row, idx );
 
-  //in case there are rows following, there is increased the id to the correct ones
-  for ( int i = idx + 1; i < mIndexedWidgets.count(); i++ )
-    mIndexedWidgets.at( i )->setData( Qt::DisplayRole, i );
-
-  if ( sorted )
-    mFieldsList->setSortingEnabled( true );
-
   for ( int i = 0; i < mFieldsList->columnCount(); i++ )
   {
     switch ( mLayer->fields().fieldOrigin( idx ) )
@@ -170,6 +163,9 @@ void QgsSourceFieldsProperties::attributeAdded( int idx )
         break;
     }
   }
+
+  if ( sorted )
+    mFieldsList->setSortingEnabled( true );
 }
 
 
@@ -204,7 +200,11 @@ void QgsSourceFieldsProperties::setRow( int row, int idx, const QgsField &field
   }
   mFieldsList->setItem( row, AttrIdCol, dataItem );
 
+  // in case we insert and not append reindex remaining widgets by 1
+  for ( int i = idx + 1; i < mIndexedWidgets.count(); i++ )
+    mIndexedWidgets.at( i )->setData( Qt::DisplayRole, i );
   mIndexedWidgets.insert( idx, mFieldsList->item( row, 0 ) );
+
   mFieldsList->setItem( row, AttrNameCol, new QTableWidgetItem( field.name() ) );
   mFieldsList->setItem( row, AttrAliasCol, new QTableWidgetItem( field.alias() ) );
   mFieldsList->setItem( row, AttrTypeCol, new QTableWidgetItem( QVariant::typeToName( field.type() ) ) );

src/core/qgsfields.cpp

@@ -69,6 +69,24 @@ bool QgsFields::append( const QgsField &field, FieldOrigin origin, int originInd
   return true;
 }
 
+bool QgsFields::rename( int fieldIdx, const QString &name )
+{
+  if ( !exists( fieldIdx ) )
+    return false;
+
+  if ( name.isEmpty() )
+    return false;
+
+  if ( d->nameToIndex.contains( name ) )
+    return false;
+
+  const QString oldName = d->fields[ fieldIdx ].field.name();
+  d->fields[ fieldIdx ].field.setName( name );
+  d->nameToIndex.remove( oldName );
+  d->nameToIndex.insert( name, fieldIdx );
+  return true;
+}
+
 bool QgsFields::appendExpressionField( const QgsField &field, int originIndex )
 {
   if ( d->nameToIndex.contains( field.name() ) )

src/core/qgsfields.h

@@ -39,7 +39,7 @@ class QgsFieldsPrivate;
  * - keeps track of where the field definition comes from (vector data provider, joined layer or newly added from an editing operation)
  * \note QgsFields objects are implicitly shared.
  */
-class CORE_EXPORT QgsFields
+class CORE_EXPORT  QgsFields
 {
   public:
 
@@ -94,16 +94,22 @@ class CORE_EXPORT QgsFields
 
     virtual ~QgsFields();
 
-    //! Remove all fields
+    //! Removes all fields
     void clear();
 
-    //! Append a field. The field must have unique name, otherwise it is rejected (returns false)
+    //! Appends a field. The field must have unique name, otherwise it is rejected (returns false)
     bool append( const QgsField &field, FieldOrigin origin = OriginProvider, int originIndex = -1 );
 
-    //! Append an expression field. The field must have unique name, otherwise it is rejected (returns false)
+    /**
+     * Renames a name of field. The field must have unique name, otherwise change is rejected (returns false)
+     * \since QGIS 3.6
+     */
+    bool rename( int fieldIdx, const QString &name );
+
+    //! Appends an expression field. The field must have unique name, otherwise it is rejected (returns false)
     bool appendExpressionField( const QgsField &field, int originIndex );
 
-    //! Remove a field with the given index
+    //! Removes a field with the given index
     void remove( int fieldIdx );
 #ifdef SIP_RUN
     % MethodCode
@@ -119,10 +125,10 @@ class CORE_EXPORT QgsFields
     % End
 #endif
 
-    //! Extend with fields from another QgsFields container
+    //! Extends with fields from another QgsFields container
     void extend( const QgsFields &other );
 
-    //! Check whether the container is empty
+    //! Checks whether the container is empty
     bool isEmpty() const;
 
     //! Returns number of items
@@ -282,7 +288,7 @@ class CORE_EXPORT QgsFields
     int indexOf( const QString &fieldName ) const;
 
     /**
-     * Look up field's index from the field name.
+     * Looks up field's index from the field name.
      * This method matches in the following order:
      *
      *  1. The exact field name taking case sensitivity into account

src/core/qgsvectorlayereditbuffer.cpp

@@ -62,16 +62,18 @@ void QgsVectorLayerEditBuffer::updateFields( QgsFields &fields )
   {
     fields.remove( mDeletedAttributeIds.at( i ) );
   }
-  // add new fields
-  for ( int i = 0; i < mAddedAttributes.count(); ++i )
-  {
-    fields.append( mAddedAttributes.at( i ), QgsFields::OriginEdit, i );
-  }
+
   // rename fields
   QgsFieldNameMap::const_iterator renameIt = mRenamedAttributes.constBegin();
   for ( ; renameIt != mRenamedAttributes.constEnd(); ++renameIt )
   {
-    fields[ renameIt.key()].setName( renameIt.value() );
+    fields.rename( renameIt.key(), renameIt.value() );
+  }
+
+  // add new fields
+  for ( int i = 0; i < mAddedAttributes.count(); ++i )
+  {
+    fields.append( mAddedAttributes.at( i ), QgsFields::OriginEdit, i );
   }
 }
 
@@ -413,6 +415,25 @@ bool QgsVectorLayerEditBuffer::commitChanges( QStringList &commitErrors )
     }
   }
 
+  // rename attributes
+  if ( !mRenamedAttributes.isEmpty() )
+  {
+    if ( ( cap & QgsVectorDataProvider::RenameAttributes ) && provider->renameAttributes( mRenamedAttributes ) )
+    {
+      commitErrors << tr( "SUCCESS: %n attribute(s) renamed.", "renamed attributes count", mRenamedAttributes.size() );
+
+      emit committedAttributesRenamed( L->id(), mRenamedAttributes );
+
+      mRenamedAttributes.clear();
+      attributesChanged = true;
+    }
+    else
+    {
+      commitErrors << tr( "ERROR: %n attribute(s) not renamed", "not renamed attributes count", mRenamedAttributes.size() );
+      success = false;
+    }
+  }
+
   //
   // add attributes
   //
@@ -442,25 +463,6 @@ bool QgsVectorLayerEditBuffer::commitChanges( QStringList &commitErrors )
     }
   }
 
-  // rename attributes
-  if ( !mRenamedAttributes.isEmpty() )
-  {
-    if ( ( cap & QgsVectorDataProvider::RenameAttributes ) && provider->renameAttributes( mRenamedAttributes ) )
-    {
-      commitErrors << tr( "SUCCESS: %n attribute(s) renamed.", "renamed attributes count", mRenamedAttributes.size() );
-
-      emit committedAttributesRenamed( L->id(), mRenamedAttributes );
-
-      mRenamedAttributes.clear();
-      attributesChanged = true;
-    }
-    else
-    {
-      commitErrors << tr( "ERROR: %n attribute(s) not renamed", "not renamed attributes count", mRenamedAttributes.size() );
-      success = false;
-    }
-  }
-
   //
   // check that addition/removal went as expected
   //

src/core/qgsvectorlayerundocommand.cpp

@@ -423,18 +423,38 @@ QgsVectorLayerUndoCommandRenameAttribute::QgsVectorLayerUndoCommandRenameAttribu
   , mOldName( layer()->fields().at( fieldIndex ).name() )
   , mNewName( newName )
 {
+  const QgsFields &fields = layer()->fields();
+  QgsFields::FieldOrigin origin = fields.fieldOrigin( mFieldIndex );
+  mOriginIndex = fields.fieldOriginIndex( mFieldIndex );
+  mProviderField = ( origin == QgsFields::OriginProvider );
 }
 
 void QgsVectorLayerUndoCommandRenameAttribute::undo()
 {
-  mBuffer->mRenamedAttributes[ mFieldIndex ] = mOldName;
+  if ( mProviderField )
+  {
+    mBuffer->mRenamedAttributes[ mFieldIndex ] = mOldName;
+  }
+  else
+  {
+    // newly added attribute
+    mBuffer->mAddedAttributes[mOriginIndex].setName( mOldName );
+  }
   mBuffer->updateLayerFields();
   emit mBuffer->attributeRenamed( mFieldIndex, mOldName );
 }
 
 void QgsVectorLayerUndoCommandRenameAttribute::redo()
 {
-  mBuffer->mRenamedAttributes[ mFieldIndex ] = mNewName;
+  if ( mProviderField )
+  {
+    mBuffer->mRenamedAttributes[ mFieldIndex ] = mNewName;
+  }
+  else
+  {
+    // newly added attribute
+    mBuffer->mAddedAttributes[mOriginIndex].setName( mNewName );
+  }
   mBuffer->updateLayerFields();
   emit mBuffer->attributeRenamed( mFieldIndex, mNewName );
 }

src/core/qgsvectorlayerundocommand.h

@@ -253,6 +253,8 @@ class CORE_EXPORT QgsVectorLayerUndoCommandRenameAttribute : public QgsVectorLay
 
   private:
     int mFieldIndex;
+    bool mProviderField;
+    int mOriginIndex;
     QString mOldName;
     QString mNewName;
 };

tests/src/core/testqgsfields.cpp

@@ -39,6 +39,7 @@ class TestQgsFields: public QObject
     void count();
     void isEmpty();
     void remove();
+    void rename();
     void extend();
     void byIndex();
     void byName();
@@ -237,6 +238,25 @@ void TestQgsFields::remove()
   QCOMPARE( fields.indexFromName( "testfield2" ), 0 );
 }
 
+void TestQgsFields::rename()
+{
+  QgsFields fields;
+
+  QVERIFY( !fields.rename( 1, "name" ) );
+
+  QgsField field( QStringLiteral( "testfield" ) );
+  fields.append( field );
+  QVERIFY( !fields.rename( 0, "" ) );
+
+  QgsField field2( QStringLiteral( "testfield2" ) );
+  fields.append( field2 );
+  QVERIFY( !fields.rename( 0, "testfield2" ) );
+
+  QVERIFY( fields.rename( 0, "newname" ) );
+  QCOMPARE( fields.at( 0 ).name(), QString( "newname" ) );
+  QCOMPARE( fields.at( 1 ).name(), QString( "testfield2" ) );
+}
+
 void TestQgsFields::extend()
 {
   QgsFields destination;