From 40ad86024353a3d24b109877dbf2cdedad8a7bc6 Mon Sep 17 00:00:00 2001 From: Blottiere Paul Date: Fri, 21 Jul 2017 09:50:49 +0100 Subject: [PATCH 1/2] Fix relation reference widget when 'Chain Filters' is activated. Fixes #16903 --- .../qgsrelationreferencewidget.cpp | 124 +++++++++++++----- .../qgsrelationreferencewidget.h | 1 + 2 files changed, 93 insertions(+), 32 deletions(-) diff --git a/src/gui/editorwidgets/qgsrelationreferencewidget.cpp b/src/gui/editorwidgets/qgsrelationreferencewidget.cpp index 0d91c9881fcc..ada14e5aba47 100644 --- a/src/gui/editorwidgets/qgsrelationreferencewidget.cpp +++ b/src/gui/editorwidgets/qgsrelationreferencewidget.cpp @@ -785,13 +785,41 @@ void QgsRelationReferenceWidget::filterChanged() { QVariant nullValue = QSettings().value( "qgis/nullValue", "NULL" ); - QStringList filters; + QMap filters; QgsAttributeList attrs; QComboBox* scb = qobject_cast( sender() ); Q_ASSERT( scb ); + QgsFeature f; + QgsFeatureIds featureIds; + QString filterExpression; + + // comboboxes have to be disabled before building filters + if ( mChainFilters ) + disableChainedComboBoxes( scb ); + + // build filters + Q_FOREACH ( QComboBox *cb, mFilterComboBoxes ) + { + if ( cb->currentIndex() != 0 ) + { + const QString fieldName = cb->property( "Field" ).toString(); + + if ( cb->currentText() == nullValue.toString() ) + { + filters[fieldName] = QString( "\"%1\" IS NULL" ).arg( fieldName ); + } + else + { + filters[fieldName] = QgsExpression::createFieldEqualityExpression( fieldName, cb->currentText() ); + } + attrs << mReferencedLayer->fieldNameIndex( fieldName ); + } + } + + bool filtered = false; if ( mChainFilters ) { QComboBox* ccb = nullptr; @@ -805,13 +833,11 @@ void QgsRelationReferenceWidget::filterChanged() continue; } - if ( ccb->currentIndex() == 0 ) - { - cb->setCurrentIndex( 0 ); - cb->setEnabled( false ); - } - else + if ( ccb->currentIndex() != 0 ) { + const QString fieldName = cb->property( "Field" ).toString(); + filtered = true; + cb->blockSignals( true ); cb->clear(); cb->addItem( cb->property( "FieldAlias" ).toString() ); @@ -821,8 +847,30 @@ void QgsRelationReferenceWidget::filterChanged() QStringList texts; Q_FOREACH ( const QString& txt, mFilterCache[ccb->property( "Field" ).toString()][ccb->currentText()] ) { - texts << txt; + QMap filtersAttrs = filters; + filtersAttrs[fieldName] = QgsExpression::createFieldEqualityExpression( fieldName, txt ); + QStringList vals = filtersAttrs.values(); + QString expression = vals.join( QString( " AND " ) ); + + QgsAttributeList subset = attrs; + subset << mReferencedLayer->fieldNameIndex( fieldName ); + + QgsFeatureIterator it( mMasterModel->layerCache()->getFeatures( QgsFeatureRequest().setFilterExpression( expression ).setSubsetOfAttributes( subset ) ) ); + + bool found = false; + while ( it.nextFeature( f ) ) + { + if ( !featureIds.contains( f.id() ) ) + featureIds << f.id(); + + found = true; + } + + // item is only provided if at least 1 feature exists + if ( found ) + texts << txt; } + texts.sort(); cb->addItems( texts ); @@ -834,34 +882,21 @@ void QgsRelationReferenceWidget::filterChanged() } } - Q_FOREACH ( QComboBox* cb, mFilterComboBoxes ) + if ( !mChainFilters || ( mChainFilters && !filtered ) ) { - if ( cb->currentIndex() != 0 ) - { - const QString fieldName = cb->property( "Field" ).toString(); + QStringList vals = filters.values(); + filterExpression = vals.join( QString( " AND " ) ); - if ( cb->currentText() == nullValue.toString() ) - { - filters << QString( "\"%1\" IS NULL" ).arg( fieldName ); - } - else - { - filters << QgsExpression::createFieldEqualityExpression( fieldName, cb->currentText() ); - } - attrs << mReferencedLayer->fieldNameIndex( fieldName ); - } - } - - QString filterExpression = filters.join( " AND " ); - - QgsFeatureIterator it( mMasterModel->layerCache()->getFeatures( QgsFeatureRequest().setFilterExpression( filterExpression ).setSubsetOfAttributes( attrs ) ) ); + QgsFeatureRequest req = QgsFeatureRequest().setSubsetOfAttributes( attrs ); + if ( !filterExpression.isEmpty() ) + req.setFilterExpression( filterExpression ); - QgsFeature f; - QgsFeatureIds featureIds; + QgsFeatureIterator it( mMasterModel->layerCache()->getFeatures( req ) ); - while ( it.nextFeature( f ) ) - { - featureIds << f.id(); + while ( it.nextFeature( f ) ) + { + featureIds << f.id(); + } } mFilterModel->setFilteredFeatures( featureIds ); @@ -896,3 +931,28 @@ void QgsRelationReferenceWidget::updateAddEntryButton() mAddEntryButton->setVisible( mAllowAddFeatures ); mAddEntryButton->setEnabled( mReferencedLayer && mReferencedLayer->isEditable() ); } + +void QgsRelationReferenceWidget::disableChainedComboBoxes( const QComboBox *scb ) +{ + QComboBox *ccb = nullptr; + Q_FOREACH ( QComboBox *cb, mFilterComboBoxes ) + { + if ( !ccb ) + { + if ( cb == scb ) + { + ccb = cb; + } + + continue; + } + + if ( ccb->currentIndex() == 0 ) + { + cb->setCurrentIndex( 0 ); + cb->setEnabled( false ); + } + else + ccb = cb; + } +} diff --git a/src/gui/editorwidgets/qgsrelationreferencewidget.h b/src/gui/editorwidgets/qgsrelationreferencewidget.h index 4031c6d2f888..5009d9b7eeaa 100644 --- a/src/gui/editorwidgets/qgsrelationreferencewidget.h +++ b/src/gui/editorwidgets/qgsrelationreferencewidget.h @@ -162,6 +162,7 @@ class GUI_EXPORT QgsRelationReferenceWidget : public QWidget private: void highlightFeature( QgsFeature f = QgsFeature(), CanvasExtent canvasExtent = Fixed ); void updateAttributeEditorFrame( const QgsFeature& feature ); + void disableChainedComboBoxes( const QComboBox *scb ); // initialized QgsAttributeEditorContext mEditorContext; From b5bdafe4d314938dd66a1bdbfc85bb27bae7cec5 Mon Sep 17 00:00:00 2001 From: Blottiere Paul Date: Fri, 21 Jul 2017 14:23:37 +0100 Subject: [PATCH 2/2] Add tests --- .../qgsrelationreferencewidget.cpp | 6 +- .../qgsrelationreferencewidget.h | 2 + tests/src/gui/CMakeLists.txt | 2 + .../gui/testqgsrelationreferencewidget.cpp | 183 ++++++++++++++++++ 4 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 tests/src/gui/testqgsrelationreferencewidget.cpp diff --git a/src/gui/editorwidgets/qgsrelationreferencewidget.cpp b/src/gui/editorwidgets/qgsrelationreferencewidget.cpp index ada14e5aba47..c636435d09d7 100644 --- a/src/gui/editorwidgets/qgsrelationreferencewidget.cpp +++ b/src/gui/editorwidgets/qgsrelationreferencewidget.cpp @@ -191,10 +191,10 @@ void QgsRelationReferenceWidget::setRelation( const QgsRelation& relation, bool mReferencedFieldIdx = mReferencedLayer->fieldNameIndex( relation.fieldPairs().at( 0 ).second ); mReferencingFieldIdx = mReferencingLayer->fieldNameIndex( relation.fieldPairs().at( 0 ).first ); - QgsAttributeEditorContext context( mEditorContext, relation, QgsAttributeEditorContext::Single, QgsAttributeEditorContext::Embed ); if ( mEmbedForm ) { + QgsAttributeEditorContext context( mEditorContext, relation, QgsAttributeEditorContext::Single, QgsAttributeEditorContext::Embed ); mAttributeEditorFrame->setTitle( mReferencedLayer->name() ); mReferencedAttributeForm = new QgsAttributeForm( relation.referencedLayer(), QgsFeature(), context, this ); mAttributeEditorLayout->addWidget( mReferencedAttributeForm ); @@ -464,6 +464,10 @@ void QgsRelationReferenceWidget::init() { QVariantList uniqueValues; int idx = mReferencedLayer->fieldNameIndex( fieldName ); + + if ( idx == -1 ) + continue; + QComboBox* cb = new QComboBox(); cb->setProperty( "Field", fieldName ); cb->setProperty( "FieldAlias", mReferencedLayer->attributeDisplayName( idx ) ); diff --git a/src/gui/editorwidgets/qgsrelationreferencewidget.h b/src/gui/editorwidgets/qgsrelationreferencewidget.h index 5009d9b7eeaa..0579612d2dc5 100644 --- a/src/gui/editorwidgets/qgsrelationreferencewidget.h +++ b/src/gui/editorwidgets/qgsrelationreferencewidget.h @@ -219,6 +219,8 @@ class GUI_EXPORT QgsRelationReferenceWidget : public QWidget QVBoxLayout* mAttributeEditorLayout; QLineEdit* mLineEdit; QLabel* mInvalidLabel; + + friend class TestQgsRelationReferenceWidget; }; #endif // QGSRELATIONREFERENCEWIDGET_H diff --git a/tests/src/gui/CMakeLists.txt b/tests/src/gui/CMakeLists.txt index 9e5ac2f18a45..bcaf2c788441 100644 --- a/tests/src/gui/CMakeLists.txt +++ b/tests/src/gui/CMakeLists.txt @@ -14,6 +14,7 @@ INCLUDE_DIRECTORIES(${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/../../../src/gui/editorwidgets/core ${CMAKE_CURRENT_SOURCE_DIR}/../../../src/gui/symbology-ng ${CMAKE_CURRENT_SOURCE_DIR}/../../../src/gui/raster + ${CMAKE_CURRENT_SOURCE_DIR}/../../../src/gui/attributetable ${CMAKE_CURRENT_SOURCE_DIR}/../../../src/core ${CMAKE_CURRENT_SOURCE_DIR}/../../../src/core/auth ${CMAKE_CURRENT_SOURCE_DIR}/../../../src/core/composer @@ -145,3 +146,4 @@ ADD_QGIS_TEST(sqlcomposerdialog testqgssqlcomposerdialog.cpp) ADD_QGIS_TEST(filedownloader testqgsfiledownloader.cpp) ADD_QGIS_TEST(composergui testqgscomposergui.cpp) ADD_QGIS_TEST(valuerelationwidgetwrapper testqgsvaluerelationwidgetwrapper.cpp) +ADD_QGIS_TEST(relationreferencewidget testqgsrelationreferencewidget.cpp) diff --git a/tests/src/gui/testqgsrelationreferencewidget.cpp b/tests/src/gui/testqgsrelationreferencewidget.cpp new file mode 100644 index 000000000000..1b1200031561 --- /dev/null +++ b/tests/src/gui/testqgsrelationreferencewidget.cpp @@ -0,0 +1,183 @@ +/*************************************************************************** + testqgsrelationreferencewidget.cpp + -------------------------------------- + Date : 21 07 2017 + Copyright : (C) 2017 Paul Blottiere + Email : paul dot blottiere at oslandia dot com + *************************************************************************** + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + ***************************************************************************/ + +#include +#include +#include +#include "qgseditorwidgetwrapper.h" +#include +#include +#include +#include +#include +#include + +class TestQgsRelationReferenceWidget : public QObject +{ + Q_OBJECT + public: + TestQgsRelationReferenceWidget() {} + + private slots: + void initTestCase(); // will be called before the first testfunction is executed. + void cleanupTestCase(); // will be called after the last testfunction was executed. + void init(); // will be called before each testfunction is executed. + void cleanup(); // will be called after every testfunction. + + void testChainFilter(); +}; + +void TestQgsRelationReferenceWidget::initTestCase() +{ + QgsApplication::init(); + QgsApplication::initQgis(); + QgsEditorWidgetRegistry::initEditors(); +} + +void TestQgsRelationReferenceWidget::cleanupTestCase() +{ + QgsApplication::exitQgis(); +} + +void TestQgsRelationReferenceWidget::init() +{ +} + +void TestQgsRelationReferenceWidget::cleanup() +{ +} + +void TestQgsRelationReferenceWidget::testChainFilter() +{ + // create layers + QgsVectorLayer vl1( QString( "LineString?crs=epsg:3111&field=pk:int&field=fk:int" ), QString( "vl1" ), QString( "memory" ) ); + QgsMapLayerRegistry::instance()->addMapLayer( &vl1 ); + QgsVectorLayer vl2( QString( "LineString?field=pk:int&field=material:string&field=diameter:int&field=raccord:string" ), QString( "vl2" ), QString( "memory" ) ); + QgsMapLayerRegistry::instance()->addMapLayer( &vl2 ); + + // create a relation between them + QgsRelation relation; + relation.setRelationId( QString( "vl1.vl2" ) ); + relation.setRelationName( QString( "vl1.vl2" ) ); + relation.setReferencingLayer( vl1.id() ); + relation.setReferencedLayer( vl2.id() ); + relation.addFieldPair( "fk", "pk" ); + QVERIFY( relation.isValid() ); + QgsProject::instance()->relationManager()->addRelation( relation ); + + // add features + QgsFeature ft0( vl1.fields() ); + ft0.setAttribute( QString( "pk" ), 0 ); + ft0.setAttribute( QString( "fk" ), 0 ); + vl1.startEditing(); + vl1.addFeature( ft0 ); + vl1.commitChanges(); + + QgsFeature ft1( vl1.fields() ); + ft1.setAttribute( QString( "pk" ), 1 ); + ft1.setAttribute( QString( "fk" ), 1 ); + vl1.startEditing(); + vl1.addFeature( ft1 ); + vl1.commitChanges(); + + QgsFeature ft2( vl2.fields() ); + ft2.setAttribute( QString( "pk" ), 10 ); + ft2.setAttribute( QString( "material" ), "iron" ); + ft2.setAttribute( QString( "diameter" ), 120 ); + ft2.setAttribute( QString( "raccord" ), "brides" ); + vl2.startEditing(); + vl2.addFeature( ft2 ); + vl2.commitChanges(); + + QgsFeature ft3( vl2.fields() ); + ft3.setAttribute( QString( "pk" ), 11 ); + ft3.setAttribute( QString( "material" ), "iron" ); + ft3.setAttribute( QString( "diameter" ), 120 ); + ft3.setAttribute( QString( "raccord" ), "sleeve" ); + vl2.startEditing(); + vl2.addFeature( ft3 ); + vl2.commitChanges(); + + QgsFeature ft4( vl2.fields() ); + ft4.setAttribute( QString( "pk" ), 12 ); + ft4.setAttribute( QString( "material" ), "steel" ); + ft4.setAttribute( QString( "diameter" ), 120 ); + ft4.setAttribute( QString( "raccord" ), "collar" ); + vl2.startEditing(); + vl2.addFeature( ft4 ); + vl2.commitChanges(); + + // init a relation reference widget + QStringList filterFields = { "material", "diameter", "raccord" }; + + QgsRelationReferenceWidget w( new QWidget() ); + w.setChainFilters( true ); + w.setFilterFields( filterFields ); + w.setRelation( relation, true ); + w.init(); + + // check default status for comboboxes + QList cbs = w.mFilterComboBoxes; + QCOMPARE( cbs.count(), 3 ); + Q_FOREACH ( const QComboBox *cb, cbs ) + { + if ( cb->currentText() == "raccord" ) + QCOMPARE( cb->count(), 5 ); + else if ( cb->currentText() == "material" ) + QCOMPARE( cb->count(), 4 ); + else if ( cb->currentText() == "diameter" ) + QCOMPARE( cb->count(), 3 ); + } + + // set first filter + cbs[0]->setCurrentIndex( cbs[0]->findText( "iron" ) ); + cbs[1]->setCurrentIndex( cbs[1]->findText( "120" ) ); + + Q_FOREACH ( const QComboBox *cb, cbs ) + { + if ( cb->itemText( 0 ) == "material" ) + QCOMPARE( cb->count(), 4 ); + else if ( cb->itemText( 0 ) == "diameter" ) + QCOMPARE( cb->count(), 2 ); + else if ( cb->itemText( 0 ) == "raccord" ) + { + QStringList items; + for ( int i = 0; i < cb->count(); i++ ) + items << cb->itemText( i ); + + QCOMPARE( cb->count(), 3 ); + QCOMPARE(( bool )items.contains( "collar" ), false ); + // collar should not be available in combobox as there's no existing + // feature with the filter expression: + // "material" == 'iron' AND "diameter" == '120' AND "raccord" = 'collar' + } + } + + // set the filter for "raccord" and then reset filter for "diameter". As + // chain filter is activated, the filter on "raccord" field should be reset + cbs[2]->setCurrentIndex( cbs[2]->findText( "brides" ) ); + cbs[1]->setCurrentIndex( cbs[1]->findText( "diameter" ) ); + + // combobox should propose NULL, 10 and 11 because the filter is now: + // "material" == 'iron' + QCOMPARE( w.mComboBox->count(), 3 ); + + // if there's no filter at all, all features' id should be proposed + cbs[0]->setCurrentIndex( cbs[0]->findText( "material" ) ); + QCOMPARE( w.mComboBox->count(), 4 ); +} + +QTEST_MAIN( TestQgsRelationReferenceWidget ) +#include "testqgsrelationreferencewidget.moc"