Align sync status icons

TheOneRing · Klaas Freitag · commit 5970ddb50ddd · 2021-08-10T18:57:01.000+02:00
Display info state if we have ignored issues Fixes: #7715 Fixes: #7365 Fixes: #7200 Fixes: #5860
diff --git a/changelog/unreleased/7715 b/changelog/unreleased/7715
@@ -0,0 +1,13 @@
+Enhancement: Display the information state in case we encountered ignored errors
+
+If syncing a file fails multiple times we mark it as ignored to skip it for a certain amount of time.
+If we have ignored files we are not in sync, we now don't display the green icon.
+
+Additionally this change aligns the icon displayed in the system tray with the icon displayed in the app.
+
+https://github.com/owncloud/client/issues/7715
+https://github.com/owncloud/client/issues/7365
+https://github.com/owncloud/client/issues/7200
+https://github.com/owncloud/client/issues/5860
+
+https://github.com/owncloud/client/pull/8858
diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp
@@ -40,9 +40,60 @@ static const char versionC[] = "version";
 static const int maxFoldersVersion = 1;
 
 namespace OCC {
-
 Q_LOGGING_CATEGORY(lcFolderMan, "gui.folder.manager", QtInfoMsg)
 
+void TrayOverallStatusResult::addResult(const SyncResult &result)
+{
+    _overallStatus._numNewConflictItems += result._numNewConflictItems;
+    _overallStatus._numErrorItems += result._numErrorItems;
+
+    switch (result.status()) {
+    case SyncResult::Success:
+        // only if every result is success
+        if (_overallStatus.status() == SyncResult::Undefined) {
+            _overallStatus.setStatus(SyncResult::Success);
+        }
+        break;
+    case SyncResult::Paused:
+        Q_FALLTHROUGH();
+    case SyncResult::SyncAbortRequested:
+        // Problem has a high enum value but real problems and errors
+        // take precedence
+        if (_overallStatus.status() < SyncResult::Problem) {
+            _overallStatus.setStatus(SyncResult::Paused);
+        }
+        break;
+    case SyncResult::NotYetStarted:
+        Q_FALLTHROUGH();
+    case SyncResult::SyncPrepare:
+        Q_FALLTHROUGH();
+    case SyncResult::SyncRunning:
+        if (_overallStatus.status() < SyncResult::Problem) {
+            _overallStatus.setStatus(SyncResult::SyncRunning);
+        }
+        break;
+    case SyncResult::Undefined:
+        if (_overallStatus.status() < SyncResult::Problem) {
+            _overallStatus.setStatus(SyncResult::Problem);
+        }
+        break;
+    case SyncResult::Problem:
+        Q_FALLTHROUGH();
+    case SyncResult::Error:
+        Q_FALLTHROUGH();
+    case SyncResult::SetupError:
+        if (_overallStatus.status() < result.status()) {
+            _overallStatus.setStatus(result.status());
+        }
+        break;
+    }
+}
+
+const SyncResult &TrayOverallStatusResult::overallStatus() const
+{
+    return _overallStatus;
+}
+
 FolderMan *FolderMan::_instance = nullptr;
 
 FolderMan::FolderMan(QObject *parent)
@@ -1220,107 +1271,39 @@ TrayOverallStatusResult FolderMan::trayOverallStatus(const QList<Folder *> &fold
 {
     TrayOverallStatusResult result;
 
-    int cnt = folders.count();
-
     // if one folder: show the state of the one folder.
     // if more folders:
     // if one of them has an error -> show error
     // if one is paused, but others ok, show ok
     // do not show "problem" in the tray
     //
-    if (cnt == 1) {
-        Folder *folder = folders.at(0);
-        if (folder) {
-            auto syncResult = folder->syncResult();
-            if (folder->syncPaused()) {
-                result.overallStatus = SyncResult::Paused;
-            } else {
-                SyncResult::Status syncStatus = syncResult.status();
-                switch (syncStatus) {
-                case SyncResult::Undefined:
-                    result.overallStatus = SyncResult::Error;
-                    break;
-                case SyncResult::Problem: // don't show the problem icon in tray.
-                    result.overallStatus = SyncResult::Success;
-                    break;
-                default:
-                    result.overallStatus = syncStatus;
-                    break;
-                }
-            }
+    if (folders.count() == 1) {
+        const auto currentFolderLastSyncDone = QDateTime::currentDateTime().addMSecs(-1 * folders.first()->msecSinceLastSync().count());
 
-            result.hasUnresolvedConflicts = syncResult.hasUnresolvedConflicts();
-
-            auto currentFolderLastSyncDone = QDateTime::currentDateTime().addMSecs(-1 * folder->msecSinceLastSync().count());
-
-            if (result.lastSyncDone.isNull()) {
+        if (result.lastSyncDone.isNull()) {
+            result.lastSyncDone = currentFolderLastSyncDone;
+        } else {
+            if (currentFolderLastSyncDone > result.lastSyncDone) {
                 result.lastSyncDone = currentFolderLastSyncDone;
-            } else {
-                if (currentFolderLastSyncDone > result.lastSyncDone) {
-                    result.lastSyncDone = currentFolderLastSyncDone;
-                }
-            }
-        }
-    } else {
-        int errorsSeen = 0;
-        int goodSeen = 0;
-        int abortOrPausedSeen = 0;
-        int runSeen = 0;
-        int various = 0;
-
-        for (auto *folder : folders) {
-            SyncResult folderResult = folder->syncResult();
-            if (folder->syncPaused()) {
-                abortOrPausedSeen++;
-            } else {
-                SyncResult::Status syncStatus = folderResult.status();
-
-                switch (syncStatus) {
-                case SyncResult::Undefined:
-                case SyncResult::NotYetStarted:
-                    various++;
-                    break;
-                case SyncResult::SyncPrepare:
-                case SyncResult::SyncRunning:
-                    runSeen++;
-                    break;
-                case SyncResult::Problem: // don't show the problem icon in tray.
-                case SyncResult::Success:
-                    goodSeen++;
-                    break;
-                case SyncResult::Error:
-                case SyncResult::SetupError:
-                    errorsSeen++;
-                    break;
-                case SyncResult::SyncAbortRequested:
-                case SyncResult::Paused:
-                    abortOrPausedSeen++;
-                    // no default case on purpose, check compiler warnings
-                }
             }
-            if (folderResult.hasUnresolvedConflicts())
-                result.hasUnresolvedConflicts = true;
         }
-        if (errorsSeen > 0) {
-            result.overallStatus = SyncResult::Error;
-        } else if (abortOrPausedSeen > 0 && abortOrPausedSeen == cnt) {
-            // only if all folders are paused
-            result.overallStatus = SyncResult::Paused;
-        } else if (runSeen > 0) {
-            result.overallStatus = SyncResult::SyncRunning;
-        } else if (goodSeen > 0) {
-            result.overallStatus = SyncResult::Success;
+    }
+    for (auto *folder : folders) {
+        const SyncResult folderResult = folder->syncResult();
+        if (folder->syncPaused()) {
+            result.addResult(SyncResult { SyncResult::Paused });
+        } else {
+            result.addResult(folderResult);
         }
     }
-
     return result;
 }
 
 QString FolderMan::trayTooltipStatusString(
-    SyncResult::Status syncStatus, bool hasUnresolvedConflicts, bool paused)
+    const SyncResult &result, bool paused)
 {
     QString folderMessage;
-    switch (syncStatus) {
+    switch (result.status()) {
     case SyncResult::Undefined:
         folderMessage = tr("Undefined State.");
         break;
@@ -1335,7 +1318,7 @@ QString FolderMan::trayTooltipStatusString(
         break;
     case SyncResult::Success:
     case SyncResult::Problem:
-        if (hasUnresolvedConflicts) {
+        if (result.hasUnresolvedConflicts()) {
             folderMessage = tr("Sync was successful, unresolved conflicts.");
         } else {
             folderMessage = tr("Last Sync was successful.");
@@ -1539,4 +1522,5 @@ void FolderMan::restartApplication()
     }
 }
 
+
 } // namespace OCC
diff --git a/src/gui/folderman.h b/src/gui/folderman.h
@@ -43,9 +43,13 @@ class LockWatcher;
 class TrayOverallStatusResult
 {
 public:
-    SyncResult::Status overallStatus;
-    bool hasUnresolvedConflicts;
     QDateTime lastSyncDone;
+
+    void addResult(const SyncResult &result);
+    const SyncResult &overallStatus() const;
+
+private:
+    SyncResult _overallStatus;
 };
 
 /**
@@ -145,7 +149,7 @@ class FolderMan : public QObject
     bool startFromScratch(const QString &);
 
     /// Produce text for use in the tray tooltip
-    static QString trayTooltipStatusString(SyncResult::Status syncStatus, bool hasUnresolvedConflicts, bool paused);
+    static QString trayTooltipStatusString(const SyncResult &result, bool paused);
 
     /**
      * Compute status summarizing multiple folders
diff --git a/src/gui/folderstatusmodel.cpp b/src/gui/folderstatusmodel.cpp
@@ -239,28 +239,10 @@ QVariant FolderStatusModel::data(const QModelIndex &index, int role) const
     case FolderStatusDelegate::FolderStatusIconRole:
         if (accountConnected) {
             auto theme = Theme::instance();
-            auto status = f->syncResult().status();
             if (f->syncPaused()) {
                 return theme->folderDisabledIcon();
             } else {
-                if (status == SyncResult::SyncPrepare) {
-                    return theme->syncStateIcon(SyncResult::SyncRunning);
-                } else if (status == SyncResult::Undefined) {
-                    return theme->syncStateIcon(SyncResult::SyncRunning);
-                } else {
-                    // The "Problem" *result* just means some files weren't
-                    // synced, so we show "Success" in these cases. But we
-                    // do use the "Problem" *icon* for unresolved conflicts.
-                    if (status == SyncResult::Success || status == SyncResult::Problem) {
-                        if (f->syncResult().hasUnresolvedConflicts()) {
-                            return theme->syncStateIcon(SyncResult::Problem);
-                        } else {
-                            return theme->syncStateIcon(SyncResult::Success);
-                        }
-                    } else {
-                        return theme->syncStateIcon(status);
-                    }
-                }
+                return theme->syncStateIcon(f->syncResult().status());
             }
         } else {
             return Theme::instance()->folderOfflineIcon();
diff --git a/src/gui/libcloudproviders/libcloudproviders.cpp b/src/gui/libcloudproviders/libcloudproviders.cpp
@@ -213,8 +213,7 @@ void LibCloudProvidersPrivate::updateFolderExport()
             status = CLOUD_PROVIDERS_ACCOUNT_STATUS_ERROR;
 
         auto message = FolderMan::trayTooltipStatusString(
-            syncResult.status(),
-            syncResult.hasUnresolvedConflicts(),
+            syncResult,
             folder->syncPaused());
 
         cloud_providers_account_exporter_set_status(folderExport._exporter, status);
diff --git a/src/gui/owncloudgui.cpp b/src/gui/owncloudgui.cpp
@@ -265,42 +265,31 @@ void ownCloudGui::slotComputeOverallSyncStatus()
 
     auto trayOverallStatusResult = FolderMan::trayOverallStatus(map.values());
 
-    // If the sync succeeded but there are unresolved conflicts,
-    // show the problem icon!
-    auto iconStatus = trayOverallStatusResult.overallStatus;
-    if (iconStatus == SyncResult::Success && trayOverallStatusResult.hasUnresolvedConflicts) {
-        iconStatus = SyncResult::Problem;
-    }
-
-    // If we don't get a status for whatever reason, that's a Problem
-    if (iconStatus == SyncResult::Undefined) {
-        iconStatus = SyncResult::Problem;
-    }
-
-    QIcon statusIcon = Theme::instance()->syncStateIcon(iconStatus, true, contextMenuVisible());
+    const QIcon statusIcon = Theme::instance()->syncStateIcon(trayOverallStatusResult.overallStatus(), true, contextMenuVisible());
     _tray->setIcon(statusIcon);
 
     // create the tray blob message, check if we have an defined state
     if (map.count() > 0) {
 #ifdef Q_OS_WIN
         // Windows has a 128-char tray tooltip length limit.
-        trayMessage = FolderMan::instance()->trayTooltipStatusString(trayOverallStatusResult.overallStatus, trayOverallStatusResult.hasUnresolvedConflicts, false);
+        trayMessage = FolderMan::instance()->trayTooltipStatusString(trayOverallStatusResult.overallStatus(), false);
 #else
         QStringList allStatusStrings;
         for (auto *folder : map) {
             QString folderMessage = FolderMan::trayTooltipStatusString(
-                folder->syncResult().status(),
-                folder->syncResult().hasUnresolvedConflicts(),
+                folder->syncResult(),
                 folder->syncPaused());
             allStatusStrings += tr("Folder %1: %2").arg(folder->shortGuiLocalPath(), folderMessage);
         }
         trayMessage = allStatusStrings.join(QLatin1String("\n"));
 #endif
         _tray->setToolTip(trayMessage);
 
-        if (trayOverallStatusResult.overallStatus == SyncResult::Success || trayOverallStatusResult.overallStatus == SyncResult::Problem) {
-            if (trayOverallStatusResult.hasUnresolvedConflicts) {
-                setStatusText(tr("Unresolved conflicts"));
+        if (trayOverallStatusResult.overallStatus().status() == SyncResult::Success || trayOverallStatusResult.overallStatus().status() == SyncResult::Problem) {
+            if (trayOverallStatusResult.overallStatus().hasUnresolvedConflicts()) {
+                setStatusText(tr("Unresolved %1 conflicts").arg(QString::number(trayOverallStatusResult.overallStatus().numNewConflictItems())));
+            } else if (trayOverallStatusResult.overallStatus().numErrorItems() != 0) {
+                setStatusText(tr("Errors %1").arg(QString::number(trayOverallStatusResult.overallStatus().numErrorItems())));
             } else {
                 QString lastSyncDoneString;
 
@@ -313,7 +302,7 @@ void ownCloudGui::slotComputeOverallSyncStatus()
 
                 setStatusText(tr("Up to date (%1)").arg(lastSyncDoneString));
             }
-        } else if (trayOverallStatusResult.overallStatus == SyncResult::Paused) {
+        } else if (trayOverallStatusResult.overallStatus().status() == SyncResult::Paused) {
             setStatusText(tr("Synchronization is paused"));
         } else {
             setStatusText(tr("Error during synchronization"));
diff --git a/src/libsync/syncresult.cpp b/src/libsync/syncresult.cpp
@@ -14,24 +14,16 @@
 
 #include "syncresult.h"
 #include "progressdispatcher.h"
+#include "theme.h"
 
 namespace OCC {
 
-SyncResult::SyncResult()
-    : _status(Undefined)
-    , _foundFilesNotSynced(false)
-    , _folderStructureWasChanged(false)
-    , _numNewItems(0)
-    , _numRemovedItems(0)
-    , _numUpdatedItems(0)
-    , _numRenamedItems(0)
-    , _numNewConflictItems(0)
-    , _numOldConflictItems(0)
-    , _numErrorItems(0)
-
+SyncResult::SyncResult(Status status)
+    : _status(status)
 {
 }
 
+
 SyncResult::Status SyncResult::status() const
 {
     return _status;
diff --git a/src/libsync/syncresult.h b/src/libsync/syncresult.h
diff --git a/src/libsync/theme.cpp b/src/libsync/theme.cpp
diff --git a/src/libsync/theme.h b/src/libsync/theme.h
