Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix identify on map in relation reference widget #10047

Merged
merged 2 commits into from May 22, 2019
Merged

Fix identify on map in relation reference widget #10047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b14dac
Fix identify on map in relation reference widget
elpaso May 21, 2019
ad019c4
Remove raw string (not necessary here)
elpaso May 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion python/gui/auto_generated/editorwidgets/qgsrelationreferencewidget.sip.in
View file
Open in desktop
Expand Up @@ -72,7 +72,7 @@ determines if the foreign key is shown in a combox box or a read-only line edit

bool allowMapIdentification();
%Docstring
determines if the widge offers the possibility to select the related feature on the map (using a dedicated map tool)
determines if the widget offers the possibility to select the related feature on the map (using a dedicated map tool)
%End
void setAllowMapIdentification( bool allowMapIdentification );

Expand Down
4 changes: 2 additions & 2 deletions src/gui/editorwidgets/qgsrelationreferencewidget.cpp
View file
Open in desktop
Expand Up @@ -41,7 +41,7 @@
#include "qgsfeatureiterator.h"
#include "qgsfeaturelistcombobox.h"
#include "qgsexpressioncontextutils.h"

#include "qgsfeaturefiltermodel.h"

QgsRelationReferenceWidget::QgsRelationReferenceWidget( QWidget *parent )
: QWidget( parent )
Expand Down Expand Up @@ -746,7 +746,7 @@ void QgsRelationReferenceWidget::featureIdentified( const QgsFeature &feature )
}
else
{
mComboBox->setCurrentIndex( mComboBox->findData( feature.id(), QgsAttributeTableModel::FeatureIdRole ) );
mComboBox->setCurrentIndex( mComboBox->findData( feature.attribute( mReferencedFieldIdx ), QgsFeatureFilterModel::Role::IdentifierValueRole ) );
mFeature = feature;
}

Expand Down
2 changes: 1 addition & 1 deletion src/gui/editorwidgets/qgsrelationreferencewidget.h
View file
Open in desktop
Expand Up @@ -99,7 +99,7 @@ class GUI_EXPORT QgsRelationReferenceWidget : public QWidget
bool readOnlySelector() { return mReadOnlySelector; }
void setReadOnlySelector( bool readOnly );

//! determines if the widge offers the possibility to select the related feature on the map (using a dedicated map tool)
//! determines if the widget offers the possibility to select the related feature on the map (using a dedicated map tool)
bool allowMapIdentification() { return mAllowMapIdentification; }
void setAllowMapIdentification( bool allowMapIdentification );

Expand Down
34 changes: 34 additions & 0 deletions tests/src/gui/testqgsrelationreferencewidget.cpp
View file
Open in desktop
Expand Up @@ -47,6 +47,7 @@ class TestQgsRelationReferenceWidget : public QObject
void testChainFilterDeleteForeignKey();
void testInvalidRelation();
void testSetGetForeignKey();
void testIdentifyOnMap();

private:
std::unique_ptr<QgsVectorLayer> mLayer1;
Expand Down Expand Up @@ -340,5 +341,38 @@ void TestQgsRelationReferenceWidget::testSetGetForeignKey()
QCOMPARE( spy.count(), 3 );
}


// Test issue https://issues.qgis.org/issues/22071
// Relation reference widget wrong feature when "on map identification"
void TestQgsRelationReferenceWidget::testIdentifyOnMap()
{
QWidget parentWidget;
QgsRelationReferenceWidget w( &parentWidget );
QVERIFY( mLayer1->startEditing() );
w.setRelation( *mRelation, true );
w.setAllowMapIdentification( true );
w.init();
QEventLoop loop;
// Populate model (I tried to listen to signals but the module reload() runs twice
// (the first load triggers a second one which does the population of the combo)
// and I haven't fin a way to properly wait for it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe casting the combobox to a QgsFeatureListComboBox and then connecting to the models isLoadingChanged signal?
But it's not trivial

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe casting the combobox to a QgsFeatureListComboBox and then connecting to the models isLoadingChanged signal?
But it's not trivial

I tried that, but the isLoadingChanged was triggered twice as well (4 times, really), and I missed the second reload (which actually loads the data in the combo), but I admit that the logic in that loading behavior is not clear to me. I thought about adding another signal but I'm not sure it would be useful out of the test, so I prefer to not add it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC: The logic is to as quickly as possible get the display value of the current selected item to show up to the user. And then with a little delay lazy-load also some additional values to fill the popup.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe connecting to the signal and then checking if the rowCount of the model is > 1?

QTimer::singleShot( 300, [&] { loop.quit(); } );
loop.exec();
QgsFeature feature;
mLayer2->getFeatures( QStringLiteral( "pk = %1" ).arg( 11 ) ).nextFeature( feature );
QVERIFY( feature.isValid() );
QCOMPARE( feature.attribute( QStringLiteral( "pk" ) ).toInt(), 11 );
w.featureIdentified( feature );
QCOMPARE( w.mComboBox->currentData( Qt::DisplayRole ).toInt(), 11 );

mLayer2->getFeatures( QStringLiteral( "pk = %1" ).arg( 10 ) ).nextFeature( feature );
QVERIFY( feature.isValid() );
QCOMPARE( feature.attribute( QStringLiteral( "pk" ) ).toInt(), 10 );
w.featureIdentified( feature );
QCOMPARE( w.mComboBox->currentData( Qt::DisplayRole ).toInt(), 10 );

mLayer1->rollBack();
}

QGSTEST_MAIN( TestQgsRelationReferenceWidget )
#include "testqgsrelationreferencewidget.moc"