Skip to content
Permalink
Browse files

Merge pull request #37929 from nyalldawson/fix_37912_refactor_fields

Fix expression previews in Refactor Fields and Aggregate algorithms
  • Loading branch information
nyalldawson committed Jul 24, 2020
2 parents b6ae643 + 6dbc178 commit a345ae94a5a03f14763efa86f3d4ecc87098d39b
@@ -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;
@@ -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,
@@ -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();
}

@@ -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 );
@@ -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( );
@@ -71,6 +71,7 @@ void QgsProcessingAggregatePanelWidget::setLayer( QgsVectorLayer *layer )
return;

mLayer = layer;
mFieldsView->setSourceLayer( mLayer );
if ( mModel->rowCount() == 0 )
{
loadFieldsFromLayer();
@@ -69,6 +69,7 @@ void QgsProcessingFieldMapPanelWidget::setLayer( QgsVectorLayer *layer )
return;

mLayer = layer;
mFieldsView->setSourceLayer( mLayer );
if ( mModel->rowCount() == 0 )
{
loadFieldsFromLayer();
@@ -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;
}
@@ -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;

};

@@ -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 ),
@@ -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( );
@@ -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" );

0 comments on commit a345ae9

Please sign in to comment.
You can’t perform that action at this time.