fixes #20673 : Correct mapFromSource method when reordering columns

troopa81 · troopa81 · commit fba4b25bc25e · 2018-12-20T17:21:56.000+01:00
diff --git 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
@@ -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
@@ -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"

Original file line number	Diff line number	Diff line change
`@@ -247,14 +247,11 @@ void QgsAttributeTableFilterModel::setSelectedOnTop( bool selectedOnTop )`
`247`	`247`	`Qt::SortOrder order = sortOrder();`
`248`	`248`
`249`	`249`	`// set default sort values if they are not correctly set`
`250`		`- if ( column < 0 )`
`251`		`- column = 0;`
`252`		`-`
`253`		`- if ( order != Qt::AscendingOrder && order != Qt::DescendingOrder )`
`254`		`- order = Qt::AscendingOrder;`
`255`		`-`
`256`		`- sort( column, order );`
`257`		`- invalidate();`
	`250`	`+ if ( column < 0 \|\| ( order != Qt::AscendingOrder && order != Qt::DescendingOrder ) )`
	`251`	`+ {`
	`252`	`+ sort( 0, Qt::AscendingOrder );`
	`253`	`+ invalidate();`
	`254`	`+ }`
`258`	`255`	`}`
`259`	`256`	`}`
`260`	`257`
`@@ -382,7 +379,6 @@ void QgsAttributeTableFilterModel::selectionChanged()`
`382`	`379`	`}`
`383`	`380`	`else if ( mSelectedOnTop )`
`384`	`381`	`{`
`385`		`- sort( sortColumn(), sortOrder() );`
`386`	`382`	`invalidate();`
`387`	`383`	`}`
`388`	`384`	`}`
`@@ -402,6 +398,14 @@ int QgsAttributeTableFilterModel::mapColumnToSource( int column ) const`
`402`	`398`	`return mColumnMapping.at( column );`
`403`	`399`	`}`
`404`	`400`
	`401`	`+int QgsAttributeTableFilterModel::mapColumnFromSource( int column ) const`
	`402`	`+{`
	`403`	`+ if ( mColumnMapping.isEmpty() )`
	`404`	`+ return column;`
	`405`	`+ else`
	`406`	`+ return mColumnMapping.indexOf( column );`
	`407`	`+}`
	`408`	`+`
`405`	`409`	`void QgsAttributeTableFilterModel::generateListOfVisibleFeatures()`
`406`	`410`	`{`
`407`	`411`	`if ( !layer() )`
`@@ -528,7 +532,8 @@ QModelIndex QgsAttributeTableFilterModel::mapFromSource( const QModelIndex &sour`
`528`	`532`	`if ( proxyIndex.column() < 0 )`
`529`	`533`	`return QModelIndex();`
`530`	`534`
`531`		`- int col = mapColumnToSource( proxyIndex.column() );`
	`535`	`+ int col = mapColumnFromSource( proxyIndex.column() );`
	`536`	`+`
`532`	`537`	`if ( col == -1 )`
`533`	`538`	`col = 0;`
`534`	`539`
`@@ -544,4 +549,3 @@ Qt::ItemFlags QgsAttributeTableFilterModel::flags( const QModelIndex &index ) co`
`544`	`549`	`QModelIndex source_index = mapToSource( index );`
`545`	`550`	`return masterModel()->flags( source_index );`
`546`	`551`	`}`
`547`		`-`