From f07389b421065d40abe9b5355c8e329229f792d9 Mon Sep 17 00:00:00 2001
From: Denis Rouzaud <denis.rouzaud@gmail.com>
Date: Sat, 24 Mar 2018 18:30:45 +0100
Subject: [PATCH] fix constraints not checked when they should be (#6550)
 (#6599)

---
 python/gui/qgsattributeform.sip.in     | 15 ++++++++++++++-
 src/gui/attributetable/qgsdualview.cpp |  9 ++++++---
 src/gui/attributetable/qgsdualview.h   |  2 +-
 src/gui/qgsattributeform.cpp           | 21 +++++++++++++++------
 src/gui/qgsattributeform.h             | 14 +++++++++++++-
 tests/src/gui/testqgsattributeform.cpp | 14 +++++++-------
 6 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/python/gui/qgsattributeform.sip.in b/python/gui/qgsattributeform.sip.in
index 8b489dd3089..45deba11a31 100644
--- a/python/gui/qgsattributeform.sip.in
+++ b/python/gui/qgsattributeform.sip.in
@@ -139,12 +139,25 @@ on all attribute widgets.
 
   signals:
 
-    void attributeChanged( const QString &attribute, const QVariant &value );
+ void attributeChanged( const QString &attribute, const QVariant &value );
 %Docstring
 Notifies about changes of attributes
 
 :param attribute: The name of the attribute that changed.
 :param value:     The new value of the attribute.
+
+.. deprecated:: since 3.0
+%End
+
+    void widgetValueChanged( const QString &attribute, const QVariant &value, bool attributeChanged );
+%Docstring
+Notifies about changes of attributes
+
+:param attribute: The name of the attribute that changed.
+:param value:     The new value of the attribute.
+:param attributeChanged: If true, it corresponds to an actual change of the feature attribute
+
+.. versionadded:: 3.0.1
 %End
 
 
diff --git a/src/gui/attributetable/qgsdualview.cpp b/src/gui/attributetable/qgsdualview.cpp
index d7ccfa1a8a1..2c640036a02 100644
--- a/src/gui/attributetable/qgsdualview.cpp
+++ b/src/gui/attributetable/qgsdualview.cpp
@@ -98,7 +98,7 @@ void QgsDualView::init( QgsVectorLayer *layer, QgsMapCanvas *mapCanvas, const Qg
     mAttributeEditor->layout()->addWidget( mAttributeForm );
   }
 
-  connect( mAttributeForm, &QgsAttributeForm::attributeChanged, this, &QgsDualView::featureFormAttributeChanged );
+  connect( mAttributeForm, &QgsAttributeForm::widgetValueChanged, this, &QgsDualView::featureFormAttributeChanged );
   connect( mAttributeForm, &QgsAttributeForm::modeChanged, this, &QgsDualView::formModeChanged );
   connect( mMasterModel, &QgsAttributeTableModel::modelChanged, mAttributeForm, &QgsAttributeForm::refreshFeature );
   connect( mAttributeForm, &QgsAttributeForm::filterExpressionSet, this, &QgsDualView::filterExpressionSet );
@@ -880,9 +880,12 @@ void QgsDualView::extentChanged()
   emit filterChanged();
 }
 
-void QgsDualView::featureFormAttributeChanged()
+void QgsDualView::featureFormAttributeChanged( const QString &attribute, const QVariant &value, bool attributeChanged )
 {
-  mFeatureList->setCurrentFeatureEdited( true );
+  Q_UNUSED( attribute );
+  Q_UNUSED( value );
+  if ( attributeChanged )
+    mFeatureList->setCurrentFeatureEdited( true );
 }
 
 void QgsDualView::setFilteredFeatures( const QgsFeatureIds &filteredFeatures )
diff --git a/src/gui/attributetable/qgsdualview.h b/src/gui/attributetable/qgsdualview.h
index 73cb420f44b..264adbacb29 100644
--- a/src/gui/attributetable/qgsdualview.h
+++ b/src/gui/attributetable/qgsdualview.h
@@ -327,7 +327,7 @@ class GUI_EXPORT QgsDualView : public QStackedWidget, private Ui::QgsDualViewBas
      * Will forward this signal to the feature list to visually represent
      * that there has been an edit event.
      */
-    void featureFormAttributeChanged();
+    void featureFormAttributeChanged( const QString &attribute, const QVariant &value, bool attributeChanged );
 
     /**
      * Will be called periodically, when loading layers from slow data providers.
diff --git a/src/gui/qgsattributeform.cpp b/src/gui/qgsattributeform.cpp
index df1e98282f6..7a707c6fd7d 100644
--- a/src/gui/qgsattributeform.cpp
+++ b/src/gui/qgsattributeform.cpp
@@ -240,6 +240,7 @@ void QgsAttributeForm::changeAttribute( const QString &field, const QVariant &va
 
 void QgsAttributeForm::setFeature( const QgsFeature &feature )
 {
+  mIsSettingFeature = true;
   mFeature = feature;
 
   switch ( mMode )
@@ -266,6 +267,7 @@ void QgsAttributeForm::setFeature( const QgsFeature &feature )
       break;
     }
   }
+  mIsSettingFeature = false;
 }
 
 bool QgsAttributeForm::saveEdits()
@@ -692,11 +694,7 @@ void QgsAttributeForm::onAttributeChanged( const QVariant &value )
   QgsEditorWidgetWrapper *eww = qobject_cast<QgsEditorWidgetWrapper *>( sender() );
   Q_ASSERT( eww );
 
-  const QVariant oldValue = mFeature.attribute( eww->fieldIdx() );
-
-  // Safety check, if we receive the same value again, no reason to do anything
-  if ( oldValue == value && oldValue.isNull() == value.isNull() )
-    return;
+  bool signalEmitted = false;
 
   if ( mValuesInitialized )
     mDirty = true;
@@ -707,7 +705,12 @@ void QgsAttributeForm::onAttributeChanged( const QVariant &value )
     case IdentifyMode:
     case AddFeatureMode:
     {
+      Q_NOWARN_DEPRECATED_PUSH
       emit attributeChanged( eww->field().name(), value );
+      Q_NOWARN_DEPRECATED_PUSH
+      emit widgetValueChanged( eww->field().name(), value, !mIsSettingFeature );
+
+      signalEmitted = true;
 
       updateJoinedFields( *eww );
 
@@ -739,7 +742,13 @@ void QgsAttributeForm::onAttributeChanged( const QVariant &value )
 
   updateConstraints( eww );
 
-  emit attributeChanged( eww->field().name(), value );
+  if ( !signalEmitted )
+  {
+    Q_NOWARN_DEPRECATED_PUSH
+    emit attributeChanged( eww->field().name(), value );
+    Q_NOWARN_DEPRECATED_PUSH
+    emit widgetValueChanged( eww->field().name(), value, !mIsSettingFeature );
+  }
 }
 
 void QgsAttributeForm::updateAllConstraints()
diff --git a/src/gui/qgsattributeform.h b/src/gui/qgsattributeform.h
index 6102825b286..41f16cd8614 100644
--- a/src/gui/qgsattributeform.h
+++ b/src/gui/qgsattributeform.h
@@ -178,8 +178,19 @@ class GUI_EXPORT QgsAttributeForm : public QWidget
      *
      * \param attribute The name of the attribute that changed.
      * \param value     The new value of the attribute.
+     * \deprecated since 3.0
      */
-    void attributeChanged( const QString &attribute, const QVariant &value );
+    Q_DECL_DEPRECATED void attributeChanged( const QString &attribute, const QVariant &value );
+
+    /**
+     * Notifies about changes of attributes
+     *
+     * \param attribute The name of the attribute that changed.
+     * \param value     The new value of the attribute.
+     * \param attributeChanged If true, it corresponds to an actual change of the feature attribute
+     * \since QGIS 3.0.1
+     */
+    void widgetValueChanged( const QString &attribute, const QVariant &value, bool attributeChanged );
 
     /**
      * Will be emitted before the feature is saved. Use this signal to perform sanity checks.
@@ -367,6 +378,7 @@ class GUI_EXPORT QgsAttributeForm : public QWidget
     QMap<const QgsVectorLayerJoinInfo *, QgsFeature> mJoinedFeatures;
     bool mValuesInitialized = false;
     bool mDirty = false;
+    bool mIsSettingFeature = false;
 
     struct ContainerInformation
     {
diff --git a/tests/src/gui/testqgsattributeform.cpp b/tests/src/gui/testqgsattributeform.cpp
index d4629cb0762..9b6d5c46444 100644
--- a/tests/src/gui/testqgsattributeform.cpp
+++ b/tests/src/gui/testqgsattributeform.cpp
@@ -111,24 +111,24 @@ void TestQgsAttributeForm::testFieldConstraint()
   // build a form for this feature
   QgsAttributeForm form2( layer );
   form2.setFeature( ft );
-  QSignalSpy spy( &form2, SIGNAL( attributeChanged( QString, QVariant ) ) );
+  QSignalSpy spy( &form2, SIGNAL( widgetValueChanged( QString, QVariant, bool ) ) );
   ww = qobject_cast<QgsEditorWidgetWrapper *>( form2.mWidgets[0] );
 
   // set value to 1
   ww->setValue( 1 );
-  QCOMPARE( spy.count(), 2 );
+  QCOMPARE( spy.count(), 1 );
   QCOMPARE( constraintsLabel( &form2, ww )->text(), validLabel );
 
   // set value to null
   spy.clear();
   ww->setValue( QVariant() );
-  QCOMPARE( spy.count(), 2 );
+  QCOMPARE( spy.count(), 1 );
   QCOMPARE( constraintsLabel( &form2, ww )->text(), invalidLabel );
 
   // set value to 1
   spy.clear();
   ww->setValue( 1 );
-  QCOMPARE( spy.count(), 2 );
+  QCOMPARE( spy.count(), 1 );
   QCOMPARE( constraintsLabel( &form2, ww )->text(), validLabel );
 
   // set a soft constraint
@@ -205,11 +205,11 @@ void TestQgsAttributeForm::testFieldMultiConstraints()
   ww1 = qobject_cast<QgsEditorWidgetWrapper *>( form2.mWidgets[1] );
   ww2 = qobject_cast<QgsEditorWidgetWrapper *>( form2.mWidgets[2] );
   ww3 = qobject_cast<QgsEditorWidgetWrapper *>( form2.mWidgets[3] );
-  QSignalSpy spy2( &form2, SIGNAL( attributeChanged( QString, QVariant ) ) );
+  QSignalSpy spy2( &form2, SIGNAL( widgetValueChanged( QString, QVariant, bool ) ) );
 
   // change value
   ww0->setValue( 2 ); // update col0
-  QCOMPARE( spy2.count(), 2 );
+  QCOMPARE( spy2.count(), 1 );
 
   QCOMPARE( constraintsLabel( &form2, ww0 )->text(), inv ); // 2 < ( 1 + 2 )
   QCOMPARE( constraintsLabel( &form2, ww1 )->text(), QString() );
@@ -219,7 +219,7 @@ void TestQgsAttributeForm::testFieldMultiConstraints()
   // change value
   spy2.clear();
   ww0->setValue( 1 ); // update col0
-  QCOMPARE( spy2.count(), 2 );
+  QCOMPARE( spy2.count(), 1 );
 
   QCOMPARE( constraintsLabel( &form2, ww0 )->text(), val ); // 1 < ( 1 + 2 )
   QCOMPARE( constraintsLabel( &form2, ww1 )->text(), QString() );
