Skip to content

Commit 212acc1

Browse files
elpasoelpaso
authoredMay 9, 2017
Merge pull request #4503 from boundlessgeo/attributetable-fixes
[bugfix] Fixes attribute table duplicated rows #15974
·
ltr-3_40final-3_0_0
2 parents a4e3c83 + 3784892 commit 212acc1

File tree

7 files changed

+76
-27
lines changed

7 files changed

+76
-27
lines changed
 

‎src/gui/attributetable/qgsattributetablefiltermodel.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -247,13 +247,14 @@ void QgsAttributeTableFilterModel::setSourceModel( QgsAttributeTableModel *sourc
247247
QSortFilterProxyModel::setSourceModel( sourceModel );
248248

249249
// Disconnect any code to update columns in the parent, we handle this manually
250-
disconnect( sourceModel, SIGNAL( columnsAboutToBeInserted( QModelIndex, int, int ) ), this, SLOT( _q_sourceColumnsAboutToBeInserted( QModelIndex, int, int ) ) );
251-
disconnect( sourceModel, SIGNAL( columnsInserted( QModelIndex, int, int ) ), this, SLOT( _q_sourceColumnsInserted( QModelIndex, int, int ) ) );
252-
disconnect( sourceModel, SIGNAL( columnsAboutToBeRemoved( QModelIndex, int, int ) ), this, SLOT( _q_sourceColumnsAboutToBeRemoved( QModelIndex, int, int ) ) );
253-
disconnect( sourceModel, SIGNAL( columnsRemoved( QModelIndex, int, int ) ), this, SLOT( _q_sourceColumnsRemoved( QModelIndex, int, int ) ) );
254-
250+
disconnect( mTableModel, SIGNAL( columnsAboutToBeInserted( QModelIndex, int, int ) ), this, SLOT( _q_sourceColumnsAboutToBeInserted( QModelIndex, int, int ) ) );
251+
disconnect( mTableModel, SIGNAL( columnsInserted( QModelIndex, int, int ) ), this, SLOT( _q_sourceColumnsInserted( QModelIndex, int, int ) ) );
252+
disconnect( mTableModel, SIGNAL( columnsAboutToBeRemoved( QModelIndex, int, int ) ), this, SLOT( _q_sourceColumnsAboutToBeRemoved( QModelIndex, int, int ) ) );
253+
disconnect( mTableModel, SIGNAL( columnsRemoved( QModelIndex, int, int ) ), this, SLOT( _q_sourceColumnsRemoved( QModelIndex, int, int ) ) );
254+
// The following connections are needed in order to keep the filter model in sync, see: regression #15974
255255
connect( mTableModel, &QAbstractItemModel::columnsAboutToBeInserted, this, &QgsAttributeTableFilterModel::onColumnsChanged );
256256
connect( mTableModel, &QAbstractItemModel::columnsAboutToBeRemoved, this, &QgsAttributeTableFilterModel::onColumnsChanged );
257+
257258
}
258259

259260
bool QgsAttributeTableFilterModel::selectedOnTop()

‎src/gui/attributetable/qgsattributetablemodel.cpp

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -150,10 +150,12 @@ void QgsAttributeTableModel::featuresDeleted( const QgsFeatureIds &fids )
150150

151151
bool QgsAttributeTableModel::removeRows( int row, int count, const QModelIndex &parent )
152152
{
153+
153154
if ( row < 0 || count < 1 )
154155
return false;
155156

156157
beginRemoveRows( parent, row, row + count - 1 );
158+
157159
#ifdef QGISDEBUG
158160
if ( 3 <= QgsLogger::debugLevel() )
159161
QgsDebugMsgLevel( QString( "remove %2 rows at %1 (rows %3, ids %4)" ).arg( row ).arg( count ).arg( mRowIdMap.size() ).arg( mIdRowMap.size() ), 3 );
@@ -222,15 +224,16 @@ void QgsAttributeTableModel::featureAdded( QgsFeatureId fid )
222224
mSortCache.insert( mFeat.id(), sortValue );
223225
}
224226

225-
int n = mRowIdMap.size();
226-
beginInsertRows( QModelIndex(), n, n );
227-
228-
mIdRowMap.insert( fid, n );
229-
mRowIdMap.insert( n, fid );
230-
231-
endInsertRows();
232-
233-
reload( index( rowCount() - 1, 0 ), index( rowCount() - 1, columnCount() ) );
227+
// Skip if the fid is already in the map (do not add twice)!
228+
if ( ! mIdRowMap.contains( fid ) )
229+
{
230+
int n = mRowIdMap.size();
231+
beginInsertRows( QModelIndex(), n, n );
232+
mIdRowMap.insert( fid, n );
233+
mRowIdMap.insert( n, fid );
234+
endInsertRows();
235+
reload( index( rowCount() - 1, 0 ), index( rowCount() - 1, columnCount() ) );
236+
}
234237
}
235238
}
236239

@@ -431,10 +434,10 @@ void QgsAttributeTableModel::loadLayer()
431434
emit finished();
432435

433436
connect( mLayerCache, &QgsVectorLayerCache::invalidated, this, &QgsAttributeTableModel::loadLayer, Qt::UniqueConnection );
434-
435437
endResetModel();
436438
}
437439

440+
438441
void QgsAttributeTableModel::fieldConditionalStyleChanged( const QString &fieldName )
439442
{
440443
if ( fieldName.isNull() )
@@ -472,6 +475,8 @@ void QgsAttributeTableModel::swapRows( QgsFeatureId a, QgsFeatureId b )
472475
mIdRowMap.remove( b );
473476
mIdRowMap.insert( a, rowB );
474477
mIdRowMap.insert( b, rowA );
478+
Q_ASSERT( mRowIdMap.size() == mIdRowMap.size() );
479+
475480

476481
//emit layoutChanged();
477482
}

‎src/gui/attributetable/qgsattributetablemodel.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,9 @@ class GUI_EXPORT QgsAttributeTableModel: public QAbstractTableModel
377377
QgsAttributeEditorContext mEditorContext;
378378

379379
int mExtraColumns;
380+
381+
friend class TestQgsAttributeTable;
382+
380383
};
381384

382385

‎src/gui/attributetable/qgsdualview.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ void QgsDualView::init( QgsVectorLayer *layer, QgsMapCanvas *mapCanvas, const Qg
111111
connect( mMasterModel, &QgsAttributeTableModel::modelChanged, mAttributeForm, &QgsAttributeForm::refreshFeature );
112112
connect( mAttributeForm, &QgsAttributeForm::filterExpressionSet, this, &QgsDualView::filterExpressionSet );
113113
connect( mFilterModel, &QgsAttributeTableFilterModel::sortColumnChanged, this, &QgsDualView::onSortColumnChanged );
114+
114115
if ( mFeatureListPreviewButton->defaultAction() )
115116
mFeatureList->setDisplayExpression( mDisplayExpression );
116117
else
@@ -302,6 +303,11 @@ void QgsDualView::initModels( QgsMapCanvas *mapCanvas, const QgsFeatureRequest &
302303

303304
mFilterModel = new QgsAttributeTableFilterModel( mapCanvas, mMasterModel, mMasterModel );
304305

306+
// The following connections to invalidate() are necessary to keep the filter model in sync
307+
// see regression https://issues.qgis.org/issues/15974
308+
connect( mMasterModel, &QgsAttributeTableModel::rowsRemoved, mFilterModel, &QgsAttributeTableFilterModel::invalidate );
309+
connect( mMasterModel, &QgsAttributeTableModel::rowsInserted, mFilterModel, &QgsAttributeTableFilterModel::invalidate );
310+
305311
connect( mFeatureList, &QgsFeatureListView::displayExpressionChanged, this, &QgsDualView::displayExpressionChanged );
306312

307313
mFeatureListModel = new QgsFeatureListModel( mFilterModel, mFilterModel );

‎src/gui/attributetable/qgsdualview.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,7 @@ class GUI_EXPORT QgsDualView : public QStackedWidget, private Ui::QgsDualViewBas
356356
QgsMapCanvas *mMapCanvas = nullptr;
357357

358358
friend class TestQgsDualView;
359+
friend class TestQgsAttributeTable;
359360
};
360361

361362
/** \ingroup gui

‎src/gui/layertree/qgslayertreeview.cpp

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -253,20 +253,23 @@ void QgsLayerTreeView::updateExpandedStateFromNode( QgsLayerTreeNode *node )
253253

254254
QgsMapLayer *QgsLayerTreeView::layerForIndex( const QModelIndex &index ) const
255255
{
256-
QgsLayerTreeNode *node = layerTreeModel()->index2node( index );
257-
if ( node )
256+
// Check if model has been set and index is valid
257+
if ( layerTreeModel() && index.isValid( ) )
258258
{
259-
if ( QgsLayerTree::isLayer( node ) )
260-
return QgsLayerTree::toLayer( node )->layer();
261-
}
262-
else
263-
{
264-
// possibly a legend node
265-
QgsLayerTreeModelLegendNode *legendNode = layerTreeModel()->index2legendNode( index );
266-
if ( legendNode )
267-
return legendNode->layerNode()->layer();
259+
QgsLayerTreeNode *node = layerTreeModel()->index2node( index );
260+
if ( node )
261+
{
262+
if ( QgsLayerTree::isLayer( node ) )
263+
return QgsLayerTree::toLayer( node )->layer();
264+
}
265+
else
266+
{
267+
// possibly a legend node
268+
QgsLayerTreeModelLegendNode *legendNode = layerTreeModel()->index2legendNode( index );
269+
if ( legendNode )
270+
return legendNode->layerNode()->layer();
271+
}
268272
}
269-
270273
return nullptr;
271274
}
272275

‎tests/src/app/testqgsattributetable.cpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "qgsmapcanvas.h"
2626
#include "qgsunittypes.h"
2727
#include "qgssettings.h"
28+
#include "qgsvectorfilewriter.h"
2829

2930
#include "qgstest.h"
3031

@@ -42,6 +43,7 @@ class TestQgsAttributeTable : public QObject
4243
void cleanupTestCase();// will be called after the last testfunction was executed.
4344
void init() {} // will be called before each testfunction is executed.
4445
void cleanup() {} // will be called after every testfunction.
46+
void testRegression15974();
4547
void testFieldCalculation();
4648
void testFieldCalculationArea();
4749
void testNoGeom();
@@ -247,6 +249,34 @@ void TestQgsAttributeTable::testSelected()
247249
QVERIFY( dlg->mMainView->masterModel()->request().filterFids().isEmpty() );
248250
}
249251

252+
void TestQgsAttributeTable::testRegression15974()
253+
{
254+
QString path = QDir::tempPath() + "/testshp15974.shp";
255+
std::unique_ptr< QgsVectorLayer> tempLayer( new QgsVectorLayer( QStringLiteral( "polygon?crs=epsg:4326&field=id:integer" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) ) );
256+
QVERIFY( tempLayer->isValid() );
257+
QgsVectorFileWriter::writeAsVectorFormat( tempLayer.get( ), path, "system", QgsCoordinateReferenceSystem( 4326 ), "ESRI Shapefile" );
258+
std::unique_ptr< QgsVectorLayer> shpLayer( new QgsVectorLayer( path, QStringLiteral( "test" ), QStringLiteral( "ogr" ) ) );
259+
QgsFeature f1( shpLayer->dataProvider()->fields(), 1 );
260+
f1.setGeometry( QgsGeometry().fromWkt( QStringLiteral( "polygon(0 0, 1 1, 1 2, 1 0, 0 0))" ) ) );
261+
QgsFeature f2( shpLayer->dataProvider()->fields(), 2 );
262+
f2.setGeometry( QgsGeometry().fromWkt( QStringLiteral( "polygon(0 0, 1 1, 1 2, 1 0, 0 0))" ) ) );
263+
QgsFeature f3( shpLayer->dataProvider()->fields(), 3 );
264+
f3.setGeometry( QgsGeometry().fromWkt( QStringLiteral( "polygon(0 0, 1 1, 1 2, 1 0, 0 0))" ) ) );
265+
QVERIFY( shpLayer->startEditing( ) );
266+
QVERIFY( shpLayer->addFeatures( QgsFeatureList() << f1 << f2 << f3 ) );
267+
std::unique_ptr< QgsAttributeTableDialog > dlg( new QgsAttributeTableDialog( shpLayer.get() ) );
268+
QCOMPARE( shpLayer->featureCount( ), 3L );
269+
mQgisApp->saveEdits( shpLayer.get( ) );
270+
QCOMPARE( shpLayer->featureCount( ), 3L );
271+
QCOMPARE( dlg->mMainView->masterModel()->rowCount(), 3 );
272+
QCOMPARE( dlg->mMainView->mLayerCache->cachedFeatureIds( ).count(), 3 );
273+
QCOMPARE( dlg->mMainView->featureCount( ), 3 );
274+
// All the following instructions made the test pass, before the connections to invalidate()
275+
// were introduced in QgsDualView::initModels
276+
// dlg->mMainView->mFilterModel->setSourceModel( dlg->mMainView->masterModel() );
277+
// dlg->mMainView->mFilterModel->invalidate();
278+
QCOMPARE( dlg->mMainView->filteredFeatureCount( ), 3 );
279+
}
250280

251281
QGSTEST_MAIN( TestQgsAttributeTable )
252282
#include "testqgsattributetable.moc"

0 commit comments

Comments
 (0)
Please sign in to comment.