From ba97282dd02c12320cb49e2f5cb39c2476c20c01 Mon Sep 17 00:00:00 2001 From: Stefanos Natsis Date: Fri, 21 Oct 2022 13:53:49 +0300 Subject: [PATCH] Allow hiding empty layers in sublayers dialog (#49870) * Sublayers dialog improvements * added tests * fix flake * check against DisplayRole instead of EditRole --- .../providers/qgsprovidersublayermodel.sip.in | 18 ++++++ src/app/qgsprovidersublayersdialog.cpp | 1 + .../providers/qgsprovidersublayermodel.cpp | 18 +++++- src/core/providers/qgsprovidersublayermodel.h | 17 ++++++ src/ui/qgsprovidersublayersdialogbase.ui | 56 ++++++++++++++----- .../python/test_qgsprovidersublayermodel.py | 52 ++++++++++++++--- 6 files changed, 137 insertions(+), 25 deletions(-) diff --git a/python/core/auto_generated/providers/qgsprovidersublayermodel.sip.in b/python/core/auto_generated/providers/qgsprovidersublayermodel.sip.in index e8a3a8ee6cc..f381ef6549a 100644 --- a/python/core/auto_generated/providers/qgsprovidersublayermodel.sip.in +++ b/python/core/auto_generated/providers/qgsprovidersublayermodel.sip.in @@ -257,6 +257,24 @@ Returns ``True`` if system and internal tables will be shown in the model. Sets whether system and internal tables will be shown in the model. .. seealso:: :py:func:`includeSystemTables` +%End + + bool includeEmptyLayers() const; +%Docstring +Returns ``True`` if empty tables will be shown in the model. + +.. seealso:: :py:func:`setIncludeEmptyLayers` + +.. versionadded:: 3.28 +%End + + void setIncludeEmptyLayers( bool include ); +%Docstring +Sets whether empty tables will be shown in the model. + +.. seealso:: :py:func:`includeEmptyLayers` + +.. versionadded:: 3.28 %End protected: diff --git a/src/app/qgsprovidersublayersdialog.cpp b/src/app/qgsprovidersublayersdialog.cpp index de831fcc3d7..aad22435775 100644 --- a/src/app/qgsprovidersublayersdialog.cpp +++ b/src/app/qgsprovidersublayersdialog.cpp @@ -190,6 +190,7 @@ QgsProviderSublayersDialog::QgsProviderSublayersDialog( const QString &uri, cons connect( mLayersTree->selectionModel(), &QItemSelectionModel::selectionChanged, this, &QgsProviderSublayersDialog::treeSelectionChanged ); connect( mSearchLineEdit, &QgsFilterLineEdit::textChanged, mProxyModel, &QgsProviderSublayerProxyModel::setFilterString ); connect( mCheckShowSystem, &QCheckBox::toggled, mProxyModel, &QgsProviderSublayerProxyModel::setIncludeSystemTables ); + connect( mCheckShowEmpty, &QCheckBox::toggled, mProxyModel, &QgsProviderSublayerProxyModel::setIncludeEmptyLayers ); connect( mLayersTree, &QTreeView::doubleClicked, this, [ = ]( const QModelIndex & index ) { const QModelIndex left = mLayersTree->model()->index( index.row(), 0, index.parent() ); diff --git a/src/core/providers/qgsprovidersublayermodel.cpp b/src/core/providers/qgsprovidersublayermodel.cpp index ee0ac76d9e0..9d2af3f3f36 100644 --- a/src/core/providers/qgsprovidersublayermodel.cpp +++ b/src/core/providers/qgsprovidersublayermodel.cpp @@ -656,13 +656,18 @@ bool QgsProviderSublayerProxyModel::filterAcceptsRow( int source_row, const QMod if ( !mIncludeSystemTables && static_cast< Qgis::SublayerFlags >( sourceModel()->data( sourceIndex, static_cast< int >( QgsProviderSublayerModel::Role::Flags ) ).toInt() ) & Qgis::SublayerFlag::SystemTable ) return false; + if ( !mIncludeEmptyLayers && sourceModel()->data( sourceIndex, static_cast< int >( QgsProviderSublayerModel::Role::FeatureCount ) ) == 0 ) + return false; + if ( mFilterString.trimmed().isEmpty() ) return true; if ( sourceModel()->data( sourceIndex, static_cast< int >( QgsProviderSublayerModel::Role::Name ) ).toString().contains( mFilterString, Qt::CaseInsensitive ) ) return true; - if ( sourceModel()->data( sourceIndex, static_cast< int >( QgsProviderSublayerModel::Role::Description ) ).toString().contains( mFilterString, Qt::CaseInsensitive ) ) + // check against the Description column's display role as it might be different from QgsProviderSublayerModel::Role::Description + const QModelIndex descriptionColumnIndex = sourceModel()->index( source_row, 1, source_parent ); + if ( sourceModel()->data( descriptionColumnIndex, static_cast< int >( Qt::DisplayRole ) ).toString().contains( mFilterString, Qt::CaseInsensitive ) ) return true; const QVariant wkbTypeVariant = sourceModel()->data( sourceIndex, static_cast< int >( QgsProviderSublayerModel::Role::WkbType ) ); @@ -703,6 +708,17 @@ void QgsProviderSublayerProxyModel::setIncludeSystemTables( bool include ) invalidateFilter(); } +bool QgsProviderSublayerProxyModel::includeEmptyLayers() const +{ + return mIncludeEmptyLayers; +} + +void QgsProviderSublayerProxyModel::setIncludeEmptyLayers( bool include ) +{ + mIncludeEmptyLayers = include; + invalidateFilter(); +} + QString QgsProviderSublayerProxyModel::filterString() const { return mFilterString; diff --git a/src/core/providers/qgsprovidersublayermodel.h b/src/core/providers/qgsprovidersublayermodel.h index c75c990cf3f..8e8bfc97773 100644 --- a/src/core/providers/qgsprovidersublayermodel.h +++ b/src/core/providers/qgsprovidersublayermodel.h @@ -415,6 +415,22 @@ class CORE_EXPORT QgsProviderSublayerProxyModel: public QSortFilterProxyModel */ void setIncludeSystemTables( bool include ); + /** + * Returns TRUE if empty tables will be shown in the model. + * + * \see setIncludeEmptyLayers() + * \since QGIS 3.28 + */ + bool includeEmptyLayers() const; + + /** + * Sets whether empty tables will be shown in the model. + * + * \see includeEmptyLayers() + * \since QGIS 3.28 + */ + void setIncludeEmptyLayers( bool include ); + protected: bool filterAcceptsRow( int source_row, const QModelIndex &source_parent ) const override; bool lessThan( const QModelIndex &source_left, const QModelIndex &source_right ) const override; @@ -423,6 +439,7 @@ class CORE_EXPORT QgsProviderSublayerProxyModel: public QSortFilterProxyModel QString mFilterString; bool mIncludeSystemTables = false; + bool mIncludeEmptyLayers = true; }; diff --git a/src/ui/qgsprovidersublayersdialogbase.ui b/src/ui/qgsprovidersublayersdialogbase.ui index e385b9636bb..77efef41dad 100644 --- a/src/ui/qgsprovidersublayersdialogbase.ui +++ b/src/ui/qgsprovidersublayersdialogbase.ui @@ -7,7 +7,7 @@ 0 0 584 - 236 + 311 @@ -29,7 +29,7 @@ 6 - + Qt::Horizontal @@ -83,11 +83,40 @@ - - - - Show system and internal tables + + + + Options + + true + + + + + + Add layers to a group + + + + + + + Show system and internal tables + + + + + + + Show empty vector layers + + + true + + + + @@ -120,17 +149,16 @@ - - - - Add layers to a group - - - + + QgsCollapsibleGroupBox + QGroupBox +
qgscollapsiblegroupbox.h
+ 1 + QgsFilterLineEdit QLineEdit @@ -142,8 +170,6 @@ mLayersTree mBtnSelectAll mBtnDeselectAll - mCbxAddToGroup - mCheckShowSystem diff --git a/tests/src/python/test_qgsprovidersublayermodel.py b/tests/src/python/test_qgsprovidersublayermodel.py index f5c005568aa..8f4fec9eb2e 100644 --- a/tests/src/python/test_qgsprovidersublayermodel.py +++ b/tests/src/python/test_qgsprovidersublayermodel.py @@ -502,7 +502,16 @@ class TestQgsProviderSublayerModel(unittest.TestCase): layer2.setFeatureCount(-1) layer2.setWkbType(QgsWkbTypes.LineString) - model.setSublayerDetails([layer1, layer2]) + layer3 = QgsProviderSublayerDetails() + layer3.setType(QgsMapLayerType.VectorLayer) + layer3.setName('yet another layer 3') + layer3.setDescription('an empty layer') + layer3.setProviderKey('ogr') + layer3.setUri('uri 3') + layer3.setFeatureCount(0) + layer3.setWkbType(QgsWkbTypes.Polygon) + + model.setSublayerDetails([layer1, layer2, layer3]) item1 = QgsProviderSublayerModel.NonLayerItem() item1.setUri('item uri 1') @@ -512,7 +521,7 @@ class TestQgsProviderSublayerModel(unittest.TestCase): model.addNonLayerItem(item1) - self.assertEqual(proxy.rowCount(QModelIndex()), 3) + self.assertEqual(proxy.rowCount(QModelIndex()), 4) self.assertEqual(proxy.data(proxy.index(0, 0), Qt.DisplayRole), 'item name 1') self.assertEqual(proxy.data(proxy.index(0, 1), Qt.DisplayRole), 'item desc 1') @@ -539,6 +548,15 @@ class TestQgsProviderSublayerModel(unittest.TestCase): self.assertEqual(proxy.data(proxy.index(2, 0), QgsProviderSublayerModel.Role.Description), 'description 1') self.assertEqual(proxy.data(proxy.index(2, 0), QgsProviderSublayerModel.Role.IsNonLayerItem), False) + self.assertEqual(proxy.data(proxy.index(3, 0), Qt.DisplayRole), 'yet another layer 3') + self.assertEqual(proxy.data(proxy.index(3, 1), Qt.DisplayRole), 'an empty layer - Polygon (0)') + self.assertEqual(proxy.data(proxy.index(3, 0), QgsProviderSublayerModel.Role.ProviderKey), 'ogr') + self.assertEqual(proxy.data(proxy.index(3, 0), QgsProviderSublayerModel.Role.LayerType), QgsMapLayerType.VectorLayer) + self.assertEqual(proxy.data(proxy.index(3, 0), QgsProviderSublayerModel.Role.Uri), 'uri 3') + self.assertEqual(proxy.data(proxy.index(3, 0), QgsProviderSublayerModel.Role.Name), 'yet another layer 3') + self.assertEqual(proxy.data(proxy.index(3, 0), QgsProviderSublayerModel.Role.Description), 'an empty layer') + self.assertEqual(proxy.data(proxy.index(3, 0), QgsProviderSublayerModel.Role.IsNonLayerItem), False) + proxy.setFilterString(' 1') self.assertEqual(proxy.rowCount(QModelIndex()), 2) self.assertEqual(proxy.data(proxy.index(0, 0), Qt.DisplayRole), 'item name 1') @@ -557,26 +575,42 @@ class TestQgsProviderSublayerModel(unittest.TestCase): self.assertEqual(proxy.rowCount(QModelIndex()), 1) self.assertEqual(proxy.data(proxy.index(0, 0), Qt.DisplayRole), 'another layer 2') + # should also allow filtering by the feature count as it appears in the description too but it's not in the description role + proxy.setFilterString('0') + self.assertEqual(proxy.rowCount(QModelIndex()), 1) + self.assertEqual(proxy.data(proxy.index(0, 0), Qt.DisplayRole), 'yet another layer 3') + proxy.setFilterString('') - self.assertEqual(proxy.rowCount(QModelIndex()), 3) + self.assertEqual(proxy.rowCount(QModelIndex()), 4) self.assertEqual(proxy.data(proxy.index(0, 0), Qt.DisplayRole), 'item name 1') self.assertEqual(proxy.data(proxy.index(1, 0), Qt.DisplayRole), 'another layer 2') self.assertEqual(proxy.data(proxy.index(2, 0), Qt.DisplayRole), 'layer 1') + self.assertEqual(proxy.data(proxy.index(3, 0), Qt.DisplayRole), 'yet another layer 3') # add a system table - layer3 = QgsProviderSublayerDetails() - layer3.setType(QgsMapLayerType.VectorLayer) - layer3.setName('system table') - layer3.setFlags(Qgis.SublayerFlags(Qgis.SublayerFlag.SystemTable)) + layer_system = QgsProviderSublayerDetails() + layer_system.setType(QgsMapLayerType.VectorLayer) + layer_system.setName('system table') + layer_system.setFlags(Qgis.SublayerFlags(Qgis.SublayerFlag.SystemTable)) - model.setSublayerDetails([layer1, layer2, layer3]) + model.setSublayerDetails([layer1, layer2, layer3, layer_system]) # system tables should be hidden by default - self.assertEqual(proxy.rowCount(QModelIndex()), 3) + self.assertEqual(proxy.rowCount(QModelIndex()), 4) self.assertEqual(proxy.data(proxy.index(0, 0), Qt.DisplayRole), 'item name 1') self.assertEqual(proxy.data(proxy.index(1, 0), Qt.DisplayRole), 'another layer 2') self.assertEqual(proxy.data(proxy.index(2, 0), Qt.DisplayRole), 'layer 1') + self.assertEqual(proxy.data(proxy.index(3, 0), Qt.DisplayRole), 'yet another layer 3') proxy.setIncludeSystemTables(True) + self.assertEqual(proxy.rowCount(QModelIndex()), 5) + self.assertEqual(proxy.data(proxy.index(0, 0), Qt.DisplayRole), 'item name 1') + self.assertEqual(proxy.data(proxy.index(1, 0), Qt.DisplayRole), 'another layer 2') + self.assertEqual(proxy.data(proxy.index(2, 0), Qt.DisplayRole), 'layer 1') + self.assertEqual(proxy.data(proxy.index(3, 0), Qt.DisplayRole), 'system table') + self.assertEqual(proxy.data(proxy.index(4, 0), Qt.DisplayRole), 'yet another layer 3') + + # hide empty vector layers + proxy.setIncludeEmptyLayers(False) self.assertEqual(proxy.rowCount(QModelIndex()), 4) self.assertEqual(proxy.data(proxy.index(0, 0), Qt.DisplayRole), 'item name 1') self.assertEqual(proxy.data(proxy.index(1, 0), Qt.DisplayRole), 'another layer 2')