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 consistency in FieldDescriptionRegistryInterface and implementation #6104

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
86 changes: 74 additions & 12 deletions src/Admin/AbstractAdmin.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,28 +76,28 @@ abstract class AbstractAdmin implements AdminInterface, DomainObjectInterface, A
/**
* The list FieldDescription constructed from the configureListField method.
*
* @var array
* @var FieldDescriptionInterface[]
*/
protected $listFieldDescriptions = [];

/**
* The show FieldDescription constructed from the configureShowFields method.
*
* @var array
* @var FieldDescriptionInterface[]
*/
protected $showFieldDescriptions = [];

/**
* The list FieldDescription constructed from the configureFormField method.
*
* @var array
* @var FieldDescriptionInterface[]
*/
protected $formFieldDescriptions = [];

/**
* The filter FieldDescription constructed from the configureFilterField method.
*
* @var array
* @var FieldDescriptionInterface[]
*/
protected $filterFieldDescriptions = [];

Expand Down Expand Up @@ -495,7 +495,7 @@ abstract class AbstractAdmin implements AdminInterface, DomainObjectInterface, A
/**
* The list collection.
*
* @var FieldDescriptionCollection
* @var FieldDescriptionCollection|null
*/
private $list;

Expand Down Expand Up @@ -1818,7 +1818,26 @@ public function getFormFieldDescriptions()

public function getFormFieldDescription($name)
{
return $this->hasFormFieldDescription($name) ? $this->formFieldDescriptions[$name] : null;
$this->buildForm();

if (!$this->hasFormFieldDescription($name)) {
@trigger_error(sprintf(
'Calling %s() when there is no form field description is deprecated since sonata-project/admin-bundle 3.x and will throw an exception in 4.0. '.
'Use %s::hasFormFieldDescription() to know if there is a form field description.',
__METHOD__,
__CLASS__
), E_USER_DEPRECATED);
// NEXT_MAJOR : remove the previous `trigger_error()` call, the `return null` statement, uncomment the following exception and declare FieldDescriptionInterface as return type
// throw new \LogicException(sprintf(
// 'Admin "%s" has no form field description for the field %s.',
// static::class,
// $name
// ));

return null;
}

return $this->formFieldDescriptions[$name];
}

/**
Expand All @@ -1830,6 +1849,8 @@ public function getFormFieldDescription($name)
*/
public function hasFormFieldDescription($name)
{
$this->buildForm();

return \array_key_exists($name, $this->formFieldDescriptions) ? true : false;
}

Expand All @@ -1851,7 +1872,7 @@ public function removeFormFieldDescription($name)
/**
* build and return the collection of form FieldDescription.
*
* @return array collection of form FieldDescription
* @return FieldDescriptionInterface[] collection of form FieldDescription
*/
public function getShowFieldDescriptions()
{
Expand All @@ -1871,11 +1892,30 @@ public function getShowFieldDescription($name)
{
$this->buildShow();

return $this->hasShowFieldDescription($name) ? $this->showFieldDescriptions[$name] : null;
if (!$this->hasShowFieldDescription($name)) {
@trigger_error(sprintf(
'Calling %s() when there is no show field description is deprecated since sonata-project/admin-bundle 3.x and will throw an exception in 4.0. '.
'Use %s::hasFormFieldDescription() to know if there is a show field description.',
__METHOD__,
__CLASS__
), E_USER_DEPRECATED);
// NEXT_MAJOR : remove the previous `trigger_error()` call, the `return null` statement, uncomment the following exception and declare FieldDescriptionInterface as return type
// throw new \LogicException(sprintf(
// 'Admin "%s" has no show field description for the field %s.',
// static::class,
// $name
// ));

return null;
}

return $this->showFieldDescriptions[$name];
}

public function hasShowFieldDescription($name)
{
$this->buildShow();

return \array_key_exists($name, $this->showFieldDescriptions);
}

Expand All @@ -1898,6 +1938,8 @@ public function getListFieldDescriptions()

public function getListFieldDescription($name)
{
$this->buildList();

if (!$this->hasListFieldDescription($name)) {
@trigger_error(sprintf(
'Calling %s() when there is no list field description is deprecated since sonata-project/admin-bundle 3.66 and will throw an exception in 4.0. '.
Expand Down Expand Up @@ -1938,11 +1980,32 @@ public function removeListFieldDescription($name)

public function getFilterFieldDescription($name)
{
return $this->hasFilterFieldDescription($name) ? $this->filterFieldDescriptions[$name] : null;
$this->buildDatagrid();

if (!$this->hasFilterFieldDescription($name)) {
@trigger_error(sprintf(
'Calling %s() when there is no filter field description is deprecated since sonata-project/admin-bundle 3.x and will throw an exception in 4.0. '.
'Use %s::hasFilterFieldDescription() to know if there is a filter field description.',
__METHOD__,
__CLASS__
), E_USER_DEPRECATED);
// NEXT_MAJOR : remove the previous `trigger_error()` call, the `return null` statement, uncomment the following exception and declare FieldDescriptionInterface as return type
// throw new \LogicException(sprintf(
// 'Admin "%s" has no filter field description for the field %s.',
// static::class,
// $name
// ));

return null;
}

return $this->filterFieldDescriptions[$name];
}

public function hasFilterFieldDescription($name)
{
$this->buildDatagrid();

return \array_key_exists($name, $this->filterFieldDescriptions) ? true : false;
}

Expand Down Expand Up @@ -3175,8 +3238,8 @@ protected function buildShow()
return;
}

$this->show = new FieldDescriptionCollection();
$mapper = new ShowMapper($this->showBuilder, $this->show, $this);
$this->show = $this->getShowBuilder()->getBaseList();
$mapper = new ShowMapper($this->getShowBuilder(), $this->show, $this);

$this->configureShowFields($mapper);

Expand All @@ -3195,7 +3258,6 @@ protected function buildList()
}

$this->list = $this->getListBuilder()->getBaseList();

$mapper = new ListMapper($this->getListBuilder(), $this->list, $this);

if (\count($this->getBatchActions()) > 0 && $this->hasRequest() && !$this->getRequest()->isXmlHttpRequest()) {
Expand Down
51 changes: 42 additions & 9 deletions src/Admin/FieldDescriptionRegistryInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@
* Implementations should provide arrays of FieldDescriptionInterface instances.
*
* @author Thomas Rabaix <[email protected]>
*
* @method bool hasFormFieldDescription($name)
* @method void addFormFieldDescription($name, FieldDescriptionInterface $fieldDescription)
* @method void removeFormFieldDescription($name)
* @method FieldDescriptionInterface getShowFieldDescription($name)
* @method FieldDescriptionInterface[] getShowFieldDescriptions()
* @method bool hasListFieldDescription()
* @method FieldDescriptionInterface getListFieldDescription($name)
* @method FieldDescriptionInterface[] getListFieldDescriptions()
*/
interface FieldDescriptionRegistryInterface
{
Expand All @@ -29,13 +38,25 @@ interface FieldDescriptionRegistryInterface
*/
public function getFormFieldDescription($name);

// NEXT_MAJOR: Uncomment the following line.
//public function hasFormFieldDescription(string $name): bool;

// NEXT_MAJOR: Uncomment the following line.
//public function addFormFieldDescription(string $name, FieldDescriptionInterface $fieldDescription): void;

// NEXT_MAJOR: Uncomment the following line.
//public function removeFormFieldDescription(string $name): void;

/**
* Build and return the collection of form FieldDescription.
*
* @return FieldDescriptionInterface[] collection of form FieldDescription
*/
public function getFormFieldDescriptions();

// NEXT_MAJOR: Uncomment the following line.
//public function getShowFieldDescription(string $name): FieldDescriptionInterface;

/**
* Returns true if the admin has a FieldDescription with the given $name.
*
Expand All @@ -59,6 +80,15 @@ public function addShowFieldDescription($name, FieldDescriptionInterface $fieldD
*/
public function removeShowFieldDescription($name);

// NEXT_MAJOR: Uncomment the following line.
//public function getShowFieldDescriptions(): array;

// NEXT_MAJOR: Uncomment the following line.
//public function hasListFieldDescription(string $name): bool;

// NEXT_MAJOR: Uncomment the following line.
//public function getListFieldDescription(string $name): FieldDescriptionInterface;

/**
* Adds a list FieldDescription.
*
Expand All @@ -73,13 +103,25 @@ public function addListFieldDescription($name, FieldDescriptionInterface $fieldD
*/
public function removeListFieldDescription($name);

// NEXT_MAJOR: Uncomment the following line.
//public function getListFieldDescriptions(): array;

/**
* Returns a list depend on the given $object.
*
* @return FieldDescriptionCollection
*/
public function getList();

/**
* Returns a filter FieldDescription.
*
* @param string $name
*
* @return FieldDescriptionInterface|null // NEXT_MAJOR: Remove the null return type
*/
public function getFilterFieldDescription($name);

/**
* Returns true if the filter FieldDescription exists.
*
Expand Down Expand Up @@ -109,13 +151,4 @@ public function removeFilterFieldDescription($name);
* @return FieldDescriptionInterface[]
*/
public function getFilterFieldDescriptions();

/**
* Returns a filter FieldDescription.
*
* @param string $name
*
* @return FieldDescriptionInterface|null
*/
public function getFilterFieldDescription($name);
}
3 changes: 3 additions & 0 deletions src/Builder/ShowBuilderInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
*/
interface ShowBuilderInterface extends BuilderInterface
{
/**
* @return FieldDescriptionCollection
*/
public function getBaseList(array $options = []);

/**
Expand Down
16 changes: 15 additions & 1 deletion tests/Form/FormMapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
use Symfony\Component\Form\FormBuilder;
use Symfony\Component\Form\FormFactoryInterface;
use Symfony\Component\Form\ResolvedFormTypeInterface;
use Symfony\Component\Validator\Mapping\MemberMetadata;
use Symfony\Component\Validator\Validator\ValidatorInterface;

class FormMapperTest extends TestCase
{
Expand Down Expand Up @@ -60,8 +62,19 @@ protected function setUp(): void
$eventDispatcher = $this->createMock(EventDispatcherInterface::class);

$formBuilder = new FormBuilder('test', \stdClass::class, $eventDispatcher, $formFactory);
$formBuilder2 = new FormBuilder('test', \stdClass::class, $eventDispatcher, $formFactory);

$this->admin = new CleanAdmin('code', 'class', 'controller');
$formFactory->method('createNamedBuilder')->willReturn($formBuilder);
$this->contractor->method('getFormBuilder')->willReturn($formBuilder2);

$this->admin = new CleanAdmin('code', \stdClass::class, 'controller');
$this->admin->setSubject(new \stdClass());

$validator = $this->createMock(ValidatorInterface::class);
$validator
->method('getMetadataFor')
->willReturn($this->createMock(MemberMetadata::class));
$this->admin->setValidator($validator);

// NEXT_MAJOR: Remove the calls to `setFormGroups()` and `setFormTabs()`
$this->admin->setFormGroups([]);
Expand All @@ -75,6 +88,7 @@ protected function setUp(): void
});

$this->admin->setSecurityHandler($securityHandler);
$this->admin->setFormContractor($this->contractor);

$this->modelManager = $this->getMockForAbstractClass(ModelManagerInterface::class);

Expand Down
3 changes: 3 additions & 0 deletions tests/Show/ShowMapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use Sonata\AdminBundle\Model\ModelManagerInterface;
use Sonata\AdminBundle\Security\Handler\SecurityHandlerInterface;
use Sonata\AdminBundle\Show\ShowMapper;
use Sonata\AdminBundle\Tests\App\Builder\ShowBuilder;
use Sonata\AdminBundle\Tests\Fixtures\Admin\CleanAdmin;
use Sonata\AdminBundle\Translator\NoopLabelTranslatorStrategy;

Expand Down Expand Up @@ -581,6 +582,8 @@ private function cleanShowMapper(): void

$this->admin->setModelManager($modelManager);
$this->admin->setLabelTranslatorStrategy(new NoopLabelTranslatorStrategy());

$this->admin->setShowBuilder(new ShowBuilder());
}

private function getFieldDescriptionMock(?string $name = null, ?string $label = null): BaseFieldDescription
Expand Down