Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix feature list combobox entries, add tests and checks
  • Loading branch information
troopa81 committed Mar 11, 2020
1 parent 3246a92 commit 1a58083
Show file tree
Hide file tree
Showing 8 changed files with 276 additions and 130 deletions.
100 changes: 53 additions & 47 deletions src/core/qgsfeaturefiltermodel.cpp
Expand Up @@ -225,9 +225,11 @@ QVariant QgsFeatureFilterModel::data( const QModelIndex &index, int role ) const
void QgsFeatureFilterModel::updateCompleter()
{
emit beginUpdate();
if ( !mGatherer )

QgsFieldExpressionValuesGatherer *gatherer = qobject_cast<QgsFieldExpressionValuesGatherer *>( sender() );
if ( gatherer->wasCanceled() )
{
emit endUpdate();
delete gatherer;
return;
}

Expand All @@ -239,7 +241,8 @@ void QgsFeatureFilterModel::updateCompleter()
}

// Only reloading the current entry?
if ( mGatherer->data().toBool() )
bool reloadCurrentFeatureOnly = mGatherer->data().toBool();
if ( reloadCurrentFeatureOnly )
{
if ( !entries.isEmpty() )
{
Expand Down Expand Up @@ -270,31 +273,30 @@ void QgsFeatureFilterModel::updateCompleter()

const int newEntriesSize = entries.size();

// Find the index of the extra entry in the new list
// fixed entry is either NULL or extra value
const int nbFixedEntry = ( mExtraValueDoesNotExist ? 1 : 0 ) + ( mAllowNull ? 1 : 0 );

// Find the index of the current entry in the new list
int currentEntryInNewList = -1;
if ( mExtraIdentifierValueIndex != -1 )
if ( mExtraIdentifierValueIndex != -1 && mExtraIdentifierValueIndex < mEntries.count() )
{
for ( int i = 0; i < newEntriesSize; ++i )
{
if ( qVariantListCompare( entries.at( i ).identifierValues, mExtraIdentifierValues ) )
if ( qVariantListCompare( entries.at( i ).identifierValues, mEntries.at( mExtraIdentifierValueIndex ).identifierValues ) )
{
currentEntryInNewList = i;
mEntries.replace( mExtraIdentifierValueIndex, entries.at( i ) );
emit dataChanged( index( mExtraIdentifierValueIndex, 0, QModelIndex() ), index( mExtraIdentifierValueIndex, 0, QModelIndex() ) );
currentEntryInNewList = i;
setExtraValueDoesNotExist( false );
break;
}
}
}
else
{
Q_ASSERT_X( false, "QgsFeatureFilterModel::updateCompleter", "No extra identifier value generated. Should not get here." );
}

int firstRow = 0;

// Move the extra entry to the first position
if ( mExtraIdentifierValueIndex != -1 && currentEntryInNewList != -1 )
// Move current entry to the first position if this is a fixed entry or because
// the entry exists in the new list
if ( mExtraIdentifierValueIndex > -1 && ( mExtraIdentifierValueIndex < nbFixedEntry || currentEntryInNewList != -1 ) )
{
if ( mExtraIdentifierValueIndex != 0 )
{
Expand All @@ -309,19 +311,24 @@ void QgsFeatureFilterModel::updateCompleter()
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;
// if we remove all rows before endRemoveRows, setExtraIdentifierValuesUnguarded will be called
// and a null value will be added to mEntries, so we block setExtraIdentifierValuesUnguarded call

mIsSettingExtraIdentifierValue = true;
endRemoveRows();
mIsSettingExtraIdentifierValue = false;


if ( currentEntryInNewList == -1 )
{
beginInsertRows( QModelIndex(), 1, entries.size() + 1 );
beginInsertRows( QModelIndex(), firstRow, entries.size() + 1 );
mEntries += entries;
endInsertRows();
setExtraIdentifierValuesIndex( mAllowNull && !mEntries.isEmpty() ? 1 : 0 );

// if all entries have been cleaned (firstRow == 0)
// and there is a value in entries, prefer this value over NULL
// else chose the first one (the previous one)
setExtraIdentifierValuesIndex( firstRow == 0 && mAllowNull && !entries.isEmpty() ? 1 : 0, firstRow == 0 );
}
else
{
Expand All @@ -336,7 +343,11 @@ void QgsFeatureFilterModel::updateCompleter()
mEntries.replace( 0, entries.at( 0 ) );
}

emit dataChanged( index( currentEntryInNewList, 0, QModelIndex() ), index( currentEntryInNewList, 0, QModelIndex() ) );
// don't notify for a change if it's a fixed entry
if ( currentEntryInNewList >= nbFixedEntry )
{
emit dataChanged( index( currentEntryInNewList, 0, QModelIndex() ), index( currentEntryInNewList, 0, QModelIndex() ) );
}

beginInsertRows( QModelIndex(), currentEntryInNewList + 1, newEntriesSize - currentEntryInNewList - 1 );
mEntries += entries.mid( currentEntryInNewList + 1 );
Expand All @@ -348,13 +359,10 @@ void QgsFeatureFilterModel::updateCompleter()
}
emit endUpdate();

gathererThreadFinished();
}

void QgsFeatureFilterModel::gathererThreadFinished()
{
// It's possible that gatherer run method is not completely over, so we wait
mGatherer->wait();
// 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 );
delete mGatherer;
mGatherer = nullptr;
emit isLoadingChanged();
Expand All @@ -369,10 +377,6 @@ void QgsFeatureFilterModel::scheduledReload()

if ( mGatherer )
{
// Send the gatherer thread to the graveyard:
// forget about it, tell it to stop and delete when finished
disconnect( mGatherer, &QgsFieldExpressionValuesGatherer::collectedValues, this, &QgsFeatureFilterModel::updateCompleter );
connect( mGatherer, &QgsFieldExpressionValuesGatherer::finished, mGatherer, &QgsFieldExpressionValuesGatherer::deleteLater );
mGatherer->stop();
wasLoading = true;
}
Expand Down Expand Up @@ -421,8 +425,8 @@ void QgsFeatureFilterModel::scheduledReload()

mGatherer = new QgsFieldExpressionValuesGatherer( mSourceLayer, mDisplayExpression, mIdentifierFields, request );
mGatherer->setData( mShouldReloadCurrentFeature );
connect( mGatherer, &QgsFieldExpressionValuesGatherer::finished, this, &QgsFeatureFilterModel::updateCompleter );

connect( mGatherer, &QgsFieldExpressionValuesGatherer::collectedValues, this, &QgsFeatureFilterModel::updateCompleter );

mGatherer->start();
if ( !wasLoading )
Expand Down Expand Up @@ -489,24 +493,26 @@ void QgsFeatureFilterModel::setExtraIdentifierValuesUnguarded( const QVariantLis
// Value not found in current entries
if ( mExtraIdentifierValueIndex != index )
{
beginInsertRows( QModelIndex(), 0, 0 );
if ( extraIdentifierValues.isEmpty() )
{
mEntries.prepend( nullEntry() );
}
else
if ( !extraIdentifierValues.isEmpty() || mAllowNull )
{
QStringList values;
for ( const QVariant &v : qgis::as_const( extraIdentifierValues ) )
values << QStringLiteral( "(%1)" ).arg( v.toString() );

mEntries.prepend( Entry( extraIdentifierValues, values.join( QStringLiteral( " " ) ), QgsFeature() ) );
}
endInsertRows();
beginInsertRows( QModelIndex(), 0, 0 );
if ( !extraIdentifierValues.isEmpty() )
{
QStringList values;
for ( const QVariant &v : qgis::as_const( extraIdentifierValues ) )
values << QStringLiteral( "(%1)" ).arg( v.toString() );

setExtraIdentifierValuesIndex( 0, true );
mEntries.prepend( Entry( extraIdentifierValues, values.join( QStringLiteral( " " ) ), QgsFeature() ) );
reloadCurrentFeature();
}
else
{
mEntries.prepend( nullEntry() );
}
endInsertRows();

reloadCurrentFeature();
setExtraIdentifierValuesIndex( 0, true );
}
}
}

Expand Down
1 change: 0 additions & 1 deletion src/core/qgsfeaturefiltermodel.h
Expand Up @@ -296,7 +296,6 @@ class CORE_EXPORT QgsFeatureFilterModel : public QAbstractItemModel

private slots:
void updateCompleter();
void gathererThreadFinished();
void scheduledReload();

private:
Expand Down
20 changes: 9 additions & 11 deletions src/core/qgsfeaturefiltermodel_p.h
Expand Up @@ -16,6 +16,7 @@
#define QGSFEATUREFILTERMODEL_P_H

#include <QThread>
#include <QMutex>
#include "qgsfeaturefiltermodel.h"
#include "qgslogger.h"
#include "qgsvectorlayerfeatureiterator.h"
Expand Down Expand Up @@ -68,21 +69,25 @@ class QgsFieldExpressionValuesGatherer: public QThread
attributes << feat.attribute( idx );
mEntries.append( QgsFeatureFilterModel::Entry( attributes, mDisplayExpression.evaluate( &mExpressionContext ).toString(), feat ) );

QMutexLocker locker( &mCancelMutex );
if ( mWasCanceled )
return;
}

emit collectedValues();
}

//! Informs the gatherer to immediately stop collecting values
void stop()
{
QMutexLocker locker( &mCancelMutex );
mWasCanceled = true;
}

//! Returns TRUE if collection was canceled before completion
bool wasCanceled() const { return mWasCanceled; }
bool wasCanceled() const
{
QMutexLocker locker( &mCancelMutex );
return mWasCanceled;
}

QVector<QgsFeatureFilterModel::Entry> entries() const
{
Expand Down Expand Up @@ -110,14 +115,6 @@ class QgsFieldExpressionValuesGatherer: public QThread
mData = data;
}

signals:

/**
* Emitted when values have been collected
* \param values list of unique matching string values
*/
void collectedValues();

private:

std::unique_ptr<QgsVectorLayerFeatureSource> mSource;
Expand All @@ -126,6 +123,7 @@ class QgsFieldExpressionValuesGatherer: public QThread
QgsFeatureRequest mRequest;
QgsFeatureIterator mIterator;
bool mWasCanceled = false;
mutable QMutex mCancelMutex;
QVector<QgsFeatureFilterModel::Entry> mEntries;
QStringList mIdentifierFields;
QVariant mData;
Expand Down
35 changes: 0 additions & 35 deletions src/gui/editorwidgets/qgsrelationreferencewidget.cpp
Expand Up @@ -164,7 +164,6 @@ QgsRelationReferenceWidget::QgsRelationReferenceWidget( QWidget *parent )
connect( mRemoveFKButton, &QAbstractButton::clicked, this, &QgsRelationReferenceWidget::deleteForeignKeys );
connect( mAddEntryButton, &QAbstractButton::clicked, this, &QgsRelationReferenceWidget::addEntry );
connect( mComboBox, &QComboBox::editTextChanged, this, &QgsRelationReferenceWidget::updateAddEntryButton );
connect( mComboBox, &QgsFeatureListComboBox::modelUpdated, this, &QgsRelationReferenceWidget::updateIndex );
}

QgsRelationReferenceWidget::~QgsRelationReferenceWidget()
Expand All @@ -173,38 +172,6 @@ QgsRelationReferenceWidget::~QgsRelationReferenceWidget()
unsetMapTool();
}

void QgsRelationReferenceWidget::updateIndex()
{
if ( mChainFilters && mComboBox->count() > 0 )
{
int index = -1;

// uninitialized filter
if ( ! mFilterComboBoxes.isEmpty()
&& mFilterComboBoxes[0]->currentIndex() == 0 && mAllowNull )
{
index = mComboBox->nullIndex();
}
else if ( mComboBox->count() > mComboBox->nullIndex() )
{
index = mComboBox->nullIndex() + 1;
}
else if ( mAllowNull )
{
index = mComboBox->nullIndex();
}
else
{
index = 0;
}

if ( mComboBox->count() > index )
{
mComboBox->setCurrentIndex( index );
}
}
}

void QgsRelationReferenceWidget::setRelation( const QgsRelation &relation, bool allowNullValue )
{
mAllowNull = allowNullValue;
Expand Down Expand Up @@ -615,8 +582,6 @@ 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 );
//call it for the first time
emit mComboBox->currentIndexChanged( mComboBox->currentIndex() );

QApplication::restoreOverrideCursor();

Expand Down
6 changes: 0 additions & 6 deletions src/gui/editorwidgets/qgsrelationreferencewidget.h
Expand Up @@ -282,12 +282,6 @@ class GUI_EXPORT QgsRelationReferenceWidget : public QWidget
void entryAdded( const QgsFeature &f );
void onKeyPressed( QKeyEvent *e );

/**
* Updates the FK index as soon as the underlying model is updated when
* the chainFilter option is activated.
*/
void updateIndex();

private:
void highlightFeature( QgsFeature f = QgsFeature(), CanvasExtent canvasExtent = Fixed );
void updateAttributeEditorFrame( const QgsFeature &feature );
Expand Down
1 change: 1 addition & 0 deletions src/gui/qgsfeaturelistcombobox.cpp
Expand Up @@ -236,6 +236,7 @@ bool QgsFeatureListComboBox::allowNull() const
void QgsFeatureListComboBox::setAllowNull( bool allowNull )
{
mModel->setAllowNull( allowNull );
mLineEdit->setClearMode( allowNull ? QgsFilterLineEdit::ClearToNull : QgsFilterLineEdit::ClearToDefault );
}

QVariant QgsFeatureListComboBox::identifierValue() const
Expand Down

0 comments on commit 1a58083

Please sign in to comment.