Fix value map widget broken when used as search widget

Fixes #17986

src/gui/editorwidgets/qgsvaluemapconfigdlg.cpp

@@ -167,6 +167,35 @@ void QgsValueMapConfigDlg::updateMap( const QMap<QString, QVariant> &map, bool i
   }
 }
 
+void QgsValueMapConfigDlg::populateComboBox( QComboBox *comboBox, const QVariantMap &config, bool skipNull )
+{
+  const QList<QVariant> valueList = config.value( QStringLiteral( "map" ) ).toList();
+
+  if ( !valueList.empty() )
+  {
+    for ( const QVariant &value : valueList )
+    {
+      const QVariantMap valueMap = value.toMap();
+
+      if ( skipNull && valueMap.constBegin().value() == QgsValueMapFieldFormatter::NULL_VALUE )
+        continue;
+
+      comboBox->addItem( valueMap.constBegin().key(), valueMap.constBegin().value() );
+    }
+  }
+  else
+  {
+    const QVariantMap map = config.value( QStringLiteral( "map" ) ).toMap();
+    for ( auto it = map.constBegin(); it != map.constEnd(); ++it )
+    {
+      if ( skipNull && it.value() == QgsValueMapFieldFormatter::NULL_VALUE )
+        continue;
+
+      comboBox->addItem( it.key(), it.value() );
+    }
+  }
+}
+
 void QgsValueMapConfigDlg::setRow( int row, const QString &value, const QString &description )
 {
   QgsSettings settings;

src/gui/editorwidgets/qgsvaluemapconfigdlg.h

@@ -21,6 +21,8 @@
 #include "qgseditorconfigwidget.h"
 #include "qgis_gui.h"
 
+class QComboBox;
+
 SIP_NO_FILE
 
 /**
@@ -40,6 +42,15 @@ class GUI_EXPORT QgsValueMapConfigDlg : public QgsEditorConfigWidget, private Ui
 
     void updateMap( const QMap<QString, QVariant> &map, bool insertNull );
 
+    /**
+     * Populates a \a comboBox with the appropriate entries based on a value map \a configuration.
+     *
+     * If \a skipNull is true, then NULL entries will not be added.
+     *
+     * \since QGIS 3.0
+     */
+    static void populateComboBox( QComboBox *comboBox, const QVariantMap &configuration, bool skipNull );
+
   private:
     void setRow( int row, const QString &value, const QString &description );
 

src/gui/editorwidgets/qgsvaluemapsearchwidgetwrapper.cpp

@@ -141,16 +141,9 @@ void QgsValueMapSearchWidgetWrapper::initWidget( QWidget *editor )
 
   if ( mComboBox )
   {
-    const QVariantMap cfg = config();
-    QVariantMap::ConstIterator it = cfg.constBegin();
-    mComboBox->addItem( tr( "Please select" ), QString() );
+    QgsValueMapConfigDlg::populateComboBox( mComboBox, config(), true );
+    mComboBox->insertItem( 0, tr( "Please select" ), QString() );
 
-    while ( it != cfg.constEnd() )
-    {
-      if ( it.value() != QgsValueMapFieldFormatter::NULL_VALUE )
-        mComboBox->addItem( it.key(), it.value() );
-      ++it;
-    }
     connect( mComboBox, static_cast<void ( QComboBox::* )( int )>( &QComboBox::currentIndexChanged ), this, &QgsValueMapSearchWidgetWrapper::comboBoxIndexChanged );
   }
 }

src/gui/editorwidgets/qgsvaluemapwidgetwrapper.cpp

@@ -58,27 +58,7 @@ void QgsValueMapWidgetWrapper::initWidget( QWidget *editor )
 
   if ( mComboBox )
   {
-    QList<QVariant> valueList = config().value( QStringLiteral( "map" ) ).toList();
-
-    if ( valueList.count() > 0 )
-    {
-      for ( int i = 0, row = 0; i < valueList.count(); i++, row++ )
-      {
-        mComboBox->addItem( valueList[i].toMap().constBegin().key(), valueList[i].toMap().constBegin().value() );
-      }
-    }
-    else
-    {
-      const QVariantMap map = config().value( QStringLiteral( "map" ) ).toMap();
-      QVariantMap::ConstIterator it = map.constBegin();
-
-      while ( it != map.constEnd() )
-      {
-        mComboBox->addItem( it.key(), it.value() );
-        ++it;
-      }
-    }
-
+    QgsValueMapConfigDlg::populateComboBox( mComboBox, config(), false );
     connect( mComboBox, static_cast<void ( QComboBox::* )( int )>( &QComboBox::currentIndexChanged ),
              this, static_cast<void ( QgsEditorWidgetWrapper::* )()>( &QgsEditorWidgetWrapper::emitValueChanged ) );
   }

tests/src/gui/CMakeLists.txt

@@ -134,5 +134,6 @@ ADD_QGIS_TEST(listwidgettest testqgslistwidget.cpp)
 ADD_QGIS_TEST(filedownloader testqgsfiledownloader.cpp)
 ADD_QGIS_TEST(layoutgui testqgslayoutgui.cpp)
 ADD_QGIS_TEST(layoutview testqgslayoutview.cpp)
+ADD_QGIS_TEST(valuemapwidgetwrapper testqgsvaluemapwidgetwrapper.cpp)
 ADD_QGIS_TEST(valuerelationwidgetwrapper testqgsvaluerelationwidgetwrapper.cpp)
 ADD_QGIS_TEST(relationreferencewidget testqgsrelationreferencewidget.cpp)

tests/src/gui/testqgsvaluemapwidgetwrapper.cpp

@@ -0,0 +1,136 @@
+/***************************************************************************
+    testqgsvaluemapwidgetwrapper.cpp
+     --------------------------------------
+    Date                 : January 2018
+    Copyright            : (C) 2018 Nyall Dawson
+    Email                : nyall dot dawson at gmail dot com
+ ***************************************************************************
+ *                                                                         *
+ *   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.                                   *
+ *                                                                         *
+ ***************************************************************************/
+
+
+#include "qgstest.h"
+
+#include "editorwidgets/core/qgseditorwidgetregistry.h"
+#include "qgsapplication.h"
+#include "qgseditorwidgetwrapper.h"
+#include "editorwidgets/qgsvaluemapwidgetwrapper.h"
+#include "qgsvaluemapfieldformatter.h"
+#include "editorwidgets/qgsvaluemapconfigdlg.h"
+#include "qgsgui.h"
+
+class TestQgsValueMapWidgetWrapper : public QObject
+{
+    Q_OBJECT
+  public:
+    TestQgsValueMapWidgetWrapper() = default;
+
+  private slots:
+    void initTestCase(); // will be called before the first testfunction is executed.
+    void cleanupTestCase(); // will be called after the last testfunction was executed.
+    void init(); // will be called before each testfunction is executed.
+    void cleanup(); // will be called after every testfunction.
+    void testPopulateComboBox();
+
+};
+
+void TestQgsValueMapWidgetWrapper::initTestCase()
+{
+  QgsApplication::init();
+  QgsApplication::initQgis();
+  QgsGui::editorWidgetRegistry()->initEditors();
+}
+
+void TestQgsValueMapWidgetWrapper::cleanupTestCase()
+{
+  QgsApplication::exitQgis();
+}
+
+void TestQgsValueMapWidgetWrapper::init()
+{
+}
+
+void TestQgsValueMapWidgetWrapper::cleanup()
+{
+}
+
+void TestQgsValueMapWidgetWrapper::testPopulateComboBox()
+{
+  // new style config
+  QVariantMap config;
+  QList<QVariant> valueList;
+  QVariantMap nullValue;
+  nullValue.insert( QgsApplication::nullRepresentation(), QgsValueMapFieldFormatter::NULL_VALUE );
+  valueList.append( nullValue );
+  QVariantMap value1;
+  value1.insert( QStringLiteral( "desc 1" ), QStringLiteral( "val 1" ) );
+  valueList.append( value1 );
+  QVariantMap value2;
+  value2.insert( QStringLiteral( "desc 2" ), QStringLiteral( "val 2" ) );
+  valueList.append( value2 );
+
+  config.insert( QStringLiteral( "map" ), valueList );
+
+
+  std::unique_ptr< QComboBox > combo = qgis::make_unique< QComboBox >();
+
+  // with nulls
+  QgsValueMapConfigDlg::populateComboBox( combo.get(), config, false );
+
+  QCOMPARE( combo->count(), 3 );
+  QCOMPARE( combo->itemText( 0 ), QgsApplication::nullRepresentation() );
+  QCOMPARE( combo->itemData( 0 ).toString(), QgsValueMapFieldFormatter::NULL_VALUE );
+  QCOMPARE( combo->itemText( 1 ), QStringLiteral( "desc 1" ) );
+  QCOMPARE( combo->itemData( 1 ).toString(), QStringLiteral( "val 1" ) );
+  QCOMPARE( combo->itemText( 2 ), QStringLiteral( "desc 2" ) );
+  QCOMPARE( combo->itemData( 2 ).toString(), QStringLiteral( "val 2" ) );
+
+  // no nulls
+  combo->clear();
+  QgsValueMapConfigDlg::populateComboBox( combo.get(), config, true );
+
+  QCOMPARE( combo->count(), 2 );
+  QCOMPARE( combo->itemText( 0 ), QStringLiteral( "desc 1" ) );
+  QCOMPARE( combo->itemData( 0 ).toString(), QStringLiteral( "val 1" ) );
+  QCOMPARE( combo->itemText( 1 ), QStringLiteral( "desc 2" ) );
+  QCOMPARE( combo->itemData( 1 ).toString(), QStringLiteral( "val 2" ) );
+
+  // old style config map (2.x)
+  config.clear();
+  QVariantMap mapValue;
+  mapValue.insert( QgsApplication::nullRepresentation(), QgsValueMapFieldFormatter::NULL_VALUE );
+  mapValue.insert( QStringLiteral( "desc 1" ), QStringLiteral( "val 1" ) );
+  mapValue.insert( QStringLiteral( "desc 2" ), QStringLiteral( "val 2" ) );
+  config.insert( QStringLiteral( "map" ), mapValue );
+
+  // with nulls
+  combo->clear();
+  QgsValueMapConfigDlg::populateComboBox( combo.get(), config, false );
+
+  QCOMPARE( combo->count(), 3 );
+  QCOMPARE( combo->itemText( 0 ), QgsApplication::nullRepresentation() );
+  QCOMPARE( combo->itemData( 0 ).toString(), QgsValueMapFieldFormatter::NULL_VALUE );
+  QCOMPARE( combo->itemText( 1 ), QStringLiteral( "desc 1" ) );
+  QCOMPARE( combo->itemData( 1 ).toString(), QStringLiteral( "val 1" ) );
+  QCOMPARE( combo->itemText( 2 ), QStringLiteral( "desc 2" ) );
+  QCOMPARE( combo->itemData( 2 ).toString(), QStringLiteral( "val 2" ) );
+
+  // no nulls
+  combo->clear();
+  QgsValueMapConfigDlg::populateComboBox( combo.get(), config, true );
+
+  QCOMPARE( combo->count(), 2 );
+  QCOMPARE( combo->itemText( 0 ), QStringLiteral( "desc 1" ) );
+  QCOMPARE( combo->itemData( 0 ).toString(), QStringLiteral( "val 1" ) );
+  QCOMPARE( combo->itemText( 1 ), QStringLiteral( "desc 2" ) );
+  QCOMPARE( combo->itemData( 1 ).toString(), QStringLiteral( "val 2" ) );
+
+}
+
+QGSTEST_MAIN( TestQgsValueMapWidgetWrapper )
+#include "testqgsvaluemapwidgetwrapper.moc"