Fix crash in merge features dialog when a field has a unique

constraint set Fixes #54856
qgis · Oct 6, 2023 · dd1a0eb · dd1a0eb
1 parent 22299c6
commit dd1a0eb
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 7 deletions.
diff --git a/src/app/qgsmergeattributesdialog.cpp b/src/app/qgsmergeattributesdialog.cpp
@@ -177,6 +177,10 @@ void QgsMergeAttributesDialog::createTableWidgetContents()
     mTableWidget->setColumnCount( col + 1 );
     mFieldToColumnMap[ mFields.at( idx ).name() ] = col;
 
+    QTableWidgetItem *item = new QTableWidgetItem( mFields.at( idx ).name() );
+    item->setData( FieldIndex, idx );
+    mTableWidget->setHorizontalHeaderItem( col, item );
+
     QComboBox *cb = createMergeComboBox( mFields.at( idx ).type(), col );
     if ( ! mVectorLayer->dataProvider()->pkAttributeIndexes().contains( mFields.fieldOriginIndex( idx ) ) &&
          mFields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique )
@@ -185,9 +189,7 @@ void QgsMergeAttributesDialog::createTableWidgetContents()
     }
     mTableWidget->setCellWidget( 0, col, cb );
 
-    QTableWidgetItem *item = new QTableWidgetItem( mFields.at( idx ).name() );
-    item->setData( FieldIndex, idx );
-    mTableWidget->setHorizontalHeaderItem( col++, item );
+    col++;
   }
 
   //insert the attribute values

diff --git a/tests/src/app/testqgsmergeattributesdialog.cpp b/tests/src/app/testqgsmergeattributesdialog.cpp
@@ -21,12 +21,13 @@
 #include "qgsvectorlayer.h"
 #include "qgsmergeattributesdialog.h"
 
-class TestQgsMergeattributesDialog : public QObject
+class TestQgsMergeattributesDialog : public QgsTest
 {
     Q_OBJECT
 
   public:
-    TestQgsMergeattributesDialog() = default;
+    TestQgsMergeattributesDialog() : QgsTest( QStringLiteral( "Merge attributes dialog" ) )
+    {}
 
   private:
     QgisApp *mQgisApp = nullptr;
@@ -86,12 +87,68 @@ class TestQgsMergeattributesDialog : public QObject
 
       // At beginning the first feature of the list is the target
       QCOMPARE( dialog.targetFeatureId(), FID_NULL );
-      QCOMPARE( dialog.mergedAttributes()[0], "Autogenerate" );
+      QCOMPARE( dialog.mergedAttributes().at( 0 ), "Autogenerate" );
 
       // Check after taking feature with largest geometry
       QVERIFY( QMetaObject::invokeMethod( &dialog, "mFromLargestPushButton_clicked" ) );
       QCOMPARE( dialog.targetFeatureId(), f2.id() );
-      QCOMPARE( dialog.mergedAttributes().first(), f2.id() );
+      QCOMPARE( dialog.mergedAttributes().at( 0 ), f2.id() );
+    }
+
+    void testWithUniqueConstraint()
+    {
+      // Create test layer
+      QgsVectorFileWriter::SaveVectorOptions options;
+      QgsVectorLayer ml( "Polygon", "test", "memory" );
+      QVERIFY( ml.isValid() );
+
+      QgsField uniqueField( QStringLiteral( "unique" ), QVariant::Int );
+      QgsFieldConstraints constraints;
+      constraints.setConstraint(
+        QgsFieldConstraints::ConstraintUnique
+      );
+      uniqueField.setConstraints(
+        constraints
+      );
+
+      QgsField notUniqueField( QStringLiteral( "not_unique" ), QVariant::Int );
+      QVERIFY( ml.dataProvider()->addAttributes(
+      { uniqueField, notUniqueField }
+               ) );
+
+      ml.updateFields();
+      QCOMPARE( ml.fields().at( 0 ).constraints().constraints(), QgsFieldConstraints::ConstraintUnique );
+      QCOMPARE( ml.fields().at( 1 ).constraints().constraints(), QgsFieldConstraints::Constraints() );
+
+      // Create a feature
+      QgsFeature f1( ml.fields(), 1 );
+      f1.setAttributes( { 11, 12} );
+      f1.setGeometry( QgsGeometry::fromWkt( "POLYGON((0 0, 5 0, 5 5, 0 5, 0 0))" ) );
+      QVERIFY( ml.dataProvider()->addFeature( f1 ) );
+      QCOMPARE( ml.featureCount(), 1 );
+
+      // And a bigger feature
+      QgsFeature f2( ml.fields(), 2 );
+      f2.setAttributes( { 21, 22} );
+      f2.setGeometry( QgsGeometry::fromWkt( "POLYGON((3 3, 10 3, 10 10, 3 10, 3 3))" ) );
+      QVERIFY( ml.dataProvider()->addFeature( f2 ) );
+      QCOMPARE( ml.featureCount(), 2 );
+
+      // Merge the attributes together
+      QgsMergeAttributesDialog dialog( QgsFeatureList() << f1 << f2, &ml, mQgisApp->mapCanvas() );
+
+      // At beginning the first feature of the list is the target
+      QCOMPARE( dialog.targetFeatureId(), FID_NULL );
+      // the first field has a unique constraint, so it should not have any value copied from the source feature
+      QVERIFY( !dialog.mergedAttributes().at( 0 ).isValid() );
+      QCOMPARE( dialog.mergedAttributes().at( 1 ), 12 );
+
+      // Check after taking feature with largest geometry
+      QVERIFY( QMetaObject::invokeMethod( &dialog, "mFromLargestPushButton_clicked" ) );
+      QCOMPARE( dialog.targetFeatureId(), f2.id() );
+      // the first field has a unique constraint, so it should not have any value copied from the source feature
+      QVERIFY( !dialog.mergedAttributes().at( 0 ).isValid() );
+      QCOMPARE( dialog.mergedAttributes().at( 1 ), 22 );
     }
 };