From 0df78742209ae2e3b0263923c65c50cd30d4606b Mon Sep 17 00:00:00 2001
From: Nyall Dawson <nyall.dawson@gmail.com>
Date: Mon, 4 Apr 2016 14:31:41 +1000
Subject: [PATCH] Fix QgsFieldExpressionWidget reporting field names with
 spaces as invalid, add tests

(cherry-picked from af8c1667dfd6f945414b4080c95ee731125f44cc)
---
 src/gui/qgsfieldexpressionwidget.cpp          |  6 +-
 .../src/gui/testqgsfieldexpressionwidget.cpp  | 57 +++++++++++++++++++
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/src/gui/qgsfieldexpressionwidget.cpp b/src/gui/qgsfieldexpressionwidget.cpp
index 4b35ec7815d0..c0af00848005 100644
--- a/src/gui/qgsfieldexpressionwidget.cpp
+++ b/src/gui/qgsfieldexpressionwidget.cpp
@@ -122,13 +122,15 @@ bool QgsFieldExpressionWidget::isExpression() const
 QString QgsFieldExpressionWidget::currentField( bool *isExpression, bool *isValid ) const
 {
   QString text = currentText();
+  bool valueIsExpression = this->isExpression();
   if ( isValid )
   {
-    *isValid = isValidExpression();
+    // valid if not an expression (ie, set to a field), or set to an expression and expression is valid
+    *isValid = !valueIsExpression || isValidExpression();
   }
   if ( isExpression )
   {
-    *isExpression = this->isExpression();
+    *isExpression = valueIsExpression;
   }
   return text;
 }
diff --git a/tests/src/gui/testqgsfieldexpressionwidget.cpp b/tests/src/gui/testqgsfieldexpressionwidget.cpp
index bdf78f53de26..ff8d5f00af25 100644
--- a/tests/src/gui/testqgsfieldexpressionwidget.cpp
+++ b/tests/src/gui/testqgsfieldexpressionwidget.cpp
@@ -49,6 +49,7 @@ class TestQgsFieldExpressionWidget : public QObject
 
     void testRemoveJoin();
     void asExpression();
+    void testIsValid();
 
   private:
     QgsFieldExpressionWidget* mWidget;
@@ -161,6 +162,62 @@ void TestQgsFieldExpressionWidget::asExpression()
   QgsMapLayerRegistry::instance()->removeMapLayer( layer );
 }
 
+void TestQgsFieldExpressionWidget::testIsValid()
+{
+  QgsVectorLayer* layer = new QgsVectorLayer( "point?field=fld:int&field=name%20with%20space:string", "x", "memory" );
+  QgsMapLayerRegistry::instance()->addMapLayer( layer );
+
+  QScopedPointer< QgsFieldExpressionWidget > widget( new QgsFieldExpressionWidget() );
+  widget->setLayer( layer );
+
+  // also check the fieldChanged signal to ensure that the emitted bool isValid value is correct
+  QSignalSpy spy( widget.data(), SIGNAL( fieldChanged( QString, bool ) ) );
+
+  // check with simple field name set
+  bool isExpression = false;
+  bool isValid = false;
+  widget->setField( "fld" );
+  QCOMPARE( widget->currentField( &isExpression, &isValid ), QString( "fld" ) );
+  QVERIFY( !isExpression );
+  QVERIFY( isValid );
+  QVERIFY( widget->isValidExpression() );
+  QCOMPARE( spy.count(), 1 );
+  QCOMPARE( spy.last().at( 0 ).toString(), QString( "fld" ) );
+  QVERIFY( spy.last().at( 1 ).toBool() );
+
+
+  //check with complex field name set
+  widget->setField( "name with space" );
+  QCOMPARE( widget->currentField( &isExpression, &isValid ), QString( "name with space" ) );
+  QVERIFY( !isExpression );
+  QVERIFY( isValid );
+  QVERIFY( !widget->isValidExpression() );
+  QCOMPARE( spy.count(), 2 );
+  QCOMPARE( spy.last().at( 0 ).toString(), QString( "name with space" ) );
+  QVERIFY( spy.last().at( 1 ).toBool() );
+
+  //check with valid expression set
+  widget->setField( "2 * 4" );
+  QCOMPARE( widget->currentField( &isExpression, &isValid ), QString( "2 * 4" ) );
+  QVERIFY( isExpression );
+  QVERIFY( isValid );
+  QVERIFY( widget->isValidExpression() );
+  QCOMPARE( spy.count(), 3 );
+  QCOMPARE( spy.last().at( 0 ).toString(), QString( "2 * 4" ) );
+  QVERIFY( spy.last().at( 1 ).toBool() );
+
+  //check with invalid expression set
+  widget->setField( "2 *" );
+  QCOMPARE( widget->currentField( &isExpression, &isValid ), QString( "2 *" ) );
+  QVERIFY( isExpression );
+  QVERIFY( !isValid );
+  QVERIFY( !widget->isValidExpression() );
+  QCOMPARE( spy.count(), 4 );
+  QCOMPARE( spy.last().at( 0 ).toString(), QString( "2 *" ) );
+  QVERIFY( !spy.last().at( 1 ).toBool() );
+
+  QgsMapLayerRegistry::instance()->removeMapLayer( layer );
+}
 
 QTEST_MAIN( TestQgsFieldExpressionWidget )
 #include "testqgsfieldexpressionwidget.moc"