summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--apps/dav/lib/SystemTag/SystemTagNode.php7
-rw-r--r--apps/dav/tests/unit/SystemTag/SystemTagNodeTest.php42
-rw-r--r--build/integration/features/tags.feature13
-rw-r--r--core/js/systemtags/systemtagsinputfield.js7
-rw-r--r--core/js/tests/specs/systemtags/systemtagsinputfieldSpec.js87
5 files changed, 103 insertions, 53 deletions
diff --git a/apps/dav/lib/SystemTag/SystemTagNode.php b/apps/dav/lib/SystemTag/SystemTagNode.php
index 36fddcd8240..bd21082f783 100644
--- a/apps/dav/lib/SystemTag/SystemTagNode.php
+++ b/apps/dav/lib/SystemTag/SystemTagNode.php
@@ -157,12 +157,13 @@ class SystemTagNode implements \Sabre\DAV\INode {
public function delete() {
try {
+ if (!$this->isAdmin) {
+ throw new Forbidden('No permission to delete tag ' . $this->tag->getId());
+ }
+
if (!$this->tagManager->canUserSeeTag($this->tag, $this->user)) {
throw new NotFound('Tag with id ' . $this->tag->getId() . ' not found');
}
- if (!$this->tagManager->canUserAssignTag($this->tag, $this->user)) {
- throw new Forbidden('No permission to delete tag ' . $this->tag->getId());
- }
$this->tagManager->deleteTags($this->tag->getId());
} catch (TagNotFoundException $e) {
diff --git a/apps/dav/tests/unit/SystemTag/SystemTagNodeTest.php b/apps/dav/tests/unit/SystemTag/SystemTagNodeTest.php
index 43674f4b795..3722bd9d25a 100644
--- a/apps/dav/tests/unit/SystemTag/SystemTagNodeTest.php
+++ b/apps/dav/tests/unit/SystemTag/SystemTagNodeTest.php
@@ -24,19 +24,17 @@
namespace OCA\DAV\Tests\unit\SystemTag;
-use Sabre\DAV\Exception\NotFound;
-use Sabre\DAV\Exception\MethodNotAllowed;
-use Sabre\DAV\Exception\Conflict;
use OC\SystemTag\SystemTag;
use OCP\SystemTag\TagNotFoundException;
use OCP\SystemTag\TagAlreadyExistsException;
use OCP\SystemTag\ISystemTag;
+use Sabre\DAV\Exception\Forbidden;
class SystemTagNodeTest extends \Test\TestCase {
/**
- * @var \OCP\SystemTag\ISystemTagManager
+ * @var \OCP\SystemTag\ISystemTagManager|\PHPUnit_Framework_MockObject_MockObject
*/
private $tagManager;
@@ -113,7 +111,7 @@ class SystemTagNodeTest extends \Test\TestCase {
/**
* @dataProvider tagNodeProvider
*/
- public function testUpdateTag($isAdmin, $originalTag, $changedArgs) {
+ public function testUpdateTag($isAdmin, ISystemTag $originalTag, $changedArgs) {
$this->tagManager->expects($this->once())
->method('canUserSeeTag')
->with($originalTag)
@@ -173,7 +171,7 @@ class SystemTagNodeTest extends \Test\TestCase {
/**
* @dataProvider tagNodeProviderPermissionException
*/
- public function testUpdateTagPermissionException($originalTag, $changedArgs, $expectedException = null) {
+ public function testUpdateTagPermissionException(ISystemTag $originalTag, $changedArgs, $expectedException = null) {
$this->tagManager->expects($this->any())
->method('canUserSeeTag')
->with($originalTag)
@@ -242,17 +240,16 @@ class SystemTagNodeTest extends \Test\TestCase {
*/
public function testDeleteTag($isAdmin) {
$tag = new SystemTag(1, 'tag1', true, true);
- $this->tagManager->expects($this->once())
+ $this->tagManager->expects($isAdmin ? $this->once() : $this->never())
->method('canUserSeeTag')
->with($tag)
->will($this->returnValue(true));
- $this->tagManager->expects($this->once())
- ->method('canUserAssignTag')
- ->with($tag)
- ->will($this->returnValue(true));
- $this->tagManager->expects($this->once())
+ $this->tagManager->expects($isAdmin ? $this->once() : $this->never())
->method('deleteTags')
->with('1');
+ if (!$isAdmin) {
+ $this->setExpectedException(Forbidden::class);
+ }
$this->getTagNode($isAdmin, $tag)->delete();
}
@@ -261,7 +258,7 @@ class SystemTagNodeTest extends \Test\TestCase {
[
// cannot delete invisible tag
new SystemTag(1, 'Original', false, true),
- 'Sabre\DAV\Exception\NotFound',
+ 'Sabre\DAV\Exception\Forbidden',
],
[
// cannot delete non-assignable tag
@@ -279,20 +276,11 @@ class SystemTagNodeTest extends \Test\TestCase {
->method('canUserSeeTag')
->with($tag)
->will($this->returnValue($tag->isUserVisible()));
- $this->tagManager->expects($this->any())
- ->method('canUserAssignTag')
- ->with($tag)
- ->will($this->returnValue($tag->isUserAssignable()));
$this->tagManager->expects($this->never())
->method('deleteTags');
- try {
- $this->getTagNode(false, $tag)->delete();
- } catch (\Exception $e) {
- $thrown = $e;
- }
-
- $this->assertInstanceOf($expectedException, $thrown);
+ $this->setExpectedException($expectedException);
+ $this->getTagNode(false, $tag)->delete();
}
/**
@@ -304,14 +292,10 @@ class SystemTagNodeTest extends \Test\TestCase {
->method('canUserSeeTag')
->with($tag)
->will($this->returnValue($tag->isUserVisible()));
- $this->tagManager->expects($this->any())
- ->method('canUserAssignTag')
- ->with($tag)
- ->will($this->returnValue($tag->isUserAssignable()));
$this->tagManager->expects($this->once())
->method('deleteTags')
->with('1')
->will($this->throwException(new TagNotFoundException()));
- $this->getTagNode(false, $tag)->delete();
+ $this->getTagNode(true, $tag)->delete();
}
}
diff --git a/build/integration/features/tags.feature b/build/integration/features/tags.feature
index 35784419080..0c6cd06f9f9 100644
--- a/build/integration/features/tags.feature
+++ b/build/integration/features/tags.feature
@@ -70,12 +70,13 @@ Feature: tags
When "user0" edits the tag with name "TagWithGroups" and sets its groups to "group1|group3"
Then The response should have a status code "403"
- Scenario: Deleting a normal tag as regular user should work
+ Scenario: Deleting a normal tag as regular user should fail
Given user "user0" exists
Given "admin" creates a "normal" tag with name "MySuperAwesomeTagName"
When "user0" deletes the tag with name "MySuperAwesomeTagName"
- Then The response should have a status code "204"
- And "0" tags should exist for "admin"
+ Then The response should have a status code "403"
+ And The following tags should exist for "admin"
+ |MySuperAwesomeTagName|true|true|
Scenario: Deleting a not user-assignable tag as regular user should fail
Given user "user0" exists
@@ -93,6 +94,12 @@ Feature: tags
And The following tags should exist for "admin"
|MySuperAwesomeTagName|false|true|
+ Scenario: Deleting a normal tag as admin should work
+ Given "admin" creates a "normal" tag with name "MySuperAwesomeTagName"
+ When "admin" deletes the tag with name "MySuperAwesomeTagName"
+ Then The response should have a status code "204"
+ And "0" tags should exist for "admin"
+
Scenario: Deleting a not user-assignable tag as admin should work
Given "admin" creates a "not user-assignable" tag with name "MySuperAwesomeTagName"
When "admin" deletes the tag with name "MySuperAwesomeTagName"
diff --git a/core/js/systemtags/systemtagsinputfield.js b/core/js/systemtags/systemtagsinputfield.js
index 690525c0ebb..d5f6bd5f97e 100644
--- a/core/js/systemtags/systemtagsinputfield.js
+++ b/core/js/systemtags/systemtagsinputfield.js
@@ -40,7 +40,9 @@
'<form class="systemtags-rename-form">' +
' <label class="hidden-visually" for="{{cid}}-rename-input">{{renameLabel}}</label>' +
' <input id="{{cid}}-rename-input" type="text" value="{{name}}">' +
- ' <a href="#" class="delete icon icon-delete" title="{{deleteTooltip}}"></a>' +
+ ' {{#if isAdmin}}' +
+ ' <a href="#" class="delete icon icon-delete" title="{{deleteTooltip}}"></a>' +
+ ' {{/if}}' +
'</form>';
/**
@@ -148,7 +150,8 @@
cid: this.cid,
name: oldName,
deleteTooltip: t('core', 'Delete'),
- renameLabel: t('core', 'Rename')
+ renameLabel: t('core', 'Rename'),
+ isAdmin: this._isAdmin
}));
$item.find('.label').after($renameForm);
$item.find('.label, .systemtags-actions').addClass('hidden');
diff --git a/core/js/tests/specs/systemtags/systemtagsinputfieldSpec.js b/core/js/tests/specs/systemtags/systemtagsinputfieldSpec.js
index 503bef7cf2b..8d3f67bfa0d 100644
--- a/core/js/tests/specs/systemtags/systemtagsinputfieldSpec.js
+++ b/core/js/tests/specs/systemtags/systemtagsinputfieldSpec.js
@@ -267,20 +267,6 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
saveStub.restore();
});
- it('deletes model and submits change when clicking delete', function() {
- var destroyStub = sinon.stub(OC.SystemTags.SystemTagModel.prototype, 'destroy');
-
- expect($dropdown.find('.delete').length).toEqual(0);
- $dropdown.find('.rename').mouseup();
- // delete button appears
- expect($dropdown.find('.delete').length).toEqual(1);
- $dropdown.find('.delete').mouseup();
-
- expect(destroyStub.calledOnce).toEqual(true);
- expect(destroyStub.calledOn(view.collection.get('1')));
-
- destroyStub.restore();
- });
});
describe('setting data', function() {
it('sets value when calling setValues', function() {
@@ -299,12 +285,18 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
});
describe('as admin', function() {
+ var $dropdown;
+
beforeEach(function() {
view = new OC.SystemTags.SystemTagsInputField({
isAdmin: true
});
- view.render();
$('.testInputContainer').append(view.$el);
+ $dropdown = $('<div class="select2-dropdown"></div>');
+ select2Stub.withArgs('dropdown').returns($dropdown);
+ $('#testArea').append($dropdown);
+
+ view.render();
});
it('formatResult renders tag name with visibility', function() {
var opts = select2Stub.getCall(0).args[0];
@@ -431,15 +423,50 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
]);
});
});
+ describe('tag actions', function() {
+ var opts;
+
+ beforeEach(function() {
+
+ opts = select2Stub.getCall(0).args[0];
+
+ view.collection.add([
+ new OC.SystemTags.SystemTagModel({id: '1', name: 'abc'}),
+ ]);
+
+ $dropdown.append(opts.formatResult(view.collection.get('1').toJSON()));
+
+ });
+ it('deletes model and submits change when clicking delete', function() {
+ var destroyStub = sinon.stub(OC.SystemTags.SystemTagModel.prototype, 'destroy');
+
+ expect($dropdown.find('.delete').length).toEqual(0);
+ $dropdown.find('.rename').mouseup();
+ // delete button appears
+ expect($dropdown.find('.delete').length).toEqual(1);
+ $dropdown.find('.delete').mouseup();
+
+ expect(destroyStub.calledOnce).toEqual(true);
+ expect(destroyStub.calledOn(view.collection.get('1')));
+
+ destroyStub.restore();
+ });
+ });
});
describe('as user', function() {
+ var $dropdown;
+
beforeEach(function() {
view = new OC.SystemTags.SystemTagsInputField({
isAdmin: false
});
- view.render();
$('.testInputContainer').append(view.$el);
+ $dropdown = $('<div class="select2-dropdown"></div>');
+ select2Stub.withArgs('dropdown').returns($dropdown);
+ $('#testArea').append($dropdown);
+
+ view.render();
});
it('formatResult renders tag name only', function() {
var opts = select2Stub.getCall(0).args[0];
@@ -570,5 +597,33 @@ describe('OC.SystemTags.SystemTagsInputField tests', function() {
]);
});
});
+ describe('tag actions', function() {
+ var opts;
+
+ beforeEach(function() {
+
+ opts = select2Stub.getCall(0).args[0];
+
+ view.collection.add([
+ new OC.SystemTags.SystemTagModel({id: '1', name: 'abc'}),
+ ]);
+
+ $dropdown.append(opts.formatResult(view.collection.get('1').toJSON()));
+
+ });
+ it('deletes model and submits change when clicking delete', function() {
+ var destroyStub = sinon.stub(OC.SystemTags.SystemTagModel.prototype, 'destroy');
+
+ expect($dropdown.find('.delete').length).toEqual(0);
+ $dropdown.find('.rename').mouseup();
+ // delete button appears only for admins
+ expect($dropdown.find('.delete').length).toEqual(0);
+ $dropdown.find('.delete').mouseup();
+
+ expect(destroyStub.notCalled).toEqual(true);
+
+ destroyStub.restore();
+ });
+ });
});
});