From e43af1da715bc511d2513279c666a8a5c8475771 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 21 Aug 2024 21:33:06 +0200 Subject: [PATCH 01/12] fix: Remove broken jQuery tooltip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In Nextcloud 29 and later the tooltip usage threw an error that caused the UI to be unusable, so it was removed. In Nextcloud 28 there is no error, but using the jQuery tooltip call causes the tooltip to be unreadable, so it was removed as well (which makes the native tooltip to be shown instead). Signed-off-by: Daniel Calviño Sánchez --- apps/files_external/js/settings.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js index db77fe4dfc18b..30bf85a6764fb 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -1327,9 +1327,6 @@ MountConfigListView.prototype = _.extend({ } if (typeof message === 'string') { $statusSpan.attr('title', message); - $statusSpan.tooltip(); - } else { - $statusSpan.tooltip('dispose'); } }, From fc93517fc5b42da57f72353f96f95dde5314646f 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 02/12] 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 30bf85a6764fb..e1d47dbdc89e7 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -981,9 +981,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 dc18849d182258f64adaaebab49f8f2dc46c64ee 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 03/12] 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 e1d47dbdc89e7..d9f940cca057a 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -1013,6 +1013,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 9108d54bda7b9901985300ce8f3334234434b1be 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 04/12] 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 2d70ada6973d0..fd7f2ec9b6d4b 100644 --- a/apps/files_external/css/settings.css +++ b/apps/files_external/css/settings.css @@ -1 +1 @@ -#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-left:-10px;width:150px}#externalStorage td.configuration,#externalStorage td.backend{white-space:normal}#externalStorage td.configuration>*{white-space:nowrap}#externalStorage td.configuration input.added{margin-right:6px}#externalStorage label>input[type=checkbox]{margin-right: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-left: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-left:10px}.files-external-select2 .select2-results .select2-result-label .avatardiv[data-type=group]+span{vertical-align:top;top:6px;position:absolute;max-width:80%;left: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;left: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-left:-10px;width:150px}#externalStorage td.configuration,#externalStorage td.backend{white-space:normal}#externalStorage td.configuration>*{white-space:nowrap}#externalStorage td.configuration input.added{margin-right:6px}#externalStorage label>input[type=checkbox]{margin-right: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-left: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-left:10px}.files-external-select2 .select2-results .select2-result-label .avatardiv[data-type=group]+span{vertical-align:top;top:6px;position:absolute;max-width:80%;left: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;left: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 c980ab3b126b6..df011384a4f2f 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,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,kBACA,YAGD,8DAEC,mBAED,oCACC,mBAGD,8CACC,iBAGD,4CACC,iBAGD,wCACC,WACA,oBACA,mBAGD,yDACC,sCAGD,qCACC,oBACA,mBAGD,kDACC,kBACA,QAGD,oDACC,sBAGD,sBACC,kBAGD,+DACC,YACA,YAED,oEACC,cACA,kBAED,0EACC,qBAED,+EACC,kBACA,QACA,iBAED,gGACC,mBACA,QACA,kBACA,cACA,UACA,uBACA,mBACA,gBAGD,2DACC,aACA,wFACC,cACA,UACA,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,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,kBACA,YAGD,8DAEC,mBAED,oCACC,mBAGD,8CACC,iBAGD,4CACC,iBAGD,wCACC,WACA,oBACA,mBAGD,yDACC,sCAGD,qCACC,oBACA,mBAGD,kDACC,kBACA,QAGD,oDACC,sBAGD,sBACC,kBAGD,+DACC,YACA,YAED,oEACC,cACA,kBAED,0EACC,qBAED,+EACC,kBACA,QACA,iBAED,gGACC,mBACA,QACA,kBACA,cACA,UACA,uBACA,mBACA,gBAGD,2DACC,aACA,wFACC,cACA,UACA,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 5acf373da731b..ae9dcae080076 100644 --- a/apps/files_external/css/settings.scss +++ b/apps/files_external/css/settings.scss @@ -24,6 +24,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 d9f940cca057a..1c27642c6a377 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -1320,6 +1320,7 @@ MountConfigListView.prototype = _.extend({ switch (status) { case null: // remove status + $statusSpan.hide(); break; case StorageConfig.Status.IN_PROGRESS: $statusSpan.attr('class', 'icon-loading-small'); @@ -1333,6 +1334,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 35f16a3d8a95530b89cb43e4275257eb9b8e64fe 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 05/12] 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 1c27642c6a377..92b14e9454de8 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -1266,7 +1266,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 029626ba8fbcfd3b05f5db8f9cebb22eebcfcac2 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 06/12] 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 92b14e9454de8..2ece2b56740d7 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -1238,8 +1238,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); } }); } @@ -1274,11 +1275,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); } } }); @@ -1302,8 +1304,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 65885fea339f67340e99cf031ae2c531fcdcb609 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 07/12] 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 2ece2b56740d7..49e6cb42219ce 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -1340,9 +1340,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 fa28a1da649d5e10d3de0963a646435db463aa40 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 08/12] 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 49e6cb42219ce..390fa8fea8cca 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -878,7 +878,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 b72aebf7bee1b3254318de90bf7e2bd1c88dfe93 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 09/12] 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 390fa8fea8cca..1225115eff2a4 100644 --- a/apps/files_external/js/settings.js +++ b/apps/files_external/js/settings.js @@ -770,6 +770,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 59b62aa50fd897efe0c381add6fee1b135b7d8a9 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 10/12] 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 + .../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 e12a40ac6b451..77083c3c69431 100644 --- a/build/integration/features/bootstrap/BasicStructure.php +++ b/build/integration/features/bootstrap/BasicStructure.php @@ -354,7 +354,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 a3a600d66256c..27ad69857e267 100644 --- a/build/integration/features/bootstrap/FeatureContext.php +++ b/build/integration/features/bootstrap/FeatureContext.php @@ -34,6 +34,7 @@ */ class FeatureContext implements Context, SnippetAcceptingContext { use ContactsMenu; + use ExternalStorage; use Search; use WebDav; use Trashbin; diff --git a/build/integration/features/external-storage.feature b/build/integration/features/external-storage.feature index d92cca3c4580d..d62c7a4f5c712 100644 --- a/build/integration/features/external-storage.feature +++ b/build/integration/features/external-storage.feature @@ -60,3 +60,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 45a0333038ec6..0d318771a7b4a 100755 --- a/build/integration/run.sh +++ b/build/integration/run.sh @@ -34,7 +34,7 @@ if [ -z "$EXECUTOR_NUMBER" ]; then fi PORT=$((8080 + $EXECUTOR_NUMBER)) echo $PORT -php -S localhost:$PORT -t ../.. & +PHP_CLI_SERVER_WORKERS=2 php -S localhost:$PORT -t ../.. & PHPPID=$! echo $PHPPID From 06443611479b6e3a4924bb6f571583af37f4c714 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 11/12] 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 +++ .../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 6312a3d136e78..bf43ceebde178 100644 --- a/apps/files_external/lib/Lib/Auth/Password/UserGlobalAuth.php +++ b/apps/files_external/lib/Lib/Auth/Password/UserGlobalAuth.php @@ -29,6 +29,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; @@ -61,6 +62,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 0c8140e3c141c..3c4163926b69d 100644 --- a/apps/files_external/lib/Lib/Auth/Password/UserProvided.php +++ b/apps/files_external/lib/Lib/Auth/Password/UserProvided.php @@ -65,6 +65,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/features/external-storage.feature b/build/integration/features/external-storage.feature index d62c7a4f5c712..2b724be44b03c 100644 --- a/build/integration/features/external-storage.feature +++ b/build/integration/features/external-storage.feature @@ -77,6 +77,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 @@ -90,3 +106,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 b6d8e6bed858f490579588f96b42cbad370e7b7a 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 12/12] 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 d2dd70410eff5..4df504edc3fd9 100644 --- a/apps/files_external/templates/settings.php +++ b/apps/files_external/templates/settings.php @@ -140,7 +140,7 @@ function writeParameterInput($parameter, $options, $classes = []) { > - +