Correct table sorting for layout tables with limited number of rows

Fixes #35427
qgis · Apr 4, 2020 · c796861 · c796861
1 parent 85b2cf2
commit c796861
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 49 deletions.
diff --git a/src/core/layout/qgslayoutitemattributetable.cpp b/src/core/layout/qgslayoutitemattributetable.cpp
@@ -32,45 +32,6 @@
 #include "qgsgeometryengine.h"
 #include "qgsconditionalstyle.h"
 
-//QgsLayoutAttributeTableCompare
-
-///@cond PRIVATE
-
-/**
- * Helper class for sorting tables, takes into account sorting column and ascending / descending
-*/
-class CORE_EXPORT QgsLayoutAttributeTableCompare
-{
-  public:
-
-    /**
-     * Constructor for QgsLayoutAttributeTableCompare.
-     */
-    QgsLayoutAttributeTableCompare() = default;
-    bool operator()( const QVector< QPair< QVariant, QgsConditionalStyle > > &m1, const QVector< QPair< QVariant, QgsConditionalStyle > > &m2 )
-    {
-      return ( mAscending ? qgsVariantLessThan( m1[mCurrentSortColumn].first, m2[mCurrentSortColumn].first )
-               : qgsVariantGreaterThan( m1[mCurrentSortColumn].first, m2[mCurrentSortColumn].first ) );
-    }
-
-    /**
-     * Sets \a column number to sort by.
-     */
-    void setSortColumn( int column ) { mCurrentSortColumn = column; }
-
-    /**
-     * Sets sort order for column sorting
-     * Set \a ascending to true to sort in ascending order, false to sort in descending order
-     */
-    void setAscending( bool ascending ) { mAscending = ascending; }
-
-  private:
-    int mCurrentSortColumn = 0;
-    bool mAscending = true;
-};
-
-///@endcond
-
 //
 // QgsLayoutItemAttributeTable
 //
@@ -519,6 +480,12 @@ bool QgsLayoutItemAttributeTable::getTableContents( QgsLayoutTableContents &cont
     req.setFilterFid( atlasFeature.id() );
   }
 
+  QVector< QPair<int, bool> > sortColumns = sortAttributes();
+  for ( int i = sortColumns.size() - 1; i >= 0; --i )
+  {
+    req = req.addOrderBy( mColumns.at( sortColumns.at( i ).first )->attribute(), sortColumns.at( i ).second );
+  }
+
   QgsFeature f;
   int counter = 0;
   QgsFeatureIterator fit = layer->getFeatures( req );
@@ -628,16 +595,6 @@ bool QgsLayoutItemAttributeTable::getTableContents( QgsLayoutTableContents &cont
     ++counter;
   }
 
-  //sort the list, starting with the last attribute
-  QgsLayoutAttributeTableCompare c;
-  QVector< QPair<int, bool> > sortColumns = sortAttributes();
-  for ( int i = sortColumns.size() - 1; i >= 0; --i )
-  {
-    c.setSortColumn( sortColumns.at( i ).first );
-    c.setAscending( sortColumns.at( i ).second );
-    std::stable_sort( tempContents.begin(), tempContents.end(), c );
-  }
-
   // build final table contents
   contents.reserve( tempContents.size() );
   mConditionalStyles.reserve( tempContents.size() );

diff --git a/tests/src/core/testqgslayouttable.cpp b/tests/src/core/testqgslayouttable.cpp
@@ -79,6 +79,8 @@ class TestQgsLayoutTable : public QObject
     void conditionalFormatting(); //test rendering with conditional formatting
     void dataDefinedSource();
     void wrappedText();
+    void testBaseSort();
+    void testExpressionSort();
 
   private:
     QgsVectorLayer *mVectorLayer = nullptr;
@@ -1582,5 +1584,52 @@ void TestQgsLayoutTable::wrappedText()
   QVERIFY( !wrapText.endsWith( "\naliqua" ) );
 }
 
+
+void TestQgsLayoutTable::testBaseSort()
+{
+  QgsLayout l( QgsProject::instance() );
+  l.initializeDefaults();
+  QgsLayoutItemAttributeTable *table = new QgsLayoutItemAttributeTable( &l );
+  table->setVectorLayer( mVectorLayer );
+  table->setDisplayOnlyVisibleFeatures( false );
+  table->setMaximumNumberOfFeatures( 1 );
+  table->columns().at( 2 )->setSortByRank( 1 );
+  table->columns().at( 2 )->setSortOrder( Qt::DescendingOrder );
+  table->refresh();
+
+  QVector<QStringList> expectedRows;
+  QStringList row;
+  row << QStringLiteral( "Jet" ) << QStringLiteral( "100" ) << QStringLiteral( "20" ) << QStringLiteral( "3" ) << QStringLiteral( "0" ) << QStringLiteral( "3" );
+  expectedRows.append( row );
+  row.clear();
+
+  //retrieve rows and check
+  compareTable( table, expectedRows );
+}
+
+void TestQgsLayoutTable::testExpressionSort()
+{
+  QgsLayout l( QgsProject::instance() );
+  l.initializeDefaults();
+  QgsLayoutItemAttributeTable *table = new QgsLayoutItemAttributeTable( &l );
+  table->setVectorLayer( mVectorLayer );
+  table->setDisplayOnlyVisibleFeatures( false );
+  table->setMaximumNumberOfFeatures( 1 );
+  table->columns().at( 0 )->setAttribute( "Heading * -1" );
+  table->columns().at( 0 )->setHeading( "exp" );
+  table->columns().at( 0 )->setSortByRank( 1 );
+  table->columns().at( 0 )->setSortOrder( Qt::AscendingOrder );
+  table->refresh();
+
+  QVector<QStringList> expectedRows;
+  QStringList row;
+  row << QStringLiteral( "-340" ) << QStringLiteral( "340" ) << QStringLiteral( "1" ) << QStringLiteral( "3" ) << QStringLiteral( "3" ) << QStringLiteral( "6" );
+  expectedRows.append( row );
+  row.clear();
+
+  //retrieve rows and check
+  compareTable( table, expectedRows );
+}
+
 QGSTEST_MAIN( TestQgsLayoutTable )
 #include "testqgslayouttable.moc"