UI: Improves texts displayed to the user when picking the wrong location for syncing

src/common/vfs.cpp

@@ -1,4 +1,4 @@
 /*
  * Copyright (C) by Dominik Schmidt <[email protected]>
  *
  * This library is free software; you can redistribute it and/or
@@ -16,7 +16,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
 #include "config.h"
 #include "vfs.h"
 #include "plugin.h"
 #include "version.h"
@@ -24,6 +24,7 @@
 
 #include "common/filesystembase.h"
 
+#include <QDir>
 #include <QPluginLoader>
 #include <QLoggingCategory>
 
@@ -65,21 +66,28 @@
     return {};
 }
 
-Result<bool, QString> Vfs::checkAvailability(const QString &path)
+Result<void, QString> Vfs::checkAvailability(const QString &path, Vfs::Mode mode)
 {
-    const auto mode = bestAvailableVfsMode();
 #ifdef Q_OS_WIN
     if (mode == Mode::WindowsCfApi) {
-        const auto fs = FileSystem::fileSystemForPath(path);
-        if (fs != QLatin1String("NTFS")) {
-            return tr("The Virtual filesystem feature requires a NTFS file system, %1 is using %2").arg(path, fs);
+        const auto info = QFileInfo(path);
+        if (QDir(info.canonicalPath()).isRoot()) {
+            return tr("Please choose a different location. %1 is a drive. It doesn't support virtual files.").arg(path);
+        }
+        if (const auto fileSystemForPath = FileSystem::fileSystemForPath(info.absoluteFilePath());
+            fileSystemForPath != QLatin1String("NTFS")) {
+            return tr("Please choose a different location. %1 isn't a NTFS files system. It doesn't support virtual files.").arg(path);
+        }
+        const auto type = GetDriveTypeW(reinterpret_cast<const wchar_t *>(QDir::toNativeSeparators(info.absoluteFilePath().mid(0, 3)).utf16()));
+        if (type == DRIVE_REMOTE) {
+            return tr("Please choose a different location. %1 is a network drive. It doesn't support virtual files.").arg(path);
         }
     }
 #else
     Q_UNUSED(mode)
     Q_UNUSED(path)
 #endif
-    return true;
+    return {};
 }
 
 void Vfs::start(const VfsSetupParams &params)

src/common/vfs.h

@@ -13,7 +13,7 @@
  */
 #pragma once
 
 #include "ocsynclib.h"
 #include "result.h"
 #include "syncfilestatus.h"
 #include "pinstate.h"
@@ -125,7 +125,7 @@
     static QString modeToString(Mode mode);
     static Optional<Mode> modeFromString(const QString &str);
 
-    static Result<bool, QString> checkAvailability(const QString &path);
+    static Result<void, QString> checkAvailability(const QString &path, OCC::Vfs::Mode mode);
 
     enum class AvailabilityError
     {

src/gui/accountsettings.cpp

@@ -1,4 +1,4 @@
 /*
  * Copyright (C) by Daniel Molkentin <[email protected]>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -708,8 +708,9 @@
         ac->setDisabled(Theme::instance()->enforceVirtualFilesSyncFolder());
     }
 
-    if (Theme::instance()->showVirtualFilesOption() && !folder->virtualFilesEnabled() && Vfs::checkAvailability(folder->path())) {
-        const auto mode = bestAvailableVfsMode();
+    if (const auto mode = bestAvailableVfsMode();
+        Theme::instance()->showVirtualFilesOption()
+        && !folder->virtualFilesEnabled() && Vfs::checkAvailability(folder->path(), mode)) {
         if (mode == Vfs::WindowsCfApi || ConfigFile().showExperimentalOptions()) {
             ac = menu->addAction(tr("Enable virtual file support %1 …").arg(mode == Vfs::WindowsCfApi ? QString() : tr("(experimental)")));
             // TODO: remove when UX decision is made

src/gui/folder.cpp

@@ -13,7 +13,7 @@
  * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
  * for more details.
  */
 #include "common/syncjournaldb.h"
 #include "config.h"
 
 #include "account.h"
@@ -81,7 +81,7 @@
     _syncResult.setStatus(status);
 
     // check if the local path exists
-    checkLocalPath();
+    const auto folderOk = checkLocalPath();
 
     _syncResult.setFolder(_definition.alias);
 
@@ -155,8 +155,10 @@
         saveToSettings();
     }
 
-    // Initialize the vfs plugin
-    startVfs();
+    if (folderOk) {
+        // Initialize the vfs plugin
+        startVfs();
+    }
 }
 
 Folder::~Folder()
@@ -169,7 +171,7 @@
     _engine.reset();
 }
 
-void Folder::checkLocalPath()
+bool Folder::checkLocalPath()
 {
     const QFileInfo fi(_definition.localPath);
     _canonicalLocalPath = fi.canonicalFilePath();
@@ -187,18 +189,22 @@
     if (FileSystem::isDir(_definition.localPath) && FileSystem::isReadable(_definition.localPath)) {
         qCDebug(lcFolder) << "Checked local path ok";
     } else {
+        QString error;
         // Check directory again
         if (!FileSystem::fileExists(_definition.localPath, fi)) {
-            _syncResult.appendErrorString(tr("Local folder %1 does not exist.").arg(_definition.localPath));
-            _syncResult.setStatus(SyncResult::SetupError);
-        } else if (!FileSystem::isDir(_definition.localPath)) {
-            _syncResult.appendErrorString(tr("%1 should be a folder but is not.").arg(_definition.localPath));
-            _syncResult.setStatus(SyncResult::SetupError);
-        } else if (!FileSystem::isReadable(_definition.localPath)) {
-            _syncResult.appendErrorString(tr("%1 is not readable.").arg(_definition.localPath));
+            error = tr("Please choose a different location. The folder %1 doesn't exist.").arg(_definition.localPath);
+        } else if (!fi.isDir()) {
+            error = tr("Please choose a different location. %1 isn't a valid folder.").arg(_definition.localPath);
+        } else if (!fi.isReadable()) {
+            error = tr("Please choose a different location. %1 isn't a readable folder.").arg(_definition.localPath);
+        }
+        if (!error.isEmpty()) {
+            _syncResult.appendErrorString(error);
             _syncResult.setStatus(SyncResult::SetupError);
+            return false;
         }
     }
+    return true;
 }
 
 QString Folder::shortGuiRemotePathOrAppName() const
@@ -297,7 +303,7 @@
 
 bool Folder::canSync() const
 {
-    return !syncPaused() && accountState()->isConnected();
+    return !syncPaused() && accountState()->isConnected() && _syncResult.status() != SyncResult::SetupError;
 }
 
 void Folder::setSyncPaused(bool paused)
@@ -507,7 +513,14 @@
     ENFORCE(_vfs);
     ENFORCE(_vfs->mode() == _definition.virtualFilesMode);
 
+    const auto result = Vfs::checkAvailability(path(), _vfs->mode());
+    if (!result) {
+        _syncResult.appendErrorString(result.error());
+        _syncResult.setStatus(SyncResult::SetupError);
+        return;
+    }
+
     VfsSetupParams vfsParams;
     vfsParams.filesystemPath = path();
     vfsParams.displayName = shortGuiRemotePathOrAppName();
     vfsParams.alias = alias();

src/gui/folder.h

@@ -17,7 +17,7 @@
 #ifndef MIRALL_FOLDER_H
 #define MIRALL_FOLDER_H
 
 #include "syncresult.h"
 #include "progressdispatcher.h"
 #include "common/syncjournaldb.h"
 #include "networkjobs.h"
@@ -474,7 +474,7 @@
 
     void showSyncResultPopup();
 
-    void checkLocalPath();
+    bool checkLocalPath();
 
     SyncOptions initializeSyncOptions() const;
 

src/gui/folderman.cpp

@@ -1,4 +1,4 @@
 /*
  * Copyright (C) by Klaas Freitag <[email protected]>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -44,6 +44,13 @@
 constexpr auto settingsFoldersC = "Folders";
 constexpr auto settingsVersionC = "version";
 constexpr auto maxFoldersVersion = 1;
+const char versionC[] = "version";
+
+int numberOfSyncJournals(const QString &path)
+{
+    return QDir(path).entryList({ QStringLiteral(".sync_*.db"), QStringLiteral("._sync_*.db") }, QDir::Hidden | QDir::Files).size();
+}
+
 }
 
 namespace OCC {
@@ -1793,36 +1800,41 @@
 static QString checkPathValidityRecursive(const QString &path)
 {
     if (path.isEmpty()) {
-        return FolderMan::tr("No valid folder selected!");
+        return FolderMan::tr("Please choose a different location. The selected folder isn't valid.");
     }
 
 #ifdef Q_OS_WIN
     Utility::NtfsPermissionLookupRAII ntfs_perm;
 #endif
     const QFileInfo selFile(path);
+    if (numberOfSyncJournals(selFile.filePath()) != 0) {
+        return FolderMan::tr("Please choose a different location. %1 is already being used as a sync folder.").arg(QDir::toNativeSeparators(selFile.filePath()));
+    }
 
     if (!FileSystem::fileExists(path)) {
         QString parentPath = selFile.dir().path();
-        if (parentPath != path)
+        if (parentPath != path) {
             return checkPathValidityRecursive(parentPath);
-        return FolderMan::tr("The selected path does not exist!");
+        }
+
+        return FolderMan::tr("Please choose a different location. The path %1 doesn't exist.").arg(QDir::toNativeSeparators(selFile.filePath()));
     }
 
     if (!FileSystem::isDir(path)) {
-        return FolderMan::tr("The selected path is not a folder!");
+        return FolderMan::tr("Please choose a different location. The path %1 isn't a folder.").arg(QDir::toNativeSeparators(selFile.filePath()));
     }
 
     #ifdef Q_OS_WIN
     if (!FileSystem::isWritable(path)) {
         // isWritable() doesn't cover all NTFS permissions
         // try creating and removing a test file, and make sure it is excluded from sync
         if (!Utility::canCreateFileInPath(path)) {
-            return FolderMan::tr("You have no permission to write to the selected folder!");
+            return FolderMan::tr("Please choose a different location. You don't have enough permissions to write to %1.", "folder location").arg(QDir::toNativeSeparators(selFile.filePath()));
         }
     }
     #else
     if (!FileSystem::isWritable(path)) {
-        return FolderMan::tr("You have no permission to write to the selected folder!");
+        return FolderMan::tr("Please choose a different location. You don't have enough permissions to write to %1.", "folder location").arg(QDir::toNativeSeparators(selFile.filePath()));
     }
     #endif
     return {};
@@ -1875,17 +1887,15 @@
         bool differentPaths = QString::compare(folderDir, userDir, cs) != 0;
         if (differentPaths && folderDir.startsWith(userDir, cs)) {
             result.first = FolderMan::PathValidityResult::ErrorContainsFolder;
-            result.second = tr("The local folder %1 already contains a folder used in a folder sync connection. "
-                              "Please pick another one!")
+            result.second = tr("Please choose a different location. %1 is already being used as a sync folder.")
                                 .arg(QDir::toNativeSeparators(path));
             return result;
         }
 
         if (differentPaths && userDir.startsWith(folderDir, cs)) {
             result.first = FolderMan::PathValidityResult::ErrorContainedInFolder;
-            result.second = tr("The local folder %1 is already contained in a folder used in a folder sync connection. "
-                      "Please pick another one!")
-                .arg(QDir::toNativeSeparators(path));
+            result.second = tr("Please choose a different location. %1 is already contained in a folder used as a sync folder.")
+                                .arg(QDir::toNativeSeparators(path));
             return result;
         }
 
@@ -1899,8 +1909,9 @@
 
             if (serverUrl == folderUrl) {
                 result.first = FolderMan::PathValidityResult::ErrorNonEmptyFolder;
-                result.second = tr("There is already a sync from the server to this local folder. "
-                          "Please pick another local folder!");
+                result.second = tr("Please choose a different location. %1 is already being used as a sync folder for %2.", "folder location, server url")
+                                    .arg(QDir::toNativeSeparators(path),
+                                         serverUrl.toString());
                 return result;
             }
         }
@@ -2027,4 +2038,20 @@
     }
 }
 
+bool FolderMan::checkVfsAvailability(const QString &path, Vfs::Mode mode) const
+{
+    return unsupportedConfiguration(path) && Vfs::checkAvailability(path, mode);
+}
+
+Result<void, QString> FolderMan::unsupportedConfiguration(const QString &path) const
+{
+    if (numberOfSyncJournals(path) > 1) {
+        return tr("Multiple accounts are sharing the folder %1.\n"
+                  "This configuration is know to lead to dataloss and is no longer supported.\n"
+                  "Please consider removing this folder from the account and adding it again.")
+            .arg(path);
+    }
+    return {};
+}
+
 } // namespace OCC

src/gui/folderman.h

@@ -16,7 +16,7 @@
 #ifndef FOLDERMAN_H
 #define FOLDERMAN_H
 
 #include <QByteArray>
 #include <QObject>
 #include <QQueue>
 #include <QList>
@@ -231,6 +231,11 @@
     /** removes current user from the share **/
     void leaveShare(const QString &localFile);
 
+    /** Whether or not vfs is supported in the location. */
+    bool checkVfsAvailability(const QString &path, Vfs::Mode mode = bestAvailableVfsMode()) const;
+
+    /** If the folder configuration is no longer supported this will return an error string */
+    Result<void, QString> unsupportedConfiguration(const QString &path) const;
 signals:
     /**
       * signal to indicate a folder has changed its sync state.

src/gui/folderstatusmodel.cpp

@@ -250,8 +250,15 @@ QVariant FolderStatusModel::data(const QModelIndex &index, int role) const
         return (folder->syncResult().hasUnresolvedConflicts())
             ? QStringList(tr("There are unresolved conflicts. Click for details."))
             : QStringList();
-    case FolderStatusDelegate::FolderErrorMsg:
-        return folder->syncResult().errorStrings();
+    case FolderStatusDelegate::FolderErrorMsg: {
+        auto errors = folder->syncResult().errorStrings();
+        const auto legacyError = FolderMan::instance()->unsupportedConfiguration(folder->path());
+        if (!legacyError) {
+            // the error message might contain new lines, the delegate only expect multiple single line values
+            errors.append(legacyError.error().split(QLatin1Char('\n')));
+        }
+        return errors;
+    }
     case FolderStatusDelegate::FolderInfoMsg:
         return folder->virtualFilesEnabled() && folder->vfs().mode() != Vfs::Mode::WindowsCfApi
             ? QStringList(tr("Virtual file support is enabled."))

src/gui/folderwizard.cpp

@@ -1,4 +1,4 @@
 /*
  * Copyright (C) by Duncan Mac-Vicar P. <[email protected]>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -63,9 +63,9 @@
 {
     QString ret;
     if (warnings.count() == 1) {
-        ret = tr("<b>Warning:</b> %1").arg(warnings.first());
+        ret = tr("%1").arg(warnings.first());
     } else if (warnings.count() > 1) {
-        ret = tr("<b>Warning:</b>") + " <ul>";
+        ret = "<ul>";
         Q_FOREACH (QString warning, warnings) {
             ret += QString::fromLatin1("<li>%1</li>").arg(warning);
         }
@@ -484,34 +484,39 @@
 
 bool FolderWizardRemotePath::isComplete() const
 {
-    if (!_ui.folderTreeWidget->currentItem())
+    if (!_ui.folderTreeWidget->currentItem()) {
         return false;
+    }
 
-    QStringList warnStrings;
-    QString dir = _ui.folderTreeWidget->currentItem()->data(0, Qt::UserRole).toString();
-    if (!dir.startsWith(QLatin1Char('/'))) {
-        dir.prepend(QLatin1Char('/'));
+    auto targetPath = _ui.folderTreeWidget->currentItem()->data(0, Qt::UserRole).toString();
+    if (!targetPath.startsWith(QLatin1Char('/'))) {
+        targetPath.prepend(QLatin1Char('/'));
     }
-    wizard()->setProperty("targetPath", dir);
+    wizard()->setProperty("targetPath", targetPath);
 
-    Folder::Map map = FolderMan::instance()->map();
-    Folder::Map::const_iterator i = map.constBegin();
-    for (i = map.constBegin(); i != map.constEnd(); i++) {
-        auto *f = static_cast<Folder *>(i.value());
-        if (f->accountState()->account() != _account) {
+    for (const auto folder : qAsConst(FolderMan::instance()->map())) {
+        if (folder->accountState()->account() != _account) {
             continue;
         }
-        QString curDir = f->remotePathTrailingSlash();
-        if (QDir::cleanPath(dir) == QDir::cleanPath(curDir)) {
-            warnStrings.append(tr("This folder is already being synced."));
-        } else if (dir.startsWith(curDir)) {
-            warnStrings.append(tr("You are already syncing <i>%1</i>, which is a parent folder of <i>%2</i>.").arg(Utility::escape(curDir), Utility::escape(dir)));
-        } else if (curDir.startsWith(dir)) {
-            warnStrings.append(tr("You are already syncing <i>%1</i>, which is a subfolder of <i>%2</i>.").arg(Utility::escape(curDir), Utility::escape(dir)));
+
+        const auto remoteDir = folder->remotePathTrailingSlash();
+        const auto localDir = folder->cleanPath();
+        if (QDir::cleanPath(targetPath) == QDir::cleanPath(remoteDir)) {
+            showWarn(tr("Please choose a different location. %1 is already being synced to %2.").arg(Utility::escape(remoteDir), Utility::escape(localDir)));
+            break;
+        }
+
+        if (targetPath.startsWith(remoteDir)) {
+            showWarn(tr("Please choose a different location. %1 is already being synced to %2.").arg(Utility::escape(targetPath), Utility::escape(localDir)));
+            break;
+        }
+
+        if (remoteDir.startsWith(targetPath)) {
+            showWarn(tr("Please choose a different location. %1 is already being synced to %2.").arg(Utility::escape(remoteDir), Utility::escape(localDir)));
+            break;
         }
     }
 
-    showWarn(formatWarnings(warnStrings));
     return true;
 }
 
@@ -620,11 +625,15 @@
 
 bool FolderWizardSelectiveSync::validatePage()
 {
-    const bool useVirtualFiles = _virtualFilesCheckBox && _virtualFilesCheckBox->isChecked();
+    const auto mode = bestAvailableVfsMode();
+    const bool useVirtualFiles = (mode == Vfs::WindowsCfApi) || (_virtualFilesCheckBox && _virtualFilesCheckBox->isChecked());
     if (useVirtualFiles) {
-        const auto availability = Vfs::checkAvailability(wizard()->field(QStringLiteral("sourceFolder")).toString());
+        const auto availability = Vfs::checkAvailability(wizard()->field(QStringLiteral("sourceFolder")).toString(), mode);
         if (!availability) {
-            auto msg = new QMessageBox(QMessageBox::Warning, tr("Virtual files are not available for the selected folder"), availability.error(), QMessageBox::Ok, this);
+            auto msg = new QMessageBox(QMessageBox::Warning,
+                                       tr("Virtual files are not supported at the selected location"),
+                                       availability.error(),
+                                       QMessageBox::Ok, this);
             msg->setAttribute(Qt::WA_DeleteOnClose);
             msg->open();
             return false;