Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WIP: Read default folder configuration from a file on the server #6761

Closed
wants to merge 1 commit into from
Closed

WIP: Read default folder configuration from a file on the server #6761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 82 additions & 24 deletions src/gui/owncloudsetupwizard.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,35 @@ void OwncloudSetupWizard::testOwnCloudConnect()
// so don't automatically follow redirects.
job->setFollowRedirects(false);
job->setProperties(QList<QByteArray>() << "getlastmodified");
connect(job, &PropfindJob::result, _ocWizard, &OwncloudWizard::successfulStep);
connect(job, &PropfindJob::result, this, [this, account] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if the code for triggering and handling the request checking the server here was a separate function. At least it must have a comment explaining what's going on.

auto *job2 = account->sendRequest("GET", Utility::concatUrlPath(account->davUrl(), ".desktopclientconfig"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if this didn't store the configuration in a user-visible / editable file. I understand getting support on the server may be hard/limiting, and that this may be a reasonable tradeoff, but:

  • users will change / delete this file
  • users will think this contains their current desktop client configuration when it doesn't

job2->setIgnoreCredentialFailure(true);
QObject::connect(job2, &SimpleNetworkJob::finishedSignal, this, [this, account] (QNetworkReply *reply) {
if (reply->error()) {
if (reply->error() == QNetworkReply::ContentNotFoundError) {
// Ignore a 404 error, the file can be missing (default case)
_ocWizard->successfulStep();
} else {
// Note: slotAuthError uses sender() which should be job2
slotAuthError();
}
return;
}
auto jsonData = reply->readAll();
QJsonParseError jsonParseError;
QJsonDocument json = QJsonDocument::fromJson(jsonData, &jsonParseError);
if (jsonParseError.error != QJsonParseError::NoError) {
// Invalid JSON
qCWarning(lcWizard) << "Error while parsing .desktopclientconfig" << jsonParseError.errorString() << jsonData;
// Silently ignore the error (consider the file was not there)
} else {
// TODO: make it a real parameter
account->setProperty("oc_serverConfig", json);
}
_ocWizard->successfulStep();
});
});

connect(job, &PropfindJob::finishedWithError, this, &OwncloudSetupWizard::slotAuthError);
job->start();
}
Expand All @@ -362,7 +390,7 @@ void OwncloudSetupWizard::slotAuthError()
{
QString errorMsg;

PropfindJob *job = qobject_cast<PropfindJob *>(sender());
AbstractNetworkJob *job = qobject_cast<AbstractNetworkJob *>(sender());
if (!job) {
qCWarning(lcWizard) << "Can't check for authed redirects. This slot should be invoked from PropfindJob!";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also update this warning

return;
Expand Down Expand Up @@ -610,32 +638,62 @@ void OwncloudSetupWizard::slotAssistantFinished(int result)
FolderMan *folderMan = FolderMan::instance();
auto account = applyAccountChanges();

QJsonDocument configJson = account->account()->property("oc_serverConfig").toJsonDocument();
bool hasServerConfig = configJson.isObject() && !configJson.object()["default_folders"].toArray().isEmpty();
qWarning() << configJson << hasServerConfig;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qCWarning()


QString localFolder = FolderDefinition::prepareLocalPath(_ocWizard->localFolder());

bool startFromScratch = _ocWizard->field("OCSyncFromScratch").toBool();
if (!startFromScratch || ensureStartFromScratch(localFolder)) {
qCInfo(lcWizard) << "Adding folder definition for" << localFolder << _remoteFolder;
FolderDefinition folderDefinition;
folderDefinition.localPath = localFolder;
folderDefinition.targetPath = FolderDefinition::prepareTargetPath(_remoteFolder);
folderDefinition.ignoreHiddenFiles = folderMan->ignoreHiddenFiles();
if (_ocWizard->useVirtualFileSync()) {
folderDefinition.virtualFilesMode = bestAvailableVfsMode();
}
if (folderMan->navigationPaneHelper().showInExplorerNavigationPane())
folderDefinition.navigationPaneClsid = QUuid::createUuid();

auto f = folderMan->addFolder(account, folderDefinition);
if (f) {
if (folderDefinition.virtualFilesMode != Vfs::Off && _ocWizard->useVirtualFileSync())
f->setNewFilesAreVirtual(true);

f->journalDb()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList,
_ocWizard->selectiveSyncBlacklist());
if (!_ocWizard->isConfirmBigFolderChecked()) {
// The user already accepted the selective sync dialog. everything is in the white list
f->journalDb()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncWhiteList,
QStringList() << QLatin1String("/"));
if (!hasServerConfig) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is getting very deeply nested. Split the two branches into functions?

qCInfo(lcWizard) << "Adding folder definition for" << localFolder << _remoteFolder;
FolderDefinition folderDefinition;
folderDefinition.localPath = localFolder;
folderDefinition.targetPath = FolderDefinition::prepareTargetPath(_remoteFolder);
folderDefinition.ignoreHiddenFiles = folderMan->ignoreHiddenFiles();
if (_ocWizard->useVirtualFileSync()) {
folderDefinition.virtualFilesMode = bestAvailableVfsMode();
}
if (folderMan->navigationPaneHelper().showInExplorerNavigationPane())
folderDefinition.navigationPaneClsid = QUuid::createUuid();

auto f = folderMan->addFolder(account, folderDefinition);
if (f) {
if (folderDefinition.virtualFilesMode != Vfs::Off && _ocWizard->useVirtualFileSync())
f->setNewFilesAreVirtual(true);

f->journalDb()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList,
_ocWizard->selectiveSyncBlacklist());
if (!_ocWizard->isConfirmBigFolderChecked()) {
// The user already accepted the selective sync dialog. everything is in the white list
f->journalDb()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncWhiteList,
QStringList() << QLatin1String("/"));
}
}
} else {
auto array = configJson.object()["default_folders"].toArray();
// FIXME! check consistency of the config and that the folder can be created
for (const auto &x : array) {
QString remoteFolder = x.toObject()["remote_folder"].toString();
QString name = QFileInfo(remoteFolder).fileName();
FolderDefinition folderDefinition;
folderDefinition.localPath = (array.size() == 1) ? localFolder : QString(localFolder + name + '/');
QDir(localFolder).mkpath(folderDefinition.localPath);
folderDefinition.targetPath = FolderDefinition::prepareTargetPath(remoteFolder);
folderDefinition.ignoreHiddenFiles = folderMan->ignoreHiddenFiles();
if (folderMan->navigationPaneHelper().showInExplorerNavigationPane())
folderDefinition.navigationPaneClsid = QUuid::createUuid();

auto f = folderMan->addFolder(account, folderDefinition);
if (f) {
if (folderDefinition.virtualFilesMode != Vfs::Off && _ocWizard->useVirtualFileSync())
f->setNewFilesAreVirtual(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be removed I think. folderDefinition.virtualFilesMode is never set and useVirtualFileSync() an't be true.


QStringList list;
foreach (const auto &l, x.toObject()["black_list"].toArray()) { list << l.toString(); }
f->journalDb()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, list);
}
}
}
_ocWizard->appendToConfigurationLog(tr("<font color=\"green\"><b>Local sync folder %1 successfully created!</b></font>").arg(localFolder));
Expand Down
42 changes: 24 additions & 18 deletions src/gui/wizard/owncloudadvancedsetuppage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
#include <QUrl>
#include <QTimer>
#include <QMessageBox>
#include <QJsonDocument>
#include <QJsonObject>
#include <QJsonArray>

#include "QProgressIndicator.h"

Expand Down Expand Up @@ -69,15 +72,6 @@ OwncloudAdvancedSetupPage::OwncloudAdvancedSetupPage()
_ui.lServerIcon->setPixmap(appIcon.pixmap(48));
_ui.lLocalIcon->setText(QString());
_ui.lLocalIcon->setPixmap(QPixmap(Theme::hidpiFileName(":/client/resources/folder-sync.png")));

if (theme->wizardHideExternalStorageConfirmationCheckbox()) {
_ui.confCheckBoxExternal->hide();
}
if (theme->wizardHideFolderSizeLimitCheckbox()) {
_ui.confCheckBoxSize->hide();
_ui.confSpinBox->hide();
_ui.confTraillingSizeLabel->hide();
}
}

void OwncloudAdvancedSetupPage::setupCustomization()
Expand Down Expand Up @@ -110,14 +104,6 @@ void OwncloudAdvancedSetupPage::initializePage()
labelSizeHint.width(),
qMax<int>(1.3 * labelSizeHint.height(), _progressIndi->height()));

if (!Theme::instance()->showVirtualFilesOption() || bestAvailableVfsMode() == Vfs::Off) {
// If the layout were wrapped in a widget, the auto-grouping of the
// radio buttons no longer works and there are surprising margins.
// Just manually hide the button and remove the layout.
_ui.rVirtualFileSync->hide();
_ui.wSyncStrategy->layout()->removeItem(_ui.lVirtualFileSync);
}

_checking = false;
_ui.lSelectiveSyncSizeLabel->setText(QString());
_ui.lSyncEverythingSizeLabel->setText(QString());
Expand All @@ -139,11 +125,31 @@ void OwncloudAdvancedSetupPage::initializePage()
connect(quotaJob, &PropfindJob::result, this, &OwncloudAdvancedSetupPage::slotQuotaRetrieved);
quotaJob->start();

QJsonDocument configJson = acc->property("oc_serverConfig").toJsonDocument();
bool hasServerConfig = configJson.isObject() && !configJson.object()["default_folders"].toArray().isEmpty();
Theme *theme = Theme::instance();

if (Theme::instance()->wizardSelectiveSyncDefaultNothing()) {
_ui.rDefaultFolders->setVisible(hasServerConfig);
_ui.rSyncEverything->setVisible(!hasServerConfig);
_ui.lSyncEverythingSizeLabel->setVisible(!hasServerConfig);
_ui.rSelectiveSync->setVisible(!hasServerConfig);
_ui.bSelectiveSync->setVisible(!hasServerConfig);
_ui.lSelectiveSyncSizeLabel->setVisible(!hasServerConfig);
_ui.rVirtualFileSync->setVisible(!hasServerConfig && Theme::instance()->showVirtualFilesOption() && bestAvailableVfsMode() != Vfs::Off);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I remember removing the layout from the wSyncStrategy widget was required for making the layout not be broken on some platforms. May need testing.


_ui.confCheckBoxExternal->setVisible(!hasServerConfig && !theme->wizardHideExternalStorageConfirmationCheckbox());
_ui.confCheckBoxSize->setVisible(!hasServerConfig && !theme->wizardHideFolderSizeLimitCheckbox());
_ui.confSpinBox->setVisible(!hasServerConfig && !theme->wizardHideFolderSizeLimitCheckbox());
_ui.confTraillingSizeLabel->setVisible(!hasServerConfig && !theme->wizardHideFolderSizeLimitCheckbox());;

if (hasServerConfig) {
setRadioChecked(_ui.rDefaultFolders);
} else if (theme->wizardSelectiveSyncDefaultNothing()) {
_selectiveSyncBlacklist = QStringList("/");
setRadioChecked(_ui.rSelectiveSync);
QTimer::singleShot(0, this, &OwncloudAdvancedSetupPage::slotSelectiveSyncClicked);
} else {
setRadioChecked(_ui.rSyncEverything);
}

ConfigFile cfgFile;
Expand Down
36 changes: 30 additions & 6 deletions src/gui/wizard/owncloudadvancedsetuppage.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
<rect>
<x>0</x>
<y>0</y>
<width>917</width>
<height>604</height>
<width>916</width>
<height>719</height>
</rect>
</property>
<property name="sizePolicy">
Expand Down Expand Up @@ -235,6 +235,30 @@
<property name="rightMargin">
<number>0</number>
</property>
<item>
<layout class="QHBoxLayout" name="horizontalLayout_4">
<item>
<widget class="QRadioButton" name="rDefaultFolders">
<property name="text">
<string>A&amp;pply default folders configuration (recommended)</string>
</property>
</widget>
</item>
<item>
<spacer name="horizontalSpacer_7">
<property name="orientation">
<enum>Qt::Horizontal</enum>
</property>
<property name="sizeHint" stdset="0">
<size>
<width>40</width>
<height>20</height>
</size>
</property>
</spacer>
</item>
</layout>
</item>
<item>
<layout class="QHBoxLayout" name="horizontalLayout_2">
<item>
Expand Down Expand Up @@ -397,7 +421,7 @@
<string>&lt;html&gt;&lt;head/&gt;&lt;body&gt;&lt;p&gt;When this option is selected, the wizard will close without synchronizing anything. You can use the &amp;quot;Add Folder Sync Connection&amp;quot; button from the account settings to choose which pair of local and remote folder you wish to synchronize&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</string>
</property>
<property name="text">
<string>Manually create folder sync connections </string>
<string>Ma&amp;nually create folder sync connections </string>
</property>
</widget>
</item>
Expand Down Expand Up @@ -427,7 +451,7 @@
</sizepolicy>
</property>
<property name="text">
<string>Use virtual files instead of downloading content immediately (e&amp;xperimental)</string>
<string>&amp;Use virtual files instead of downloading content immediately (experimental)</string>
</property>
<property name="checkable">
<bool>false</bool>
Expand Down Expand Up @@ -486,8 +510,8 @@
<rect>
<x>0</x>
<y>0</y>
<width>899</width>
<height>68</height>
<width>904</width>
<height>80</height>
</rect>
</property>
<property name="sizePolicy">
Expand Down