Skip to content

Commit

Permalink
Merge pull request #4503 from boundlessgeo/attributetable-fixes
Browse files Browse the repository at this point in the history
[bugfix] Fixes attribute table duplicated rows #15974
  • Loading branch information
elpaso authored May 9, 2017
2 parents a4e3c83 + 3784892 commit 212acc1
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 27 deletions.
11 changes: 6 additions & 5 deletions src/gui/attributetable/qgsattributetablefiltermodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -247,13 +247,14 @@ void QgsAttributeTableFilterModel::setSourceModel( QgsAttributeTableModel *sourc
QSortFilterProxyModel::setSourceModel( sourceModel );

// Disconnect any code to update columns in the parent, we handle this manually
disconnect( sourceModel, SIGNAL( columnsAboutToBeInserted( QModelIndex, int, int ) ), this, SLOT( _q_sourceColumnsAboutToBeInserted( QModelIndex, int, int ) ) );
disconnect( sourceModel, SIGNAL( columnsInserted( QModelIndex, int, int ) ), this, SLOT( _q_sourceColumnsInserted( QModelIndex, int, int ) ) );
disconnect( sourceModel, SIGNAL( columnsAboutToBeRemoved( QModelIndex, int, int ) ), this, SLOT( _q_sourceColumnsAboutToBeRemoved( QModelIndex, int, int ) ) );
disconnect( sourceModel, SIGNAL( columnsRemoved( QModelIndex, int, int ) ), this, SLOT( _q_sourceColumnsRemoved( QModelIndex, int, int ) ) );

disconnect( mTableModel, SIGNAL( columnsAboutToBeInserted( QModelIndex, int, int ) ), this, SLOT( _q_sourceColumnsAboutToBeInserted( QModelIndex, int, int ) ) );
disconnect( mTableModel, SIGNAL( columnsInserted( QModelIndex, int, int ) ), this, SLOT( _q_sourceColumnsInserted( QModelIndex, int, int ) ) );
disconnect( mTableModel, SIGNAL( columnsAboutToBeRemoved( QModelIndex, int, int ) ), this, SLOT( _q_sourceColumnsAboutToBeRemoved( QModelIndex, int, int ) ) );
disconnect( mTableModel, SIGNAL( columnsRemoved( QModelIndex, int, int ) ), this, SLOT( _q_sourceColumnsRemoved( QModelIndex, int, int ) ) );
// The following connections are needed in order to keep the filter model in sync, see: regression #15974
connect( mTableModel, &QAbstractItemModel::columnsAboutToBeInserted, this, &QgsAttributeTableFilterModel::onColumnsChanged );
connect( mTableModel, &QAbstractItemModel::columnsAboutToBeRemoved, this, &QgsAttributeTableFilterModel::onColumnsChanged );

}

bool QgsAttributeTableFilterModel::selectedOnTop()
Expand Down
25 changes: 15 additions & 10 deletions src/gui/attributetable/qgsattributetablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,12 @@ void QgsAttributeTableModel::featuresDeleted( const QgsFeatureIds &fids )

bool QgsAttributeTableModel::removeRows( int row, int count, const QModelIndex &parent )
{

if ( row < 0 || count < 1 )
return false;

beginRemoveRows( parent, row, row + count - 1 );

#ifdef QGISDEBUG
if ( 3 <= QgsLogger::debugLevel() )
QgsDebugMsgLevel( QString( "remove %2 rows at %1 (rows %3, ids %4)" ).arg( row ).arg( count ).arg( mRowIdMap.size() ).arg( mIdRowMap.size() ), 3 );
Expand Down Expand Up @@ -222,15 +224,16 @@ void QgsAttributeTableModel::featureAdded( QgsFeatureId fid )
mSortCache.insert( mFeat.id(), sortValue );
}

int n = mRowIdMap.size();
beginInsertRows( QModelIndex(), n, n );

mIdRowMap.insert( fid, n );
mRowIdMap.insert( n, fid );

endInsertRows();

reload( index( rowCount() - 1, 0 ), index( rowCount() - 1, columnCount() ) );
// Skip if the fid is already in the map (do not add twice)!
if ( ! mIdRowMap.contains( fid ) )
{
int n = mRowIdMap.size();
beginInsertRows( QModelIndex(), n, n );
mIdRowMap.insert( fid, n );
mRowIdMap.insert( n, fid );
endInsertRows();
reload( index( rowCount() - 1, 0 ), index( rowCount() - 1, columnCount() ) );
}
}
}

Expand Down Expand Up @@ -431,10 +434,10 @@ void QgsAttributeTableModel::loadLayer()
emit finished();

connect( mLayerCache, &QgsVectorLayerCache::invalidated, this, &QgsAttributeTableModel::loadLayer, Qt::UniqueConnection );

endResetModel();
}


void QgsAttributeTableModel::fieldConditionalStyleChanged( const QString &fieldName )
{
if ( fieldName.isNull() )
Expand Down Expand Up @@ -472,6 +475,8 @@ void QgsAttributeTableModel::swapRows( QgsFeatureId a, QgsFeatureId b )
mIdRowMap.remove( b );
mIdRowMap.insert( a, rowB );
mIdRowMap.insert( b, rowA );
Q_ASSERT( mRowIdMap.size() == mIdRowMap.size() );


//emit layoutChanged();
}
Expand Down
3 changes: 3 additions & 0 deletions src/gui/attributetable/qgsattributetablemodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,9 @@ class GUI_EXPORT QgsAttributeTableModel: public QAbstractTableModel
QgsAttributeEditorContext mEditorContext;

int mExtraColumns;

friend class TestQgsAttributeTable;

};


Expand Down
6 changes: 6 additions & 0 deletions src/gui/attributetable/qgsdualview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ void QgsDualView::init( QgsVectorLayer *layer, QgsMapCanvas *mapCanvas, const Qg
connect( mMasterModel, &QgsAttributeTableModel::modelChanged, mAttributeForm, &QgsAttributeForm::refreshFeature );
connect( mAttributeForm, &QgsAttributeForm::filterExpressionSet, this, &QgsDualView::filterExpressionSet );
connect( mFilterModel, &QgsAttributeTableFilterModel::sortColumnChanged, this, &QgsDualView::onSortColumnChanged );

if ( mFeatureListPreviewButton->defaultAction() )
mFeatureList->setDisplayExpression( mDisplayExpression );
else
Expand Down Expand Up @@ -302,6 +303,11 @@ void QgsDualView::initModels( QgsMapCanvas *mapCanvas, const QgsFeatureRequest &

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

// The following connections to invalidate() are necessary to keep the filter model in sync
// see regression https://issues.qgis.org/issues/15974
connect( mMasterModel, &QgsAttributeTableModel::rowsRemoved, mFilterModel, &QgsAttributeTableFilterModel::invalidate );
connect( mMasterModel, &QgsAttributeTableModel::rowsInserted, mFilterModel, &QgsAttributeTableFilterModel::invalidate );

connect( mFeatureList, &QgsFeatureListView::displayExpressionChanged, this, &QgsDualView::displayExpressionChanged );

mFeatureListModel = new QgsFeatureListModel( mFilterModel, mFilterModel );
Expand Down
1 change: 1 addition & 0 deletions src/gui/attributetable/qgsdualview.h
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ class GUI_EXPORT QgsDualView : public QStackedWidget, private Ui::QgsDualViewBas
QgsMapCanvas *mMapCanvas = nullptr;

friend class TestQgsDualView;
friend class TestQgsAttributeTable;
};

/** \ingroup gui
Expand Down
27 changes: 15 additions & 12 deletions src/gui/layertree/qgslayertreeview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -253,20 +253,23 @@ void QgsLayerTreeView::updateExpandedStateFromNode( QgsLayerTreeNode *node )

QgsMapLayer *QgsLayerTreeView::layerForIndex( const QModelIndex &index ) const
{
QgsLayerTreeNode *node = layerTreeModel()->index2node( index );
if ( node )
// Check if model has been set and index is valid
if ( layerTreeModel() && index.isValid( ) )
{
if ( QgsLayerTree::isLayer( node ) )
return QgsLayerTree::toLayer( node )->layer();
}
else
{
// possibly a legend node
QgsLayerTreeModelLegendNode *legendNode = layerTreeModel()->index2legendNode( index );
if ( legendNode )
return legendNode->layerNode()->layer();
QgsLayerTreeNode *node = layerTreeModel()->index2node( index );
if ( node )
{
if ( QgsLayerTree::isLayer( node ) )
return QgsLayerTree::toLayer( node )->layer();
}
else
{
// possibly a legend node
QgsLayerTreeModelLegendNode *legendNode = layerTreeModel()->index2legendNode( index );
if ( legendNode )
return legendNode->layerNode()->layer();
}
}

return nullptr;
}

Expand Down
30 changes: 30 additions & 0 deletions tests/src/app/testqgsattributetable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "qgsmapcanvas.h"
#include "qgsunittypes.h"
#include "qgssettings.h"
#include "qgsvectorfilewriter.h"

#include "qgstest.h"

Expand All @@ -42,6 +43,7 @@ class TestQgsAttributeTable : public QObject
void cleanupTestCase();// will be called after the last testfunction was executed.
void init() {} // will be called before each testfunction is executed.
void cleanup() {} // will be called after every testfunction.
void testRegression15974();
void testFieldCalculation();
void testFieldCalculationArea();
void testNoGeom();
Expand Down Expand Up @@ -247,6 +249,34 @@ void TestQgsAttributeTable::testSelected()
QVERIFY( dlg->mMainView->masterModel()->request().filterFids().isEmpty() );
}

void TestQgsAttributeTable::testRegression15974()
{
QString path = QDir::tempPath() + "/testshp15974.shp";
std::unique_ptr< QgsVectorLayer> tempLayer( new QgsVectorLayer( QStringLiteral( "polygon?crs=epsg:4326&field=id:integer" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) ) );
QVERIFY( tempLayer->isValid() );
QgsVectorFileWriter::writeAsVectorFormat( tempLayer.get( ), path, "system", QgsCoordinateReferenceSystem( 4326 ), "ESRI Shapefile" );
std::unique_ptr< QgsVectorLayer> shpLayer( new QgsVectorLayer( path, QStringLiteral( "test" ), QStringLiteral( "ogr" ) ) );
QgsFeature f1( shpLayer->dataProvider()->fields(), 1 );
f1.setGeometry( QgsGeometry().fromWkt( QStringLiteral( "polygon(0 0, 1 1, 1 2, 1 0, 0 0))" ) ) );
QgsFeature f2( shpLayer->dataProvider()->fields(), 2 );
f2.setGeometry( QgsGeometry().fromWkt( QStringLiteral( "polygon(0 0, 1 1, 1 2, 1 0, 0 0))" ) ) );
QgsFeature f3( shpLayer->dataProvider()->fields(), 3 );
f3.setGeometry( QgsGeometry().fromWkt( QStringLiteral( "polygon(0 0, 1 1, 1 2, 1 0, 0 0))" ) ) );
QVERIFY( shpLayer->startEditing( ) );
QVERIFY( shpLayer->addFeatures( QgsFeatureList() << f1 << f2 << f3 ) );
std::unique_ptr< QgsAttributeTableDialog > dlg( new QgsAttributeTableDialog( shpLayer.get() ) );
QCOMPARE( shpLayer->featureCount( ), 3L );
mQgisApp->saveEdits( shpLayer.get( ) );
QCOMPARE( shpLayer->featureCount( ), 3L );
QCOMPARE( dlg->mMainView->masterModel()->rowCount(), 3 );
QCOMPARE( dlg->mMainView->mLayerCache->cachedFeatureIds( ).count(), 3 );
QCOMPARE( dlg->mMainView->featureCount( ), 3 );
// All the following instructions made the test pass, before the connections to invalidate()
// were introduced in QgsDualView::initModels
// dlg->mMainView->mFilterModel->setSourceModel( dlg->mMainView->masterModel() );
// dlg->mMainView->mFilterModel->invalidate();
QCOMPARE( dlg->mMainView->filteredFeatureCount( ), 3 );
}

QGSTEST_MAIN( TestQgsAttributeTable )
#include "testqgsattributetable.moc"

0 comments on commit 212acc1

Please sign in to comment.