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

ogoffart wants to merge 1 commit into master from serverconfig

Contributor

ogoffart commented Sep 3, 2018 •

edited

Loading

This commit will read a file called .desktopclientconfig at the root
of the webdav, while running the wizard, before showing the last page.

If the file exist and contains a list of default folder to configure,
hide all the option to configure the remote folders, as the default
configurtation will be applied.

The config file should contains a json object. The currently supported
config can be seen in this these example:

{ "default_folders": [
    {
        "remote_folder" : "Documents",
        "black_list" :  [ "Archives/1999", "Archives/1998" ]
    },
    {  "remote_folder": "Music"  }
]}

The default_folders property is a list of folder configuration. The
remote_folder property is the path of the folder on the remote. The
black_list property is a list of entries for the "choose what to sync".

ogoffart added DO NOT MERGE YET WIP labels

ogoffart requested a review from ckamm

September 3, 2018 09:24

Contributor Author

ogoffart commented Sep 3, 2018

@michaelstingl Before continuing, it would be nice to know the use case of what should be configured.

ChrisEdS commented Sep 4, 2018

Is it also possible to set a default local folder with this .desktopclientconfig file?

Contributor

michaelstingl commented Sep 4, 2018

@ChrisEdS We'll collect the requirements in a different issue…

Contributor Author

ogoffart commented Nov 27, 2018

@michaelstingl Can you please create an issue tracking what needs to be configured?


          Read default folder configuration from a file on the server

3ab722f

This commit will read a file called `.desktopclientconfig` at the root
of the webdav, while running the wizard, before showing the last page.

If the file exist and contains a list of default folder to configure,
hide all the option to configure the remote folders, as the default
configurtation will be applied.

The config file should contains a json object. The currently supported
config can be seen in this these example:

```
{ "default_folders": [
    {
        "remote_folder" : "Documents",
        "black_list" :  [ "Archives/1999", "Archives/1998" ]
    },
    {  "remote_folder": "Music"  }
]}
```

The `default_folders` property is a list of folder configuration. The
`remote_folder` property is the path of the folder on the remote. The
`black_list` property is a list of entries for the "choose what to sync".

ogoffart force-pushed the serverconfig branch from 26a97e0 to 3ab722f Compare

March 14, 2019 14:10

michaelstingl mentioned this pull request

[Selective Sync] Push default selective sync policy to client? #2517

Open

ckamm reviewed

View reviewed changes

Contributor

ckamm left a comment

Initial review of wip version

src/gui/owncloudsetupwizard.cpp

@@ @@ -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] {
+                      auto *job2 = account->sendRequest("GET", Utility::concatUrlPath(account->davUrl(), ".desktopclientconfig"));

Contributor

ckamm Jun 21, 2019

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

src/gui/owncloudsetupwizard.cpp

@@ @@ -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] {

Contributor

ckamm Jun 21, 2019

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.

src/gui/owncloudsetupwizard.cpp

    
            @@ -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!";

Contributor

ckamm Jun 21, 2019

Also update this warning

src/gui/owncloudsetupwizard.cpp

                       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;

Contributor

ckamm Jun 21, 2019

qCWarning()

src/gui/owncloudsetupwizard.cpp

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

Contributor

ckamm Jun 21, 2019

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

src/gui/owncloudsetupwizard.cpp

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

Contributor

ckamm Jun 21, 2019

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

src/gui/wizard/owncloudadvancedsetuppage.cpp

+                  _ui.rSelectiveSync->setVisible(!hasServerConfig);
+                  _ui.bSelectiveSync->setVisible(!hasServerConfig);
+                  _ui.lSelectiveSyncSizeLabel->setVisible(!hasServerConfig);
+                  _ui.rVirtualFileSync->setVisible(!hasServerConfig && Theme::instance()->showVirtualFilesOption() && bestAvailableVfsMode() != Vfs::Off);

Contributor

ckamm Jun 21, 2019

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.

michaelstingl mentioned this pull request

Updates to doc for configuration file #7378

Merged

butonic mentioned this pull request

Feedback from CERN, followup ocConf 2019 owncloud/ocis#43

Closed

guruz assigned michaelstingl and pmaier1

TheOneRing closed this

TheOneRing deleted the serverconfig branch

April 18, 2023 17:39

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DO NOT MERGE YET