From: Claudio Cambra Date: Wed, 14 Sep 2022 00:26:38 +0000 (+0200) Subject: Remove sorting code in activitylistmodel as not needed with sortedactivitylistmodel X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~12^2~11^2~249^2 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=e721b03106cc606372e657cf45e8fb8eb3e86e84;p=nextcloud-desktop.git Remove sorting code in activitylistmodel as not needed with sortedactivitylistmodel Signed-off-by: Claudio Cambra --- diff --git a/src/gui/tray/activitylistmodel.cpp b/src/gui/tray/activitylistmodel.cpp index 65a6d7ce3..ebcd646e1 100644 --- a/src/gui/tray/activitylistmodel.cpp +++ b/src/gui/tray/activitylistmodel.cpp @@ -464,7 +464,7 @@ void ActivityListModel::ingestActivities(const QJsonArray &activities) } if (list.size() > 0) { - addEntriesToActivityList(list, ActivityEntryType::ActivityType); + addEntriesToActivityList(list); appendMoreActivitiesAvailableEntry(); _activityLists.append(list); } @@ -488,7 +488,7 @@ void ActivityListModel::appendMoreActivitiesAvailableEntry() a._link = app->url(); } - addEntriesToActivityList({a}, ActivityEntryType::MoreActivitiesAvailableType); + addEntriesToActivityList({a}); } } @@ -505,7 +505,7 @@ void ActivityListModel::insertOrRemoveDummyFetchingActivity() _dummyFetchingActivities._dateTime = QDateTime::currentDateTime(); _dummyFetchingActivities._icon = QLatin1String("qrc:///client/theme/colored/change-bordered.svg"); - addEntriesToActivityList({_dummyFetchingActivities}, ActivityEntryType::DummyFetchingActivityType); + addEntriesToActivityList({_dummyFetchingActivities}); } else if (!_finalList.isEmpty() && _finalList.first()._objectType == dummyFetchingActivityObjectType) { removeActivityFromActivityList(_dummyFetchingActivities); } @@ -530,157 +530,25 @@ void ActivityListModel::activitiesReceived(const QJsonDocument &json, int status emit activityJobStatusCode(statusCode); } -std::pair ActivityListModel::rowRangeForEntryType(const ActivityEntryType type) const -{ - // We want to present activities in a certain order, and we want to ensure entry types are grouped together. - // We therefore need to find the range of rows in the finalList that represent an entry type group. - int startRow = 0; - - // We start from the type that we want to push down the furthest. Cascade through switch cases. - // Each time we fall through we add the count of elements in each of the sections that go above. - // So, items at the top of the activity list have a startRow of 1. The next section gets the count of that first - // section's elements added to startRow. Section 3 gets the count of Section 1 and Section 2 added to startRow. - // This goes on and on, with the last section getting startRow as the count of ALL section's element counts. - - switch(type) { - case ActivityEntryType::MoreActivitiesAvailableType: // Always needs to be at the bottom - startRow = _finalList.count(); - break; - case ActivityEntryType::ActivityType: - startRow += _syncFileItemLists.count(); - Q_FALLTHROUGH(); - case ActivityEntryType::SyncFileItemType: - startRow += _notificationLists.count(); - Q_FALLTHROUGH(); - case ActivityEntryType::NotificationType: - // We only show one activity for ignored files - if(_listOfIgnoredFiles.count() > 0) { - startRow += 1; - } - Q_FALLTHROUGH(); - case ActivityEntryType::IgnoredFileType: - startRow += _notificationErrorsLists.count(); - Q_FALLTHROUGH(); - // Remaining types should go at top - case ActivityEntryType::ErrorType: - case ActivityEntryType::DummyFetchingActivityType: - break; - } - - // To calculate the end row of the section, we just get the number of entries in the relevant section and then - // add it to the startRow. - - int entryRowCount = -1; - - switch(type) { - case ActivityEntryType::ActivityType: - entryRowCount = _activityLists.count(); - break; - case ActivityEntryType::SyncFileItemType: - entryRowCount = _syncFileItemLists.count(); - break; - case ActivityEntryType::NotificationType: - entryRowCount = _notificationLists.count(); - break; - case ActivityEntryType::ErrorType: - entryRowCount = _notificationErrorsLists.count(); - break; - - // Single activity sections - case ActivityEntryType::IgnoredFileType: - if(_listOfIgnoredFiles.count() == 0) { - break; - } - Q_FALLTHROUGH(); - case ActivityEntryType::MoreActivitiesAvailableType: - if(!_showMoreActivitiesAvailableEntry) { - break; - } - Q_FALLTHROUGH(); - case ActivityEntryType::DummyFetchingActivityType: - if(_finalList.count() > 0 && _finalList.first() != _dummyFetchingActivities) { - break; - } - - // All cascade down to here - entryRowCount = 1; - break; - } - - // Even though we always return a startRow even if the section does not exist, - // we return -1 as endRow if the section does not exist. - // If we have a -1 we also know that the startRow is "theoretical", where the section - // "should" begin, not necessarily where it "does" begin - const auto endRow = entryRowCount > 0 ? startRow + entryRowCount - 1 : -1; - - return {startRow, endRow}; -} - -// Make sure to add activities to their specific entry type lists AFTER adding them to the main list -void ActivityListModel::addEntriesToActivityList(const ActivityList &activityList, const ActivityEntryType type) +void ActivityListModel::addEntriesToActivityList(const ActivityList &activityList) { if(activityList.isEmpty()) { return; } - const auto entryTypeSectionRowRange = rowRangeForEntryType(type); - - auto sortedList = activityList; - std::sort(sortedList.begin(), sortedList.end()); - - if(_finalList.count() == 0 || entryTypeSectionRowRange.second == -1) { - // If the finalList is empty or there are no entries belonging to the entry type section, we don't - // need to bother with inserting in a correct order and can more quickly just insert all activities. - const auto startRow = entryTypeSectionRowRange.first; - const auto endRow = startRow + sortedList.count() - 1; + const auto startRow = _finalList.count(); - beginInsertRows({}, startRow, endRow); - int i = startRow; - for(const auto &activity : sortedList) { - _finalList.insert(i, activity); - ++i; - } - endInsertRows(); - return; + beginInsertRows({}, startRow, startRow + activityList.count() - 1); + for(const auto &activity : activityList) { + _finalList.append(activity); } - - // If the finalList is not empty and the entry type's section actually exists (i.e. there is at least - // one entry belonging to this entry in the finalList) then we are going to add them granularly. - // We make sure to insert the item in a specific place so as to preserve the sort order. - int sectionRowEnd = entryTypeSectionRowRange.second; - - const auto insertRow = [&](const int row, const Activity &activity) { - beginInsertRows({}, row, row); - _finalList.insert(row, activity); - endInsertRows(); - ++sectionRowEnd; - }; - - for(const auto &activity : sortedList) { - auto currentRow = entryTypeSectionRowRange.first; - - while(currentRow <= sectionRowEnd) { - if(currentRow == sectionRowEnd) { - insertRow(currentRow + 1, activity); - break; - } - - if(activity < _finalList[currentRow]) { - insertRow(currentRow, activity); - break; - } - - ++currentRow; - } - } - - return; + endInsertRows(); } void ActivityListModel::addErrorToActivityList(const Activity &activity) { qCInfo(lcActivity) << "Error successfully added to the notification list: " << activity._subject; - addEntriesToActivityList({activity}, ActivityEntryType::ErrorType); + addEntriesToActivityList({activity}); _notificationErrorsLists.prepend(activity); } @@ -692,7 +560,7 @@ void ActivityListModel::addIgnoredFileToList(const Activity &newActivity) if (_listOfIgnoredFiles.size() == 0) { _notificationIgnoredFiles = newActivity; _notificationIgnoredFiles._subject = tr("Files from the ignore list as well as symbolic links are not synced."); - addEntriesToActivityList({_notificationIgnoredFiles}, ActivityEntryType::IgnoredFileType); + addEntriesToActivityList({_notificationIgnoredFiles}); _listOfIgnoredFiles.append(newActivity); return; } @@ -712,14 +580,14 @@ void ActivityListModel::addIgnoredFileToList(const Activity &newActivity) void ActivityListModel::addNotificationToActivityList(const Activity &activity) { qCInfo(lcActivity) << "Notification successfully added to the notification list: " << activity._subject; - addEntriesToActivityList({activity}, ActivityEntryType::NotificationType); + addEntriesToActivityList({activity}); _notificationLists.prepend(activity); } void ActivityListModel::addSyncFileItemToActivityList(const Activity &activity) { qCInfo(lcActivity) << "Successfully added to the activity list: " << activity._subject; - addEntriesToActivityList({activity}, ActivityEntryType::SyncFileItemType); + addEntriesToActivityList({activity}); _syncFileItemLists.prepend(activity); } @@ -744,16 +612,11 @@ void ActivityListModel::removeActivityFromActivityList(const Activity &activity) endRemoveRows(); } - if (activity._type == Activity::ActivityType) { - const auto activityListIndex = _activityLists.indexOf(activity); - if (activityListIndex != -1) { - _activityLists.removeAt(activityListIndex); - } - } else if (activity._type == Activity::NotificationType) { - const auto notificationListIndex = _notificationLists.indexOf(activity); - if (notificationListIndex != -1) - _notificationLists.removeAt(notificationListIndex); - } else { + if (activity._type != Activity::ActivityType && + activity._type != Activity::DummyFetchingActivityType && + activity._type != Activity::DummyMoreActivitiesAvailableType && + activity._type != Activity::NotificationType) { + const auto notificationErrorsListIndex = _notificationErrorsLists.indexOf(activity); if (notificationErrorsListIndex != -1) _notificationErrorsLists.removeAt(notificationErrorsListIndex); diff --git a/src/gui/tray/activitylistmodel.h b/src/gui/tray/activitylistmodel.h index 994e6bce5..3274863ff 100644 --- a/src/gui/tray/activitylistmodel.h +++ b/src/gui/tray/activitylistmodel.h @@ -78,17 +78,6 @@ public: }; Q_ENUM(DataRole) - enum class ActivityEntryType { - DummyFetchingActivityType, - ActivityType, - NotificationType, - ErrorType, - IgnoredFileType, - SyncFileItemType, - MoreActivitiesAvailableType, - }; - Q_ENUM(ActivityEntryType); - explicit ActivityListModel(QObject *parent = nullptr); explicit ActivityListModel(AccountState *accountState, @@ -154,14 +143,14 @@ protected slots: virtual void startFetchJob(); +private slots: + void addEntriesToActivityList(const ActivityList &activityList); + private: static QVariantList convertLinksToMenuEntries(const Activity &activity); static QVariantList convertLinksToActionButtons(const Activity &activity); static QVariant convertLinkToActionButton(const ActivityLink &activityLink); - std::pair 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); diff --git a/test/testactivitylistmodel.cpp b/test/testactivitylistmodel.cpp index 624657592..16c1267f5 100644 --- a/test/testactivitylistmodel.cpp +++ b/test/testactivitylistmodel.cpp @@ -481,7 +481,7 @@ private slots: testSyncResultErrorActivity._id = 2; testSyncResultErrorActivity._type = OCC::Activity::SyncResultType; - testSyncResultErrorActivity._status = OCC::SyncResult::Error; + testSyncResultErrorActivity._syncResultStatus = OCC::SyncResult::Error; testSyncResultErrorActivity._dateTime = QDateTime::currentDateTime(); testSyncResultErrorActivity._subject = QStringLiteral("Sample failed sync text"); testSyncResultErrorActivity._message = QStringLiteral("/path/to/thingy"); @@ -490,7 +490,7 @@ private slots: testSyncFileItemActivity._id = 3; testSyncFileItemActivity._type = OCC::Activity::SyncFileItemType; //client activity - testSyncFileItemActivity._status = OCC::SyncFileItem::Success; + testSyncFileItemActivity._syncFileItemStatus = OCC::SyncFileItem::Success; testSyncFileItemActivity._dateTime = QDateTime::currentDateTime(); testSyncFileItemActivity._message = QStringLiteral("Sample file successfully synced text"); testSyncFileItemActivity._link = accountState->account()->url(); @@ -499,7 +499,7 @@ private slots: testFileIgnoredActivity._id = 4; testFileIgnoredActivity._type = OCC::Activity::SyncFileItemType; - testFileIgnoredActivity._status = OCC::SyncFileItem::FileIgnored; + testFileIgnoredActivity._syncFileItemStatus = OCC::SyncFileItem::FileIgnored; testFileIgnoredActivity._dateTime = QDateTime::currentDateTime(); testFileIgnoredActivity._subject = QStringLiteral("Sample ignored file sync text"); testFileIgnoredActivity._link = accountState->account()->url(); @@ -599,50 +599,15 @@ private slots: model->addNotificationToActivityList(testNotificationActivity); QCOMPARE(model->rowCount(), 54); - const auto desiredOrder = QVector{ - OCC::ActivityListModel::ActivityEntryType::ErrorType, - OCC::ActivityListModel::ActivityEntryType::IgnoredFileType, - OCC::ActivityListModel::ActivityEntryType::NotificationType, - OCC::ActivityListModel::ActivityEntryType::SyncFileItemType, - OCC::ActivityListModel::ActivityEntryType::ActivityType}; - // Test all rows for things in common for (int i = 0; i < model->rowCount(); i++) { const auto index = model->index(i, 0); - int expectedEntryType = qMin(i, desiredOrder.count() - 1); - const auto activity = index.data(OCC::ActivityListModel::ActivityRole).value(); - - // Make sure the model has sorted our activities in the right order - switch(desiredOrder[expectedEntryType]) { - case OCC::ActivityListModel::ActivityEntryType::DummyFetchingActivityType: - break; - case OCC::ActivityListModel::ActivityEntryType::ErrorType: - QCOMPARE(activity._type, OCC::Activity::SyncResultType); - QCOMPARE(activity._status, OCC::SyncResult::Error); - break; - case OCC::ActivityListModel::ActivityEntryType::IgnoredFileType: - QCOMPARE(activity._type, OCC::Activity::SyncFileItemType); - QCOMPARE(activity._status, OCC::SyncFileItem::FileIgnored); - break; - case OCC::ActivityListModel::ActivityEntryType::NotificationType: - QCOMPARE(activity._type, OCC::Activity::NotificationType); - break; - case OCC::ActivityListModel::ActivityEntryType::SyncFileItemType: - QCOMPARE(activity._type, OCC::Activity::SyncFileItemType); - QCOMPARE(activity._status, OCC::SyncFileItem::Success); - break; - case OCC::ActivityListModel::ActivityEntryType::ActivityType: - QCOMPARE(activity._type, OCC::Activity::ActivityType); - case OCC::ActivityListModel::ActivityEntryType::MoreActivitiesAvailableType: - break; - } - auto text = index.data(OCC::ActivityListModel::ActionTextRole).toString(); QVERIFY(index.data(OCC::ActivityListModel::ActionRole).canConvert()); const auto type = index.data(OCC::ActivityListModel::ActionRole).toInt(); - QVERIFY(type >= OCC::Activity::ActivityType); + QVERIFY(type >= OCC::Activity::DummyFetchingActivityType); QVERIFY(!index.data(OCC::ActivityListModel::AccountRole).toString().isEmpty()); QVERIFY(!index.data(OCC::ActivityListModel::ActionTextColorRole).toString().isEmpty()); @@ -664,6 +629,8 @@ private slots: QVERIFY(index.data(OCC::ActivityListModel::TalkNotificationMessageIdRole).canConvert()); QVERIFY(index.data(OCC::ActivityListModel::TalkNotificationMessageSentRole).canConvert()); + QVERIFY(index.data(OCC::ActivityListModel::ActivityRole).canConvert()); + // Unfortunately, trying to check anything relating to filepaths causes a crash // when the folder manager is invoked by the model to look for the relevant file }