diff --git a/module/Core/src/Exception/TagConflictException.php b/module/Core/src/Exception/TagConflictException.php index 235cccc5..db26682e 100644 --- a/module/Core/src/Exception/TagConflictException.php +++ b/module/Core/src/Exception/TagConflictException.php @@ -8,8 +8,8 @@ use function sprintf; class TagConflictException extends RuntimeException { - public static function fromExistingTag(string $tag): self + public static function fromExistingTag(string $oldName, string $newName): self { - return new self(sprintf('Tag with name %s already exists', $tag)); + return new self(sprintf('You cannot rename tag %s to %s, because it already exists', $oldName, $newName)); } } diff --git a/module/Core/src/Service/Tag/TagService.php b/module/Core/src/Service/Tag/TagService.php index 24856188..4a75a061 100644 --- a/module/Core/src/Service/Tag/TagService.php +++ b/module/Core/src/Service/Tag/TagService.php @@ -82,7 +82,7 @@ class TagService implements TagServiceInterface $newNameExists = $newName !== $oldName && $repo->count(['name' => $newName]) > 0; if ($newNameExists) { - throw TagConflictException::fromExistingTag($newName); + throw TagConflictException::fromExistingTag($oldName, $newName); } $tag->rename($newName); diff --git a/module/Rest/src/Action/Tag/UpdateTagAction.php b/module/Rest/src/Action/Tag/UpdateTagAction.php index 64ec7a21..d494673c 100644 --- a/module/Rest/src/Action/Tag/UpdateTagAction.php +++ b/module/Rest/src/Action/Tag/UpdateTagAction.php @@ -8,6 +8,7 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; +use Shlinkio\Shlink\Core\Exception\TagConflictException; use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; use Shlinkio\Shlink\Rest\Util\RestUtils; @@ -58,6 +59,15 @@ class UpdateTagAction extends AbstractRestAction 'error' => RestUtils::NOT_FOUND_ERROR, 'message' => sprintf('It was not possible to find a tag with name %s', $body['oldName']), ], self::STATUS_NOT_FOUND); + } catch (TagConflictException $e) { + return new JsonResponse([ + 'error' => 'TAG_CONFLICT', + 'message' => sprintf( + 'You cannot rename tag %s to %s, because it already exists', + $body['oldName'], + $body['newName'] + ), + ], self::STATUS_CONFLICT); } } } diff --git a/module/Rest/test-api/Action/UpdateTagActionTest.php b/module/Rest/test-api/Action/UpdateTagActionTest.php new file mode 100644 index 00000000..206428af --- /dev/null +++ b/module/Rest/test-api/Action/UpdateTagActionTest.php @@ -0,0 +1,35 @@ +callApiWithKey(self::METHOD_PUT, '/tags', [RequestOptions::JSON => [ + 'oldName' => 'foo', + 'newName' => 'bar', + ]]); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertEquals(self::STATUS_CONFLICT, $resp->getStatusCode()); + $this->assertEquals('TAG_CONFLICT', $payload['error']); + } + + /** @test */ + public function tagIsProperlyRenamedWhenRenamingToItself(): void + { + $resp = $this->callApiWithKey(self::METHOD_PUT, '/tags', [RequestOptions::JSON => [ + 'oldName' => 'foo', + 'newName' => 'foo', + ]]); + + $this->assertEquals(self::STATUS_NO_CONTENT, $resp->getStatusCode()); + } +}