From 63d648738d893890a48ab3fea175ded725f648e2 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Tue, 5 Mar 2019 17:04:00 +1000 Subject: [PATCH] [processing][needs-docs] By default, hide algorithms with known issues from toolbox And add a Processing setting to allow these to be shown. When shown, they are highlighted in red with a tooltip explaining that the algorithm has known issues --- .../qgsprocessingtoolboxmodel.sip.in | 1 + .../qgsprocessingtoolboxtreeview.sip.in | 10 +++++ .../processing/core/ProcessingConfig.py | 5 +++ .../processing/gui/AlgorithmLocatorFilter.py | 4 ++ .../processing/gui/ProcessingToolbox.py | 28 ++++++++++--- .../processing/modeler/ModelerDialog.py | 6 ++- .../processing/qgsprocessingtoolboxmodel.cpp | 23 ++++++++++- .../processing/qgsprocessingtoolboxmodel.h | 1 + .../qgsprocessingtoolboxtreeview.cpp | 5 +++ .../processing/qgsprocessingtoolboxtreeview.h | 7 ++++ tests/src/gui/testqgsprocessingmodel.cpp | 41 +++++++++++++++++++ 11 files changed, 122 insertions(+), 9 deletions(-) diff --git a/python/gui/auto_generated/processing/qgsprocessingtoolboxmodel.sip.in b/python/gui/auto_generated/processing/qgsprocessingtoolboxmodel.sip.in index 80f82b6d44a..bc8d859dcc3 100644 --- a/python/gui/auto_generated/processing/qgsprocessingtoolboxmodel.sip.in +++ b/python/gui/auto_generated/processing/qgsprocessingtoolboxmodel.sip.in @@ -380,6 +380,7 @@ the results. FilterToolbox, FilterModeler, FilterInPlace, + FilterShowKnownIssues, }; typedef QFlags Filters; diff --git a/python/gui/auto_generated/processing/qgsprocessingtoolboxtreeview.sip.in b/python/gui/auto_generated/processing/qgsprocessingtoolboxtreeview.sip.in index 71a8bb193d0..7298216e4d5 100644 --- a/python/gui/auto_generated/processing/qgsprocessingtoolboxtreeview.sip.in +++ b/python/gui/auto_generated/processing/qgsprocessingtoolboxtreeview.sip.in @@ -71,8 +71,18 @@ if no algorithm is currently selected. void setFilters( QgsProcessingToolboxProxyModel::Filters filters ); %Docstring Sets ``filters`` controlling the view's contents. + +.. seealso:: :py:func:`filters` %End + QgsProcessingToolboxProxyModel::Filters filters() const; +%Docstring +Returns the current filters controlling the view's contents. + +.. seealso:: :py:func:`setFilters` + +.. versionadded:: 3.8 +%End void setInPlaceLayer( QgsVectorLayer *layer ); %Docstring diff --git a/python/plugins/processing/core/ProcessingConfig.py b/python/plugins/processing/core/ProcessingConfig.py index 3b347bc6644..a61d518f9b9 100644 --- a/python/plugins/processing/core/ProcessingConfig.py +++ b/python/plugins/processing/core/ProcessingConfig.py @@ -59,6 +59,7 @@ class ProcessingConfig: SHOW_CRS_DEF = 'SHOW_CRS_DEF' WARN_UNMATCHING_CRS = 'WARN_UNMATCHING_CRS' SHOW_PROVIDERS_TOOLTIP = 'SHOW_PROVIDERS_TOOLTIP' + SHOW_ALGORITHMS_KNOWN_ISSUES = 'SHOW_ALGORITHMS_KNOWN_ISSUES' settings = {} settingIcons = {} @@ -92,6 +93,10 @@ class ProcessingConfig: ProcessingConfig.tr('General'), ProcessingConfig.WARN_UNMATCHING_CRS, ProcessingConfig.tr("Warn before executing if parameter CRS's do not match"), True)) + ProcessingConfig.addSetting(Setting( + ProcessingConfig.tr('General'), + ProcessingConfig.SHOW_ALGORITHMS_KNOWN_ISSUES, + ProcessingConfig.tr("Show algorithms with known issues"), False)) ProcessingConfig.addSetting(Setting( ProcessingConfig.tr('General'), ProcessingConfig.RASTER_STYLE, diff --git a/python/plugins/processing/gui/AlgorithmLocatorFilter.py b/python/plugins/processing/gui/AlgorithmLocatorFilter.py index d75edbf98b4..ff1ee4bfbdf 100644 --- a/python/plugins/processing/gui/AlgorithmLocatorFilter.py +++ b/python/plugins/processing/gui/AlgorithmLocatorFilter.py @@ -40,6 +40,7 @@ from processing.gui.MessageDialog import MessageDialog from processing.gui.AlgorithmDialog import AlgorithmDialog from processing.gui.AlgorithmExecutor import execute_in_place from qgis.utils import iface +from processing.core.ProcessingConfig import ProcessingConfig class AlgorithmLocatorFilter(QgsLocatorFilter): @@ -71,6 +72,9 @@ class AlgorithmLocatorFilter(QgsLocatorFilter): for a in QgsApplication.processingRegistry().algorithms(): if a.flags() & QgsProcessingAlgorithm.FlagHideFromToolbox: continue + if not ProcessingConfig.getSetting(ProcessingConfig.SHOW_ALGORITHMS_KNOWN_ISSUES) and \ + a.flags() & QgsProcessingAlgorithm.FlagKnownIssues: + continue if QgsLocatorFilter.stringMatches(a.displayName(), string) or [t for t in a.tags() if QgsLocatorFilter.stringMatches(t, string)] or \ (context.usingPrefix and not string): diff --git a/python/plugins/processing/gui/ProcessingToolbox.py b/python/plugins/processing/gui/ProcessingToolbox.py index 7811b903d72..5c6f472f5ec 100644 --- a/python/plugins/processing/gui/ProcessingToolbox.py +++ b/python/plugins/processing/gui/ProcessingToolbox.py @@ -81,11 +81,14 @@ class ProcessingToolbox(QgsDockWidget, WIDGET): self.algorithmTree.setRegistry(QgsApplication.processingRegistry(), QgsGui.instance().processingRecentAlgorithmLog()) - self.algorithmTree.setFilters(QgsProcessingToolboxProxyModel.FilterToolbox) + filters = QgsProcessingToolboxProxyModel.Filters(QgsProcessingToolboxProxyModel.FilterToolbox) + if ProcessingConfig.getSetting(ProcessingConfig.SHOW_ALGORITHMS_KNOWN_ISSUES): + filters |= QgsProcessingToolboxProxyModel.FilterShowKnownIssues + self.algorithmTree.setFilters(filters) self.searchBox.setShowSearchIcon(True) - self.searchBox.textChanged.connect(self.algorithmTree.setFilterString) + self.searchBox.textChanged.connect(self.set_filter_string) self.searchBox.returnPressed.connect(self.activateCurrent) self.algorithmTree.customContextMenuRequested.connect( self.showPopupMenu) @@ -114,11 +117,24 @@ class ProcessingToolbox(QgsDockWidget, WIDGET): iface.currentLayerChanged.connect(self.layer_changed) - def set_in_place_edit_mode(self, enabled): - if enabled: - self.algorithmTree.setFilters(QgsProcessingToolboxProxyModel.Filters(QgsProcessingToolboxProxyModel.FilterToolbox | QgsProcessingToolboxProxyModel.FilterInPlace)) + def set_filter_string(self, string): + filters = self.algorithmTree.filters() + if ProcessingConfig.getSetting(ProcessingConfig.SHOW_ALGORITHMS_KNOWN_ISSUES): + filters |= QgsProcessingToolboxProxyModel.FilterShowKnownIssues else: - self.algorithmTree.setFilters(QgsProcessingToolboxProxyModel.FilterToolbox) + filters &= ~QgsProcessingToolboxProxyModel.FilterShowKnownIssues + self.algorithmTree.setFilters(filters) + self.algorithmTree.setFilterString(string) + + def set_in_place_edit_mode(self, enabled): + filters = QgsProcessingToolboxProxyModel.Filters(QgsProcessingToolboxProxyModel.FilterToolbox) + if ProcessingConfig.getSetting(ProcessingConfig.SHOW_ALGORITHMS_KNOWN_ISSUES): + filters |= QgsProcessingToolboxProxyModel.FilterShowKnownIssues + + if enabled: + self.algorithmTree.setFilters(filters | QgsProcessingToolboxProxyModel.FilterInPlace) + else: + self.algorithmTree.setFilters(filters) self.in_place_mode = enabled def layer_changed(self, layer): diff --git a/python/plugins/processing/modeler/ModelerDialog.py b/python/plugins/processing/modeler/ModelerDialog.py index 95e6f9127ff..f7d0c2f58af 100644 --- a/python/plugins/processing/modeler/ModelerDialog.py +++ b/python/plugins/processing/modeler/ModelerDialog.py @@ -99,6 +99,7 @@ from processing.modeler.ModelerUtils import ModelerUtils from processing.modeler.ModelerScene import ModelerScene from processing.modeler.ProjectProvider import PROJECT_PROVIDER_ID from processing.script.ScriptEditorDialog import ScriptEditorDialog +from processing.core.ProcessingConfig import ProcessingConfig from qgis.utils import iface @@ -439,7 +440,10 @@ class ModelerDialog(BASE, WIDGET): self.algorithmTree.setDragDropMode(QTreeWidget.DragOnly) self.algorithmTree.setDropIndicatorShown(True) - self.algorithmTree.setFilters(QgsProcessingToolboxProxyModel.FilterModeler) + filters = QgsProcessingToolboxProxyModel.Filters(QgsProcessingToolboxProxyModel.FilterModeler) + if ProcessingConfig.getSetting(ProcessingConfig.SHOW_ALGORITHMS_KNOWN_ISSUES): + filters |= QgsProcessingToolboxProxyModel.FilterShowKnownIssues + self.algorithmTree.setFilters(filters) if hasattr(self.searchBox, 'setPlaceholderText'): self.searchBox.setPlaceholderText(QCoreApplication.translate('ModelerDialog', 'Search…')) diff --git a/src/gui/processing/qgsprocessingtoolboxmodel.cpp b/src/gui/processing/qgsprocessingtoolboxmodel.cpp index 0d32617fe24..d334eff1b20 100644 --- a/src/gui/processing/qgsprocessingtoolboxmodel.cpp +++ b/src/gui/processing/qgsprocessingtoolboxmodel.cpp @@ -316,10 +316,11 @@ bool QgsProcessingToolboxModel::isTopLevelProvider( const QString &providerId ) QString QgsProcessingToolboxModel::toolTipForAlgorithm( const QgsProcessingAlgorithm *algorithm ) { - return QStringLiteral( "

%1

%2

%3

" ).arg( + return QStringLiteral( "

%1

%2

%3

%4" ).arg( algorithm->displayName(), !algorithm->shortDescription().isEmpty() ? QStringLiteral( "

%1

" ).arg( algorithm->shortDescription() ) : QString(), - QObject::tr( "Algorithm ID: ‘%1’" ).arg( QStringLiteral( "%1" ).arg( algorithm->id() ) ) + QObject::tr( "Algorithm ID: ‘%1’" ).arg( QStringLiteral( "%1" ).arg( algorithm->id() ) ), + algorithm->flags() & QgsProcessingAlgorithm::FlagKnownIssues ? QStringLiteral( "%1" ).arg( QObject::tr( "Warning: Algorithm has known issues" ) ) : QString() ); } @@ -388,7 +389,16 @@ QVariant QgsProcessingToolboxModel::data( const QModelIndex &index, int role ) c return QVariant(); } + case Qt::ForegroundRole: + { + if ( algorithm && algorithm->flags() & QgsProcessingAlgorithm::FlagKnownIssues ) + return QBrush( QColor( Qt::red ) ); + else + return QVariant(); + } + case Qt::DecorationRole: + { switch ( index.column() ) { case 0: @@ -396,7 +406,11 @@ QVariant QgsProcessingToolboxModel::data( const QModelIndex &index, int role ) c if ( provider ) return provider->icon(); else if ( algorithm ) + { + if ( algorithm->flags() & QgsProcessingAlgorithm::FlagKnownIssues ) + return QgsApplication::getThemeIcon( QStringLiteral( "mIconWarning.svg" ) ); return algorithm->icon(); + } else if ( isRecentNode ) return QgsApplication::getThemeIcon( QStringLiteral( "/mIconHistory.svg" ) ); else if ( !index.parent().isValid() ) @@ -410,6 +424,7 @@ QVariant QgsProcessingToolboxModel::data( const QModelIndex &index, int role ) c return QVariant(); } break; + } case RoleAlgorithmFlags: switch ( index.column() ) @@ -674,6 +689,10 @@ bool QgsProcessingToolboxProxyModel::filterAcceptsRow( int sourceRow, const QMod QModelIndex sourceIndex = mModel->index( sourceRow, 0, sourceParent ); if ( mModel->isAlgorithm( sourceIndex ) ) { + const bool hasKnownIssues = sourceModel()->data( sourceIndex, QgsProcessingToolboxModel::RoleAlgorithmFlags ).toInt() & QgsProcessingAlgorithm::FlagKnownIssues; + if ( hasKnownIssues && !( mFilters & FilterShowKnownIssues ) ) + return false; + if ( !mFilterString.trimmed().isEmpty() ) { const QString algId = sourceModel()->data( sourceIndex, QgsProcessingToolboxModel::RoleAlgorithmId ).toString(); diff --git a/src/gui/processing/qgsprocessingtoolboxmodel.h b/src/gui/processing/qgsprocessingtoolboxmodel.h index 2e7bbe236db..75a0ae93e8b 100644 --- a/src/gui/processing/qgsprocessingtoolboxmodel.h +++ b/src/gui/processing/qgsprocessingtoolboxmodel.h @@ -425,6 +425,7 @@ class GUI_EXPORT QgsProcessingToolboxProxyModel: public QSortFilterProxyModel FilterToolbox = 1 << 1, //!< Filters out any algorithms and content which should not be shown in the toolbox FilterModeler = 1 << 2, //!< Filters out any algorithms and content which should not be shown in the modeler FilterInPlace = 1 << 3, //!< Only show algorithms which support in-place edits + FilterShowKnownIssues = 1 << 4, //!< Show algorithms with known issues (hidden by default) }; Q_DECLARE_FLAGS( Filters, Filter ) Q_FLAG( Filters ) diff --git a/src/gui/processing/qgsprocessingtoolboxtreeview.cpp b/src/gui/processing/qgsprocessingtoolboxtreeview.cpp index 47791dd3e86..8784f19e0cd 100644 --- a/src/gui/processing/qgsprocessingtoolboxtreeview.cpp +++ b/src/gui/processing/qgsprocessingtoolboxtreeview.cpp @@ -93,6 +93,11 @@ void QgsProcessingToolboxTreeView::setFilters( QgsProcessingToolboxProxyModel::F mModel->setFilters( filters ); } +QgsProcessingToolboxProxyModel::Filters QgsProcessingToolboxTreeView::filters() const +{ + return mModel->filters(); +} + void QgsProcessingToolboxTreeView::setInPlaceLayer( QgsVectorLayer *layer ) { mModel->setInPlaceLayer( layer ); diff --git a/src/gui/processing/qgsprocessingtoolboxtreeview.h b/src/gui/processing/qgsprocessingtoolboxtreeview.h index 806d6145597..c9160a1259d 100644 --- a/src/gui/processing/qgsprocessingtoolboxtreeview.h +++ b/src/gui/processing/qgsprocessingtoolboxtreeview.h @@ -82,9 +82,16 @@ class GUI_EXPORT QgsProcessingToolboxTreeView : public QTreeView /** * Sets \a filters controlling the view's contents. + * \see filters() */ void setFilters( QgsProcessingToolboxProxyModel::Filters filters ); + /** + * Returns the current filters controlling the view's contents. + * \see setFilters() + * \since QGIS 3.8 + */ + QgsProcessingToolboxProxyModel::Filters filters() const; /** * Sets the vector \a layer for the in-place algorithms diff --git a/tests/src/gui/testqgsprocessingmodel.cpp b/tests/src/gui/testqgsprocessingmodel.cpp index b7a6ebe5531..5770e9768fd 100644 --- a/tests/src/gui/testqgsprocessingmodel.cpp +++ b/tests/src/gui/testqgsprocessingmodel.cpp @@ -113,6 +113,7 @@ class TestQgsProcessingModel: public QObject void init() {} // will be called before each testfunction is executed. void cleanup() {} // will be called after every testfunction. void testModel(); + void testKnownIssues(); void testProxyModel(); void testView(); }; @@ -597,6 +598,7 @@ void TestQgsProcessingModel::testView() // empty providers/groups should not be shown view.setFilters( QgsProcessingToolboxProxyModel::FilterModeler ); + QCOMPARE( view.filters(), QgsProcessingToolboxProxyModel::FilterModeler ); QCOMPARE( view.model()->rowCount(), 1 ); provider2Index = view.model()->index( 0, 0, QModelIndex() ); QCOMPARE( view.model()->data( provider2Index, Qt::DisplayRole ).toString(), QStringLiteral( "provider2" ) ); @@ -614,6 +616,7 @@ void TestQgsProcessingModel::testView() QCOMPARE( view.algorithmForIndex( view.model()->index( 0, 0, group2Index ) )->id(), QStringLiteral( "p1:a2" ) ); view.setFilters( nullptr ); + QCOMPARE( view.filters(), QgsProcessingToolboxProxyModel::Filters() ); // test filter strings view.setFilterString( "a1" ); provider2Index = view.model()->index( 0, 0, QModelIndex() ); @@ -651,6 +654,44 @@ void TestQgsProcessingModel::testView() QVERIFY( view.mModel ); QVERIFY( view.mToolboxModel ); QCOMPARE( view.mModel->toolboxModel(), view.mToolboxModel ); +} + +void TestQgsProcessingModel::testKnownIssues() +{ + QgsProcessingRegistry registry; + QgsProcessingRecentAlgorithmLog recentLog; + QgsProcessingToolboxModel model( nullptr, ®istry, &recentLog ); + DummyAlgorithm *a1 = new DummyAlgorithm( "a1", "group1", QgsProcessingAlgorithm::FlagKnownIssues, QStringLiteral( "tag1,tag2" ), QStringLiteral( "short desc a" ) ); + DummyAlgorithm *a2 = new DummyAlgorithm( "b1", "group1", nullptr, QStringLiteral( "tag1,tag2" ), QStringLiteral( "short desc b" ) ); + DummyProvider *p = new DummyProvider( "p3", "provider3", QList< QgsProcessingAlgorithm * >() << a1 << a2 ); + registry.addProvider( p ); + + QModelIndex providerIndex = model.index( 1, 0, QModelIndex() ); + QModelIndex group1Index = model.index( 0, 0, providerIndex ); + QCOMPARE( model.data( model.index( 0, 0, group1Index ), Qt::DisplayRole ).toString(), QStringLiteral( "a1" ) ); + QVERIFY( model.data( model.index( 0, 0, group1Index ), Qt::ToolTipRole ).toString().contains( QStringLiteral( "known issues" ) ) ); + QCOMPARE( model.data( model.index( 0, 0, group1Index ), Qt::ForegroundRole ).value< QBrush >().color().name(), QStringLiteral( "#ff0000" ) ); + QCOMPARE( model.data( model.index( 1, 0, group1Index ), Qt::DisplayRole ).toString(), QStringLiteral( "b1" ) ); + QVERIFY( !model.data( model.index( 1, 0, group1Index ), Qt::ToolTipRole ).toString().contains( QStringLiteral( "known issues" ) ) ); + QCOMPARE( model.data( model.index( 1, 0, group1Index ), Qt::ForegroundRole ).value< QBrush >().color().name(), QStringLiteral( "#000000" ) ); + + QgsProcessingToolboxProxyModel proxyModel( nullptr, ®istry, &recentLog ); + providerIndex = proxyModel.index( 0, 0, QModelIndex() ); + group1Index = proxyModel.index( 0, 0, providerIndex ); + // by default known issues are filtered out + QCOMPARE( proxyModel.rowCount( group1Index ), 1 ); + QCOMPARE( proxyModel.data( proxyModel.index( 0, 0, group1Index ), Qt::DisplayRole ).toString(), QStringLiteral( "b1" ) ); + QVERIFY( !proxyModel.data( proxyModel.index( 0, 0, group1Index ), Qt::ToolTipRole ).toString().contains( QStringLiteral( "known issues" ) ) ); + QCOMPARE( proxyModel.data( proxyModel.index( 0, 0, group1Index ), Qt::ForegroundRole ).value< QBrush >().color().name(), QStringLiteral( "#000000" ) ); + proxyModel.setFilters( QgsProcessingToolboxProxyModel::Filters( QgsProcessingToolboxProxyModel::FilterToolbox | QgsProcessingToolboxProxyModel::FilterShowKnownIssues ) ); + QCOMPARE( proxyModel.rowCount( group1Index ), 2 ); + QCOMPARE( proxyModel.data( proxyModel.index( 0, 0, group1Index ), Qt::DisplayRole ).toString(), QStringLiteral( "a1" ) ); + QVERIFY( proxyModel.data( proxyModel.index( 0, 0, group1Index ), Qt::ToolTipRole ).toString().contains( QStringLiteral( "known issues" ) ) ); + QCOMPARE( proxyModel.data( proxyModel.index( 0, 0, group1Index ), Qt::ForegroundRole ).value< QBrush >().color().name(), QStringLiteral( "#ff0000" ) ); + QCOMPARE( proxyModel.data( proxyModel.index( 1, 0, group1Index ), Qt::DisplayRole ).toString(), QStringLiteral( "b1" ) ); + QVERIFY( !proxyModel.data( proxyModel.index( 1, 0, group1Index ), Qt::ToolTipRole ).toString().contains( QStringLiteral( "known issues" ) ) ); + QCOMPARE( proxyModel.data( proxyModel.index( 1, 0, group1Index ), Qt::ForegroundRole ).value< QBrush >().color().name(), QStringLiteral( "#000000" ) ); + }