From 14b828eaa9d88b0e5189cb21ccd0debccb1276f4 Mon Sep 17 00:00:00 2001 From: alex-z Date: Tue, 19 Sep 2023 15:50:29 +0200 Subject: [PATCH] Small improvement to activities sort function. Fixed unit tests. Signed-off-by: alex-z --- src/gui/tray/sortedactivitylistmodel.cpp | 49 ++++++++------ test/activitylistmodeltestutils.cpp | 19 ++++++ test/activitylistmodeltestutils.h | 3 +- test/testsortedactivitylistmodel.cpp | 85 +++++++++++++++++------- 4 files changed, 109 insertions(+), 47 deletions(-) diff --git a/src/gui/tray/sortedactivitylistmodel.cpp b/src/gui/tray/sortedactivitylistmodel.cpp index 36fd6ac83e9ee..fd8712cb8d2bf 100644 --- a/src/gui/tray/sortedactivitylistmodel.cpp +++ b/src/gui/tray/sortedactivitylistmodel.cpp @@ -23,6 +23,7 @@ namespace bool hasPOST = false; bool hasREPLY = false; bool hasWEB = false; + bool hasDELETE = false; }; ActivityLinksSearchResult searchForVerbsInLinks(const QVector &links) @@ -35,6 +36,8 @@ namespace result.hasREPLY = true; } else if (link._verb == QByteArrayLiteral("WEB")) { result.hasWEB = true; + } else if (link._verb == QByteArrayLiteral("DELETE")) { + result.hasDELETE = true; } } return result; @@ -69,6 +72,31 @@ bool SortedActivityListModel::lessThan(const QModelIndex &sourceLeft, const QMod return false; } + const auto leftActivityVerbsSearchResult = searchForVerbsInLinks(leftActivity._links); + const auto rightActivityVerbsSearchResult = searchForVerbsInLinks(rightActivity._links); + + if (leftActivityVerbsSearchResult.hasPOST != rightActivityVerbsSearchResult.hasPOST) { + return leftActivityVerbsSearchResult.hasPOST; + } + + if (leftActivityVerbsSearchResult.hasREPLY != rightActivityVerbsSearchResult.hasREPLY) { + return leftActivityVerbsSearchResult.hasREPLY; + } + + if (leftActivityVerbsSearchResult.hasWEB != rightActivityVerbsSearchResult.hasWEB) { + return leftActivityVerbsSearchResult.hasWEB; + } + + if (leftActivityVerbsSearchResult.hasDELETE != rightActivityVerbsSearchResult.hasDELETE) { + return leftActivityVerbsSearchResult.hasDELETE; + } + + const auto leftActivityIsSecurityAction = leftActivity._fileAction == QStringLiteral("security"); + const auto rightActivityIsSecurityAction = rightActivity._fileAction == QStringLiteral("security"); + if (leftActivityIsSecurityAction != rightActivityIsSecurityAction) { + return leftActivityIsSecurityAction; + } + // Let's now check for errors as we want those near the top too // Sync result errors go first const auto leftSyncResultStatus = leftActivity._syncResultStatus; @@ -99,27 +127,6 @@ bool SortedActivityListModel::lessThan(const QModelIndex &sourceLeft, const QMod return leftIsErrorFileItemStatus; } - const auto leftActivityVerbsSearchResult = searchForVerbsInLinks(leftActivity._links); - const auto rightActivityVerbsSearchResult = searchForVerbsInLinks(rightActivity._links); - - if (leftActivityVerbsSearchResult.hasPOST && !rightActivityVerbsSearchResult.hasPOST) { - return true; - } else if (!leftActivityVerbsSearchResult.hasPOST && rightActivityVerbsSearchResult.hasPOST) { - return false; - } - - if (leftActivityVerbsSearchResult.hasREPLY && !rightActivityVerbsSearchResult.hasREPLY) { - return true; - } else if (!leftActivityVerbsSearchResult.hasREPLY && rightActivityVerbsSearchResult.hasREPLY) { - return false; - } - - if (leftActivityVerbsSearchResult.hasWEB && !rightActivityVerbsSearchResult.hasWEB) { - return true; - } else if (!leftActivityVerbsSearchResult.hasWEB && rightActivityVerbsSearchResult.hasWEB) { - return false; - } - // Let's go back to more broadly comparing by type if (const auto rightType = rightActivity._type; leftType != rightType) { return leftType < rightType; diff --git a/test/activitylistmodeltestutils.cpp b/test/activitylistmodeltestutils.cpp index 4700a6b6d3e4d..e854383e818bb 100644 --- a/test/activitylistmodeltestutils.cpp +++ b/test/activitylistmodeltestutils.cpp @@ -387,6 +387,25 @@ void FakeRemoteActivityStorage::initActivityData() _startingId++; } + // Insert notification data + for (quint32 i = 0; i < _numItemsToInsert; i++) { + QJsonObject activity; + activity.insert(QStringLiteral("activity_id"), _startingId); + activity.insert(QStringLiteral("object_type"), QStringLiteral("")); + activity.insert(QStringLiteral("type"), QStringLiteral("security")); + activity.insert(QStringLiteral("subject"), QStringLiteral("You successfully logged in using two-factor authentication (Nextcloud Notification)")); + activity.insert(QStringLiteral("message"), QStringLiteral("")); + activity.insert(QStringLiteral("object_name"), QStringLiteral("")); + activity.insert(QStringLiteral("datetime"), QDateTime::currentDateTime().toString(Qt::ISODate)); + activity.insert(QStringLiteral("icon"), QStringLiteral("http://example.de/core/img/places/password.svg")); + + _activityData.push_back(activity); + + _startingId++; + } + + auto inserted = _activityData.size(); + _startingId--; } diff --git a/test/activitylistmodeltestutils.h b/test/activitylistmodeltestutils.h index 55310f197b925..b098ea7a40105 100644 --- a/test/activitylistmodeltestutils.h +++ b/test/activitylistmodeltestutils.h @@ -64,7 +64,8 @@ class FakeRemoteActivityStorage private: QJsonArray _activityData; QVariantMap _metaSuccess; - quint32 _numItemsToInsert = 30; + quint32 _numItemsToInsert = 10; + quint32 _numPriorityActivitiesInserted = 0; int _startingId = 90000; static FakeRemoteActivityStorage *_instance; diff --git a/test/testsortedactivitylistmodel.cpp b/test/testsortedactivitylistmodel.cpp index 90b8d74c8b6c8..a897b61011d23 100644 --- a/test/testsortedactivitylistmodel.cpp +++ b/test/testsortedactivitylistmodel.cpp @@ -153,7 +153,7 @@ private slots: sourceModel->startMaxActivitiesFetchJob(); QSignalSpy activitiesJob(sourceModel, &TestingALM::activitiesProcessed); QVERIFY(activitiesJob.wait(3000)); - QCOMPARE(sourceModel->rowCount(), sourceModel->maxPossibleActivities()); + QCOMPARE(sourceModel->rowCount(), FakeRemoteActivityStorage::instance()->totalNumActivites()); auto errorSyncFileItemActivity = exampleSyncFileItemActivity(accountState->account()->displayName(), {}); errorSyncFileItemActivity._message = QStringLiteral("Something went wrong and eveything exploded!"); @@ -165,38 +165,73 @@ private slots: addActivity(model, &TestingALM::addErrorToActivityList, testSyncResultErrorActivity, OCC::ActivityListModel::ErrorType::SyncError); addActivity(model, &TestingALM::addIgnoredFileToList, testFileIgnoredActivity); - const QVector activityDefaultTypeOrder { - OCC::Activity::DummyFetchingActivityType, - OCC::Activity::NotificationType, - OCC::Activity::SyncResultType, - OCC::Activity::SyncFileItemType, - OCC::Activity::ActivityType, - OCC::Activity::DummyMoreActivitiesAvailableType}; + // first let's go through priority activities (interactive ones and those with _fileAction == "security" + auto i = 0; + for (; i < model->rowCount(); ++i) { + const auto index = model->index(i, 0); + const auto activity = index.data(OCC::ActivityListModel::ActivityRole).value(); + + const auto foundIt = std::find_if(std::cbegin(activity._links), std::cend(activity._links), [](const auto &link) { + return link._verb == QByteArrayLiteral("POST") || link._verb == QByteArrayLiteral("REPLY") || link._verb == QByteArrayLiteral("WEB") + || link._verb == QByteArrayLiteral("DELETE"); + }); + const auto isInteractiveOrSecurityActivity = foundIt != std::cend(activity._links) || activity._fileAction == QStringLiteral("security"); + if (!isInteractiveOrSecurityActivity) { + break; + } + } + auto lasIndex = i; + + // now, let's check if activity is an error + for (; i < lasIndex + 1 && i < model->rowCount(); ++i) { + const auto index = model->index(i, 0); + const auto activity = index.data(OCC::ActivityListModel::ActivityRole).value(); + + QCOMPARE(activity._type, OCC::Activity::SyncResultType); + QCOMPARE(activity._syncResultStatus, OCC::SyncResult::Error); + } + lasIndex = i; + + // now, let's check if activity is a fatal error + for (; i < lasIndex + 1 && i < model->rowCount(); ++i) { + const auto index = model->index(i, 0); + const auto activity = index.data(OCC::ActivityListModel::ActivityRole).value(); + + QCOMPARE(activity._type, OCC::Activity::SyncFileItemType); + QCOMPARE(activity._syncFileItemStatus, OCC::SyncFileItem::FatalError); + } + lasIndex = i; + + // now, let's check if activity is an ignored file + for (; i < lasIndex + 1 && i < model->rowCount(); ++i) { + const auto index = model->index(i, 0); + const auto activity = index.data(OCC::ActivityListModel::ActivityRole).value(); + QCOMPARE(activity._type, OCC::Activity::SyncFileItemType); + QCOMPARE(activity._syncFileItemStatus, OCC::SyncFileItem::FileIgnored); + } + lasIndex = i; + + const QVector activityDefaultTypeOrder{OCC::Activity::DummyFetchingActivityType, + OCC::Activity::SyncResultType, + OCC::Activity::NotificationType, + OCC::Activity::SyncFileItemType, + OCC::Activity::ActivityType, + OCC::Activity::DummyMoreActivitiesAvailableType}; auto currentTypeSection = 1; auto previousType = activityDefaultTypeOrder[currentTypeSection]; - for (auto i = 0; i < model->rowCount(); ++i) { + // let's go through rest of activities (Now normal type order) + for (i; i < model->rowCount(); ++i) { const auto index = model->index(i, 0); const auto activity = index.data(OCC::ActivityListModel::ActivityRole).value(); qDebug() << i << activity._type << activity._subject << activity._message; - if (i == 0) { // Error syncresult activity should be at top - QCOMPARE(activity._type, OCC::Activity::SyncResultType); - QCOMPARE(activity._syncResultStatus, OCC::SyncResult::Error); - } else if (i == 1) { // Error syncfileitem activity should be next up - QCOMPARE(activity._type, OCC::Activity::SyncFileItemType); - QCOMPARE(activity._syncFileItemStatus, OCC::SyncFileItem::FatalError); - } else if (i == 2) { // Ignored file syncfileitem activity should be next up - QCOMPARE(activity._type, OCC::Activity::SyncFileItemType); - QCOMPARE(activity._syncFileItemStatus, OCC::SyncFileItem::FileIgnored); - } else { // Now normal type order - while (i != 3 && activity._type != previousType) { - ++currentTypeSection; - previousType = activityDefaultTypeOrder[currentTypeSection]; - } - - QCOMPARE(activity._type, activityDefaultTypeOrder[currentTypeSection]); + + while (activity._type != previousType) { + ++currentTypeSection; + previousType = activityDefaultTypeOrder[currentTypeSection]; } + QCOMPARE(activity._type, activityDefaultTypeOrder[currentTypeSection]); } } };