From 4b2619530b72a0db802bd91619b723c72c867d52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 29 Jul 2024 12:08:55 +0200 Subject: [PATCH 01/11] refactor: Store result in its own variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- apps/files_external/js/settings.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index 52dd3b6bf9099..3db2e67763a1e 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -977,9 +977,10 @@ MountConfigListView.prototype = _.extend({ data: {'testOnly' : true}, contentType: 'application/json', success: function(result) { + result = Object.values(result); var onCompletion = jQuery.Deferred(); var $rows = $(); - Object.values(result).forEach(function(storageParams) { + result.forEach(function(storageParams) { var storageConfig; var isUserGlobal = storageParams.type === 'system' && self._isPersonal; storageParams.mountPoint = storageParams.mountPoint.substr(1); // trim leading slash From 05bf5730ad7df2ee285709cb47998cb9bfed376e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 29 Jul 2024 12:09:38 +0200 Subject: [PATCH 02/11] fix: Recheck userglobal storages when loaded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Userglobal storages are now automatically recheck when loaded, similarly to how it is done for global storages. Signed-off-by: Daniel Calviño Sánchez --- apps/files_external/js/settings.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index 3db2e67763a1e..fd1946b629c88 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -1009,6 +1009,13 @@ MountConfigListView.prototype = _.extend({ // userglobal storages do not expose configuration data $tr.find('.configuration').text(t('files_external', 'Admin defined')); } + + // don't recheck config automatically when there are a large number of storages + if (result.length < 20) { + self.recheckStorageConfig($tr); + } else { + self.updateStatus($tr, StorageConfig.Status.INDETERMINATE, t('files_external', 'Automatic status checking is disabled due to the large number of configured storages, click to check status')); + } $rows = $rows.add($tr); }); initApplicableUsersMultiselect(self.$el.find('.applicableUsers'), this._userListLimit); From d0ba10d6ae3d0187ac70dae6e2c24b7e9bca519e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 29 Jul 2024 12:13:21 +0200 Subject: [PATCH 03/11] fix: Remove status check when configuration was changed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Setting a null status was supposed to remove the status check, but nothing was changed in that case. Now the status check is properly removed, and doing that by hiding the element rather than just turning it invisible also prevents that clicking on the invisible status triggers a check, as until the new configuration is saved the check will still be performed with the old configuration, which could be misleading for the user. Additionally, an explicit width is set to the parent of the span element to prevent its width from changing when the span is shown and hidden. Signed-off-by: Daniel Calviño Sánchez --- apps/files_external/css/settings.css | 2 +- apps/files_external/css/settings.css.map | 2 +- apps/files_external/css/settings.scss | 2 ++ apps/files_external/js/settings.js | 4 ++++ 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/apps/files_external/css/settings.css b/apps/files_external/css/settings.css index b0b6e53a17117..0fba3680b790b 100644 --- a/apps/files_external/css/settings.css +++ b/apps/files_external/css/settings.css @@ -1,4 +1,4 @@ /*! * SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later - */#files_external{margin-bottom:0px}#externalStorage{margin:15px 0 20px 0}#externalStorage tr.externalStorageLoading>td{text-align:center}#externalStorage td>input:not(.applicableToAllUsers),#externalStorage td>select{width:100%}#externalStorage .popovermenu li>.menuitem{width:fit-content !important}#externalStorage td.status{display:table-cell;vertical-align:middle}#externalStorage td.status>span{display:inline-block;height:28px;width:28px;vertical-align:text-bottom;border-radius:50%;cursor:pointer}#externalStorage td.mountPoint,#externalStorage td.backend,#externalStorage td.authentication,#externalStorage td.configuration{min-width:160px;width:15%}#externalStorage td>img{padding-top:7px;opacity:.5}#externalStorage td>img:hover{padding-top:7px;cursor:pointer;opacity:1}#addMountPoint>td{border:none}#addMountPoint>td.applicable{visibility:hidden}#addMountPoint>td.hidden{visibility:hidden}#externalStorage td{height:50px}#externalStorage td.mountOptionsToggle,#externalStorage td.remove,#externalStorage td.save{position:relative;padding:0 !important;width:44px}#externalStorage td.mountOptionsToggle [class^=icon-],#externalStorage td.mountOptionsToggle [class*=" icon-"],#externalStorage td.remove [class^=icon-],#externalStorage td.remove [class*=" icon-"],#externalStorage td.save [class^=icon-],#externalStorage td.save [class*=" icon-"]{width:44px;height:44px;margin:3px;opacity:.5;padding:14px;vertical-align:text-bottom;cursor:pointer}#externalStorage td.mountOptionsToggle [class^=icon-]:hover,#externalStorage td.mountOptionsToggle [class*=" icon-"]:hover,#externalStorage td.remove [class^=icon-]:hover,#externalStorage td.remove [class*=" icon-"]:hover,#externalStorage td.save [class^=icon-]:hover,#externalStorage td.save [class*=" icon-"]:hover{opacity:1}#selectBackend{margin-inline-start:-10px;width:150px}#externalStorage td.configuration,#externalStorage td.backend{white-space:normal}#externalStorage td.configuration>*{white-space:nowrap}#externalStorage td.configuration input.added{margin-inline-end:6px}#externalStorage label>input[type=checkbox]{margin-inline-end:3px}#externalStorage td.configuration label{width:100%;display:inline-flex;align-items:center}#externalStorage td.configuration input.disabled-success{background-color:rgba(134,255,110,.9)}#externalStorage td.applicable label{display:inline-flex;align-items:center}#externalStorage td.applicable div.chzn-container{position:relative;top:3px}#externalStorage .select2-container.applicableUsers{width:100% !important}#userMountingBackends{padding-inline-start:25px}.files-external-select2 .select2-results .select2-result-label{height:32px;padding:3px}.files-external-select2 .select2-results .select2-result-label>span{display:block;position:relative}.files-external-select2 .select2-results .select2-result-label .avatardiv{display:inline-block}.files-external-select2 .select2-results .select2-result-label .avatardiv+span{position:absolute;top:5px;margin-inline-start:10px}.files-external-select2 .select2-results .select2-result-label .avatardiv[data-type=group]+span{vertical-align:top;top:6px;position:absolute;max-width:80%;inset-inline-start:30px;text-overflow:ellipsis;white-space:nowrap;overflow:hidden}#externalStorage .select2-container .select2-search-choice{display:flex}#externalStorage .select2-container .select2-search-choice .select2-search-choice-close{display:block;inset-inline-start:auto;position:relative;width:20px}#externalStorage .mountOptionsToggle .dropdown{width:auto}.nav-icon-external-storage{background-image:var(--icon-external-dark)}.global_credentials__personal{margin:10px auto;text-align:center;width:min(400px,100vw)}/*# sourceMappingURL=settings.css.map */ + */#files_external{margin-bottom:0px}#externalStorage{margin:15px 0 20px 0}#externalStorage tr.externalStorageLoading>td{text-align:center}#externalStorage td>input:not(.applicableToAllUsers),#externalStorage td>select{width:100%}#externalStorage .popovermenu li>.menuitem{width:fit-content !important}#externalStorage td.status{display:table-cell;vertical-align:middle;width:43px}#externalStorage td.status>span{display:inline-block;height:28px;width:28px;vertical-align:text-bottom;border-radius:50%;cursor:pointer}#externalStorage td.mountPoint,#externalStorage td.backend,#externalStorage td.authentication,#externalStorage td.configuration{min-width:160px;width:15%}#externalStorage td>img{padding-top:7px;opacity:.5}#externalStorage td>img:hover{padding-top:7px;cursor:pointer;opacity:1}#addMountPoint>td{border:none}#addMountPoint>td.applicable{visibility:hidden}#addMountPoint>td.hidden{visibility:hidden}#externalStorage td{height:50px}#externalStorage td.mountOptionsToggle,#externalStorage td.remove,#externalStorage td.save{position:relative;padding:0 !important;width:44px}#externalStorage td.mountOptionsToggle [class^=icon-],#externalStorage td.mountOptionsToggle [class*=" icon-"],#externalStorage td.remove [class^=icon-],#externalStorage td.remove [class*=" icon-"],#externalStorage td.save [class^=icon-],#externalStorage td.save [class*=" icon-"]{width:44px;height:44px;margin:3px;opacity:.5;padding:14px;vertical-align:text-bottom;cursor:pointer}#externalStorage td.mountOptionsToggle [class^=icon-]:hover,#externalStorage td.mountOptionsToggle [class*=" icon-"]:hover,#externalStorage td.remove [class^=icon-]:hover,#externalStorage td.remove [class*=" icon-"]:hover,#externalStorage td.save [class^=icon-]:hover,#externalStorage td.save [class*=" icon-"]:hover{opacity:1}#selectBackend{margin-inline-start:-10px;width:150px}#externalStorage td.configuration,#externalStorage td.backend{white-space:normal}#externalStorage td.configuration>*{white-space:nowrap}#externalStorage td.configuration input.added{margin-inline-end:6px}#externalStorage label>input[type=checkbox]{margin-inline-end:3px}#externalStorage td.configuration label{width:100%;display:inline-flex;align-items:center}#externalStorage td.configuration input.disabled-success{background-color:rgba(134,255,110,.9)}#externalStorage td.applicable label{display:inline-flex;align-items:center}#externalStorage td.applicable div.chzn-container{position:relative;top:3px}#externalStorage .select2-container.applicableUsers{width:100% !important}#userMountingBackends{padding-inline-start:25px}.files-external-select2 .select2-results .select2-result-label{height:32px;padding:3px}.files-external-select2 .select2-results .select2-result-label>span{display:block;position:relative}.files-external-select2 .select2-results .select2-result-label .avatardiv{display:inline-block}.files-external-select2 .select2-results .select2-result-label .avatardiv+span{position:absolute;top:5px;margin-inline-start:10px}.files-external-select2 .select2-results .select2-result-label .avatardiv[data-type=group]+span{vertical-align:top;top:6px;position:absolute;max-width:80%;inset-inline-start:30px;text-overflow:ellipsis;white-space:nowrap;overflow:hidden}#externalStorage .select2-container .select2-search-choice{display:flex}#externalStorage .select2-container .select2-search-choice .select2-search-choice-close{display:block;inset-inline-start:auto;position:relative;width:20px}#externalStorage .mountOptionsToggle .dropdown{width:auto}.nav-icon-external-storage{background-image:var(--icon-external-dark)}.global_credentials__personal{margin:10px auto;text-align:center;width:min(400px,100vw)}/*# sourceMappingURL=settings.css.map */ diff --git a/apps/files_external/css/settings.css.map b/apps/files_external/css/settings.css.map index 9c976499eb42b..bb9d95168450f 100644 --- a/apps/files_external/css/settings.css.map +++ b/apps/files_external/css/settings.css.map @@ -1 +1 @@ -{"version":3,"sourceRoot":"","sources":["settings.scss"],"names":[],"mappings":"AAAA;AAAA;AAAA;AAAA,GAIA,gBACC,kBAGD,iBACC,qBAEA,8CACC,kBAKD,gFACC,WAIF,2CACI,6BAGJ,2BAEC,mBACA,sBAGD,gCACC,qBACA,YACA,WACA,2BACA,kBACA,eAGA,gIACC,gBACA,UAGF,mDACA,uEACA,8BACA,+CACA,2CAEA,oBACC,YACA,2FAGC,kBACA,qBACA,WACA,yRAEC,WACA,YACA,WACA,WACA,aACA,2BACA,eACA,6TACC,UAMJ,eACC,0BACA,YAGD,8DAEC,mBAED,oCACC,mBAGD,8CACC,sBAGD,4CACC,sBAGD,wCACC,WACA,oBACA,mBAGD,yDACC,sCAGD,qCACC,oBACA,mBAGD,kDACC,kBACA,QAGD,oDACC,sBAGD,sBACC,0BAGD,+DACC,YACA,YAED,oEACC,cACA,kBAED,0EACC,qBAED,+EACC,kBACA,QACA,yBAED,gGACC,mBACA,QACA,kBACA,cACA,wBACA,uBACA,mBACA,gBAGD,2DACC,aACA,wFACC,cACA,wBACA,kBACA,WAIF,+CACC,WAGD,2BACC,2CAGD,8BACI,iBACA,kBACA","file":"settings.css"} \ No newline at end of file +{"version":3,"sourceRoot":"","sources":["settings.scss"],"names":[],"mappings":"AAAA;AAAA;AAAA;AAAA,GAIA,gBACC,kBAGD,iBACC,qBAEA,8CACC,kBAKD,gFACC,WAIF,2CACI,6BAGJ,2BAEC,mBACA,sBAEA,WAGD,gCACC,qBACA,YACA,WACA,2BACA,kBACA,eAGA,gIACC,gBACA,UAGF,mDACA,uEACA,8BACA,+CACA,2CAEA,oBACC,YACA,2FAGC,kBACA,qBACA,WACA,yRAEC,WACA,YACA,WACA,WACA,aACA,2BACA,eACA,6TACC,UAMJ,eACC,0BACA,YAGD,8DAEC,mBAED,oCACC,mBAGD,8CACC,sBAGD,4CACC,sBAGD,wCACC,WACA,oBACA,mBAGD,yDACC,sCAGD,qCACC,oBACA,mBAGD,kDACC,kBACA,QAGD,oDACC,sBAGD,sBACC,0BAGD,+DACC,YACA,YAED,oEACC,cACA,kBAED,0EACC,qBAED,+EACC,kBACA,QACA,yBAED,gGACC,mBACA,QACA,kBACA,cACA,wBACA,uBACA,mBACA,gBAGD,2DACC,aACA,wFACC,cACA,wBACA,kBACA,WAIF,+CACC,WAGD,2BACC,2CAGD,8BACI,iBACA,kBACA","file":"settings.css"} \ No newline at end of file diff --git a/apps/files_external/css/settings.scss b/apps/files_external/css/settings.scss index 6d21317294204..8ffdc769dfb1d 100644 --- a/apps/files_external/css/settings.scss +++ b/apps/files_external/css/settings.scss @@ -28,6 +28,8 @@ /* overwrite conflicting core styles */ display: table-cell; vertical-align: middle; + /* ensure width does not change even if internal span is not displayed */ + width: 43px; } #externalStorage td.status > span { diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index fd1946b629c88..b4e1cc0449806 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -1316,6 +1316,7 @@ MountConfigListView.prototype = _.extend({ switch (status) { case null: // remove status + $statusSpan.hide(); break; case StorageConfig.Status.IN_PROGRESS: $statusSpan.attr('class', 'icon-loading-small'); @@ -1329,6 +1330,9 @@ MountConfigListView.prototype = _.extend({ default: $statusSpan.attr('class', 'error icon-error-white'); } + if (status !== null) { + $statusSpan.show(); + } if (typeof message === 'string') { $statusSpan.attr('title', message); } From 9eeb3cf423ed2a2e4d06d5774905b8c57f1e1f5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 29 Jul 2024 12:15:18 +0200 Subject: [PATCH 04/11] fix: Set status tooltip to status message when saving an storage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a storage is saved the status check can fail even if saving the storage succeeds. In those cases further details are provided in the status message of the storage, which is now set as the tooltip, similarly to how it is done when rechecking the storage. Signed-off-by: Daniel Calviño Sánchez --- apps/files_external/js/settings.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index b4e1cc0449806..7b571e834e5c1 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -1262,7 +1262,7 @@ MountConfigListView.prototype = _.extend({ if (concurrentTimer === undefined || $tr.data('save-timer') === concurrentTimer ) { - self.updateStatus($tr, result.status); + self.updateStatus($tr, result.status, result.statusMessage); $tr.data('id', result.id); if (_.isFunction(callback)) { From ac1c8c423725712f712593d1cc97813996c92e78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 29 Jul 2024 12:15:50 +0200 Subject: [PATCH 05/11] fix: Set status tooltip to error message on failed actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When saving, updating and rechecking an storage fails (which is different to the soft-fail when the action itself succeeds but the status check does not) further details are provided in the error message of the response, which is now set as the tooltip. Signed-off-by: Daniel Calviño Sánchez --- apps/files_external/js/settings.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index 7b571e834e5c1..475387cbfa27e 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -1234,8 +1234,9 @@ MountConfigListView.prototype = _.extend({ success: function () { $tr.remove(); }, - error: function () { - self.updateStatus($tr, StorageConfig.Status.ERROR); + error: function (result) { + const statusMessage = (result && result.responseJSON) ? result.responseJSON.message : undefined; + self.updateStatus($tr, StorageConfig.Status.ERROR, statusMessage); } }); } @@ -1270,11 +1271,12 @@ MountConfigListView.prototype = _.extend({ } } }, - error: function() { + error: function(result) { if (concurrentTimer === undefined || $tr.data('save-timer') === concurrentTimer ) { - self.updateStatus($tr, StorageConfig.Status.ERROR); + const statusMessage = (result && result.responseJSON) ? result.responseJSON.message : undefined; + self.updateStatus($tr, StorageConfig.Status.ERROR, statusMessage); } } }); @@ -1298,8 +1300,9 @@ MountConfigListView.prototype = _.extend({ success: function(result) { self.updateStatus($tr, result.status, result.statusMessage); }, - error: function() { - self.updateStatus($tr, StorageConfig.Status.ERROR); + error: function(result) { + const statusMessage = (result && result.responseJSON) ? result.responseJSON.message : undefined; + self.updateStatus($tr, StorageConfig.Status.ERROR, statusMessage); } }); }, From 26919835e7beb9d19542b5cb05327de9c937f541 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 29 Jul 2024 12:18:12 +0200 Subject: [PATCH 06/11] fix: Restore default status tooltip when no status message is provided MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the status is updated but no explicit message is provided (for example, if the status check succeeded) the default tooltip (from the template) is now set to prevent a mismatch between the status and the tooltip (for example, if the configuration is fixed after a failed status check). Signed-off-by: Daniel Calviño Sánchez --- apps/files_external/js/settings.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index 475387cbfa27e..7df4544392d3d 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -1336,9 +1336,10 @@ MountConfigListView.prototype = _.extend({ if (status !== null) { $statusSpan.show(); } - if (typeof message === 'string') { - $statusSpan.attr('title', message); + if (typeof message !== 'string') { + message = t('files_external', 'Click to recheck the configuration'); } + $statusSpan.attr('title', message); }, /** From baaed7f191cfe779c7ef231c21cc87f9041e36ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 29 Jul 2024 12:20:18 +0200 Subject: [PATCH 07/11] fix: Add missing translation for UI string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- apps/files_external/js/settings.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index 7df4544392d3d..c003778db6f6d 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -874,7 +874,7 @@ MountConfigListView.prototype = _.extend({ $tr.find('.applicable,.mountOptionsToggle').empty(); $tr.find('.save').empty(); if (backend.invalid) { - this.updateStatus($tr, false, 'Unknown backend: ' + backend.name); + this.updateStatus($tr, false, t('files_external', 'Unknown backend: {backendName}', {backendName: backend.name})); } return $tr; } From 8e5dba2be0ad4726da4a3a2a89a21462bd3e84b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 29 Jul 2024 12:21:13 +0200 Subject: [PATCH 08/11] fix: Reset selected backend when adding a new storage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As a new storage is added by selecting a backend the selected backend needs to be reset. Otherwise it is not possible to add another storage with the same backend. Signed-off-by: Daniel Calviño Sánchez --- apps/files_external/js/settings.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index c003778db6f6d..582276cad0940 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -766,6 +766,8 @@ MountConfigListView.prototype = _.extend({ storageConfig.backend = $target.val(); $tr.find('.mountPoint input').val(''); + $tr.find('.selectBackend').prop('selectedIndex', 0) + var onCompletion = jQuery.Deferred(); $tr = this.newStorage(storageConfig, onCompletion); $tr.find('.applicableToAllUsers').prop('checked', false).trigger('change'); From 8350aef72310160f8ab3b0798befb89acaad1874 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 30 Jul 2024 03:04:40 +0200 Subject: [PATCH 09/11] test: Add integration tests for saving external userglobal storages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As the external storage uses the Nextcloud server itself the number of workers of the PHP process running the Nextcloud server had to be increased. Otherwise if a request is sent for the external storage while handling a request from the integration tests a deadlock would occur. Signed-off-by: Daniel Calviño Sánchez --- .../features/bootstrap/BasicStructure.php | 2 +- .../features/bootstrap/ExternalStorage.php | 103 ++++++++++++++++++ .../features/bootstrap/FeatureContext.php | 1 + .../files_features/external-storage.feature | 30 +++++ build/integration/run.sh | 2 +- 5 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 build/integration/features/bootstrap/ExternalStorage.php diff --git a/build/integration/features/bootstrap/BasicStructure.php b/build/integration/features/bootstrap/BasicStructure.php index e4aad79858c04..8500eca6b0fa9 100644 --- a/build/integration/features/bootstrap/BasicStructure.php +++ b/build/integration/features/bootstrap/BasicStructure.php @@ -327,7 +327,7 @@ public function sendingAToWithRequesttoken($method, $url, $body = null) { $fd = $body->getRowsHash(); $options['form_params'] = $fd; } elseif ($body) { - $options = array_merge($options, $body); + $options = array_merge_recursive($options, $body); } $client = new Client(); diff --git a/build/integration/features/bootstrap/ExternalStorage.php b/build/integration/features/bootstrap/ExternalStorage.php new file mode 100644 index 0000000000000..2a73f22c1b44c --- /dev/null +++ b/build/integration/features/bootstrap/ExternalStorage.php @@ -0,0 +1,103 @@ +storageIds as $storageId) { + $this->deleteStorage($storageId); + } + $this->storageIds = []; + } + + private function deleteStorage(string $storageId): void { + // Based on "runOcc" from CommandLine trait + $args = ['files_external:delete', '--yes', $storageId]; + $args = array_map(function ($arg) { + return escapeshellarg($arg); + }, $args); + $args[] = '--no-ansi --no-warnings'; + $args = implode(' ', $args); + + $descriptor = [ + 0 => ['pipe', 'r'], + 1 => ['pipe', 'w'], + 2 => ['pipe', 'w'], + ]; + $process = proc_open('php console.php ' . $args, $descriptor, $pipes, $ocPath = '../..'); + $lastStdOut = stream_get_contents($pipes[1]); + proc_close($process); + } + + /** + * @When logged in user creates external global storage + * + * @param TableNode $fields + */ + public function loggedInUserCreatesExternalGlobalStorage(TableNode $fields): void { + $this->sendJsonWithRequestToken('POST', '/index.php/apps/files_external/globalstorages', $fields); + $this->theHTTPStatusCodeShouldBe('201'); + + $this->lastExternalStorageData = json_decode($this->response->getBody(), $asAssociativeArray = true); + + $this->storageIds[] = $this->lastExternalStorageData['id']; + } + + /** + * @When logged in user updates last external userglobal storage + * + * @param TableNode $fields + */ + public function loggedInUserUpdatesLastExternalUserglobalStorage(TableNode $fields): void { + $this->sendJsonWithRequestToken('PUT', '/index.php/apps/files_external/userglobalstorages/' . $this->lastExternalStorageData['id'], $fields); + $this->theHTTPStatusCodeShouldBe('200'); + + $this->lastExternalStorageData = json_decode($this->response->getBody(), $asAssociativeArray = true); + } + + /** + * @Then fields of last external storage match with + * + * @param TableNode $fields + */ + public function fieldsOfLastExternalStorageMatchWith(TableNode $fields): void { + foreach ($fields->getRowsHash() as $expectedField => $expectedValue) { + if (!array_key_exists($expectedField, $this->lastExternalStorageData)) { + Assert::fail("$expectedField was not found in response"); + } + + Assert::assertEquals($expectedValue, $this->lastExternalStorageData[$expectedField], "Field '$expectedField' does not match ({$this->lastExternalStorageData[$expectedField]})"); + } + } + + private function sendJsonWithRequestToken(string $method, string $url, TableNode $fields): void { + $isFirstField = true; + $fieldsAsJsonString = '{'; + foreach ($fields->getRowsHash() as $key => $value) { + $fieldsAsJsonString .= ($isFirstField ? '' : ',') . '"' . $key . '":' . $value; + $isFirstField = false; + } + $fieldsAsJsonString .= '}'; + + $body = [ + 'headers' => [ + 'Content-Type' => 'application/json', + ], + 'body' => $fieldsAsJsonString, + ]; + $this->sendingAToWithRequesttoken($method, $url, $body); + } +} diff --git a/build/integration/features/bootstrap/FeatureContext.php b/build/integration/features/bootstrap/FeatureContext.php index 638772da0b7e6..893dc3094bacc 100644 --- a/build/integration/features/bootstrap/FeatureContext.php +++ b/build/integration/features/bootstrap/FeatureContext.php @@ -15,6 +15,7 @@ */ class FeatureContext implements Context, SnippetAcceptingContext { use ContactsMenu; + use ExternalStorage; use Search; use WebDav; use Trashbin; diff --git a/build/integration/files_features/external-storage.feature b/build/integration/files_features/external-storage.feature index 111b5686e0e6e..134b8c54c73e9 100644 --- a/build/integration/files_features/external-storage.feature +++ b/build/integration/files_features/external-storage.feature @@ -63,3 +63,33 @@ Feature: external-storage Then as "user1" the file "/local_storage/foo2/textfile0.txt" does not exist And as "user0" the file "/local_storage/foo2/textfile0.txt" does not exist And as "user1" the file "/local.txt" exists + + + + Scenario: Save an external storage with password provided by user + Given Logging in using web as "admin" + And logged in user creates external global storage + | mountPoint | "ExternalStorageTest" | + | backend | "owncloud" | + | authMechanism | "password::userprovided" | + | backendOptions | {"host":"http://localhost:8080","secure":false} | + And fields of last external storage match with + | status | 2 | + When logged in user updates last external userglobal storage + | backendOptions | {"user":"admin","password":"admin"} | + Then fields of last external storage match with + | status | 0 | + + Scenario: Save an external storage with global credentials provided by user + Given Logging in using web as "admin" + And logged in user creates external global storage + | mountPoint | "ExternalStorageTest" | + | backend | "owncloud" | + | authMechanism | "password::global::user" | + | backendOptions | {"host":"http://localhost:8080","secure":false} | + And fields of last external storage match with + | status | 2 | + When logged in user updates last external userglobal storage + | backendOptions | {"user":"admin","password":"admin"} | + Then fields of last external storage match with + | status | 0 | diff --git a/build/integration/run.sh b/build/integration/run.sh index cef59c976af21..8f5320af8f5f7 100755 --- a/build/integration/run.sh +++ b/build/integration/run.sh @@ -41,7 +41,7 @@ echo $PORT echo "" > phpserver.log -php -S localhost:$PORT -t ../.. &> phpserver.log & +PHP_CLI_SERVER_WORKERS=2 php -S localhost:$PORT -t ../.. &> phpserver.log & PHPPID=$! echo $PHPPID From bd4de5208d6e8fb052f9b36f0a3b2f2d66c7c8cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 30 Jul 2024 03:05:27 +0200 Subject: [PATCH 10/11] fix: Fix unmodified placeholder replacing the actual value when updating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When updating global storages and user storages a property is not updated by "StoragesService::updateStorage()" if the value matches the unmodified placeholder. However, userglobal storages are not updated through the "StoragesService"; as only the authentication mechanism is updated it is directly done with "saveBackendOptions()" in "IUserProvided" or "UserGlobalAuth". Due to this the unmodified placeholder value needs to be explicitly checked in those cases and replaced by the actual value (note that in this case it is not possible to just skip updating a specific property). Signed-off-by: Daniel Calviño Sánchez --- .../lib/Lib/Auth/Password/UserGlobalAuth.php | 7 ++++ .../lib/Lib/Auth/Password/UserProvided.php | 5 +++ .../files_features/external-storage.feature | 32 +++++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/apps/files_external/lib/Lib/Auth/Password/UserGlobalAuth.php b/apps/files_external/lib/Lib/Auth/Password/UserGlobalAuth.php index 4c277405b18c0..84f2c698f073a 100644 --- a/apps/files_external/lib/Lib/Auth/Password/UserGlobalAuth.php +++ b/apps/files_external/lib/Lib/Auth/Password/UserGlobalAuth.php @@ -9,6 +9,7 @@ namespace OCA\Files_External\Lib\Auth\Password; use OCA\Files_External\Lib\Auth\AuthMechanism; +use OCA\Files_External\Lib\DefinitionParameter; use OCA\Files_External\Lib\InsufficientDataForMeaningfulAnswerException; use OCA\Files_External\Lib\StorageConfig; use OCA\Files_External\Service\BackendService; @@ -41,6 +42,12 @@ public function saveBackendOptions(IUser $user, $id, $backendOptions) { if (!isset($backendOptions['user']) && !isset($backendOptions['password'])) { return; } + + if ($backendOptions['password'] === DefinitionParameter::UNMODIFIED_PLACEHOLDER) { + $oldCredentials = $this->credentialsManager->retrieve($user->getUID(), self::CREDENTIALS_IDENTIFIER); + $backendOptions['password'] = $oldCredentials['password']; + } + // make sure we're not setting any unexpected keys $credentials = [ 'user' => $backendOptions['user'], diff --git a/apps/files_external/lib/Lib/Auth/Password/UserProvided.php b/apps/files_external/lib/Lib/Auth/Password/UserProvided.php index fe9fd357b8935..a7c51d7353a83 100644 --- a/apps/files_external/lib/Lib/Auth/Password/UserProvided.php +++ b/apps/files_external/lib/Lib/Auth/Password/UserProvided.php @@ -47,6 +47,11 @@ private function getCredentialsIdentifier($storageId) { } public function saveBackendOptions(IUser $user, $mountId, array $options) { + if ($options['password'] === DefinitionParameter::UNMODIFIED_PLACEHOLDER) { + $oldCredentials = $this->credentialsManager->retrieve($user->getUID(), $this->getCredentialsIdentifier($mountId)); + $options['password'] = $oldCredentials['password']; + } + $this->credentialsManager->store($user->getUID(), $this->getCredentialsIdentifier($mountId), [ 'user' => $options['user'], // explicitly copy the fields we want instead of just passing the entire $options array 'password' => $options['password'] // this way we prevent users from being able to modify any other field diff --git a/build/integration/files_features/external-storage.feature b/build/integration/files_features/external-storage.feature index 134b8c54c73e9..d313cfb328765 100644 --- a/build/integration/files_features/external-storage.feature +++ b/build/integration/files_features/external-storage.feature @@ -80,6 +80,22 @@ Feature: external-storage Then fields of last external storage match with | status | 0 | + Scenario: Save an external storage again with an unmodified password provided by user + Given Logging in using web as "admin" + And logged in user creates external global storage + | mountPoint | "ExternalStorageTest" | + | backend | "owncloud" | + | authMechanism | "password::userprovided" | + | backendOptions | {"host":"http://localhost:8080","secure":false} | + And fields of last external storage match with + | status | 2 | + And logged in user updates last external userglobal storage + | backendOptions | {"user":"admin","password":"admin"} | + When logged in user updates last external userglobal storage + | backendOptions | {"user":"admin","password":"__unmodified__"} | + Then fields of last external storage match with + | status | 0 | + Scenario: Save an external storage with global credentials provided by user Given Logging in using web as "admin" And logged in user creates external global storage @@ -93,3 +109,19 @@ Feature: external-storage | backendOptions | {"user":"admin","password":"admin"} | Then fields of last external storage match with | status | 0 | + + Scenario: Save an external storage again with unmodified global credentials provided by user + Given Logging in using web as "admin" + And logged in user creates external global storage + | mountPoint | "ExternalStorageTest" | + | backend | "owncloud" | + | authMechanism | "password::global::user" | + | backendOptions | {"host":"http://localhost:8080","secure":false} | + And fields of last external storage match with + | status | 2 | + And logged in user updates last external userglobal storage + | backendOptions | {"user":"admin","password":"admin"} | + When logged in user updates last external userglobal storage + | backendOptions | {"user":"admin","password":"__unmodified__"} | + Then fields of last external storage match with + | status | 0 | From fa0862c656d4fbe381a418015a16b5dc2e85408c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 21 Aug 2024 21:26:23 +0200 Subject: [PATCH 11/11] fix: Hide status tooltip in row to add a new mount point MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The row to add a new mount point is cloned when a new mountpoint is added, so it is expected that it includes a status span. However, it should not be displayed in that row, only in the cloned row when its status is updated. Signed-off-by: Daniel Calviño Sánchez --- apps/files_external/templates/settings.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/files_external/templates/settings.php b/apps/files_external/templates/settings.php index 54407836d6398..c4d14ecb2b4ed 100644 --- a/apps/files_external/templates/settings.php +++ b/apps/files_external/templates/settings.php @@ -146,7 +146,7 @@ function writeParameterInput($parameter, $options, $classes = []) { > - +