[bugfix][attrtable] Convert comma to dot for floating point input

This fixes an unreported bug that without detecting an invalid input when using a comma as a decimal separator silently converts the entered value to NULL. Since locale support in QGIS is in its early stages we convert commas to dots within the validator, this is common practice in almost all web applications where you can enter a comma instead of a dot and the conversion appears while you digit. This comes with brand new tests for QgsFieldValidator. Bonus: small fix in sipify.
qgis · Jan 31, 2018 · cc1625c · cc1625c
1 parent cbd0ece
commit cc1625c
Show file tree

Hide file tree

Showing 6 changed files with 114 additions and 3 deletions.
diff --git a/python/gui/qgsfieldvalidator.sip.in b/python/gui/qgsfieldvalidator.sip.in
@@ -19,7 +19,7 @@ class QgsFieldValidator : QValidator
     QgsFieldValidator( QObject *parent, const QgsField &field, const QString &defaultValue, const QString &dateFormat = "yyyy-MM-dd" );
     ~QgsFieldValidator();
 
-    virtual State validate( QString &s /Constrained/, int &i /In,Out/ ) const;
+    virtual State validate( QString &s /Constrained,In,Out/, int &i /In,Out/ ) const;
 
     virtual void fixup( QString &s /Constrained/ ) const;
 

diff --git a/src/gui/qgsfieldvalidator.cpp b/src/gui/qgsfieldvalidator.cpp
@@ -84,7 +84,6 @@ QgsFieldValidator::QgsFieldValidator( QObject *parent, const QgsField &field, co
       mValidator = nullptr;
   }
 
-  QgsSettings settings;
   mNullValue = QgsApplication::nullRepresentation();
 }
 
@@ -113,6 +112,12 @@ QValidator::State QgsFieldValidator::validate( QString &s, int &i ) const
   // delegate to the child validator if any
   if ( mValidator )
   {
+    // force to use the '.' as a decimal point or in case we are using QDoubleValidator
+    // we can get a valid number with a comma depending on current locale
+    // ... but this will fail subsequently when converted from string to double and
+    // becomes a NULL!
+    if ( mField.type() == QVariant::Double )
+      s = s.replace( ',', '.' );
     QValidator::State result = mValidator->validate( s, i );
     return result;
   }

diff --git a/src/gui/qgsfieldvalidator.h b/src/gui/qgsfieldvalidator.h
@@ -38,7 +38,7 @@ class GUI_EXPORT QgsFieldValidator : public QValidator
     QgsFieldValidator( QObject *parent, const QgsField &field, const QString &defaultValue, const QString &dateFormat = "yyyy-MM-dd" );
     ~QgsFieldValidator() override;
 
-    State validate( QString &s SIP_CONSTRAINED, int &i SIP_INOUT ) const override;
+    State validate( QString &s SIP_CONSTRAINED SIP_INOUT, int &i SIP_INOUT ) const override;
     void fixup( QString &s SIP_CONSTRAINED ) const override;
 
     QString dateFormat() const { return mDateFormat; }

diff --git a/tests/src/python/CMakeLists.txt b/tests/src/python/CMakeLists.txt
@@ -196,6 +196,7 @@ ADD_PYTHON_TEST(PyQgsAuthManagerProxy test_authmanager_proxy.py)
 ADD_PYTHON_TEST(PyQgsAuthSettingsWidget test_authsettingswidget.py)
 ADD_PYTHON_TEST(PyQgsAuxiliaryStorage test_qgsauxiliarystorage.py)
 ADD_PYTHON_TEST(PyQgsAuthManagerOgr test_authmanager_ogr.py)
+ADD_PYTHON_TEST(PyQgsFieldValidator test_qgsfieldvalidator.py)
 
 IF (NOT WIN32)
   ADD_PYTHON_TEST(PyQgsLogger test_qgslogger.py)

diff --git a/tests/src/python/test_qgsfieldvalidator.py b/tests/src/python/test_qgsfieldvalidator.py
@@ -0,0 +1,105 @@
+# -*- coding: utf-8 -*-
+"""QGIS Unit tests for QgsFieldValidator.
+
+.. note:: 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.
+"""
+__author__ = 'Alessandro Pasotti'
+__date__ = '31/01/2018'
+__copyright__ = 'Copyright 2018, The QGIS Project'
+# This will get replaced with a git SHA1 when you do a git archive
+__revision__ = '$Format:%H$'
+
+import qgis  # NOQA
+
+from qgis.PyQt.QtCore import QVariant
+from qgis.PyQt.QtGui import QValidator
+from qgis.core import QgsVectorLayer
+from qgis.gui import QgsFieldValidator
+from qgis.testing import start_app, unittest
+from utilities import unitTestDataPath
+
+TEST_DATA_DIR = unitTestDataPath()
+
+
+start_app()
+
+
+class TestQgsFieldValidator(unittest.TestCase):
+
+    def setUp(self):
+        """Run before each test."""
+        testPath = TEST_DATA_DIR + '/' + 'bug_17878.gpkg|layername=bug_17878'
+        self.vl = QgsVectorLayer(testPath, "test_data", "ogr")
+        assert self.vl.isValid()
+
+    def tearDown(self):
+        """Run after each test."""
+        pass
+
+    def test_validator(self):
+        # Test the double
+        """ 
+        Expected results from validate
+        QValidator::Invalid 0 The string is clearly invalid.
+        QValidator::Intermediate 1 The string is a plausible intermediate value.
+        QValidator::Acceptable 2 The string is acceptable as a final result; i.e. it is valid.
+        """
+
+        double_field = self.vl.fields()[self.vl.fields().indexFromName('double_field')]
+        self.assertEqual(double_field.precision(), 0) # this is what the provider reports :(
+        self.assertEqual(double_field.length(), 0) # not set
+        self.assertEqual(double_field.type(), QVariant.Double)
+
+        validator = QgsFieldValidator(None, double_field, '0.0', '')
+
+        def _test(value, expected):
+            ret = validator.validate(value, 0)
+            self.assertEqual(ret[0], expected, value)
+            if value:
+                self.assertEqual(validator.validate('-' + value, 0)[0], expected, '-' + value)
+            # Check the decimal comma separator has been properly transformed
+            if expected != QValidator.Invalid:
+                self.assertEqual(ret[1], value.replace(',', '.'))
+
+        # Valid
+        _test('0.1234', QValidator.Acceptable)
+        _test('0,1234', QValidator.Acceptable)
+        _test('12345.1234e+123', QValidator.Acceptable)
+        _test('12345.1234e-123', QValidator.Acceptable)
+        _test('12345,1234e+123', QValidator.Acceptable)
+        _test('12345,1234e-123', QValidator.Acceptable)
+        _test('', QValidator.Acceptable)
+
+        # Out of range
+        _test('12345.1234e+823', QValidator.Intermediate)
+        _test('12345.1234e-823', QValidator.Intermediate)
+        _test('12345,1234e+823', QValidator.Intermediate)
+        _test('12345,1234e-823', QValidator.Intermediate)
+
+        # Invalid
+        _test('12345-1234', QValidator.Invalid)
+        _test('onetwothree', QValidator.Invalid)
+
+        int_field = self.vl.fields()[self.vl.fields().indexFromName('int_field')]
+        self.assertEqual(int_field.precision(), 0) # this is what the provider reports :(
+        self.assertEqual(int_field.length(), 0) # not set
+        self.assertEqual(int_field.type(), QVariant.Int)
+
+        validator = QgsFieldValidator(None, int_field, '0', '')
+
+        # Valid
+        _test('0', QValidator.Acceptable)
+        _test('1234', QValidator.Acceptable)
+        _test('', QValidator.Acceptable)
+
+        # Invalid
+        _test('12345-1234', QValidator.Invalid)
+        _test('12345.1234', QValidator.Invalid)
+        _test('onetwothree', QValidator.Invalid)
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/tests/testdata/bug_17878.gpkg b/tests/testdata/bug_17878.gpkg