From dffda6f55b7924953f9154d55b1230d9ef354c4c Mon Sep 17 00:00:00 2001
From: Roni Lindholm <roni.lindholm@gmail.com>
Date: Mon, 11 Mar 2024 08:42:43 +0200
Subject: [PATCH] Fix merge features with hidden fields

Instead of ignoring always hidden fields, use field default value or value
from one of merged features. Fixes #28253

Co-authored-by: Joonalai <joona.laine@cofactor.fi>
---
 src/app/qgsmergeattributesdialog.cpp          | 17 ++---
 .../src/app/testqgsmergeattributesdialog.cpp  | 68 +++++++++++++++++++
 2 files changed, 77 insertions(+), 8 deletions(-)

diff --git a/src/app/qgsmergeattributesdialog.cpp b/src/app/qgsmergeattributesdialog.cpp
index 18355e717b79..a94cf742f183 100644
--- a/src/app/qgsmergeattributesdialog.cpp
+++ b/src/app/qgsmergeattributesdialog.cpp
@@ -171,6 +171,9 @@ void QgsMergeAttributesDialog::createTableWidgetContents()
     if ( setup.type() == QLatin1String( "Hidden" ) || setup.type() == QLatin1String( "Immutable" ) )
     {
       mHiddenAttributes.insert( idx );
+    }
+    if ( setup.type() == QLatin1String( "Immutable" ) )
+    {
       continue;
     }
 
@@ -183,12 +186,17 @@ void QgsMergeAttributesDialog::createTableWidgetContents()
 
     QComboBox *cb = createMergeComboBox( mFields.at( idx ).type(), col );
     if ( ! mVectorLayer->dataProvider()->pkAttributeIndexes().contains( mFields.fieldOriginIndex( idx ) ) &&
-         mFields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique )
+         mFields.at( idx ).constraints().constraints() & QgsFieldConstraints::ConstraintUnique || mHiddenAttributes.contains( idx ) )
     {
       cb->setCurrentIndex( cb->findData( "skip" ) );
     }
     mTableWidget->setCellWidget( 0, col, cb );
 
+    if ( mHiddenAttributes.contains( idx ) )
+    {
+      mTableWidget->setColumnHidden( idx, col );
+    }
+
     col++;
   }
 
@@ -768,13 +776,6 @@ QgsAttributes QgsMergeAttributesDialog::mergedAttributes() const
   QgsAttributes results( mFields.count() );
   for ( int fieldIdx = 0; fieldIdx < mFields.count(); ++fieldIdx )
   {
-    if ( mHiddenAttributes.contains( fieldIdx ) )
-    {
-      //hidden attribute, set to default value
-      results[fieldIdx] = QVariant();
-      continue;
-    }
-
     QComboBox *comboBox = qobject_cast<QComboBox *>( mTableWidget->cellWidget( 0, widgetIndex ) );
     if ( !comboBox )
       continue;
diff --git a/tests/src/app/testqgsmergeattributesdialog.cpp b/tests/src/app/testqgsmergeattributesdialog.cpp
index f6e9a99ca17d..d847226e67f2 100644
--- a/tests/src/app/testqgsmergeattributesdialog.cpp
+++ b/tests/src/app/testqgsmergeattributesdialog.cpp
@@ -150,6 +150,74 @@ class TestQgsMergeattributesDialog : public QgsTest
       QVERIFY( !dialog.mergedAttributes().at( 0 ).isValid() );
       QCOMPARE( dialog.mergedAttributes().at( 1 ), 22 );
     }
+
+    void testWithHiddenField()
+    {
+      // Create test layer
+      QgsVectorFileWriter::SaveVectorOptions options;
+      QgsVectorLayer ml( "LineString", "test", "memory" );
+      QVERIFY( ml.isValid() );
+
+      QgsField notHiddenField( QStringLiteral( "not_hidden" ), QVariant::Int );
+      QgsField hiddenField( QStringLiteral( "hidden" ), QVariant::Int );
+      // hide the field
+      ml.setEditorWidgetSetup( 1, QgsEditorWidgetSetup( QStringLiteral( "Hidden" ), QVariantMap() ) );
+      QVERIFY( ml.dataProvider()->addAttributes( { notHiddenField, hiddenField } ) );
+      ml.updateFields();
+
+      // Create a features
+      QgsFeature f1( ml.fields(), 1 );
+      f1.setAttributes( QVector<QVariant>() << 1 << 2 );
+      f1.setGeometry( QgsGeometry::fromWkt( "LINESTRING(0 0, 10 0)" ) );
+      QVERIFY( ml.dataProvider()->addFeature( f1 ) );
+      QCOMPARE( ml.featureCount(), 1 );
+
+      QgsFeature f2( ml.fields(), 2 );
+      f2.setAttributes( QVector<QVariant>() << 3 << 4 );
+      f2.setGeometry( QgsGeometry::fromWkt( "LINESTRING(10 0, 15 0)" ) );
+      QVERIFY( ml.dataProvider()->addFeature( f2 ) );
+      QCOMPARE( ml.featureCount(), 2 );
+
+      // Merge the attributes together
+      QgsMergeAttributesDialog dialog( QgsFeatureList() << f1 << f2, &ml, mQgisApp->mapCanvas() );
+      QVERIFY( QMetaObject::invokeMethod( &dialog, "mFromLargestPushButton_clicked" ) );
+      QCOMPARE( dialog.mergedAttributes(), QgsAttributes() << 1 << 2 );
+    }
+
+    void testWithHiddenFieldDefaultsToEmpty()
+    {
+      // Create test layer
+      QgsVectorFileWriter::SaveVectorOptions options;
+      QgsVectorLayer ml( "LineString", "test", "memory" );
+      QVERIFY( ml.isValid() );
+
+      QgsField notHiddenField( QStringLiteral( "not_hidden" ), QVariant::Int );
+      QgsField hiddenField( QStringLiteral( "hidden" ), QVariant::Int );
+      QVERIFY( ml.dataProvider()->addAttributes( { notHiddenField, hiddenField } ) );
+      ml.updateFields();
+
+      // hide the field
+      ml.setEditorWidgetSetup( 1, QgsEditorWidgetSetup( QStringLiteral( "Hidden" ), QVariantMap() ) );
+
+
+      // Create a features
+      QgsFeature f1( ml.fields(), 1 );
+      f1.setAttributes( QVector<QVariant>() << 1 << 2 );
+      f1.setGeometry( QgsGeometry::fromWkt( "LINESTRING(0 0, 10 0)" ) );
+      QVERIFY( ml.dataProvider()->addFeature( f1 ) );
+      QCOMPARE( ml.featureCount(), 1 );
+
+      QgsFeature f2( ml.fields(), 2 );
+      f2.setAttributes( QVector<QVariant>() << 3 << 4 );
+      f2.setGeometry( QgsGeometry::fromWkt( "LINESTRING(10 0, 15 0)" ) );
+      QVERIFY( ml.dataProvider()->addFeature( f2 ) );
+      QCOMPARE( ml.featureCount(), 2 );
+
+      // Merge the attributes together
+      QgsMergeAttributesDialog dialog( QgsFeatureList() << f1 << f2, &ml, mQgisApp->mapCanvas() );
+      // QVariant gets turned into default value while saving the layer
+      QCOMPARE( dialog.mergedAttributes(), QgsAttributes() << 1 << QVariant() );
+    }
 };
 
 QGSTEST_MAIN( TestQgsMergeattributesDialog )