Skip to content

Commit 3ea1674

Browse files
domi4484m-kuhn
domi4484
authored and
m-kuhn
committedMar 30, 2023
Explicitly set the target feature for merge operation
·
final-3_28_15final-3_28_5
1 parent ae3573b commit 3ea1674

File tree

9 files changed

+148
-83
lines changed

9 files changed

+148
-83
lines changed
 

‎python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,10 +312,11 @@ editing.
312312
.. versionadded:: 3.16
313313
%End
314314

315-
bool mergeFeatures( const QgsFeatureIds &mergeFeatureIds, const QgsAttributes &mergeAttributes, QgsGeometry unionGeometry, QString &errorMessage /Out/ );
315+
bool mergeFeatures( const QgsFeatureId &targetFeatureId, const QgsFeatureIds &mergeFeatureIds, const QgsAttributes &mergeAttributes, const QgsGeometry &unionGeometry, QString &errorMessage /Out/ );
316316
%Docstring
317317
Merge features into a single one.
318318

319+
:param targetFeatureId: id of the target feature
319320
:param mergeFeatureIds: id list of features to merge
320321
:param mergeAttributes: are the resulting attributes in the merged feature
321322
:param unionGeometry: is the resulting geometry of the merged feature

‎src/app/qgisapp.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9609,7 +9609,7 @@ void QgisApp::mergeSelectedFeatures()
96099609

96109610
QString errorMessage;
96119611
QgsVectorLayerEditUtils vectorLayerEditUtils( vl );
9612-
bool success = vectorLayerEditUtils.mergeFeatures( vl->selectedFeatureIds(), d.mergedAttributes(), unionGeom, errorMessage );
9612+
bool success = vectorLayerEditUtils.mergeFeatures( d.targetFeatureId(), vl->selectedFeatureIds(), d.mergedAttributes(), unionGeom, errorMessage );
96139613

96149614
if ( !success )
96159615
{

‎src/app/qgsmergeattributesdialog.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ QgsMergeAttributesDialog::QgsMergeAttributesDialog( const QgsFeatureList &featur
108108
break;
109109
}
110110

111+
if ( ! mFeatureList.isEmpty() )
112+
mTargetFeatureId = mFeatureList.first().id();
113+
111114
connect( mSkipAllButton, &QAbstractButton::clicked, this, &QgsMergeAttributesDialog::setAllToSkip );
112115
connect( mTableWidget, &QTableWidget::cellChanged, this, &QgsMergeAttributesDialog::tableWidgetCellChanged );
113116

@@ -465,6 +468,8 @@ void QgsMergeAttributesDialog::setAllAttributesFromFeature( QgsFeatureId feature
465468
currentComboBox->setCurrentIndex( currentComboBox->findData( QStringLiteral( "f%1" ).arg( FID_TO_STRING( featureId ) ) ) );
466469
}
467470
}
471+
472+
mTargetFeatureId = featureId;
468473
}
469474

470475
QVariant QgsMergeAttributesDialog::calcStatistic( int col, QgsStatisticalSummary::Statistic stat )
@@ -666,12 +671,18 @@ void QgsMergeAttributesDialog::mRemoveFeatureFromSelectionButton_clicked()
666671
f_it != mFeatureList.end();
667672
++f_it )
668673
{
674+
if ( f_it->id() == mTargetFeatureId )
675+
mTargetFeatureId = FID_NULL;
676+
669677
if ( f_it->id() == featureId )
670678
{
671679
mFeatureList.erase( f_it );
672680
break;
673681
}
674682
}
683+
684+
if ( mTargetFeatureId == FID_NULL && !mFeatureList.isEmpty() )
685+
mTargetFeatureId = mFeatureList.first().id();
675686
}
676687

677688
void QgsMergeAttributesDialog::tableWidgetCellChanged( int row, int column )
@@ -745,6 +756,11 @@ QgsAttributes QgsMergeAttributesDialog::mergedAttributes() const
745756
return results;
746757
}
747758

759+
QgsFeatureId QgsMergeAttributesDialog::targetFeatureId() const
760+
{
761+
return mTargetFeatureId;
762+
}
763+
748764
QSet<int> QgsMergeAttributesDialog::skippedAttributeIndexes() const
749765
{
750766
QSet<int> skipped;

‎src/app/qgsmergeattributesdialog.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,18 @@ class APP_EXPORT QgsMergeAttributesDialog: public QDialog, private Ui::QgsMergeA
4949

5050
QgsAttributes mergedAttributes() const;
5151

52+
/**
53+
* Returns the id of the target feature.
54+
* By default it is the first feature of the list. Otherwise the feature explicitly selected
55+
* with buttons "Take attributes from selected feature" or "Take attributes from feature with
56+
* the largest area".
57+
*
58+
* \returns The id of the target feature.
59+
*
60+
* \since QGIS 3.30
61+
*/
62+
QgsFeatureId targetFeatureId() const;
63+
5264
/**
5365
* Returns a list of attribute indexes which should be skipped when merging (e.g., attributes
5466
* which have been set to "skip"
@@ -102,6 +114,7 @@ class APP_EXPORT QgsMergeAttributesDialog: public QDialog, private Ui::QgsMergeA
102114
void createRubberBandForFeature( QgsFeatureId featureId );
103115

104116
QgsFeatureList mFeatureList;
117+
QgsFeatureId mTargetFeatureId = FID_NULL;
105118
QgsVectorLayer *mVectorLayer = nullptr;
106119
QgsMapCanvas *mMapCanvas = nullptr;
107120
//! Item that highlights the selected feature in the merge table

‎src/core/vector/qgsvectorlayereditutils.cpp

Lines changed: 12 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -721,18 +721,17 @@ int QgsVectorLayerEditUtils::addTopologicalPoints( const QgsPointXY &p )
721721
return addTopologicalPoints( QgsPoint( p ) );
722722
}
723723

724-
bool QgsVectorLayerEditUtils::mergeFeatures( const QgsFeatureIds &mergeFeatureIds, const QgsAttributes &mergeAttributes, QgsGeometry unionGeometry, QString &errorMessage )
724+
bool QgsVectorLayerEditUtils::mergeFeatures( const QgsFeatureId &targetFeatureId, const QgsFeatureIds &mergeFeatureIds, const QgsAttributes &mergeAttributes, const QgsGeometry &unionGeometry, QString &errorMessage )
725725
{
726726
errorMessage.clear();
727727

728-
if ( mergeFeatureIds.size() < 2 )
728+
if ( mergeFeatureIds.isEmpty() )
729729
{
730-
errorMessage = QObject::tr( "Not enough features to merge, the merge tool requires at least two features" );
730+
errorMessage = QObject::tr( "List of features to merge is empty" );
731731
return false;
732732
}
733733

734734
QgsAttributeMap newAttributes;
735-
QgsFeatureId mergeFeatureId = FID_NULL;
736735
for ( int i = 0; i < mergeAttributes.count(); ++i )
737736
{
738737
QVariant val = mergeAttributes.at( i );
@@ -741,48 +740,6 @@ bool QgsVectorLayerEditUtils::mergeFeatures( const QgsFeatureIds &mergeFeatureId
741740
mLayer->dataProvider() &&
742741
mLayer->dataProvider()->defaultValueClause( mLayer->fields().fieldOriginIndex( i ) ) == val;
743742

744-
// Check if features can merged into an existing one
745-
if ( mergeFeatureId == FID_NULL )
746-
{
747-
bool isPrimaryKey = mLayer->fields().fieldOrigin( i ) == QgsFields::OriginProvider &&
748-
mLayer->dataProvider() &&
749-
mLayer->dataProvider()->pkAttributeIndexes().contains( mLayer->fields().fieldOriginIndex( i ) );
750-
751-
if ( isPrimaryKey && !isDefaultValue )
752-
{
753-
QgsFeatureRequest request;
754-
request.setFlags( QgsFeatureRequest::Flag::NoGeometry );
755-
// Handle multi pks
756-
if ( mLayer->dataProvider()->pkAttributeIndexes().count() > 1 && mLayer->dataProvider()->pkAttributeIndexes().count() <= mergeAttributes.count() )
757-
{
758-
const auto pkIdxList { mLayer->dataProvider()->pkAttributeIndexes() };
759-
QStringList conditions;
760-
QStringList fieldNames;
761-
for ( const int &pkIdx : std::as_const( pkIdxList ) )
762-
{
763-
const QgsField pkField { mLayer->fields().field( pkIdx ) };
764-
conditions.push_back( QgsExpression::createFieldEqualityExpression( pkField.name(), mergeAttributes.at( pkIdx ), pkField.type( ) ) );
765-
fieldNames.push_back( pkField.name() );
766-
}
767-
request.setSubsetOfAttributes( fieldNames, mLayer->fields( ) );
768-
request.setFilterExpression( conditions.join( QLatin1String( " AND " ) ) );
769-
}
770-
else // single pk
771-
{
772-
const QgsField pkField { mLayer->fields().field( i ) };
773-
request.setSubsetOfAttributes( QStringList() << pkField.name(), mLayer->fields( ) );
774-
request.setFilterExpression( QgsExpression::createFieldEqualityExpression( pkField.name(), val, pkField.type( ) ) );
775-
}
776-
777-
QgsFeature f;
778-
QgsFeatureIterator featureIterator = mLayer->getFeatures( request );
779-
if ( featureIterator.nextFeature( f ) )
780-
{
781-
mergeFeatureId = f.id( );
782-
}
783-
}
784-
}
785-
786743
// convert to destination data type
787744
QString errorMessageConvertCompatible;
788745
if ( !isDefaultValue && !mLayer->fields().at( i ).convertCompatible( val, &errorMessageConvertCompatible ) )
@@ -795,38 +752,18 @@ bool QgsVectorLayerEditUtils::mergeFeatures( const QgsFeatureIds &mergeFeatureId
795752

796753
mLayer->beginEditCommand( QObject::tr( "Merged features" ) );
797754

798-
QgsFeatureIds featureIdsToDelete = mergeFeatureIds;
799-
800-
QgsFeature mergeFeature;
801-
if ( mergeFeatureId == FID_NULL )
802-
{
803-
// Create new feature
804-
mergeFeature = QgsVectorLayerUtils::createFeature( mLayer, unionGeometry, newAttributes );
805-
}
806-
else
807-
{
808-
// Merge into existing feature
809-
featureIdsToDelete.remove( mergeFeatureId );
810-
}
811-
812-
// Delete other features
813-
QgsFeatureIds::const_iterator feature_it = featureIdsToDelete.constBegin();
814-
for ( ; feature_it != featureIdsToDelete.constEnd(); ++feature_it )
755+
// Delete other features but the target feature
756+
QgsFeatureIds::const_iterator feature_it = mergeFeatureIds.constBegin();
757+
for ( ; feature_it != mergeFeatureIds.constEnd(); ++feature_it )
815758
{
816-
mLayer->deleteFeature( *feature_it );
759+
if ( *feature_it != targetFeatureId )
760+
mLayer->deleteFeature( *feature_it );
817761
}
818762

819-
if ( mergeFeatureId == FID_NULL )
820-
{
821-
// Add the new feature
822-
mLayer->addFeature( mergeFeature );
823-
}
824-
else
825-
{
826-
// Modify merge feature
827-
mLayer->changeGeometry( mergeFeatureId, unionGeometry );
828-
mLayer->changeAttributeValues( mergeFeatureId, newAttributes );
829-
}
763+
// Modify merge feature
764+
QgsGeometry mergeGeometry = unionGeometry;
765+
mLayer->changeGeometry( targetFeatureId, mergeGeometry );
766+
mLayer->changeAttributeValues( targetFeatureId, newAttributes );
830767

831768
mLayer->endEditCommand();
832769

‎src/core/vector/qgsvectorlayereditutils.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ class CORE_EXPORT QgsVectorLayerEditUtils
270270

271271
/**
272272
* Merge features into a single one.
273+
* \param targetFeatureId id of the target feature
273274
* \param mergeFeatureIds id list of features to merge
274275
* \param mergeAttributes are the resulting attributes in the merged feature
275276
* \param unionGeometry is the resulting geometry of the merged feature
@@ -279,7 +280,7 @@ class CORE_EXPORT QgsVectorLayerEditUtils
279280
*
280281
* \since QGIS 3.30
281282
*/
282-
bool mergeFeatures( const QgsFeatureIds &mergeFeatureIds, const QgsAttributes &mergeAttributes, QgsGeometry unionGeometry, QString &errorMessage SIP_OUT );
283+
bool mergeFeatures( const QgsFeatureId &targetFeatureId, const QgsFeatureIds &mergeFeatureIds, const QgsAttributes &mergeAttributes, const QgsGeometry &unionGeometry, QString &errorMessage SIP_OUT );
283284

284285
private:
285286

‎tests/src/app/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ set(TESTS
4646
testqgsmeshcalculatordialog.cpp
4747
testqgsgpsinformationwidget.cpp
4848
testqgslabelpropertydialog.cpp
49+
testqgsmergeattributesdialog.cpp
4950
)
5051

5152
if (HAVE_GEOREFERENCER)

‎tests/src/app/testqgsmergeattributesdialog.cpp

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/***************************************************************************
2+
testqgsmergeattributesdialog.cpp
3+
--------------------------------
4+
Date : Feb 2023
5+
Copyright : (C) 2023 by Damiano Lombardi
6+
Email : damiano at opengis dot ch
7+
***************************************************************************
8+
* *
9+
* This program is free software; you can redistribute it and/or modify *
10+
* it under the terms of the GNU General Public License as published by *
11+
* the Free Software Foundation; either version 2 of the License, or *
12+
* (at your option) any later version. *
13+
* *
14+
***************************************************************************/
15+
16+
#include "qgstest.h"
17+
#include <QObject>
18+
19+
#include "qgisapp.h"
20+
#include "qgsapplication.h"
21+
#include "qgsvectorlayer.h"
22+
#include "qgsmergeattributesdialog.h"
23+
24+
class TestQgsMergeattributesDialog : public QObject
25+
{
26+
Q_OBJECT
27+
28+
public:
29+
TestQgsMergeattributesDialog() = default;
30+
31+
private:
32+
QgisApp *mQgisApp = nullptr;
33+
34+
private slots:
35+
36+
void initTestCase()
37+
{
38+
QgsApplication::init();
39+
QgsApplication::initQgis();
40+
mQgisApp = new QgisApp();
41+
}
42+
43+
void cleanupTestCase()
44+
{
45+
QgsApplication::exitQgis();
46+
}
47+
48+
void test()
49+
{
50+
// Create test layer
51+
QgsVectorFileWriter::SaveVectorOptions options;
52+
QgsVectorLayer ml( "Polygon", "test", "memory" );
53+
QVERIFY( ml.isValid() );
54+
QTemporaryFile tmpFile( QDir::tempPath() + "/TestQgsMergeattributesDialog" );
55+
tmpFile.open();
56+
const QString fileName( tmpFile.fileName( ) );
57+
options.driverName = "GPKG";
58+
options.layerName = "test";
59+
QString newFilename;
60+
const QgsVectorFileWriter::WriterError error( QgsVectorFileWriter::writeAsVectorFormatV3(
61+
&ml,
62+
fileName,
63+
ml.transformContext(),
64+
options, nullptr,
65+
&newFilename ) );
66+
67+
QCOMPARE( error, QgsVectorFileWriter::WriterError::NoError );
68+
QgsVectorLayer layer( QStringLiteral( "%1|layername=test" ).arg( newFilename ), "src_test", "ogr" );
69+
QVERIFY( layer.startEditing() );
70+
QgsVectorDataProvider *pr = layer.dataProvider();
71+
72+
// Create a feature
73+
QgsFeature f1( layer.fields(), 1 );
74+
f1.setGeometry( QgsGeometry::fromWkt( "POLYGON((0 0, 5 0, 5 5, 0 5, 0 0))" ) );
75+
QVERIFY( pr->addFeature( f1 ) );
76+
QCOMPARE( layer.featureCount(), 1 );
77+
78+
// And a bigger feature
79+
QgsFeature f2( layer.fields(), 2 );
80+
f2.setGeometry( QgsGeometry::fromWkt( "POLYGON((3 3, 10 3, 10 10, 3 10, 3 3))" ) );
81+
QVERIFY( pr->addFeature( f2 ) );
82+
QCOMPARE( layer.featureCount(), 2 );
83+
84+
// Merge the attributes together
85+
QgsMergeAttributesDialog dialog( QgsFeatureList() << f1 << f2, &layer, mQgisApp->mapCanvas() );
86+
87+
// At beginnning the first feature of the list is the target
88+
QCOMPARE( dialog.targetFeatureId(), f1.id() );
89+
90+
// Check after taking feature with largest geometry
91+
QVERIFY( QMetaObject::invokeMethod( &dialog, "mFromLargestPushButton_clicked" ) );
92+
QCOMPARE( dialog.targetFeatureId(), f2.id() );
93+
}
94+
};
95+
96+
QGSTEST_MAIN( TestQgsMergeattributesDialog )
97+
#include "testqgsmergeattributesdialog.moc"

‎tests/src/python/test_qgsvectorlayereditutils.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -494,13 +494,13 @@ def testMergeFeatures(self):
494494
self.assertTrue(layer.startEditing())
495495
layer.addAttribute(QgsField('name', QVariant.String))
496496
pr = layer.dataProvider()
497+
497498
f1 = QgsFeature(layer.fields(), 1)
498499
f1.setGeometry(QgsGeometry.fromWkt('POLYGON((0 0, 5 0, 5 5, 0 5, 0 0))'))
499500
f1.setAttribute('name', 'uno')
500501
assert pr.addFeatures([f1])
501502
self.assertEqual(layer.featureCount(), 1)
502503

503-
pr = layer.dataProvider()
504504
f2 = QgsFeature(layer.fields(), 2)
505505
f2.setGeometry(QgsGeometry.fromWkt('POLYGON((3 3, 8 3, 8 8, 3 8, 3 3))'))
506506
f2.setAttribute('name', 'due')
@@ -514,7 +514,7 @@ def testMergeFeatures(self):
514514
self.assertFalse(unionGeom.isNull())
515515

516516
vle = QgsVectorLayerEditUtils(layer)
517-
success, errorMessage = vle.mergeFeatures(featureIds, ['tre'], unionGeom)
517+
success, errorMessage = vle.mergeFeatures(f1.id(), featureIds, ['tre'], unionGeom)
518518
self.assertFalse(errorMessage)
519519
self.assertTrue(success)
520520

@@ -545,16 +545,15 @@ def testMergeFeaturesIntoExisting(self):
545545
options=options)
546546

547547
layer = QgsVectorLayer(tempgpkg, 'my_layer', 'ogr')
548-
549548
self.assertTrue(layer.startEditing())
550549
pr = layer.dataProvider()
550+
551551
f1 = QgsFeature(layer.fields(), 1)
552552
f1.setGeometry(QgsGeometry.fromWkt('POLYGON((0 0, 5 0, 5 5, 0 5, 0 0))'))
553553
f1.setAttribute('name', 'uno')
554554
assert pr.addFeatures([f1])
555555
self.assertEqual(layer.featureCount(), 1)
556556

557-
pr = layer.dataProvider()
558557
f2 = QgsFeature(layer.fields(), 2)
559558
f2.setGeometry(QgsGeometry.fromWkt('POLYGON((3 3, 8 3, 8 8, 3 8, 3 3))'))
560559
f2.setAttribute('name', 'due')
@@ -568,7 +567,7 @@ def testMergeFeaturesIntoExisting(self):
568567
self.assertFalse(unionGeom.isNull())
569568

570569
vle = QgsVectorLayerEditUtils(layer)
571-
success, errorMessage = vle.mergeFeatures(featureIds, [2, 'tre'], unionGeom)
570+
success, errorMessage = vle.mergeFeatures(f2.id(), featureIds, [2, 'tre'], unionGeom)
572571
self.assertFalse(errorMessage)
573572
self.assertTrue(success)
574573

0 commit comments

Comments
 (0)
Please sign in to comment.