Merge pull request #5623 from nextcloud/backport/5583/stable-3.8

[stable-3.8] Properly handle all fatal errors during edit locally setup procedure
nextcloud · Apr 24, 2023 · e96bd76 · e96bd76
2 parents 4906327 + 435bd81
commit e96bd76
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 17 deletions.
diff --git a/src/gui/editlocallyjob.cpp b/src/gui/editlocallyjob.cpp
@@ -89,6 +89,9 @@ void EditLocallyJob::startTokenRemoteCheck()
                                         << "accountState:" << _accountState
                                         << "relPath:" << _relPath
                                         << "token:" << _token;
+
+        showError(tr("Could not start editing locally."),
+                  tr("An error occurred trying to verify the request to edit locally."));
         return;
     }
 
@@ -152,7 +155,7 @@ void EditLocallyJob::proceedWithSetup()
     _localFilePath = _folderForFile->path() + _relativePathToRemoteRoot;
 
     Systray::instance()->destroyEditFileLocallyLoadingDialog();
-    Q_EMIT setupFinished();
+    startEditLocally();
 }
 
 void EditLocallyJob::findAfolderAndConstructPaths()
@@ -198,6 +201,8 @@ void EditLocallyJob::fetchRemoteFileParentInfo()
 
     if (_relPathParent == QStringLiteral("/")) {
         qCWarning(lcEditLocallyJob) << "LsColJob must only be used for nested folders.";
+        showError(tr("Could not start editing locally."),
+                  tr("An error occurred during data retrieval."));
         return;
     }
 
@@ -251,7 +256,12 @@ bool EditLocallyJob::checkIfFileParentSyncIsNeeded()
 
 void EditLocallyJob::startSyncBeforeOpening()
 {
-    eraseBlacklistRecordForItem();
+    if (!eraseBlacklistRecordForItem()) {
+        showError(tr("Could not start editing locally."),
+                  tr("An error occurred trying to synchronise the file to edit locally."));
+        return;
+    }
+
     if (!checkIfFileParentSyncIsNeeded()) {
         processLocalItem();
         return;
@@ -263,20 +273,24 @@ void EditLocallyJob::startSyncBeforeOpening()
     FolderMan::instance()->forceSyncForFolder(_folderForFile);
 }
 
-void EditLocallyJob::eraseBlacklistRecordForItem()
+bool EditLocallyJob::eraseBlacklistRecordForItem()
 {
     if (!_folderForFile || !_fileParentItem) {
         qCWarning(lcEditLocallyJob) << "_folderForFile or _fileParentItem is invalid!";
-        return;
+        return false;
     }
+
     Q_ASSERT(!_folderForFile->isSyncRunning());
     if (_folderForFile->isSyncRunning()) {
         qCWarning(lcEditLocallyJob) << "_folderForFile is syncing";
-        return;
+        return false;
     }
+
     if (_folderForFile->journalDb()->errorBlacklistEntry(_fileParentItem->_file).isValid()) {
         _folderForFile->journalDb()->wipeErrorBlacklistEntry(_fileParentItem->_file);
     }
+
+    return true;
 }
 
 const QString EditLocallyJob::getRelativePathToRemoteRootForFile() const
@@ -438,6 +452,8 @@ void EditLocallyJob::startEditLocally()
                                         << "fileName:" << _fileName
                                         << "localFilePath:" << _localFilePath
                                         << "folderForFile:" << _folderForFile;
+
+        showError(tr("Could not start editing locally."), tr("An error occurred during setup."));
         return;
     }
 
@@ -494,13 +510,17 @@ void EditLocallyJob::slotDirectoryListingIterated(const QString &name, const QMa
 
     if (_relPathParent == QStringLiteral("/")) {
         qCWarning(lcEditLocallyJob) << "LsColJob must only be used for nested folders.";
+        showError(tr("Could not start editing locally."),
+                  tr("An error occurred during data retrieval."));
         return;
     }
 
     const auto job = qobject_cast<LsColJob*>(sender());
     Q_ASSERT(job);
     if (!job) {
         qCWarning(lcEditLocallyJob) << "Must call slotDirectoryListingIterated from a signal.";
+        showError(tr("Could not start editing locally."),
+                  tr("An error occurred during data retrieval."));
         return;
     }
 
@@ -524,8 +544,9 @@ void EditLocallyJob::slotItemDiscovered(const OCC::SyncFileItemPtr &item)
     Q_ASSERT(item && !item->isEmpty());
     if (!item || item->isEmpty()) {
         qCWarning(lcEditLocallyJob) << "invalid item";
-    }
-    if (item->_file == _relativePathToRemoteRoot) {
+        showError(tr("Could not start editing locally."),
+                  tr("An error occurred trying to synchronise the file to edit locally."));
+    } else if (item->_file == _relativePathToRemoteRoot) {
         disconnect(&_folderForFile->syncEngine(), &SyncEngine::itemDiscovered, this, &EditLocallyJob::slotItemDiscovered);
         if (item->_instruction == CSYNC_INSTRUCTION_NONE) {
             // return early if the file is already in sync
@@ -543,6 +564,8 @@ void EditLocallyJob::openFile()
 
     if(_localFilePath.isEmpty()) {
         qCWarning(lcEditLocallyJob) << "Could not edit locally. Invalid local file path.";
+        showError(tr("Could not start editing locally."),
+                  tr("Invalid local file path."));
         return;
     }
 

diff --git a/src/gui/editlocallyjob.h b/src/gui/editlocallyjob.h
@@ -43,18 +43,17 @@ class EditLocallyJob : public QObject
     [[nodiscard]] static QString prefixSlashToPath(const QString &path);
 
 signals:
-    void setupFinished();
     void error(const QString &message, const QString &informativeText);
     void finished();
     void callShowError(const QString &message, const QString &informativeText);
+
 public slots:
     void startSetup();
     void startEditLocally();
 
 private slots:
     void fetchRemoteFileParentInfo();
     void startSyncBeforeOpening();
-    void eraseBlacklistRecordForItem();
 
     void startTokenRemoteCheck();
     void proceedWithSetup();
@@ -85,6 +84,7 @@ private slots:
 
 private:
     [[nodiscard]] bool checkIfFileParentSyncIsNeeded(); // returns true if sync will be needed, false otherwise
+    [[nodiscard]] bool eraseBlacklistRecordForItem();
     [[nodiscard]] const QString getRelativePathToRemoteRootForFile() const; // returns either '/' or a (relative path - Folder::remotePath()) for folders pointing to a non-root remote path e.g. '/subfolder' instead of '/'
     [[nodiscard]] const QString getRelativePathParent() const;
 

diff --git a/src/gui/editlocallymanager.cpp b/src/gui/editlocallymanager.cpp
@@ -79,14 +79,9 @@ void EditLocallyManager::createJob(const QString &userId,
     _jobs.insert(token, job);
 
     const auto removeJob = [this, token] { _jobs.remove(token); };
-    const auto setupJob = [job] { job->startEditLocally(); };
-
-    connect(job.data(), &EditLocallyJob::error,
-            this, removeJob);
-    connect(job.data(), &EditLocallyJob::finished,
-            this, removeJob);
-    connect(job.data(), &EditLocallyJob::setupFinished,
-            job.data(), setupJob);
+
+    connect(job.data(), &EditLocallyJob::error, this, removeJob);
+    connect(job.data(), &EditLocallyJob::finished, this, removeJob);
 
     job->startSetup();
 }