Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add accessor property instead of code #6837

Merged
merged 3 commits into from
Feb 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions UPGRADE-3.x.md
Original file line number Diff line number Diff line change
Expand Up @@ -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()`
Expand Down
10 changes: 10 additions & 0 deletions docs/reference/action_list.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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' => 'description'
])
->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' => [
Expand Down
54 changes: 26 additions & 28 deletions src/Admin/BaseFieldDescription.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
* - code : 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.
Expand Down Expand Up @@ -418,40 +419,37 @@ 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');

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
);
@trigger_error(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since code is always set in the persistence bundles, this is always triggered: https://github.com/sonata-project/SonataDoctrineORMAdminBundle/runs/1903176971?check_suite_focus=true#step:8:108

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should remove the code in the persistence bundle.

If the name is getfoo and the fieldName is bar, because we're passing the code, the value will be $object->getFoo() instead of relying on bar ; this seems to be a bug to me.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

'The "code" option is deprecated since sonata-project/admin-bundle 3.x.'
.' Use the "accessor" code instead',
\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
Expand All @@ -461,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)),
Expand Down
25 changes: 25 additions & 0 deletions tests/Admin/BaseFieldDescriptionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,31 @@ public function testGetFieldValueWithNullObject(): void
$this->assertNull($description->getFieldValue(null, 'fake'));
}

public function testGetFieldValueWithAccessor(): void
{
$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'));
}

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']);
Expand Down