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 db77fe4dfc1..30bf85a6764 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 30bf85a6764..e1d47dbdc89 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 e1d47dbdc89..d9f940cca05 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 2d70ada6973..fd7f2ec9b6d 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 c980ab3b126..df011384a4f 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 5acf373da73..ae9dcae0800 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 d9f940cca05..1c27642c6a3 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 1c27642c6a3..92b14e9454d 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 92b14e9454d..2ece2b56740 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 2ece2b56740..49e6cb42219 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 49e6cb42219..390fa8fea8c 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 390fa8fea8c..1225115eff2 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 e12a40ac6b4..77083c3c694 100644 --- a/build/integration/features/bootstrap/BasicStructure.php +++ b/build/integration/features/bootstrap/BasicStructure.php @@ -354,7 +354,7 @@ trait BasicStructure { $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 00000000000..2a73f22c1b4 --- /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 a3a600d6625..27ad69857e2 100644 --- a/build/integration/features/bootstrap/FeatureContext.php +++ b/build/integration/features/bootstrap/FeatureContext.php @@ -34,6 +34,7 @@ require __DIR__ . '/../../vendor/autoload.php'; */ 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 d92cca3c458..d62c7a4f5c7 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 45a0333038e..0d318771a7b 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 6312a3d136e..bf43ceebde1 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 @@ declare(strict_types=1); 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 @@ class UserGlobalAuth extends AuthMechanism { 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 0c8140e3c14..3c4163926b6 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 @@ class UserProvided extends AuthMechanism implements IUserProvided { } 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 d62c7a4f5c7..2b724be44b0 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 d2dd70410ef..4df504edc3f 100644 --- a/apps/files_external/templates/settings.php +++ b/apps/files_external/templates/settings.php @@ -140,7 +140,7 @@ function writeParameterInput($parameter, $options, $classes = []) { > - +