From 73e036ebdb28df98ba615ec221dfcc189fa2ead5 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 19 Dec 2021 11:31:50 -0500 Subject: [PATCH] Correctly handle sort field as field in atlas sorting Fixes #40332 --- .../expression/qgsexpression.sip.in | 15 +++++++++++ src/core/expression/qgsexpression.cpp | 7 +++++ src/core/expression/qgsexpression.h | 14 ++++++++++ src/gui/layout/qgslayoutatlaswidget.cpp | 3 ++- tests/src/core/testqgsexpression.cpp | 26 +++++++++++++++++++ 5 files changed, 64 insertions(+), 1 deletion(-) diff --git a/python/core/auto_generated/expression/qgsexpression.sip.in b/python/core/auto_generated/expression/qgsexpression.sip.in index 38b85dd5f76..82e2f07f2f4 100644 --- a/python/core/auto_generated/expression/qgsexpression.sip.in +++ b/python/core/auto_generated/expression/qgsexpression.sip.in @@ -304,6 +304,21 @@ method will return the corresponding field index. .. seealso:: :py:func:`isField` .. versionadded:: 3.22 +%End + + static QString quoteFieldExpression( const QString &expression, const QgsVectorLayer *layer ); +%Docstring +Validate if the expression is a field in the ``layer`` and ensure it is quoted. + +Given a string which may either directly match a field name from a layer, OR may +be an expression which consists only of a single field reference for that layer, this +method will return the quoted field. + +:return: the ``expression`` if not a field or quotes are not required, otherwise requite a quoted field. + +.. seealso:: :py:func:`expressionToLayerFieldIndex` + +.. versionadded:: 3.24 %End static bool checkExpression( const QString &text, const QgsExpressionContext *context, QString &errorMessage /Out/ ); diff --git a/src/core/expression/qgsexpression.cpp b/src/core/expression/qgsexpression.cpp index 5bc0705ab88..6fc045d9bc0 100644 --- a/src/core/expression/qgsexpression.cpp +++ b/src/core/expression/qgsexpression.cpp @@ -1371,6 +1371,13 @@ int QgsExpression::expressionToLayerFieldIndex( const QString &expression, const return -1; } +QString QgsExpression::quoteFieldExpression( const QString &expression, const QgsVectorLayer *layer ) +{ + if ( !expression.contains( '\"' ) && QgsExpression::expressionToLayerFieldIndex( expression, layer ) != -1 ) + return QgsExpression::quotedColumnRef( expression ); + return expression; +} + QList QgsExpression::nodes() const { if ( !d->mRootNode ) diff --git a/src/core/expression/qgsexpression.h b/src/core/expression/qgsexpression.h index ed303a66083..9b189ffabf5 100644 --- a/src/core/expression/qgsexpression.h +++ b/src/core/expression/qgsexpression.h @@ -369,6 +369,20 @@ class CORE_EXPORT QgsExpression */ static int expressionToLayerFieldIndex( const QString &expression, const QgsVectorLayer *layer ); + /** + * Validate if the expression is a field in the \a layer and ensure it is quoted. + * + * Given a string which may either directly match a field name from a layer, OR may + * be an expression which consists only of a single field reference for that layer, this + * method will return the quoted field. + * + * \returns the \a expression if not a field or quotes are not required, otherwise requite a quoted field. + * + * \see expressionToLayerFieldIndex() + * \since QGIS 3.24 + */ + static QString quoteFieldExpression( const QString &expression, const QgsVectorLayer *layer ); + /** * Tests whether a string is a valid expression. * \param text string to test diff --git a/src/gui/layout/qgslayoutatlaswidget.cpp b/src/gui/layout/qgslayoutatlaswidget.cpp index a315efb8cd0..7981f0f8bfc 100644 --- a/src/gui/layout/qgslayoutatlaswidget.cpp +++ b/src/gui/layout/qgslayoutatlaswidget.cpp @@ -240,7 +240,8 @@ void QgsLayoutAtlasWidget::changesSortFeatureExpression( const QString &expressi mBlockUpdates = true; mLayout->undoStack()->beginCommand( mAtlas, tr( "Change Atlas Sort" ) ); - mAtlas->setSortExpression( expression ); + QgsVectorLayer *vlayer = qobject_cast( mAtlasCoverageLayerComboBox->currentLayer() ); + mAtlas->setSortExpression( QgsExpression::quoteFieldExpression( expression, vlayer ) ); mLayout->undoStack()->endCommand(); mBlockUpdates = false; updateAtlasFeatures(); diff --git a/tests/src/core/testqgsexpression.cpp b/tests/src/core/testqgsexpression.cpp index 88acbcb12ab..fb4d94b4bd0 100644 --- a/tests/src/core/testqgsexpression.cpp +++ b/tests/src/core/testqgsexpression.cpp @@ -4103,6 +4103,32 @@ class TestQgsExpression: public QObject QCOMPARE( QgsExpression::expressionToLayerFieldIndex( " ( \"ANOTHER FIELD\" ) ", layer.get() ), 1 ); } + void test_quoteFieldExpression() + { + std::unique_ptr layer = std::make_unique< QgsVectorLayer >( QStringLiteral( "Point" ), QStringLiteral( "test" ), QStringLiteral( "memory" ) ); + layer->dataProvider()->addAttributes( { QgsField( QStringLiteral( "field1" ), QVariant::String ), + QgsField( QStringLiteral( "another FIELD" ), QVariant::String ) } ); + layer->updateFields(); + + QCOMPARE( QgsExpression::quoteFieldExpression( "", layer.get() ), "" ); + QCOMPARE( QgsExpression::quoteFieldExpression( "42", layer.get() ), "42" ); + QCOMPARE( QgsExpression::quoteFieldExpression( "foo", layer.get() ), "foo" ); + QCOMPARE( QgsExpression::quoteFieldExpression( "\"foo bar\"", layer.get() ), "\"foo bar\"" ); + QCOMPARE( QgsExpression::quoteFieldExpression( "sqrt(foo)", layer.get() ), "sqrt(foo)" ); + QCOMPARE( QgsExpression::quoteFieldExpression( "foo + bar", layer.get() ), "foo + bar" ); + QCOMPARE( QgsExpression::quoteFieldExpression( "field1", layer.get() ), "\"field1\"" ); + QCOMPARE( QgsExpression::quoteFieldExpression( "FIELD1", layer.get() ), "\"FIELD1\"" ); + QCOMPARE( QgsExpression::quoteFieldExpression( "\"field1\"", layer.get() ), "\"field1\"" ); + QCOMPARE( QgsExpression::quoteFieldExpression( "\"FIELD1\"", layer.get() ), "\"FIELD1\"" ); + QCOMPARE( QgsExpression::quoteFieldExpression( " ( \"field1\" ) ", layer.get() ), " ( \"field1\" ) " ); + QCOMPARE( QgsExpression::quoteFieldExpression( "another FIELD", layer.get() ), "\"another FIELD\"" ); + QCOMPARE( QgsExpression::quoteFieldExpression( "ANOTHER field", layer.get() ), "\"ANOTHER field\"" ); + QCOMPARE( QgsExpression::quoteFieldExpression( " ANOTHER field ", layer.get() ), "\" ANOTHER field \"" ); + QCOMPARE( QgsExpression::quoteFieldExpression( "\"another field\"", layer.get() ), "\"another field\"" ); + QCOMPARE( QgsExpression::quoteFieldExpression( "\"ANOTHER FIELD\"", layer.get() ), "\"ANOTHER FIELD\"" ); + QCOMPARE( QgsExpression::quoteFieldExpression( " ( \"ANOTHER FIELD\" ) ", layer.get() ), " ( \"ANOTHER FIELD\" ) " ); + } + void test_implicitSharing() { QgsExpression *exp = new QgsExpression( QStringLiteral( "Pilots > 2" ) );