From 1039d72ffaacf938a2d26102f198a5473de9daef Mon Sep 17 00:00:00 2001 From: Peter Gribanov Date: Thu, 5 Mar 2020 13:22:56 +0300 Subject: [PATCH] Data Transformers in list fields editable fix #5693 allow use data_transformer in SetObjectFieldValueAction create BooleanToStringTransformer for allows to use non-strings update SetObjectFieldValueActionTest use yoda conditions fix errors in HelperControllerTest test BooleanToStringTransformer allow override transformers for 'date', 'boolean' and 'choice' field types mark BooleanToStringTransformer and BooleanToStringTransformer classes as final add example of using the data_transformer option in docs --- docs/reference/action_list.rst | 8 ++ src/Action/SetObjectFieldValueAction.php | 68 ++++++--- .../BooleanToStringTransformer.php | 42 ++++++ tests/Action/Bar.php | 10 +- tests/Action/Baz.php | 2 +- tests/Action/Foo.php | 10 +- .../Action/SetObjectFieldValueActionTest.php | 135 +++++++++++++++++- tests/Controller/HelperControllerTest.php | 7 + .../BooleanToStringTransformerTest.php | 79 ++++++++++ 9 files changed, 335 insertions(+), 26 deletions(-) create mode 100644 src/Form/DataTransformer/BooleanToStringTransformer.php create mode 100644 tests/Form/DataTransformer/BooleanToStringTransformerTest.php diff --git a/docs/reference/action_list.rst b/docs/reference/action_list.rst index db547eeebd6..de3a26c575f 100644 --- a/docs/reference/action_list.rst +++ b/docs/reference/action_list.rst @@ -80,6 +80,14 @@ Here is an example:: ], ]) + // you can use Symfony Data Transformers to transform value into the data form expected by your + // application. For example, in a Value Object + ->add('permission', 'choice', [ + 'editable' => true, + 'choices' => Permission::choices(), + 'data_transformer' => new PermissionDataTransformer(), + ]) + // we can add options to the field depending on the type ->add('price', 'currency', [ 'currency' => $this->currencyDetector->getCurrency()->getLabel() diff --git a/src/Action/SetObjectFieldValueAction.php b/src/Action/SetObjectFieldValueAction.php index 37cae1f92c8..24cb0980cbd 100644 --- a/src/Action/SetObjectFieldValueAction.php +++ b/src/Action/SetObjectFieldValueAction.php @@ -13,8 +13,14 @@ namespace Sonata\AdminBundle\Action; +use Sonata\AdminBundle\Admin\FieldDescriptionInterface; use Sonata\AdminBundle\Admin\Pool; +use Sonata\AdminBundle\Form\DataTransformer\BooleanToStringTransformer; +use Sonata\AdminBundle\Form\DataTransformer\ModelToIdTransformer; +use Sonata\AdminBundle\Model\ModelManagerInterface; use Sonata\AdminBundle\Twig\Extension\SonataAdminExtension; +use Symfony\Component\Form\DataTransformerInterface; +use Symfony\Component\Form\Extension\Core\DataTransformer\DateTimeToStringTransformer; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -113,34 +119,25 @@ public function __invoke(Request $request): JsonResponse $propertyPath = new PropertyPath($field); } - // Handle date type has setter expect a DateTime object - if ('' !== $value && 'date' === $fieldDescription->getType()) { - $value = new \DateTime($value); - } + if ('' === $value) { + $this->pool->getPropertyAccessor()->setValue($object, $propertyPath, null); + } else { + $dataTransformer = $this->getFieldDataTransformer($fieldDescription, $admin->getModelManager()); - // Handle boolean type transforming the value into a boolean - if ('' !== $value && 'boolean' === $fieldDescription->getType()) { - $value = filter_var($value, FILTER_VALIDATE_BOOLEAN); - } + if ($dataTransformer instanceof DataTransformerInterface) { + $value = $dataTransformer->reverseTransform($value); + } - // Handle entity choice association type, transforming the value into entity - if ('' !== $value - && 'choice' === $fieldDescription->getType() - && null !== $fieldDescription->getOption('class') - && $fieldDescription->getOption('class') === $fieldDescription->getTargetEntity() - ) { - $value = $admin->getModelManager()->find($fieldDescription->getOption('class'), $value); - - if (!$value) { + if (!$value && 'choice' === $fieldDescription->getType()) { return new JsonResponse(sprintf( 'Edit failed, object with id: %s not found in association: %s.', $originalValue, $field ), Response::HTTP_NOT_FOUND); } - } - $this->pool->getPropertyAccessor()->setValue($object, $propertyPath, '' !== $value ? $value : null); + $this->pool->getPropertyAccessor()->setValue($object, $propertyPath, $value); + } $violations = $this->validator->validate($object); @@ -164,4 +161,37 @@ public function __invoke(Request $request): JsonResponse return new JsonResponse($content, Response::HTTP_OK); } + + private function getFieldDataTransformer( + FieldDescriptionInterface $fieldDescription, + ModelManagerInterface $modelManager + ): ?DataTransformerInterface { + $dataTransformer = $fieldDescription->getOption('data_transformer'); + + // allow override transformers for 'date', 'boolean' and 'choice' field types + if ($dataTransformer instanceof DataTransformerInterface) { + return $dataTransformer; + } + + // Handle date type has setter expect a DateTime object + if ('date' === $fieldDescription->getType()) { + $dataTransformer = new DateTimeToStringTransformer(); + } + + // Handle boolean type transforming the value into a boolean + if ('boolean' === $fieldDescription->getType()) { + $dataTransformer = new BooleanToStringTransformer('1'); + } + + // Handle entity choice association type, transforming the value into entity + if ('choice' === $fieldDescription->getType()) { + $className = $fieldDescription->getOption('class'); + + if (null !== $className && $className === $fieldDescription->getTargetEntity()) { + $dataTransformer = new ModelToIdTransformer($modelManager, $className); + } + } + + return $dataTransformer; + } } diff --git a/src/Form/DataTransformer/BooleanToStringTransformer.php b/src/Form/DataTransformer/BooleanToStringTransformer.php new file mode 100644 index 00000000000..ef8fced8ad5 --- /dev/null +++ b/src/Form/DataTransformer/BooleanToStringTransformer.php @@ -0,0 +1,42 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Sonata\AdminBundle\Form\DataTransformer; + +use Symfony\Component\Form\DataTransformerInterface; + +/** + * This is analog of Symfony\Component\Form\Extension\Core\DataTransformer\BooleanToStringTransformer + * which allows you to use non-strings in reverseTransform() method. + * + * @author Peter Gribanov + */ +final class BooleanToStringTransformer implements DataTransformerInterface +{ + private $trueValue; + + public function __construct(string $trueValue) + { + $this->trueValue = $trueValue; + } + + public function transform($value): ?string + { + return $value ? $this->trueValue : null; + } + + public function reverseTransform($value): bool + { + return filter_var($value, FILTER_VALIDATE_BOOLEAN); + } +} diff --git a/tests/Action/Bar.php b/tests/Action/Bar.php index 89ad2123db4..9ab4a79d8bf 100644 --- a/tests/Action/Bar.php +++ b/tests/Action/Bar.php @@ -15,7 +15,15 @@ class Bar { - public function setEnabled($value): void + private $enabled; + + public function getEnabled(): bool + { + return $this->enabled; + } + + public function setEnabled(bool $enabled): void { + $this->enabled = $enabled; } } diff --git a/tests/Action/Baz.php b/tests/Action/Baz.php index 126fa794208..9f2ac0f9e02 100644 --- a/tests/Action/Baz.php +++ b/tests/Action/Baz.php @@ -22,7 +22,7 @@ public function setBar(Bar $bar): void $this->bar = $bar; } - public function getBar() + public function getBar(): Bar { return $this->bar; } diff --git a/tests/Action/Foo.php b/tests/Action/Foo.php index 8a5126e4cdf..44b96ba6cc3 100644 --- a/tests/Action/Foo.php +++ b/tests/Action/Foo.php @@ -15,7 +15,15 @@ class Foo { - public function setEnabled($value): void + private $enabled; + + public function getEnabled(): bool + { + return $this->enabled; + } + + public function setEnabled(bool $enabled): void { + $this->enabled = $enabled; } } diff --git a/tests/Action/SetObjectFieldValueActionTest.php b/tests/Action/SetObjectFieldValueActionTest.php index 7a37862b997..326a2db528a 100644 --- a/tests/Action/SetObjectFieldValueActionTest.php +++ b/tests/Action/SetObjectFieldValueActionTest.php @@ -24,6 +24,7 @@ use Sonata\AdminBundle\Templating\TemplateRegistryInterface; use Sonata\AdminBundle\Twig\Extension\SonataAdminExtension; use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\Form\CallbackTransformer; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\PropertyAccess\PropertyAccessor; @@ -61,6 +62,11 @@ final class SetObjectFieldValueActionTest extends TestCase */ private $validator; + /** + * @var ModelManagerInterface + */ + private $modelManager; + protected function setUp(): void { $this->twig = new Environment(new ArrayLoader([ @@ -72,11 +78,13 @@ protected function setUp(): void $this->pool->getInstance(Argument::any())->willReturn($this->admin->reveal()); $this->admin->setRequest(Argument::type(Request::class))->shouldBeCalled(); $this->validator = $this->prophesize(ValidatorInterface::class); + $this->modelManager = $this->prophesize(ModelManagerInterface::class); $this->action = new SetObjectFieldValueAction( $this->twig, $this->pool->reveal(), $this->validator->reveal() ); + $this->admin->getModelManager()->willReturn($this->modelManager->reveal()); } public function testSetObjectFieldValueAction(): void @@ -118,11 +126,13 @@ public function testSetObjectFieldValueAction(): void $fieldDescription->getType()->willReturn('boolean'); $fieldDescription->getTemplate()->willReturn(false); $fieldDescription->getValue(Argument::cetera())->willReturn('some value'); + $fieldDescription->getOption('data_transformer')->willReturn(null); $this->validator->validate($object)->willReturn(new ConstraintViolationList([])); $response = ($this->action)($request); + $this->assertTrue($object->getEnabled()); $this->assertSame(Response::HTTP_OK, $response->getStatusCode()); } @@ -139,7 +149,6 @@ public function testSetObjectFieldValueActionOnARelationField(): void ], [], [], [], [], ['REQUEST_METHOD' => Request::METHOD_POST, 'HTTP_X_REQUESTED_WITH' => 'XMLHttpRequest']); $fieldDescription = $this->prophesize(FieldDescriptionInterface::class); - $modelManager = $this->prophesize(ModelManagerInterface::class); $translator = $this->prophesize(TranslatorInterface::class); $propertyAccessor = new PropertyAccessor(); $templateRegistry = $this->prophesize(TemplateRegistryInterface::class); @@ -155,7 +164,6 @@ public function testSetObjectFieldValueActionOnARelationField(): void // NEXT_MAJOR: Remove this line $this->admin->getTemplate('base_list_field')->willReturn('admin_template'); $templateRegistry->getTemplate('base_list_field')->willReturn('admin_template'); - $this->admin->getModelManager()->willReturn($modelManager->reveal()); $this->twig->addExtension(new SonataAdminExtension( $this->pool->reveal(), null, @@ -170,12 +178,14 @@ public function testSetObjectFieldValueActionOnARelationField(): void $fieldDescription->getAdmin()->willReturn($this->admin->reveal()); $fieldDescription->getTemplate()->willReturn('field_template'); $fieldDescription->getValue(Argument::cetera())->willReturn('some value'); - $modelManager->find(\get_class($associationObject), 1)->willReturn($associationObject); + $fieldDescription->getOption('data_transformer')->willReturn(null); + $this->modelManager->find(\get_class($associationObject), 1)->willReturn($associationObject); $this->validator->validate($object)->willReturn(new ConstraintViolationList([])); $response = ($this->action)($request); + $this->assertSame($associationObject, $object->getBar()); $this->assertSame(Response::HTTP_OK, $response->getStatusCode()); } @@ -205,6 +215,7 @@ public function testSetObjectFieldValueActionWithViolations(): void ])); $fieldDescription->getOption('editable')->willReturn(true); $fieldDescription->getType()->willReturn('boolean'); + $fieldDescription->getOption('data_transformer')->willReturn(null); $response = ($this->action)($request); @@ -250,14 +261,130 @@ public function testSetObjectFieldEditableMultipleValue(): void $fieldDescription->getOption('editable')->willReturn(true); $fieldDescription->getOption('multiple')->willReturn(true); $fieldDescription->getAdmin()->willReturn($this->admin->reveal()); - $fieldDescription->getType()->willReturn('boolean'); + $fieldDescription->getType()->willReturn(null); $fieldDescription->getTemplate()->willReturn(false); $fieldDescription->getValue(Argument::cetera())->willReturn(['some value']); + $fieldDescription->getOption('data_transformer')->willReturn(null); + + $this->validator->validate($object)->willReturn(new ConstraintViolationList([])); + + $response = ($this->action)($request); + + $this->assertSame([1, 2], $object->status); + $this->assertSame(Response::HTTP_OK, $response->getStatusCode()); + } + + public function testSetObjectFieldTransformed(): void + { + $object = new Foo(); + $request = new Request([ + 'code' => 'sonata.post.admin', + 'objectId' => 42, + 'field' => 'enabled', + 'value' => 'yes', + 'context' => 'list', + ], [], [], [], [], ['REQUEST_METHOD' => Request::METHOD_POST, 'HTTP_X_REQUESTED_WITH' => 'XMLHttpRequest']); + + $data_transformer = new CallbackTransformer(static function ($value) { + return (string) (int) $value; + }, static function ($value) { + return filter_var($value, FILTER_VALIDATE_BOOLEAN); + }); + + $fieldDescription = $this->prophesize(FieldDescriptionInterface::class); + $pool = $this->prophesize(Pool::class); + $translator = $this->prophesize(TranslatorInterface::class); + $propertyAccessor = new PropertyAccessor(); + $templateRegistry = $this->prophesize(TemplateRegistryInterface::class); + $container = $this->prophesize(ContainerInterface::class); + + $this->admin->getObject(42)->willReturn($object); + $this->admin->getCode()->willReturn('sonata.post.admin'); + $this->admin->hasAccess('edit', $object)->willReturn(true); + $this->admin->getListFieldDescription('enabled')->willReturn($fieldDescription->reveal()); + $this->admin->update($object)->shouldBeCalled(); + // NEXT_MAJOR: Remove this line + $this->admin->getTemplate('base_list_field')->willReturn('admin_template'); + $templateRegistry->getTemplate('base_list_field')->willReturn('admin_template'); + $container->get('sonata.post.admin.template_registry')->willReturn($templateRegistry->reveal()); + $this->pool->getPropertyAccessor()->willReturn($propertyAccessor); + $this->twig->addExtension(new SonataAdminExtension( + $pool->reveal(), + null, + $translator->reveal(), + $container->reveal() + )); + $fieldDescription->getOption('editable')->willReturn(true); + $fieldDescription->getAdmin()->willReturn($this->admin->reveal()); + $fieldDescription->getType()->willReturn(null); + $fieldDescription->getTemplate()->willReturn(false); + $fieldDescription->getValue(Argument::cetera())->willReturn('some value'); + $fieldDescription->getOption('data_transformer')->willReturn($data_transformer); + + $this->validator->validate($object)->willReturn(new ConstraintViolationList([])); + + $response = ($this->action)($request); + + $this->assertTrue($object->getEnabled()); + $this->assertSame(Response::HTTP_OK, $response->getStatusCode()); + } + + public function testSetObjectFieldOverrideTransformer(): void + { + $object = new Foo(); + $request = new Request([ + 'code' => 'sonata.post.admin', + 'objectId' => 42, + 'field' => 'enabled', + 'value' => 'yes', + 'context' => 'list', + ], [], [], [], [], ['REQUEST_METHOD' => Request::METHOD_POST, 'HTTP_X_REQUESTED_WITH' => 'XMLHttpRequest']); + + $isOverridden = false; + $data_transformer = new CallbackTransformer(static function ($value) { + return (string) (int) $value; + }, static function ($value) use (&$isOverridden) { + $isOverridden = true; + + return filter_var($value, FILTER_VALIDATE_BOOLEAN); + }); + + $fieldDescription = $this->prophesize(FieldDescriptionInterface::class); + $pool = $this->prophesize(Pool::class); + $translator = $this->prophesize(TranslatorInterface::class); + $propertyAccessor = new PropertyAccessor(); + $templateRegistry = $this->prophesize(TemplateRegistryInterface::class); + $container = $this->prophesize(ContainerInterface::class); + + $this->admin->getObject(42)->willReturn($object); + $this->admin->getCode()->willReturn('sonata.post.admin'); + $this->admin->hasAccess('edit', $object)->willReturn(true); + $this->admin->getListFieldDescription('enabled')->willReturn($fieldDescription->reveal()); + $this->admin->update($object)->shouldBeCalled(); + // NEXT_MAJOR: Remove this line + $this->admin->getTemplate('base_list_field')->willReturn('admin_template'); + $templateRegistry->getTemplate('base_list_field')->willReturn('admin_template'); + $container->get('sonata.post.admin.template_registry')->willReturn($templateRegistry->reveal()); + $this->pool->getPropertyAccessor()->willReturn($propertyAccessor); + $this->twig->addExtension(new SonataAdminExtension( + $pool->reveal(), + null, + $translator->reveal(), + $container->reveal() + )); + $fieldDescription->getOption('editable')->willReturn(true); + $fieldDescription->getAdmin()->willReturn($this->admin->reveal()); + $fieldDescription->getType()->willReturn('boolean'); + $fieldDescription->getTemplate()->willReturn(false); + $fieldDescription->getValue(Argument::cetera())->willReturn('some value'); + $fieldDescription->getOption('data_transformer')->willReturn($data_transformer); $this->validator->validate($object)->willReturn(new ConstraintViolationList([])); $response = ($this->action)($request); + $this->assertTrue($object->getEnabled()); + $this->assertTrue($isOverridden); $this->assertSame(Response::HTTP_OK, $response->getStatusCode()); } } diff --git a/tests/Controller/HelperControllerTest.php b/tests/Controller/HelperControllerTest.php index ec4b1b4e751..b353b846251 100644 --- a/tests/Controller/HelperControllerTest.php +++ b/tests/Controller/HelperControllerTest.php @@ -200,6 +200,7 @@ public function testSetObjectFieldValueAction(): void $propertyAccessor = new PropertyAccessor(); $templateRegistry = $this->prophesize(TemplateRegistryInterface::class); $container = $this->prophesize(ContainerInterface::class); + $modelManager = $this->prophesize(ModelManagerInterface::class); $this->admin->getObject(42)->willReturn($object); $this->admin->getCode()->willReturn('sonata.post.admin'); @@ -208,6 +209,7 @@ public function testSetObjectFieldValueAction(): void $this->admin->update($object)->shouldBeCalled(); // NEXT_MAJOR: Remove this line $this->admin->getTemplate('base_list_field')->willReturn('admin_template'); + $this->admin->getModelManager()->willReturn($modelManager->reveal()); $templateRegistry->getTemplate('base_list_field')->willReturn('admin_template'); $container->get('sonata.post.admin.template_registry')->willReturn($templateRegistry->reveal()); $this->pool->getPropertyAccessor()->willReturn($propertyAccessor); @@ -221,6 +223,7 @@ public function testSetObjectFieldValueAction(): void $fieldDescription->getType()->willReturn('boolean'); $fieldDescription->getTemplate()->willReturn(false); $fieldDescription->getValue(Argument::cetera())->willReturn('some value'); + $fieldDescription->getOption('data_transformer')->willReturn(null); $this->validator->validate($object)->willReturn(new ConstraintViolationList([])); $response = $this->controller->setObjectFieldValueAction($request); @@ -274,6 +277,7 @@ public function testSetObjectFieldValueActionOnARelationField(): void $fieldDescription->getAdmin()->willReturn($this->admin->reveal()); $fieldDescription->getTemplate()->willReturn('field_template'); $fieldDescription->getValue(Argument::cetera())->willReturn('some value'); + $fieldDescription->getOption('data_transformer')->willReturn(null); $modelManager->find(\get_class($associationObject), 1)->willReturn($associationObject); $response = $this->controller->setObjectFieldValueAction($request); @@ -373,17 +377,20 @@ public function testSetObjectFieldValueActionWithViolations(): void $fieldDescription = $this->prophesize(FieldDescriptionInterface::class); $propertyAccessor = new PropertyAccessor(); + $modelManager = $this->prophesize(ModelManagerInterface::class); $this->pool->getPropertyAccessor()->willReturn($propertyAccessor); $this->admin->getObject(42)->willReturn($object); $this->admin->hasAccess('edit', $object)->willReturn(true); $this->admin->getListFieldDescription('bar.enabled')->willReturn($fieldDescription->reveal()); + $this->admin->getModelManager()->willReturn($modelManager->reveal()); $this->validator->validate($bar)->willReturn(new ConstraintViolationList([ new ConstraintViolation('error1', null, [], null, 'enabled', null), new ConstraintViolation('error2', null, [], null, 'enabled', null), ])); $fieldDescription->getOption('editable')->willReturn(true); $fieldDescription->getType()->willReturn('boolean'); + $fieldDescription->getOption('data_transformer')->willReturn(null); $response = $this->controller->setObjectFieldValueAction($request); diff --git a/tests/Form/DataTransformer/BooleanToStringTransformerTest.php b/tests/Form/DataTransformer/BooleanToStringTransformerTest.php new file mode 100644 index 00000000000..f5f06ff87bf --- /dev/null +++ b/tests/Form/DataTransformer/BooleanToStringTransformerTest.php @@ -0,0 +1,79 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Sonata\AdminBundle\Tests\Form\DataTransformer; + +use PHPUnit\Framework\TestCase; +use Sonata\AdminBundle\Form\DataTransformer\BooleanToStringTransformer; + +/** + * @author Peter Gribanov + */ +final class BooleanToStringTransformerTest extends TestCase +{ + public function provideTransform(): array + { + return [ + [null, null, '1'], + [false, null, '1'], + [true, '1', '1'], + [true, 'true', 'true'], + [true, 'yes', 'yes'], + [true, 'on', 'on'], + ]; + } + + /** + * @dataProvider provideTransform + */ + public function testTransform($value, ?string $expected, string $trueValue) + { + $transformer = new BooleanToStringTransformer($trueValue); + + $this->assertSame($expected, $transformer->transform($value)); + } + + public function provideReverseTransform(): array + { + return [ + [null, false], + [true, true], + [1, true], + ['1', true], + ['true', true], + ['yes', true], + ['on', true], + [false, false], + [0, false], + ['0', false], + ['false', false], + ['no', false], + ['off', false], + ['', false], + // invalid values + ['foo', false], + [new \DateTime(), false], + [PHP_INT_MAX, false], + ]; + } + + /** + * @dataProvider provideReverseTransform + */ + public function testReverseTransform($value, bool $expected) + { + $transformer = new BooleanToStringTransformer('1'); + + $this->assertSame($expected, $transformer->reverseTransform($value)); + } +}