fix edge case where fkey is 0 in relation reference widget

the list of identifiers was initialized with a list QVariant(Int) for Null, but they are equal to 0 now, we also control that the value is valid to determine if the identifier is not null
qgis · Sep 23, 2020 · 756b293 · 756b293
1 parent 194a517
commit 756b293
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 10 deletions.
diff --git a/src/core/qgsfeaturefiltermodel.cpp b/src/core/qgsfeaturefiltermodel.cpp
@@ -96,7 +96,7 @@ bool QgsFeatureFilterModel::identifierIsNull( const QVariant &identifier ) const
   const QVariantList values = identifier.toList();
   for ( const QVariant &value : values )
   {
-    if ( !value.isNull() )
+    if ( !value.isNull() && value.isValid() )
     {
       return false;
     }
@@ -151,6 +151,6 @@ void QgsFeatureFilterModel::setExtraIdentifierValues( const QVariantList &extraI
 
 void QgsFeatureFilterModel::setExtraIdentifierValueToNull()
 {
-  setExtraIdentifierValue( QVariantList() );
+  setExtraIdentifierValue( nullIdentifier() );
 }
 
diff --git a/src/core/qgsfeaturepickermodelbase.cpp b/src/core/qgsfeaturepickermodelbase.cpp
@@ -612,7 +612,7 @@ void QgsFeaturePickerModelBase::reload()
 
 void QgsFeaturePickerModelBase::setExtraIdentifierValue( const QVariant &extraIdentifierValue )
 {
-  if ( extraIdentifierValue == mExtraIdentifierValue && !identifierIsNull( extraIdentifierValue ) )
+  if ( extraIdentifierValue == mExtraIdentifierValue && !identifierIsNull( extraIdentifierValue ) && !identifierIsNull( mExtraIdentifierValue ) )
     return;
 
   if ( mIsSettingExtraIdentifierValue )

diff --git a/src/gui/editorwidgets/qgsrelationreferencewidget.cpp b/src/gui/editorwidgets/qgsrelationreferencewidget.cpp
@@ -54,7 +54,7 @@ bool qVariantListIsNull( const QVariantList &list )
 
   for ( int i = 0; i < list.size(); ++i )
   {
-    if ( !list.at( i ).isNull() )
+    if ( !list.at( i ).isNull() && list.at( i ).isValid() )
       return false;
   }
   return true;
@@ -599,7 +599,7 @@ void QgsRelationReferenceWidget::init()
     }
 
     // Only connect after iterating, to have only one iterator on the referenced table at once
-    connect( mComboBox, static_cast<void ( QComboBox::* )( int )>( &QComboBox::currentIndexChanged ), this, &QgsRelationReferenceWidget::comboReferenceChanged );
+    connect( mComboBox, qgis::overload<int>::of( &QComboBox::currentIndexChanged ), this, &QgsRelationReferenceWidget::comboReferenceChanged );
 
     QApplication::restoreOverrideCursor();
 
@@ -1074,7 +1074,7 @@ void QgsRelationReferenceWidget::disableChainedComboBoxes( const QComboBox *scb
 
 void QgsRelationReferenceWidget::emitForeignKeysChanged( const QVariantList &foreignKeys, bool force )
 {
-  if ( foreignKeys == mForeignKeys && force == false )
+  if ( foreignKeys == mForeignKeys && force == false && qVariantListIsNull( foreignKeys ) == qVariantListIsNull( mForeignKeys ) )
     return;
 
   mForeignKeys = foreignKeys;

diff --git a/tests/src/gui/testqgsrelationreferencewidget.cpp b/tests/src/gui/testqgsrelationreferencewidget.cpp
@@ -518,22 +518,27 @@ void TestQgsRelationReferenceWidget::testSetGetForeignKey()
   w.setRelation( *mRelation, true );
   w.init();
 
-  QSignalSpy spy( &w, SIGNAL( foreignKeyChanged( QVariant ) ) );
+  QSignalSpy spy( &w, &QgsRelationReferenceWidget::foreignKeysChanged );
+
+  w.setForeignKeys( QVariantList() << 0 );
+  QCOMPARE( w.foreignKeys().at( 0 ), QVariant( 0 ) );
+  QCOMPARE( w.mComboBox->currentText(), QStringLiteral( "(0)" ) );
+  QCOMPARE( spy.count(), 1 );
 
   w.setForeignKeys( QVariantList() << 11 );
   QCOMPARE( w.foreignKeys().at( 0 ), QVariant( 11 ) );
   QCOMPARE( w.mComboBox->currentText(), QStringLiteral( "(11)" ) );
-  QCOMPARE( spy.count(), 1 );
+  QCOMPARE( spy.count(), 2 );
 
   w.setForeignKeys( QVariantList() << 12 );
   QCOMPARE( w.foreignKeys().at( 0 ), QVariant( 12 ) );
   QCOMPARE( w.mComboBox->currentText(), QStringLiteral( "(12)" ) );
-  QCOMPARE( spy.count(), 2 );
+  QCOMPARE( spy.count(), 3 );
 
   w.setForeignKeys( QVariantList() << QVariant() );
   QVERIFY( w.foreignKeys().at( 0 ).isNull() );
   QVERIFY( w.foreignKeys().at( 0 ).isValid() );
-  QCOMPARE( spy.count(), 3 );
+  QCOMPARE( spy.count(), 4 );
 }
 
 // Test issue https://github.com/qgis/QGIS/issues/29884