Small improvement to activities sort function. Fixed unit tests.

Signed-off-by: alex-z <[email protected]>
nextcloud · Sep 19, 2023 · 0381813 · 0381813
1 parent 2ab0f9f
commit 0381813
Show file tree

Hide file tree

Showing 4 changed files with 109 additions and 47 deletions.
diff --git 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<OCC::ActivityLink> &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
@@ -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
@@ -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
@@ -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 everything exploded!");
@@ -165,38 +165,73 @@ private slots:
         addActivity(model, &TestingALM::addErrorToActivityList, testSyncResultErrorActivity, OCC::ActivityListModel::ErrorType::SyncError);
         addActivity(model, &TestingALM::addIgnoredFileToList, testFileIgnoredActivity);
 
-        const QVector<OCC::Activity::Type> 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<OCC::Activity>();
+
+            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<OCC::Activity>();
+
+            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<OCC::Activity>();
+
+            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<OCC::Activity>();
+            QCOMPARE(activity._type, OCC::Activity::SyncFileItemType);
+            QCOMPARE(activity._syncFileItemStatus, OCC::SyncFileItem::FileIgnored);
+        }
+        lasIndex = i;
+
+        const QVector<OCC::Activity::Type> 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<OCC::Activity>();
 
             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]);
         }
     }
 };