[processing] Also hide empty providers in toolbox

nyalldawson · nyalldawson · commit c76410bdbd38 · 2018-09-17T12:53:05.000+10:00
Initially empty providers were shown for discoverability, but discussion with @nirvn and @elpaso have convinced me this is a bad approach.
diff --git a/src/gui/processing/qgsprocessingtoolboxmodel.cpp b/src/gui/processing/qgsprocessingtoolboxmodel.cpp
@@ -742,8 +742,7 @@ bool QgsProcessingToolboxProxyModel::filterAcceptsRow( int sourceRow, const QMod
   }
 
   bool hasChildren = false;
-  // groups are shown only if they have visible children
-  // but providers are shown if they have visible children, OR the filter string is empty
+  // groups/providers are shown only if they have visible children
   int count = sourceModel()->rowCount( sourceIndex );
   for ( int i = 0; i < count; ++i )
   {
@@ -754,15 +753,7 @@ bool QgsProcessingToolboxProxyModel::filterAcceptsRow( int sourceRow, const QMod
     }
   }
 
-  if ( QgsProcessingProvider *provider = mModel->providerForIndex( sourceIndex ) )
-  {
-    return ( hasChildren || mFilterString.trimmed().isEmpty() ) && provider->isActive();
-  }
-  else
-  {
-    // group
-    return hasChildren; // || isRecentNode;
-  }
+  return hasChildren;
 }
 
 bool QgsProcessingToolboxProxyModel::lessThan( const QModelIndex &left, const QModelIndex &right ) const
diff --git a/tests/src/gui/testqgsprocessingmodel.cpp b/tests/src/gui/testqgsprocessingmodel.cpp
@@ -425,17 +425,13 @@ void TestQgsProcessingModel::testProxyModel()
   QModelIndex alg2Index = model.index( 0, 0, group1Index );
   QCOMPARE( model.data( alg2Index, Qt::DisplayRole ).toString(), QStringLiteral( "a2" ) );
 
+  // empty providers/groups should not be shown
   model.setFilters( QgsProcessingToolboxProxyModel::FilterModeler );
   group1Index = model.index( 0, 0, QModelIndex() );
-  QCOMPARE( model.rowCount(), 3 );
+  QCOMPARE( model.rowCount(), 1 );
   QCOMPARE( model.rowCount( group1Index ), 1 );
   QCOMPARE( model.data( group1Index, Qt::DisplayRole ).toString(), QStringLiteral( "group1" ) );
   QCOMPARE( model.data( model.index( 0, 0, group1Index ), Qt::DisplayRole ).toString(), QStringLiteral( "a2" ) );
-  // no filter string, so empty providers should be shown
-  QCOMPARE( model.data( model.index( 1, 0, QModelIndex() ), Qt::DisplayRole ).toString(), QStringLiteral( "provider1" ) );
-  QCOMPARE( model.rowCount( model.index( 1, 0, QModelIndex() ) ), 0 );
-  QCOMPARE( model.data( model.index( 2, 0, QModelIndex() ), Qt::DisplayRole ).toString(), QStringLiteral( "provider2" ) );
-  QCOMPARE( model.rowCount( model.index( 2, 0, QModelIndex() ) ), 0 );
   model.setFilters( QgsProcessingToolboxProxyModel::FilterToolbox );
   group2Index = model.index( 0, 0, QModelIndex() );
   QCOMPARE( model.rowCount(), 3 );
@@ -590,23 +586,20 @@ void TestQgsProcessingModel::testView()
   QModelIndex alg1Index = view.model()->index( 0, 0, group2Index );
   QCOMPARE( view.algorithmForIndex( alg1Index )->id(), QStringLiteral( "p2:a1" ) );
 
+  // empty providers/groups should not be shown
   view.setFilters( QgsProcessingToolboxProxyModel::FilterModeler );
-  QCOMPARE( view.model()->rowCount(), 2 );
-  QCOMPARE( view.model()->data( view.model()->index( 0, 0, QModelIndex() ), Qt::DisplayRole ).toString(), QStringLiteral( "provider1" ) );
-  QCOMPARE( view.model()->rowCount( view.model()->index( 0, 0, QModelIndex() ) ), 0 );
-  provider2Index = view.model()->index( 1, 0, QModelIndex() );
+  QCOMPARE( view.model()->rowCount(), 1 );
+  provider2Index = view.model()->index( 0, 0, QModelIndex() );
   QCOMPARE( view.model()->data( provider2Index, Qt::DisplayRole ).toString(), QStringLiteral( "provider2" ) );
   QCOMPARE( view.model()->rowCount( provider2Index ), 1 );
   group2Index = view.model()->index( 0, 0, provider2Index );
   QCOMPARE( view.model()->rowCount( group2Index ), 1 );
   QCOMPARE( view.algorithmForIndex( view.model()->index( 0, 0, group2Index ) )->id(), QStringLiteral( "p2:a1" ) );
   view.setFilters( QgsProcessingToolboxProxyModel::FilterToolbox );
-  QCOMPARE( view.model()->rowCount(), 2 );
+  QCOMPARE( view.model()->rowCount(), 1 );
   provider1Index = view.model()->index( 0, 0, QModelIndex() );
   QCOMPARE( view.model()->data( provider1Index, Qt::DisplayRole ).toString(), QStringLiteral( "provider1" ) );
   QCOMPARE( view.model()->rowCount( provider1Index ), 1 );
-  QCOMPARE( view.model()->data( view.model()->index( 1, 0, QModelIndex() ), Qt::DisplayRole ).toString(), QStringLiteral( "provider2" ) );
-  QCOMPARE( view.model()->rowCount( view.model()->index( 1, 0, QModelIndex() ) ), 0 );
   group2Index = view.model()->index( 0, 0, provider1Index );
   QCOMPARE( view.model()->rowCount( group2Index ), 1 );
   QCOMPARE( view.algorithmForIndex( view.model()->index( 0, 0, group2Index ) )->id(), QStringLiteral( "p1:a2" ) );

Original file line number	Diff line number	Diff line change
`@@ -742,8 +742,7 @@ bool QgsProcessingToolboxProxyModel::filterAcceptsRow( int sourceRow, const QMod`
`742`	`742`	`}`
`743`	`743`
`744`	`744`	`bool hasChildren = false;`
`745`		`- // groups are shown only if they have visible children`
`746`		`- // but providers are shown if they have visible children, OR the filter string is empty`
	`745`	`+ // groups/providers are shown only if they have visible children`
`747`	`746`	`int count = sourceModel()->rowCount( sourceIndex );`
`748`	`747`	`for ( int i = 0; i < count; ++i )`
`749`	`748`	`{`
`@@ -754,15 +753,7 @@ bool QgsProcessingToolboxProxyModel::filterAcceptsRow( int sourceRow, const QMod`
`754`	`753`	`}`
`755`	`754`	`}`
`756`	`755`
`757`		`- if ( QgsProcessingProvider *provider = mModel->providerForIndex( sourceIndex ) )`
`758`		`- {`
`759`		`- return ( hasChildren \|\| mFilterString.trimmed().isEmpty() ) && provider->isActive();`
`760`		`- }`
`761`		`- else`
`762`		`- {`
`763`		`- // group`
`764`		`- return hasChildren; // \|\| isRecentNode;`
`765`		`- }`
	`756`	`+ return hasChildren;`
`766`	`757`	`}`
`767`	`758`
`768`	`759`	`bool QgsProcessingToolboxProxyModel::lessThan( const QModelIndex &left, const QModelIndex &right ) const`