From 9f91d3b6ceb7b09e0630e0e2872112285d91fcbf Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Thu, 4 Feb 2021 16:29:03 +0100 Subject: [PATCH 1/3] Add accessor property instead of code --- UPGRADE-3.x.md | 4 ++++ docs/reference/action_list.rst | 10 ++++++++++ src/Admin/BaseFieldDescription.php | 20 ++++++++++++++++--- tests/Admin/BaseFieldDescriptionTest.php | 25 ++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 3 deletions(-) diff --git a/UPGRADE-3.x.md b/UPGRADE-3.x.md index b1750d450d..9019163f04 100644 --- a/UPGRADE-3.x.md +++ b/UPGRADE-3.x.md @@ -34,6 +34,10 @@ final class MyAdmin extends AbstractAdmin } ``` +### Deprecated the `Sonata\AdminBundle\AdminFieldDescription` `'code'` option. + +Use the `accessor` option instead. + ### Deprecated `Sonata\AdminBundle\Admin\AbstractAdmin::formOptions` property. This property has been replaced by the new method `Sonata\AdminBundle\Admin\AbstractAdmin::configureFormOptions()` diff --git a/docs/reference/action_list.rst b/docs/reference/action_list.rst index 674b6ce595..033d960dd9 100644 --- a/docs/reference/action_list.rst +++ b/docs/reference/action_list.rst @@ -99,6 +99,16 @@ Here is an example:: // specific properties of a relation to the entity ->add('image.name') + // you may also use a custom accessor + ->add('description1', null, [ + 'accessor' => 'getDescription' + ]) + ->add('description2', null, [ + 'accessor' => function ($subject) { + return $this->customService->formatDescription($subject); + } + ]) + // You may also specify the actions you want to be displayed in the list ->add('_action', null, [ 'actions' => [ diff --git a/src/Admin/BaseFieldDescription.php b/src/Admin/BaseFieldDescription.php index 3def42383e..db509fd28c 100644 --- a/src/Admin/BaseFieldDescription.php +++ b/src/Admin/BaseFieldDescription.php @@ -36,7 +36,7 @@ * - name (o) : the name used (label in the form, title in the list) * - link_parameters (o) : add link parameter to the related Admin class when * the Admin.generateUrl is called - * - code : the method name to retrieve the related value + * - accessor : the method or the method name to retrieve the related value * - associated_tostring : (deprecated, use associated_property option) * the method to retrieve the "string" representation * of the collection element. @@ -418,9 +418,23 @@ public function getFieldValue($object, $fieldName) return $this->getFieldValue($child, substr($fieldName, $dotPos + 1)); } - // prefer method name given in the code option + // NEXT_MAJOR: Remove this code. if ($this->getOption('code')) { - $getter = $this->getOption('code'); + @trigger_error( + 'The "code" option is deprecated since sonata-project/admin-bundle 3.x.' + .' Use the "accessor" code instead', + \E_USER_DEPRECATED + ); + } + + // prefer method name given in the code option + // NEXT_MAJOR: Remove this line and uncomment the following + $getter = $this->getOption('accessor', $this->getOption('code')); +// $getter = $this->getOption('accessor'); + if ($getter) { + if (\is_callable($getter)) { + return $getter($object); + } if (!method_exists($object, $getter)) { @trigger_error( diff --git a/tests/Admin/BaseFieldDescriptionTest.php b/tests/Admin/BaseFieldDescriptionTest.php index d131f24b31..b9c26a2f67 100644 --- a/tests/Admin/BaseFieldDescriptionTest.php +++ b/tests/Admin/BaseFieldDescriptionTest.php @@ -186,6 +186,31 @@ public function testGetFieldValueWithNullObject(): void $this->assertNull($description->getFieldValue(null, 'fake')); } + public function testGetFieldValueWithAccessor(): void + { + $description = new FieldDescription('name', ['accessor' => 'getFoo']); + $mock = $this->getMockBuilder(\stdClass::class)->addMethods(['getFoo'])->getMock(); + $mock->expects($this->once())->method('getFoo')->willReturn(42); + $this->assertSame(42, $description->getFieldValue($mock, 'fake')); + } + + public function testGetFieldValueWithCallableAccessor(): void + { + $description = new FieldDescription('name', [ + 'accessor' => static function (object $object): int { + return $object->getFoo(); + }, + ]); + $mock = $this->getMockBuilder(\stdClass::class)->addMethods(['getFoo'])->getMock(); + $mock->expects($this->once())->method('getFoo')->willReturn(42); + $this->assertSame(42, $description->getFieldValue($mock, 'fake')); + } + + /** + * NEXT_MAJOR: Remove this test. + * + * @group legacy + */ public function testGetFieldValueWithCode(): void { $description = new FieldDescription('name', ['code' => 'getFoo']); From 6dee4e4f58bf3128c6519448625ee6f146127e5e Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Mon, 8 Feb 2021 17:28:32 +0100 Subject: [PATCH 2/3] Prefer property path instead of method name --- src/Admin/BaseFieldDescription.php | 56 +++++++++--------------- tests/Admin/BaseFieldDescriptionTest.php | 2 +- 2 files changed, 21 insertions(+), 37 deletions(-) diff --git a/src/Admin/BaseFieldDescription.php b/src/Admin/BaseFieldDescription.php index db509fd28c..fd05832c58 100644 --- a/src/Admin/BaseFieldDescription.php +++ b/src/Admin/BaseFieldDescription.php @@ -18,6 +18,7 @@ use Sonata\AdminBundle\Exception\NoValueException; use Symfony\Component\PropertyAccess\Exception\ExceptionInterface; use Symfony\Component\PropertyAccess\PropertyAccess; +use Symfony\Component\PropertyAccess\PropertyPathInterface; /** * A FieldDescription hold the information about a field. A typical @@ -36,7 +37,7 @@ * - name (o) : the name used (label in the form, title in the list) * - link_parameters (o) : add link parameter to the related Admin class when * the Admin.generateUrl is called - * - accessor : the method or the method name to retrieve the related value + * - accessor : the method or the property path to retrieve the related value * - associated_tostring : (deprecated, use associated_property option) * the method to retrieve the "string" representation * of the collection element. @@ -425,47 +426,30 @@ public function getFieldValue($object, $fieldName) .' Use the "accessor" code instead', \E_USER_DEPRECATED ); - } - - // prefer method name given in the code option - // NEXT_MAJOR: Remove this line and uncomment the following - $getter = $this->getOption('accessor', $this->getOption('code')); -// $getter = $this->getOption('accessor'); - if ($getter) { - if (\is_callable($getter)) { - return $getter($object); - } - - if (!method_exists($object, $getter)) { - @trigger_error( - 'Passing a non-existing method in the "code" option is deprecated' - .' since sonata-project/admin-bundle 3.x and will throw an exception in 4.0.', - \E_USER_DEPRECATED - ); - // NEXT_MAJOR: Remove the deprecation and uncomment the next line. -// throw new \LogicException('The method "%s"() does not exist.', $getter); - } elseif (!\is_callable([$object, $getter])) { + $getter = $this->getOption('code'); + if ($this->getOption('parameters')) { @trigger_error( - 'Passing a non-callable method in the "code" option is deprecated' - .' since sonata-project/admin-bundle 3.x and will throw an exception in 4.0.', + 'The option "parameters" is deprecated since sonata-project/admin-bundle 3.x and will be removed in 4.0.', \E_USER_DEPRECATED ); - // NEXT_MAJOR: Remove the deprecation and uncomment the next line. -// throw new \LogicException('The method "%s"() does not have public access.', $getter); - } else { - if ($this->getOption('parameters')) { - @trigger_error( - 'The option "parameters" is deprecated since sonata-project/admin-bundle 3.x and will be removed in 4.0.', - \E_USER_DEPRECATED - ); + return $object->{$getter}(...$this->getOption('parameters')); + } - return $object->{$getter}(...$this->getOption('parameters')); - } + return $object->{$getter}(); + } - return $object->{$getter}(); - } + // prefer the method or the property path given in the code option + $accessor = $this->getOption('accessor', $fieldName); + if (\is_callable($accessor)) { + return $accessor($object); + } elseif (!\is_string($accessor) && !$accessor instanceof PropertyPathInterface) { + throw new \TypeError(sprintf( + 'The option "accessor" must be a string, a callable or a %s, %s given.', + PropertyPathInterface::class, + \is_object($accessor) ? 'instance of '.\get_class($accessor) : \gettype($accessor) + )); } // NEXT_MAJOR: Remove the condition code and the else part @@ -475,7 +459,7 @@ public function getFieldValue($object, $fieldName) ->getPropertyAccessor(); try { - return $propertyAccesor->getValue($object, $fieldName); + return $propertyAccesor->getValue($object, $accessor); } catch (ExceptionInterface $exception) { throw new NoValueException( sprintf('Cannot access property "%s" in class "%s".', $this->getName(), \get_class($object)), diff --git a/tests/Admin/BaseFieldDescriptionTest.php b/tests/Admin/BaseFieldDescriptionTest.php index b9c26a2f67..2dfcfa5b90 100644 --- a/tests/Admin/BaseFieldDescriptionTest.php +++ b/tests/Admin/BaseFieldDescriptionTest.php @@ -188,7 +188,7 @@ public function testGetFieldValueWithNullObject(): void public function testGetFieldValueWithAccessor(): void { - $description = new FieldDescription('name', ['accessor' => 'getFoo']); + $description = new FieldDescription('name', ['accessor' => 'foo']); $mock = $this->getMockBuilder(\stdClass::class)->addMethods(['getFoo'])->getMock(); $mock->expects($this->once())->method('getFoo')->willReturn(42); $this->assertSame(42, $description->getFieldValue($mock, 'fake')); From bf8376069d7cfb7df15f839f83bedcdb31a265ef Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Mon, 8 Feb 2021 18:02:45 +0100 Subject: [PATCH 3/3] Update doc --- docs/reference/action_list.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/action_list.rst b/docs/reference/action_list.rst index 033d960dd9..f658462be0 100644 --- a/docs/reference/action_list.rst +++ b/docs/reference/action_list.rst @@ -101,7 +101,7 @@ Here is an example:: // you may also use a custom accessor ->add('description1', null, [ - 'accessor' => 'getDescription' + 'accessor' => 'description' ]) ->add('description2', null, [ 'accessor' => function ($subject) {