From 8f043b2958477d3fe2ef094b7e42f792f4cf0b02 Mon Sep 17 00:00:00 2001 From: Akseli Lahtinen Date: Fri, 15 Dec 2023 13:07:12 +0000 Subject: [PATCH] Resort directory size count after refreshing After refreshing the view when size count is selected as the sortRole, count is 0 at first. When the actual count is loaded, the sorting is done according to the count being 0. This can break the sort order and cause view and model to be out of sync. Making sure we always resort all items when the directory size/item count is changed fixes this BUG:473999 --- src/kitemviews/kfileitemmodel.cpp | 7 ++ src/kitemviews/kfileitemmodel.h | 2 + src/kitemviews/kfileitemmodelrolesupdater.cpp | 3 + src/tests/kfileitemmodeltest.cpp | 103 ++++++++++++++++++ 4 files changed, 115 insertions(+) diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp index 4fd1ebd570..8de22318bb 100644 --- a/src/kitemviews/kfileitemmodel.cpp +++ b/src/kitemviews/kfileitemmodel.cpp @@ -260,6 +260,13 @@ void KFileItemModel::setShowTrashMime(bool show) } } +void KFileItemModel::scheduleResortAllItems() +{ + if (!m_resortAllItemsTimer->isActive()) { + m_resortAllItemsTimer->start(); + } +} + void KFileItemModel::setShowHiddenFiles(bool show) { m_dirLister->setShowHiddenFiles(show); diff --git a/src/kitemviews/kfileitemmodel.h b/src/kitemviews/kfileitemmodel.h index 3c2721d8ff..f87d2d5437 100644 --- a/src/kitemviews/kfileitemmodel.h +++ b/src/kitemviews/kfileitemmodel.h @@ -205,6 +205,8 @@ public: /** set to true to hide application/x-trash files */ void setShowTrashMime(bool show); + void scheduleResortAllItems(); + Q_SIGNALS: /** * Is emitted if the loading of a directory has been started. It is diff --git a/src/kitemviews/kfileitemmodelrolesupdater.cpp b/src/kitemviews/kfileitemmodelrolesupdater.cpp index e2d32265ef..070b958650 100644 --- a/src/kitemviews/kfileitemmodelrolesupdater.cpp +++ b/src/kitemviews/kfileitemmodelrolesupdater.cpp @@ -1346,6 +1346,9 @@ void KFileItemModelRolesUpdater::startDirectorySizeCounting(const KFileItem &ite disconnect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); m_model->setData(index, data); connect(m_model, &KFileItemModel::itemsChanged, this, &KFileItemModelRolesUpdater::slotItemsChanged); + if (m_model->sortRole() == "size") { + m_model->scheduleResortAllItems(); + } } }); return; diff --git a/src/tests/kfileitemmodeltest.cpp b/src/tests/kfileitemmodeltest.cpp index 16189a0fc6..973b9c08c7 100644 --- a/src/tests/kfileitemmodeltest.cpp +++ b/src/tests/kfileitemmodeltest.cpp @@ -92,6 +92,7 @@ private Q_SLOTS: void testDeleteFileMoreThanOnce(); void testInsertAfterExpand(); void testCurrentDirRemoved(); + void testSizeSortingAfterRefresh(); private: QStringList itemsInModel() const; @@ -846,6 +847,108 @@ void KFileItemModelTest::testRemoveFilteredExpandedItems() << "file"); } +void KFileItemModelTest::testSizeSortingAfterRefresh() +{ + // testDir structure is as follows + // ./ + // ├─ a + // ├─ b + // ├─ c/ + // │ ├─ c-2/ + // │ │ ├─ c-3 + // │ ├─ c-1 + // ├─ d + // ├─ e + + QSignalSpy itemsInsertedSpy(m_model, &KFileItemModel::itemsInserted); + QSignalSpy itemsMovedSpy(m_model, &KFileItemModel::itemsMoved); + QVERIFY(itemsMovedSpy.isValid()); + + // Create some files with different sizes and modification times to check the different sorting options + QDateTime now = QDateTime::currentDateTime(); + + QSet roles; + roles.insert("text"); + roles.insert("isExpanded"); + roles.insert("isExpandable"); + roles.insert("expandedParentsCount"); + m_model->setRoles(roles); + + m_testDir->createDir("c/c-2"); + m_testDir->createFile("c/c-2/c-3"); + m_testDir->createFile("c/c-1"); + + m_testDir->createFile("a", "A file", now.addDays(-3)); + m_testDir->createFile("b", "A larger file", now.addDays(0)); + m_testDir->createDir("c", now.addDays(-2)); + m_testDir->createFile("d", "The largest file in this directory", now.addDays(-1)); + m_testDir->createFile("e", "An even larger file", now.addDays(-4)); + + m_model->loadDirectory(m_testDir->url()); + QVERIFY(itemsInsertedSpy.wait()); + + int index = m_model->index(QUrl(m_testDir->url().url() + "/c")); + m_model->setExpanded(index, true); + QVERIFY(itemsInsertedSpy.wait()); + + index = m_model->index(QUrl(m_testDir->url().url() + "/c/c-2")); + m_model->setExpanded(index, true); + QVERIFY(itemsInsertedSpy.wait()); + + // Default: Sort by Name, ascending + QCOMPARE(m_model->sortRole(), QByteArray("text")); + QCOMPARE(m_model->sortOrder(), Qt::AscendingOrder); + QCOMPARE(itemsInModel(), + QStringList() << "c" + << "c-2" + << "c-3" + << "c-1" + << "a" + << "b" + << "d" + << "e"); + + // Sort by Size, ascending, before refresh + m_model->setSortRole("size"); + QCOMPARE(m_model->sortRole(), QByteArray("size")); + QCOMPARE(m_model->sortOrder(), Qt::AscendingOrder); + QCOMPARE(itemsInModel(), + QStringList() << "c" + << "c-2" + << "c-3" + << "c-1" + << "a" + << "b" + << "e" + << "d"); + + // Refresh directory + m_model->refreshDirectory(m_model->directory()); + QVERIFY(itemsInsertedSpy.wait()); + + // Expand folders again + index = m_model->index(QUrl(m_testDir->url().url() + "/c")); + m_model->setExpanded(index, true); + QVERIFY(itemsInsertedSpy.wait()); + + index = m_model->index(QUrl(m_testDir->url().url() + "/c/c-2")); + m_model->setExpanded(index, true); + QVERIFY(itemsInsertedSpy.wait()); + + // Sort by Size, ascending, after refresh + QCOMPARE(m_model->sortRole(), QByteArray("size")); + QCOMPARE(m_model->sortOrder(), Qt::AscendingOrder); + QCOMPARE(itemsInModel(), + QStringList() << "c" + << "c-2" + << "c-3" + << "c-1" + << "a" + << "b" + << "e" + << "d"); +} + void KFileItemModelTest::testSorting() { // testDir structure is as follows