fix(StatusQ/Aggregator): Proper recalculation on layout change removing rows

Closes: #14554

ui/StatusQ/include/StatusQ/aggregator.h

@@ -35,5 +35,7 @@ private:
     QAbstractItemModel* m_model = nullptr;
     QVariant m_value;
 
+    int m_onLayoutToBeChangedCount = 0;
+
     void connectToModel();
 };

ui/StatusQ/src/aggregator.cpp

@@ -1,16 +1,19 @@
 #include "StatusQ/aggregator.h"
 #include <QAbstractItemModel>
 
-Aggregator::Aggregator(QObject *parent)
-    : QObject(parent){
+Aggregator::Aggregator(QObject* parent)
+    : QObject(parent)
+{
     connect(this, &Aggregator::modelChanged, this, &Aggregator::recalculate);
 }
 
-QAbstractItemModel* Aggregator::model() const {
+QAbstractItemModel* Aggregator::model() const
+{
     return m_model;
 }
 
-void Aggregator::setModel(QAbstractItemModel* model) {
+void Aggregator::setModel(QAbstractItemModel* model)
+{
     if(m_model == model)
         return;
 
@@ -22,7 +25,8 @@ void Aggregator::setModel(QAbstractItemModel* model) {
     emit modelChanged();
 }
 
-QVariant Aggregator::value() const {
+QVariant Aggregator::value() const
+{
     return m_value;
 }
 
@@ -37,15 +41,38 @@ void Aggregator::recalculate()
     emit valueChanged();
 }
 
-void Aggregator:: connectToModel() {
-    if(m_model) {
-        connect(m_model, &QAbstractItemModel::rowsInserted, this, &Aggregator::recalculate);
-        connect(m_model, &QAbstractItemModel::rowsRemoved, this, &Aggregator::recalculate);
-        connect(m_model, &QAbstractItemModel::modelReset, this, &Aggregator::recalculate);
-        connect(m_model, &QAbstractItemModel::dataChanged, this,
-              [this](auto&, auto&, const QVector<int>& roles) {
-          if (this->acceptRoles(roles))
-              this->recalculate();
-        });
-    }
+void Aggregator:: connectToModel()
+{
+    if(m_model == nullptr)
+        return;
+
+    connect(m_model, &QAbstractItemModel::rowsInserted, this, &Aggregator::recalculate);
+    connect(m_model, &QAbstractItemModel::rowsRemoved, this, &Aggregator::recalculate);
+    connect(m_model, &QAbstractItemModel::modelReset, this, &Aggregator::recalculate);
+
+    // Some models (like SFPM) emit layoutAboutToBeChanged/layoutChanged while
+    // removing some rows in the meantime. If the row count is changed, then
+    // aggregation must be recalculated. In most cases, for regular layout
+    // change events it's not necessary to recalculate.
+    connect(m_model, &QAbstractItemModel::layoutAboutToBeChanged, this, [this] {
+        auto model = this->model();
+
+        if (model == nullptr)
+            return;
+
+        m_onLayoutToBeChangedCount = model->rowCount();
+    });
+
+    connect(m_model, &QAbstractItemModel::layoutChanged, this, [this] {
+        auto model = this->model();
+
+        if (model != nullptr && m_onLayoutToBeChangedCount != model->rowCount())
+            this->recalculate();
+    });
+
+    connect(m_model, &QAbstractItemModel::dataChanged, this,
+          [this](auto&, auto&, const QVector<int>& roles) {
+      if (this->acceptRoles(roles))
+          this->recalculate();
+    });
 }

ui/StatusQ/tests/src/TestHelpers/testmodel.cpp

@@ -128,6 +128,12 @@ void TestModel::removeEverySecond()
     emit layoutChanged();
 }
 
+void TestModel::reset()
+{
+    beginResetModel();
+    endResetModel();
+}
+
 void TestModel::initRoles()
 {
     m_roles.reserve(m_data.size());

ui/StatusQ/tests/src/TestHelpers/testmodel.h

@@ -26,6 +26,9 @@ public:
     // during SFPM initialization where initial filtering is notified this way.
     void removeEverySecond();
 
+    // emits modelAboutToBeReset/modelReset, content remains the same
+    void reset();
+
 private:
     void initRoles();
 

ui/StatusQ/tests/tst_Aggregator.cpp

@@ -5,19 +5,26 @@
 
 namespace {
 
-class ChildAggregator : public Aggregator {
- Q_OBJECT
+class ChildAggregator : public Aggregator
+{
+    Q_OBJECT
 
 public:
-    explicit ChildAggregator(QObject *parent = nullptr) {}
+    explicit ChildAggregator(QObject* parent = nullptr) {}
 
 protected slots:
-    QVariant calculateAggregation() override {
-        return {counter++};
+    QVariant calculateAggregation() override
+    {
+        return {m_counter++};
+    }
+
+    bool acceptRoles(const QVector<int>& roles) override
+    {
+        return roles.contains(model()->roleNames().key("balance", -1));
     }
 
 private:
-    int counter = 0;
+    int m_counter = 0;
 };
 
 } // anonymous namespace
@@ -27,11 +34,14 @@ class TestAggregator : public QObject
     Q_OBJECT
 
 private:
-    QString m_roleNameWarningText = "Provided role name does not exist in the current model";
-    QString m_unsuportedTypeWarningText = "Unsupported type for given role (not convertible to double)";
+    static constexpr auto s_roleNameWarningText
+        = "Provided role name does not exist in the current model";
+    static constexpr auto s_unsuportedTypeWarningText
+        = "Unsupported type for given role (not convertible to double)";
 
 private slots:
-    void testModel() {
+    void testModel()
+    {
         ChildAggregator aggregator;
         TestModel sourceModel({
             { "chainId", { "12", "13", "1", "321" }},
@@ -53,7 +63,8 @@ private slots:
         QCOMPARE(valueChangedSpy.count(), 2);
     }
 
-    void testCalculateAggregationTrigger() {
+    void testCalculateAggregationTrigger()
+    {
         ChildAggregator aggregator;
         TestModel sourceModel({
             { "chainId", { "12", "13", "1", "321" }},
@@ -72,10 +83,28 @@ private slots:
         valueChangedSpyCount++;
         QCOMPARE(valueChangedSpy.count(), valueChangedSpyCount);
 
-        // Test 3 - Update value row:
+        // Test 3 - Update value row of accepted role:
         sourceModel.update(2, 1, 26.45);
         valueChangedSpyCount++;
         QCOMPARE(valueChangedSpy.count(), valueChangedSpyCount);
+
+        // Test 4 - Update value row of not accepted role:
+        sourceModel.update(2, 0, "3");
+        QCOMPARE(valueChangedSpy.count(), valueChangedSpyCount);
+
+        // Test 5 - Layout change, no removals:
+        sourceModel.invert();
+        QCOMPARE(valueChangedSpy.count(), valueChangedSpyCount);
+
+        // Test 6 - Layout change, with removing rows:
+        sourceModel.removeEverySecond();
+        valueChangedSpyCount++;
+        QCOMPARE(valueChangedSpy.count(), valueChangedSpyCount);
+
+        // Test 7 - Model reset:
+        sourceModel.reset();
+        valueChangedSpyCount++;
+        QCOMPARE(valueChangedSpy.count(), valueChangedSpyCount);
     }
 };