Merge pull request #8079 from elpaso/bugfix-19981-value-relation-0

Fixes value relation widget 0 indexed related items
qgis · Oct 1, 2018 · 27e4b19 · 27e4b19
2 parents 3f24857 + d8fcb65
commit 27e4b19
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 1 deletion.
diff --git a/src/gui/editorwidgets/qgsvaluerelationwidgetwrapper.cpp b/src/gui/editorwidgets/qgsvaluerelationwidgetwrapper.cpp
@@ -179,7 +179,19 @@ void QgsValueRelationWidgetWrapper::setValue( const QVariant &value )
   }
   else if ( mComboBox )
   {
-    mComboBox->setCurrentIndex( mComboBox->findData( value ) );
+    // findData fails to tell a 0 from a NULL
+    // See: "Value relation, value 0 = NULL" - https://issues.qgis.org/issues/19981
+    int idx = -1; // default to not found
+    for ( int i = 0; i < mComboBox->count(); i++ )
+    {
+      QVariant v( mComboBox->itemData( i ) );
+      if ( v.isNull() == value.isNull() && v == value )
+      {
+        idx = i;
+        break;
+      }
+    }
+    mComboBox->setCurrentIndex( idx );
   }
   else if ( mLineEdit )
   {

diff --git a/tests/src/gui/testqgsvaluerelationwidgetwrapper.cpp b/tests/src/gui/testqgsvaluerelationwidgetwrapper.cpp
@@ -42,6 +42,8 @@ class TestQgsValueRelationWidgetWrapper : public QObject
     void testScrollBarUnlocked();
     void testDrillDown();
     void testDrillDownMulti();
+    //! Checks that a related value of 0 is not interpreted as a NULL
+    void testZeroIndexInRelatedTable();
 };
 
 void TestQgsValueRelationWidgetWrapper::initTestCase()
@@ -200,6 +202,7 @@ void TestQgsValueRelationWidgetWrapper::testDrillDown()
   // Check null is selected
   QCOMPARE( w_municipality.mComboBox->count(), 3 );
   QCOMPARE( w_municipality.mComboBox->itemText( 0 ), QStringLiteral( "(no selection)" ) );
+  QVERIFY( w_municipality.value().isNull() );
   QCOMPARE( w_municipality.value().toString(), QString() );
 
   // Check order by value false
@@ -311,5 +314,59 @@ void TestQgsValueRelationWidgetWrapper::testDrillDownMulti()
 
 }
 
+void TestQgsValueRelationWidgetWrapper::testZeroIndexInRelatedTable()
+{
+  // findData fails to tell a 0 from a NULL
+  // See: "Value relation, value 0 = NULL" - https://issues.qgis.org/issues/19981
+
+  // create a vector layer
+  QgsVectorLayer vl1( QStringLiteral( "Polygon?crs=epsg:4326&field=pk:int&field=province:int&field=municipality:string" ), QStringLiteral( "vl1" ), QStringLiteral( "memory" ) );
+  QgsVectorLayer vl2( QStringLiteral( "Point?crs=epsg:4326&field=pk:int&field=fk_province:int&field=fk_municipality:int" ), QStringLiteral( "vl2" ), QStringLiteral( "memory" ) );
+  QgsProject::instance()->addMapLayer( &vl1, false, false );
+  QgsProject::instance()->addMapLayer( &vl2, false, false );
+
+  // insert some features
+  QgsFeature f1( vl1.fields() );
+  f1.setAttribute( QStringLiteral( "pk" ), 0 );  // !!! Notice: pk 0
+  f1.setAttribute( QStringLiteral( "province" ), 123 );
+  f1.setAttribute( QStringLiteral( "municipality" ), QStringLiteral( "Some Place By The River" ) );
+  f1.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "POLYGON(( 0 0, 0 1, 1 1, 1 0, 0 0 ))" ) ) );
+  QVERIFY( f1.isValid() );
+  QgsFeature f2( vl1.fields() );
+  f2.setAttribute( QStringLiteral( "pk" ), 2 );
+  f2.setAttribute( QStringLiteral( "province" ), 245 );
+  f2.setAttribute( QStringLiteral( "municipality" ), QStringLiteral( "Dreamland By The Clouds" ) );
+  f2.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "POLYGON(( 1 0, 1 1, 2 1, 2 0, 1 0 ))" ) ) );
+  QVERIFY( f2.isValid() );
+  QVERIFY( vl1.dataProvider()->addFeatures( QgsFeatureList() << f1 << f2 ) );
+
+  QgsFeature f3( vl2.fields() );
+  f3.setAttribute( QStringLiteral( "fk_province" ), 123 );
+  f3.setAttribute( QStringLiteral( "fk_municipality" ), QStringLiteral( "0" ) );
+  f3.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "POINT( 0.5 0.5)" ) ) );
+  QVERIFY( f3.isValid() );
+  QVERIFY( f3.geometry().isGeosValid() );
+  QVERIFY( vl2.dataProvider()->addFeature( f3 ) );
+
+  // build a value relation widget wrapper for municipality
+  QgsValueRelationWidgetWrapper w_municipality( &vl2, vl2.fields().indexOf( QStringLiteral( "fk_municipality" ) ), nullptr, nullptr );
+  QVariantMap cfg_municipality;
+  cfg_municipality.insert( QStringLiteral( "Layer" ), vl1.id() );
+  cfg_municipality.insert( QStringLiteral( "Key" ),  QStringLiteral( "pk" ) );
+  cfg_municipality.insert( QStringLiteral( "Value" ), QStringLiteral( "municipality" ) );
+  cfg_municipality.insert( QStringLiteral( "AllowMulti" ), false );
+  cfg_municipality.insert( QStringLiteral( "NofColumns" ), 1 );
+  cfg_municipality.insert( QStringLiteral( "AllowNull" ), true );
+  cfg_municipality.insert( QStringLiteral( "OrderByValue" ), false );
+  cfg_municipality.insert( QStringLiteral( "UseCompleter" ), false );
+  w_municipality.setConfig( cfg_municipality );
+  w_municipality.widget();
+  w_municipality.setEnabled( true );
+
+  w_municipality.setValue( 0 );
+  QCOMPARE( w_municipality.mComboBox->currentIndex(), 1 );
+  QCOMPARE( w_municipality.mComboBox->currentText(), QStringLiteral( "Some Place By The River" ) );
+}
+
 QGSTEST_MAIN( TestQgsValueRelationWidgetWrapper )
 #include "testqgsvaluerelationwidgetwrapper.moc"