From 3596a8cdc7d5b564c502503271ab0e30bee1f63e Mon Sep 17 00:00:00 2001 From: Marcin Kurczewski Date: Mon, 12 May 2014 18:30:31 +0200 Subject: [PATCH] Fixed tag renaming --- src/Models/Enums/Privilege.php | 2 +- src/Models/TagModel.php | 21 ++++-- tests/JobTests/MergeTagsJobTest.php | 107 +++++++++++++++++++++++++++ tests/JobTests/RenameTagsJobTest.php | 104 ++++++++++++++++++++++++++ 4 files changed, 225 insertions(+), 9 deletions(-) create mode 100644 tests/JobTests/MergeTagsJobTest.php create mode 100644 tests/JobTests/RenameTagsJobTest.php diff --git a/src/Models/Enums/Privilege.php b/src/Models/Enums/Privilege.php index a38d7c6f..9ba5f80d 100644 --- a/src/Models/Enums/Privilege.php +++ b/src/Models/Enums/Privilege.php @@ -49,7 +49,7 @@ class Privilege extends Enum const ListTags = 21; const MergeTags = 27; - const RenameTags = 27; + const RenameTags = 47; const MassTag = 29; const ListLogs = 32; diff --git a/src/Models/TagModel.php b/src/Models/TagModel.php index a3a7c798..472f4f3e 100644 --- a/src/Models/TagModel.php +++ b/src/Models/TagModel.php @@ -47,11 +47,16 @@ final class TagModel extends AbstractCrudModel { Database::transaction(function() use ($sourceName, $targetName) { - $sourceTag = TagModel::getByName($sourceName); - $targetTag = TagModel::tryGetByName($targetName); + $sourceTag = self::getByName($sourceName); + $targetTag = self::tryGetByName($targetName); + + if ($targetTag) + { + if ($sourceTag->getId() == $targetTag->getId()) + throw new SimpleException('Source and target tag are the same'); - if ($targetTag and $targetTag->getId() != $sourceTag->getId()) throw new SimpleException('Target tag already exists'); + } $sourceTag->setName($targetName); self::save($sourceTag); @@ -62,8 +67,8 @@ final class TagModel extends AbstractCrudModel { Database::transaction(function() use ($sourceName, $targetName) { - $sourceTag = TagModel::getByName($sourceName); - $targetTag = TagModel::getByName($targetName); + $sourceTag = self::getByName($sourceName); + $targetTag = self::getByName($targetName); if ($sourceTag->getId() == $targetTag->getId()) throw new SimpleException('Source and target tag are the same'); @@ -162,12 +167,12 @@ final class TagModel extends AbstractCrudModel $tags = []; foreach ($tagNames as $tagName) { - $tag = TagModel::tryGetByName($tagName); + $tag = self::tryGetByName($tagName); if (!$tag) { - $tag = TagModel::spawn(); + $tag = self::spawn(); $tag->setName($tagName); - TagModel::save($tag); + self::save($tag); } $tags []= $tag; } diff --git a/tests/JobTests/MergeTagsJobTest.php b/tests/JobTests/MergeTagsJobTest.php new file mode 100644 index 00000000..6b9db464 --- /dev/null +++ b/tests/JobTests/MergeTagsJobTest.php @@ -0,0 +1,107 @@ +grantAccess('mergeTags'); + + $sourceTag = $this->mockTag(); + $randomTag = $this->mockTag(); + $targetTag = $this->mockTag(); + + $posts = []; + foreach (range(0, 5) as $i) + $posts []= $this->mockPost($this->mockUser()); + + $posts[0]->setTags([$sourceTag]); + $posts[1]->setTags([$randomTag]); + $posts[2]->setTags([$sourceTag, $randomTag]); + + $posts[3]->setTags([$sourceTag, $targetTag]); + $posts[4]->setTags([$randomTag, $targetTag]); + $posts[5]->setTags([$sourceTag, $randomTag, $targetTag]); + + foreach ($posts as $post) + PostModel::save($post); + + $this->assert->doesNotThrow(function() use ($sourceTag, $targetTag) + { + Api::run( + new MergeTagsJob(), + [ + JobArgs::ARG_SOURCE_TAG_NAME => $sourceTag->getName(), + JobArgs::ARG_TARGET_TAG_NAME => $targetTag->getName(), + ]); + }); + + foreach ($posts as $k => $post) + $posts[$k] = PostModel::getById($post->getId()); + + $this->assertTagNames($posts[0], [$targetTag]); + $this->assertTagNames($posts[1], [$randomTag]); + $this->assertTagNames($posts[2], [$randomTag, $targetTag]); + + $this->assertTagNames($posts[3], [$targetTag]); + $this->assertTagNames($posts[4], [$randomTag, $targetTag]); + $this->assertTagNames($posts[5], [$randomTag, $targetTag]); + } + + public function testMergingToNonExistingTag() + { + $this->grantAccess('mergeTags'); + + $sourceTag = $this->mockTag(); + $post = $this->mockPost($this->mockUser()); + $post->setTags([$sourceTag]); + PostModel::save($post); + + $this->assert->throws(function() use ($sourceTag) + { + Api::run( + new MergeTagsJob(), + [ + JobArgs::ARG_SOURCE_TAG_NAME => $sourceTag->getName(), + JobArgs::ARG_TARGET_TAG_NAME => 'nonsense', + ]); + }, 'Invalid tag name'); + } + + public function testMergingToItself() + { + $this->grantAccess('mergeTags'); + + $sourceTag = $this->mockTag(); + $post = $this->mockPost($this->mockUser()); + $post->setTags([$sourceTag]); + PostModel::save($post); + + $this->assert->throws(function() use ($sourceTag) + { + Api::run( + new MergeTagsJob(), + [ + JobArgs::ARG_SOURCE_TAG_NAME => $sourceTag->getName(), + JobArgs::ARG_TARGET_TAG_NAME => $sourceTag->getName(), + ]); + }, 'Source and target tag are the same'); + } + + private function assertTagNames($post, $tags) + { + $tagNames = $this->getTagNames($tags); + $postTagNames = $this->getTagNames($post->getTags()); + $this->assert->areEquivalent($tagNames, $postTagNames); + } + + private function getTagNames($tags) + { + $tagNames = array_map( + function($tag) + { + return $tag->getName(); + }, $tags); + natcasesort($tagNames); + $tagNames = array_values($tagNames); + return $tagNames; + } +} diff --git a/tests/JobTests/RenameTagsJobTest.php b/tests/JobTests/RenameTagsJobTest.php new file mode 100644 index 00000000..4c321772 --- /dev/null +++ b/tests/JobTests/RenameTagsJobTest.php @@ -0,0 +1,104 @@ +grantAccess('renameTags'); + + $sourceTag = $this->mockTag(); + $randomTag = $this->mockTag(); + $targetTag = $this->mockTag(); + + $posts = []; + foreach (range(0, 2) as $i) + $posts []= $this->mockPost($this->mockUser()); + + $posts[0]->setTags([$sourceTag]); + $posts[1]->setTags([$randomTag]); + $posts[2]->setTags([$sourceTag, $randomTag]); + + foreach ($posts as $post) + PostModel::save($post); + + $this->assert->doesNotThrow(function() use ($sourceTag, $targetTag) + { + Api::run( + new RenameTagsJob(), + [ + JobArgs::ARG_SOURCE_TAG_NAME => $sourceTag->getName(), + JobArgs::ARG_TARGET_TAG_NAME => $targetTag->getName(), + ]); + }); + + foreach ($posts as $k => $post) + $posts[$k] = PostModel::getById($post->getId()); + + $this->assertTagNames($posts[0], [$targetTag]); + $this->assertTagNames($posts[1], [$randomTag]); + $this->assertTagNames($posts[2], [$randomTag, $targetTag]); + } + + public function testRenamingToExistingTag() + { + $this->grantAccess('renameTags'); + + $sourceTag = $this->mockTag(); + $post = $this->mockPost($this->mockUser()); + $post->setTags([$sourceTag]); + PostModel::save($post); + + $targetTag = $this->mockTag(); + $post = $this->mockPost($this->mockUser()); + $post->setTags([$targetTag]); + PostModel::save($post); + + $this->assert->throws(function() use ($sourceTag, $targetTag) + { + Api::run( + new RenameTagsJob(), + [ + JobArgs::ARG_SOURCE_TAG_NAME => $sourceTag->getName(), + JobArgs::ARG_TARGET_TAG_NAME => $targetTag->getName(), + ]); + }, 'Target tag already exists'); + } + + public function testRenamingToItself() + { + $this->grantAccess('renameTags'); + + $sourceTag = $this->mockTag(); + $post = $this->mockPost($this->mockUser()); + $post->setTags([$sourceTag]); + PostModel::save($post); + + $this->assert->throws(function() use ($sourceTag) + { + Api::run( + new RenameTagsJob(), + [ + JobArgs::ARG_SOURCE_TAG_NAME => $sourceTag->getName(), + JobArgs::ARG_TARGET_TAG_NAME => $sourceTag->getName(), + ]); + }, 'Source and target tag are the same'); + } + + private function assertTagNames($post, $tags) + { + $tagNames = $this->getTagNames($tags); + $postTagNames = $this->getTagNames($post->getTags()); + $this->assert->areEquivalent($tagNames, $postTagNames); + } + + private function getTagNames($tags) + { + $tagNames = array_map( + function($tag) + { + return $tag->getName(); + }, $tags); + natcasesort($tagNames); + $tagNames = array_values($tagNames); + return $tagNames; + } +}