Fix QgsFieldExpressionWidget reporting field names with spaces as

invalid, add tests

Author: nyalldawson

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;
 }

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"