From fba4b25bc25e87aa6f3a45c64acd7fd243b30bfb Mon Sep 17 00:00:00 2001
From: Julien Cabieces <julien.cabieces@oslandia.com>
Date: Thu, 20 Dec 2018 17:16:24 +0100
Subject: [PATCH] fixes #20673 : Correct mapFromSource method when reordering
 columns

---
 .../qgsattributetablefiltermodel.cpp          | 26 ++++++-----
 .../qgsattributetablefiltermodel.h            |  1 +
 tests/src/app/testqgsattributetable.cpp       | 44 +++++++++++++++++++
 3 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/src/gui/attributetable/qgsattributetablefiltermodel.cpp b/src/gui/attributetable/qgsattributetablefiltermodel.cpp
index e3133790e670..61e0020a7419 100644
--- a/src/gui/attributetable/qgsattributetablefiltermodel.cpp
+++ b/src/gui/attributetable/qgsattributetablefiltermodel.cpp
@@ -247,14 +247,11 @@ void QgsAttributeTableFilterModel::setSelectedOnTop( bool selectedOnTop )
     Qt::SortOrder order = sortOrder();
 
     // set default sort values if they are not correctly set
-    if ( column < 0 )
-      column = 0;
-
-    if ( order != Qt::AscendingOrder && order != Qt::DescendingOrder )
-      order = Qt::AscendingOrder;
-
-    sort( column, order );
-    invalidate();
+    if ( column < 0 || ( order != Qt::AscendingOrder && order != Qt::DescendingOrder ) )
+    {
+      sort( 0, Qt::AscendingOrder );
+      invalidate();
+    }
   }
 }
 
@@ -382,7 +379,6 @@ void QgsAttributeTableFilterModel::selectionChanged()
   }
   else if ( mSelectedOnTop )
   {
-    sort( sortColumn(), sortOrder() );
     invalidate();
   }
 }
@@ -402,6 +398,14 @@ int QgsAttributeTableFilterModel::mapColumnToSource( int column ) const
     return mColumnMapping.at( column );
 }
 
+int QgsAttributeTableFilterModel::mapColumnFromSource( int column ) const
+{
+  if ( mColumnMapping.isEmpty() )
+    return column;
+  else
+    return mColumnMapping.indexOf( column );
+}
+
 void QgsAttributeTableFilterModel::generateListOfVisibleFeatures()
 {
   if ( !layer() )
@@ -528,7 +532,8 @@ QModelIndex QgsAttributeTableFilterModel::mapFromSource( const QModelIndex &sour
   if ( proxyIndex.column() < 0 )
     return QModelIndex();
 
-  int col = mapColumnToSource( proxyIndex.column() );
+  int col = mapColumnFromSource( proxyIndex.column() );
+
   if ( col == -1 )
     col = 0;
 
@@ -544,4 +549,3 @@ Qt::ItemFlags QgsAttributeTableFilterModel::flags( const QModelIndex &index ) co
   QModelIndex source_index = mapToSource( index );
   return masterModel()->flags( source_index );
 }
-
diff --git a/src/gui/attributetable/qgsattributetablefiltermodel.h b/src/gui/attributetable/qgsattributetablefiltermodel.h
index 83498119b36f..b5c541b91a36 100644
--- a/src/gui/attributetable/qgsattributetablefiltermodel.h
+++ b/src/gui/attributetable/qgsattributetablefiltermodel.h
@@ -275,6 +275,7 @@ class GUI_EXPORT QgsAttributeTableFilterModel: public QSortFilterProxyModel, pub
     QgsAttributeTableConfig mConfig;
     QVector<int> mColumnMapping;
     int mapColumnToSource( int column ) const;
+    int mapColumnFromSource( int column ) const;
 
 };
 
diff --git a/tests/src/app/testqgsattributetable.cpp b/tests/src/app/testqgsattributetable.cpp
index 5dae6046918f..6969559a3d77 100644
--- a/tests/src/app/testqgsattributetable.cpp
+++ b/tests/src/app/testqgsattributetable.cpp
@@ -51,6 +51,7 @@ class TestQgsAttributeTable : public QObject
     void testNoGeom();
     void testSelected();
     void testSortByDisplayExpression();
+    void testOrderColumn();
 
   private:
     QgisApp *mQgisApp = nullptr;
@@ -306,6 +307,49 @@ void TestQgsAttributeTable::testRegression15974()
   QCOMPARE( dlg->mMainView->filteredFeatureCount(), 3 );
 }
 
+void TestQgsAttributeTable::testOrderColumn()
+{
+  std::unique_ptr< QgsVectorLayer> tempLayer( new QgsVectorLayer( QStringLiteral( "LineString?crs=epsg:3111&field=pk:int&field=col1:int&field=col2:int" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) ) );
+  QVERIFY( tempLayer->isValid() );
+
+  QgsFeature f1( tempLayer->dataProvider()->fields(), 1 );
+  f1.setAttribute( 0, 1 );
+  f1.setAttribute( 1, 13 );
+  f1.setAttribute( 2, 7 );
+  QVERIFY( tempLayer->dataProvider()->addFeatures( QgsFeatureList() << f1 ) );
+
+  std::unique_ptr< QgsAttributeTableDialog > dlg( new QgsAttributeTableDialog( tempLayer.get() ) );
+
+  // Issue https://issues.qgis.org/issues/20673
+  // When we reorder column (last column becomes first column), and we select an entire row
+  // the currentIndex is no longer the first column, and consequently it breaks edition
+
+  QgsAttributeTableConfig config = QgsAttributeTableConfig();
+  config.update( tempLayer->dataProvider()->fields() );
+  QVector<QgsAttributeTableConfig::ColumnConfig> columns = config.columns();
+
+  // move last column in first position
+  columns.move( 2, 0 );
+  config.setColumns( columns );
+
+  dlg->mMainView->setAttributeTableConfig( config );
+
+  QgsAttributeTableFilterModel *filterModel = static_cast<QgsAttributeTableFilterModel *>( dlg->mMainView->mTableView->model() );
+  filterModel->sort( 0, Qt::AscendingOrder );
+
+  QModelIndex index = filterModel->mapToSource( filterModel->sourceModel()->index( 0, 0 ) );
+  QCOMPARE( index.row(), 0 );
+  QCOMPARE( index.column(), 2 );
+
+  index = filterModel->mapFromSource( filterModel->sourceModel()->index( 0, 0 ) );
+  QCOMPARE( index.row(), 0 );
+  QCOMPARE( index.column(), 1 );
+
+  qDebug() << filterModel->mapFromSource( filterModel->sourceModel()->index( 0, 0 ) );
+
+  // column 0 is indeed column 2 since we move it
+  QCOMPARE( filterModel->sortColumn(), 2 );
+}
 
 QGSTEST_MAIN( TestQgsAttributeTable )
 #include "testqgsattributetable.moc"