StatusQ(LeftJoinModel): Handling of layoutChanged from source models …

…fixed Closes: #13683
status-im · Feb 27, 2024 · 0497ecd · 0497ecd
1 parent 1639f1f
commit 0497ecd
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 9 deletions.
diff --git a/ui/StatusQ/include/StatusQ/leftjoinmodel.h b/ui/StatusQ/include/StatusQ/leftjoinmodel.h
@@ -60,4 +60,8 @@ class LeftJoinModel : public QAbstractListModel
     bool m_initialized = false;
 
     mutable QPersistentModelIndex m_lastUsedRightModelIndex;
+
+    // helpers for handling layoutChanged from source
+    QList<QPersistentModelIndex> m_layoutChangePersistentIndexes;
+    QModelIndexList m_proxyIndexes;
 };
diff --git a/ui/StatusQ/src/leftjoinmodel.cpp b/ui/StatusQ/src/leftjoinmodel.cpp
@@ -145,19 +145,39 @@ void LeftJoinModel::connectLeftModelSignals()
 
     connect(m_leftModel, &QAbstractItemModel::layoutAboutToBeChanged, this, [this]() {
         emit layoutAboutToBeChanged();
+
+        const auto persistentIndexes = persistentIndexList();
+
+        for (const QModelIndex& persistentIndex: persistentIndexes) {
+            m_proxyIndexes << persistentIndex;
+            Q_ASSERT(persistentIndex.isValid());
+            const auto srcIndex = m_leftModel->index(
+                        persistentIndex.row(),
+                        persistentIndex.column());
+
+            Q_ASSERT(srcIndex.isValid());
+            m_layoutChangePersistentIndexes << srcIndex;
+        }
     });
 
     connect(m_leftModel, &QAbstractItemModel::layoutChanged, this, [this]() {
+        for (int i = 0; i < m_proxyIndexes.size(); ++i) {
+            auto p = m_layoutChangePersistentIndexes.at(i);
+            changePersistentIndex(m_proxyIndexes.at(i), index(
+                                      p.row(), p.column(), p.parent()));
+        }
+
+        m_layoutChangePersistentIndexes.clear();
+        m_proxyIndexes.clear();
+
         emit layoutChanged();
     });
 
-    connect(m_leftModel, &QAbstractItemModel::modelAboutToBeReset, this, [this]() {
-        beginResetModel();
-    });
+    connect(m_leftModel, &QAbstractItemModel::modelAboutToBeReset, this,
+            &LeftJoinModel::beginResetModel);
 
-    connect(m_leftModel, &QAbstractItemModel::modelReset, this, [this]() {
-        endResetModel();
-    });
+    connect(m_leftModel, &QAbstractItemModel::modelReset, this,
+            &LeftJoinModel::endResetModel);
 }
 
 void LeftJoinModel::connectRightModelSignals()
@@ -188,8 +208,6 @@ void LeftJoinModel::connectRightModelSignals()
             emitJoinedRolesChanged);
     connect(m_rightModel, &QAbstractItemModel::modelReset, this,
             emitJoinedRolesChanged);
-    connect(m_rightModel, &QAbstractItemModel::layoutChanged, this,
-            emitJoinedRolesChanged);
 }
 
 QVariant LeftJoinModel::data(const QModelIndex& index, int role) const

diff --git a/ui/StatusQ/tests/src/TestHelpers/persistentindexestester.cpp b/ui/StatusQ/tests/src/TestHelpers/persistentindexestester.cpp
@@ -10,6 +10,8 @@ PersistentIndexesTester::PersistentIndexesTester(QAbstractItemModel* model)
     storeIndexesAndData();
 }
 
+PersistentIndexesTester::~PersistentIndexesTester() = default;
+
 void PersistentIndexesTester::storeIndexesAndData()
 {
     if (m_model == nullptr) {

diff --git a/ui/StatusQ/tests/src/TestHelpers/persistentindexestester.h b/ui/StatusQ/tests/src/TestHelpers/persistentindexestester.h
@@ -19,6 +19,7 @@ class PersistentIndexesTester
 {
 public:
     explicit PersistentIndexesTester(QAbstractItemModel* model);
+    ~PersistentIndexesTester();
 
     void storeIndexesAndData();
     bool compare();

diff --git a/ui/StatusQ/tests/tst_LeftJoinModel.cpp b/ui/StatusQ/tests/tst_LeftJoinModel.cpp
@@ -5,6 +5,8 @@
 #include <memory>
 
 #include <StatusQ/leftjoinmodel.h>
+
+#include <TestHelpers/persistentindexestester.h>
 #include <TestHelpers/testmodel.h>
 
 class TestLeftJoinModel: public QObject
@@ -387,7 +389,7 @@ private slots:
         }
     }
 
-    // TODO: cover also move and layoutChanged
+    // TODO: cover also move
     void insertRemovePropagationTest()
     {
         TestModel leftModel({
@@ -445,6 +447,63 @@ private slots:
         QCOMPARE(rowsRemovedSpy.first().at(2), 1);
     }
 
+    void layoutChangePropagationTest()
+    {
+        TestModel leftModel({
+           { "title", { "Token 1", "Token 2" }},
+           { "communityId", { "community_1", "community_2" }}
+        });
+
+        TestModel rightModel({
+           { "name", { "Community 1", "Community 2" }},
+           { "communityId", { "community_1", "community_2" }},
+           { "color", { "red", "green" }}
+        });
+
+        LeftJoinModel model;
+        QAbstractItemModelTester tester(&model);
+
+        model.setLeftModel(&leftModel);
+        model.setRightModel(&rightModel);
+        model.setJoinRole("communityId");
+
+        // register types to avoid warnings regarding signal params
+        qRegisterMetaType<QList<QPersistentModelIndex>>();
+        qRegisterMetaType<QAbstractItemModel::LayoutChangeHint>();
+
+        QSignalSpy layoutAboutToBeChangedSpy(
+                    &model, &LeftJoinModel::layoutAboutToBeChanged);
+        QSignalSpy layoutChangedSpy(&model, &LeftJoinModel::layoutChanged);
+        QSignalSpy dataChangedSpy(&model, &LeftJoinModel::dataChanged);
+
+        PersistentIndexesTester indexesTester(&model);
+        leftModel.invert();
+
+        QCOMPARE(layoutAboutToBeChangedSpy.count(), 1);
+        QCOMPARE(layoutChangedSpy.count(), 1);
+        QCOMPARE(dataChangedSpy.count(), 0);
+
+        QVERIFY(indexesTester.compare());
+
+        QCOMPARE(model.rowCount(), 2);
+        QCOMPARE(model.data(model.index(1, 0), 0), QString("Token 1"));
+        QCOMPARE(model.data(model.index(0, 0), 0), QString("Token 2"));
+        QCOMPARE(model.data(model.index(1, 0), 2), QString("Community 1"));
+        QCOMPARE(model.data(model.index(0, 0), 2), QString("Community 2"));
+
+        rightModel.invert();
+
+        QCOMPARE(layoutAboutToBeChangedSpy.count(), 1);
+        QCOMPARE(layoutChangedSpy.count(), 1);
+        QCOMPARE(dataChangedSpy.count(), 0);
+
+        QCOMPARE(model.data(model.index(1, 0), 0), QString("Token 1"));
+        QCOMPARE(model.data(model.index(0, 0), 0), QString("Token 2"));
+        QCOMPARE(model.data(model.index(1, 0), 2), QString("Community 1"));
+        QCOMPARE(model.data(model.index(0, 0), 2), QString("Community 2"));
+    }
+
+
     void rightModelJoinRoleChangesPropagationTest()
     {
         TestModel leftModel({