Fix chain filter when null values are not allowed

qgis · Apr 15, 2020 · afdb622 · afdb622
1 parent d2d8c84
commit afdb622
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 8 deletions.
diff --git a/src/core/qgsfeaturefiltermodel.cpp b/src/core/qgsfeaturefiltermodel.cpp
@@ -288,7 +288,7 @@ void QgsFeatureFilterModel::updateCompleter()
     int firstRow = 0;
 
     // Move the extra entry to the first position
-    if ( mExtraIdentifierValueIndex != -1 )
+    if ( mExtraIdentifierValueIndex != -1 && currentEntryInNewList != -1 )
     {
       if ( mExtraIdentifierValueIndex != 0 )
       {
@@ -302,14 +302,20 @@ void QgsFeatureFilterModel::updateCompleter()
     // Remove all entries (except for extra entry if existent)
     beginRemoveRows( QModelIndex(), firstRow, mEntries.size() - firstRow );
     mEntries.remove( firstRow, mEntries.size() - firstRow );
+
+    // we need to reset mExtraIdentifierValueIndex variable if we remove all rows
+    // before endRemoveRows, if not setExtraIdentifierValuesUnguarded will be called
+    // and a null value will be added to mEntries
+    mExtraIdentifierValueIndex = firstRow > 0 ? mExtraIdentifierValueIndex : 0;
+
     endRemoveRows();
 
     if ( currentEntryInNewList == -1 )
     {
       beginInsertRows( QModelIndex(), 1, entries.size() + 1 );
       mEntries += entries;
       endInsertRows();
-      setExtraIdentifierValuesIndex( 0 );
+      setExtraIdentifierValuesIndex( mAllowNull && !mEntries.isEmpty() ? 1 : 0 );
     }
     else
     {

diff --git a/tests/src/gui/testqgsrelationreferencewidget.cpp b/tests/src/gui/testqgsrelationreferencewidget.cpp
@@ -46,6 +46,7 @@ class TestQgsRelationReferenceWidget : public QObject
     void cleanup(); // will be called after every testfunction.
 
     void testChainFilter();
+    void testChainFilter_data();
     void testChainFilterRefreshed();
     void testChainFilterDeleteForeignKey();
     void testInvalidRelation();
@@ -146,16 +147,26 @@ void TestQgsRelationReferenceWidget::cleanup()
   QgsProject::instance()->removeMapLayer( mLayer2.get() );
 }
 
+void TestQgsRelationReferenceWidget::testChainFilter_data()
+{
+  QTest::addColumn<bool>( "allowNull" );
+
+  QTest::newRow( "allowNull=true" ) << true;
+  QTest::newRow( "allowNull=false" ) << false;
+}
+
 void TestQgsRelationReferenceWidget::testChainFilter()
 {
+  QFETCH( bool, allowNull );
+
   // init a relation reference widget
   QStringList filterFields = { "material", "diameter", "raccord" };
 
   QWidget parentWidget;
   QgsRelationReferenceWidget w( &parentWidget );
   w.setChainFilters( true );
   w.setFilterFields( filterFields );
-  w.setRelation( *mRelation, true );
+  w.setRelation( *mRelation, allowNull );
   w.init();
 
   // check default status for comboboxes
@@ -202,7 +213,7 @@ void TestQgsRelationReferenceWidget::testChainFilter()
 
   cbs[0]->setCurrentIndex( 0 );
   loop.exec();
-  QCOMPARE( w.mComboBox->currentText(), QString( "NULL" ) );
+  QCOMPARE( w.mComboBox->currentText(), allowNull ? QString( "NULL" ) : QString( "10" ) );
 
   cbs[0]->setCurrentIndex( cbs[0]->findText( "iron" ) );
   loop.exec();
@@ -228,15 +239,15 @@ void TestQgsRelationReferenceWidget::testChainFilter()
   loop.exec();
   QCOMPARE( w.mComboBox->currentText(), QString( "10" ) );
 
-  // combobox should propose NULL, 10 and 11 because the filter is now:
+  // combobox should propose NULL (if allowNull is true), 10 and 11 because the filter is now:
   // "material" == 'iron'
-  QCOMPARE( w.mComboBox->count(), 3 );
+  QCOMPARE( w.mComboBox->count(), allowNull ? 3 : 2 );
 
   // if there's no filter at all, all features' id should be proposed
   cbs[0]->setCurrentIndex( cbs[0]->findText( QStringLiteral( "material" ) ) );
   loop.exec();
-  QCOMPARE( w.mComboBox->count(), 4 );
-  QCOMPARE( w.mComboBox->currentText(), QString( "NULL" ) );
+  QCOMPARE( w.mComboBox->count(), allowNull ? 4 : 3 );
+  QCOMPARE( w.mComboBox->currentText(), allowNull ? QString( "NULL" ) : QString( "10" ) );
 }
 
 void TestQgsRelationReferenceWidget::testChainFilterRefreshed()