Skip to content

Commit

Permalink
fix(AttributeForm): Update same field widgets before firing signals
Browse files Browse the repository at this point in the history
If not, saveEdits would be called when we fire widgetValueChanged
signal and widgets containing non updated old values could potentially
overwrite new ones, leading to discard user modifications.
  • Loading branch information
troopa81 authored and nyalldawson committed Oct 12, 2024
1 parent ca9a5cf commit cc20aa1
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 13 deletions.
26 changes: 13 additions & 13 deletions src/gui/qgsattributeform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1036,6 +1036,19 @@ void QgsAttributeForm::onAttributeChanged( const QVariant &value, const QVariant

mCurrentFormFeature.setAttribute( eww->field().name(), value );

// Update other widgets pointing to the same field, required to happen now to insure
// currentFormValuesFeature() gets the right value when processing constraints
const QList<QgsAttributeFormEditorWidget *> formEditorWidgets = mFormEditorWidgets.values( eww->fieldIdx() );
for ( QgsAttributeFormEditorWidget *formEditorWidget : formEditorWidgets )
{
if ( formEditorWidget->editorWidget() == eww )
continue;

// formEditorWidget and eww points to the same field, so block signals
// as there is no need to handle valueChanged again for each duplicate
whileBlocking( formEditorWidget->editorWidget() )->setValue( value );
}

switch ( mMode )
{
case QgsAttributeEditorContext::SingleEditMode:
Expand Down Expand Up @@ -1091,19 +1104,6 @@ void QgsAttributeForm::onAttributeChanged( const QVariant &value, const QVariant
break;
}

// Update other widgets pointing to the same field, required to happen now to insure
// currentFormValuesFeature() gets the right value when processing constraints
const QList<QgsAttributeFormEditorWidget *> formEditorWidgets = mFormEditorWidgets.values( eww->fieldIdx() );
for ( QgsAttributeFormEditorWidget *formEditorWidget : formEditorWidgets )
{
if ( formEditorWidget->editorWidget() == eww )
continue;

// formEditorWidget and eww points to the same field, so block signals
// as there is no need to handle valueChanged again for each duplicate
whileBlocking( formEditorWidget->editorWidget() )->setValue( value );
}

updateConstraints( eww );

// Update dependent fields (only if form is not initializing)
Expand Down
46 changes: 46 additions & 0 deletions tests/src/gui/testqgsdualview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
#include <attributetable/qgsdualview.h>
#include <editform/qgsattributeeditorhtmlelement.h>
#include "qgsattributeform.h"
#include "qgsattributeeditorcontainer.h"
#include "qgsattributeeditorfield.h"
#include "qgsattributeformeditorwidget.h"
#include <qgsapplication.h>
#include "qgsfeatureiterator.h"
#include <qgsvectorlayer.h>
Expand Down Expand Up @@ -58,6 +61,8 @@ class TestQgsDualView : public QObject
void testAttributeFormSharedValueScanning();
void testNoGeom();

void testDuplicateField();

#ifdef WITH_QTWEBKIT
void testHtmlWidget_data();
void testHtmlWidget();
Expand Down Expand Up @@ -404,5 +409,46 @@ void TestQgsDualView::testHtmlWidget()
}
#endif

void TestQgsDualView::testDuplicateField()
{
// test updating same field appearing in different widget

// make a temporary vector layer
const QString def = QStringLiteral( "Point?field=col0:integer" );
QgsVectorLayer *layer = new QgsVectorLayer( def, QStringLiteral( "test" ), QStringLiteral( "memory" ) );
layer->setEditorWidgetSetup( 0, QgsEditorWidgetSetup( QStringLiteral( "Range" ), QVariantMap() ) );

// add same field twice so they get synced
QgsEditFormConfig editFormConfig = layer->editFormConfig();
editFormConfig.clearTabs();
editFormConfig.invisibleRootContainer()->addChildElement( new QgsAttributeEditorField( "col0", 0, editFormConfig.invisibleRootContainer() ) );
editFormConfig.invisibleRootContainer()->addChildElement( new QgsAttributeEditorField( "col0", 0, editFormConfig.invisibleRootContainer() ) );
editFormConfig.setLayout( Qgis::AttributeFormLayout::DragAndDrop );
layer->setEditFormConfig( editFormConfig );

// add a feature to the vector layer
QgsFeature ft( layer->dataProvider()->fields(), 1 );
ft.setAttribute( QStringLiteral( "col0" ), 1 );
layer->dataProvider()->addFeature( ft );

QgsDualView dualView;
dualView.init( layer, mCanvas );

layer->startEditing();

const QList<QgsAttributeFormEditorWidget *> formEditorWidgets = dualView.mAttributeForm->mFormEditorWidgets.values( 0 );

formEditorWidgets[0]->editorWidget()->setValues( 20, QVariantList() );
ft = layer->getFeature( ft.id() );
QCOMPARE( ft.attribute( QStringLiteral( "col0" ) ).toInt(), 20 );

formEditorWidgets[1]->editorWidget()->setValues( 21, QVariantList() );
ft = layer->getFeature( ft.id() );
QCOMPARE( ft.attribute( QStringLiteral( "col0" ) ).toInt(), 21 );

layer->rollBack();
}


QGSTEST_MAIN( TestQgsDualView )
#include "testqgsdualview.moc"

0 comments on commit cc20aa1

Please sign in to comment.