Remove sorting code in activitylistmodel as not needed with sortedactivitylistmodel

Signed-off-by: Claudio Cambra <claudio.cambra@gmail.com>

src/gui/tray/activitylistmodel.cpp

@@ -464,7 +464,7 @@ void ActivityListModel::ingestActivities(const QJsonArray &activities)
     }
 
     if (list.size() > 0) {
-        addEntriesToActivityList(list, ActivityEntryType::ActivityType);
+        addEntriesToActivityList(list);
         appendMoreActivitiesAvailableEntry();
         _activityLists.append(list);
     }
@@ -488,7 +488,7 @@ void ActivityListModel::appendMoreActivitiesAvailableEntry()
             a._link = app->url();
         }
 
-        addEntriesToActivityList({a}, ActivityEntryType::MoreActivitiesAvailableType);
+        addEntriesToActivityList({a});
     }
 }
 
@@ -505,7 +505,7 @@ void ActivityListModel::insertOrRemoveDummyFetchingActivity()
         _dummyFetchingActivities._dateTime = QDateTime::currentDateTime();
         _dummyFetchingActivities._icon = QLatin1String("qrc:///client/theme/colored/change-bordered.svg");
 
-        addEntriesToActivityList({_dummyFetchingActivities}, ActivityEntryType::DummyFetchingActivityType);
+        addEntriesToActivityList({_dummyFetchingActivities});
     } else if (!_finalList.isEmpty() && _finalList.first()._objectType == dummyFetchingActivityObjectType) {
         removeActivityFromActivityList(_dummyFetchingActivities);
     }
@@ -530,157 +530,25 @@ void ActivityListModel::activitiesReceived(const QJsonDocument &json, int status
     emit activityJobStatusCode(statusCode);
 }
 
-std::pair<int, int> ActivityListModel::rowRangeForEntryType(const ActivityEntryType type) const
-{
-    // We want to present activities in a certain order, and we want to ensure entry types are grouped together.
-    // We therefore need to find the range of rows in the finalList that represent an entry type group.
-    int startRow = 0;
-
-    // We start from the type that we want to push down the furthest. Cascade through switch cases.
-    // Each time we fall through we add the count of elements in each of the sections that go above.
-    // So, items at the top of the activity list have a startRow of 1. The next section gets the count of that first
-    // section's elements added to startRow. Section 3 gets the count of Section 1 and Section 2 added to startRow.
-    // This goes on and on, with the last section getting startRow as the count of ALL section's element counts.
-
-    switch(type) {
-    case ActivityEntryType::MoreActivitiesAvailableType: // Always needs to be at the bottom
-        startRow = _finalList.count();
-        break;
-    case ActivityEntryType::ActivityType:
-        startRow += _syncFileItemLists.count();
-        Q_FALLTHROUGH();
-    case ActivityEntryType::SyncFileItemType:
-        startRow += _notificationLists.count();
-        Q_FALLTHROUGH();
-    case ActivityEntryType::NotificationType:
-        // We only show one activity for ignored files
-        if(_listOfIgnoredFiles.count() > 0) {
-            startRow += 1;
-        }
-        Q_FALLTHROUGH();
-    case ActivityEntryType::IgnoredFileType:
-        startRow += _notificationErrorsLists.count();
-        Q_FALLTHROUGH();
-    // Remaining types should go at top
-    case ActivityEntryType::ErrorType:
-    case ActivityEntryType::DummyFetchingActivityType:
-        break;
-    }
-
-    // To calculate the end row of the section, we just get the number of entries in the relevant section and then
-    // add it to the startRow.
-
-    int entryRowCount = -1;
-
-    switch(type) {
-    case ActivityEntryType::ActivityType:
-        entryRowCount = _activityLists.count();
-        break;
-    case ActivityEntryType::SyncFileItemType:
-        entryRowCount = _syncFileItemLists.count();
-        break;
-    case ActivityEntryType::NotificationType:
-        entryRowCount = _notificationLists.count();
-        break;
-    case ActivityEntryType::ErrorType:
-        entryRowCount = _notificationErrorsLists.count();
-        break;
-
-    // Single activity sections
-    case ActivityEntryType::IgnoredFileType:
-        if(_listOfIgnoredFiles.count() == 0) {
-            break;
-        }
-        Q_FALLTHROUGH();
-    case ActivityEntryType::MoreActivitiesAvailableType:
-        if(!_showMoreActivitiesAvailableEntry) {
-            break;
-        }
-        Q_FALLTHROUGH();
-    case ActivityEntryType::DummyFetchingActivityType:
-        if(_finalList.count() > 0 && _finalList.first() != _dummyFetchingActivities) {
-            break;
-        }
-
-        // All cascade down to here
-        entryRowCount = 1;
-        break;
-    }
-
-    // Even though we always return a startRow even if the section does not exist,
-    // we return -1 as endRow if the section does not exist.
-    // If we have a -1 we also know that the startRow is "theoretical", where the section
-    // "should" begin, not necessarily where it "does" begin
-    const auto endRow = entryRowCount > 0 ? startRow + entryRowCount - 1 : -1;
-
-    return {startRow, endRow};
-}
-
-// Make sure to add activities to their specific entry type lists AFTER adding them to the main list
-void ActivityListModel::addEntriesToActivityList(const ActivityList &activityList, const ActivityEntryType type)
+void ActivityListModel::addEntriesToActivityList(const ActivityList &activityList)
 {
     if(activityList.isEmpty()) {
         return;
     }
 
-    const auto entryTypeSectionRowRange = rowRangeForEntryType(type);
-
-    auto sortedList = activityList;
-    std::sort(sortedList.begin(), sortedList.end());
-
-    if(_finalList.count() == 0 || entryTypeSectionRowRange.second == -1) {
-        // If the finalList is empty or there are no entries belonging to the entry type section, we don't
-        // need to bother with inserting in a correct order and can more quickly just insert all activities.
-        const auto startRow = entryTypeSectionRowRange.first;
-        const auto endRow = startRow + sortedList.count() - 1;
+    const auto startRow = _finalList.count();
 
-        beginInsertRows({}, startRow, endRow);
-        int i = startRow;
-        for(const auto &activity : sortedList) {
-            _finalList.insert(i, activity);
-            ++i;
-        }
-        endInsertRows();
-        return;
+    beginInsertRows({}, startRow, startRow + activityList.count() - 1);
+    for(const auto &activity : activityList) {
+        _finalList.append(activity);
     }
-
-    // If the finalList is not empty and the entry type's section actually exists (i.e. there is at least
-    // one entry belonging to this entry in the finalList) then we are going to add them granularly.
-    // We make sure to insert the item in a specific place so as to preserve the sort order.
-    int sectionRowEnd = entryTypeSectionRowRange.second;
-
-    const auto insertRow = [&](const int row, const Activity &activity) {
-        beginInsertRows({}, row, row);
-        _finalList.insert(row, activity);
-        endInsertRows();
-        ++sectionRowEnd;
-    };
-
-    for(const auto &activity : sortedList) {
-        auto currentRow = entryTypeSectionRowRange.first;
-
-        while(currentRow <= sectionRowEnd) {
-            if(currentRow == sectionRowEnd) {
-                insertRow(currentRow + 1, activity);
-                break;
-            }
-
-            if(activity < _finalList[currentRow]) {
-                insertRow(currentRow, activity);
-                break;
-            }
-
-            ++currentRow;
-        }
-    }
-
-    return;
+    endInsertRows();
 }
 
 void ActivityListModel::addErrorToActivityList(const Activity &activity)
 {
     qCInfo(lcActivity) << "Error successfully added to the notification list: " << activity._subject;
-    addEntriesToActivityList({activity}, ActivityEntryType::ErrorType);
+    addEntriesToActivityList({activity});
     _notificationErrorsLists.prepend(activity);
 }
 
@@ -692,7 +560,7 @@ void ActivityListModel::addIgnoredFileToList(const Activity &newActivity)
     if (_listOfIgnoredFiles.size() == 0) {
         _notificationIgnoredFiles = newActivity;
         _notificationIgnoredFiles._subject = tr("Files from the ignore list as well as symbolic links are not synced.");
-        addEntriesToActivityList({_notificationIgnoredFiles}, ActivityEntryType::IgnoredFileType);
+        addEntriesToActivityList({_notificationIgnoredFiles});
         _listOfIgnoredFiles.append(newActivity);
         return;
     }
@@ -712,14 +580,14 @@ void ActivityListModel::addIgnoredFileToList(const Activity &newActivity)
 void ActivityListModel::addNotificationToActivityList(const Activity &activity)
 {
     qCInfo(lcActivity) << "Notification successfully added to the notification list: " << activity._subject;
-    addEntriesToActivityList({activity}, ActivityEntryType::NotificationType);
+    addEntriesToActivityList({activity});
     _notificationLists.prepend(activity);
 }
 
 void ActivityListModel::addSyncFileItemToActivityList(const Activity &activity)
 {
     qCInfo(lcActivity) << "Successfully added to the activity list: " << activity._subject;
-    addEntriesToActivityList({activity}, ActivityEntryType::SyncFileItemType);
+    addEntriesToActivityList({activity});
     _syncFileItemLists.prepend(activity);
 }
 
@@ -744,16 +612,11 @@ void ActivityListModel::removeActivityFromActivityList(const Activity &activity)
         endRemoveRows();
     }
 
-    if (activity._type == Activity::ActivityType) {
-        const auto activityListIndex = _activityLists.indexOf(activity);
-        if (activityListIndex != -1) {
-            _activityLists.removeAt(activityListIndex);
-        }
-    } else if (activity._type == Activity::NotificationType) {
-        const auto notificationListIndex = _notificationLists.indexOf(activity);
-        if (notificationListIndex != -1)
-            _notificationLists.removeAt(notificationListIndex);
-    } else {
+    if (activity._type != Activity::ActivityType &&
+            activity._type != Activity::DummyFetchingActivityType &&
+            activity._type != Activity::DummyMoreActivitiesAvailableType &&
+            activity._type != Activity::NotificationType) {
+
         const auto notificationErrorsListIndex = _notificationErrorsLists.indexOf(activity);
         if (notificationErrorsListIndex != -1)
             _notificationErrorsLists.removeAt(notificationErrorsListIndex);

src/gui/tray/activitylistmodel.h

@@ -78,17 +78,6 @@ public:
     };
     Q_ENUM(DataRole)
 
-    enum class ActivityEntryType {
-        DummyFetchingActivityType,
-        ActivityType,
-        NotificationType,
-        ErrorType,
-        IgnoredFileType,
-        SyncFileItemType,
-        MoreActivitiesAvailableType,
-    };
-    Q_ENUM(ActivityEntryType);
-
     explicit ActivityListModel(QObject *parent = nullptr);
 
     explicit ActivityListModel(AccountState *accountState,
@@ -154,14 +143,14 @@ protected slots:
 
     virtual void startFetchJob();
 
+private slots:
+    void addEntriesToActivityList(const ActivityList &activityList);
+
 private:
     static QVariantList convertLinksToMenuEntries(const Activity &activity);
     static QVariantList convertLinksToActionButtons(const Activity &activity);
     static QVariant convertLinkToActionButton(const ActivityLink &activityLink);
 
-    std::pair<int, int> rowRangeForEntryType(const ActivityEntryType type) const;
-    void addEntriesToActivityList(const ActivityList &activityList, const ActivityEntryType type);
-    void clearEntriesInActivityList(ActivityEntryType type);
     bool canFetchActivities() const;
 
     void ingestActivities(const QJsonArray &activities);

test/testactivitylistmodel.cpp

@@ -481,7 +481,7 @@ private slots:
 
         testSyncResultErrorActivity._id = 2;
         testSyncResultErrorActivity._type = OCC::Activity::SyncResultType;
-        testSyncResultErrorActivity._status = OCC::SyncResult::Error;
+        testSyncResultErrorActivity._syncResultStatus = OCC::SyncResult::Error;
         testSyncResultErrorActivity._dateTime = QDateTime::currentDateTime();
         testSyncResultErrorActivity._subject = QStringLiteral("Sample failed sync text");
         testSyncResultErrorActivity._message = QStringLiteral("/path/to/thingy");
@@ -490,7 +490,7 @@ private slots:
 
         testSyncFileItemActivity._id = 3;
         testSyncFileItemActivity._type = OCC::Activity::SyncFileItemType; //client activity
-        testSyncFileItemActivity._status = OCC::SyncFileItem::Success;
+        testSyncFileItemActivity._syncFileItemStatus = OCC::SyncFileItem::Success;
         testSyncFileItemActivity._dateTime = QDateTime::currentDateTime();
         testSyncFileItemActivity._message = QStringLiteral("Sample file successfully synced text");
         testSyncFileItemActivity._link = accountState->account()->url();
@@ -499,7 +499,7 @@ private slots:
 
         testFileIgnoredActivity._id = 4;
         testFileIgnoredActivity._type = OCC::Activity::SyncFileItemType;
-        testFileIgnoredActivity._status = OCC::SyncFileItem::FileIgnored;
+        testFileIgnoredActivity._syncFileItemStatus = OCC::SyncFileItem::FileIgnored;
         testFileIgnoredActivity._dateTime = QDateTime::currentDateTime();
         testFileIgnoredActivity._subject = QStringLiteral("Sample ignored file sync text");
         testFileIgnoredActivity._link = accountState->account()->url();
@@ -599,50 +599,15 @@ private slots:
         model->addNotificationToActivityList(testNotificationActivity);
         QCOMPARE(model->rowCount(), 54);
 
-        const auto desiredOrder = QVector<OCC::ActivityListModel::ActivityEntryType>{
-                OCC::ActivityListModel::ActivityEntryType::ErrorType,
-                OCC::ActivityListModel::ActivityEntryType::IgnoredFileType,
-                OCC::ActivityListModel::ActivityEntryType::NotificationType,
-                OCC::ActivityListModel::ActivityEntryType::SyncFileItemType,
-                OCC::ActivityListModel::ActivityEntryType::ActivityType};
-
         // Test all rows for things in common
         for (int i = 0; i < model->rowCount(); i++) {
             const auto index = model->index(i, 0);
 
-            int expectedEntryType = qMin(i, desiredOrder.count() - 1);
-            const auto activity = index.data(OCC::ActivityListModel::ActivityRole).value<OCC::Activity>();
-
-            // Make sure the model has sorted our activities in the right order
-            switch(desiredOrder[expectedEntryType]) {
-            case OCC::ActivityListModel::ActivityEntryType::DummyFetchingActivityType:
-                break;
-            case OCC::ActivityListModel::ActivityEntryType::ErrorType:
-                QCOMPARE(activity._type, OCC::Activity::SyncResultType);
-                QCOMPARE(activity._status, OCC::SyncResult::Error);
-                break;
-            case OCC::ActivityListModel::ActivityEntryType::IgnoredFileType:
-                QCOMPARE(activity._type, OCC::Activity::SyncFileItemType);
-                QCOMPARE(activity._status, OCC::SyncFileItem::FileIgnored);
-                break;
-            case OCC::ActivityListModel::ActivityEntryType::NotificationType:
-                QCOMPARE(activity._type, OCC::Activity::NotificationType);
-                break;
-            case OCC::ActivityListModel::ActivityEntryType::SyncFileItemType:
-                QCOMPARE(activity._type, OCC::Activity::SyncFileItemType);
-                QCOMPARE(activity._status, OCC::SyncFileItem::Success);
-                break;
-            case OCC::ActivityListModel::ActivityEntryType::ActivityType:
-                QCOMPARE(activity._type, OCC::Activity::ActivityType);
-            case OCC::ActivityListModel::ActivityEntryType::MoreActivitiesAvailableType:
-                break;
-            }
-
             auto text = index.data(OCC::ActivityListModel::ActionTextRole).toString();
 
             QVERIFY(index.data(OCC::ActivityListModel::ActionRole).canConvert<int>());
             const auto type = index.data(OCC::ActivityListModel::ActionRole).toInt();
-            QVERIFY(type >= OCC::Activity::ActivityType);
+            QVERIFY(type >= OCC::Activity::DummyFetchingActivityType);
 
             QVERIFY(!index.data(OCC::ActivityListModel::AccountRole).toString().isEmpty());
             QVERIFY(!index.data(OCC::ActivityListModel::ActionTextColorRole).toString().isEmpty());
@@ -664,6 +629,8 @@ private slots:
             QVERIFY(index.data(OCC::ActivityListModel::TalkNotificationMessageIdRole).canConvert<QString>());
             QVERIFY(index.data(OCC::ActivityListModel::TalkNotificationMessageSentRole).canConvert<QString>());
 
+            QVERIFY(index.data(OCC::ActivityListModel::ActivityRole).canConvert<OCC::Activity>());
+
             // Unfortunately, trying to check anything relating to filepaths causes a crash
             // when the folder manager is invoked by the model to look for the relevant file
         }