Merge pull request #51215 from uclaros/fix-relation-multi-completer

Allow filtering in value relation widget when allowing multiple selections

python/gui/auto_generated/attributetable/qgsattributetableview.sip.in

@@ -78,6 +78,12 @@ Optionally a ``column`` can be specified, which will also bring that column into
 .. versionadded:: 3.16
 %End
 
+    void closeCurrentEditor();
+%Docstring
+Closes the editor delegate for the current item, committing its changes to the model.
+
+.. versionadded:: 3.30
+%End
   protected:
 
     virtual void mousePressEvent( QMouseEvent *event );

src/app/qgsattributetabledialog.cpp

@@ -857,10 +857,11 @@ void QgsAttributeTableDialog::mActionToggleEditing_toggled( bool )
   if ( !mLayer )
     return;
 
-  //this has to be done, because in case only one cell has been changed and is still enabled, the change
+  if ( mLayer->isEditable() )
-  //would not be added to the mEditBuffer. By disabling, it looses focus and the change will be stored.
+  {
-  if ( mLayer->isEditable() && mMainView->tableView()->indexWidget( mMainView->tableView()->currentIndex() ) )
+    // commit changes in the currently active cell
-    mMainView->tableView()->indexWidget( mMainView->tableView()->currentIndex() )->setEnabled( false );
+    mMainView->tableView()->closeCurrentEditor();
+  }
 
   if ( !QgisApp::instance()->toggleEditing( mLayer ) )
   {

src/gui/attributetable/qgsattributetableview.cpp

@@ -549,3 +549,10 @@ void QgsAttributeTableView::scrollToFeature( const QgsFeatureId &fid, int col )
 
   selectionModel()->setCurrentIndex( index, QItemSelectionModel::SelectCurrent );
 }
+
+void QgsAttributeTableView::closeCurrentEditor()
+{
+  QWidget *editor = indexWidget( currentIndex() );
+  commitData( editor );
+  closeEditor( editor, QAbstractItemDelegate::NoHint );
+}

src/gui/attributetable/qgsattributetableview.h

@@ -104,6 +104,12 @@ class GUI_EXPORT QgsAttributeTableView : public QgsTableView
      */
     void scrollToFeature( const QgsFeatureId &fid, int column = -1 );
 
+    /**
+     * Closes the editor delegate for the current item, committing its changes to the model.
+     *
+     * \since QGIS 3.30
+     */
+    void closeCurrentEditor();
   protected:
 
     /**

src/gui/editorwidgets/qgsvaluerelationwidgetwrapper.cpp

@@ -29,24 +29,178 @@
 #include "qgspostgresstringutils.h"
 #include "qgsapplication.h"
 
-#include <QHeaderView>
 #include <QComboBox>
 #include <QLineEdit>
-#include <QTableWidget>
 #include <QStringListModel>
 #include <QCompleter>
 #include <QTimer>
+#include <QVBoxLayout>
+#include <QHeaderView>
+#include <QKeyEvent>
 
 #include <nlohmann/json.hpp>
 using namespace nlohmann;
 
+///@cond PRIVATE
+QgsFilteredTableWidget::QgsFilteredTableWidget( QWidget *parent, bool showSearch )
+  : QWidget( parent )
+{
+  mSearchWidget = new QgsFilterLineEdit( this );
+  mSearchWidget->setShowSearchIcon( true );
+  mSearchWidget->setShowClearButton( true );
+  mTableWidget  = new QTableWidget( this );
+  mTableWidget->horizontalHeader()->setSectionResizeMode( QHeaderView::Stretch );
+  mTableWidget->horizontalHeader()->setVisible( false );
+  mTableWidget->verticalHeader()->setSectionResizeMode( QHeaderView::Stretch );
+  mTableWidget->verticalHeader()->setVisible( false );
+  mTableWidget->setShowGrid( false );
+  mTableWidget->setEditTriggers( QAbstractItemView::NoEditTriggers );
+  mTableWidget->setSelectionMode( QAbstractItemView::NoSelection );
+  QVBoxLayout *layout = new QVBoxLayout();
+  layout->addWidget( mSearchWidget );
+  layout->addWidget( mTableWidget );
+  layout->setContentsMargins( 0, 0, 0, 0 );
+  layout->setSpacing( 0 );
+  if ( showSearch )
+  {
+    mTableWidget->setFocusProxy( mSearchWidget );
+    connect( mSearchWidget, &QgsFilterLineEdit::textChanged, this, &QgsFilteredTableWidget::filterStringChanged );
+    installEventFilter( this );
+  }
+  else
+  {
+    mSearchWidget->setVisible( false );
+  }
+  setLayout( layout );
+  connect( mTableWidget, &QTableWidget::itemChanged, this, &QgsFilteredTableWidget::itemChanged_p );
+}
+
+bool QgsFilteredTableWidget::eventFilter( QObject *watched, QEvent *event )
+{
+  Q_UNUSED( watched )
+  if ( event->type() == QEvent::KeyPress )
+  {
+    QKeyEvent *keyEvent = static_cast<QKeyEvent *>( event );
+    if ( keyEvent->key() == Qt::Key_Escape &&
+         !mSearchWidget->text().isEmpty() )
+    {
+      mSearchWidget->clear();
+      return true;
+    }
+  }
+  return false;
+}
+
+void QgsFilteredTableWidget::filterStringChanged( const QString &filterString )
+{
+  auto signalBlockedTableWidget = whileBlocking( mTableWidget );
+  Q_UNUSED( signalBlockedTableWidget )
+  mTableWidget->clearContents();
+  const int rCount = std::max( 1, ( int ) std::ceil( ( float ) mCache.count() / ( float ) mColumnCount ) );
+  mTableWidget->setRowCount( rCount );
+  int row = 0;
+  int column = 0;
+
+  for ( const QPair<QgsValueRelationFieldFormatter::ValueRelationItem, Qt::CheckState> &pair : std::as_const( mCache ) )
+  {
+    if ( column == mColumnCount )
+    {
+      row++;
+      column = 0;
+    }
+    if ( pair.first.value.contains( filterString, Qt::CaseInsensitive ) )
+    {
+      QTableWidgetItem *item = nullptr;
+      item = new QTableWidgetItem( pair.first.value );
+      item->setData( Qt::UserRole, pair.first.key );
+      item->setData( Qt::ToolTipRole, pair.first.description );
+      item->setCheckState( pair.second );
+      item->setFlags( mEnabledTable ? item->flags() | Qt::ItemIsEnabled : item->flags() & ~Qt::ItemIsEnabled );
+      mTableWidget->setItem( row, column, item );
+      column++;
+    }
+  }
+  mTableWidget->setRowCount( row + 1 );
+}
+
+QStringList QgsFilteredTableWidget::selection() const
+{
+  QStringList sel;
+  for ( const QPair<QgsValueRelationFieldFormatter::ValueRelationItem, Qt::CheckState> &pair : std::as_const( mCache ) )
+  {
+    if ( pair.second == Qt::Checked )
+      sel.append( pair.first.key.toString() );
+  }
+  return sel;
+}
+
+void QgsFilteredTableWidget::checkItems( const QStringList &checked )
+{
+  for ( QPair<QgsValueRelationFieldFormatter::ValueRelationItem, Qt::CheckState> &pair : mCache )
+  {
+    const bool isChecked = checked.contains( pair.first.key.toString() );
+    pair.second = isChecked ? Qt::Checked : Qt::Unchecked;
+  }
+
+  filterStringChanged( mSearchWidget->text() );
+}
+
+void QgsFilteredTableWidget::populate( QgsValueRelationFieldFormatter::ValueRelationCache cache )
+{
+  mCache.clear();
+  for ( const QgsValueRelationFieldFormatter::ValueRelationItem &element : std::as_const( cache ) )
+  {
+    mCache.append( qMakePair( element, Qt::Unchecked ) );
+  }
+  filterStringChanged( mSearchWidget->text() );
+}
+
+void QgsFilteredTableWidget::setIndeterminateState()
+{
+  for ( int j = 0; j < mTableWidget->rowCount(); j++ )
+  {
+    for ( int i = 0; i < mColumnCount; ++i )
+    {
+      whileBlocking( mTableWidget )->item( j, i )->setCheckState( Qt::PartiallyChecked );
+    }
+  }
+}
+
+void QgsFilteredTableWidget::setEnabledTable( const bool enabled )
+{
+  if ( mEnabledTable == enabled )
+    return;
+
+  mEnabledTable = enabled;
+  if ( !enabled )
+    mSearchWidget->clear();
+
+  filterStringChanged( mSearchWidget->text() );
+}
+
+void QgsFilteredTableWidget::setColumnCount( const int count )
+{
+  mColumnCount = count;
+  mTableWidget->setColumnCount( count );
+}
+
+void QgsFilteredTableWidget::itemChanged_p( QTableWidgetItem *item )
+{
+  for ( QPair<QgsValueRelationFieldFormatter::ValueRelationItem, Qt::CheckState> &pair : mCache )
+  {
+    if ( pair.first.key == item->data( Qt::UserRole ) )
+      pair.second = item->checkState();
+  }
+  emit itemChanged( item );
+}
+///@endcond
+
 
 QgsValueRelationWidgetWrapper::QgsValueRelationWidgetWrapper( QgsVectorLayer *layer, int fieldIdx, QWidget *editor, QWidget *parent )
   : QgsEditorWidgetWrapper( layer, fieldIdx, editor, parent )
 {
 }
 
-
 QVariant QgsValueRelationWidgetWrapper::value() const
 {
   QVariant v;
@@ -63,20 +217,7 @@ QVariant QgsValueRelationWidgetWrapper::value() const
   }
   else if ( mTableWidget )
   {
-    const int nofColumns = columnCount();
+    QStringList selection = mTableWidget->selection();
-    QStringList selection;
-    for ( int j = 0; j < mTableWidget->rowCount(); j++ )
-    {
-      for ( int i = 0; i < nofColumns; ++i )
-      {
-        QTableWidgetItem *item = mTableWidget->item( j, i );
-        if ( item )
-        {
-          if ( item->checkState() == Qt::Checked )
-            selection << item->data( Qt::UserRole ).toString();
-        }
-      }
-    }
 
     // If there is no selection and allow NULL is not checked return NULL.
     if ( selection.isEmpty() && ! config( QStringLiteral( "AllowNull" ) ).toBool( ) )
@@ -138,11 +279,13 @@ QWidget *QgsValueRelationWidgetWrapper::createWidget( QWidget *parent )
 
   mExpression = config().value( QStringLiteral( "FilterExpression" ) ).toString();
 
-  if ( config( QStringLiteral( "AllowMulti" ) ).toBool() )
+  const bool allowMulti = config( QStringLiteral( "AllowMulti" ) ).toBool();
+  const bool useCompleter = config( QStringLiteral( "UseCompleter" ) ).toBool();
+  if ( allowMulti )
   {
-    return new QTableWidget( parent );
+    return new QgsFilteredTableWidget( parent, useCompleter );
   }
-  else if ( config( QStringLiteral( "UseCompleter" ) ).toBool() )
+  else if ( useCompleter )
   {
     return new QgsFilterLineEdit( parent );
   }
@@ -156,7 +299,7 @@ void QgsValueRelationWidgetWrapper::initWidget( QWidget *editor )
 {
 
   mComboBox = qobject_cast<QComboBox *>( editor );
-  mTableWidget = qobject_cast<QTableWidget *>( editor );
+  mTableWidget = qobject_cast<QgsFilteredTableWidget *>( editor );
   mLineEdit = qobject_cast<QLineEdit *>( editor );
 
   // Read current initial form values from the editor context
@@ -170,14 +313,7 @@ void QgsValueRelationWidgetWrapper::initWidget( QWidget *editor )
   }
   else if ( mTableWidget )
   {
-    mTableWidget->horizontalHeader()->setSectionResizeMode( QHeaderView::Stretch );
+    connect( mTableWidget, &QgsFilteredTableWidget::itemChanged, this, static_cast<void ( QgsEditorWidgetWrapper::* )()>( &QgsEditorWidgetWrapper::emitValueChanged ), Qt::UniqueConnection );
-    mTableWidget->horizontalHeader()->setVisible( false );
-    mTableWidget->verticalHeader()->setSectionResizeMode( QHeaderView::Stretch );
-    mTableWidget->verticalHeader()->setVisible( false );
-    mTableWidget->setShowGrid( false );
-    mTableWidget->setEditTriggers( QAbstractItemView::NoEditTriggers );
-    mTableWidget->setSelectionMode( QAbstractItemView::NoSelection );
-    connect( mTableWidget, &QTableWidget::itemChanged, this, static_cast<void ( QgsEditorWidgetWrapper::* )()>( &QgsEditorWidgetWrapper::emitValueChanged ), Qt::UniqueConnection );
   }
   else if ( mLineEdit )
   {
@@ -206,33 +342,7 @@ void QgsValueRelationWidgetWrapper::updateValues( const QVariant &value, const Q
       checkList = QgsValueRelationFieldFormatter::valueToStringList( value );
     }
 
-    QTableWidgetItem *lastChangedItem = nullptr;
+    mTableWidget->checkItems( checkList );
-
-    const int nofColumns = columnCount();
-
-    // This block is needed because item->setCheckState triggers dataChanged gets back to value()
-    // and iterate over all items again! This can be extremely slow on large items sets.
-    for ( int j = 0; j < mTableWidget->rowCount(); j++ )
-    {
-      auto signalBlockedTableWidget = whileBlocking( mTableWidget );
-      Q_UNUSED( signalBlockedTableWidget )
-
-      for ( int i = 0; i < nofColumns; ++i )
-      {
-        QTableWidgetItem *item = mTableWidget->item( j, i );
-        if ( item )
-        {
-          item->setCheckState( checkList.contains( item->data( Qt::UserRole ).toString() ) ? Qt::Checked : Qt::Unchecked );
-          //re-set enabled state because it's lost after reloading items
-          item->setFlags( mEnabled ? item->flags() | Qt::ItemIsEnabled : item->flags() & ~Qt::ItemIsEnabled );
-          lastChangedItem = item;
-        }
-      }
-    }
-    // let's trigger the signal now, once and for all
-    if ( lastChangedItem )
-      lastChangedItem->setCheckState( checkList.contains( lastChangedItem->data( Qt::UserRole ).toString() ) ? Qt::Checked : Qt::Unchecked );
-
   }
   else if ( mComboBox )
   {
@@ -415,33 +525,8 @@ void QgsValueRelationWidgetWrapper::populate( )
   }
   else if ( mTableWidget )
   {
-    const int nofColumns = columnCount();
+    mTableWidget->setColumnCount( columnCount() );
-
+    mTableWidget->populate( mCache );
-    if ( ! mCache.empty() )
-    {
-      mTableWidget->setRowCount( ( mCache.size() + nofColumns - 1 ) / nofColumns );
-    }
-    else
-      mTableWidget->setRowCount( 1 );
-    mTableWidget->setColumnCount( nofColumns );
-
-    whileBlocking( mTableWidget )->clear();
-    int row = 0;
-    int column = 0;
-    for ( const QgsValueRelationFieldFormatter::ValueRelationItem &element : std::as_const( mCache ) )
-    {
-      if ( column == nofColumns )
-      {
-        row++;
-        column = 0;
-      }
-      QTableWidgetItem *item = nullptr;
-      item = new QTableWidgetItem( element.value );
-      item->setData( Qt::UserRole, element.key );
-      whileBlocking( mTableWidget )->setItem( row, column, item );
-      column++;
-    }
-
   }
   else if ( mLineEdit )
   {
@@ -460,17 +545,9 @@ void QgsValueRelationWidgetWrapper::populate( )
 
 void QgsValueRelationWidgetWrapper::showIndeterminateState()
 {
-  const int nofColumns = columnCount();
-
   if ( mTableWidget )
   {
-    for ( int j = 0; j < mTableWidget->rowCount(); j++ )
+    mTableWidget->setIndeterminateState();
-    {
-      for ( int i = 0; i < nofColumns; ++i )
-      {
-        whileBlocking( mTableWidget )->item( j, i )->setCheckState( Qt::PartiallyChecked );
-      }
-    }
   }
   else if ( mComboBox )
   {
@@ -491,20 +568,7 @@ void QgsValueRelationWidgetWrapper::setEnabled( bool enabled )
 
   if ( mTableWidget )
   {
-    auto signalBlockedTableWidget = whileBlocking( mTableWidget );
+    mTableWidget->setEnabledTable( enabled );
-    Q_UNUSED( signalBlockedTableWidget )
-
-    for ( int j = 0; j < mTableWidget->rowCount(); j++ )
-    {
-      for ( int i = 0; i < mTableWidget->columnCount(); ++i )
-      {
-        QTableWidgetItem *item = mTableWidget->item( j, i );
-        if ( item )
-        {
-          item->setFlags( enabled ? item->flags() | Qt::ItemIsEnabled : item->flags() & ~Qt::ItemIsEnabled );
-        }
-      }
-    }
   }
   else
     QgsEditorWidgetWrapper::setEnabled( enabled );

src/gui/editorwidgets/qgsvaluerelationwidgetwrapper.h

@@ -16,17 +16,98 @@
 #ifndef QGSVALUERELATIONWIDGETWRAPPER_H
 #define QGSVALUERELATIONWIDGETWRAPPER_H
 
+#include <QTableWidget>
+
 #include "qgseditorwidgetwrapper.h"
 #include "qgsvaluerelationfieldformatter.h"
 #include "qgis_gui.h"
 
-class QTableWidget;
 class QComboBox;
 class QLineEdit;
+class QgsValueRelationWidgetFactory;
+class QgsFilterLineEdit;
 
 SIP_NO_FILE
 
-class QgsValueRelationWidgetFactory;
+///@cond PRIVATE
+
+/**
+ * \brief The QgsFilteredTableWidget class
+ *
+ * This is a helper widget for QgsValueRelationWidgetWrapper
+ * This widget is a QTableWidget showing checkable items, with an optional QgsFilterLineEdit on top that allows filtering the table's items
+ */
+class QgsFilteredTableWidget : public QWidget
+{
+    Q_OBJECT
+
+  public:
+
+    /**
+     * \brief QgsFilteredTableWidget constructor
+     * \param parent
+     * \param showSearch Whether the search QgsFilterLineEdit should be visible or not
+     */
+    QgsFilteredTableWidget( QWidget *parent, bool showSearch );
+
+    bool eventFilter( QObject *watched, QEvent *event ) override;
+
+    /**
+     * Returns the list of selected (checked) items
+     */
+    QStringList selection() const;
+
+    /**
+     * Updates the check state of the table's items. Items whose DisplayRole is contained in \a checked are checked, the rest are unchecked
+     */
+    void checkItems( const QStringList &checked );
+
+    /**
+     * Populate the table using \a cache
+     */
+    void populate( QgsValueRelationFieldFormatter::ValueRelationCache cache );
+
+    /**
+     * Sets all items to be partially checked
+     */
+    void setIndeterminateState();
+
+    /**
+     * Set all table items to \a enabled.
+     */
+    void setEnabledTable( const bool enabled );
+
+    /**
+     * Sets the number of columns of the table
+     */
+    void setColumnCount( const int count );
+
+    /**
+     * Returns the number of rows of the table
+     */
+    int rowCount() const { return mTableWidget->rowCount(); }
+
+  signals:
+
+    /**
+     * Emitted when an \a item is changed by the user
+     */
+    void itemChanged( QTableWidgetItem *item );
+
+  private:
+    void filterStringChanged( const QString &filterString );
+    void itemChanged_p( QTableWidgetItem *item );
+    QTableWidgetItem *item( const int row, const int column ) const { return mTableWidget->item( row, column ); }
+
+    int mColumnCount = 1;
+    QgsFilterLineEdit *mSearchWidget = nullptr;
+    QTableWidget *mTableWidget = nullptr;
+    bool mEnabledTable = true;
+    QVector<QPair<QgsValueRelationFieldFormatter::ValueRelationItem, Qt::CheckState>> mCache;
+    friend class TestQgsValueRelationWidgetWrapper;
+};
+
+///@endcond
 
 /**
  * \ingroup gui
@@ -126,7 +207,7 @@ class GUI_EXPORT QgsValueRelationWidgetWrapper : public QgsEditorWidgetWrapper
     void populate( );
 
     QComboBox *mComboBox = nullptr;
-    QTableWidget *mTableWidget = nullptr;
+    QgsFilteredTableWidget *mTableWidget = nullptr;
     QLineEdit *mLineEdit = nullptr;
 
     QgsValueRelationFieldFormatter::ValueRelationCache mCache;

tests/src/gui/testqgsvaluerelationwidgetwrapper.cpp

@@ -24,7 +24,7 @@
 #include <qgsvectorlayer.h>
 #include "qgseditorwidgetwrapper.h"
 #include <editorwidgets/qgsvaluerelationwidgetwrapper.h>
-#include <QTableWidget>
+#include "qgsfilterlineedit.h"
 #include <QComboBox>
 #include "qgsgui.h"
 #include <gdal_version.h>
@@ -60,6 +60,8 @@ class TestQgsValueRelationWidgetWrapper : public QObject
     void testMatchLayerName();
     //! Check that setFeature works correctly after regression #42003
     void testRegressionGH42003();
+    void testAllowMultiColumns();
+    void testAllowMultiAndCompleter();
 };
 
 void TestQgsValueRelationWidgetWrapper::initTestCase()
@@ -86,24 +88,48 @@ void TestQgsValueRelationWidgetWrapper::cleanup()
 void TestQgsValueRelationWidgetWrapper::testScrollBarUnlocked()
 {
   // create a vector layer
-  QgsVectorLayer vl1( QStringLiteral( "LineString?crs=epsg:3111&field=pk:int&field=fk|:int" ), QStringLiteral( "vl1" ), QStringLiteral( "memory" ) );
+  QgsVectorLayer vl1( QStringLiteral( "Polygon?crs=epsg:4326&field=pk:int&field=province:int&field=municipality:string" ), QStringLiteral( "vl1" ), QStringLiteral( "memory" ) );
+  QgsVectorLayer vl2( QStringLiteral( "Point?crs=epsg:4326&field=pk:int&field=fk_province:int&field=fk_municipality:int" ), QStringLiteral( "vl2" ), QStringLiteral( "memory" ) );
   QgsProject::instance()->addMapLayer( &vl1, false, false );
+  QgsProject::instance()->addMapLayer( &vl2, false, false );
 
-  // build a value relation widget wrapper
+  // insert some features
-  QgsValueRelationWidgetWrapper w( &vl1, 0, nullptr, nullptr );
+  QgsFeature f1( vl1.fields() );
+  f1.setAttribute( QStringLiteral( "pk" ), 1 );
+  f1.setAttribute( QStringLiteral( "province" ), 123 );
+  f1.setAttribute( QStringLiteral( "municipality" ), QStringLiteral( "Some Place By The River" ) );
+  f1.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "POLYGON(( 0 0, 0 1, 1 1, 1 0, 0 0 ))" ) ) );
+  QVERIFY( f1.isValid() );
+  QgsFeature f2( vl1.fields() );
+  f2.setAttribute( QStringLiteral( "pk" ), 2 );
+  f2.setAttribute( QStringLiteral( "province" ), 245 );
+  f2.setAttribute( QStringLiteral( "municipality" ), QStringLiteral( "Dreamland By The Clouds" ) );
+  f2.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "POLYGON(( 1 0, 1 1, 2 1, 2 0, 1 0 ))" ) ) );
+  QVERIFY( f2.isValid() );
+  QVERIFY( vl1.dataProvider()->addFeatures( QgsFeatureList() << f1 << f2 ) );
 
-  QVariantMap config;
+  QgsFeature f3( vl2.fields() );
-  config.insert( QStringLiteral( "AllowMulti" ), true );
+  f3.setAttribute( QStringLiteral( "fk_province" ), 123 );
-  w.setConfig( config );
+  f3.setAttribute( QStringLiteral( "fk_municipality" ), 1 );
+  f3.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "POINT( 0.5 0.5)" ) ) );
+  QVERIFY( f3.isValid() );
+  QVERIFY( f3.geometry().isGeosValid() );
+  QVERIFY( vl2.dataProvider()->addFeature( f3 ) );
+
+  // build a value relation widget wrapper for municipality
+  QgsValueRelationWidgetWrapper w( &vl2, vl2.fields().indexOf( QLatin1String( "fk_municipality" ) ), nullptr, nullptr );
+  QVariantMap cfg;
+  cfg.insert( QStringLiteral( "Layer" ), vl1.id() );
+  cfg.insert( QStringLiteral( "Key" ),  QStringLiteral( "pk" ) );
+  cfg.insert( QStringLiteral( "Value" ), QStringLiteral( "municipality" ) );
+  cfg.insert( QStringLiteral( "AllowMulti" ), true );
+  cfg.insert( QStringLiteral( "NofColumns" ), 1 );
+  cfg.insert( QStringLiteral( "AllowNull" ), false );
+  cfg.insert( QStringLiteral( "OrderByValue" ), true );
+  cfg.insert( QStringLiteral( "FilterExpression" ), QStringLiteral( "\"province\" = current_value('fk_province')" ) );
+  cfg.insert( QStringLiteral( "UseCompleter" ), false );
+  w.setConfig( cfg );
   w.widget();
-  w.setEnabled( true );
-
-  // add an item virtually
-  QTableWidgetItem item;
-  item.setText( QStringLiteral( "MyText" ) );
-  w.mTableWidget->setItem( 0, 0, &item );
-
-  QCOMPARE( w.mTableWidget->item( 0, 0 )->text(), QString( "MyText" ) );
 
   // when the widget wrapper is enabled, the container should be enabled
   // as well as items
@@ -1672,5 +1698,124 @@ void TestQgsValueRelationWidgetWrapper::testRegressionGH42003()
 
 }
 
+void TestQgsValueRelationWidgetWrapper::testAllowMultiColumns()
+{
+  // create ogr gpkg layers
+  const QString myFileName( TEST_DATA_DIR ); //defined in CmakeLists.txt
+  const QString myTempDirName = tempDir.path();
+  QFile::copy( myFileName + "/provider/test_json.gpkg", myTempDirName + "/test_json.gpkg" );
+  const QString myTempFileName = myTempDirName + "/test_json.gpkg";
+  const QFileInfo myMapFileInfo( myTempFileName );
+  std::unique_ptr<QgsVectorLayer> vl_text( new QgsVectorLayer( myMapFileInfo.filePath() + "|layername=foo", "test", QStringLiteral( "ogr" ) ) );
+  std::unique_ptr<QgsVectorLayer> vl_authors( new QgsVectorLayer( myMapFileInfo.filePath() + "|layername=author", "test", QStringLiteral( "ogr" ) ) );
+  QVERIFY( vl_text->isValid() );
+  QVERIFY( vl_authors->isValid() );
+
+  QgsProject::instance()->addMapLayer( vl_text.get(), false, false );
+  QgsProject::instance()->addMapLayer( vl_authors.get(), false, false );
+  vl_text->startEditing();
+
+  // build a value relation widget wrapper for authors
+  QgsValueRelationWidgetWrapper w_favoriteauthors( vl_text.get(), vl_text->fields().indexOf( QLatin1String( "PRFEDEA" ) ), nullptr, nullptr );
+  QVariantMap cfg_favoriteauthors;
+  cfg_favoriteauthors.insert( QStringLiteral( "Layer" ), vl_authors->id() );
+  cfg_favoriteauthors.insert( QStringLiteral( "Key" ),  QStringLiteral( "fid" ) );
+  cfg_favoriteauthors.insert( QStringLiteral( "Value" ), QStringLiteral( "NAME" ) );
+  cfg_favoriteauthors.insert( QStringLiteral( "AllowMulti" ), true );
+  cfg_favoriteauthors.insert( QStringLiteral( "NofColumns" ), 3 );
+  cfg_favoriteauthors.insert( QStringLiteral( "AllowNull" ), false );
+  cfg_favoriteauthors.insert( QStringLiteral( "OrderByValue" ), false );
+  cfg_favoriteauthors.insert( QStringLiteral( "UseCompleter" ), false );
+  w_favoriteauthors.setConfig( cfg_favoriteauthors );
+  w_favoriteauthors.widget();
+  w_favoriteauthors.setEnabled( true );
+
+  //check if set up nice
+  QCOMPARE( w_favoriteauthors.mTableWidget->rowCount(), 2 );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 0 )->text(), QStringLiteral( "Erich Gamma" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 0 )->data( Qt::UserRole ).toString(), QStringLiteral( "1" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 1 )->text(), QStringLiteral( "Richard Helm" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 1 )->data( Qt::UserRole ).toString(), QStringLiteral( "2" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 2 )->text(), QStringLiteral( "Ralph Johnson" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 2 )->data( Qt::UserRole ).toString(), QStringLiteral( "3" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 0 )->text(), QStringLiteral( "John Vlissides" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 0 )->data( Qt::UserRole ).toString(), QStringLiteral( "4" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 1 )->text(), QStringLiteral( "Douglas Adams" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 1 )->data( Qt::UserRole ).toString(), QStringLiteral( "5" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 2 )->text(), QStringLiteral( "Ken Follett" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 2 )->data( Qt::UserRole ).toString(), QStringLiteral( "6" ) );
+}
+
+void TestQgsValueRelationWidgetWrapper::testAllowMultiAndCompleter()
+{
+  // create ogr gpkg layers
+  const QString myFileName( TEST_DATA_DIR ); //defined in CmakeLists.txt
+  const QString myTempDirName = tempDir.path();
+  QFile::copy( myFileName + "/provider/test_json.gpkg", myTempDirName + "/test_json.gpkg" );
+  const QString myTempFileName = myTempDirName + "/test_json.gpkg";
+  const QFileInfo myMapFileInfo( myTempFileName );
+  std::unique_ptr<QgsVectorLayer> vl_text( new QgsVectorLayer( myMapFileInfo.filePath() + "|layername=foo", "test", QStringLiteral( "ogr" ) ) );
+  std::unique_ptr<QgsVectorLayer> vl_authors( new QgsVectorLayer( myMapFileInfo.filePath() + "|layername=author", "test", QStringLiteral( "ogr" ) ) );
+  QVERIFY( vl_text->isValid() );
+  QVERIFY( vl_authors->isValid() );
+
+  QgsProject::instance()->addMapLayer( vl_text.get(), false, false );
+  QgsProject::instance()->addMapLayer( vl_authors.get(), false, false );
+  vl_text->startEditing();
+
+  // build a value relation widget wrapper for authors
+  QgsValueRelationWidgetWrapper w_favoriteauthors( vl_text.get(), vl_text->fields().indexOf( QLatin1String( "PRFEDEA" ) ), nullptr, nullptr );
+  QVariantMap cfg_favoriteauthors;
+  cfg_favoriteauthors.insert( QStringLiteral( "Layer" ), vl_authors->id() );
+  cfg_favoriteauthors.insert( QStringLiteral( "Key" ),  QStringLiteral( "fid" ) );
+  cfg_favoriteauthors.insert( QStringLiteral( "Value" ), QStringLiteral( "NAME" ) );
+  cfg_favoriteauthors.insert( QStringLiteral( "AllowMulti" ), true );
+  cfg_favoriteauthors.insert( QStringLiteral( "NofColumns" ), 3 );
+  cfg_favoriteauthors.insert( QStringLiteral( "AllowNull" ), false );
+  cfg_favoriteauthors.insert( QStringLiteral( "OrderByValue" ), false );
+  cfg_favoriteauthors.insert( QStringLiteral( "UseCompleter" ), true );
+  w_favoriteauthors.setConfig( cfg_favoriteauthors );
+  w_favoriteauthors.widget();
+  w_favoriteauthors.setEnabled( true );
+
+  //check if set up nice
+  QCOMPARE( w_favoriteauthors.mTableWidget->rowCount(), 2 );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 0 )->text(), QStringLiteral( "Erich Gamma" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 0 )->data( Qt::UserRole ).toString(), QStringLiteral( "1" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 1 )->text(), QStringLiteral( "Richard Helm" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 1 )->data( Qt::UserRole ).toString(), QStringLiteral( "2" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 2 )->text(), QStringLiteral( "Ralph Johnson" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 2 )->data( Qt::UserRole ).toString(), QStringLiteral( "3" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 0 )->text(), QStringLiteral( "John Vlissides" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 0 )->data( Qt::UserRole ).toString(), QStringLiteral( "4" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 1 )->text(), QStringLiteral( "Douglas Adams" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 1 )->data( Qt::UserRole ).toString(), QStringLiteral( "5" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 2 )->text(), QStringLiteral( "Ken Follett" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 2 )->data( Qt::UserRole ).toString(), QStringLiteral( "6" ) );
+
+  // set a filter string and check if items are filtered
+  w_favoriteauthors.mTableWidget->mSearchWidget->setText( QStringLiteral( "john" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->rowCount(), 1 );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 0 )->text(), QStringLiteral( "Ralph Johnson" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 0 )->data( Qt::UserRole ).toString(), QStringLiteral( "3" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 1 )->text(), QStringLiteral( "John Vlissides" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 1 )->data( Qt::UserRole ).toString(), QStringLiteral( "4" ) );
+
+  // clear the filter and check that all are back
+  w_favoriteauthors.mTableWidget->mSearchWidget->clear();
+  QCOMPARE( w_favoriteauthors.mTableWidget->rowCount(), 2 );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 0 )->text(), QStringLiteral( "Erich Gamma" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 0 )->data( Qt::UserRole ).toString(), QStringLiteral( "1" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 1 )->text(), QStringLiteral( "Richard Helm" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 1 )->data( Qt::UserRole ).toString(), QStringLiteral( "2" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 2 )->text(), QStringLiteral( "Ralph Johnson" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 0, 2 )->data( Qt::UserRole ).toString(), QStringLiteral( "3" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 0 )->text(), QStringLiteral( "John Vlissides" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 0 )->data( Qt::UserRole ).toString(), QStringLiteral( "4" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 1 )->text(), QStringLiteral( "Douglas Adams" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 1 )->data( Qt::UserRole ).toString(), QStringLiteral( "5" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 2 )->text(), QStringLiteral( "Ken Follett" ) );
+  QCOMPARE( w_favoriteauthors.mTableWidget->item( 1, 2 )->data( Qt::UserRole ).toString(), QStringLiteral( "6" ) );
+}
 QGSTEST_MAIN( TestQgsValueRelationWidgetWrapper )
 #include "testqgsvaluerelationwidgetwrapper.moc"

tests/src/python/test_qgseditwidgets.py

@@ -125,24 +125,6 @@ class TestQgsValueRelationWidget(unittest.TestCase):
         wrapper.setEnabled(True)
         self.assertTrue(widget.isEnabled())
 
-    def test_enableDisableOnTableWidget(self):
-        reg = QgsGui.editorWidgetRegistry()
-        layer = QgsVectorLayer("none?field=number:integer", "layer", "memory")
-        wrapper = reg.create('ValueRelation', layer, 0, {'AllowMulti': 'True'}, None, None)
-
-        widget = wrapper.widget()
-        item = QTableWidgetItem('first item')
-        widget.setItem(0, 0, item)
-
-        # does not change the state the whole widget but the single items instead
-        wrapper.setEnabled(False)
-        # widget still true, but items false
-        self.assertTrue(widget.isEnabled())
-        self.assertNotEqual(widget.item(0, 0).flags(), widget.item(0, 0).flags() | Qt.ItemIsEnabled)
-        wrapper.setEnabled(True)
-        self.assertTrue(widget.isEnabled())
-        self.assertEqual(widget.item(0, 0).flags(), widget.item(0, 0).flags() | Qt.ItemIsEnabled)
-
     def test_value_relation_set_value_not_in_map(self):
         """
         Test that setting a value not in the map is correctly handled