Merge pull request #37929 from nyalldawson/fix_37912_refactor_fields

Fix expression previews in Refactor Fields and Aggregate algorithms
qgis · Jul 24, 2020 · a345ae9 · a345ae9
2 parents b6ae643 + 6dbc178
commit a345ae9
Show file tree

Hide file tree

Showing 12 changed files with 165 additions and 4 deletions.
diff --git a/python/gui/auto_generated/processing/qgsprocessingaggregatewidgets.sip.in b/python/gui/auto_generated/processing/qgsprocessingaggregatewidgets.sip.in
@@ -166,6 +166,22 @@ Returns the selection model
     void setSourceFields( const QgsFields &sourceFields );
 %Docstring
 Set source fields of the underlying mapping model to ``sourceFields``
+%End
+
+    void setSourceLayer( QgsVectorLayer *layer );
+%Docstring
+Sets a source ``layer`` to use when generating expression previews in the widget.
+
+.. versionadded:: 3.16
+%End
+
+    QgsVectorLayer *sourceLayer();
+%Docstring
+Returns the source layer for use when generating expression previews.
+
+Returned value may be ``None``.
+
+.. versionadded:: 3.16
 %End
 
     void scrollTo( const QModelIndex &index ) const;

diff --git a/python/gui/auto_generated/qgsfieldmappingwidget.sip.in b/python/gui/auto_generated/qgsfieldmappingwidget.sip.in
@@ -79,6 +79,22 @@ Returns the selection model
     void setSourceFields( const QgsFields &sourceFields );
 %Docstring
 Set source fields of the underlying mapping model to ``sourceFields``
+%End
+
+    void setSourceLayer( QgsVectorLayer *layer );
+%Docstring
+Sets a source ``layer`` to use when generating expression previews in the widget.
+
+.. versionadded:: 3.16
+%End
+
+    QgsVectorLayer *sourceLayer();
+%Docstring
+Returns the source layer for use when generating expression previews.
+
+Returned value may be ``None``.
+
+.. versionadded:: 3.16
 %End
 
     void setDestinationFields( const QgsFields &destinationFields,

diff --git a/src/core/expression/qgsexpressionnodeimpl.cpp b/src/core/expression/qgsexpressionnodeimpl.cpp
@@ -1307,8 +1307,13 @@ QVariant QgsExpressionNodeColumnRef::evalNode( QgsExpression *parent, const QgsE
       else
         return feature.attribute( mName );
     }
+    else
+    {
+      parent->setEvalErrorString( tr( "No feature available for field '%1' evaluation" ).arg( mName ) );
+    }
   }
-  parent->setEvalErrorString( tr( "Field '%1' not found" ).arg( mName ) );
+  if ( index < 0 )
+    parent->setEvalErrorString( tr( "Field '%1' not found" ).arg( mName ) );
   return QVariant();
 }
 

diff --git a/src/gui/processing/qgsprocessingaggregatewidgets.cpp b/src/gui/processing/qgsprocessingaggregatewidgets.cpp
@@ -257,6 +257,9 @@ bool QgsAggregateMappingModel::moveUpOrDown( const QModelIndex &index, bool up )
 void QgsAggregateMappingModel::setSourceFields( const QgsFields &sourceFields )
 {
   mSourceFields = sourceFields;
+  if ( mExpressionContextGenerator )
+    mExpressionContextGenerator->setSourceFields( mSourceFields );
+
   QStringList usedFields;
   beginResetModel();
   mMapping.clear();
@@ -356,7 +359,7 @@ QgsAggregateMappingWidget::QgsAggregateMappingWidget( QWidget *parent,
 
   mModel = new QgsAggregateMappingModel( sourceFields, this );
   mTableView->setModel( mModel );
-  mTableView->setItemDelegateForColumn( static_cast<int>( QgsAggregateMappingModel::ColumnDataIndex::SourceExpression ), new QgsFieldMappingWidget::ExpressionDelegate( mTableView ) );
+  mTableView->setItemDelegateForColumn( static_cast<int>( QgsAggregateMappingModel::ColumnDataIndex::SourceExpression ), new QgsFieldMappingWidget::ExpressionDelegate( this ) );
   mTableView->setItemDelegateForColumn( static_cast<int>( QgsAggregateMappingModel::ColumnDataIndex::Aggregate ), new QgsAggregateMappingWidget::AggregateDelegate( mTableView ) );
   mTableView->setItemDelegateForColumn( static_cast<int>( QgsAggregateMappingModel::ColumnDataIndex::DestinationType ), new QgsFieldMappingWidget::TypeDelegate( mTableView ) );
   updateColumns();
@@ -394,6 +397,16 @@ void QgsAggregateMappingWidget::setSourceFields( const QgsFields &sourceFields )
   model()->setSourceFields( sourceFields );
 }
 
+void QgsAggregateMappingWidget::setSourceLayer( QgsVectorLayer *layer )
+{
+  mSourceLayer = layer;
+}
+
+QgsVectorLayer *QgsAggregateMappingWidget::sourceLayer()
+{
+  return mSourceLayer;
+}
+
 void QgsAggregateMappingWidget::scrollTo( const QModelIndex &index ) const
 {
   mTableView->scrollTo( index );

diff --git a/src/gui/processing/qgsprocessingaggregatewidgets.h b/src/gui/processing/qgsprocessingaggregatewidgets.h
@@ -18,6 +18,7 @@
 
 #include <QAbstractTableModel>
 #include <QStyledItemDelegate>
+#include <QPointer>
 
 #include "qgsfields.h"
 #include "qgsexpressioncontextgenerator.h"
@@ -175,6 +176,22 @@ class GUI_EXPORT QgsAggregateMappingWidget : public QgsPanelWidget
     //! Set source fields of the underlying mapping model to \a sourceFields
     void setSourceFields( const QgsFields &sourceFields );
 
+    /**
+     * Sets a source \a layer to use when generating expression previews in the widget.
+     *
+     * \since QGIS 3.16
+     */
+    void setSourceLayer( QgsVectorLayer *layer );
+
+    /**
+     * Returns the source layer for use when generating expression previews.
+     *
+     * Returned value may be NULLPTR.
+     *
+     * \since QGIS 3.16
+     */
+    QgsVectorLayer *sourceLayer();
+
     /**
      * Scroll the fields view to \a index
      */
@@ -211,6 +228,7 @@ class GUI_EXPORT QgsAggregateMappingWidget : public QgsPanelWidget
 
     QTableView *mTableView = nullptr;
     QAbstractTableModel *mModel = nullptr;
+    QPointer< QgsVectorLayer > mSourceLayer;
     void updateColumns();
     //! Returns selected row indexes in ascending order
     std::list<int> selectedRows( );

diff --git a/src/gui/processing/qgsprocessingaggregatewidgetwrapper.cpp b/src/gui/processing/qgsprocessingaggregatewidgetwrapper.cpp
@@ -71,6 +71,7 @@ void QgsProcessingAggregatePanelWidget::setLayer( QgsVectorLayer *layer )
     return;
 
   mLayer = layer;
+  mFieldsView->setSourceLayer( mLayer );
   if ( mModel->rowCount() == 0 )
   {
     loadFieldsFromLayer();

diff --git a/src/gui/processing/qgsprocessingfieldmapwidgetwrapper.cpp b/src/gui/processing/qgsprocessingfieldmapwidgetwrapper.cpp
@@ -69,6 +69,7 @@ void QgsProcessingFieldMapPanelWidget::setLayer( QgsVectorLayer *layer )
     return;
 
   mLayer = layer;
+  mFieldsView->setSourceLayer( mLayer );
   if ( mModel->rowCount() == 0 )
   {
     loadFieldsFromLayer();

diff --git a/src/gui/qgsfieldmappingmodel.cpp b/src/gui/qgsfieldmappingmodel.cpp
@@ -310,6 +310,8 @@ QString QgsFieldMappingModel::findExpressionForDestinationField( const QgsFieldM
 void QgsFieldMappingModel::setSourceFields( const QgsFields &sourceFields )
 {
   mSourceFields = sourceFields;
+  if ( mExpressionContextGenerator )
+    mExpressionContextGenerator->setSourceFields( mSourceFields );
   QStringList usedFields;
   beginResetModel();
   for ( const Field &f : qgis::as_const( mMapping ) )
@@ -525,3 +527,8 @@ void QgsFieldMappingModel::ExpressionContextGenerator::setBaseExpressionContextG
 {
   mBaseGenerator = generator;
 }
+
+void QgsFieldMappingModel::ExpressionContextGenerator::setSourceFields( const QgsFields &fields )
+{
+  mSourceFields = fields;
+}
diff --git a/src/gui/qgsfieldmappingmodel.h b/src/gui/qgsfieldmappingmodel.h
@@ -165,12 +165,13 @@ class GUI_EXPORT QgsFieldMappingModel: public QAbstractTableModel
         // QgsExpressionContextGenerator interface
         QgsExpressionContext createExpressionContext() const override;
         void setBaseExpressionContextGenerator( const QgsExpressionContextGenerator *generator );
+        void setSourceFields( const QgsFields &fields );
 
       private:
 
         const QgsExpressionContextGenerator *mBaseGenerator = nullptr;
 
-        const QgsFields mSourceFields;
+        QgsFields mSourceFields;
 
     };
 

diff --git a/src/gui/qgsfieldmappingwidget.cpp b/src/gui/qgsfieldmappingwidget.cpp
@@ -18,6 +18,7 @@
 #include "qgsfieldexpressionwidget.h"
 #include "qgsexpression.h"
 #include "qgsprocessingaggregatewidgets.h"
+#include "qgsvectorlayer.h"
 
 #include <QTableView>
 #include <QVBoxLayout>
@@ -45,7 +46,7 @@ QgsFieldMappingWidget::QgsFieldMappingWidget( QWidget *parent,
 #endif
 
   mTableView->setModel( mModel );
-  mTableView->setItemDelegateForColumn( static_cast<int>( QgsFieldMappingModel::ColumnDataIndex::SourceExpression ), new ExpressionDelegate( mTableView ) );
+  mTableView->setItemDelegateForColumn( static_cast<int>( QgsFieldMappingModel::ColumnDataIndex::SourceExpression ), new ExpressionDelegate( this ) );
   mTableView->setItemDelegateForColumn( static_cast<int>( QgsFieldMappingModel::ColumnDataIndex::DestinationType ), new TypeDelegate( mTableView ) );
   updateColumns();
   // Make sure columns are updated when rows are added
@@ -98,6 +99,16 @@ void QgsFieldMappingWidget::setSourceFields( const QgsFields &sourceFields )
   model()->setSourceFields( sourceFields );
 }
 
+void QgsFieldMappingWidget::setSourceLayer( QgsVectorLayer *layer )
+{
+  mSourceLayer = layer;
+}
+
+QgsVectorLayer *QgsFieldMappingWidget::sourceLayer()
+{
+  return mSourceLayer;
+}
+
 void QgsFieldMappingWidget::setDestinationFields( const QgsFields &destinationFields, const QMap<QString, QString> &expressions )
 {
   model()->setDestinationFields( destinationFields, expressions );
@@ -257,6 +268,18 @@ QWidget *QgsFieldMappingWidget::ExpressionDelegate::createEditor( QWidget *paren
   {
     Q_ASSERT( false );
   }
+
+  if ( QgsFieldMappingWidget *mappingWidget = qobject_cast< QgsFieldMappingWidget *>( ExpressionDelegate::parent() ) )
+  {
+    if ( mappingWidget->sourceLayer() )
+      editor->setLayer( mappingWidget->sourceLayer() );
+  }
+  else if ( QgsAggregateMappingWidget *aggregateWidget = qobject_cast< QgsAggregateMappingWidget *>( ExpressionDelegate::parent() ) )
+  {
+    if ( aggregateWidget->sourceLayer() )
+      editor->setLayer( aggregateWidget->sourceLayer() );
+  }
+
   editor->setField( index.model()->data( index, Qt::DisplayRole ).toString() );
   connect( editor,
            qgis::overload<const  QString &, bool >::of( &QgsFieldExpressionWidget::fieldChanged ),

diff --git a/src/gui/qgsfieldmappingwidget.h b/src/gui/qgsfieldmappingwidget.h
@@ -19,13 +19,15 @@
 #include <QWidget>
 #include <QAbstractTableModel>
 #include <QStyledItemDelegate>
+#include <QPointer>
 
 #include "qgis_gui.h"
 #include "qgsfieldmappingmodel.h"
 #include "qgspanelwidget.h"
 
 class QTableView;
 class QItemSelectionModel;
+class QgsVectorLayer;
 
 /**
  * \ingroup gui
@@ -85,6 +87,22 @@ class GUI_EXPORT QgsFieldMappingWidget : public QgsPanelWidget
     //! Set source fields of the underlying mapping model to \a sourceFields
     void setSourceFields( const QgsFields &sourceFields );
 
+    /**
+     * Sets a source \a layer to use when generating expression previews in the widget.
+     *
+     * \since QGIS 3.16
+     */
+    void setSourceLayer( QgsVectorLayer *layer );
+
+    /**
+     * Returns the source layer for use when generating expression previews.
+     *
+     * Returned value may be NULLPTR.
+     *
+     * \since QGIS 3.16
+     */
+    QgsVectorLayer *sourceLayer();
+
     /**
      * Set destination fields to \a destinationFields in the underlying model,
      * initial values for the expressions can be optionally specified through
@@ -130,6 +148,8 @@ class GUI_EXPORT QgsFieldMappingWidget : public QgsPanelWidget
 
     QTableView *mTableView = nullptr;
     QAbstractTableModel *mModel = nullptr;
+
+    QPointer< QgsVectorLayer > mSourceLayer;
     void updateColumns();
     //! Returns selected row indexes in ascending order
     std::list<int> selectedRows( );

diff --git a/tests/src/core/testqgsexpression.cpp b/tests/src/core/testqgsexpression.cpp
@@ -549,6 +549,46 @@ class TestQgsExpression: public QObject
       QVERIFY( expression4.hasEvalError() );
     }
 
+    void fieldsButNoFeature()
+    {
+      // test evaluating an expression with fields in the context but no feature
+      QgsExpressionContext context;
+      QgsExpressionContextScope *scope = new QgsExpressionContextScope();
+
+      QgsFields fields;
+      fields.append( QgsField( QStringLiteral( "x" ) ) );
+      fields.append( QgsField( QStringLiteral( "y" ) ) );
+      fields.append( QgsField( QStringLiteral( "z" ) ) );
+      scope->setFields( fields );
+      context.appendScope( scope );
+
+      // doesn't exist
+      QgsExpression expression( "\"a\"" );
+      QVERIFY( !expression.hasParserError() );
+      QVERIFY( !expression.evaluate( &context ).isValid() );
+      QVERIFY( expression.hasEvalError() );
+      QCOMPARE( expression.evalErrorString(), QStringLiteral( "Field 'a' not found" ) );
+      expression = QgsExpression( "\"x\"" );
+      QVERIFY( !expression.hasParserError() );
+      QVERIFY( !expression.evaluate( &context ).isValid() );
+      QVERIFY( expression.hasEvalError() );
+      QCOMPARE( expression.evalErrorString(), QStringLiteral( "No feature available for field 'x' evaluation" ) );
+      expression = QgsExpression( "\"y\"" );
+      QVERIFY( !expression.hasParserError() );
+      QVERIFY( !expression.evaluate( &context ).isValid() );
+      QVERIFY( expression.hasEvalError() );
+      QCOMPARE( expression.evalErrorString(), QStringLiteral( "No feature available for field 'y' evaluation" ) );
+
+      QgsFeature f( fields );
+      f.setValid( true );
+      f.setAttributes( QgsAttributes() << 1 << 2 << 3 );
+      scope->setFeature( f );
+      expression = QgsExpression( "\"z\"" );
+      QVERIFY( !expression.hasParserError() );
+      QCOMPARE( expression.evaluate( &context ).toInt(), 3 );
+      QVERIFY( !expression.hasEvalError() );
+    }
+
     void evaluation_data()
     {
       QTest::addColumn<QString>( "string" );