diff --git a/apps/files_sharing/css/sharetabview.css b/apps/files_sharing/css/sharetabview.css index 9d2a6c1d8813..fb709e4f0ec7 100644 --- a/apps/files_sharing/css/sharetabview.css +++ b/apps/files_sharing/css/sharetabview.css @@ -39,6 +39,58 @@ margin-right: 0; } +#shareWithList { + list-style-type: none; + padding : 0 0 16px; +} + +#shareWithList li { + padding-top : 5px; + padding-bottom: 5px; + font-weight : bold; + white-space : normal; +} + +#shareWithList .unshare img { + vertical-align: text-bottom; + /* properly align icons */ +} + +#shareWithList label input[type=checkbox] { + margin-left: 0; + position : relative; +} + +#shareWithList .username { + padding-right : 8px; + white-space : nowrap; + text-overflow : ellipsis; + max-width : 254px; + display : inline-block; + overflow : hidden; + vertical-align: middle; +} + +#shareWithList li label { + margin-right: 8px; +} + +#shareWithList .expiration { + max-width: 190px; + text-align: left; + margin: 0 0 0 5px; +} + +#shareWithList .removeExpiration { + text-indent: -9999px; + border: 0 none; + transform: translate(calc(-100% - 5px), -1px); + background-color: #F8F8F8; + background-position: center; + background-repeat: no-repeat; + background-image: url('../../../core/img/actions/close.svg'); +} + #shareDialogLinkList .link-shares + #shareTreeUserGroupList { margin-top: -20px; } @@ -99,15 +151,6 @@ background-size: 16px 16px; } -.shareTabView .action-item { - display: inherit; - opacity: .5; -} - -.shareTabView .action-item + .action-item { - margin-left: 5px; -} - .shareTabView .privacyWarningMessage { margin-top: 20px; } @@ -316,94 +359,4 @@ color: red; padding: 4px; display: block; -} - -/* ---------------------------------------------------- ShareWithList --- */ - -.shareWithList { - list-style-type : none; - padding : 0 0 16px; -} - -[class^='shareWithList__item'] { - display : flex; - justify-content : space-between; - flex-wrap : wrap; - padding-top : 10px; - padding-bottom : 10px; - font-weight : bold; -} - -.shareWithList__item--detailed { - background-color: #f8f8f8; - margin-left: -15px; - margin-right: -15px; - padding: 10px 15px; -} - -.shareWithList__item--detailed + .shareWithList__item--detailed { - border-top: 1px solid #eee -} - -[class^='shareWithList__container'] { - display : flex; - align-items : center; -} - -.shareWithList__container--left { - justify-content: flex-start; -} - -.shareWithList__container--right { - justify-content: flex-end; -} - -.shareWithList__item label input[type=checkbox] { - margin-left: 0; - position : relative; -} - -.shareWithList__item .username { - padding-right : 8px; - white-space : nowrap; - text-overflow : ellipsis; - max-width : 254px; - display : inline-block; - overflow : hidden; - vertical-align: middle; -} - -.shareWithList__item label { - margin-right: 8px; -} - -.shareWithList__item .expiration { - max-width: 190px; - text-align: left; - margin: 0 0 0 5px; -} - -.shareWithList__item .removeExpiration { - text-indent: -9999px; - border: 0 none; - transform: translate(calc(-100% - 5px), -1px); - background-color: #F8F8F8; - background-position: center; - background-repeat: no-repeat; - background-image: url('../../../core/img/actions/close.svg'); -} - -.shareWithList__item .toggleShareDetails { - display: inherit; - opacity: 0.5; -} - -.shareWithList__details { - width: 100%; - background-color: #f8f8f8; -} - -.shareWithList__details-group:first-of-type, -.shareWithList__details-group + .shareWithList__details-group { - margin-top: 10px; } \ No newline at end of file diff --git a/apps/files_sharing/lib/Controller/Share20OcsController.php b/apps/files_sharing/lib/Controller/Share20OcsController.php index 051088be91b9..73884274d1b4 100644 --- a/apps/files_sharing/lib/Controller/Share20OcsController.php +++ b/apps/files_sharing/lib/Controller/Share20OcsController.php @@ -499,7 +499,8 @@ public function createShare() { $share->setShareType($shareType); $share->setSharedBy($this->userSession->getUser()->getUID()); - $share = $this->setShareAttributes($share, $this->request->getParam('attributes', null)); + // set attributes array from request, or if not provided set empty array + $share = $this->setShareAttributes($share, $this->request->getParam('attributes', [])); try { $share = $this->shareManager->createShare($share); @@ -846,7 +847,11 @@ public function updateShare($id) { $share->setExpirationDate($expireDate); } - $share = $this->setShareAttributes($share, $this->request->getParam('attributes', null)); + // update attributes if provided + $newAttributes = $this->request->getParam('attributes', null); + if ($newAttributes !== null) { + $share = $this->setShareAttributes($share, $newAttributes); + } try { $share = $this->shareManager->updateShare($share); @@ -1113,24 +1118,22 @@ private function getShareById($id, $recipient = null) { /** * @param IShare $share - * @param string[][]|null $formattedShareAttributes + * @param string[][] $formattedShareAttributes * @return IShare modified share */ private function setShareAttributes(IShare $share, $formattedShareAttributes) { $newShareAttributes = $this->shareManager->newShare()->newAttributes(); - if ($formattedShareAttributes !== null) { - foreach ($formattedShareAttributes as $formattedAttr) { - $value = isset($formattedAttr['value']) ? $formattedAttr['value'] : null; - if (isset($formattedAttr['enabled'])) { - $value = (bool) \json_decode($formattedAttr['enabled']); - } - if ($value !== null) { - $newShareAttributes->setAttribute( - $formattedAttr['scope'], - $formattedAttr['key'], - $value - ); - } + foreach ($formattedShareAttributes as $formattedAttr) { + $value = isset($formattedAttr['value']) ? $formattedAttr['value'] : null; + if (isset($formattedAttr['enabled'])) { + $value = (bool) \json_decode($formattedAttr['enabled']); + } + if ($value !== null) { + $newShareAttributes->setAttribute( + $formattedAttr['scope'], + $formattedAttr['key'], + $value + ); } } $share->setAttributes($newShareAttributes); diff --git a/core/css/share.css b/core/css/share.css index 970899c56f9d..272fa4e63dab 100644 --- a/core/css/share.css +++ b/core/css/share.css @@ -22,7 +22,6 @@ left: 20px; } } - .shareTabView .unshare.icon-loading-small { margin-top: 1px; } @@ -95,6 +94,13 @@ padding: 8px; } +#shareWithList li { + padding-top: 10px; + padding-bottom: 10px; + font-weight: bold; + line-height: 21px; + white-space: normal; +} #shareWithList .shareOption { white-space: nowrap; display: inline-block; @@ -134,6 +140,37 @@ margin: 0 3px 0 8px; vertical-align: middle; } +a.showCruds { + display: inline; + opacity: 0.5; +} + +a.unshare { + display: inline; + float: right; + opacity: 0.5; + padding: 10px; + margin-top: -5px; + margin-right: -10px; +} + +a.time { + display: inline; + float: right; + opacity: 0.5; + padding: 10px; + margin-top: -5px; + margin-right: -10px; +} + +a.toggleShareDetails { + display: inline; + float: right; + opacity: 0.5; + padding: 10px; + margin-top: -5px; + margin-right: -10px; +} #link { border-top: 1px solid #ddd; @@ -178,6 +215,9 @@ a.showCruds:hover, a.unshare:hover { opacity: 1; } +a.toggleShareDetails:hover { + opacity: 1; +} #defaultExpireMessage, /* fix expire message going out of box */ .reshare { @@ -264,4 +304,4 @@ a.unshare:hover { .select2-container-multi .select2-choices .select2-search-choice { padding: 3px 18px 3px 5px; -} +} \ No newline at end of file diff --git a/core/js/sharedialogshareelistview.js b/core/js/sharedialogshareelistview.js index 1f776b869ca2..68b6f316a451 100644 --- a/core/js/sharedialogshareelistview.js +++ b/core/js/sharedialogshareelistview.js @@ -16,90 +16,84 @@ var TEMPLATE = ''; /** @@ -122,13 +116,15 @@ /** @type {Function} **/ _template: undefined, + _currentlyToggled: [], + events: { 'click .unshare': 'onUnshare', 'click .permissions': 'onPermissionChange', 'click .attributes': 'onPermissionChange', - 'click .toggleShareDetails' : 'onToggleShareDetails', 'click .mailNotification': 'onSendMailNotification', - 'click .removeExpiration' : 'onRemoveExpiration' + 'click .removeExpiration' : 'onRemoveExpiration', + 'click .toggleShareDetails' : 'onToggleShareDetails' }, initialize: function(options) { @@ -297,15 +293,32 @@ self._setDatepicker(this, { maxDate : self.configModel.getDefaultExpireDateUser(), enforced : self.configModel.isDefaultExpireDateUserEnforced() - }) - }) + }); + }); this.$el.find('.expiration-group:not(.hasDatepicker)').each(function(){ self._setDatepicker(this, { maxDate : self.configModel.getDefaultExpireDateGroup(), enforced : self.configModel.isDefaultExpireDateGroupEnforced() - }) - }) + }); + }); + + this.$el.bind("DOMNodeInserted", function(){ + // make sure to always enable toggled divs + if (!_.isUndefined(self._currentlyToggled)) { + + self.$el.find('li').each(function() + { + var $li = $(this); + var shareId = $li.data('share-id'); + if (!_.isUndefined(self._currentlyToggled[shareId])) { + self._toggleShareDivs(shareId, true); + } else { + self._toggleShareDivs(shareId, false); + } + }); + } + }); this.delegateEvents(); @@ -396,9 +409,7 @@ this.model.updateShare(shareId, { permissions: permissions, attributes: attributes - }, { - silent: true - } + }, {} ); }, @@ -432,24 +443,49 @@ this.model.updateShare( shareId, { expireDate: null - }); + }, {}); }, onToggleShareDetails: function(event) { - var $target = $(event.target); - $target.closest('.shareWithList__item').toggleClass('shareWithList__item--detailed').find('.shareWithList__details').toggleClass('hidden'); + this._toggleShareDetails(event); + }, + + _toggleShareDetails: function(event) { + var $li = $(event.target).closest('li'); + var shareId = $li.data('share-id'); + + if (!_.isUndefined(this._currentlyToggled[shareId])) { + this._currentlyToggled.splice(shareId, 1); + this._toggleShareDivs(shareId, false); + } else { + this._currentlyToggled[shareId] = true; + this._toggleShareDivs(shareId, true); + } + }, + + _toggleShareDivs: function(shareId, enabled) { + var $li = this.$el.find('li[data-share-id=' + shareId + ']'); + $li.children("div").each(function() + { + var $div = $(this); + if (!$div.hasClass( "avatar" )) { + if (enabled) { + $div.css('display', 'block'); + } else { + $div.css('display', 'none'); + } + } + }); }, _onExpirationChange: function(el) { - var $el = $(el) + var $el = $(el); var shareId = $el.closest('li').data('share-id'); - var expiration = moment($el.val(), 'DD-MM-YYYY').format() + var expiration = moment($el.val(), 'DD-MM-YYYY').format(); this.model.updateShare( shareId, { - expireDate: expiration, - }, { - silent: true - }); + expireDate: expiration + }, {}); }, _setDatepicker: function(el, params) { @@ -460,7 +496,7 @@ minDate: "+1d", dateFormat : 'dd-mm-yy', onSelect : function() { - self._onExpirationChange(el) + self._onExpirationChange(el); } }); diff --git a/core/js/shareitemmodel.js b/core/js/shareitemmodel.js index 600230b1a31a..24abd0479586 100644 --- a/core/js/shareitemmodel.js +++ b/core/js/shareitemmodel.js @@ -220,12 +220,13 @@ updateShare: function(shareId, properties, options) { var self = this; - options = _.defaults(options, { - silent: false - }) - - // Extend attributes for update share - properties.attributes = this._handleUpdateShareAttributes(shareId, properties, options); + // Extend attributes for update share if provided for update. + if (!_.isUndefined(properties.permissions) || !_.isUndefined(properties.attributes)) { + // Add empty string to array as data is not serialized + // with JSON.stringify and ajax omits empty array + properties.attributes = this._handleUpdateShareAttributes(shareId, properties, options); + properties.attributes.push(''); + } return $.ajax({ type: 'PUT', @@ -234,7 +235,6 @@ dataType: 'json' }).done(function() { self.fetch({ - silent: options.silent, success: function() { if (_.isFunction(options.success)) { options.success(self); @@ -672,10 +672,6 @@ fetch: function(options) { var model = this; - options = _.defaults(options, { - silent: false - }) - this.trigger('request', this); var deferred = $.when( @@ -695,11 +691,15 @@ reshare = model._groupReshares(data2[0].ocs.data); } + // update model properties, + // this should cause rerendering of the object model.set(model.parse({ shares: sharesMap, reshare: reshare }), { - silent: options.silent + // do not allow silent, as apps might rely on reacting to + // changes to the model + silent: false, }); }); @@ -873,6 +873,7 @@ * * @param shareIndex * @returns OC.Share.Types.ShareAttribute[] + * @deprecated ShareAttributesAPI v1 registration will be deprecated. ShareAttributesAPI v2 requires apps to extend this class */ getShareAttributes: function(shareIndex) { /** @type OC.Share.Types.ShareInfo **/ @@ -891,12 +892,10 @@ * @param {array} properties * @param {array} options * @returns {OC.Share.Types.ShareAttribute[]} + * @deprecated ShareAttributesAPI v1 will be deprecated. ShareAttributesAPI v2 requires apps to overwrite addShare * @private */ _handleAddShareAttributes: function(properties, options) { - /** - * @deprecated ShareAttributesAPI v1 will be depreciated. ShareAttributesAPI v2 requires apps to overwrite addShare - */ var shareAttributesV1 = []; var filteredRegisteredAttributes = this._filterRegisteredAttributes(properties.permissions); _.map(filteredRegisteredAttributes, function (filteredRegisteredAttribute) { @@ -939,12 +938,10 @@ * @param {array} properties * @param {array} options * @returns {OC.Share.Types.ShareAttribute[]} + * @deprecated ShareAttributesAPI v1 will be deprecated. ShareAttributesAPI v2 requires apps to overwrite updateShare * @private */ _handleUpdateShareAttributes: function(shareId, properties, options) { - /** - * @deprecated ShareAttributesAPI v1 will be depreciated. ShareAttributesAPI v2 requires apps to overwrite updateShare - */ var shareAttributesV1 = []; var filteredAttributes = []; var filteredRegisteredAttributes = this._filterRegisteredAttributes(properties.permissions); @@ -1041,7 +1038,7 @@ * * @param {number} permissions * @returns {OC.Share.Types.RegisteredShareAttribute[]} - * @deprecated ShareAttributesAPI v1 registration will be depreciated. ShareAttributesAPI v2 requires apps to extend this class + * @deprecated ShareAttributesAPI v1 registration will be deprecated. ShareAttributesAPI v2 requires apps to extend this class * @private */ _filterRegisteredAttributes: function(permissions) { @@ -1075,7 +1072,7 @@ * @param scope * @param key * @returns {OC.Share.Types.RegisteredShareAttribute} - * @deprecated ShareAttributesAPI v1 registration will be depreciated. ShareAttributesAPI v2 requires apps to extend this class + * @deprecated ShareAttributesAPI v1 registration will be deprecated. ShareAttributesAPI v2 requires apps to extend this class */ getRegisteredShareAttribute: function(scope, key) { for(var i in this._registeredAttributes) { @@ -1095,7 +1092,7 @@ * incompatible attribute -> functionality is ignored * * @param {OC.Share.Types.RegisteredShareAttribute} $shareAttribute - * @deprecated ShareAttributesAPI v1 registration will be depreciated. ShareAttributesAPI v2 requires apps to extend this class + * @deprecated ShareAttributesAPI v1 registration will be deprecated. ShareAttributesAPI v2 requires apps to extend this class */ registerShareAttribute: function($shareAttribute) { // Add extra permission or update if already existing diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index b7d4a7aa9870..9253acebe12e 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -334,8 +334,10 @@ protected function validatePermissions(IShare $share) { /* Use share node permission as default $maxPermissions */ $maxPermissions = $shareNode->getPermissions(); - /* Attributes default is null, as attributes are restricted only in reshare */ - $maxAttributes = null; + /* By default, there are no required attributes to be set on a file */ + $requiredAttributes = $this->newShare()->newAttributes(); + $currentAttributes = $share->getAttributes() !== null ? + $share->getAttributes() : $this->newShare()->newAttributes(); /* * Quick fix for #23536 @@ -367,19 +369,24 @@ protected function validatePermissions(IShare $share) { '@phan-var \OCA\Files_Sharing\SharedStorage $shareFileStorage'; $parentShare = $shareFileStorage->getShare(); $maxPermissions = $parentShare->getPermissions(); - $maxAttributes = $parentShare->getAttributes(); + $requiredAttributes = $parentShare->getAttributes(); } } } - /* Check that we do not share with more permissions than we have */ + /** + * Check that we do not share with more permissions than we have + */ if (!$this->strictSubsetOfPermissions($maxPermissions, $share->getPermissions())) { $message_t = $this->l->t('Cannot set the requested share permissions for %s', [$share->getNode()->getName()]); throw new GenericShareException($message_t, $message_t, 404); } - /* Check that we do not share with more attributes than we have */ - if ($maxAttributes !== null && !$this->strictSubsetOfAttributes($maxAttributes, $share->getAttributes())) { + /** + * Check that all required share attributes that were set on the file + * will be respected when e.g. reshared. + */ + if (!$this->strictSubsetOfAttributes($requiredAttributes, $currentAttributes)) { $message_t = $this->l->t('Cannot set the requested share attributes for %s', [$share->getNode()->getName()]); throw new GenericShareException($message_t, $message_t, 404); } @@ -1824,31 +1831,17 @@ private function strictSubsetOfPermissions($allowedPermissions, $newPermissions) } /** - * Check $newAttributes attribute is a subset of $allowedAttributes + * Check $currentAttributes attribute is a subset of $requiredAttributes. + * Existing attributes cannot be modified * - * @param IAttributes $allowedAttributes - * @param IAttributes $newAttributes - * @return boolean ,true if $allowedAttributes enabled is super set of $newAttributes enabled, else false + * @param IAttributes $requiredAttributes + * @param IAttributes $currentAttributes + * @return boolean ,true if $currentAttributes is super set of $requiredAttributes, else false */ - private function strictSubsetOfAttributes($allowedAttributes, $newAttributes) { - // if both are empty, it is strict subset - if ((!$allowedAttributes || empty($allowedAttributes->toArray())) - && (!$newAttributes || empty($newAttributes->toArray()))) { - return true; - } - - // make sure that number of attributes is the same - if (\count($allowedAttributes->toArray()) !== \count($newAttributes->toArray())) { - return false; - } - - // if number of attributes is the same, make sure that attributes are - // existing in allowed set and disabled attribute is not being enabled - foreach ($newAttributes->toArray() as $newAttribute) { - $allowedEnabled = $allowedAttributes->getAttribute($newAttribute['scope'], $newAttribute['key']); - if (($newAttribute['enabled'] === true && $allowedEnabled === false) - || ($newAttribute['enabled'] === null && $allowedEnabled !== null) - || $allowedEnabled === null) { + private function strictSubsetOfAttributes(IAttributes $requiredAttributes, IAttributes $currentAttributes) { + foreach ($requiredAttributes->toArray() as $requiredAttribute) { + $currentAttribute = $currentAttributes->getAttribute($requiredAttribute['scope'], $requiredAttribute['key']); + if ($requiredAttribute['enabled'] !== $currentAttribute) { return false; } } diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 1f5d67f7d7e7..9d7ea28278d8 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -921,10 +921,10 @@ public function dataShareNotEnoughPermissions() { $data[] = [$share, $fileReshare, 'Cannot set the requested share permissions for sharedfile', true]; // Normal reshare should just use supershare node attributes - // exception when trying to share with more attributes than supershare has + // exception when trying to remove share attributes that supershare has $superShareAttributes = new ShareAttributes(); + $superShareAttributes->setAttribute('test', 'test', true); $shareAttributes = new ShareAttributes(); - $shareAttributes->setAttribute('test', 'test', true); $superShare = $this->createMock(IShare::class); $superShare->method('getPermissions')->willReturn(17); $superShare->method('getAttributes')->willReturn($superShareAttributes); @@ -3637,17 +3637,17 @@ public function testGetAllSharedWith() { /** * @dataProvider strictSubsetOfAttributesDataProvider * - * @param IAttributes $allowedAttributes - * @param IAttributes $newAttributes + * @param IAttributes $requiredAttributes + * @param IAttributes $currentAttributes * @param boolean $expected */ - public function testStrictSubsetOfAttributes($allowedAttributes, $newAttributes, $expected) { + public function testStrictSubsetOfAttributes($requiredAttributes, $currentAttributes, $expected) { $this->assertEquals( $expected, $this->invokePrivate( $this->manager, 'strictSubsetOfAttributes', - [$allowedAttributes, $newAttributes] + [$requiredAttributes, $currentAttributes] ) ); } @@ -3665,17 +3665,17 @@ public function strictSubsetOfAttributesDataProvider() { $shareAttributes->setAttribute('test', 'test', true); $data[] = [$superShareAttributes,$shareAttributes, true]; - // no exception - disabling attribute that supershare has enabled + // no exception - adding an attribute while supershare has none $superShareAttributes = new ShareAttributes(); - $superShareAttributes->setAttribute('test', 'test', true); $shareAttributes = new ShareAttributes(); - $shareAttributes->setAttribute('test', 'test', false); + $shareAttributes->setAttribute('test', 'test', true); $data[] = [$superShareAttributes,$shareAttributes, true]; - // exception - adding an attribute while supershare has none + // exception - disabling attribute that supershare has enabled $superShareAttributes = new ShareAttributes(); + $superShareAttributes->setAttribute('test', 'test', true); $shareAttributes = new ShareAttributes(); - $shareAttributes->setAttribute('test', 'test', true); + $shareAttributes->setAttribute('test', 'test', false); $data[] = [$superShareAttributes,$shareAttributes, false]; // exception - enabling attribute that supershare has disabled