diff options
author | Claudio Cambra <claudio.cambra@gmail.com> | 2022-07-13 21:11:37 +0300 |
---|---|---|
committer | Claudio Cambra <claudio.cambra@gmail.com> | 2022-08-02 18:42:18 +0300 |
commit | 4fdc78d773ab6730f4b788aef264112633f1b84b (patch) | |
tree | 1e2361f3b5b427f7e7946b5e61b8d314b0e77c54 | |
parent | 9cb255f0465361685b9077afc64c5f52ffaa770e (diff) |
Refactor ActivityListModel population mechanismsbugfix/refactor-activity-list-model
Signed-off-by: Claudio Cambra <claudio.cambra@gmail.com>
-rw-r--r-- | src/gui/fileactivitylistmodel.h | 2 | ||||
-rw-r--r-- | src/gui/tray/activitydata.cpp | 10 | ||||
-rw-r--r-- | src/gui/tray/activitydata.h | 3 | ||||
-rw-r--r-- | src/gui/tray/activitylistmodel.cpp | 339 | ||||
-rw-r--r-- | src/gui/tray/activitylistmodel.h | 69 | ||||
-rw-r--r-- | src/gui/tray/usermodel.cpp | 2 | ||||
-rw-r--r-- | test/testactivitylistmodel.cpp | 292 |
7 files changed, 413 insertions, 304 deletions
diff --git a/src/gui/fileactivitylistmodel.h b/src/gui/fileactivitylistmodel.h index 2c698ddde..18d8d9830 100644 --- a/src/gui/fileactivitylistmodel.h +++ b/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: diff --git a/src/gui/tray/activitydata.cpp b/src/gui/tray/activitydata.cpp index 1b8bfc34f..a013086d1 100644 --- a/src/gui/tray/activitydata.cpp +++ b/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); diff --git a/src/gui/tray/activitydata.h b/src/gui/tray/activitydata.h index 944e1f617..6ae9bebc7 100644 --- a/src/gui/tray/activitydata.h +++ b/src/gui/tray/activitydata.h @@ -156,7 +156,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); /* ==================================================================== */ /** @@ -168,6 +170,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) diff --git a/src/gui/tray/activitylistmodel.cpp b/src/gui/tray/activitylistmodel.cpp index f0c6673c6..c217d37a8 100644 --- a/src/gui/tray/activitylistmodel.cpp +++ b/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 @@ -82,6 +82,7 @@ QHash<int, QByteArray> ActivityListModel::roleNames() const roles[TalkNotificationMessageIdRole] = "messageId"; roles[TalkNotificationMessageSentRole] = "messageSent"; roles[TalkNotificationUserAvatarRole] = "userAvatar"; + roles[ActivityRole] = "activity"; return roles; } @@ -276,7 +277,7 @@ QVariant ActivityListModel::data(const QModelIndex &index, int role) const case ActionsLinksContextMenuRole: { return ActivityListModel::convertLinksToMenuEntries(a); } - + case ActionsLinksForActionButtonsRole: { return ActivityListModel::convertLinksToActionButtons(a); } @@ -354,9 +355,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(); } @@ -429,11 +431,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; @@ -441,15 +447,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); } } @@ -458,6 +459,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(); @@ -470,56 +472,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); } } @@ -542,14 +514,155 @@ 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(); + case ActivityEntryType::SyncFileItemType: + startRow += _notificationLists.count(); + case ActivityEntryType::NotificationType: + // We only show one activity for ignored files + if(_listOfIgnoredFiles.count() > 0) { + startRow += 1; + } + case ActivityEntryType::IgnoredFileType: + startRow += _notificationErrorsLists.count(); + // 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; + } + case ActivityEntryType::MoreActivitiesAvailableType: + if(!_showMoreActivitiesAvailableEntry) { + break; + } + 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) { + const 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; @@ -557,6 +670,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; } @@ -573,65 +687,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); } } @@ -821,47 +924,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(); @@ -876,17 +938,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(); } } @@ -901,10 +960,10 @@ void ActivityListModel::slotRemoveAccount() { _finalList.clear(); _activityLists.clear(); + _presentedActivities.clear(); setAndRefreshCurrentlyFetching(false); _doneFetching = false; _currentItem = 0; - _totalActivitiesFetched = 0; _showMoreActivitiesAvailableEntry = false; } diff --git a/src/gui/tray/activitylistmodel.h b/src/gui/tray/activitylistmodel.h index 8ea3a49a1..b11b27040 100644 --- a/src/gui/tray/activitylistmodel.h +++ b/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, @@ -72,9 +72,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, @@ -88,25 +100,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: @@ -117,55 +120,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; diff --git a/src/gui/tray/usermodel.cpp b/src/gui/tray/usermodel.cpp index 782de47a1..9fe72e44b 100644 --- a/src/gui/tray/usermodel.cpp +++ b/src/gui/tray/usermodel.cpp @@ -114,8 +114,6 @@ void User::showDesktopNotification(const QString &title, const QString &message, void User::slotBuildNotificationDisplay(const ActivityList &list) { - _activityModel->clearNotifications(); - const auto multipleAccounts = AccountManager::instance()->accounts().count() > 1; ActivityList toNotifyList; diff --git a/test/testactivitylistmodel.cpp b/test/testactivitylistmodel.cpp index 17c5de729..624657592 100644 --- a/test/testactivitylistmodel.cpp +++ b/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()); }; }; |