Merge pull request #52045 from signedav/yellow_index_lost

Fix lost index on selection change in Attribute Table

src/gui/attributetable/qgsfeaturelistview.cpp

@@ -34,6 +34,22 @@ QgsFeatureListView::QgsFeatureListView( QWidget *parent )
   : QListView( parent )
 {
   setSelectionMode( QAbstractItemView::ExtendedSelection );
+
+  mUpdateEditSelectionTimerWithSelection.setSingleShot( true );
+  connect( &mUpdateEditSelectionTimerWithSelection, &QTimer::timeout, this, [ this ]()
+  {
+    updateEditSelection( true );
+  } );
+
+  mUpdateEditSelectionTimerWithSelection.setInterval( 0 );
+
+  mUpdateEditSelectionTimerWithoutSelection.setSingleShot( true );
+  connect( &mUpdateEditSelectionTimerWithoutSelection, &QTimer::timeout, this, [ this ]()
+  {
+    updateEditSelection( false );
+  } );
+
+  mUpdateEditSelectionTimerWithoutSelection.setInterval( 0 );
 }
 
 QgsVectorLayerCache *QgsFeatureListView::layerCache()
@@ -406,6 +422,19 @@ void QgsFeatureListView::selectRow( const QModelIndex &index, bool anchor )
 }
 
 void QgsFeatureListView::ensureEditSelection( bool inSelection )
+{
+
+  if ( inSelection )
+  {
+    mUpdateEditSelectionTimerWithSelection.start();
+  }
+  else
+  {
+    mUpdateEditSelectionTimerWithoutSelection.start();
+  }
+}
+
+void QgsFeatureListView::updateEditSelection( bool inSelection )
 {
   if ( !mModel->rowCount() )
   {
@@ -465,46 +494,37 @@ void QgsFeatureListView::ensureEditSelection( bool inSelection )
 
   if ( editSelectionUpdateRequested )
   {
-    if ( !mUpdateEditSelectionTimer.isSingleShot() )
+    // The layer might have been removed between timer start and timer triggered
+    // in this case there is nothing left for us to do.
+    if ( !layerCache() )
+      return;
+
+    int rowToSelect = -1;
+
+    if ( inSelection )
     {
-      mUpdateEditSelectionTimer.setSingleShot( true );
-      connect( &mUpdateEditSelectionTimer, &QTimer::timeout, this, [ this, inSelection, validEditSelectionAvailable ]()
+      const QgsFeatureIds selectedFids = layerCache()->layer()->selectedFeatureIds();
+      const int rowCount = mModel->rowCount();
+
+      for ( int i = 0; i < rowCount; i++ )
       {
-        // The layer might have been removed between timer start and timer triggered
-        // in this case there is nothing left for us to do.
-        if ( !layerCache() )
-          return;
-
-        int rowToSelect = -1;
-
-        if ( inSelection )
+        if ( selectedFids.contains( mModel->idxToFid( mModel->index( i, 0 ) ) ) )
         {
-          const QgsFeatureIds selectedFids = layerCache()->layer()->selectedFeatureIds();
-          const int rowCount = mModel->rowCount();
-
-          for ( int i = 0; i < rowCount; i++ )
-          {
-            if ( selectedFids.contains( mModel->idxToFid( mModel->index( i, 0 ) ) ) )
-            {
-              rowToSelect = i;
-              break;
-            }
-
-            if ( rowToSelect == -1 && !validEditSelectionAvailable )
-              rowToSelect = 0;
-          }
+          rowToSelect = i;
+          break;
         }
-        else
+
+        if ( rowToSelect == -1 && !validEditSelectionAvailable )
           rowToSelect = 0;
-
-        if ( rowToSelect != -1 )
-        {
-          setEditSelection( mModel->mapToMaster( mModel->index( rowToSelect, 0 ) ), QItemSelectionModel::ClearAndSelect );
-        }
-      } );
-      mUpdateEditSelectionTimer.setInterval( 0 );
+      }
+    }
+    else
+      rowToSelect = 0;
+
+    if ( rowToSelect != -1 )
+    {
+      setEditSelection( mModel->mapToMaster( mModel->index( rowToSelect, 0 ) ), QItemSelectionModel::ClearAndSelect );
     }
-    mUpdateEditSelectionTimer.start();
   }
 }
 

src/gui/attributetable/qgsfeaturelistview.h

@@ -234,6 +234,8 @@ class GUI_EXPORT QgsFeatureListView : public QListView
   private:
     void selectRow( const QModelIndex &index, bool anchor );
 
+    void updateEditSelection( bool inSelection = false );
+
     enum PositionInList
     {
       First,
@@ -264,7 +266,8 @@ class GUI_EXPORT QgsFeatureListView : public QListView
     int mRowAnchor = 0;
     QItemSelectionModel::SelectionFlags mCtrlDragSelectionFlag;
 
-    QTimer mUpdateEditSelectionTimer;
+    QTimer mUpdateEditSelectionTimerWithSelection;
+    QTimer mUpdateEditSelectionTimerWithoutSelection;
 
     friend class QgsDualView;
 };

tests/src/app/testqgsattributetable.cpp

@@ -32,6 +32,8 @@
 #include "qgsgui.h"
 #include "qgseditorwidgetregistry.h"
 
+#include <QSignalSpy>
+
 /**
  * \ingroup UnitTests
  * This is a unit test for the attribute table dialog
@@ -66,6 +68,7 @@ class TestQgsAttributeTable : public QObject
     void testStartMultiEditNoChanges();
     void testMultiEditMakeUncommittedChanges();
     void testInvalidView();
+    void testEnsureEditSelection();
 
   private:
     QgisApp *mQgisApp = nullptr;
@@ -830,5 +833,68 @@ void TestQgsAttributeTable::testInvalidView()
   QCOMPARE( dlg->mMainView->filteredFeatures(), QgsFeatureIds() << 1 << 3 );
 }
 
+void TestQgsAttributeTable::testEnsureEditSelection()
+{
+  std::unique_ptr< QgsVectorLayer > layer = std::make_unique< QgsVectorLayer >( QStringLiteral( "Point?field=col0:integer&field=col1:integer" ), QStringLiteral( "test" ), QStringLiteral( "memory" ) );
+  QVERIFY( layer->isValid() );
+
+  QgsFeature ft1( layer->dataProvider()->fields(), 1 );
+  ft1.setAttributes( QgsAttributes() << 1 << 2 );
+  layer->dataProvider()->addFeature( ft1 );
+  QgsFeature ft2( layer->dataProvider()->fields(), 2 );
+  ft2.setAttributes( QgsAttributes() << 3 << 4 );
+  layer->dataProvider()->addFeature( ft2 );
+  QgsFeature ft3( layer->dataProvider()->fields(), 3 );
+  ft3.setAttributes( QgsAttributes() << 5 << 6 );
+  layer->dataProvider()->addFeature( ft3 );
+  QgsFeature ft4( layer->dataProvider()->fields(), 4 );
+  ft4.setAttributes( QgsAttributes() << 7 << 8 );
+  layer->dataProvider()->addFeature( ft4 );
+
+  layer->removeSelection();
+
+  std::unique_ptr< QgsAttributeTableDialog > dlg( new QgsAttributeTableDialog( layer.get() ) );
+
+  //since the update is done by timer, we have to wait (at least one millisecond) or until the current edit selection changed
+  qRegisterMetaType<QgsFeature>( "QgsFeature&" );
+  QSignalSpy spy( dlg->mMainView->mFeatureListView, &QgsFeatureListView::currentEditSelectionChanged );
+
+  // we set the index to ft3
+  dlg->mMainView->setCurrentEditSelection( {ft3.id()} );
+  // ... and the currentEditSelection is on ft3
+  QVERIFY( dlg->mMainView->mFeatureListView->currentEditSelection().contains( 3 ) );
+
+  // we make a featureselection on ft1, ft2 and ft3
+  layer->selectByIds( QgsFeatureIds() << 1 << 2 << 3 );
+  spy.wait( 1 );
+  // ... and the currentEditSelection stays on ft3 (since it's in the featureselection)
+  QVERIFY( dlg->mMainView->mFeatureListView->currentEditSelection().contains( 3 ) );
+
+  // we release the featureselection
+  layer->removeSelection();
+  spy.wait( 1 );
+  // ... and the currentEditSelection persists on 3 (since it does not make an update)
+  QVERIFY( dlg->mMainView->mFeatureListView->currentEditSelection().contains( 3 ) );
+
+  // we make afeatureselection on ft4
+  layer->selectByIds( QgsFeatureIds() << 4 );
+  spy.wait( 1 );
+  // ... and the currentEditSelection goes to ft4
+  QVERIFY( dlg->mMainView->mFeatureListView->currentEditSelection().contains( 4 ) );
+
+  // we make afeatureselection on ft2 and ft3
+  layer->selectByIds( QgsFeatureIds() << 2 << 3 );
+  spy.wait( 1 );
+  // ... and the currentEditSelection goes to the first one of the featureselection (means ft2)
+  QVERIFY( dlg->mMainView->mFeatureListView->currentEditSelection().contains( 2 ) );
+
+  // we reload the layer
+  layer->reload();
+  spy.wait( 1 );
+  // ... and the currentEditSelection jumps to the first one (instead of staying at 2, since it's NOT persistend)
+  QVERIFY( dlg->mMainView->mFeatureListView->currentEditSelection().contains( 1 ) );
+
+}
+
 QGSTEST_MAIN( TestQgsAttributeTable )
 #include "testqgsattributetable.moc"