Merge pull request #36087 from nextcloud/enh/noid/improve-applicable-ext-storage

Improve saving applicable users in ext storage

6 files changed, 150 insertions, 32 deletions

diff --git a/apps/files_external/css/settings.css b/apps/files_external/css/settings.css
index f9c98a81a7a..69cfdecc551 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,#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 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)}/*# 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}#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)}/*# sourceMappingURL=settings.css.map */
diff --git a/apps/files_external/css/settings.css.map b/apps/files_external/css/settings.css.map
index ad59643d517..12e0754812c 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,qDACC,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,sCAID,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","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,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","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 7485faa4c92..d3df5704d3b 100644
--- a/apps/files_external/css/settings.scss
+++ b/apps/files_external/css/settings.scss
@@ -11,7 +11,7 @@
 }
 
 #externalStorage td {
-	& > input, & > select {
+	& > input:not(.applicableToAllUsers), & > select {
 		width: 100%;
 	}
 }
@@ -101,6 +101,10 @@
 	background-color: rgba(134, 255, 110, 0.9);
 }
 
+#externalStorage td.applicable label {
+	display: inline-flex;
+	align-items: center;
+}
 
 #externalStorage td.applicable div.chzn-container {
 	position: relative;
diff --git a/apps/files_external/js/settings.js b/apps/files_external/js/settings.js
index 5228b958b67..d3e9fd44c0d 100644
--- a/apps/files_external/js/settings.js
+++ b/apps/files_external/js/settings.js
@@ -24,6 +24,28 @@ function getSelection($row) {
 	return values;
 }
 
+function getSelectedApplicable($row) {
+	var users = [];
+	var groups = [];
+	var multiselect = getSelection($row);
+	$.each(multiselect, function(index, value) {
+		// FIXME: don't rely on string parts to detect groups...
+		var pos = (value.indexOf)?value.indexOf('(group)'): -1;
+		if (pos !== -1) {
+			groups.push(value.substr(0, pos));
+		} else {
+			users.push(value);
+		}
+	});
+
+	// FIXME: this should be done in the multiselect change event instead
+	$row.find('.applicable')
+		.data('applicable-groups', groups)
+		.data('applicable-users', users);
+
+	return { users, groups };
+}
+
 function highlightBorder($element, highlight) {
 	$element.toggleClass('warning-input', highlight);
 	return highlight;
@@ -56,7 +78,7 @@ function highlightInput($input) {
  * @param {Array<Object>} array of jQuery elements
  * @param {number} userListLimit page size for result list
  */
-function addSelect2 ($elements, userListLimit) {
+function initApplicableUsersMultiselect($elements, userListLimit) {
 	var escapeHTML = function (text) {
 		return text.toString()
 			.split('&').join('&amp;')
@@ -68,8 +90,8 @@ function addSelect2 ($elements, userListLimit) {
 	if (!$elements.length) {
 		return;
 	}
-	$elements.select2({
-		placeholder: t('files_external', 'All users. Type to select user or group.'),
+	return $elements.select2({
+		placeholder: t('files_external', 'Type to select user or group.'),
 		allowClear: true,
 		multiple: true,
 		toggleSelect: true,
@@ -167,6 +189,8 @@ function addSelect2 ($elements, userListLimit) {
 				$div.avatar($div.data('name'),32);
 			}
 		});
+	}).on('change', function(event) {
+		highlightBorder($(event.target).closest('.applicableUsersContainer').find('.select2-choices'), !event.val.length);
 	});
 }
 
@@ -721,6 +745,8 @@ MountConfigListView.prototype = _.extend({
 
 		this.$el.on('change', '.selectBackend', _.bind(this._onSelectBackend, this));
 		this.$el.on('change', '.selectAuthMechanism', _.bind(this._onSelectAuthMechanism, this));
+
+		this.$el.on('change', '.applicableToAllUsers', _.bind(this._onChangeApplicableToAllUsers, this));
 	},
 
 	_onChange: function(event) {
@@ -746,6 +772,7 @@ MountConfigListView.prototype = _.extend({
 
 		var onCompletion = jQuery.Deferred();
 		$tr = this.newStorage(storageConfig, onCompletion);
+		$tr.find('.applicableToAllUsers').prop('checked', false).trigger('change');
 		onCompletion.resolve();
 
 		$tr.find('td.configuration').children().not('[type=hidden]').first().focus();
@@ -764,6 +791,19 @@ MountConfigListView.prototype = _.extend({
 		this.saveStorageConfig($tr);
 	},
 
+	_onChangeApplicableToAllUsers: function(event) {
+		var $target = $(event.target);
+		var $tr = $target.closest('tr');
+		var checked = $target.is(':checked');
+
+		$tr.find('.applicableUsersContainer').toggleClass('hidden', checked);
+		if (!checked) {
+			$tr.find('.applicableUsers').select2('val', '', true);
+		}
+
+		this.saveStorageConfig($tr);
+	},
+
 	/**
 	 * Configure the storage config with a new authentication mechanism
 	 *
@@ -818,7 +858,7 @@ MountConfigListView.prototype = _.extend({
 		$tr.removeAttr('id');
 		$tr.find('select#selectBackend');
 		if (!deferAppend) {
-			addSelect2($tr.find('.applicableUsers'), this._userListLimit);
+			initApplicableUsersMultiselect($tr.find('.applicableUsers'), this._userListLimit);
 		}
 
 		if (storageConfig.id) {
@@ -890,7 +930,14 @@ MountConfigListView.prototype = _.extend({
 				})
 			);
 		}
-		$tr.find('.applicableUsers').val(applicable).trigger('change');
+		if (applicable.length) {
+			$tr.find('.applicableUsers').val(applicable).trigger('change')
+			$tr.find('.applicableUsersContainer').removeClass('hidden');
+		} else {
+			// applicable to all
+			$tr.find('.applicableUsersContainer').addClass('hidden');
+		}
+		$tr.find('.applicableToAllUsers').prop('checked', !applicable.length);
 
 		var priorityEl = $('<input type="hidden" class="priority" value="' + backend.priority + '" />');
 		$tr.append(priorityEl);
@@ -967,7 +1014,7 @@ MountConfigListView.prototype = _.extend({
 						}
 						$rows = $rows.add($tr);
 					});
-					addSelect2(self.$el.find('.applicableUsers'), this._userListLimit);
+					initApplicableUsersMultiselect(self.$el.find('.applicableUsers'), this._userListLimit);
 					self.$el.find('tr#addMountPoint').before($rows);
 					var mainForm = $('#files_external');
 					if (result.length === 0 && mainForm.attr('data-can-create') === 'false') {
@@ -1007,7 +1054,7 @@ MountConfigListView.prototype = _.extend({
 					}
 					$rows = $rows.add($tr);
 				});
-				addSelect2($rows.find('.applicableUsers'), this._userListLimit);
+				initApplicableUsersMultiselect($rows.find('.applicableUsers'), this._userListLimit);
 				self.$el.find('tr#addMountPoint').before($rows);
 				onCompletion.resolve();
 				onLoaded2.resolve();
@@ -1126,24 +1173,25 @@ MountConfigListView.prototype = _.extend({
 
 		// gather selected users and groups
 		if (!this._isPersonal) {
-			var groups = [];
-			var users = [];
-			var multiselect = getSelection($tr);
-			$.each(multiselect, function(index, value) {
-				var pos = (value.indexOf)?value.indexOf('(group)'): -1;
-				if (pos !== -1) {
-					groups.push(value.substr(0, pos));
-				} else {
-					users.push(value);
-				}
-			});
-			// FIXME: this should be done in the multiselect change event instead
-			$tr.find('.applicable')
-				.data('applicable-groups', groups)
-				.data('applicable-users', users);
+			var multiselect = getSelectedApplicable($tr);
+			var users = multiselect.users || [];
+			var groups = multiselect.groups || [];
+			var isApplicableToAllUsers = $tr.find('.applicableToAllUsers').is(':checked');
+
+			if (isApplicableToAllUsers) {
+				storage.applicableUsers = [];
+				storage.applicableGroups = [];
+			} else {
+				storage.applicableUsers = users;
+				storage.applicableGroups = groups;
 
-			storage.applicableUsers = users;
-			storage.applicableGroups = groups;
+				if (!storage.applicableUsers.length && !storage.applicableGroups.length) {
+					if (!storage.errors) {
+						storage.errors = {};
+					}
+					storage.errors['requiredApplicable'] = true;
+				}
+			}
 
 			storage.priority = parseInt($tr.find('input.priority').val() || '100', 10);
 		}
diff --git a/apps/files_external/templates/settings.php b/apps/files_external/templates/settings.php
index af7438e28d7..d715e5946f1 100644
--- a/apps/files_external/templates/settings.php
+++ b/apps/files_external/templates/settings.php
@@ -171,7 +171,10 @@ $canCreateMounts = $_['visibilityType'] === BackendService::VISIBILITY_ADMIN ||
 				<td class="configuration"></td>
 				<?php if ($_['visibilityType'] === BackendService::VISIBILITY_ADMIN): ?>
 					<td class="applicable" align="right">
-						<input type="hidden" class="applicableUsers" style="width:20em;" value="" />
+						<label><input type="checkbox" class="applicableToAllUsers" checked="" /><?php p($l->t('All users')); ?></label>
+						<div class="applicableUsersContainer">
+							<input type="hidden" class="applicableUsers" style="width:20em;" value="" />
+						</div>
 					</td>
 				<?php endif; ?>
 				<td class="mountOptionsToggle hidden">
diff --git a/apps/files_external/tests/js/settingsSpec.js b/apps/files_external/tests/js/settingsSpec.js
index 18997941111..4032f6f6a37 100644
--- a/apps/files_external/tests/js/settingsSpec.js
+++ b/apps/files_external/tests/js/settingsSpec.js
@@ -35,12 +35,13 @@ describe('OCA.Files_External.Settings tests', function() {
 
 	beforeEach(function() {
 		clock = sinon.useFakeTimers();
+		select2ApplicableUsers = [];
 		select2Stub = sinon.stub($.fn, 'select2').callsFake(function(args) {
 			if (args === 'val') {
 				return select2ApplicableUsers;
 			}
 			return {
-				on: function() {}
+				on: function() { return this; }
 			};
 		});
 
@@ -63,6 +64,7 @@ describe('OCA.Files_External.Settings tests', function() {
 			'<td class="authentication"></td>' +
 			'<td class="configuration"></td>' +
 			'<td class="applicable">' +
+			'<input type="checkbox" class="applicableToAllUsers">' +
 			'<input type="hidden" class="applicableUsers">' +
 			'</td>' +
 			'<td class="mountOptionsToggle">'+
@@ -172,6 +174,7 @@ describe('OCA.Files_External.Settings tests', function() {
 
 		function selectBackend(backendName) {
 			view.$el.find('.selectBackend:first').val(backendName).trigger('change');
+			view.$el.find('.applicableToAllUsers').prop('checked', true).trigger('change');
 		}
 
 		beforeEach(function() {
@@ -255,6 +258,59 @@ describe('OCA.Files_External.Settings tests', function() {
 
 				// TODO: respond and check data-id
 			});
+			it('saves storage with applicable users', function() {
+				var $field1 = $tr.find('input[data-parameter=field1]');
+				expect($field1.length).toEqual(1);
+				$field1.val('test');
+				$field1.trigger(new $.Event('keyup', {keyCode: 97}));
+
+				$tr.find('.applicableToAllUsers').prop('checked', false).trigger('change');
+				select2ApplicableUsers = ['user1', 'user2', 'group1(group)', 'group2(group)'];
+
+				var $saveButton = $tr.find('td.save .icon-checkmark');
+				$saveButton.click();
+
+				expect(fakeServer.requests.length).toEqual(1);
+				var request = fakeServer.requests[0];
+				expect(request.url).toEqual(OC.getRootPath() + '/index.php/apps/files_external/globalstorages');
+				expect(JSON.parse(request.requestBody)).toEqual({
+					backend: '\\OC\\TestBackend',
+					authMechanism: 'mechanism1',
+					backendOptions: {
+						'field1': 'test',
+						'field2': ''
+					},
+					mountPoint: 'TestBackend',
+					priority: 11,
+					applicableUsers: ['user1', 'user2'],
+					applicableGroups: ['group1', 'group2'],
+					mountOptions: {
+						encrypt: true,
+						previews: true,
+						enable_sharing: false,
+						filesystem_check_changes: 1,
+						encoding_compatibility: false,
+						readonly: false,
+					},
+					testOnly: true
+				});
+
+				// TODO: respond and check data-id
+			});
+			it('does not saves storage without applicable users and unchecked all users checkbox', function() {
+				var $field1 = $tr.find('input[data-parameter=field1]');
+				expect($field1.length).toEqual(1);
+				$field1.val('test');
+				$field1.trigger(new $.Event('keyup', {keyCode: 97}));
+
+				$tr.find('.applicableToAllUsers').prop('checked', false).trigger('change');
+
+				var $saveButton = $tr.find('td.save .icon-checkmark');
+				$saveButton.click();
+
+				expect(fakeServer.requests.length).toEqual(0);
+			});
+
 			it('saves storage after closing mount options popovermenu', function() {
 				$tr.find('.mountOptionsToggle .icon-more').click();
 				$tr.find('[name=previews]').trigger(new $.Event('keyup', {keyCode: 97}));
@@ -279,6 +335,16 @@ describe('OCA.Files_External.Settings tests', function() {
 			});
 
 			it('lists missing fields in storage errors', function() {
+				$tr.find('.applicableToAllUsers').prop('checked', false).trigger('change');
+				var storage = view.getStorageConfig($tr);
+
+				expect(storage.errors).toEqual({
+					backendOptions: ['field_text', 'field_password'],
+					requiredApplicable: true,
+				});
+			});
+
+			it('does not list applicable when all users checkbox is ticked', function() {
 				var storage = view.getStorageConfig($tr);
 
 				expect(storage.errors).toEqual({
@@ -399,9 +465,6 @@ describe('OCA.Files_External.Settings tests', function() {
 			});
 		});
 	});
-	describe('applicable user list', function() {
-		// TODO: test select2 retrieval logic
-	});
 	describe('allow user mounts section', function() {
 		// TODO: test allowUserMounting section
 	});