Skip to content

Commit a6319a4

Browse files
nyalldawsonnyalldawson
committedNov 2, 2016
If the layer is NOT being edited then only check layer based constraints
and not any constraints enforced by the provider Because: 1. we want to keep browsing features nice and responsive. It's nice to give feedback as to whether the value checks out, but not if it's too slow to do so. Some constraints (eg unique) can be expensive to test. A user can freely remove a layer-based constraint if it proves to be too slow to test, but they are unlikely to have any control over provider-side constraints 2. the provider has already accepted the value, so presumably it doesn't violate the constraint and there's no point rechecking!
·
ltr-3_40final-3_0_0
1 parent c98d380 commit a6319a4

File tree

7 files changed

+47
-37
lines changed

7 files changed

+47
-37
lines changed
 

‎python/core/qgsvectorlayerutils.sip

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ class QgsVectorLayerUtils
2222
/**
2323
* Tests an attribute value to check whether it passes all constraints which are present on the corresponding field.
2424
* Returns true if the attribute value is valid for the field. Any constraint failures will be reported in the errors argument.
25+
* If the origin parameter is set then only constraints with a matching origin will be checked.
2526
*/
26-
static bool validateAttribute( const QgsVectorLayer* layer, const QgsFeature& feature, int attributeIndex, QStringList& errors /Out/ );
27+
static bool validateAttribute( const QgsVectorLayer* layer, const QgsFeature& feature, int attributeIndex, QStringList& errors /Out/,
28+
QgsField::ConstraintOrigin origin = QgsField::ConstraintOriginNotSet );
2729

2830
};

‎src/core/qgsvectorlayerutils.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ bool QgsVectorLayerUtils::valueExists( const QgsVectorLayer* layer, int fieldInd
5050
return false;
5151
}
5252

53-
bool QgsVectorLayerUtils::validateAttribute( const QgsVectorLayer* layer, const QgsFeature& feature, int attributeIndex, QStringList& errors )
53+
bool QgsVectorLayerUtils::validateAttribute( const QgsVectorLayer* layer, const QgsFeature& feature, int attributeIndex, QStringList& errors, QgsField::ConstraintOrigin origin )
5454
{
5555
if ( !layer )
5656
return false;
@@ -63,7 +63,8 @@ bool QgsVectorLayerUtils::validateAttribute( const QgsVectorLayer* layer, const
6363
bool valid = true;
6464
errors.clear();
6565

66-
if ( field.constraints() & QgsField::ConstraintExpression && !field.constraintExpression().isEmpty() )
66+
if ( field.constraints() & QgsField::ConstraintExpression && !field.constraintExpression().isEmpty()
67+
&& ( origin == QgsField::ConstraintOriginNotSet || origin == field.constraintOrigin( QgsField::ConstraintExpression ) ) )
6768
{
6869
QgsExpressionContext context = layer->createExpressionContext();
6970
context.setFeature( feature );
@@ -86,7 +87,8 @@ bool QgsVectorLayerUtils::validateAttribute( const QgsVectorLayer* layer, const
8687
}
8788
}
8889

89-
if ( field.constraints() & QgsField::ConstraintNotNull )
90+
if ( field.constraints() & QgsField::ConstraintNotNull
91+
&& ( origin == QgsField::ConstraintOriginNotSet || origin == field.constraintOrigin( QgsField::ConstraintNotNull ) ) )
9092
{
9193
valid = valid && !value.isNull();
9294

@@ -96,7 +98,8 @@ bool QgsVectorLayerUtils::validateAttribute( const QgsVectorLayer* layer, const
9698
}
9799
}
98100

99-
if ( field.constraints() & QgsField::ConstraintUnique )
101+
if ( field.constraints() & QgsField::ConstraintUnique
102+
&& ( origin == QgsField::ConstraintOriginNotSet || origin == field.constraintOrigin( QgsField::ConstraintUnique ) ) )
100103
{
101104
bool alreadyExists = QgsVectorLayerUtils::valueExists( layer, attributeIndex, value, QgsFeatureIds() << feature.id() );
102105
valid = valid && !alreadyExists;

‎src/core/qgsvectorlayerutils.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,10 @@ class CORE_EXPORT QgsVectorLayerUtils
3939
/**
4040
* Tests an attribute value to check whether it passes all constraints which are present on the corresponding field.
4141
* Returns true if the attribute value is valid for the field. Any constraint failures will be reported in the errors argument.
42+
* If the origin parameter is set then only constraints with a matching origin will be checked.
4243
*/
43-
static bool validateAttribute( const QgsVectorLayer* layer, const QgsFeature& feature, int attributeIndex, QStringList& errors );
44+
static bool validateAttribute( const QgsVectorLayer* layer, const QgsFeature& feature, int attributeIndex, QStringList& errors,
45+
QgsField::ConstraintOrigin origin = QgsField::ConstraintOriginNotSet );
4446

4547
};
4648

‎src/gui/editorwidgets/core/qgseditorwidgetwrapper.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ void QgsEditorWidgetWrapper::updateConstraintWidgetStatus( bool constraintValid
106106
widget()->setStyleSheet( QStringLiteral( "background-color: #dd7777;" ) );
107107
}
108108

109-
void QgsEditorWidgetWrapper::updateConstraint( const QgsFeature &ft )
109+
void QgsEditorWidgetWrapper::updateConstraint( const QgsFeature &ft, QgsField::ConstraintOrigin constraintOrigin )
110110
{
111111
bool toEmit( false );
112112
QgsField field = layer()->fields().at( mFieldIdx );
@@ -150,7 +150,7 @@ void QgsEditorWidgetWrapper::updateConstraint( const QgsFeature &ft )
150150
}
151151

152152
QStringList errors;
153-
mValidConstraint = QgsVectorLayerUtils::validateAttribute( layer(), ft, mFieldIdx, errors );
153+
mValidConstraint = QgsVectorLayerUtils::validateAttribute( layer(), ft, mFieldIdx, errors, constraintOrigin );
154154
mConstraintFailureReason = errors.join( ", " );
155155

156156
if ( toEmit )

‎src/gui/editorwidgets/core/qgseditorwidgetwrapper.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,11 @@ class GUI_EXPORT QgsEditorWidgetWrapper : public QgsWidgetWrapper
118118
/**
119119
* Update constraint.
120120
* @param featureContext the feature to use to evaluate the constraint
121+
* @param constraintOrigin optional origin for constraints to check. This can be used to limit the constraints tested
122+
* to only provider or layer based constraints.
121123
* @note added in QGIS 2.16
122124
*/
123-
void updateConstraint( const QgsFeature &featureContext );
125+
void updateConstraint( const QgsFeature &featureContext, QgsField::ConstraintOrigin constraintOrigin = QgsField::ConstraintOriginNotSet );
124126

125127
/**
126128
* Get the current constraint status.

‎src/gui/qgsattributeform.cpp

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -674,30 +674,6 @@ void QgsAttributeForm::onAttributeChanged( const QVariant& value )
674674
break;
675675
}
676676

677-
if ( eww->layer()->fields().at( eww->fieldIdx() ).constraints() & QgsField::ConstraintNotNull )
678-
{
679-
QLabel* buddy = mBuddyMap.value( eww->widget() );
680-
681-
if ( buddy )
682-
{
683-
if ( !buddy->property( "originalText" ).isValid() )
684-
buddy->setProperty( "originalText", buddy->text() );
685-
686-
QString text = buddy->property( "originalText" ).toString();
687-
688-
if ( value.isNull() )
689-
{
690-
// not good
691-
buddy->setText( QStringLiteral( "%1<font color=\"red\">❌</font>" ).arg( text ) );
692-
}
693-
else
694-
{
695-
// good
696-
buddy->setText( QStringLiteral( "%1<font color=\"green\">✔</font>" ).arg( text ) );
697-
}
698-
}
699-
}
700-
701677
updateConstraints( eww );
702678

703679
// emit
@@ -720,14 +696,25 @@ void QgsAttributeForm::updateConstraints( QgsEditorWidgetWrapper *eww )
720696
QgsFeature ft;
721697
if ( currentFormFeature( ft ) )
722698
{
699+
// if the layer is NOT being edited then we only check layer based constraints, and not
700+
// any constraints enforced by the provider. Because:
701+
// 1. we want to keep browsing features nice and responsive. It's nice to give feedback as to whether
702+
// the value checks out, but not if it's too slow to do so. Some constraints (eg unique) can be
703+
// expensive to test. A user can freely remove a layer-based constraint if it proves to be too slow
704+
// to test, but they are unlikely to have any control over provider-side constraints
705+
// 2. the provider has already accepted the value, so presumably it doesn't violate the constraint
706+
// and there's no point rechecking!
707+
QgsField::ConstraintOrigin constraintOrigin = mLayer->isEditable() ? QgsField::ConstraintOriginNotSet
708+
: QgsField::ConstraintOriginLayer;
709+
723710
// update eww constraint
724-
eww->updateConstraint( ft );
711+
eww->updateConstraint( ft, constraintOrigin );
725712

726713
// update eww dependencies constraint
727714
QList<QgsEditorWidgetWrapper*> deps = constraintDependencies( eww );
728715

729716
Q_FOREACH ( QgsEditorWidgetWrapper* depsEww, deps )
730-
depsEww->updateConstraint( ft );
717+
depsEww->updateConstraint( ft, constraintOrigin );
731718

732719
// sync ok button status
733720
synchronizeEnabledState();
@@ -914,12 +901,12 @@ void QgsAttributeForm::onConstraintStatusChanged( const QString& constraint,
914901
if ( !ok )
915902
{
916903
// not good
917-
buddy->setText( QStringLiteral( "%1<font color=\"red\">*</font>" ).arg( text ) );
904+
buddy->setText( QStringLiteral( "%1<font color=\"red\">❌</font>" ).arg( text ) );
918905
}
919906
else
920907
{
921908
// good
922-
buddy->setText( QStringLiteral( "%1<font color=\"green\">*</font>" ).arg( text ) );
909+
buddy->setText( QStringLiteral( "%1<font color=\"green\">✔</font>" ).arg( text ) );
923910
}
924911
}
925912
}

‎tests/src/python/test_qgsvectorlayerutils.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ def test_validate_attribute(self):
101101
self.assertFalse(res)
102102
self.assertEqual(len(errors), 1)
103103
print(errors)
104+
# checking only for provider constraints
105+
res, errors = QgsVectorLayerUtils.validateAttribute(layer, f, 1, QgsField.ConstraintOriginProvider)
106+
self.assertTrue(res)
107+
self.assertEqual(len(errors), 0)
104108

105109
# bad field expression check
106110
layer.setConstraintExpression(1, 'fldint>')
@@ -123,6 +127,11 @@ def test_validate_attribute(self):
123127
self.assertEqual(len(errors), 1)
124128
print(errors)
125129

130+
# checking only for provider constraints
131+
res, errors = QgsVectorLayerUtils.validateAttribute(layer, f, 1, QgsField.ConstraintOriginProvider)
132+
self.assertTrue(res)
133+
self.assertEqual(len(errors), 0)
134+
126135
# unique constraint
127136
f.setAttributes(["test123", 123])
128137
layer.setFieldConstraints(1, QgsField.Constraints())
@@ -135,6 +144,11 @@ def test_validate_attribute(self):
135144
self.assertEqual(len(errors), 1)
136145
print(errors)
137146

147+
# checking only for provider constraints
148+
res, errors = QgsVectorLayerUtils.validateAttribute(layer, f, 1, QgsField.ConstraintOriginProvider)
149+
self.assertTrue(res)
150+
self.assertEqual(len(errors), 0)
151+
138152
# check - same id should be ignored when testing for uniqueness
139153
f1 = QgsFeature(1)
140154
f1.setAttributes(["test123", 123])

0 commit comments

Comments
 (0)
Please sign in to comment.