Deprecate mapping setters from FieldDescriptionInterface (#6648)

Mapping should not be changed in the FieldDescriptionInterface once it is set, deprecating these methods forces to set these mapping attributes when constructing the object.
sonata-project · Dec 5, 2020 · 2122648 · 2122648
1 parent a4829f3
commit 2122648
Show file tree

Hide file tree

Showing 6 changed files with 121 additions and 13 deletions.
diff --git a/UPGRADE-3.x.md b/UPGRADE-3.x.md
@@ -4,6 +4,32 @@ UPGRADE 3.x
 UPGRADE FROM 3.xx to 3.xx
 =========================
 
+### Sonata\AdminBundle\Admin\FieldDescriptionInterface
+
+The following methods have been deprecated from the interface and will be added as abstract methods to
+`Sonata\AdminBundle\Admin\BaseFieldDescription` in the next major version:
+- `setFieldMapping()`
+- `setAssociationMapping()`
+- `setParentAssociationMappings()`
+- `setMappingType()`
+
+### Sonata\AdminBundle\Admin\BaseFieldDescription
+
+Constructor has been modified to allow 3 more parameters
+(`$fieldMapping`, `$associationMapping` and `$parentAssociationMapping`):
+
+```php
+public function __construct(
+    ?string $name = null,
+    array $options = [],
+    array $fieldMapping = [],
+    array $associationMapping = [],
+    array $parentAssociationMappings = []
+) {
+```
+
+Deprecated `Sonata\AdminBundle\Admin\BaseFieldDescription::setMappingType()`.
+
 ### Deprecated `AdminInterface::getValidator()` and  `AdminInterface::setValidator()` methods, `AbstractAdmin::$validator` property.
 
 Methods are deprecated without replacement.

diff --git a/src/Admin/BaseFieldDescription.php b/src/Admin/BaseFieldDescription.php
@@ -59,6 +59,10 @@
  *   - field_options (o): the options to give to the widget
  *
  * @author Thomas Rabaix <[email protected]>
+ *
+ * @method void setFieldMapping(array $fieldMapping)
+ * @method void setAssociationMapping(array $associationMapping)
+ * @method void setParentAssociationMappings(array $parentAssociationMappings)
  */
 abstract class BaseFieldDescription implements FieldDescriptionInterface
 {
@@ -83,17 +87,17 @@ abstract class BaseFieldDescription implements FieldDescriptionInterface
     protected $fieldName;
 
     /**
-     * @var array the ORM association mapping
+     * @var array the association mapping
      */
     protected $associationMapping = [];
 
     /**
-     * @var array the ORM field information
+     * @var array the field information
      */
     protected $fieldMapping = [];
 
     /**
-     * @var array the ORM parent mapping association
+     * @var array the parent mapping association
      */
     protected $parentAssociationMappings = [];
 
@@ -135,8 +139,13 @@ abstract class BaseFieldDescription implements FieldDescriptionInterface
     /**
      * NEXT_MAJOR: Remove the null default value and restrict param type to `string`.
      */
-    public function __construct(?string $name = null, array $options = [])
-    {
+    public function __construct(
+        ?string $name = null,
+        array $options = [],
+        array $fieldMapping = [],
+        array $associationMapping = [],
+        array $parentAssociationMappings = []
+    ) {
         // NEXT_MAJOR: Remove this check and keep the else part.
         if (null === $name) {
             @trigger_error(sprintf(
@@ -149,8 +158,25 @@ public function __construct(?string $name = null, array $options = [])
         }
 
         $this->setOptions($options);
+
+        if ([] !== $fieldMapping) {
+            $this->setFieldMapping($fieldMapping);
+        }
+
+        if ([] !== $associationMapping) {
+            $this->setAssociationMapping($associationMapping);
+        }
+
+        if ([] !== $parentAssociationMappings) {
+            $this->setParentAssociationMappings($parentAssociationMappings);
+        }
     }
 
+    // NEXT_MAJOR: Uncomment the following lines.
+    // abstract protected function setFieldMapping(array $fieldMapping): void;
+    // abstract protected function setAssociationMapping(array $associationMapping): void;
+    // abstract protected function setParentAssociationMappings(array $parentAssociationMappings): void;
+
     public function setFieldName($fieldName)
     {
         $this->fieldName = $fieldName;
@@ -438,8 +464,18 @@ public function mergeOptions(array $options = [])
         $this->setOptions(array_merge_recursive($this->options, $options));
     }
 
+    /**
+     * NEXT_MAJOR: Remove this method.
+     *
+     * @deprecated since sonata-project/admin-bundle 3.x and will be removed in 4.0.
+     */
     public function setMappingType($mappingType)
     {
+        @trigger_error(sprintf(
+            'The "%s()" method is deprecated since version 3.x and will be removed in 4.0.',
+            __METHOD__
+        ), E_USER_DEPRECATED);
+
         $this->mappingType = $mappingType;
     }
 

diff --git a/src/Admin/FieldDescriptionInterface.php b/src/Admin/FieldDescriptionInterface.php
@@ -134,6 +134,8 @@ public function getParent();
     /**
      * Define the association mapping definition.
      *
+     * @deprecated since sonata-project/admin-bundle 3.x and will be removed in 4.0.
+     *
      * @param array $associationMapping
      */
     public function setAssociationMapping($associationMapping);
@@ -166,6 +168,8 @@ public function getTargetEntity();
     /**
      * set the field mapping information.
      *
+     * @deprecated since sonata-project/admin-bundle 3.x and will be removed in 4.0.
+     *
      * @param array $fieldMapping
      */
     public function setFieldMapping($fieldMapping);
@@ -178,6 +182,8 @@ public function setFieldMapping($fieldMapping);
     public function getFieldMapping();
 
     /**
+     * @deprecated since sonata-project/admin-bundle 3.x and will be removed in 4.0.
+     *
      * set the parent association mappings information.
      */
     public function setParentAssociationMappings(array $parentAssociationMappings);
@@ -250,8 +256,12 @@ public function mergeOption($name, array $options = []);
     public function mergeOptions(array $options = []);
 
     /**
+     * NEXT_MAJOR: Remove this method.
+     *
      * set the original mapping type (only used if the field is linked to an entity).
      *
+     * @deprecated since sonata-project/admin-bundle 3.x and will be removed in 4.0.
+     *
      * @param string|int $mappingType
      */
     public function setMappingType($mappingType);

diff --git a/src/Builder/BuilderInterface.php b/src/Builder/BuilderInterface.php
@@ -22,7 +22,7 @@
 interface BuilderInterface
 {
     /**
-     * Adds missing information to the given field description from the model manager metadata, and the given admin.
+     * Adds missing information to the given field description and the given admin.
      *
      * @param AdminInterface            $admin            will be used to gather information
      * @param FieldDescriptionInterface $fieldDescription will be modified

diff --git a/tests/Admin/BaseFieldDescriptionTest.php b/tests/Admin/BaseFieldDescriptionTest.php
@@ -21,9 +21,31 @@
 use Sonata\AdminBundle\Tests\Fixtures\Entity\Foo;
 use Sonata\AdminBundle\Tests\Fixtures\Entity\FooBoolean;
 use Sonata\AdminBundle\Tests\Fixtures\Entity\FooCall;
+use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
 
 class BaseFieldDescriptionTest extends TestCase
 {
+    use ExpectDeprecationTrait;
+
+    public function testConstructingWithMapping(): void
+    {
+        $fieldMapping = ['field_name' => 'fieldName'];
+        $associationMapping = ['association_model' => 'association_bar'];
+        $parentAssociationMapping = ['parent_mapping' => 'parent_bar'];
+
+        $description = new FieldDescription(
+            'foo',
+            ['foo' => 'bar'],
+            $fieldMapping,
+            $associationMapping,
+            $parentAssociationMapping,
+        );
+
+        $this->assertSame($fieldMapping, $description->getFieldMapping());
+        $this->assertSame($associationMapping, $description->getAssociationMapping());
+        $this->assertSame($parentAssociationMapping, $description->getParentAssociationMappings());
+    }
+
     public function testSetName(): void
     {
         $description = new FieldDescription('foo');
@@ -65,9 +87,6 @@ public function testOptions(): void
 
         $this->assertCount(2, $description->getOptions());
 
-        $description->setMappingType('int');
-        $this->assertSame('int', $description->getMappingType());
-
         $this->assertSame('short_object_description_placeholder', $description->getOption('placeholder'));
         $description->setOptions(['placeholder' => false]);
         $this->assertFalse($description->getOption('placeholder'));
@@ -79,6 +98,21 @@ public function testOptions(): void
         $this->assertTrue($description->isSortable());
     }
 
+    /**
+     * NEXT_MAJOR: Remove this test.
+     *
+     * @group legacy
+     */
+    public function testSetMappingType(): void
+    {
+        $description = new FieldDescription('name');
+
+        $this->expectDeprecation('The "Sonata\AdminBundle\Admin\BaseFieldDescription::setMappingType()" method is deprecated since version 3.x and will be removed in 4.0.');
+
+        $description->setMappingType('int');
+        $this->assertSame('int', $description->getMappingType());
+    }
+
     /**
      * NEXT_MAJOR: Remove this test.
      *

diff --git a/tests/Fixtures/Admin/FieldDescription.php b/tests/Fixtures/Admin/FieldDescription.php
@@ -17,9 +17,10 @@
 
 class FieldDescription extends BaseFieldDescription
 {
+    // NEXT_MAJOR: Make this method protected.
     public function setAssociationMapping($associationMapping): void
     {
-        throw new \BadMethodCallException(sprintf('Implement %s() method.', __METHOD__));
+        $this->associationMapping = $associationMapping;
     }
 
     public function getTargetEntity(): void
@@ -32,9 +33,10 @@ public function getTargetModel(): ?string
         throw new \BadMethodCallException(sprintf('Implement %s() method.', __METHOD__));
     }
 
+    // NEXT_MAJOR: Make this method protected.
     public function setFieldMapping($fieldMapping): void
     {
-        throw new \BadMethodCallException(sprintf('Implement %s() method.', __METHOD__));
+        $this->fieldMapping = $fieldMapping;
     }
 
     public function isIdentifier(): void
@@ -43,13 +45,13 @@ public function isIdentifier(): void
     }
 
     /**
-     * set the parent association mappings information.
+     * NEXT_MAJOR: Make this method protected.
      *
      * @param array $parentAssociationMappings
      */
     public function setParentAssociationMappings(array $parentAssociationMappings): void
     {
-        throw new \BadMethodCallException(sprintf('Implement %s() method.', __METHOD__));
+        $this->parentAssociationMappings = $parentAssociationMappings;
     }
 
     /**