Improve the way we append parent object if any

sonata-project · Jun 28, 2020 · 46c9d00 · 46c9d00
1 parent 5aa95f2
commit 46c9d00
Show file tree

Hide file tree

Showing 12 changed files with 308 additions and 387 deletions.
diff --git a/UPGRADE-3.x.md b/UPGRADE-3.x.md
@@ -1,6 +1,21 @@
 UPGRADE 3.x
 ===========
 
+## Deprecated `SonataAdminBundle\Admin\AdminHelper::addNewInstance`
+
+Use
+```
+$instance = $fieldDescription->getAssociationAdmin()->getNewInstance();
+SonataAdminBundle\Admin\AdminHelper::addInstance($object, $fieldDescription, $instance);
+```
+Instead of
+```
+$this->adminHelper->addNewInstance($object, $fieldDescription);
+```
+
+The static method `addInstance()` avoids the need to inject the admin helper dependency,
+and adds more flexibility with the instance you're adding to the object.
+
 UPGRADE FROM 3.68 to 3.69
 =========================
 

diff --git a/src/Admin/AbstractAdmin.php b/src/Admin/AbstractAdmin.php
@@ -1344,6 +1344,34 @@ public function getTemplate($name)
     public function getNewInstance()
     {
         $object = $this->getModelManager()->getModelInstance($this->getClass());
+
+        // Append parent object if any
+        if ($this->isChild() && $this->getParentAssociationMapping()) {
+            $parentAdmin = $this->getParent();
+            $parent = $parentAdmin->getObject($this->request->get($parentAdmin->getIdParameter()));
+
+            if (null !== $parent) {
+                $propertyAccessor = $this->getConfigurationPool()->getPropertyAccessor();
+                $propertyPath = new PropertyPath($this->getParentAssociationMapping());
+
+                $value = $propertyAccessor->getValue($object, $propertyPath);
+
+                if (\is_array($value) || $value instanceof \ArrayAccess) {
+                    $value[] = $parent;
+                    $propertyAccessor->setValue($object, $propertyPath, $value);
+                } else {
+                    $propertyAccessor->setValue($object, $propertyPath, $parent);
+                }
+            }
+        } elseif ($this->hasParentFieldDescription()) {
+            $parentAdmin = $this->getParentFieldDescription()->getAdmin();
+            $parent = $parentAdmin->getObject($this->request->get($parentAdmin->getIdParameter()));
+
+            if (null !== $parent) {
+                AdminHelper::addInstance($parent, $this->getParentFieldDescription(), $object);
+            }
+        }
+
         foreach ($this->getExtensions() as $extension) {
             $extension->alterNewInstance($this, $object);
         }
@@ -3352,26 +3380,6 @@ protected function buildForm()
 
         $this->loaded['form'] = true;
 
-        // append parent object if any
-        // todo : clean the way the Admin class can retrieve set the object
-        if ($this->isChild() && $this->getParentAssociationMapping()) {
-            $parent = $this->getParent()->getObject($this->request->get($this->getParent()->getIdParameter()));
-
-            $propertyAccessor = $this->getConfigurationPool()->getPropertyAccessor();
-            $propertyPath = new PropertyPath($this->getParentAssociationMapping());
-
-            $object = $this->getSubject();
-
-            $value = $propertyAccessor->getValue($object, $propertyPath);
-
-            if (\is_array($value) || $value instanceof \ArrayAccess) {
-                $value[] = $parent;
-                $propertyAccessor->setValue($object, $propertyPath, $value);
-            } else {
-                $propertyAccessor->setValue($object, $propertyPath, $parent);
-            }
-        }
-
         $formBuilder = $this->getFormBuilder();
         $formBuilder->addEventListener(FormEvents::POST_SUBMIT, function (FormEvent $event) {
             $this->preValidate($event->getData());

diff --git a/src/Admin/AdminHelper.php b/src/Admin/AdminHelper.php
@@ -179,16 +179,17 @@ public function appendFormFieldElement(AdminInterface $admin, $subject, $element
             $objectCount = null === $value ? 0 : \count($value);
             $postCount = \count($data[$childFormBuilder->getName()]);
 
+            $associationAdmin = $fieldDescription->getAssociationAdmin();
+
             // add new elements to the subject
             while ($objectCount < $postCount) {
                 // append a new instance into the object
-                $this->addNewInstance($form->getData(), $fieldDescription);
+                self::addInstance($form->getData(), $fieldDescription, $associationAdmin->getNewInstance());
                 ++$objectCount;
             }
 
-            $newInstance = $this->addNewInstance($form->getData(), $fieldDescription);
+            $newInstance = self::addInstance($form->getData(), $fieldDescription, $associationAdmin->getNewInstance());
 
-            $associationAdmin = $fieldDescription->getAssociationAdmin();
             $associationAdmin->setSubject($newInstance);
         }
 
@@ -213,6 +214,10 @@ public function appendFormFieldElement(AdminInterface $admin, $subject, $element
     }
 
     /**
+     * NEXT_MAJOR: remove this method.
+     *
+     * @deprecated since sonata-project/admin-bundle 3.x, use to be removed with 4.0.
+     *
      * Add a new instance to the related FieldDescriptionInterface value.
      *
      * @param object $object
@@ -223,7 +228,25 @@ public function appendFormFieldElement(AdminInterface $admin, $subject, $element
      */
     public function addNewInstance($object, FieldDescriptionInterface $fieldDescription)
     {
+        @trigger_error(sprintf(
+            'Method %s() is deprecated since sonata-project/admin-bundle 3.x. It will be removed in version 4.0.'
+            .' Use %s::addInstance() instead.',
+            __METHOD__,
+            __CLASS__
+        ), E_USER_DEPRECATED);
+
         $instance = $fieldDescription->getAssociationAdmin()->getNewInstance();
+
+        return self::addInstance($object, $fieldDescription, $instance);
+    }
+
+    /**
+     * @template T of object
+     * @phpstan-param T $instance
+     * @phpstan-return T
+     */
+    public static function addInstance(object $object, FieldDescriptionInterface $fieldDescription, object $instance): object
+    {
         $mapping = $fieldDescription->getAssociationMapping();
         $parentMappings = $fieldDescription->getParentAssociationMappings();
 

diff --git a/src/Form/Type/AdminType.php b/src/Form/Type/AdminType.php
@@ -36,22 +36,22 @@
 class AdminType extends AbstractType
 {
     /**
-     * @var AdminHelper
+     * NEXT_MAJOR: Remove this property.
+     *
+     * @var AdminHelper|null
      */
     private $adminHelper;
 
     /**
-     * NEXT_MAJOR: Allow only `AdminHelper` for argument 1 and remove the default null value.
+     * NEXT_MAJOR: Remove the __construct method.
      */
     public function __construct(?AdminHelper $adminHelper = null)
     {
-        // NEXT_MAJOR: Remove this condition.
-        if (null === $adminHelper) {
+        if (\func_num_args() > 0) {
             @trigger_error(sprintf(
-                'Calling %s without passing an %s as argument is deprecated since sonata-project/admin-bundle 3.66'
+                'Calling %s with passing an argument is deprecated since sonata-project/admin-bundle 3.x'
                 .' and will throw an exception in 4.0.',
-                __METHOD__,
-                AdminHelper::class
+                __METHOD__
             ), E_USER_DEPRECATED);
         }
 
@@ -107,10 +107,7 @@ static function (array $associationMapping): string {
                         $subject = $p->getValue($parentSubject, $parentPath.$path);
                     } catch (NoSuchIndexException $e) {
                         // no object here, we create a new one
-                        // NEXT_MAJOR: Remove the null check.
-                        if (null !== $this->adminHelper) {
-                            $subject = $this->adminHelper->addNewInstance($parentSubject, $parentFieldDescription);
-                        }
+                        $subject = AdminHelper::addInstance($parentSubject, $parentFieldDescription, $admin->getNewInstance());
                     }
                 }
             }

diff --git a/tests/Admin/AdminHelperTest.php b/tests/Admin/AdminHelperTest.php
@@ -72,6 +72,11 @@ public function testGetChildFormView(): void
         $this->assertInstanceOf(FormView::class, $this->helper->getChildFormView($formView, 'test_elementId'));
     }
 
+    /**
+     * @group legacy
+     *
+     * @expectedDeprecation Method Sonata\AdminBundle\Admin\AdminHelper::addNewInstance() is deprecated since sonata-project/admin-bundle 3.x. It will be removed in version 4.0. Use Sonata\AdminBundle\Admin\AdminHelper::addInstance() instead.
+     */
     public function testAddNewInstance(): void
     {
         $admin = $this->createMock(AdminInterface::class);
@@ -90,6 +95,11 @@ public function testAddNewInstance(): void
         $this->helper->addNewInstance($object, $fieldDescription);
     }
 
+    /**
+     * @group legacy
+     *
+     * @expectedDeprecation Method Sonata\AdminBundle\Admin\AdminHelper::addNewInstance() is deprecated since sonata-project/admin-bundle 3.x. It will be removed in version 4.0. Use Sonata\AdminBundle\Admin\AdminHelper::addInstance() instead.
+     */
     public function testAddNewInstanceWithParentAssociation(): void
     {
         $admin = $this->createMock(AdminInterface::class);
@@ -113,6 +123,11 @@ public function testAddNewInstanceWithParentAssociation(): void
         $this->helper->addNewInstance($object1, $fieldDescription);
     }
 
+    /**
+     * @group legacy
+     *
+     * @expectedDeprecation Method Sonata\AdminBundle\Admin\AdminHelper::addNewInstance() is deprecated since sonata-project/admin-bundle 3.x. It will be removed in version 4.0. Use Sonata\AdminBundle\Admin\AdminHelper::addInstance() instead.
+     */
     public function testAddNewInstancePlural(): void
     {
         $admin = $this->createMock(AdminInterface::class);
@@ -131,6 +146,11 @@ public function testAddNewInstancePlural(): void
         $this->helper->addNewInstance($object, $fieldDescription);
     }
 
+    /**
+     * @group legacy
+     *
+     * @expectedDeprecation Method Sonata\AdminBundle\Admin\AdminHelper::addNewInstance() is deprecated since sonata-project/admin-bundle 3.x. It will be removed in version 4.0. Use Sonata\AdminBundle\Admin\AdminHelper::addInstance() instead.
+     */
     public function testAddNewInstanceInflector(): void
     {
         $admin = $this->createMock(AdminInterface::class);
@@ -149,6 +169,67 @@ public function testAddNewInstanceInflector(): void
         $this->helper->addNewInstance($object, $fieldDescription);
     }
 
+    public function testAddInstance(): void
+    {
+        $fieldDescription = $this->createMock(FieldDescriptionInterface::class);
+        $fieldDescription->expects($this->once())->method('getAssociationMapping')->willReturn(['fieldName' => 'fooBar']);
+        $fieldDescription->expects($this->once())->method('getParentAssociationMappings')->willReturn([]);
+
+        $object = $this->getMockBuilder(\stdClass::class)
+            ->setMethods(['addFooBar'])
+            ->getMock();
+        $object->expects($this->once())->method('addFooBar');
+
+        AdminHelper::addInstance($object, $fieldDescription, new \stdClass());
+    }
+
+    public function testAddInstanceWithParentAssociation(): void
+    {
+        $fieldDescription = $this->createMock(FieldDescriptionInterface::class);
+        $fieldDescription->expects($this->once())->method('getAssociationMapping')->willReturn(['fieldName' => 'fooBar']);
+        $fieldDescription->expects($this->once())->method('getParentAssociationMappings')->willReturn([['fieldName' => 'parent']]);
+
+        $object2 = $this->getMockBuilder(\stdClass::class)
+            ->setMethods(['addFooBar'])
+            ->getMock();
+        $object2->expects($this->once())->method('addFooBar');
+
+        $object1 = $this->getMockBuilder(\stdClass::class)
+            ->setMethods(['getParent'])
+            ->getMock();
+        $object1->expects($this->once())->method('getParent')->willReturn($object2);
+
+        AdminHelper::addInstance($object1, $fieldDescription, new \stdClass());
+    }
+
+    public function testAddInstancePlural(): void
+    {
+        $fieldDescription = $this->createMock(FieldDescriptionInterface::class);
+        $fieldDescription->expects($this->once())->method('getAssociationMapping')->willReturn(['fieldName' => 'fooBars']);
+        $fieldDescription->expects($this->once())->method('getParentAssociationMappings')->willReturn([]);
+
+        $object = $this->getMockBuilder(\stdClass::class)
+            ->setMethods(['addFooBar'])
+            ->getMock();
+        $object->expects($this->once())->method('addFooBar');
+
+        AdminHelper::addInstance($object, $fieldDescription, new \stdClass());
+    }
+
+    public function testAddInstanceInflector(): void
+    {
+        $fieldDescription = $this->createMock(FieldDescriptionInterface::class);
+        $fieldDescription->expects($this->once())->method('getAssociationMapping')->willReturn(['fieldName' => 'entries']);
+        $fieldDescription->expects($this->once())->method('getParentAssociationMappings')->willReturn([]);
+
+        $object = $this->getMockBuilder(\stdClass::class)
+            ->setMethods(['addEntry'])
+            ->getMock();
+        $object->expects($this->once())->method('addEntry');
+
+        AdminHelper::addInstance($object, $fieldDescription, new \stdClass());
+    }
+
     public function testGetElementAccessPath(): void
     {
         $object = $this->getMockBuilder(\stdClass::class)