From 1a5fa50fbbfb006232fcc9d92733cdf2e9573056 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Wed, 13 Jul 2022 20:11:37 +0200 Subject: [PATCH] Refactor ActivityListModel population mechanisms Signed-off-by: Claudio Cambra --- src/gui/fileactivitylistmodel.h | 2 +- src/gui/tray/activitydata.cpp | 10 + src/gui/tray/activitydata.h | 3 + src/gui/tray/activitylistmodel.cpp | 349 +++++++++++++++++------------ src/gui/tray/activitylistmodel.h | 69 +++--- test/testactivitylistmodel.cpp | 286 ++++++++++++----------- 6 files changed, 418 insertions(+), 301 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 0875ee463..f7f943a5c 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 9371fd736..a6fe46651 100644 --- a/src/gui/tray/activitydata.h +++ b/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; } +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 905ebcf42..556715ade 100644 --- a/src/gui/tray/activitylistmodel.cpp +++ b/src/gui/tray/activitylistmodel.cpp @@ -1,4 +1,4 @@ -/* +/* * Copyright (C) by Klaas Freitag * * This program is free software; you can redistribute it and/or modify @@ -84,6 +84,7 @@ QHash 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"); + _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"); - beginInsertRows({}, 0, 0); - _finalList.prepend(a); - endInsertRows(); + 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 ActivityListModel::rowRangeForEntryType(const ActivityEntryType type) const { - qCInfo(lcActivity) << "Error successfully added to the notification list: " << activity._subject; - _notificationErrorsLists.prepend(activity); - combineActivityLists(); + // 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}; } -void ActivityListModel::addIgnoredFileToList(Activity newActivity) +// 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); +} + +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; - 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(); - } - } - } else if (activity._type == Activity::NotificationType) { - index = _notificationLists.indexOf(activity); - if (index != -1) - _notificationLists.removeAt(index); - } else { - index = _notificationErrorsLists.indexOf(activity); - if (index != -1) - _notificationErrorsLists.removeAt(index); - } - + 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."; - combineActivityLists(); + + beginRemoveRows({}, index, index); + _finalList.removeAt(index); + 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 { + 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; } diff --git a/src/gui/tray/activitylistmodel.h b/src/gui/tray/activitylistmodel.h index 482e80483..bdb82dec0 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, @@ -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 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 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 _presentedActivities; bool _displayActions = true; - int _totalActivitiesFetched = 0; + int _currentItem = 0; int _maxActivities = 100; int _maxActivitiesDays = 30; bool _showMoreActivitiesAvailableEntry = false; 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 accountState; OCC::Activity testNotificationActivity; + OCC::Activity testSyncResultErrorActivity; + OCC::Activity testSyncFileItemActivity; + OCC::Activity testFileIgnoredActivity; static constexpr int searchResultsReplyDelay = 100; + QSharedPointer testingALM() { + QSharedPointer 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); + const auto model = testingALM(); + QCOMPARE(model->rowCount(), 0); - 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); - QCOMPARE(model.rowCount(), 0); + model->addNotificationToActivityList(testNotificationActivity); + QCOMPARE(model->rowCount(), 1); - model.addNotificationToActivityList(testNotificationActivity); - QCOMPARE(model.rowCount(), 1); + model->removeActivityFromActivityList(testNotificationActivity); + QCOMPARE(model->rowCount(), 0); + } - model.removeActivityFromActivityList(testNotificationActivity); - QCOMPARE(model.rowCount(), 0); + void testDummyFetchingActivitiesActivity() { + const auto model = testingALM(); + QCOMPARE(model->rowCount(), 0); + + model->setCurrentItem(FakeRemoteActivityStorage::instance()->startingIdLast()); + model->startFetchJob(); + + // 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); + 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); - model.addNotificationToActivityList(testNotificationActivity); - QCOMPARE(model.rowCount(), 51); + model->addSyncFileItemToActivityList(testSyncFileItemActivity); + 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); + model->addErrorToActivityList(testSyncResultErrorActivity); + 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); + model->addIgnoredFileToList(testFileIgnoredActivity); + QCOMPARE(model->rowCount(), 53); + + model->addNotificationToActivityList(testNotificationActivity); + QCOMPARE(model->rowCount(), 54); + + const auto desiredOrder = QVector{ + 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(); + + // 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); + const auto model = testingALM(); + QCOMPARE(model->rowCount(), 0); + model->setCurrentItem(FakeRemoteActivityStorage::instance()->startingIdLast()); - 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()); }; };