From d4238a52b29033716e5836b426f2c6b91d11e11c Mon Sep 17 00:00:00 2001 From: Maximilian Wende Date: Thu, 15 Feb 2018 21:10:39 +0100 Subject: [PATCH 1/3] Add indeterminate state to 'can edit' share permission checkbox, see #8371 Signed-off-by: Maximilian Wende --- core/js/sharedialogshareelistview.js | 7 ++++--- core/js/shareitemmodel.js | 24 +++++++++++++++++++----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index 30cbeff3c64..c4b56627093 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -30,7 +30,7 @@ '' + '{{#if editPermissionPossible}}' + '' + - '' + + '' + '' + '' + '{{/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), @@ -384,7 +384,8 @@ checkBoxId = '#' + checkBoxId.replace( /(:|\.|\[|\]|,|=|@)/g, "\\$1"); var $edit = $li.parent().find(checkBoxId); if($edit.length === 1) { - $edit.prop('checked', sharee.hasEditPermission); + $edit.prop('checked', sharee.editPermissionState === 'checked'); + $edit.prop('indeterminate', sharee.editPermissionState === 'indeterminate'); } } 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} + * @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 */ - hasEditPermission: function(shareIndex) { - return this.hasCreatePermission(shareIndex) - || this.hasUpdatePermission(shareIndex) - || this.hasDeletePermission(shareIndex); + 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'; }, /** From 602f50b2d4781cd9a874e7354b44315d6c0debb5 Mon Sep 17 00:00:00 2001 From: Maximilian Wende Date: Thu, 15 Feb 2018 22:41:09 +0100 Subject: [PATCH 2/3] Fix indeterminate checkbox state not showing correctly Also, the checkbox is updated to the correct state while a permission change is in progress. should fix issue #8371 Signed-off-by: Maximilian Wende --- core/js/sharedialogshareelistview.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index c4b56627093..41eed751afb 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -30,7 +30,7 @@ '' + '{{#if editPermissionPossible}}' + '' + - '' + + '' + '' + '' + '{{/if}}' + @@ -379,17 +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; + var _this = this; + this.getShareeList().forEach(function(sharee) { + var checkBoxId = 'canEdit-' + _this.cid + '-' + sharee.shareWith; checkBoxId = '#' + checkBoxId.replace( /(:|\.|\[|\]|,|=|@)/g, "\\$1"); - var $edit = $li.parent().find(checkBoxId); + var $edit = _this.$(checkBoxId); if($edit.length === 1) { $edit.prop('checked', sharee.editPermissionState === 'checked'); $edit.prop('indeterminate', sharee.editPermissionState === 'indeterminate'); } - } - - var _this = this; + }); this.$('.popovermenu').on('afterHide', function() { _this._menuOpen = false; }); @@ -628,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')) { From 7c453b24259a9a6fa03bb26cc2213013ecff4b8e Mon Sep 17 00:00:00 2001 From: Maximilian Wende Date: Fri, 16 Feb 2018 11:42:41 +0100 Subject: [PATCH 3/3] Update tests for indeterminate state, fix slashes not being escaped Signed-off-by: Maximilian Wende --- core/js/sharedialogshareelistview.js | 2 +- .../tests/specs/sharedialogshareelistview.js | 50 ++++++++++++++++++- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index 41eed751afb..e76a25bd020 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -384,7 +384,7 @@ var _this = this; this.getShareeList().forEach(function(sharee) { var checkBoxId = 'canEdit-' + _this.cid + '-' + sharee.shareWith; - checkBoxId = '#' + checkBoxId.replace( /(:|\.|\[|\]|,|=|@)/g, "\\$1"); + checkBoxId = '#' + checkBoxId.replace( /(:|\.|\[|\]|,|=|@|\/)/g, "\\$1"); var $edit = _this.$(checkBoxId); if($edit.length === 1) { $edit.prop('checked', sharee.editPermissionState === '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); });