Refactor ActivityListModel population mechanisms

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

src/gui/fileactivitylistmodel.h

@@ -29,7 +29,7 @@ public:
 public slots:
     void load(AccountState *accountState, const int objectId);
 
-protected:
+protected slots:
     void startFetchJob() override;
 
 private:

src/gui/tray/activitydata.cpp

@@ -25,11 +25,21 @@ bool operator<(const Activity &rhs, const Activity &lhs)
     return rhs._dateTime > lhs._dateTime;
 }
 
+bool operator>(const Activity &rhs, const Activity &lhs)
+{
+    return rhs._dateTime < lhs._dateTime;
+}
+
 bool operator==(const Activity &rhs, const Activity &lhs)
 {
     return (rhs._type == lhs._type && rhs._id == lhs._id && rhs._accName == lhs._accName);
 }
 
+bool operator!=(const Activity &rhs, const Activity &lhs)
+{
+    return !(rhs == lhs);
+}
+
 Activity::Identifier Activity::ident() const
 {
     return Identifier(_id, _accName);

src/gui/tray/activitydata.h

@@ -158,7 +158,9 @@ public:
 };
 
 bool operator==(const Activity &rhs, const Activity &lhs);
+bool operator!=(const Activity &rhs, const Activity &lhs);
 bool operator<(const Activity &rhs, const Activity &lhs);
+bool operator>(const Activity &rhs, const Activity &lhs);
 
 /* ==================================================================== */
 /**
@@ -170,6 +172,7 @@ bool operator<(const Activity &rhs, const Activity &lhs);
 using ActivityList = QList<Activity>;
 }
 
+Q_DECLARE_METATYPE(OCC::Activity)
 Q_DECLARE_METATYPE(OCC::Activity::Type)
 Q_DECLARE_METATYPE(OCC::ActivityLink)
 Q_DECLARE_METATYPE(OCC::PreviewData)

src/gui/tray/activitylistmodel.cpp

@@ -1,4 +1,4 @@
-/*
+/*
  * Copyright (C) by Klaas Freitag <freitag@owncloud.com>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -84,6 +84,7 @@ QHash<int, QByteArray> ActivityListModel::roleNames() const
     roles[TalkNotificationMessageIdRole] = "messageId";
     roles[TalkNotificationMessageSentRole] = "messageSent";
     roles[TalkNotificationUserAvatarRole] = "userAvatar";
+    roles[ActivityRole] = "activity";
 
     return roles;
 }
@@ -280,7 +281,7 @@ QVariant ActivityListModel::data(const QModelIndex &index, int role) const
     case ActionsLinksContextMenuRole: {
         return ActivityListModel::convertLinksToMenuEntries(a);
     }
-    
+
     case ActionsLinksForActionButtonsRole: {
         return ActivityListModel::convertLinksToActionButtons(a);
     }
@@ -358,9 +359,10 @@ QVariant ActivityListModel::data(const QModelIndex &index, int role) const
         return replyMessageSent(a);
     case TalkNotificationUserAvatarRole:
         return a._talkNotificationData.userAvatar;
-    default:
-        return QVariant();
+    case ActivityRole:
+        return QVariant::fromValue(a);
     }
+
     return QVariant();
 }
 
@@ -433,11 +435,15 @@ void ActivityListModel::ingestActivities(const QJsonArray &activities)
 
         auto a = Activity::fromActivityJson(json, _accountState->account());
 
+        if(_presentedActivities.contains(a._id)) {
+            continue;
+        }
+
         list.append(a);
+        _presentedActivities.insert(a._id);
         _currentItem = list.last()._id;
 
-        _totalActivitiesFetched++;
-        if (_totalActivitiesFetched == _maxActivities
+        if (_presentedActivities.count() >= _maxActivities
             || (_hideOldActivities && a._dateTime < oldestDate)) {
             _showMoreActivitiesAvailableEntry = true;
             _doneFetching = true;
@@ -445,15 +451,10 @@ void ActivityListModel::ingestActivities(const QJsonArray &activities)
         }
     }
 
-    _activityLists.append(list);
-
     if (list.size() > 0) {
-        std::sort(list.begin(), list.end());
-        beginInsertRows({}, _finalList.size(), _finalList.size() + list.size() - 1);
-        _finalList.append(list);
-        endInsertRows();
-
+        addEntriesToActivityList(list, ActivityEntryType::ActivityType);
         appendMoreActivitiesAvailableEntry();
+        _activityLists.append(list);
     }
 }
 
@@ -462,6 +463,7 @@ void ActivityListModel::appendMoreActivitiesAvailableEntry()
     const QString moreActivitiesEntryObjectType = QLatin1String("activity_fetch_more_activities");
     if (_showMoreActivitiesAvailableEntry && !_finalList.isEmpty()
         && _finalList.last()._objectType != moreActivitiesEntryObjectType) {
+
         Activity a;
         a._type = Activity::ActivityType;
         a._accName = _accountState->account()->displayName();
@@ -474,56 +476,26 @@ void ActivityListModel::appendMoreActivitiesAvailableEntry()
             a._link = app->url();
         }
 
-        beginInsertRows({}, _finalList.size(), _finalList.size());
-        _finalList.append(a);
-        endInsertRows();
+        addEntriesToActivityList({a}, ActivityEntryType::MoreActivitiesAvailableType);
     }
 }
 
 void ActivityListModel::insertOrRemoveDummyFetchingActivity()
 {
     const QString dummyFetchingActivityObjectType = QLatin1String("dummy_fetching_activity");
-    if (_currentlyFetching && _finalList.isEmpty()) {
-        Activity a;
-        a._type = Activity::ActivityType;
-        a._accName = _accountState->account()->displayName();
-        a._id = -2;
-        a._objectType = dummyFetchingActivityObjectType;
-        a._subject = tr("Fetching activities …");
-        a._dateTime = QDateTime::currentDateTime();
-        a._icon = QLatin1String("qrc:///client/theme/colored/change-bordered.svg");
 
-        beginInsertRows({}, 0, 0);
-        _finalList.prepend(a);
-        endInsertRows();
+    if (_currentlyFetching && _finalList.isEmpty()) {
+        _dummyFetchingActivities._type = Activity::ActivityType;
+        _dummyFetchingActivities._accName = _accountState->account()->displayName();
+        _dummyFetchingActivities._id = -2;
+        _dummyFetchingActivities._objectType = dummyFetchingActivityObjectType;
+        _dummyFetchingActivities._subject = tr("Fetching activities …");
+        _dummyFetchingActivities._dateTime = QDateTime::currentDateTime();
+        _dummyFetchingActivities._icon = QLatin1String("qrc:///client/theme/colored/change-bordered.svg");
+
+        addEntriesToActivityList({_dummyFetchingActivities}, ActivityEntryType::DummyFetchingActivityType);
     } else if (!_finalList.isEmpty() && _finalList.first()._objectType == dummyFetchingActivityObjectType) {
-        beginRemoveRows({}, 0, 0);
-        _finalList.removeAt(0);
-        endRemoveRows();
-    }
-}
-
-void ActivityListModel::clearActivities()
-{
-    _activityLists.clear();
-    if (!_finalList.isEmpty()) {
-        const auto firstActivityIt = std::find_if(std::begin(_finalList), std::end(_finalList),
-            [&](const Activity &activity) { return activity._type == Activity::ActivityType; });
-
-        if (firstActivityIt != std::end(_finalList)) {
-            const auto lastActivityItReverse = std::find_if(std::rbegin(_finalList), std::rend(_finalList),
-                    [&](const Activity &activity) { return activity._type == Activity::ActivityType; });
-
-            const auto lastActivityIt = (lastActivityItReverse + 1).base();
-
-            if (lastActivityIt != std::end(_finalList)) {
-                const int beginRemoveIndex = std::distance(std::begin(_finalList), firstActivityIt);
-                const int endRemoveIndex = std::distance(std::begin(_finalList), lastActivityIt);
-                beginRemoveRows({}, beginRemoveIndex, endRemoveIndex);
-                _finalList.erase(firstActivityIt, std::end(_finalList));
-                endRemoveRows();
-            }
-        }
+        removeActivityFromActivityList(_dummyFetchingActivities);
     }
 }
 
@@ -546,14 +518,161 @@ void ActivityListModel::activitiesReceived(const QJsonDocument &json, int status
     emit activityJobStatusCode(statusCode);
 }
 
-void ActivityListModel::addErrorToActivityList(Activity activity)
+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)
+{
+    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;
+
+        beginInsertRows({}, startRow, endRow);
+        int i = startRow;
+        for(const auto &activity : sortedList) {
+            _finalList.insert(i, activity);
+            ++i;
+        }
+        endInsertRows();
+        return;
+    }
+
+    // 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;
+}
+
+void ActivityListModel::addErrorToActivityList(const Activity &activity)
 {
     qCInfo(lcActivity) << "Error successfully added to the notification list: " << activity._subject;
+    addEntriesToActivityList({activity}, ActivityEntryType::ErrorType);
     _notificationErrorsLists.prepend(activity);
-    combineActivityLists();
 }
 
-void ActivityListModel::addIgnoredFileToList(Activity newActivity)
+void ActivityListModel::addIgnoredFileToList(const Activity &newActivity)
 {
     qCInfo(lcActivity) << "First checking for duplicates then add file to the notification list of ignored files: " << newActivity._file;
 
@@ -561,6 +680,7 @@ void ActivityListModel::addIgnoredFileToList(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);
         _listOfIgnoredFiles.append(newActivity);
         return;
     }
@@ -577,65 +697,54 @@ void ActivityListModel::addIgnoredFileToList(Activity newActivity)
     }
 }
 
-void ActivityListModel::addNotificationToActivityList(Activity activity)
+void ActivityListModel::addNotificationToActivityList(const Activity &activity)
 {
     qCInfo(lcActivity) << "Notification successfully added to the notification list: " << activity._subject;
+    addEntriesToActivityList({activity}, ActivityEntryType::NotificationType);
     _notificationLists.prepend(activity);
-    combineActivityLists();
 }
 
-void ActivityListModel::clearNotifications()
+void ActivityListModel::addSyncFileItemToActivityList(const Activity &activity)
 {
-    qCInfo(lcActivity) << "Clear the notifications";
-    _notificationLists.clear();
-    combineActivityLists();
+    qCInfo(lcActivity) << "Successfully added to the activity list: " << activity._subject;
+    addEntriesToActivityList({activity}, ActivityEntryType::SyncFileItemType);
+    _syncFileItemLists.prepend(activity);
 }
 
 void ActivityListModel::removeActivityFromActivityList(int row)
 {
     Activity activity = _finalList.at(row);
     removeActivityFromActivityList(activity);
-    combineActivityLists();
-}
-
-void ActivityListModel::addSyncFileItemToActivityList(Activity activity)
-{
-    qCInfo(lcActivity) << "Successfully added to the activity list: " << activity._subject;
-    _syncFileItemLists.prepend(activity);
-    combineActivityLists();
 }
 
-void ActivityListModel::removeActivityFromActivityList(Activity activity)
+void ActivityListModel::removeActivityFromActivityList(const Activity &activity)
 {
     qCInfo(lcActivity) << "Activity/Notification/Error successfully dismissed: " << activity._subject;
     qCInfo(lcActivity) << "Trying to remove Activity/Notification/Error from view... ";
 
-    int index = -1;
+    const auto index = _finalList.indexOf(activity);
+    if (index != -1) {
+        qCInfo(lcActivity) << "Activity/Notification/Error successfully removed from the list.";
+        qCInfo(lcActivity) << "Updating Activity/Notification/Error view.";
+
+        beginRemoveRows({}, index, index);
+        _finalList.removeAt(index);
+        endRemoveRows();
+    }
+
     if (activity._type == Activity::ActivityType) {
-        index = _activityLists.indexOf(activity);
-        if (index != -1) {
-            _activityLists.removeAt(index);
-            const auto indexInFinalList = _finalList.indexOf(activity);
-            if (indexInFinalList != -1) {
-                beginRemoveRows({}, indexInFinalList, indexInFinalList);
-                _finalList.removeAt(indexInFinalList);
-                endRemoveRows();
-            }
+        const auto activityListIndex = _activityLists.indexOf(activity);
+        if (activityListIndex != -1) {
+            _activityLists.removeAt(activityListIndex);
         }
     } else if (activity._type == Activity::NotificationType) {
-        index = _notificationLists.indexOf(activity);
-        if (index != -1)
-            _notificationLists.removeAt(index);
+        const auto notificationListIndex = _notificationLists.indexOf(activity);
+        if (notificationListIndex != -1)
+            _notificationLists.removeAt(notificationListIndex);
     } else {
-        index = _notificationErrorsLists.indexOf(activity);
-        if (index != -1)
-            _notificationErrorsLists.removeAt(index);
-    }
-
-    if (index != -1) {
-        qCInfo(lcActivity) << "Activity/Notification/Error successfully removed from the list.";
-        qCInfo(lcActivity) << "Updating Activity/Notification/Error view.";
-        combineActivityLists();
+        const auto notificationErrorsListIndex = _notificationErrorsLists.indexOf(activity);
+        if (notificationErrorsListIndex != -1)
+            _notificationErrorsLists.removeAt(notificationErrorsListIndex);
     }
 }
 
@@ -825,47 +934,6 @@ QVariantList ActivityListModel::convertLinksToMenuEntries(const Activity &activi
     return customList;
 }
 
-void ActivityListModel::combineActivityLists()
-{
-    ActivityList resultList;
-
-    if (_notificationErrorsLists.count() > 0) {
-        std::sort(_notificationErrorsLists.begin(), _notificationErrorsLists.end());
-        resultList.append(_notificationErrorsLists);
-    }
-    if (_listOfIgnoredFiles.size() > 0)
-        resultList.append(_notificationIgnoredFiles);
-
-    if (_notificationLists.count() > 0) {
-        std::sort(_notificationLists.begin(), _notificationLists.end());
-        resultList.append(_notificationLists);
-    }
-
-    if (_syncFileItemLists.count() > 0) {
-        std::sort(_syncFileItemLists.begin(), _syncFileItemLists.end());
-        resultList.append(_syncFileItemLists);
-    }
-
-    if (_activityLists.count() > 0) {
-        std::sort(_activityLists.begin(), _activityLists.end());
-        resultList.append(_activityLists);
-    }
-
-    if (_finalList.isEmpty() && !resultList.isEmpty()) {
-        beginInsertRows({}, 0, resultList.size() - 1);
-        _finalList = resultList;
-        endInsertRows();
-    } else if (!_finalList.isEmpty()) {
-        beginResetModel();
-        _finalList = resultList;
-        endResetModel();
-    }
-
-    if (_activityLists.size() > 0) {
-        appendMoreActivitiesAvailableEntry();
-    }
-}
-
 bool ActivityListModel::canFetchActivities() const
 {
     return _accountState->isConnected() && _accountState->account()->capabilities().hasActivities();
@@ -880,17 +948,14 @@ void ActivityListModel::fetchMore(const QModelIndex &)
 
 void ActivityListModel::slotRefreshActivity()
 {
-    clearActivities();
     _doneFetching = false;
     _currentItem = 0;
-    _totalActivitiesFetched = 0;
     _showMoreActivitiesAvailableEntry = false;
 
     if (canFetchActivities()) {
         startFetchJob();
     } else {
         _doneFetching = true;
-        combineActivityLists();
     }
 }
 
@@ -905,10 +970,10 @@ void ActivityListModel::slotRemoveAccount()
 {
     _finalList.clear();
     _activityLists.clear();
+    _presentedActivities.clear();
     setAndRefreshCurrentlyFetching(false);
     _doneFetching = false;
     _currentItem = 0;
-    _totalActivitiesFetched = 0;
     _showMoreActivitiesAvailableEntry = false;
 }
 

src/gui/tray/activitylistmodel.h

@@ -41,8 +41,8 @@ class ActivityListModel : public QAbstractListModel
     Q_OBJECT
 
     Q_PROPERTY(quint32 maxActionButtons READ maxActionButtons CONSTANT)
-
     Q_PROPERTY(AccountState *accountState READ accountState CONSTANT)
+
 public:
     enum DataRole {
         DarkIconRole = Qt::UserRole + 1,
@@ -73,9 +73,21 @@ public:
         TalkNotificationMessageIdRole,
         TalkNotificationMessageSentRole,
         TalkNotificationUserAvatarRole,
+        ActivityRole,
     };
     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,
@@ -89,25 +101,16 @@ public:
 
     ActivityList activityList() { return _finalList; }
     ActivityList errorsList() { return _notificationErrorsLists; }
-    void addNotificationToActivityList(Activity activity);
-    void clearNotifications();
-    void addErrorToActivityList(Activity activity);
-    void addIgnoredFileToList(Activity newActivity);
-    void addSyncFileItemToActivityList(Activity activity);
-    void removeActivityFromActivityList(int row);
-    void removeActivityFromActivityList(Activity activity);
 
     AccountState *accountState() const;
-    void setAccountState(AccountState *state);
+
+    int currentItem() const;
 
     static constexpr quint32 maxActionButtons()
     {
         return MaxActionButtons;
     }
 
-    void setCurrentItem(const int currentItem);
-
-    void setReplyMessageSent(const int activityIndex, const QString &message);
     QString replyMessageSent(const Activity &activity) const;
 
 public slots:
@@ -118,55 +121,67 @@ public slots:
     void slotTriggerAction(const int activityIndex, const int actionIndex);
     void slotTriggerDismiss(const int activityIndex);
 
+    void addNotificationToActivityList(const Activity &activity);
+    void addErrorToActivityList(const Activity &activity);
+    void addIgnoredFileToList(const Activity &newActivity);
+    void addSyncFileItemToActivityList(const Activity &activity);
+    void removeActivityFromActivityList(int row);
+    void removeActivityFromActivityList(const Activity &activity);
+
+    void setAccountState(AccountState *state);
+    void setReplyMessageSent(const int activityIndex, const QString &message);
+    void setCurrentItem(const int currentItem);
+
 signals:
     void activityJobStatusCode(int statusCode);
     void sendNotificationRequest(const QString &accountName, const QString &link, const QByteArray &verb, int row);
 
 protected:
-    void setup();
-    void activitiesReceived(const QJsonDocument &json, int statusCode);
     QHash<int, QByteArray> roleNames() const override;
 
-    void setAndRefreshCurrentlyFetching(bool value);
     bool currentlyFetching() const;
+
+    const ActivityList &finalList() const; // added for unit tests
+
+protected slots:
+    void activitiesReceived(const QJsonDocument &json, int statusCode);
+    void setAndRefreshCurrentlyFetching(bool value);
     void setDoneFetching(bool value);
     void setHideOldActivities(bool value);
     void setDisplayActions(bool value);
+    void setFinalList(const ActivityList &finalList); // added for unit tests
 
     virtual void startFetchJob();
 
-    // added these for unit tests
-    void setFinalList(const ActivityList &finalList);
-    const ActivityList &finalList() const;
-    int currentItem() const;
-    //
-
 private:
     static QVariantList convertLinksToMenuEntries(const Activity &activity);
     static QVariantList convertLinksToActionButtons(const Activity &activity);
     static QVariant convertLinkToActionButton(const ActivityLink &activityLink);
-    void combineActivityLists();
+
+    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);
     void appendMoreActivitiesAvailableEntry();
-
     void insertOrRemoveDummyFetchingActivity();
 
-    void clearActivities();
+    Activity _notificationIgnoredFiles;
+    Activity _dummyFetchingActivities;
 
     ActivityList _activityLists;
     ActivityList _syncFileItemLists;
     ActivityList _notificationLists;
     ActivityList _listOfIgnoredFiles;
-    Activity _notificationIgnoredFiles;
     ActivityList _notificationErrorsLists;
     ActivityList _finalList;
-    int _currentItem = 0;
+
+    QSet<qint64> _presentedActivities;
 
     bool _displayActions = true;
 
-    int _totalActivitiesFetched = 0;
+    int _currentItem = 0;
     int _maxActivities = 100;
     int _maxActivitiesDays = 30;
     bool _showMoreActivitiesAvailableEntry = false;

test/testactivitylistmodel.cpp

@@ -349,6 +349,7 @@ public:
         params.addQueryItem(QLatin1String("limit"), QString::number(50));
         job->addQueryParams(params);
 
+        setAndRefreshCurrentlyFetching(true);
         job->start();
     }
 
@@ -379,6 +380,7 @@ public slots:
             setFinalList(finalListCopy);
         }
         _numRowsPrev = rowCount();
+        setAndRefreshCurrentlyFetching(false);
         emit activitiesProcessed();
     }
 signals:
@@ -394,7 +396,8 @@ class TestActivityListModel : public QObject
 
 public:
     TestActivityListModel() = default;
-    ~TestActivityListModel() override {
+    ~TestActivityListModel() override
+    {
         OCC::AccountManager::instance()->deleteAccount(accountState.data());
     }
 
@@ -403,9 +406,31 @@ public:
     QScopedPointer<OCC::AccountState> accountState;
 
     OCC::Activity testNotificationActivity;
+    OCC::Activity testSyncResultErrorActivity;
+    OCC::Activity testSyncFileItemActivity;
+    OCC::Activity testFileIgnoredActivity;
 
     static constexpr int searchResultsReplyDelay = 100;
 
+    QSharedPointer<TestingALM> testingALM() {
+        QSharedPointer<TestingALM> model(new TestingALM);
+        model->setAccountState(accountState.data());
+        QAbstractItemModelTester modelTester(model.data());
+
+        return model;
+    }
+
+    void testActivityAdd(void(OCC::ActivityListModel::*addingMethod)(const OCC::Activity&), OCC::Activity &activity) {
+        const auto model = testingALM();
+        QCOMPARE(model->rowCount(), 0);
+
+        (model.data()->*addingMethod)(activity);
+        QCOMPARE(model->rowCount(), 1);
+
+        const auto index = model->index(0, 0);
+        QVERIFY(index.isValid());
+    }
+
 private slots:
     void initTestCase()
     {
@@ -452,164 +477,166 @@ private slots:
         testNotificationActivity._id = 1;
         testNotificationActivity._type = OCC::Activity::NotificationType;
         testNotificationActivity._dateTime = QDateTime::currentDateTime();
+        testNotificationActivity._subject = QStringLiteral("Sample notification text");
+
+        testSyncResultErrorActivity._id = 2;
+        testSyncResultErrorActivity._type = OCC::Activity::SyncResultType;
+        testSyncResultErrorActivity._status = OCC::SyncResult::Error;
+        testSyncResultErrorActivity._dateTime = QDateTime::currentDateTime();
+        testSyncResultErrorActivity._subject = QStringLiteral("Sample failed sync text");
+        testSyncResultErrorActivity._message = QStringLiteral("/path/to/thingy");
+        testSyncResultErrorActivity._link = QStringLiteral("/path/to/thingy");
+        testSyncResultErrorActivity._accName = accountState->account()->displayName();
+
+        testSyncFileItemActivity._id = 3;
+        testSyncFileItemActivity._type = OCC::Activity::SyncFileItemType; //client activity
+        testSyncFileItemActivity._status = OCC::SyncFileItem::Success;
+        testSyncFileItemActivity._dateTime = QDateTime::currentDateTime();
+        testSyncFileItemActivity._message = QStringLiteral("Sample file successfully synced text");
+        testSyncFileItemActivity._link = accountState->account()->url();
+        testSyncFileItemActivity._accName = accountState->account()->displayName();
+        testSyncFileItemActivity._file = QStringLiteral("xyz.pdf");
+
+        testFileIgnoredActivity._id = 4;
+        testFileIgnoredActivity._type = OCC::Activity::SyncFileItemType;
+        testFileIgnoredActivity._status = OCC::SyncFileItem::FileIgnored;
+        testFileIgnoredActivity._dateTime = QDateTime::currentDateTime();
+        testFileIgnoredActivity._subject = QStringLiteral("Sample ignored file sync text");
+        testFileIgnoredActivity._link = accountState->account()->url();
+        testFileIgnoredActivity._accName = accountState->account()->displayName();
+        testFileIgnoredActivity._folder = QStringLiteral("thingy");
+        testFileIgnoredActivity._file = QStringLiteral("test.txt");
     };
 
     // Test receiving activity from server
     void testFetchingRemoteActivity() {
-        TestingALM model;
-        model.setAccountState(accountState.data());
-        QAbstractItemModelTester modelTester(&model);
+        const auto model = testingALM();
+        QCOMPARE(model->rowCount(), 0);
 
-        QCOMPARE(model.rowCount(), 0);
-
-        model.setCurrentItem(FakeRemoteActivityStorage::instance()->startingIdLast());
-        model.startFetchJob();
-        QSignalSpy activitiesJob(&model, &TestingALM::activitiesProcessed);
+        model->setCurrentItem(FakeRemoteActivityStorage::instance()->startingIdLast());
+        model->startFetchJob();
+        QSignalSpy activitiesJob(model.data(), &TestingALM::activitiesProcessed);
         QVERIFY(activitiesJob.wait(3000));
-        QCOMPARE(model.rowCount(), 50);
+        QCOMPARE(model->rowCount(), 50);
     };
 
     // Test receiving activity from local user action
     void testLocalSyncFileAction() {
-        TestingALM model;
-        model.setAccountState(accountState.data());
-        QAbstractItemModelTester modelTester(&model);
-
-        QCOMPARE(model.rowCount(), 0);
-
-        OCC::Activity activity;
-
-        model.addSyncFileItemToActivityList(activity);
-        QCOMPARE(model.rowCount(), 1);
-
-        const auto index = model.index(0, 0);
-        QVERIFY(index.isValid());
+        testActivityAdd(&TestingALM::addSyncFileItemToActivityList, testSyncFileItemActivity);
     };
 
     void testAddNotification() {
-        TestingALM model;
-        model.setAccountState(accountState.data());
-        QAbstractItemModelTester modelTester(&model);
-
-        QCOMPARE(model.rowCount(), 0);
-
-        model.addNotificationToActivityList(testNotificationActivity);
-        QCOMPARE(model.rowCount(), 1);
-
-        const auto index = model.index(0, 0);
-        QVERIFY(index.isValid());
+        testActivityAdd(&TestingALM::addNotificationToActivityList, testNotificationActivity);
     };
 
     void testAddError() {
-        TestingALM model;
-        model.setAccountState(accountState.data());
-        QAbstractItemModelTester modelTester(&model);
-
-        QCOMPARE(model.rowCount(), 0);
-
-        OCC::Activity activity;
-
-        model.addErrorToActivityList(activity);
-        QCOMPARE(model.rowCount(), 1);
-
-        const auto index = model.index(0, 0);
-        QVERIFY(index.isValid());
+        testActivityAdd(&TestingALM::addErrorToActivityList, testSyncResultErrorActivity);
     };
 
     void testAddIgnoredFile() {
-        TestingALM model;
-        model.setAccountState(accountState.data());
-        QAbstractItemModelTester modelTester(&model);
-
-        QCOMPARE(model.rowCount(), 0);
-
-        OCC::Activity activity;
-        activity._folder = QStringLiteral("thingy");
-        activity._file = QStringLiteral("test.txt");
-
-        model.addIgnoredFileToList(activity);
-        // We need to add another activity to the model for the combineActivityLists method to be called
-        model.addNotificationToActivityList(testNotificationActivity);
-        QCOMPARE(model.rowCount(), 2);
-
-        const auto index = model.index(0, 0);
-        QVERIFY(index.isValid());
+        testActivityAdd(&TestingALM::addIgnoredFileToList, testFileIgnoredActivity);
     };
 
     // Test removing activity from list
     void testRemoveActivityWithRow() {
-        TestingALM model;
-        model.setAccountState(accountState.data());
-        QAbstractItemModelTester modelTester(&model);
-
-        QCOMPARE(model.rowCount(), 0);
+        const auto model = testingALM();
+        QCOMPARE(model->rowCount(), 0);
 
-        model.addNotificationToActivityList(testNotificationActivity);
-        QCOMPARE(model.rowCount(), 1);
+        model->addNotificationToActivityList(testNotificationActivity);
+        QCOMPARE(model->rowCount(), 1);
 
-        model.removeActivityFromActivityList(0);
-        QCOMPARE(model.rowCount(), 0);
+        model->removeActivityFromActivityList(0);
+        QCOMPARE(model->rowCount(), 0);
     }
 
     void testRemoveActivityWithActivity() {
-        TestingALM model;
-        model.setAccountState(accountState.data());
-        QAbstractItemModelTester modelTester(&model);
+        const auto model = testingALM();
+        QCOMPARE(model->rowCount(), 0);
+
+        model->addNotificationToActivityList(testNotificationActivity);
+        QCOMPARE(model->rowCount(), 1);
+
+        model->removeActivityFromActivityList(testNotificationActivity);
+        QCOMPARE(model->rowCount(), 0);
+    }
 
-        QCOMPARE(model.rowCount(), 0);
+    void testDummyFetchingActivitiesActivity() {
+        const auto model = testingALM();
+        QCOMPARE(model->rowCount(), 0);
 
-        model.addNotificationToActivityList(testNotificationActivity);
-        QCOMPARE(model.rowCount(), 1);
+        model->setCurrentItem(FakeRemoteActivityStorage::instance()->startingIdLast());
+        model->startFetchJob();
 
-        model.removeActivityFromActivityList(testNotificationActivity);
-        QCOMPARE(model.rowCount(), 0);
+        // Check for the dummy before activities have arrived
+        QCOMPARE(model->rowCount(), 1);
+
+        QSignalSpy activitiesJob(model.data(), &TestingALM::activitiesProcessed);
+        QVERIFY(activitiesJob.wait(3000));
+        // Test the dummy was removed
+        QCOMPARE(model->rowCount(), 50);
     }
 
     // Test getting the data from the model
     void testData() {
-        TestingALM model;
-        model.setAccountState(accountState.data());
-        QAbstractItemModelTester modelTester(&model);
-
-        QCOMPARE(model.rowCount(), 0);
+        const auto model = testingALM();
+        QCOMPARE(model->rowCount(), 0);
 
-        model.setCurrentItem(FakeRemoteActivityStorage::instance()->startingIdLast());
-        model.startFetchJob();
-        QSignalSpy activitiesJob(&model, &TestingALM::activitiesProcessed);
+        model->setCurrentItem(FakeRemoteActivityStorage::instance()->startingIdLast());
+        model->startFetchJob();
+        QSignalSpy activitiesJob(model.data(), &TestingALM::activitiesProcessed);
         QVERIFY(activitiesJob.wait(3000));
-        QCOMPARE(model.rowCount(), 50);
-
-        model.addNotificationToActivityList(testNotificationActivity);
-        QCOMPARE(model.rowCount(), 51);
-
-        OCC::Activity syncResultActivity;
-        syncResultActivity._id = 2;
-        syncResultActivity._type = OCC::Activity::SyncResultType;
-        syncResultActivity._status = OCC::SyncResult::Error;
-        syncResultActivity._dateTime = QDateTime::currentDateTime();
-        syncResultActivity._subject = QStringLiteral("Sample failed sync text");
-        syncResultActivity._message = QStringLiteral("/path/to/thingy");
-        syncResultActivity._link = QStringLiteral("/path/to/thingy");
-        syncResultActivity._accName = accountState->account()->displayName();
-        model.addSyncFileItemToActivityList(syncResultActivity);
-        QCOMPARE(model.rowCount(), 52);
-
-        OCC::Activity syncFileItemActivity;
-        syncFileItemActivity._id = 3;
-        syncFileItemActivity._type = OCC::Activity::SyncFileItemType; //client activity
-        syncFileItemActivity._status = OCC::SyncFileItem::Success;
-        syncFileItemActivity._dateTime = QDateTime::currentDateTime();
-        syncFileItemActivity._message = QStringLiteral("You created xyz.pdf");
-        syncFileItemActivity._link = accountState->account()->url();
-        syncFileItemActivity._accName = accountState->account()->displayName();
-        syncFileItemActivity._file = QStringLiteral("xyz.pdf");
-        syncFileItemActivity._fileAction = "";
-        model.addSyncFileItemToActivityList(syncFileItemActivity);
-        QCOMPARE(model.rowCount(), 53);
+        QCOMPARE(model->rowCount(), 50);
+
+        model->addSyncFileItemToActivityList(testSyncFileItemActivity);
+        QCOMPARE(model->rowCount(), 51);
+
+        model->addErrorToActivityList(testSyncResultErrorActivity);
+        QCOMPARE(model->rowCount(), 52);
+
+        model->addIgnoredFileToList(testFileIgnoredActivity);
+        QCOMPARE(model->rowCount(), 53);
+
+        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);
+        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();
 
@@ -642,26 +669,23 @@ private slots:
         }
     };
 
-    void tesActivityActionstData()
+    void testActivityActionsData()
     {
-        TestingALM model;
-        model.setAccountState(accountState.data());
-        QAbstractItemModelTester modelTester(&model);
-
-        QCOMPARE(model.rowCount(), 0);
-        model.setCurrentItem(FakeRemoteActivityStorage::instance()->startingIdLast());
+        const auto model = testingALM();
+        QCOMPARE(model->rowCount(), 0);
+        model->setCurrentItem(FakeRemoteActivityStorage::instance()->startingIdLast());
 
-        int prevModelRowCount = model.rowCount();
+        int prevModelRowCount = model->rowCount();
 
         do {
-            prevModelRowCount = model.rowCount();
-            model.startFetchJob();
-            QSignalSpy activitiesJob(&model, &TestingALM::activitiesProcessed);
+            prevModelRowCount = model->rowCount();
+            model->startFetchJob();
+            QSignalSpy activitiesJob(model.data(), &TestingALM::activitiesProcessed);
             QVERIFY(activitiesJob.wait(3000));
 
 
-            for (int i = prevModelRowCount; i < model.rowCount(); i++) {
-                const auto index = model.index(i, 0);
+            for (int i = prevModelRowCount; i < model->rowCount(); i++) {
+                const auto index = model->index(i, 0);
 
                 const auto actionsLinks = index.data(OCC::ActivityListModel::ActionsLinksRole).toList();
                 if (!actionsLinks.isEmpty()) {
@@ -720,7 +744,7 @@ private slots:
                 }
             }
 
-        } while (prevModelRowCount < model.rowCount());
+        } while (prevModelRowCount < model->rowCount());
     };
 
 };