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

Introduce field description factory #6854

Merged
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
12 changes: 12 additions & 0 deletions UPGRADE-3.x.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,25 @@ UPGRADE 3.x
UPGRADE FROM 3.89 to 3.90
=========================

### Deprecated `Sonata\AdminBundle\Guesser\TypeGuesserInterface` interface.

Use `Sonata\AdminBundle\FieldDescription\TypeGuesserInterface` interface instead.

### Deprecated `Sonata\AdminBundle\Guesser\TypeGuesserChain` class.

Use `Sonata\AdminBundle\FieldDescription\TypeGuesserChain` class instead.

### Deprecated `Sonata\AdminBundle\Model\ModelManagerInterface::getModelInstance()` method.

Use `Sonata\AdminBundle\Admin\AbstractAdmin::createNewInstance()` method instead.

UPGRADE FROM 3.88 to 3.89
=========================

### Deprecated `Sonata\AdminBundle\Model\ModelManager::getNewFieldDescriptionInstance()` method.

This method has been deprecated in favor of `FieldFactoryInterface::create()`.

### Deprecated overriding `AbstractAdmin::getNewInstance()`.

Use `AbstractAdmin::alterNewInstance()` instead.
Expand Down
34 changes: 24 additions & 10 deletions src/Admin/AbstractAdmin.php
Original file line number Diff line number Diff line change
Expand Up @@ -746,10 +746,8 @@ public function buildDatagrid()
if ($this->hasListFieldDescription($filterParameters['_sort_by'])) {
$filterParameters['_sort_by'] = $this->getListFieldDescription($filterParameters['_sort_by']);
} else {
$filterParameters['_sort_by'] = $this->getModelManager()->getNewFieldDescriptionInstance(
$this->getClass(),
$filterParameters['_sort_by'],
[]
$filterParameters['_sort_by'] = $this->createFieldDescription(
$filterParameters['_sort_by']
);

$this->getListBuilder()->buildField(null, $filterParameters['_sort_by'], $this);
Expand Down Expand Up @@ -2892,6 +2890,26 @@ final public function hasTemplateRegistry(): bool
return null !== $this->templateRegistry;
}

final public function createFieldDescription(string $propertyName, array $options = []): FieldDescriptionInterface
Copy link
Member

Choose a reason for hiding this comment

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

Can you provide type hints for the $options?

{
$fieldDescriptionFactory = $this->getFieldDescriptionFactory();

// NEXT_MAJOR: Remove the "if" block and leave the "else" one.
if (null === $fieldDescriptionFactory) {
$fieldDescription = $this->getModelManager()->getNewFieldDescriptionInstance(
$this->getClass(),
$propertyName,
$options
);
} else {
$fieldDescription = $fieldDescriptionFactory->create($this->getClass(), $propertyName, $options);
}

$fieldDescription->setAdmin($this);

return $fieldDescription;
}

/**
* @phpstan-return T
*/
Expand Down Expand Up @@ -3096,8 +3114,7 @@ protected function buildList()
$mapper = new ListMapper($this->getListBuilder(), $this->list, $this);

if (\count($this->getBatchActions()) > 0 && $this->hasRequest() && !$this->getRequest()->isXmlHttpRequest()) {
$fieldDescription = $this->getModelManager()->getNewFieldDescriptionInstance(
$this->getClass(),
$fieldDescription = $this->createFieldDescription(
'batch',
[
'label' => 'batch',
Expand All @@ -3107,7 +3124,6 @@ protected function buildList()
]
);

$fieldDescription->setAdmin($this);
Copy link
Member

Choose a reason for hiding this comment

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

You have to keep this, no ?

// NEXT_MAJOR: Remove this line and use commented line below it instead
$fieldDescription->setTemplate($this->getTemplate('batch'));
// $fieldDescription->setTemplate($this->getTemplateRegistry()->getTemplate('batch'));
Expand All @@ -3122,8 +3138,7 @@ protected function buildList()
}

if ($this->hasRequest() && $this->getRequest()->isXmlHttpRequest()) {
$fieldDescription = $this->getModelManager()->getNewFieldDescriptionInstance(
$this->getClass(),
$fieldDescription = $this->createFieldDescription(
'select',
[
'label' => false,
Expand All @@ -3133,7 +3148,6 @@ protected function buildList()
]
);

$fieldDescription->setAdmin($this);
Copy link
Member

Choose a reason for hiding this comment

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

Same

// NEXT_MAJOR: Remove this line and use commented line below it instead
$fieldDescription->setTemplate($this->getTemplate('select'));
// $fieldDescription->setTemplate($this->getTemplateRegistry()->getTemplate('select'));
Expand Down
4 changes: 4 additions & 0 deletions src/Admin/AdminInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
* @method string|null getParentAssociationMapping()
* @method void reorderFormGroup(string $group, array $keys)
* @method void defineFormBuilder(FormBuilderInterface $formBuilder)
* @method FieldDescriptionInterface createFieldDescription(string $propertyName, array $options = [])
*
* @phpstan-template T of object
* @phpstan-extends AccessRegistryInterface<T>
Expand Down Expand Up @@ -789,6 +790,9 @@ public function getListMode();
// * the getFormBuilder is only call by the main admin class.
// */
// public function defineFormBuilder(FormBuilderInterface $formBuilder): void;

// NEXT_MAJOR: uncomment this method in 4.0
// public function createFieldDescription(string $propertyName, array $options = []): FieldDescriptionInterface;
}

class_exists(\Sonata\Form\Validator\ErrorElement::class);
5 changes: 5 additions & 0 deletions src/Admin/BaseFieldDescription.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@
*
* @author Thomas Rabaix <[email protected]>
*
* @psalm-import-type FieldDescriptionOptions from FieldDescriptionInterface
*
* @method void setFieldMapping(array $fieldMapping)
* @method void setAssociationMapping(array $associationMapping)
* @method void setParentAssociationMappings(array $parentAssociationMappings)
Expand Down Expand Up @@ -141,6 +143,9 @@ abstract class BaseFieldDescription implements FieldDescriptionInterface

/**
* NEXT_MAJOR: Remove the null default value for $name and restrict param type to `string`.
*
* @psalm-param FieldDescriptionOptions $options
* @phpstan-param array<string, mixed> $options
*/
public function __construct(
?string $name = null,
Expand Down
42 changes: 42 additions & 0 deletions src/Admin/FieldDescriptionInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,46 @@
namespace Sonata\AdminBundle\Admin;

use Sonata\AdminBundle\Exception\NoValueException;
use Symfony\Component\Form\DataTransformerInterface;
use Symfony\Component\PropertyAccess\PropertyPathInterface;

/**
* @author Thomas Rabaix <[email protected]>
*
* @psalm-type FieldDescriptionOptions = array{
* accessor?: string|callable|PropertyPathInterface,
* actions?: array,
* admin_code?: string,
* associated_property?: string,
* block_name?: string,
* catalogue?: string,
* data_transformer?: DataTransformerInterface,
* edit?: string,
* editable?: bool,
* field_name?: string,
* field_options?: array,
* field_type?: string,
* header_class?: string,
* identifier?: bool,
* inline?: string,
* label?: bool|string|null,
* link_parameters?: array,
* multiple?: bool,
* placeholder?: string,
* required?: bool,
* role?: string|string[],
* route?: array,
* safe?: bool,
* sort_field_mapping?: array,
* sort_parent_association_mappings?: array,
* sortable?: bool,
* template?: string,
* timezone?: string|\DateTimeZone,
* translation_domain?: string,
* type?: string,
* virtual_field?: bool
* }
*
* @method string|null getTargetModel()
* @method bool hasAdmin()
* @method bool hasParent()
Expand Down Expand Up @@ -105,13 +141,19 @@ public function setOption($name, $value);
* - template.
*
* Then the value are copied across to the related property value
*
* @psalm-param FieldDescriptionOptions $options
* @phpstan-param array<string, mixed> $options
*/
public function setOptions(array $options);

/**
* Returns options.
*
* @return array<string, mixed> options
*
* @psalm-return FieldDescriptionOptions
* @phpstan-return array<string, mixed>
*/
public function getOptions();

Expand Down
18 changes: 13 additions & 5 deletions src/Datagrid/DatagridMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,19 @@ public function add(
$filterOptions['field_name'] = substr(strrchr('.'.$name, '.'), 1);
}

$fieldDescription = $this->admin->getModelManager()->getNewFieldDescriptionInstance(
$this->admin->getClass(),
$name,
array_merge($filterOptions, $fieldDescriptionOptions)
);
// NEXT_MAJOR: Remove the check and use `createFieldDescription`.
if (method_exists($this->admin, 'createFieldDescription')) {
$fieldDescription = $this->admin->createFieldDescription(
$name,
array_merge($filterOptions, $fieldDescriptionOptions)
);
} else {
$fieldDescription = $this->admin->getModelManager()->getNewFieldDescriptionInstance(
$this->admin->getClass(),
$name,
array_merge($filterOptions, $fieldDescriptionOptions)
);
}
} else {
throw new \TypeError(
'Unknown field name in datagrid mapper.'
Expand Down
18 changes: 13 additions & 5 deletions src/Datagrid/ListMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,19 @@ public function add($name, $type = null, array $fieldDescriptionOptions = [])
));
}

$fieldDescription = $this->admin->getModelManager()->getNewFieldDescriptionInstance(
$this->admin->getClass(),
$name,
$fieldDescriptionOptions
);
// NEXT_MAJOR: Remove the check and use `createFieldDescription`.
if (method_exists($this->admin, 'createFieldDescription')) {
$fieldDescription = $this->admin->createFieldDescription(
$name,
$fieldDescriptionOptions
);
} else {
$fieldDescription = $this->admin->getModelManager()->getNewFieldDescriptionInstance(
$this->admin->getClass(),
$name,
$fieldDescriptionOptions
);
}
} else {
throw new \TypeError(
'Unknown field name in list mapper.'
Expand Down
32 changes: 32 additions & 0 deletions src/DependencyInjection/Admin/AbstractTaggedAdmin.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
use Sonata\AdminBundle\Builder\ShowBuilderInterface;
use Sonata\AdminBundle\Datagrid\Pager;
use Sonata\AdminBundle\Exporter\DataSourceInterface;
use Sonata\AdminBundle\FieldDescription\FieldDescriptionFactoryInterface;
use Sonata\AdminBundle\Filter\Persister\FilterPersisterInterface;
use Sonata\AdminBundle\Model\ModelManagerInterface;
use Sonata\AdminBundle\Route\RouteGeneratorInterface;
Expand Down Expand Up @@ -202,6 +203,11 @@ abstract class AbstractTaggedAdmin implements TaggedAdminInterface
*/
protected $labelTranslatorStrategy;

/**
* @var FieldDescriptionFactoryInterface|null
*/
private $fieldDescriptionFactory;

/**
* NEXT_MAJOR: Change signature to __construct(string $code, string $class, string $baseControllerName).
*
Expand Down Expand Up @@ -423,6 +429,32 @@ public function getDataSource(): ?DataSourceInterface
return $this->dataSource;
}

final public function setFieldDescriptionFactory(FieldDescriptionFactoryInterface $fieldDescriptionFactory): void
{
$this->fieldDescriptionFactory = $fieldDescriptionFactory;
}

/**
* NEXT_MAJOR: Change typehint for FieldDescriptionFactoryInterface.
*/
public function getFieldDescriptionFactory(): ?FieldDescriptionFactoryInterface
{
if (null === $this->fieldDescriptionFactory) {
// NEXT_MAJOR: Remove this deprecation and uncomment the following exception
@trigger_error(sprintf(
'Calling %s() when no field description factory is set is deprecated since sonata-project/admin-bundle 3.x'
.' and will throw a LogicException in 4.0',
__METHOD__,
), \E_USER_DEPRECATED);
// throw new \LogicException(sprintf(
// 'Admin "%s" has no field description factory.',
// static::class
// ));
}

return $this->fieldDescriptionFactory;
}

/**
* @final since sonata-admin/admin-bundle 3.84
*/
Expand Down
51 changes: 32 additions & 19 deletions src/DependencyInjection/Admin/TaggedAdminInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use Sonata\AdminBundle\Builder\RouteBuilderInterface;
use Sonata\AdminBundle\Builder\ShowBuilderInterface;
use Sonata\AdminBundle\Exporter\DataSourceInterface;
use Sonata\AdminBundle\FieldDescription\FieldDescriptionFactoryInterface;
use Sonata\AdminBundle\Filter\Persister\FilterPersisterInterface;
use Sonata\AdminBundle\Model\ModelManagerInterface;
use Sonata\AdminBundle\Route\RouteGeneratorInterface;
Expand All @@ -41,25 +42,27 @@
* - The first and third argument are automatically injected by the AddDependencyCallsCompilerPass.
* - The second one is used as a reference of the Admin in the Pool, with the `setAdminClasses` call.
*
* @method void initialize()
* @method void setLabel(?string $label)
* @method void showMosaicButton(bool $isShown)
* @method void setPagerType(string $pagerType)
* @method string getPagerType()
* @method void setManagerType(string $managerType)
* @method void setSecurityInformation(array $information)
* @method void setFilterPersister(?FilterPersisterInterface $filterPersister = null)
* @method FilterPersisterInterface|null getFilterPersister()
* @method bool hasFilterPersister()
* @method void setModelManager(ModelManagerInterface $modelManager)
* @method void setDataSource(DataSourceInterface $dataSource)
* @method DataSourceInterface getDataSource()
* @method FormContractorInterface getFormContractor()
* @method void setShowBuilder(ShowBuilderInterface $showBuilder)
* @method ShowBuilderInterface getShowBuilder()
* @method Pool getConfigurationPool()
* @method void setRouteGenerator(RouteGeneratorInterface $routeGenerator)
* @method RouteGeneratorInterface getRouteGenerator()
* @method void initialize()
Copy link
Member

Choose a reason for hiding this comment

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

All of these method annotations look weird 🧐

Copy link
Member

Choose a reason for hiding this comment

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

why ?

Copy link
Member

Choose a reason for hiding this comment

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

Just an idea for all these methods...

Can we add a new NextMajorTaggedAdminInterface with all these methods and mark it as @internal.

We can then use @mixin NextMajorTaggedAdminInterface to include all these methods.

* @method void setLabel(?string $label)
* @method void showMosaicButton(bool $isShown)
* @method void setPagerType(string $pagerType)
* @method string getPagerType()
* @method void setManagerType(string $managerType)
* @method void setSecurityInformation(array $information)
* @method void setFilterPersister(?FilterPersisterInterface $filterPersister = null)
* @method FilterPersisterInterface|null getFilterPersister()
* @method bool hasFilterPersister()
* @method void setModelManager(ModelManagerInterface $modelManager)
* @method void setDataSource(DataSourceInterface $dataSource)
* @method DataSourceInterface getDataSource()
* @method void setFieldDescriptionFactory(FieldDescriptionFactoryInterface $fieldDescriptionFactory)
* @method FieldDescriptionFactoryInterface getFieldDescriptionFactory()
* @method FormContractorInterface getFormContractor()
* @method void setShowBuilder(ShowBuilderInterface $showBuilder)
* @method ShowBuilderInterface getShowBuilder()
* @method Pool getConfigurationPool()
* @method void setRouteGenerator(RouteGeneratorInterface $routeGenerator)
* @method RouteGeneratorInterface getRouteGenerator()
*/
interface TaggedAdminInterface
{
Expand Down Expand Up @@ -168,6 +171,16 @@ public function getModelManager();
*/
// public function getDataSource(): DataSourceInterface;

/**
* NEXT_MAJOR: Uncomment this method.
*/
// public function setFieldDescriptionFactory(FieldDescriptionFactoryInterface $fieldDescriptionFactory): void;

/**
* NEXT_MAJOR: Uncomment this method.
*/
// public function getFieldDescriptionFactory(): FieldDescriptionFactoryInterface;

/**
* @return void
*/
Expand Down
Loading