Skip to content

Commit

Permalink
Add null handling to value map edit widget (fixes #15215) (#3274)
Browse files Browse the repository at this point in the history
* Add null handling to value map edit widget (fixes #15215)

* Return QVariant type

* Use hardcoded value for 'null' representation

* Detect "null" value when loading value map from csv; use null QString constructor

* Use configured "null" representation for display in value map

* Use single definition for value map null representation guid

* Added unit test for value map widget and fixed value displaying bug

(cherry picked from commit d2c9863)
  • Loading branch information
dgoedkoop authored and wonder-sk committed Apr 16, 2018
1 parent 36749c1 commit 73f7bd3
Show file tree
Hide file tree
Showing 6 changed files with 155 additions and 43 deletions.
67 changes: 46 additions & 21 deletions src/gui/editorwidgets/qgsvaluemapconfigdlg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ QgsValueMapConfigDlg::QgsValueMapConfigDlg( QgsVectorLayer* vl, int fieldIdx, QW

tableWidget->insertRow( 0 );

connect( addNullButton, SIGNAL( clicked() ), this, SLOT( addNullButtonPushed() ) );
connect( removeSelectedButton, SIGNAL( clicked() ), this, SLOT( removeSelectedButtonPushed() ) );
connect( loadFromLayerButton, SIGNAL( clicked() ), this, SLOT( loadFromLayerButtonPushed() ) );
connect( loadFromCSVButton, SIGNAL( clicked() ), this, SLOT( loadFromCSVButtonPushed() ) );
Expand All @@ -38,6 +39,7 @@ QgsValueMapConfigDlg::QgsValueMapConfigDlg( QgsVectorLayer* vl, int fieldIdx, QW
QgsEditorWidgetConfig QgsValueMapConfigDlg::config()
{
QgsEditorWidgetConfig cfg;
QSettings settings;

//store data to map
for ( int i = 0; i < tableWidget->rowCount() - 1; i++ )
Expand All @@ -48,13 +50,17 @@ QgsEditorWidgetConfig QgsValueMapConfigDlg::config()
if ( !ki )
continue;

QString ks = ki->text();
if (( ks == settings.value( "qgis/nullValue", "NULL" ).toString() ) && !( ki->flags() & Qt::ItemIsEditable ) )
ks = VALUEMAP_NULL_TEXT;

if ( !vi || vi->text().isNull() )
{
cfg.insert( ki->text(), ki->text() );
cfg.insert( ks, ks );
}
else
{
cfg.insert( vi->text(), ki->text() );
cfg.insert( vi->text(), ks );
}
}

Expand All @@ -72,16 +78,10 @@ void QgsValueMapConfigDlg::setConfig( const QgsEditorWidgetConfig& config )
int row = 0;
for ( QgsEditorWidgetConfig::ConstIterator mit = config.begin(); mit != config.end(); mit++, row++ )
{
tableWidget->insertRow( row );
if ( mit.value().isNull() )
{
tableWidget->setItem( row, 0, new QTableWidgetItem( mit.key() ) );
}
setRow( row, mit.key(), QString() );
else
{
tableWidget->setItem( row, 0, new QTableWidgetItem( mit.value().toString() ) );
tableWidget->setItem( row, 1, new QTableWidgetItem( mit.key() ) );
}
setRow( row, mit.value().toString(), mit.key() );
}
}

Expand Down Expand Up @@ -132,27 +132,47 @@ void QgsValueMapConfigDlg::updateMap( const QMap<QString, QVariant> &map, bool i

if ( insertNull )
{
QSettings settings;
tableWidget->setItem( row, 0, new QTableWidgetItem( settings.value( "qgis/nullValue", "NULL" ).toString() ) );
tableWidget->setItem( row, 1, new QTableWidgetItem( "<NULL>" ) );
setRow( row, VALUEMAP_NULL_TEXT, "<NULL>" );
++row;
}

for ( QMap<QString, QVariant>::const_iterator mit = map.begin(); mit != map.end(); ++mit, ++row )
{
tableWidget->insertRow( row );
if ( mit.value().isNull() )
{
tableWidget->setItem( row, 0, new QTableWidgetItem( mit.key() ) );
}
setRow( row, mit.key(), QString() );
else
{
tableWidget->setItem( row, 0, new QTableWidgetItem( mit.key() ) );
tableWidget->setItem( row, 1, new QTableWidgetItem( mit.value().toString() ) );
}
setRow( row, mit.key(), mit.value().toString() );
}
}

void QgsValueMapConfigDlg::setRow( int row, const QString value, const QString description )
{
QSettings settings;
QTableWidgetItem* valueCell;
QTableWidgetItem* descriptionCell = new QTableWidgetItem( description );
tableWidget->insertRow( row );
if ( value == QString( VALUEMAP_NULL_TEXT ) )
{
QFont cellFont;
cellFont.setItalic( true );
valueCell = new QTableWidgetItem( settings.value( "qgis/nullValue", "NULL" ).toString() );
valueCell->setFont( cellFont );
valueCell->setFlags( Qt::ItemIsSelectable | Qt::ItemIsEnabled );
descriptionCell->setFont( cellFont );
}
else
{
valueCell = new QTableWidgetItem( value );
}
tableWidget->setItem( row, 0, valueCell );
tableWidget->setItem( row, 1, descriptionCell );
}

void QgsValueMapConfigDlg::addNullButtonPushed()
{
setRow( tableWidget->rowCount() - 1, VALUEMAP_NULL_TEXT, "<NULL>" );
}

void QgsValueMapConfigDlg::loadFromLayerButtonPushed()
{
QgsAttributeTypeLoadDialog layerDialog( layer() );
Expand All @@ -164,6 +184,8 @@ void QgsValueMapConfigDlg::loadFromLayerButtonPushed()

void QgsValueMapConfigDlg::loadFromCSVButtonPushed()
{
QSettings settings;

QString fileName = QFileDialog::getOpenFileName( nullptr, tr( "Select a file" ), QDir::homePath() );
if ( fileName.isNull() )
return;
Expand Down Expand Up @@ -220,6 +242,9 @@ void QgsValueMapConfigDlg::loadFromCSVButtonPushed()
val = val.mid( 1, val.length() - 2 );
}

if ( key == settings.value( "qgis/nullValue", "NULL" ).toString() )
key = QString( VALUEMAP_NULL_TEXT );

map[ key ] = val;
}

Expand Down
6 changes: 6 additions & 0 deletions src/gui/editorwidgets/qgsvaluemapconfigdlg.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

#include "qgseditorconfigwidget.h"

#define VALUEMAP_NULL_TEXT "{2839923C-8B7D-419E-B84B-CA2FE9B80EC7}"

/** \ingroup gui
* \class QgsValueMapConfigDlg
* \note not available in Python bindings
Expand All @@ -36,8 +38,12 @@ class GUI_EXPORT QgsValueMapConfigDlg : public QgsEditorConfigWidget, private Ui

void updateMap( const QMap<QString, QVariant> &map, bool insertNull );

private:
void setRow( int row, const QString value, const QString description );

private slots:
void vCellChanged( int row, int column );
void addNullButtonPushed();
void removeSelectedButtonPushed();
void loadFromLayerButtonPushed();
void loadFromCSVButtonPushed();
Expand Down
14 changes: 11 additions & 3 deletions src/gui/editorwidgets/qgsvaluemapwidgetfactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#include "qgsdefaultsearchwidgetwrapper.h"
#include "qgsvaluemapconfigdlg.h"

#include <QSettings>

QgsValueMapWidgetFactory::QgsValueMapWidgetFactory( const QString& name )
: QgsEditorWidgetFactory( name )
{
Expand Down Expand Up @@ -82,11 +84,17 @@ void QgsValueMapWidgetFactory::writeConfig( const QgsEditorWidgetConfig& config,

QString QgsValueMapWidgetFactory::representValue( QgsVectorLayer* vl, int fieldIdx, const QgsEditorWidgetConfig& config, const QVariant& cache, const QVariant& value ) const
{
Q_UNUSED( vl )
Q_UNUSED( fieldIdx )
Q_UNUSED( cache )

return config.key( value, QVariant( QString( "(%1)" ).arg( value.toString() ) ).toString() );
QString valueInternalText;
QString valueDisplayText;
QSettings settings;
if ( value.isNull() )
valueInternalText = QString( VALUEMAP_NULL_TEXT );
else
valueInternalText = value.toString();

return config.key( valueInternalText, QVariant( QString( "(%1)" ).arg( vl->fields().at( fieldIdx ).displayString( value ) ) ).toString() );
}

QVariant QgsValueMapWidgetFactory::sortValue( QgsVectorLayer* vl, int fieldIdx, const QgsEditorWidgetConfig& config, const QVariant& cache, const QVariant& value ) const
Expand Down
14 changes: 13 additions & 1 deletion src/gui/editorwidgets/qgsvaluemapwidgetwrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
***************************************************************************/

#include "qgsvaluemapwidgetwrapper.h"
#include "qgsvaluemapconfigdlg.h"

#include <QSettings>

QgsValueMapWidgetWrapper::QgsValueMapWidgetWrapper( QgsVectorLayer* vl, int fieldIdx, QWidget* editor, QWidget* parent )
: QgsEditorWidgetWrapper( vl, fieldIdx, editor, parent )
Expand All @@ -29,6 +32,9 @@ QVariant QgsValueMapWidgetWrapper::value() const
if ( mComboBox )
v = mComboBox->itemData( mComboBox->currentIndex() );

if ( v == QString( VALUEMAP_NULL_TEXT ) )
v = QVariant( field().type() );

return v;
}

Expand Down Expand Up @@ -70,6 +76,12 @@ bool QgsValueMapWidgetWrapper::valid() const

void QgsValueMapWidgetWrapper::setValue( const QVariant& value )
{
QString v;
if ( value.isNull() )
v = QString( VALUEMAP_NULL_TEXT );
else
v = value.toString();

if ( mComboBox )
mComboBox->setCurrentIndex( mComboBox->findData( value ) );
mComboBox->setCurrentIndex( mComboBox->findData( v ) );
}
43 changes: 25 additions & 18 deletions src/ui/editorwidgets/qgsvaluemapconfigdlgbase.ui
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<string>Form</string>
</property>
<layout class="QGridLayout" name="gridLayout">
<item row="0" column="0" colspan="3">
<item row="0" column="0" colspan="5">
<widget class="QLabel" name="valueMapLabel">
<property name="text">
<string>Combo box with predefined items. Value is stored in the attribute, description is shown in the combo box.</string>
Expand All @@ -31,14 +31,7 @@
</property>
</widget>
</item>
<item row="1" column="1">
<widget class="QPushButton" name="loadFromCSVButton">
<property name="text">
<string>Load Data from CSV File</string>
</property>
</widget>
</item>
<item row="1" column="2">
<item row="1" column="4">
<spacer name="horizontalSpacer">
<property name="orientation">
<enum>Qt::Horizontal</enum>
Expand All @@ -51,7 +44,7 @@
</property>
</spacer>
</item>
<item row="2" column="0" colspan="3">
<item row="2" column="0" colspan="5">
<widget class="QTableWidget" name="tableWidget">
<column>
<property name="text">
Expand All @@ -65,14 +58,7 @@
</column>
</widget>
</item>
<item row="3" column="0">
<widget class="QPushButton" name="removeSelectedButton">
<property name="text">
<string>Remove Selected</string>
</property>
</widget>
</item>
<item row="3" column="1" colspan="2">
<item row="3" column="3" colspan="2">
<spacer name="horizontalSpacer_2">
<property name="orientation">
<enum>Qt::Horizontal</enum>
Expand All @@ -85,6 +71,27 @@
</property>
</spacer>
</item>
<item row="3" column="0">
<widget class="QPushButton" name="addNullButton">
<property name="text">
<string>Add &quot;NULL&quot; value</string>
</property>
</widget>
</item>
<item row="3" column="1">
<widget class="QPushButton" name="removeSelectedButton">
<property name="text">
<string>Remove Selected</string>
</property>
</widget>
</item>
<item row="1" column="1">
<widget class="QPushButton" name="loadFromCSVButton">
<property name="text">
<string>Load Data from CSV File</string>
</property>
</widget>
</item>
</layout>
</widget>
<resources/>
Expand Down
54 changes: 54 additions & 0 deletions tests/src/python/test_qgseditwidgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@

class TestQgsTextEditWidget(unittest.TestCase):

VALUEMAP_NULL_TEXT = "{2839923C-8B7D-419E-B84B-CA2FE9B80EC7}"

@classmethod
def setUpClass(cls):
QgsEditorWidgetRegistry.initEditors()
Expand Down Expand Up @@ -93,6 +95,57 @@ def testStringWithMaxLen(self):

QgsMapLayerRegistry.instance().removeAllMapLayers()

def test_ValueMap_representValue(self):
layer = QgsVectorLayer("none?field=number1:integer&field=number2:double&field=text1:string&field=number3:integer&field=number4:double&field=text2:string",
"layer", "memory")
assert layer.isValid()
QgsMapLayerRegistry.instance().addMapLayer(layer)
f = QgsFeature()
f.setAttributes([2, 2.5, 'NULL', None, None, None])
assert layer.dataProvider().addFeatures([f])
reg = QgsEditorWidgetRegistry.instance()
factory = reg.factory("ValueMap")
self.assertIsNotNone(factory)

# Tests with different value types occuring in the value map
config = {'two': '2', 'twoandhalf': '2.5', 'NULL text': 'NULL',
'nothing': self.VALUEMAP_NULL_TEXT}
self.assertEqual(factory.representValue(layer, 0, config, None, 2), 'two')
self.assertEqual(factory.representValue(layer, 1, config, None, 2.5), 'twoandhalf')
self.assertEqual(factory.representValue(layer, 2, config, None, 'NULL'), 'NULL text')
# Tests with null values of different types, if value map contains null
self.assertEqual(factory.representValue(layer, 3, config, None, None), 'nothing')
self.assertEqual(factory.representValue(layer, 4, config, None, None), 'nothing')
self.assertEqual(factory.representValue(layer, 5, config, None, None), 'nothing')
# Tests with fallback display for different value types
config = {}
self.assertEqual(factory.representValue(layer, 0, config, None, 2), '(2)')
self.assertEqual(factory.representValue(layer, 1, config, None, 2.5), '(2.50000)')
self.assertEqual(factory.representValue(layer, 2, config, None, 'NULL'), '(NULL)')
# Tests with fallback display for null in different types of fields
self.assertEqual(factory.representValue(layer, 3, config, None, None), '(NULL)')
self.assertEqual(factory.representValue(layer, 4, config, None, None), '(NULL)')
self.assertEqual(factory.representValue(layer, 5, config, None, None), '(NULL)')

QgsMapLayerRegistry.instance().removeAllMapLayers()

def test_ValueMap_set_get(self):
layer = QgsVectorLayer("none?field=number:integer", "layer", "memory")
assert layer.isValid()
QgsMapLayerRegistry.instance().addMapLayer(layer)
reg = QgsEditorWidgetRegistry.instance()
configWdg = reg.createConfigWidget('ValueMap', layer, 0, None)

config = {'two': '2', 'twoandhalf': '2.5', 'NULL text': 'NULL',
'nothing': self.VALUEMAP_NULL_TEXT}

# Set a configuration containing values and NULL and check if it
# is returned intact.
configWdg.setConfig(config)
self.assertEqual(configWdg.config(), config)

QgsMapLayerRegistry.instance().removeAllMapLayers()

def test_ValueRelation_representValue(self):

first_layer = QgsVectorLayer("none?field=foreign_key:integer",
Expand Down Expand Up @@ -227,5 +280,6 @@ def test_RelationReference_representValue(self):

QgsMapLayerRegistry.instance().removeAllMapLayers()


if __name__ == '__main__':
unittest.main()

0 comments on commit 73f7bd3

Please sign in to comment.