Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit c0ef75c

Browse files
domi4484domi4484
committedMar 27, 2023
Explicitly set the target feature for merge operation
1 parent aa21e30 commit c0ef75c

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
@@ -9619,7 +9619,7 @@ void QgisApp::mergeSelectedFeatures()
96199619

96209620
QString errorMessage;
96219621
QgsVectorLayerEditUtils vectorLayerEditUtils( vl );
9622-
bool success = vectorLayerEditUtils.mergeFeatures( vl->selectedFeatureIds(), d.mergedAttributes(), unionGeom, errorMessage );
9622+
bool success = vectorLayerEditUtils.mergeFeatures( d.targetFeatureId(), vl->selectedFeatureIds(), d.mergedAttributes(), unionGeom, errorMessage );
96239623

96249624
if ( !success )
96259625
{

‎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
@@ -708,18 +708,17 @@ int QgsVectorLayerEditUtils::addTopologicalPoints( const QgsPointXY &p )
708708
return addTopologicalPoints( QgsPoint( p ) );
709709
}
710710

711-
bool QgsVectorLayerEditUtils::mergeFeatures( const QgsFeatureIds &mergeFeatureIds, const QgsAttributes &mergeAttributes, QgsGeometry unionGeometry, QString &errorMessage )
711+
bool QgsVectorLayerEditUtils::mergeFeatures( const QgsFeatureId &targetFeatureId, const QgsFeatureIds &mergeFeatureIds, const QgsAttributes &mergeAttributes, const QgsGeometry &unionGeometry, QString &errorMessage )
712712
{
713713
errorMessage.clear();
714714

715-
if ( mergeFeatureIds.size() < 2 )
715+
if ( mergeFeatureIds.isEmpty() )
716716
{
717-
errorMessage = QObject::tr( "Not enough features to merge, the merge tool requires at least two features" );
717+
errorMessage = QObject::tr( "List of features to merge is empty" );
718718
return false;
719719
}
720720

721721
QgsAttributeMap newAttributes;
722-
QgsFeatureId mergeFeatureId = FID_NULL;
723722
for ( int i = 0; i < mergeAttributes.count(); ++i )
724723
{
725724
QVariant val = mergeAttributes.at( i );
@@ -728,48 +727,6 @@ bool QgsVectorLayerEditUtils::mergeFeatures( const QgsFeatureIds &mergeFeatureId
728727
mLayer->dataProvider() &&
729728
mLayer->dataProvider()->defaultValueClause( mLayer->fields().fieldOriginIndex( i ) ) == val;
730729

731-
// Check if features can merged into an existing one
732-
if ( mergeFeatureId == FID_NULL )
733-
{
734-
bool isPrimaryKey = mLayer->fields().fieldOrigin( i ) == QgsFields::OriginProvider &&
735-
mLayer->dataProvider() &&
736-
mLayer->dataProvider()->pkAttributeIndexes().contains( mLayer->fields().fieldOriginIndex( i ) );
737-
738-
if ( isPrimaryKey && !isDefaultValue )
739-
{
740-
QgsFeatureRequest request;
741-
request.setFlags( QgsFeatureRequest::Flag::NoGeometry );
742-
// Handle multi pks
743-
if ( mLayer->dataProvider()->pkAttributeIndexes().count() > 1 && mLayer->dataProvider()->pkAttributeIndexes().count() <= mergeAttributes.count() )
744-
{
745-
const auto pkIdxList { mLayer->dataProvider()->pkAttributeIndexes() };
746-
QStringList conditions;
747-
QStringList fieldNames;
748-
for ( const int &pkIdx : std::as_const( pkIdxList ) )
749-
{
750-
const QgsField pkField { mLayer->fields().field( pkIdx ) };
751-
conditions.push_back( QgsExpression::createFieldEqualityExpression( pkField.name(), mergeAttributes.at( pkIdx ), pkField.type( ) ) );
752-
fieldNames.push_back( pkField.name() );
753-
}
754-
request.setSubsetOfAttributes( fieldNames, mLayer->fields( ) );
755-
request.setFilterExpression( conditions.join( QLatin1String( " AND " ) ) );
756-
}
757-
else // single pk
758-
{
759-
const QgsField pkField { mLayer->fields().field( i ) };
760-
request.setSubsetOfAttributes( QStringList() << pkField.name(), mLayer->fields( ) );
761-
request.setFilterExpression( QgsExpression::createFieldEqualityExpression( pkField.name(), val, pkField.type( ) ) );
762-
}
763-
764-
QgsFeature f;
765-
QgsFeatureIterator featureIterator = mLayer->getFeatures( request );
766-
if ( featureIterator.nextFeature( f ) )
767-
{
768-
mergeFeatureId = f.id( );
769-
}
770-
}
771-
}
772-
773730
// convert to destination data type
774731
QString errorMessageConvertCompatible;
775732
if ( !isDefaultValue && !mLayer->fields().at( i ).convertCompatible( val, &errorMessageConvertCompatible ) )
@@ -782,38 +739,18 @@ bool QgsVectorLayerEditUtils::mergeFeatures( const QgsFeatureIds &mergeFeatureId
782739

783740
mLayer->beginEditCommand( QObject::tr( "Merged features" ) );
784741

785-
QgsFeatureIds featureIdsToDelete = mergeFeatureIds;
786-
787-
QgsFeature mergeFeature;
788-
if ( mergeFeatureId == FID_NULL )
789-
{
790-
// Create new feature
791-
mergeFeature = QgsVectorLayerUtils::createFeature( mLayer, unionGeometry, newAttributes );
792-
}
793-
else
794-
{
795-
// Merge into existing feature
796-
featureIdsToDelete.remove( mergeFeatureId );
797-
}
798-
799-
// Delete other features
800-
QgsFeatureIds::const_iterator feature_it = featureIdsToDelete.constBegin();
801-
for ( ; feature_it != featureIdsToDelete.constEnd(); ++feature_it )
742+
// Delete other features but the target feature
743+
QgsFeatureIds::const_iterator feature_it = mergeFeatureIds.constBegin();
744+
for ( ; feature_it != mergeFeatureIds.constEnd(); ++feature_it )
802745
{
803-
mLayer->deleteFeature( *feature_it );
746+
if ( *feature_it != targetFeatureId )
747+
mLayer->deleteFeature( *feature_it );
804748
}
805749

806-
if ( mergeFeatureId == FID_NULL )
807-
{
808-
// Add the new feature
809-
mLayer->addFeature( mergeFeature );
810-
}
811-
else
812-
{
813-
// Modify merge feature
814-
mLayer->changeGeometry( mergeFeatureId, unionGeometry );
815-
mLayer->changeAttributeValues( mergeFeatureId, newAttributes );
816-
}
750+
// Modify merge feature
751+
QgsGeometry mergeGeometry = unionGeometry;
752+
mLayer->changeGeometry( targetFeatureId, mergeGeometry );
753+
mLayer->changeAttributeValues( targetFeatureId, newAttributes );
817754

818755
mLayer->endEditCommand();
819756

‎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
@@ -45,6 +45,7 @@ set(TESTS
4545
testqgsmeshcalculatordialog.cpp
4646
testqgsgpsinformationwidget.cpp
4747
testqgslabelpropertydialog.cpp
48+
testqgsmergeattributesdialog.cpp
4849
)
4950

5051
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
@@ -488,13 +488,13 @@ def testMergeFeatures(self):
488488
self.assertTrue(layer.startEditing())
489489
layer.addAttribute(QgsField('name', QVariant.String))
490490
pr = layer.dataProvider()
491+
491492
f1 = QgsFeature(layer.fields(), 1)
492493
f1.setGeometry(QgsGeometry.fromWkt('POLYGON((0 0, 5 0, 5 5, 0 5, 0 0))'))
493494
f1.setAttribute('name', 'uno')
494495
assert pr.addFeatures([f1])
495496
self.assertEqual(layer.featureCount(), 1)
496497

497-
pr = layer.dataProvider()
498498
f2 = QgsFeature(layer.fields(), 2)
499499
f2.setGeometry(QgsGeometry.fromWkt('POLYGON((3 3, 8 3, 8 8, 3 8, 3 3))'))
500500
f2.setAttribute('name', 'due')
@@ -508,7 +508,7 @@ def testMergeFeatures(self):
508508
self.assertFalse(unionGeom.isNull())
509509

510510
vle = QgsVectorLayerEditUtils(layer)
511-
success, errorMessage = vle.mergeFeatures(featureIds, ['tre'], unionGeom)
511+
success, errorMessage = vle.mergeFeatures(f1.id(), featureIds, ['tre'], unionGeom)
512512
self.assertFalse(errorMessage)
513513
self.assertTrue(success)
514514

@@ -539,16 +539,15 @@ def testMergeFeaturesIntoExisting(self):
539539
options=options)
540540

541541
layer = QgsVectorLayer(tempgpkg, 'my_layer', 'ogr')
542-
543542
self.assertTrue(layer.startEditing())
544543
pr = layer.dataProvider()
544+
545545
f1 = QgsFeature(layer.fields(), 1)
546546
f1.setGeometry(QgsGeometry.fromWkt('POLYGON((0 0, 5 0, 5 5, 0 5, 0 0))'))
547547
f1.setAttribute('name', 'uno')
548548
assert pr.addFeatures([f1])
549549
self.assertEqual(layer.featureCount(), 1)
550550

551-
pr = layer.dataProvider()
552551
f2 = QgsFeature(layer.fields(), 2)
553552
f2.setGeometry(QgsGeometry.fromWkt('POLYGON((3 3, 8 3, 8 8, 3 8, 3 3))'))
554553
f2.setAttribute('name', 'due')
@@ -562,7 +561,7 @@ def testMergeFeaturesIntoExisting(self):
562561
self.assertFalse(unionGeom.isNull())
563562

564563
vle = QgsVectorLayerEditUtils(layer)
565-
success, errorMessage = vle.mergeFeatures(featureIds, [2, 'tre'], unionGeom)
564+
success, errorMessage = vle.mergeFeatures(f2.id(), featureIds, [2, 'tre'], unionGeom)
566565
self.assertFalse(errorMessage)
567566
self.assertTrue(success)
568567

0 commit comments

Comments
 (0)
Please sign in to comment.