From 476cd4edc96221bf984175671f29e76c6b43d831 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Mon, 7 Jun 2021 16:13:44 +0200 Subject: [PATCH 1/4] IBX-461: Fixed roles with deleted limitations --- .../UDWBasedValueModelTransformer.php | 15 +++++---------- .../Templating/LimitationBlockRenderer.php | 17 +++++------------ 2 files changed, 10 insertions(+), 22 deletions(-) diff --git a/lib/Limitation/DataTransformer/UDWBasedValueModelTransformer.php b/lib/Limitation/DataTransformer/UDWBasedValueModelTransformer.php index d190054c9..b330822e2 100644 --- a/lib/Limitation/DataTransformer/UDWBasedValueModelTransformer.php +++ b/lib/Limitation/DataTransformer/UDWBasedValueModelTransformer.php @@ -24,9 +24,6 @@ class UDWBasedValueModelTransformer implements DataTransformerInterface /** @var \eZ\Publish\API\Repository\LocationService */ private $locationService; - /** - * @param \eZ\Publish\API\Repository\LocationService $locationService - */ public function __construct(LocationService $locationService) { $this->locationService = $locationService; @@ -37,7 +34,7 @@ public function __construct(LocationService $locationService) */ public function transform($value) { - if (!is_array($value)) { + if (!\is_array($value)) { return null; } @@ -47,7 +44,9 @@ public function transform($value) $this->extractLocationIdFromPath($path) ); }, $value); - } catch (NotFoundException | UnauthorizedException $e) { + } catch (NotFoundException $e) { + return null; + } catch (UnauthorizedException $e) { throw new TransformationFailedException($e->getMessage(), $e->getCode(), $e); } } @@ -57,7 +56,7 @@ public function transform($value) */ public function reverseTransform($value) { - if (!is_array($value)) { + if (!\is_array($value)) { return null; } @@ -66,10 +65,6 @@ public function reverseTransform($value) /** * Extracts and returns an item id from a path, e.g. /1/2/58/ => 58. - * - * @param string $path - * - * @return string|null */ private function extractLocationIdFromPath(string $path): ?string { diff --git a/lib/Limitation/Templating/LimitationBlockRenderer.php b/lib/Limitation/Templating/LimitationBlockRenderer.php index 9aa80e467..4402b04de 100644 --- a/lib/Limitation/Templating/LimitationBlockRenderer.php +++ b/lib/Limitation/Templating/LimitationBlockRenderer.php @@ -7,6 +7,7 @@ */ namespace EzSystems\RepositoryForms\Limitation\Templating; +use eZ\Publish\API\Repository\Exceptions\NotFoundException; use eZ\Publish\API\Repository\Values\User\Limitation; use EzSystems\RepositoryForms\Limitation\Exception\MissingLimitationBlockException; use EzSystems\RepositoryForms\Limitation\Exception\ValueMapperNotFoundException; @@ -36,9 +37,6 @@ class LimitationBlockRenderer implements LimitationBlockRendererInterface /** * LimitationRenderer constructor. - * - * @param LimitationValueMapperRegistryInterface $valueMapperRegistry - * @param Twig_Environment $twig */ public function __construct(LimitationValueMapperRegistryInterface $valueMapperRegistry, Twig_Environment $twig) { @@ -51,7 +49,7 @@ public function renderLimitationValue(Limitation $limitation, array $parameters try { $blockName = $this->getValueBlockName($limitation); $parameters = $this->getValueBlockParameters($limitation, $parameters); - } catch (ValueMapperNotFoundException $exception) { + } catch (ValueMapperNotFoundException | NotFoundException $exception) { $blockName = self::LIMITATION_VALUE_BLOCK_NAME_FALLBACK; $parameters = $this->getValueFallbackBlockParameters($limitation, $parameters); } @@ -82,7 +80,6 @@ public function setLimitationValueResources(array $resources) /** * Generates value block name based on Limitation. * - * @param Limitation $limitation * @return string */ protected function getValueBlockName(Limitation $limitation) @@ -95,12 +92,12 @@ protected function getValueBlockName(Limitation $limitation) * * @param string $blockName * @param string|Twig_Template $localTemplate - * @return null|\Twig_TemplateWrapper + * @return \Twig_TemplateWrapper|null */ protected function findTemplateWithBlock($blockName, $localTemplate = null) { if ($localTemplate !== null) { - if (is_string($localTemplate)) { + if (\is_string($localTemplate)) { $localTemplate = $this->twig->load($localTemplate); } @@ -110,7 +107,7 @@ protected function findTemplateWithBlock($blockName, $localTemplate = null) } foreach ($this->limitationValueResources as &$template) { - if (is_string($template)) { + if (\is_string($template)) { // Load the template if it is necessary $template = $this->twig->load($template); } @@ -126,8 +123,6 @@ protected function findTemplateWithBlock($blockName, $localTemplate = null) /** * Get parameters passed as context of value block render. * - * @param Limitation $limitation - * @param array $parameters * @return array */ protected function getValueBlockParameters(Limitation $limitation, array $parameters) @@ -147,8 +142,6 @@ protected function getValueBlockParameters(Limitation $limitation, array $parame /** * Get parameters passed as context of value fallback block. * - * @param Limitation $limitation - * @param array $parameters * @return array */ protected function getValueFallbackBlockParameters(Limitation $limitation, array $parameters) From 94e85105abf2f788b3bf7fdf75ef51b499df3d74 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Mon, 7 Jun 2021 17:05:26 +0200 Subject: [PATCH 2/4] IBX-461: Refactored unit tests --- .../UDWBasedValueModelTransformerTest.php | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/tests/RepositoryForms/Limitation/DataTransformer/UDWBasedValueModelTransformerTest.php b/tests/RepositoryForms/Limitation/DataTransformer/UDWBasedValueModelTransformerTest.php index 0f04f7a1c..cb77d4897 100644 --- a/tests/RepositoryForms/Limitation/DataTransformer/UDWBasedValueModelTransformerTest.php +++ b/tests/RepositoryForms/Limitation/DataTransformer/UDWBasedValueModelTransformerTest.php @@ -65,10 +65,7 @@ public function dataProviderForTransform(): array ]; } - /** - * @dataProvider dataProviderForTransformThrowsTransformationFailedException - */ - public function testTransformThrowsTransformationFailedException(string $exceptionClass) + public function testTransformThrowsTransformationFailedException() { $this->expectException(TransformationFailedException::class); @@ -76,16 +73,28 @@ public function testTransformThrowsTransformationFailedException(string $excepti ->expects($this->any()) ->method('loadLocation') ->willThrowException( - $this->createMock($exceptionClass) + $this->createMock(UnauthorizedException::class) ); $this->transformer->transform(['/1/2/54']); } + public function testTransformWithDeletedLocation() + { + $this->locationService + ->expects($this->any()) + ->method('loadLocation') + ->willThrowException( + $this->createMock(NotFoundException::class) + ); + + self::assertNull($this->transformer->transform(['/1/2/54'])); + } + public function dataProviderForTransformThrowsTransformationFailedException(): array { return [ - [NotFoundException::class], + //[NotFoundException::class], [UnauthorizedException::class], ]; } From d12119d8b373d37c3eb787817e9c6b5655b044e7 Mon Sep 17 00:00:00 2001 From: barw4 Date: Mon, 14 Jun 2021 11:48:09 +0200 Subject: [PATCH 3/4] IBX-461: Applied review remarks --- .../UDWBasedValueModelTransformerTest.php | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/tests/RepositoryForms/Limitation/DataTransformer/UDWBasedValueModelTransformerTest.php b/tests/RepositoryForms/Limitation/DataTransformer/UDWBasedValueModelTransformerTest.php index cb77d4897..151ce38ae 100644 --- a/tests/RepositoryForms/Limitation/DataTransformer/UDWBasedValueModelTransformerTest.php +++ b/tests/RepositoryForms/Limitation/DataTransformer/UDWBasedValueModelTransformerTest.php @@ -65,7 +65,7 @@ public function dataProviderForTransform(): array ]; } - public function testTransformThrowsTransformationFailedException() + public function testTransformThrowsTransformationFailedException(): void { $this->expectException(TransformationFailedException::class); @@ -79,7 +79,7 @@ public function testTransformThrowsTransformationFailedException() $this->transformer->transform(['/1/2/54']); } - public function testTransformWithDeletedLocation() + public function testTransformWithDeletedLocation(): void { $this->locationService ->expects($this->any()) @@ -91,14 +91,6 @@ public function testTransformWithDeletedLocation() self::assertNull($this->transformer->transform(['/1/2/54'])); } - public function dataProviderForTransformThrowsTransformationFailedException(): array - { - return [ - //[NotFoundException::class], - [UnauthorizedException::class], - ]; - } - /** * @dataProvider dataProviderForReverseTransform */ From 0212f12d5a5ed12ec64442fd1ad988c7182c4c9e Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Tue, 15 Jun 2021 11:45:58 +0200 Subject: [PATCH 4/4] IBX-461: Applied review remarks --- .../DataTransformer/UDWBasedValueModelTransformerTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/RepositoryForms/Limitation/DataTransformer/UDWBasedValueModelTransformerTest.php b/tests/RepositoryForms/Limitation/DataTransformer/UDWBasedValueModelTransformerTest.php index 151ce38ae..fc82e17d1 100644 --- a/tests/RepositoryForms/Limitation/DataTransformer/UDWBasedValueModelTransformerTest.php +++ b/tests/RepositoryForms/Limitation/DataTransformer/UDWBasedValueModelTransformerTest.php @@ -70,7 +70,6 @@ public function testTransformThrowsTransformationFailedException(): void $this->expectException(TransformationFailedException::class); $this->locationService - ->expects($this->any()) ->method('loadLocation') ->willThrowException( $this->createMock(UnauthorizedException::class) @@ -82,7 +81,6 @@ public function testTransformThrowsTransformationFailedException(): void public function testTransformWithDeletedLocation(): void { $this->locationService - ->expects($this->any()) ->method('loadLocation') ->willThrowException( $this->createMock(NotFoundException::class)