Fix sorting of recently used algorithms

nyalldawson · nyalldawson · commit e304539dd8e7 · 2018-07-16T11:38:13.000+10:00
diff --git a/python/gui/auto_generated/processing/qgsprocessingtoolboxmodel.sip.in b/python/gui/auto_generated/processing/qgsprocessingtoolboxmodel.sip.in
@@ -346,6 +346,13 @@ Returns the index corresponding to the specified ``providerId``.
     QModelIndex indexOfParentTreeNode( QgsProcessingToolboxModelNode *parentNode ) const;
 %Docstring
 Returns the index corresponding to the parent of a given node.
+%End
+
+  signals:
+
+    void recentAlgorithmAdded();
+%Docstring
+Emitted whenever recent algorithms are added to the model.
 %End
 
 };
diff --git a/src/gui/processing/qgsprocessingtoolboxmodel.cpp b/src/gui/processing/qgsprocessingtoolboxmodel.cpp
@@ -135,7 +135,7 @@ void QgsProcessingToolboxModel::rebuild()
     std::unique_ptr< QgsProcessingToolboxModelRecentNode > recentNode = qgis::make_unique< QgsProcessingToolboxModelRecentNode >();
     mRecentNode = recentNode.get();
     mRootNode->addChildNode( recentNode.release() );
-    repopulateRecentAlgorithms();
+    repopulateRecentAlgorithms( true );
   }
 
   const QList< QgsProcessingProvider * > providers = mRegistry->providers();
@@ -171,7 +171,10 @@ void QgsProcessingToolboxModel::repopulateRecentAlgorithms( bool resetting )
   }
 
   if ( recentAlgorithms.empty() )
+  {
+    emit recentAlgorithmAdded();
     return;
+  }
 
   if ( !resetting )
   {
@@ -180,15 +183,15 @@ void QgsProcessingToolboxModel::repopulateRecentAlgorithms( bool resetting )
 
   for ( const QgsProcessingAlgorithm *algorithm : qgis::as_const( recentAlgorithms ) )
   {
-    std::unique_ptr< QgsProcessingToolboxModelAlgorithmNode > algorithmNode = qgis::make_unique< QgsProcessingToolboxModelAlgorithmNode >( algorithm, mRegistry );
+    std::unique_ptr< QgsProcessingToolboxModelAlgorithmNode > algorithmNode = qgis::make_unique< QgsProcessingToolboxModelAlgorithmNode >( algorithm );
     mRecentNode->addChildNode( algorithmNode.release() );
   }
 
   if ( !resetting )
   {
     endInsertRows();
   }
-
+  emit recentAlgorithmAdded();
 }
 
 void QgsProcessingToolboxModel::providerAdded( const QString &id )
@@ -615,6 +618,8 @@ QgsProcessingToolboxProxyModel::QgsProcessingToolboxProxyModel( QObject *parent,
   setSortLocaleAware( true );
   setFilterCaseSensitivity( Qt::CaseInsensitive );
   sort( 0 );
+
+  connect( mModel, &QgsProcessingToolboxModel::recentAlgorithmAdded, this, [ = ] { invalidateFilter(); } );
 }
 
 void QgsProcessingToolboxProxyModel::setFilters( QgsProcessingToolboxProxyModel::Filters filters )
@@ -701,8 +706,6 @@ bool QgsProcessingToolboxProxyModel::filterAcceptsRow( int sourceRow, const QMod
     }
   }
 
-  bool isRecentNode = mModel->data( sourceIndex, QgsProcessingToolboxModel::RoleNodeType ).toInt() == QgsProcessingToolboxModelNode::NodeRecent;
-
   if ( QgsProcessingProvider *provider = mModel->providerForIndex( sourceIndex ) )
   {
     return hasChildren && provider->isActive();
@@ -735,6 +738,23 @@ bool QgsProcessingToolboxProxyModel::lessThan( const QModelIndex &left, const QM
       return true;
   }
 
+  // if node represents a recent algorithm, it's not sorted at all
+  bool isRecentNode = false;
+  QModelIndex parent = left.parent();
+  while ( parent.isValid() )
+  {
+    if ( mModel->data( parent, QgsProcessingToolboxModel::RoleNodeType ).toInt() == QgsProcessingToolboxModelNode::NodeRecent )
+    {
+      isRecentNode = true;
+      break;
+    }
+  }
+  if ( isRecentNode )
+  {
+    return left.row() < right.row();
+  }
+
+
   // default mode is alphabetical order
   QString leftStr = sourceModel()->data( left ).toString();
   QString rightStr = sourceModel()->data( right ).toString();
diff --git a/src/gui/processing/qgsprocessingtoolboxmodel.h b/src/gui/processing/qgsprocessingtoolboxmodel.h
@@ -359,6 +359,13 @@ class GUI_EXPORT QgsProcessingToolboxModel : public QAbstractItemModel
      */
     QModelIndex indexOfParentTreeNode( QgsProcessingToolboxModelNode *parentNode ) const;
 
+  signals:
+
+    /**
+     * Emitted whenever recent algorithms are added to the model.
+     */
+    void recentAlgorithmAdded();
+
   private slots:
 
     void rebuild();
diff --git a/tests/src/gui/testqgsprocessingmodel.cpp b/tests/src/gui/testqgsprocessingmodel.cpp
@@ -519,18 +519,34 @@ void TestQgsProcessingModel::testProxyModel()
   QCOMPARE( model.rowCount(), 4 );
 
   // check sort order of recent algorithms
-  recentLog.push( QStringLiteral( "qgis:a1" ) );
+  recentLog.push( QStringLiteral( "p2:a1" ) );
   QCOMPARE( model.rowCount(), 5 );
   QModelIndex recentIndex = model.index( 0, 0, QModelIndex() );
   QCOMPARE( model.data( recentIndex, Qt::DisplayRole ).toString(), QStringLiteral( "Recently used" ) );
+  QCOMPARE( model.rowCount( recentIndex ), 1 );
+  QCOMPARE( model.data( model.index( 0, 0, recentIndex ), QgsProcessingToolboxModel::RoleAlgorithmId ).toString(), QStringLiteral( "p2:a1" ) );
+  recentLog.push( QStringLiteral( "p1:a2" ) );
+  QCOMPARE( model.rowCount( recentIndex ), 2 );
+  QCOMPARE( model.data( model.index( 0, 0, recentIndex ), QgsProcessingToolboxModel::RoleAlgorithmId ).toString(), QStringLiteral( "p1:a2" ) );
+  QCOMPARE( model.data( model.index( 1, 0, recentIndex ), QgsProcessingToolboxModel::RoleAlgorithmId ).toString(), QStringLiteral( "p2:a1" ) );
+  recentLog.push( QStringLiteral( "qgis:a1" ) );
+  QCOMPARE( model.rowCount( recentIndex ), 3 );
+  QCOMPARE( model.data( model.index( 0, 0, recentIndex ), QgsProcessingToolboxModel::RoleAlgorithmId ).toString(), QStringLiteral( "qgis:a1" ) );
+  QCOMPARE( model.data( model.index( 1, 0, recentIndex ), QgsProcessingToolboxModel::RoleAlgorithmId ).toString(), QStringLiteral( "p1:a2" ) );
+  QCOMPARE( model.data( model.index( 2, 0, recentIndex ), QgsProcessingToolboxModel::RoleAlgorithmId ).toString(), QStringLiteral( "p2:a1" ) );
+  recentLog.push( QStringLiteral( "p2:a1" ) );
+  QCOMPARE( model.rowCount( recentIndex ), 3 );
+  QCOMPARE( model.data( model.index( 0, 0, recentIndex ), QgsProcessingToolboxModel::RoleAlgorithmId ).toString(), QStringLiteral( "p2:a1" ) );
+  QCOMPARE( model.data( model.index( 1, 0, recentIndex ), QgsProcessingToolboxModel::RoleAlgorithmId ).toString(), QStringLiteral( "qgis:a1" ) );
+  QCOMPARE( model.data( model.index( 2, 0, recentIndex ), QgsProcessingToolboxModel::RoleAlgorithmId ).toString(), QStringLiteral( "p1:a2" ) );
 
   // inactive provider - should not be visible
-  QCOMPARE( model.rowCount(), 4 );
+  QCOMPARE( model.rowCount(), 5 );
   DummyAlgorithm *qgisA31 = new DummyAlgorithm( "a3", "group1" );
   DummyProvider *p3 = new DummyProvider( "p3", "provider3", QList< QgsProcessingAlgorithm * >() << qgisA31 );
   p3->mActive = false;
   registry.addProvider( p3 );
-  QCOMPARE( model.rowCount(), 4 );
+  QCOMPARE( model.rowCount(), 5 );
 }
 
 QGSTEST_MAIN( TestQgsProcessingModel )

Original file line number	Diff line number	Diff line change
`@@ -135,7 +135,7 @@ void QgsProcessingToolboxModel::rebuild()`
`135`	`135`	`std::unique_ptr< QgsProcessingToolboxModelRecentNode > recentNode = qgis::make_unique< QgsProcessingToolboxModelRecentNode >();`
`136`	`136`	`mRecentNode = recentNode.get();`
`137`	`137`	`mRootNode->addChildNode( recentNode.release() );`
`138`		`- repopulateRecentAlgorithms();`
	`138`	`+ repopulateRecentAlgorithms( true );`
`139`	`139`	`}`
`140`	`140`
`141`	`141`	`const QList< QgsProcessingProvider * > providers = mRegistry->providers();`
`@@ -171,7 +171,10 @@ void QgsProcessingToolboxModel::repopulateRecentAlgorithms( bool resetting )`
`171`	`171`	`}`
`172`	`172`
`173`	`173`	`if ( recentAlgorithms.empty() )`
	`174`	`+ {`
	`175`	`+ emit recentAlgorithmAdded();`
`174`	`176`	`return;`
	`177`	`+ }`
`175`	`178`
`176`	`179`	`if ( !resetting )`
`177`	`180`	`{`
`@@ -180,15 +183,15 @@ void QgsProcessingToolboxModel::repopulateRecentAlgorithms( bool resetting )`
`180`	`183`
`181`	`184`	`for ( const QgsProcessingAlgorithm *algorithm : qgis::as_const( recentAlgorithms ) )`
`182`	`185`	`{`
`183`		`- std::unique_ptr< QgsProcessingToolboxModelAlgorithmNode > algorithmNode = qgis::make_unique< QgsProcessingToolboxModelAlgorithmNode >( algorithm, mRegistry );`
	`186`	`+ std::unique_ptr< QgsProcessingToolboxModelAlgorithmNode > algorithmNode = qgis::make_unique< QgsProcessingToolboxModelAlgorithmNode >( algorithm );`
`184`	`187`	`mRecentNode->addChildNode( algorithmNode.release() );`
`185`	`188`	`}`
`186`	`189`
`187`	`190`	`if ( !resetting )`
`188`	`191`	`{`
`189`	`192`	`endInsertRows();`
`190`	`193`	`}`
`191`		`-`
	`194`	`+ emit recentAlgorithmAdded();`
`192`	`195`	`}`
`193`	`196`
`194`	`197`	`void QgsProcessingToolboxModel::providerAdded( const QString &id )`
`@@ -615,6 +618,8 @@ QgsProcessingToolboxProxyModel::QgsProcessingToolboxProxyModel( QObject *parent,`
`615`	`618`	`setSortLocaleAware( true );`
`616`	`619`	`setFilterCaseSensitivity( Qt::CaseInsensitive );`
`617`	`620`	`sort( 0 );`
	`621`	`+`
	`622`	`+ connect( mModel, &QgsProcessingToolboxModel::recentAlgorithmAdded, this, [ = ] { invalidateFilter(); } );`
`618`	`623`	`}`
`619`	`624`
`620`	`625`	`void QgsProcessingToolboxProxyModel::setFilters( QgsProcessingToolboxProxyModel::Filters filters )`
`@@ -701,8 +706,6 @@ bool QgsProcessingToolboxProxyModel::filterAcceptsRow( int sourceRow, const QMod`
`701`	`706`	`}`
`702`	`707`	`}`
`703`	`708`
`704`		`- bool isRecentNode = mModel->data( sourceIndex, QgsProcessingToolboxModel::RoleNodeType ).toInt() == QgsProcessingToolboxModelNode::NodeRecent;`
`705`		`-`
`706`	`709`	`if ( QgsProcessingProvider *provider = mModel->providerForIndex( sourceIndex ) )`
`707`	`710`	`{`
`708`	`711`	`return hasChildren && provider->isActive();`
`@@ -735,6 +738,23 @@ bool QgsProcessingToolboxProxyModel::lessThan( const QModelIndex &left, const QM`
`735`	`738`	`return true;`
`736`	`739`	`}`
`737`	`740`
	`741`	`+ // if node represents a recent algorithm, it's not sorted at all`
	`742`	`+ bool isRecentNode = false;`
	`743`	`+ QModelIndex parent = left.parent();`
	`744`	`+ while ( parent.isValid() )`
	`745`	`+ {`
	`746`	`+ if ( mModel->data( parent, QgsProcessingToolboxModel::RoleNodeType ).toInt() == QgsProcessingToolboxModelNode::NodeRecent )`
	`747`	`+ {`
	`748`	`+ isRecentNode = true;`
	`749`	`+ break;`
	`750`	`+ }`
	`751`	`+ }`
	`752`	`+ if ( isRecentNode )`
	`753`	`+ {`
	`754`	`+ return left.row() < right.row();`
	`755`	`+ }`
	`756`	`+`
	`757`	`+`
`738`	`758`	`// default mode is alphabetical order`
`739`	`759`	`QString leftStr = sourceModel()->data( left ).toString();`
`740`	`760`	`QString rightStr = sourceModel()->data( right ).toString();`