Skip to content
Permalink
Browse files

fix feature list combo box having identifier value further than fetch…

…ing limit (#37280)

* test for feature list combo box having identifier value further than fetching limit

* fix feature list combo box having identifier value further than fetching limit

fixes #37266

* make the test a bit more robust

* fix compilation with older Qt

* correctly keep current value

* determine if keep current entry using the current index

* Revert "determine if keep current entry using the current index"

This reverts commit ff3658b.
  • Loading branch information
3nids committed Jun 19, 2020
1 parent 1dc0595 commit fe14dc9bb7018293c0acea053e9e7f3f834fec8a
@@ -251,6 +251,7 @@ void QgsFeaturePickerModelBase::updateCompleter()
setExtraValueDoesNotExist( true );
}

mKeepCurrentEntry = true;
mShouldReloadCurrentFeature = false;

if ( mFilterValue.isEmpty() )
@@ -269,7 +270,7 @@ void QgsFeaturePickerModelBase::updateCompleter()
const int newEntriesSize = entries.size();

// fixed entry is either NULL or extra value
const int nbFixedEntry = ( mExtraValueDoesNotExist ? 1 : 0 ) + ( mAllowNull ? 1 : 0 );
const int nbFixedEntry = ( mKeepCurrentEntry ? 1 : 0 ) + ( mAllowNull ? 1 : 0 );

// Find the index of the current entry in the new list
int currentEntryInNewList = -1;
@@ -313,7 +314,6 @@ void QgsFeaturePickerModelBase::updateCompleter()
endRemoveRows();
mIsSettingExtraIdentifierValue = false;


if ( currentEntryInNewList == -1 )
{
beginInsertRows( QModelIndex(), firstRow, entries.size() + 1 );
@@ -351,10 +351,11 @@ void QgsFeaturePickerModelBase::updateCompleter()
}

emit filterJobCompleted();

mKeepCurrentEntry = false;
}
emit endUpdate();


// scheduleReload and updateCompleter lives in the same thread so if the gatherer hasn't been stopped
// (checked before), mGatherer still references the current gatherer
Q_ASSERT( gatherer == mGatherer );
@@ -416,7 +417,6 @@ void QgsFeaturePickerModelBase::scheduledReload()
mGatherer->setData( mShouldReloadCurrentFeature );
connect( mGatherer, &QgsFeatureExpressionValuesGatherer::finished, this, &QgsFeaturePickerModelBase::updateCompleter );


mGatherer->start();
if ( !wasLoading )
emit isLoadingChanged();
@@ -339,9 +339,12 @@ class CORE_EXPORT QgsFeaturePickerModelBase : public QAbstractItemModel SIP_ABST

QTimer mReloadTimer;
bool mShouldReloadCurrentFeature = false;
bool mKeepCurrentEntry = false; // need to keep the current value after a reload or if the value does not exist
bool mExtraValueDoesNotExist = false;
bool mAllowNull = false;
bool mIsSettingExtraIdentifierValue = false;

friend class TestQgsFeatureListComboBox;
};

#endif // QGSFEATUREFILTERMODELBASE_H
@@ -31,6 +31,7 @@ QgsFeatureListComboBox::QgsFeatureListComboBox( QWidget *parent )
{
mCompleter->setCaseSensitivity( Qt::CaseInsensitive );
mCompleter->setFilterMode( Qt::MatchContains );
setEditable( true );
setCompleter( mCompleter );
mCompleter->setWidget( this );
connect( mModel, &QgsFeatureFilterModel::sourceLayerChanged, this, &QgsFeatureListComboBox::sourceLayerChanged );
@@ -55,7 +56,6 @@ QgsFeatureListComboBox::QgsFeatureListComboBox( QWidget *parent )
mLineEdit->setSelectOnFocus( true );
mLineEdit->setShowClearButton( true );

setEditable( true );
setLineEdit( mLineEdit );
setModel( mModel );

@@ -50,6 +50,7 @@ class TestQgsFeatureListComboBox : public QObject
void testValuesAndSelection_data();
void nullRepresentation();
void testNotExistingYetFeature();
void testFeatureFurtherThanFetchLimit();

private:

@@ -104,6 +105,18 @@ void TestQgsFeatureListComboBox::init()
ft3.setAttribute( QStringLiteral( "raccord" ), "collar" );
mLayer->addFeature( ft3 );

QgsFeatureList flist;
for ( int i = 13; i < 40; i++ )
{
QgsFeature f( mLayer->fields() );
f.setAttribute( QStringLiteral( "pk" ), i );
f.setAttribute( QStringLiteral( "material" ), QStringLiteral( "material_%1" ).arg( i ) );
f.setAttribute( QStringLiteral( "diameter" ), i );
f.setAttribute( QStringLiteral( "raccord" ), QStringLiteral( "raccord_%1" ).arg( i ) );
flist << f;
}
mLayer->addFeatures( flist );

mLayer->commitChanges();
}

@@ -200,20 +213,18 @@ void TestQgsFeatureListComboBox::testValuesAndSelection()
{
QFETCH( bool, allowNull );


QgsApplication::setNullRepresentation( QStringLiteral( "nope" ) );
std::unique_ptr<QgsFeatureListComboBox> cb( new QgsFeatureListComboBox() );

QgsFeatureFilterModel *model = qobject_cast<QgsFeatureFilterModel *>( cb->model() );
QEventLoop loop;
connect( model, &QgsFeatureFilterModel::filterJobCompleted, &loop, &QEventLoop::quit );
QSignalSpy spy( cb.get(), &QgsFeatureListComboBox::identifierValueChanged );

cb->setSourceLayer( mLayer.get() );
cb->setDisplayExpression( QStringLiteral( "\"raccord\"" ) );
cb->setAllowNull( allowNull );
cb->setIdentifierFields( {QStringLiteral( "raccord" )} );
cb->setDisplayExpression( QStringLiteral( "\"raccord\"" ) );

//check if everything is fine:
loop.exec();
spy.wait();
QCOMPARE( cb->currentIndex(), allowNull ? cb->nullIndex() : 0 );
QCOMPARE( cb->currentText(), allowNull ? QStringLiteral( "nope" ) : QStringLiteral( "brides" ) );

@@ -233,7 +244,7 @@ void TestQgsFeatureListComboBox::testValuesAndSelection()

//check with another entry, clear button needs to be there then:
QTest::keyClicks( cb.get(), QStringLiteral( "sleeve" ) );
loop.exec();
spy.wait();
QCOMPARE( cb->currentText(), QStringLiteral( "sleeve" ) );
QVERIFY( cb->mLineEdit->mClearAction );
}
@@ -279,5 +290,27 @@ void TestQgsFeatureListComboBox::testNotExistingYetFeature()
QCOMPARE( cb->currentText(), QStringLiteral( "(42)" ) );
}

void TestQgsFeatureListComboBox::testFeatureFurtherThanFetchLimit()
{
int fetchLimit = 20;
QVERIFY( fetchLimit < mLayer->featureCount() );
std::unique_ptr<QgsFeatureListComboBox> cb( new QgsFeatureListComboBox() );
QgsFeatureFilterModel *model = qobject_cast<QgsFeatureFilterModel *>( cb->model() );
QSignalSpy spy( cb.get(), &QgsFeatureListComboBox::identifierValueChanged );
model->setFetchLimit( 20 );
model->setAllowNull( false );
cb->setSourceLayer( mLayer.get() );
cb->setIdentifierFields( {QStringLiteral( "pk" )} );
spy.wait();
QCOMPARE( model->mEntries.count(), 20 );
for ( int i = 0; i < 20; i++ )
QCOMPARE( model->mEntries.at( i ).identifierFields.at( 0 ).toInt(), i + 10 );
cb->setIdentifierValues( {33} );
spy.wait();
QCOMPARE( cb->lineEdit()->text(), QStringLiteral( "33" ) );
QCOMPARE( model->mEntries.count(), 21 );
QCOMPARE( model->mEntries.at( 0 ).identifierFields.at( 0 ).toInt(), 33 );
}

QGSTEST_MAIN( TestQgsFeatureListComboBox )
#include "testqgsfeaturelistcombobox.moc"

1 comment on commit fe14dc9

@mhugent

This comment has been minimized.

Copy link
Contributor

@mhugent mhugent commented on fe14dc9 Aug 6, 2020

It would be great if this change was ported to 3.10.x.

Please sign in to comment.
You can’t perform that action at this time.