diff --git a/apps/dav/lib/SystemTag/SystemTagNode.php b/apps/dav/lib/SystemTag/SystemTagNode.php index 94c310b8cb10..2852a1941bcb 100644 --- a/apps/dav/lib/SystemTag/SystemTagNode.php +++ b/apps/dav/lib/SystemTag/SystemTagNode.php @@ -111,11 +111,12 @@ public function setName($name) { * @param string $name new tag name * @param bool $userVisible user visible * @param bool $userAssignable user assignable + * @param bool $userEditable user editable * @throws NotFound whenever the given tag id does not exist * @throws Forbidden whenever there is no permission to update said tag * @throws Conflict whenever a tag already exists with the given attributes */ - public function update($name, $userVisible, $userAssignable) { + public function update($name, $userVisible, $userAssignable, $userEditable = false) { try { if (!$this->tagManager->canUserSeeTag($this->tag, $this->user)) { throw new NotFound('Tag with id ' . $this->tag->getId() . ' does not exist'); @@ -134,7 +135,7 @@ public function update($name, $userVisible, $userAssignable) { } } - $this->tagManager->updateTag($this->tag->getId(), $name, $userVisible, $userAssignable); + $this->tagManager->updateTag($this->tag->getId(), $name, $userVisible, $userAssignable, $userEditable); } catch (TagNotFoundException $e) { throw new NotFound('Tag with id ' . $this->tag->getId() . ' does not exist'); } catch (TagAlreadyExistsException $e) { diff --git a/apps/dav/lib/SystemTag/SystemTagPlugin.php b/apps/dav/lib/SystemTag/SystemTagPlugin.php index ed3151f4a240..5d722c40bba4 100644 --- a/apps/dav/lib/SystemTag/SystemTagPlugin.php +++ b/apps/dav/lib/SystemTag/SystemTagPlugin.php @@ -50,9 +50,11 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin { const ID_PROPERTYNAME = '{http://owncloud.org/ns}id'; const DISPLAYNAME_PROPERTYNAME = '{http://owncloud.org/ns}display-name'; const USERVISIBLE_PROPERTYNAME = '{http://owncloud.org/ns}user-visible'; + const USEREDITABLE_PROPERTYNAME = '{http://owncloud.org/ns}user-editable'; const USERASSIGNABLE_PROPERTYNAME = '{http://owncloud.org/ns}user-assignable'; const GROUPS_PROPERTYNAME = '{http://owncloud.org/ns}groups'; const CANASSIGN_PROPERTYNAME = '{http://owncloud.org/ns}can-assign'; + const WHITELISTEDINGROUP = '{http://owncloud.org/ns}editable-in-group'; /** * @var \Sabre\DAV\Server $server @@ -172,6 +174,7 @@ private function createTag($data, $contentType = 'application/json') { $tagName = $data['name']; $userVisible = true; $userAssignable = true; + $userEditable = false; if (isset($data['userVisible'])) { $userVisible = (bool)$data['userVisible']; @@ -181,6 +184,10 @@ private function createTag($data, $contentType = 'application/json') { $userAssignable = (bool)$data['userAssignable']; } + if (isset($data['userEditable'])) { + $userEditable = (bool)$data['userEditable']; + } + $groups = []; if (isset($data['groups'])) { $groups = $data['groups']; @@ -196,7 +203,7 @@ private function createTag($data, $contentType = 'application/json') { } try { - $tag = $this->tagManager->createTag($tagName, $userVisible, $userAssignable); + $tag = $this->tagManager->createTag($tagName, $userVisible, $userAssignable, $userEditable); if (!empty($groups)) { $this->tagManager->setTagGroups($tag, $groups); } @@ -232,6 +239,11 @@ public function handleGetProperties( return $node->getSystemTag()->isUserVisible() ? 'true' : 'false'; }); + $propFind->handle(self::USEREDITABLE_PROPERTYNAME, function () use ($node) { + // this is the tag's inherent property "is user editable" + return $node->getSystemTag()->isUserEditable() ? 'true' : 'false'; + }); + $propFind->handle(self::USERASSIGNABLE_PROPERTYNAME, function () use ($node) { // this is the tag's inherent property "is user assignable" return $node->getSystemTag()->isUserAssignable() ? 'true' : 'false'; @@ -249,11 +261,20 @@ public function handleGetProperties( } $groups = []; // no need to retrieve groups for namespaces that don't qualify - if ($node->getSystemTag()->isUserVisible() && !$node->getSystemTag()->isUserAssignable()) { + $restrictedTagCondition = $node->getSystemTag()->isUserVisible() && + (!$node->getSystemTag()->isUserAssignable()); + $editableTagCondition = $node->getSystemTag()->isUserVisible() && + $node->getSystemTag()->isUserAssignable() && + (!$node->getSystemTag()->isUserEditable()); + if ($restrictedTagCondition || $editableTagCondition) { $groups = $this->tagManager->getTagGroups($node->getSystemTag()); } return \implode('|', $groups); }); + + $propFind->handle(self::WHITELISTEDINGROUP, function () use ($node) { + return $this->tagManager->canUserUseStaticTagInGroup($node->getSystemTag(), $this->userSession->getUser()) ? 'true' : 'false'; + }); } /** @@ -273,12 +294,14 @@ public function handleUpdateProperties($path, PropPatch $propPatch) { $propPatch->handle([ self::DISPLAYNAME_PROPERTYNAME, self::USERVISIBLE_PROPERTYNAME, + self::USEREDITABLE_PROPERTYNAME, self::USERASSIGNABLE_PROPERTYNAME, self::GROUPS_PROPERTYNAME, ], function ($props) use ($node) { $tag = $node->getSystemTag(); $name = $tag->getName(); $userVisible = $tag->isUserVisible(); + $userEditable = $tag->isUserEditable(); $userAssignable = $tag->isUserAssignable(); $updateTag = false; @@ -294,6 +317,12 @@ public function handleUpdateProperties($path, PropPatch $propPatch) { $updateTag = true; } + if (isset($props[self::USEREDITABLE_PROPERTYNAME])) { + $propValue = $props[self::USEREDITABLE_PROPERTYNAME]; + $userEditable = ($propValue !== 'true' && $propValue !== '1'); + $updateTag = true; + } + if (isset($props[self::USERASSIGNABLE_PROPERTYNAME])) { $propValue = $props[self::USERASSIGNABLE_PROPERTYNAME]; $userAssignable = ($propValue !== 'false' && $propValue !== '0'); @@ -312,7 +341,7 @@ public function handleUpdateProperties($path, PropPatch $propPatch) { } if ($updateTag) { - $node->update($name, $userVisible, $userAssignable); + $node->update($name, $userVisible, $userAssignable, $userEditable); } return true; diff --git a/apps/dav/lib/SystemTag/SystemTagsByIdCollection.php b/apps/dav/lib/SystemTag/SystemTagsByIdCollection.php index dbe3079d3d0e..7c7104a26b9a 100644 --- a/apps/dav/lib/SystemTag/SystemTagsByIdCollection.php +++ b/apps/dav/lib/SystemTag/SystemTagsByIdCollection.php @@ -119,7 +119,7 @@ public function getChildren() { $visibilityFilter = null; } - $tags = $this->tagManager->getAllTags($visibilityFilter); + $tags = $this->tagManager->getAllTags($visibilityFilter, null); return \array_map(function ($tag) { return $this->makeNode($tag); }, $tags); diff --git a/apps/dav/tests/unit/SystemTag/SystemTagPluginTest.php b/apps/dav/tests/unit/SystemTag/SystemTagPluginTest.php index 472846b753ce..8279fa7a3c51 100644 --- a/apps/dav/tests/unit/SystemTag/SystemTagPluginTest.php +++ b/apps/dav/tests/unit/SystemTag/SystemTagPluginTest.php @@ -39,6 +39,8 @@ class SystemTagPluginTest extends \Test\TestCase { const USERASSIGNABLE_PROPERTYNAME = \OCA\DAV\SystemTag\SystemTagPlugin::USERASSIGNABLE_PROPERTYNAME; const CANASSIGN_PROPERTYNAME = \OCA\DAV\SystemTag\SystemTagPlugin::CANASSIGN_PROPERTYNAME; const GROUPS_PROPERTYNAME = \OCA\DAV\SystemTag\SystemTagPlugin::GROUPS_PROPERTYNAME; + const USEREDITABLE_PROPERTYNAME = \OCA\DAV\SystemTag\SystemTagPlugin::USEREDITABLE_PROPERTYNAME; + const WHITELISTEDINGROUP = \OCA\DAV\SystemTag\SystemTagPlugin::WHITELISTEDINGROUP; /** * @var \Sabre\DAV\Server @@ -114,7 +116,7 @@ public function getPropertiesDataProvider() { self::DISPLAYNAME_PROPERTYNAME, self::USERVISIBLE_PROPERTYNAME, self::USERASSIGNABLE_PROPERTYNAME, - self::CANASSIGN_PROPERTYNAME, + self::CANASSIGN_PROPERTYNAME ], [ self::ID_PROPERTYNAME => '1', @@ -155,16 +157,20 @@ public function getPropertiesDataProvider() { ] ], [ - new SystemTag(1, 'Test', true, true), + new SystemTag(1, 'Test', true, true, true), ['group1', 'group2'], [ self::ID_PROPERTYNAME, self::GROUPS_PROPERTYNAME, + self::USEREDITABLE_PROPERTYNAME, + self::WHITELISTEDINGROUP, ], [ self::ID_PROPERTYNAME => '1', // groups only returned when userAssignable is false self::GROUPS_PROPERTYNAME => '', + self::USEREDITABLE_PROPERTYNAME => 'true', + self::WHITELISTEDINGROUP => 'false' ] ], ]; @@ -297,6 +303,7 @@ public function testUpdatePropertiesAdmin() { $propPatch = new \Sabre\DAV\PropPatch([ self::DISPLAYNAME_PROPERTYNAME => 'Test changed', self::USERVISIBLE_PROPERTYNAME => 'false', + self::USEREDITABLE_PROPERTYNAME => 'true', self::USERASSIGNABLE_PROPERTYNAME => 'true', self::GROUPS_PROPERTYNAME => 'group1|group2', ]); @@ -315,6 +322,7 @@ public function testUpdatePropertiesAdmin() { $this->assertEquals(200, $result[self::DISPLAYNAME_PROPERTYNAME]); $this->assertEquals(200, $result[self::USERASSIGNABLE_PROPERTYNAME]); $this->assertEquals(200, $result[self::USERVISIBLE_PROPERTYNAME]); + $this->assertEquals(200, $result[self::USEREDITABLE_PROPERTYNAME]); } /** diff --git a/core/Migrations/Version20181017105216.php b/core/Migrations/Version20181017105216.php new file mode 100644 index 000000000000..f097c6afaa7f --- /dev/null +++ b/core/Migrations/Version20181017105216.php @@ -0,0 +1,43 @@ + + * + * @copyright Copyright (c) 2018, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Migrations; + +use Doctrine\DBAL\Schema\Schema; +use OCP\Migration\ISchemaMigration; + +class Version20181017105216 implements ISchemaMigration { + + /** @var string */ + private $prefix; + + public function changeSchema(Schema $schema, array $options) { + $this->prefix = $options['tablePrefix']; + $table = $schema->getTable("{$this->prefix}systemtag"); + if ($schema->hasTable("{$this->prefix}systemtag")) { + if (!$table->hasColumn('assignable')) { + $table->addColumn('assignable', 'integer'); + $assignableColumn = $table->getColumn('assignable'); + $assignableColumn->setDefault(0); + } + } + } +} diff --git a/core/Migrations/Version20181017120818.php b/core/Migrations/Version20181017120818.php new file mode 100644 index 000000000000..c7b2cab16682 --- /dev/null +++ b/core/Migrations/Version20181017120818.php @@ -0,0 +1,55 @@ + + * + * @copyright Copyright (c) 2018, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Migrations; + +use OCP\IDBConnection; +use OCP\Migration\ISqlMigration; + +/** + * Added extra column assignable to systemtag table, post 10.0.10.1 version + * this is required to identify editable tag. It is only + * set to "1" for visible and editable tags. + */ +class Version20181017120818 implements ISqlMigration { + public function sql(IDBConnection $connection) { + $valueToUpdate = 1; + $qb = $connection->getQueryBuilder(); + $qb->update('systemtag') + ->set( + 'assignable', + $qb->expr()->literal($valueToUpdate) + ) + ->where( + $qb->expr()->eq( + 'visibility', + $qb->expr()->literal($valueToUpdate) + ) + ) + ->andWhere( + $qb->expr()->eq( + 'editable', + $qb->expr()->literal($valueToUpdate) + ) + ); + return [$qb->getSQL()]; + } +} diff --git a/core/js/systemtags/systemtagmodel.js b/core/js/systemtags/systemtagmodel.js index 72450a2abd04..864eaf436144 100644 --- a/core/js/systemtags/systemtagmodel.js +++ b/core/js/systemtags/systemtagmodel.js @@ -15,7 +15,9 @@ PROPERTY_CAN_ASSIGN:'{' + OC.Files.Client.NS_OWNCLOUD + '}can-assign', PROPERTY_DISPLAYNAME: '{' + OC.Files.Client.NS_OWNCLOUD + '}display-name', PROPERTY_USERVISIBLE: '{' + OC.Files.Client.NS_OWNCLOUD + '}user-visible', + PROPERTY_USEREDITABLE:'{' + OC.Files.Client.NS_OWNCLOUD + '}user-editable', PROPERTY_USERASSIGNABLE:'{' + OC.Files.Client.NS_OWNCLOUD + '}user-assignable', + PROPERTY_WHITELISTEDINGROUP:'{' + OC.Files.Client.NS_OWNCLOUD + '}editable-in-group' }); /** @@ -39,18 +41,29 @@ 'id': OC.Files.Client.PROPERTY_FILEID, 'name': OC.Files.Client.PROPERTY_DISPLAYNAME, 'userVisible': OC.Files.Client.PROPERTY_USERVISIBLE, + 'userEditable': OC.Files.Client.PROPERTY_USEREDITABLE, 'userAssignable': OC.Files.Client.PROPERTY_USERASSIGNABLE, + 'editableInGroup': OC.Files.Client.PROPERTY_WHITELISTEDINGROUP, // read-only, effective permissions computed by the server, 'canAssign': OC.Files.Client.PROPERTY_CAN_ASSIGN }, parse: function(data) { + var userEditable; + //If assignable is false, just set editable to false + if (data.userAssignable === false) { + userEditable = false; + } else { + userEditable = (data.userEditable === true || data.userEditable === 'true'); + } return { id: data.id, name: data.name, userVisible: data.userVisible === true || data.userVisible === 'true', + userEditable: userEditable, userAssignable: data.userAssignable === true || data.userAssignable === 'true', - canAssign: data.canAssign === true || data.canAssign === 'true' + canAssign: data.canAssign === true || data.canAssign === 'true', + editableInGroup: data.editableInGroup === true || data.editableInGroup === 'true' }; } }); diff --git a/core/js/systemtags/systemtags.js b/core/js/systemtags/systemtags.js index 05ead6f3dcd4..4ea7f4c4ad5f 100644 --- a/core/js/systemtags/systemtags.js +++ b/core/js/systemtags/systemtags.js @@ -42,6 +42,13 @@ // invisible also implicitly means not assignable scope = t('core', 'invisible'); } + if (tag.userVisible === true && tag.userEditable === false && tag.userAssignable === true) { + /** + * Users can edit the tag, if they are admin or belong to whitelisted + * group by the edit tag. + */ + scope = t('core', 'Static') + } if (scope) { var $tag = $('').text(' ' + t('core', '({scope})', { diff --git a/core/js/systemtags/systemtagsinputfield.js b/core/js/systemtags/systemtagsinputfield.js index 2eb8d0a44cb2..98c4fc03ad6d 100644 --- a/core/js/systemtags/systemtagsinputfield.js +++ b/core/js/systemtags/systemtagsinputfield.js @@ -207,6 +207,7 @@ name: e.object.name.trim(), userVisible: true, userAssignable: true, + userEditable: true, canAssign: true }, { success: function(model) { @@ -278,6 +279,16 @@ e.stopPropagation(); }, + /** + * Returns true if tag is static tag else false + * + * @param data + * @returns {boolean} + */ + _isStaticTag: function(data) { + return data.userEditable === false && data.userAssignable === true; + }, + /** * Formats a single dropdown result * @@ -288,6 +299,25 @@ if (!this._resultTemplate) { this._resultTemplate = Handlebars.compile(RESULT_TEMPLATE); } + + /** + * Static tags are shown if the user belongs to group which is + * whitelisted in the tag. Else the tag is not seen. If the tag + * is visible, then no edit options are availbe to users. Admin user + * is the only exception here. Admin user can edit, delete, assign or + * unassign the tag. + * + */ + this._allowActions = true; + if (data.editableInGroup === false && this._isStaticTag(data)) { + //No need to show the static tag as it is not viewable for the user + return; + } + if (this._isStaticTag(data)) { + //Show the name of the static tag, rename and delete actions are forbidden for the user + this._allowActions = false; + } + return this._resultTemplate(_.extend({ renameTooltip: t('core', 'Rename'), allowActions: this._allowActions, diff --git a/core/js/tests/specs/systemtags/systemtagsinputfieldSpec.js b/core/js/tests/specs/systemtags/systemtagsinputfieldSpec.js index ae9b4ddb71d9..5c2d74b5c994 100644 --- a/core/js/tests/specs/systemtags/systemtagsinputfieldSpec.js +++ b/core/js/tests/specs/systemtags/systemtagsinputfieldSpec.js @@ -113,6 +113,7 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() { name: 'newname', userVisible: true, userAssignable: true, + userEditable: true, canAssign: true }); @@ -191,6 +192,7 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() { name: 'newname', userVisible: true, userAssignable: true, + userEditable: true, canAssign: true }); @@ -369,6 +371,7 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() { new OC.SystemTags.SystemTagModel({id: '2', name: 'def'}), new OC.SystemTags.SystemTagModel({id: '3', name: 'abd', userAssignable: false, canAssign: false}), new OC.SystemTags.SystemTagModel({id: '4', name: 'Deg'}), + new OC.SystemTags.SystemTagModel({id: '5', name: 'staticTag', editableInGroup: true, canAssign: true, userAssignable: true, userEditable: false, userVisible: true}) ]); }); afterEach(function() { @@ -402,6 +405,28 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() { } ]); }); + it('check static tag result', function () { + var callback = sinon.stub(); + opts.query({ + term: 'sta', + callback: callback + }); + expect(fetchStub.calledOnce).toEqual(true); + + fetchStub.yieldTo('success', view.collection); + + expect(callback.getCall(0).args[0].results).toEqual([ + { + id: '5', + name: 'staticTag', + editableInGroup: true, + canAssign: true, + userAssignable: true, + userEditable: false, + userVisible: true + } + ]); + }); it('completes case insensitive', function() { var callback = sinon.stub(); opts.query({ @@ -451,6 +476,30 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() { var $el = $(opts.formatSelection({id: '1', name: 'test'})); expect($el.text().trim()).toEqual('test'); }); + it('formatSelection renders visibleTag', function () { + var opts = select2Stub.getCall(0).args[0]; + var $el = $(opts.formatResult({id: '1', name: 'visibleTag', userAssignable: true, userEditable: true, userVisible: true, canAssign: true, editableInGroup: false})); + expect($el.find('.label').text()).toEqual('visibleTag'); + expect($el.find('.rename').length).toEqual(1); + }); + it('formatSelection renders restrictedTag', function () { + var opts = select2Stub.getCall(0).args[0]; + var $el = $(opts.formatResult({id: '1', name: 'restrictTag', userAssignable: false, userEditable: false, userVisible: true, canAssign: true, editableInGroup: true})); + expect($el.find('.label').text()).toEqual('restrictTag'); + expect($el.find('.rename').length).toEqual(1); + }); + it('formatSelection renders staticTag', function () { + var opts = select2Stub.getCall(0).args[0]; + var $el = $(opts.formatResult({id: '1', name: 'staticTag', userAssignable: true, userEditable: false, userVisible: true, canAssign: true, editableInGroup: true})); + expect($el.find('.label').text()).toEqual('staticTag'); + expect($el.find('.rename').length).toEqual(0); + }); + it('formatSelection renders staticTag should not showup', function () { + var opts = select2Stub.getCall(0).args[0]; + var $el = $(opts.formatResult({id: '1', name: 'staticTag', userAssignable: true, userEditable: false, userVisible: true, canAssign: true, editableInGroup: false})); + expect($el.find('.label').text()).toEqual(''); + expect($el.find('.rename').length).toEqual(0); + }); describe('initSelection', function() { var fetchStub; var testTags; diff --git a/db_structure.xml b/db_structure.xml index 3043561f040b..e65060897583 100644 --- a/db_structure.xml +++ b/db_structure.xml @@ -1085,6 +1085,15 @@ 1 + + + assignable + integer + 1 + true + 1 + + tag_ident true @@ -1100,6 +1109,10 @@ editable ascending + + assignable + ascending + diff --git a/lib/private/SystemTag/SystemTag.php b/lib/private/SystemTag/SystemTag.php index 493a982387f5..d1c4acf6b4dd 100644 --- a/lib/private/SystemTag/SystemTag.php +++ b/lib/private/SystemTag/SystemTag.php @@ -41,6 +41,11 @@ class SystemTag implements ISystemTag { */ private $userVisible; + /** + * @var bool + */ + private $userEditable; + /** * @var bool */ @@ -53,11 +58,13 @@ class SystemTag implements ISystemTag { * @param string $name tag name * @param bool $userVisible whether the tag is user visible * @param bool $userAssignable whether the tag is user assignable + * @param bool $userEditable whether the tag is user assignable */ - public function __construct($id, $name, $userVisible, $userAssignable) { + public function __construct($id, $name, $userVisible, $userAssignable, $userEditable = false) { $this->id = $id; $this->name = $name; $this->userVisible = $userVisible; + $this->userEditable = $userEditable; $this->userAssignable = $userAssignable; } @@ -88,4 +95,11 @@ public function isUserVisible() { public function isUserAssignable() { return $this->userAssignable; } + + /** + * {@inheritdoc} + */ + public function isUserEditable() { + return $this->userEditable; + } } diff --git a/lib/private/SystemTag/SystemTagManager.php b/lib/private/SystemTag/SystemTagManager.php index d2887e3ba899..1f1dbff88a73 100644 --- a/lib/private/SystemTag/SystemTagManager.php +++ b/lib/private/SystemTag/SystemTagManager.php @@ -77,7 +77,8 @@ public function __construct( ->from(self::TAG_TABLE) ->where($query->expr()->eq('name', $query->createParameter('name'))) ->andWhere($query->expr()->eq('visibility', $query->createParameter('visibility'))) - ->andWhere($query->expr()->eq('editable', $query->createParameter('editable'))); + ->andWhere($query->expr()->eq('editable', $query->createParameter('editable'))) + ->andWhere($query->expr()->eq('assignable', $query->createParameter('assignable'))); } /** @@ -104,6 +105,7 @@ public function getTagsByIds($tagIds) { ->addOrderBy('name', 'ASC') ->addOrderBy('visibility', 'ASC') ->addOrderBy('editable', 'ASC') + ->addOrderBy('assignable', 'ASC') ->setParameter('tagids', $tagIds, IQueryBuilder::PARAM_INT_ARRAY); $result = $query->execute(); @@ -148,7 +150,8 @@ public function getAllTags($visibilityFilter = null, $nameSearchPattern = null) $query ->addOrderBy('name', 'ASC') ->addOrderBy('visibility', 'ASC') - ->addOrderBy('editable', 'ASC'); + ->addOrderBy('editable', 'ASC') + ->addOrderBy('assignable', 'ASC'); $result = $query->execute(); while ($row = $result->fetch()) { @@ -163,14 +166,16 @@ public function getAllTags($visibilityFilter = null, $nameSearchPattern = null) /** * {@inheritdoc} */ - public function getTag($tagName, $userVisible, $userAssignable) { + public function getTag($tagName, $userVisible, $userAssignable, $userEditable = false) { $userVisible = (int)$userVisible; $userAssignable = (int)$userAssignable; + $userEditable = (int)$userEditable; $result = $this->selectTagQuery ->setParameter('name', $tagName) ->setParameter('visibility', $userVisible) ->setParameter('editable', $userAssignable) + ->setParameter('assignable', $userEditable) ->execute(); $row = $result->fetch(); @@ -187,16 +192,25 @@ public function getTag($tagName, $userVisible, $userAssignable) { /** * {@inheritdoc} */ - public function createTag($tagName, $userVisible, $userAssignable) { + public function createTag($tagName, $userVisible, $userAssignable, $userEditable = false) { $userVisible = (int)$userVisible; $userAssignable = (int)$userAssignable; + $userEditable = (int)$userEditable; + + if ($userEditable === 1) { + $editable = $userAssignable; + } else { + $editable = 0; + $userAssignable = 1; + } $query = $this->connection->getQueryBuilder(); $query->insert(self::TAG_TABLE) ->values([ 'name' => $query->createNamedParameter($tagName), 'visibility' => $query->createNamedParameter($userVisible), - 'editable' => $query->createNamedParameter($userAssignable), + 'editable' => $query->createNamedParameter($editable), + 'assignable' => $query->createNamedParameter($userAssignable) ]); try { @@ -215,7 +229,8 @@ public function createTag($tagName, $userVisible, $userAssignable) { (int)$tagId, $tagName, (bool)$userVisible, - (bool)$userAssignable + (bool)$userAssignable, + (bool)$editable ); $this->dispatcher->dispatch(ManagerEvent::EVENT_CREATE, new ManagerEvent( @@ -228,9 +243,10 @@ public function createTag($tagName, $userVisible, $userAssignable) { /** * {@inheritdoc} */ - public function updateTag($tagId, $tagName, $userVisible, $userAssignable) { + public function updateTag($tagId, $tagName, $userVisible, $userAssignable, $userEditable = false) { $userVisible = (int)$userVisible; $userAssignable = (int)$userAssignable; + $userEditable = (int)$userEditable; try { $tags = $this->getTagsByIds($tagId); @@ -245,7 +261,8 @@ public function updateTag($tagId, $tagName, $userVisible, $userAssignable) { (int) $tagId, $tagName, (bool) $userVisible, - (bool) $userAssignable + (bool) $userAssignable, + (bool) $userEditable ); $query = $this->connection->getQueryBuilder(); @@ -253,10 +270,12 @@ public function updateTag($tagId, $tagName, $userVisible, $userAssignable) { ->set('name', $query->createParameter('name')) ->set('visibility', $query->createParameter('visibility')) ->set('editable', $query->createParameter('editable')) + ->set('assignable', $query->createParameter('assignable')) ->where($query->expr()->eq('id', $query->createParameter('tagid'))) ->setParameter('name', $tagName) ->setParameter('visibility', $userVisible) - ->setParameter('editable', $userAssignable) + ->setParameter('editable', $userEditable) + ->setParameter('assignable', $userAssignable) ->setParameter('tagid', $tagId); try { @@ -374,7 +393,7 @@ public function canUserSeeTag(ISystemTag $tag, IUser $user) { } private function createSystemTagFromRow($row) { - return new SystemTag((int)$row['id'], $row['name'], (bool)$row['visibility'], (bool)$row['editable']); + return new SystemTag((int)$row['id'], $row['name'], (bool)$row['visibility'], (bool)$row['assignable'], (bool)$row['editable']); } /** @@ -431,4 +450,23 @@ public function getTagGroups(ISystemTag $tag) { return $groupIds; } + + /** + * {@inheritdoc} + */ + public function canUserUseStaticTagInGroup(ISystemTag $tag, IUser $user) { + if ($this->groupManager->isAdmin($user->getUID())) { + return true; + } + if ($tag->isUserEditable() === false) { + $groupIds = $this->groupManager->getUserGroupIds($user); + if (!empty($groupIds)) { + $matchingGroups = \array_intersect($groupIds, $this->getTagGroups($tag)); + if (!empty($matchingGroups)) { + return true; + } + } + } + return false; + } } diff --git a/lib/public/SystemTag/ISystemTag.php b/lib/public/SystemTag/ISystemTag.php index 7ccf97235229..a25fc1fb49ce 100644 --- a/lib/public/SystemTag/ISystemTag.php +++ b/lib/public/SystemTag/ISystemTag.php @@ -63,4 +63,13 @@ public function isUserVisible(); * @since 9.0.0 */ public function isUserAssignable(); + + /** + * Returns whether the tag can be assigned to objects by regular users + * + * @return bool true if editable, false otherwise + * + * @since 10.0.11 + */ + public function isUserEditable(); } diff --git a/lib/public/SystemTag/ISystemTagManager.php b/lib/public/SystemTag/ISystemTagManager.php index 7bcf185a0f07..92960661285c 100644 --- a/lib/public/SystemTag/ISystemTagManager.php +++ b/lib/public/SystemTag/ISystemTagManager.php @@ -59,7 +59,7 @@ public function getTagsByIds($tagIds); * * @since 9.0.0 */ - public function getTag($tagName, $userVisible, $userAssignable); + public function getTag($tagName, $userVisible, $userAssignable, $userEditable = false); /** * Creates the tag object using the given attributes. @@ -67,6 +67,7 @@ public function getTag($tagName, $userVisible, $userAssignable); * @param string $tagName tag name * @param bool $userVisible whether the tag is visible by users * @param bool $userAssignable whether the tag is assignable by users + * @param bool $userEditable whether the tag is editable by users * * @return \OCP\SystemTag\ISystemTag system tag * @@ -74,7 +75,7 @@ public function getTag($tagName, $userVisible, $userAssignable); * * @since 9.0.0 */ - public function createTag($tagName, $userVisible, $userAssignable); + public function createTag($tagName, $userVisible, $userAssignable, $userEditable = false); /** * Returns all known tags, optionally filtered by visibility. @@ -95,6 +96,7 @@ public function getAllTags($visibilityFilter = null, $nameSearchPattern = null); * @param string $newName the new tag name * @param bool $userVisible whether the tag is visible by users * @param bool $userAssignable whether the tag is assignable by users + * @param bool $userEditable whether the tag is assignable by users * * @throws \OCP\SystemTag\TagNotFoundException if tag with the given id does not exist * @throws \OCP\SystemTag\TagAlreadyExistsException if there is already another tag @@ -102,7 +104,7 @@ public function getAllTags($visibilityFilter = null, $nameSearchPattern = null); * * @since 9.0.0 */ - public function updateTag($tagId, $newName, $userVisible, $userAssignable); + public function updateTag($tagId, $newName, $userVisible, $userAssignable, $userEditable = false); /** * Delete the given tags from the database and all their relationships. @@ -160,4 +162,14 @@ public function setTagGroups(ISystemTag $tag, $groupIds); * @since 9.1.0 */ public function getTagGroups(ISystemTag $tag); + + /** + * Verify if users of group can use static tags + * + * @param ISystemTag $tag + * @param IUser $user + * @return bool, true if user of group can use staic tags, else false + * @since 10.0.11 + */ + public function canUserUseStaticTagInGroup(ISystemTag $tag, Iuser $user); } diff --git a/tests/TestHelpers/TagsHelper.php b/tests/TestHelpers/TagsHelper.php index 16cd105eebf6..058e49838cfa 100644 --- a/tests/TestHelpers/TagsHelper.php +++ b/tests/TestHelpers/TagsHelper.php @@ -164,6 +164,7 @@ public static function createTag( $name, $userVisible = true, $userAssignable = true, + $userEditable = false, $groups = null, $davPathVersionToUse = 2 ) { @@ -172,6 +173,7 @@ public static function createTag( 'name' => $name, 'userVisible' => $userVisible, 'userAssignable' => $userAssignable, + 'userEditable' => $userEditable ]; if ($groups !== null) { @@ -225,21 +227,22 @@ public static function deleteTag( * @return boolean[] */ public static function validateTypeOfTag($type) { - $userVisible = true; - $userAssignable = true; + $userVisible = "1"; + $userAssignable = "1"; + $userEditable = "1"; switch ($type) { case 'normal': break; case 'not user-assignable': - $userAssignable = false; + $userAssignable = "0"; break; case 'not user-visible': - $userVisible = false; + $userVisible = "0"; break; default: throw new \Exception('Unsupported type'); } - return [$userVisible, $userAssignable]; + return [$userVisible, $userAssignable, $userEditable]; } } diff --git a/tests/acceptance/features/bootstrap/Tags.php b/tests/acceptance/features/bootstrap/Tags.php index 3f6bfcf1e964..c337675e3a8a 100644 --- a/tests/acceptance/features/bootstrap/Tags.php +++ b/tests/acceptance/features/bootstrap/Tags.php @@ -40,19 +40,20 @@ trait Tags { * @param string $user * @param bool $userVisible * @param bool $userAssignable + * @param bool$userEditable * @param string $name * @param string $groups * * @return void */ private function createTag( - $user, $userVisible, $userAssignable, $name, $groups = null + $user, $userVisible, $userAssignable, $userEditable, $name, $groups = null ) { $this->response = TagsHelper::createTag( $this->getBaseUrl(), $this->getActualUsername($user), $this->getPasswordForUser($user), - $name, $userVisible, $userAssignable, $groups, + $name, $userVisible, $userAssignable, $userEditable, $groups, $this->getDavPathVersion('systemtags') ); $responseHeaders = $this->response->getHeaders(); @@ -153,6 +154,7 @@ public function createsATagWithName($user, $type, $name) { $user, TagsHelper::validateTypeOfTag($type)[0], TagsHelper::validateTypeOfTag($type)[1], + TagsHelper::validateTypeOfTag($type)[2], $name ); } @@ -204,6 +206,7 @@ public function createsATagWithNameAndGroups($user, $type, $name, $groups) { $user, TagsHelper::validateTypeOfTag($type)[0], TagsHelper::validateTypeOfTag($type)[1], + TagsHelper::validateTypeOfTag($type)[2], $name, $groups ); diff --git a/tests/lib/SystemTag/SystemTagManagerTest.php b/tests/lib/SystemTag/SystemTagManagerTest.php index 441112d5d947..cb5b0f771a14 100644 --- a/tests/lib/SystemTag/SystemTagManagerTest.php +++ b/tests/lib/SystemTag/SystemTagManagerTest.php @@ -14,6 +14,7 @@ use OC\SystemTag\SystemTagObjectMapper; use OCP\IDBConnection; use OCP\IGroupManager; +use OCP\IUser; use OCP\SystemTag\ISystemTag; use OCP\SystemTag\ISystemTagManager; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -85,19 +86,19 @@ public function getAllTagsDataProvider() { [ // simple [ - ['one', false, false], - ['two', false, false], + ['one', false, false, false], + ['two', false, false, false], ] ], [ // duplicate names, different flags [ - ['one', false, false], - ['one', true, false], - ['one', false, true], - ['one', true, true], - ['two', false, false], - ['two', false, true], + ['one', false, false, false], + ['one', true, false, false], + ['one', false, true, true], + ['one', true, true, true], + ['two', false, false, true], + ['two', false, true, false], ] ] ]; @@ -109,7 +110,7 @@ public function getAllTagsDataProvider() { public function testGetAllTags($testTags) { $testTagsById = []; foreach ($testTags as $testTag) { - $tag = $this->tagManager->createTag($testTag[0], $testTag[1], $testTag[2]); + $tag = $this->tagManager->createTag($testTag[0], $testTag[1], $testTag[2], $testTag[3]); $testTagsById[$tag->getId()] = $tag; } @@ -137,8 +138,8 @@ public function getAllTagsFilteredDataProvider() { [ // none visible [ - ['one', false, false], - ['two', false, false], + ['one', false, false, false], + ['two', false, false, false], ], true, null, @@ -147,66 +148,66 @@ public function getAllTagsFilteredDataProvider() { [ // one visible [ - ['one', true, false], - ['two', false, false], + ['one', true, false, true], + ['two', false, false, true], ], true, null, [ - ['one', true, false], + ['one', true, false, true], ] ], [ // one invisible [ - ['one', true, false], - ['two', false, false], + ['one', true, false, true], + ['two', false, false, true], ], false, null, [ - ['two', false, false], + ['two', false, false, true], ] ], // filter by name pattern [ [ - ['one', true, false], - ['one', false, false], - ['two', true, false], + ['one', true, false, false], + ['one', false, false, false], + ['two', true, false, false], ], null, 'on', [ - ['one', true, false], - ['one', false, false], + ['one', true, false, true], + ['one', false, false, true], ] ], // filter by name pattern and visibility [ // one visible [ - ['one', true, false], - ['two', true, false], - ['one', false, false], + ['one', true, false, false], + ['two', true, false, false], + ['one', false, false, false], ], true, 'on', [ - ['one', true, false], + ['one', true, false, true], ] ], // filter by name pattern in the middle [ // one visible [ - ['abcdefghi', true, false], - ['two', true, false], + ['abcdefghi', true, false, false], + ['two', true, false, false], ], null, 'def', [ - ['abcdefghi', true, false], + ['abcdefghi', true, false, true], ] ] ]; @@ -222,7 +223,7 @@ public function testGetAllTagsFiltered($testTags, $visibilityFilter, $nameSearch $testTagsById = []; foreach ($expectedResults as $expectedTag) { - $tag = $this->tagManager->getTag($expectedTag[0], $expectedTag[1], $expectedTag[2]); + $tag = $this->tagManager->getTag($expectedTag[0], $expectedTag[1], $expectedTag[2], $expectedTag[3]); $testTagsById[$tag->getId()] = $tag; } @@ -238,10 +239,10 @@ public function testGetAllTagsFiltered($testTags, $visibilityFilter, $nameSearch public function oneTagMultipleFlagsProvider() { return [ - ['one', false, false], - ['one', true, false], - ['one', false, true], - ['one', true, true], + ['one', false, false, false], + ['one', true, false, false], + ['one', false, true, true], + ['one', true, true, true], ]; } @@ -261,9 +262,12 @@ public function testCreateDuplicate($name, $userVisible, $userAssignable) { /** * @dataProvider oneTagMultipleFlagsProvider */ - public function testGetExistingTag($name, $userVisible, $userAssignable) { - $tag1 = $this->tagManager->createTag($name, $userVisible, $userAssignable); - $tag2 = $this->tagManager->getTag($name, $userVisible, $userAssignable); + public function testGetExistingTag($name, $userVisible, $userAssignable, $userEditable) { + $tag1 = $this->tagManager->createTag($name, $userVisible, $userAssignable, $userEditable); + if ($userEditable === false) { + $userEditable = true; + } + $tag2 = $this->tagManager->getTag($name, $userVisible, $userAssignable, $userEditable); $this->assertSameTag($tag1, $tag2); } @@ -307,23 +311,23 @@ public function updateTagProvider() { return [ [ // update name - ['one', true, true], - ['two', true, true] + ['one', true, true, true], + ['two', true, true, true] ], [ // update one flag - ['one', false, true], - ['one', true, true] + ['one', false, true, true], + ['one', true, true, true] ], [ // update all flags - ['one', false, false], - ['one', true, true] + ['one', false, false, false], + ['one', true, true, true] ], [ // update all - ['one', false, false], - ['two', true, true] + ['one', false, false, false], + ['two', true, true, true] ], ]; } @@ -335,24 +339,28 @@ public function testUpdateTag($tagCreate, $tagUpdated) { $tag1 = $this->tagManager->createTag( $tagCreate[0], $tagCreate[1], - $tagCreate[2] + $tagCreate[2], + $tagCreate[3] ); $this->tagManager->updateTag( $tag1->getId(), $tagUpdated[0], $tagUpdated[1], - $tagUpdated[2] + $tagUpdated[2], + $tagUpdated[3] ); $tag2 = $this->tagManager->getTag( $tagUpdated[0], $tagUpdated[1], - $tagUpdated[2] + $tagUpdated[2], + $tagUpdated[3] ); $this->assertEquals($tag2->getId(), $tag1->getId()); $this->assertEquals($tag2->getName(), $tagUpdated[0]); $this->assertEquals($tag2->isUserVisible(), $tagUpdated[1]); $this->assertEquals($tag2->isUserAssignable(), $tagUpdated[2]); + $this->assertEquals($tag2->isUserEditable(), $tagUpdated[3]); } /** @@ -363,19 +371,25 @@ public function testUpdateTagDuplicate($tagCreate, $tagUpdated) { $this->tagManager->createTag( $tagCreate[0], $tagCreate[1], - $tagCreate[2] + $tagCreate[2], + $tagCreate[3] ); $tag2 = $this->tagManager->createTag( $tagUpdated[0], $tagUpdated[1], - $tagUpdated[2] + $tagUpdated[2], + $tagUpdated[3] ); + if ($tagCreate[3] === false) { + $tagCreate[3] = true; + } // update to match the first tag $this->tagManager->updateTag( $tag2->getId(), $tagCreate[0], $tagCreate[1], + $tagCreate[3], $tagCreate[2] ); } @@ -449,41 +463,41 @@ public function testVisibilityCheck($userVisible, $userAssignable, $isAdmin, $ex public function assignabilityCheckProvider() { return [ // no groups - [false, false, false, false], - [true, false, false, false], - [true, true, false, true], - [false, true, false, false], + [false, false, false, false, false], + [true, false, true, false, false], + [true, true, true, false, true], + [false, true, true, false, false], // admin rulez - [false, false, true, true], - [false, true, true, true], - [true, false, true, true], - [true, true, true, true], + [false, false, true, true, true], + [false, true, true, true, true], + [true, false, true, true, true], + [true, true, true, true, true], // ignored groups - [false, false, false, false, ['group1'], ['group1']], - [true, true, false, true, ['group1'], ['group1']], - [true, true, false, true, ['group1'], ['anothergroup']], - [false, true, false, false, ['group1'], ['group1']], + [false, false, true, false, false, ['group1'], ['group1']], + [true, true, true, false, true, ['group1'], ['group1']], + [true, true, false, false, true, ['group1'], ['anothergroup']], + [false, true, true, false, false, ['group1'], ['group1']], // admin has precedence over groups - [false, false, true, true, ['group1'], ['anothergroup']], - [false, true, true, true, ['group1'], ['anothergroup']], - [true, false, true, true, ['group1'], ['anothergroup']], - [true, true, true, true, ['group1'], ['anothergroup']], + [false, false, true, true, true, ['group1'], ['anothergroup']], + [false, true, true, true, true, ['group1'], ['anothergroup']], + [true, false, true, true, true, ['group1'], ['anothergroup']], + [true, true, true, true, true, ['group1'], ['anothergroup']], // groups only checked when visible and user non-assignable and non-admin - [true, false, false, false, ['group1'], ['anothergroup1']], - [true, false, false, true, ['group1'], ['group1']], - [true, false, false, true, ['group1', 'group2'], ['group2', 'group3']], + [true, false, true, false, false, ['group1'], ['anothergroup1']], + [true, false, true, false, true, ['group1'], ['group1']], + [true, false, true, false, true, ['group1', 'group2'], ['group2', 'group3']], ]; } /** * @dataProvider assignabilityCheckProvider */ - public function testAssignabilityCheck($userVisible, $userAssignable, $isAdmin, $expectedResult, $userGroupIds = [], $tagGroupIds = []) { + public function testAssignabilityCheck($userVisible, $userAssignable, $userEditable, $isAdmin, $expectedResult, $userGroupIds = [], $tagGroupIds = []) { $user = $this->getMockBuilder('\OCP\IUser')->getMock(); $user->expects($this->any()) ->method('getUID') ->will($this->returnValue('test')); - $tag1 = $this->tagManager->createTag('one', $userVisible, $userAssignable); + $tag1 = $this->tagManager->createTag('one', $userVisible, $userAssignable, $userEditable); $this->tagManager->setTagGroups($tag1, $tagGroupIds); $this->groupManager->expects($this->any()) @@ -534,4 +548,36 @@ private function assertSameTag($tag1, $tag2) { $this->assertEquals($tag1->isUserVisible(), $tag2->isUserVisible()); $this->assertEquals($tag1->isUserAssignable(), $tag2->isUserAssignable()); } + + public function provideCanUserUseStaticTagInGroup() { + return [ + [['group1'], true, ['group1', 'group2'], true], + [['group1'], false, ['group1', 'group2'], true], + [['group1'], false, ['group2', 'group3'], false] + ]; + } + + /** + * @param $userGroups + * @param $isAdmin + * @param $tagGroups + * @param $expectedResult + * @dataProvider provideCanUserUseStaticTagInGroup + */ + public function testCanUserUseStaticTagInGroup($userGroups, $isAdmin, $tagGroups, $expectedResult) { + $tag1 = $this->tagManager->createTag('tag1', true, false, true); + if (!empty($tagGroups)) { + $this->tagManager->setTagGroups($tag1, $tagGroups); + } + $user = $this->createMock(IUser::class); + $this->groupManager + ->method('isAdmin') + ->willReturn($isAdmin); + $this->groupManager + ->method('getUserGroupIds') + ->with($user) + ->willReturn($userGroups); + $result = $this->tagManager->canUserUseStaticTagInGroup($tag1, $user); + $this->assertEquals($result, $expectedResult); + } }