From dd27c3c5a2534cccd28a166e89bf27d56e36015a Mon Sep 17 00:00:00 2001 From: Sujith H Date: Fri, 12 Oct 2018 11:58:44 +0530 Subject: [PATCH] [stable10] Backport of new tag editable added to systemtag Added new tag editable to the systemtag. This tag could be editable only to admins and to the groups who have permission to edit. For all other users, this tag could be used to assign/unassign. Signed-off-by: Sujith H --- apps/dav/lib/SystemTag/SystemTagNode.php | 5 +- apps/dav/lib/SystemTag/SystemTagPlugin.php | 35 +++- .../SystemTag/SystemTagsByIdCollection.php | 2 +- .../unit/SystemTag/SystemTagPluginTest.php | 12 +- core/Migrations/Version20181017105216.php | 43 ++++ core/Migrations/Version20181017120818.php | 55 +++++ core/js/systemtags/systemtagmodel.js | 15 +- core/js/systemtags/systemtags.js | 7 + core/js/systemtags/systemtagsinputfield.js | 30 +++ .../systemtags/systemtagsinputfieldSpec.js | 49 +++++ db_structure.xml | 13 ++ lib/private/SystemTag/SystemTag.php | 16 +- lib/private/SystemTag/SystemTagManager.php | 58 +++++- lib/public/SystemTag/ISystemTag.php | 9 + lib/public/SystemTag/ISystemTagManager.php | 18 +- tests/TestHelpers/TagsHelper.php | 13 +- tests/acceptance/features/bootstrap/Tags.php | 7 +- tests/lib/SystemTag/SystemTagManagerTest.php | 188 +++++++++++------- 18 files changed, 474 insertions(+), 101 deletions(-) create mode 100644 core/Migrations/Version20181017105216.php create mode 100644 core/Migrations/Version20181017120818.php 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); + } }