Merge pull request #8390 from dasisdormax/master

Add indeterminate state to share permission checkbox

3 files changed, 84 insertions, 18 deletions

diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js
index 30cbeff3c64..e76a25bd020 100644
--- a/core/js/sharedialogshareelistview.js
+++ b/core/js/sharedialogshareelistview.js
@@ -30,7 +30,7 @@
 					'<span class="sharingOptionsGroup">' +
 						'{{#if editPermissionPossible}}' +
 						'<span class="shareOption">' +
-							'<input id="canEdit-{{cid}}-{{shareWith}}" type="checkbox" name="edit" class="permissions checkbox" {{#if hasEditPermission}}checked="checked"{{/if}} />' +
+							'<input id="canEdit-{{cid}}-{{shareWith}}" type="checkbox" name="edit" class="permissions checkbox" />' +
 							'<label for="canEdit-{{cid}}-{{shareWith}}">{{canEditLabel}}</label>' +
 						'</span>' +
 						'{{/if}}' +
@@ -232,7 +232,7 @@
 			return _.extend(hasPermissionOverride, {
 				cid: this.cid,
 				hasSharePermission: this.model.hasSharePermission(shareIndex),
-				hasEditPermission: this.model.hasEditPermission(shareIndex),
+				editPermissionState: this.model.editPermissionState(shareIndex),
 				hasCreatePermission: this.model.hasCreatePermission(shareIndex),
 				hasUpdatePermission: this.model.hasUpdatePermission(shareIndex),
 				hasDeletePermission: this.model.hasDeletePermission(shareIndex),
@@ -379,16 +379,18 @@
 				$.extend(sharee, this.getShareProperties());
 				var $li = this.$('li[data-share-id=' + permissionChangeShareId + ']');
 				$li.find('.sharingOptionsGroup .popovermenu').replaceWith(this.popoverMenuTemplate(sharee));
-
-				var checkBoxId = 'canEdit-' + this.cid + '-' + sharee.shareWith;
-				checkBoxId = '#' + checkBoxId.replace( /(:|\.|\[|\]|,|=|@)/g, "\\$1");
-				var $edit = $li.parent().find(checkBoxId);
-				if($edit.length === 1) {
-					$edit.prop('checked', sharee.hasEditPermission);
-				}
 			}
 
 			var _this = this;
+			this.getShareeList().forEach(function(sharee) {
+				var checkBoxId = 'canEdit-' + _this.cid + '-' + sharee.shareWith;
+				checkBoxId = '#' + checkBoxId.replace( /(:|\.|\[|\]|,|=|@|\/)/g, "\\$1");
+				var $edit = _this.$(checkBoxId);
+				if($edit.length === 1) {
+					$edit.prop('checked', sharee.editPermissionState === 'checked');
+					$edit.prop('indeterminate', sharee.editPermissionState === 'indeterminate');
+				}
+			});
 			this.$('.popovermenu').on('afterHide', function() {
 				_this._menuOpen = false;
 			});
@@ -627,8 +629,10 @@
 					}
 				} else {
 					var numberChecked = $checkboxes.filter(':checked').length;
-					checked = numberChecked > 0;
-					$('input[name="edit"]', $li).prop('checked', checked);
+					checked = numberChecked === $checkboxes.length;
+					var $editCb = $('input[name="edit"]', $li);
+					$editCb.prop('checked', checked);
+					$editCb.prop('indeterminate', !checked && numberChecked > 0);
 				}
 			} else {
 				if ($element.attr('name') === 'edit' && $element.is(':checked')) {
diff --git a/core/js/shareitemmodel.js b/core/js/shareitemmodel.js
index afe86fa464b..b699513c734 100644
--- a/core/js/shareitemmodel.js
+++ b/core/js/shareitemmodel.js
@@ -567,12 +567,26 @@
 		},
 
 		/**
-		 * @returns {boolean}
-		 */
-		hasEditPermission: function(shareIndex) {
-			return    this.hasCreatePermission(shareIndex)
-				   || this.hasUpdatePermission(shareIndex)
-				   || this.hasDeletePermission(shareIndex);
+		 * @returns {string}
+		 *     The state that the 'can edit' permission checkbox should have.
+		 *     Possible values:
+		 *     - empty string: no permission
+		 *     - 'checked': all applicable permissions
+		 *     - 'indeterminate': some but not all permissions
+		 */
+		editPermissionState: function(shareIndex) {
+			var hcp = this.hasCreatePermission(shareIndex);
+			var hup = this.hasUpdatePermission(shareIndex);
+			var hdp = this.hasDeletePermission(shareIndex);
+			if (!hcp && !hup && !hdp) {
+				return '';
+			}
+			if (   (this.createPermissionPossible() && !hcp)
+				|| (this.updatePermissionPossible() && !hup)
+				|| (this.deletePermissionPossible() && !hdp)   ) {
+				return 'indeterminate';
+			}
+			return 'checked';
 		},
 
 		/**
diff --git a/core/js/tests/specs/sharedialogshareelistview.js b/core/js/tests/specs/sharedialogshareelistview.js
index 8ee2c48fe39..cc0268ba580 100644
--- a/core/js/tests/specs/sharedialogshareelistview.js
+++ b/core/js/tests/specs/sharedialogshareelistview.js
@@ -89,6 +89,37 @@ describe('OC.Share.ShareDialogShareeListView', function () {
 		updateShareStub.restore();
 	});
 
+	describe('Sets correct initial checkbox state', function () {
+		it('marks edit box as indeterminate when only some permissions are given', function () {
+			shareModel.set('shares', [{
+				id: 100,
+				item_source: 123,
+				permissions: 1 | OC.PERMISSION_UPDATE,
+				share_type: OC.Share.SHARE_TYPE_USER,
+				share_with: 'user1',
+				share_with_displayname: 'User One',
+				itemType: 'folder'
+			}]);
+			shareModel.set('itemType', 'folder');
+			listView.render();
+			expect(listView.$el.find("input[name='edit']").is(':indeterminate')).toEqual(true);
+		});
+
+		it('Checks edit box when all permissions are given', function () {
+			shareModel.set('shares', [{
+				id: 100,
+				item_source: 123,
+				permissions: 1 | OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE | OC.PERMISSION_DELETE,
+				share_type: OC.Share.SHARE_TYPE_USER,
+				share_with: 'user1',
+				share_with_displayname: 'User One',
+				itemType: 'folder'
+			}]);
+			shareModel.set('itemType', 'folder');
+			listView.render();
+			expect(listView.$el.find("input[name='edit']").is(':checked')).toEqual(true);
+		});
+	});
 	describe('Manages checkbox events correctly', function () {
 		it('Checks cruds boxes when edit box checked', function () {
 			shareModel.set('shares', [{
@@ -106,7 +137,7 @@ describe('OC.Share.ShareDialogShareeListView', function () {
 			expect(updateShareStub.calledOnce).toEqual(true);
 		});
 
-		it('Checks edit box when create/update/delete are checked', function () {
+		it('marks edit box as indeterminate when some of create/update/delete are checked', function () {
 			shareModel.set('shares', [{
 				id: 100,
 				item_source: 123,
@@ -119,6 +150,23 @@ describe('OC.Share.ShareDialogShareeListView', function () {
 			shareModel.set('itemType', 'folder');
 			listView.render();
 			listView.$el.find("input[name='update']").click();
+			expect(listView.$el.find("input[name='edit']").is(':indeterminate')).toEqual(true);
+			expect(updateShareStub.calledOnce).toEqual(true);
+		});
+
+		it('Checks edit box when all of create/update/delete are checked', function () {
+			shareModel.set('shares', [{
+				id: 100,
+				item_source: 123,
+				permissions: 1 | OC.PERMISSION_CREATE | OC.PERMISSION_DELETE,
+				share_type: OC.Share.SHARE_TYPE_USER,
+				share_with: 'user1',
+				share_with_displayname: 'User One',
+				itemType: 'folder'
+			}]);
+			shareModel.set('itemType', 'folder');
+			listView.render();
+			listView.$el.find("input[name='update']").click();
 			expect(listView.$el.find("input[name='edit']").is(':checked')).toEqual(true);
 			expect(updateShareStub.calledOnce).toEqual(true);
 		});