- fix nested admin (one-to-one-to-many relation) that opens inline

- remove useless if branch (and some lines in phpstan baseline because of this)
-  cover all cases of this method

phpstan-baseline.neon

@@ -205,37 +205,6 @@ parameters:
             count: 1
             path: src/Form/FormMapper.php
 
-# next 6 errors are due to not installed Doctrine ORM\ODM
-        -
-            message: "#^Class Doctrine\\\\ODM\\\\MongoDB\\\\PersistentCollection not found\\.$#"
-            count: 1
-            path: src/Admin/AdminHelper.php
-
-        -
-            message: "#^Class Doctrine\\\\ORM\\\\PersistentCollection not found\\.$#"
-            count: 1
-            path: src/Admin/AdminHelper.php
-
-        -
-            message: "#^Call to method getTypeClass\\(\\) on an unknown class Doctrine\\\\ODM\\\\MongoDB\\\\PersistentCollection\\.$#"
-            count: 1
-            path: src/Admin/AdminHelper.php
-
-        -
-            message: "#^Call to method getTypeClass\\(\\) on an unknown class Doctrine\\\\ORM\\\\PersistentCollection\\.$#"
-            count: 1
-            path: src/Admin/AdminHelper.php
-
-        -
-            message: "#^Call to method add\\(\\) on an unknown class Doctrine\\\\ODM\\\\MongoDB\\\\PersistentCollection\\.$#"
-            count: 1
-            path: src/Admin/AdminHelper.php
-
-        -
-            message: "#^Call to method add\\(\\) on an unknown class Doctrine\\\\ORM\\\\PersistentCollection\\.$#"
-            count: 1
-            path: src/Admin/AdminHelper.php
-
         -
             # will be fixed in v4
             message: "#^Method Sonata\\\\AdminBundle\\\\Datagrid\\\\Pager::next\\(\\) with return type void returns mixed but should not return anything\\.$#"

src/Admin/AdminHelper.php

@@ -16,8 +16,6 @@
 use Doctrine\Common\Collections\Collection;
 use Doctrine\Inflector\Inflector;
 use Doctrine\Inflector\InflectorFactory;
-use Doctrine\ODM\MongoDB\PersistentCollection;
-use Doctrine\ORM\PersistentCollection as DoctrinePersistentCollection;
 use Sonata\AdminBundle\Exception\NoValueException;
 use Sonata\AdminBundle\Manipulator\ObjectManipulator;
 use Sonata\AdminBundle\Util\FormBuilderIterator;
@@ -184,7 +182,7 @@ public function appendFormFieldElement(AdminInterface $admin, $subject, $element
             if (\array_key_exists($childFormBuilder->getName(), $formData)) {
                 $formData = $admin->getRequest()->get($formBuilder->getName(), []);
                 $i = 0;
-                foreach ($formData[$childFormBuilder->getName()] as $name => &$field) {
+                foreach ($formData[$childFormBuilder->getName()] as &$field) {
                     $toDelete[$i] = false;
                     if (\array_key_exists(self::FORM_FIELD_DELETE, $field)) {
                         $toDelete[$i] = true;
@@ -200,27 +198,7 @@ public function appendFormFieldElement(AdminInterface $admin, $subject, $element
         $form->setData($subject);
         $form->handleRequest($admin->getRequest());
 
-        //Child form not found (probably nested one)
-        //if childFormBuilder was not found resulted in fatal error getName() method call on non object
-        if (!$childFormBuilder) {
-            $path = $this->getElementAccessPath($elementId, $subject);
-
-            $collection = $this->propertyAccessor->getValue($subject, $path);
-
-            if ($collection instanceof DoctrinePersistentCollection || $collection instanceof PersistentCollection) {
-                //since doctrine 2.4
-                $modelClassName = $collection->getTypeClass()->getName();
-            } elseif ($collection instanceof Collection) {
-                $modelClassName = $this->getEntityClassName($admin, explode('.', preg_replace('#\[\d*?\]#', '', $path)));
-            } else {
-                throw new \Exception('unknown collection class');
-            }
-
-            $collection->add(new $modelClassName());
-            $this->propertyAccessor->setValue($subject, $path, $collection);
-
-            $fieldDescription = null;
-        } else {
+        if ($childFormBuilder && $admin->hasFormFieldDescription($childFormBuilder->getName())) {
             // retrieve the FieldDescription
             $fieldDescription = $admin->getFormFieldDescription($childFormBuilder->getName());
 
@@ -252,6 +230,26 @@ public function appendFormFieldElement(AdminInterface $admin, $subject, $element
             $newInstance = ObjectManipulator::addInstance($form->getData(), $associationAdmin->getNewInstance(), $fieldDescription);
 
             $associationAdmin->setSubject($newInstance);
+        } else {
+            //Else branch is executed when child form not found (probably nested one)
+            //if childFormBuilder was not found resulted in fatal error getName() method call on non object
+            //OR if childFormBuilder was found, but admin object does not have formFieldDescription with
+            //same name as childFormBuilder
+
+            $path = $this->getElementAccessPath($elementId, $subject);
+
+            $collection = $this->propertyAccessor->getValue($subject, $path);
+
+            if ($collection instanceof Collection) {
+                $modelClassName = $this->getEntityClassName($admin, explode('.', preg_replace('#\[\d*?\]#', '', $path)));
+            } else {
+                throw new \Exception('unknown collection class');
+            }
+
+            $collection->add(new $modelClassName());
+            $this->propertyAccessor->setValue($subject, $path, $collection);
+
+            $fieldDescription = null;
         }
 
         $finalForm = $admin->getFormBuilder()->getForm();

tests/Admin/AdminHelperTest.php

@@ -13,6 +13,7 @@
 
 namespace Sonata\AdminBundle\Tests\Admin;
 
+use Doctrine\Common\Collections\ArrayCollection;
 use PHPUnit\Framework\TestCase;
 use Sonata\AdminBundle\Admin\AdminHelper;
 use Sonata\AdminBundle\Admin\AdminInterface;
@@ -235,7 +236,9 @@ public function testAppendFormFieldElement(): void
     {
         $helper = new AdminHelper($this->propertyAccessor);
 
-        $admin = $this->createStub(AdminInterface::class);
+        $admin = $this->getMockBuilder(AdminInterface::class)
+            ->addMethods(['hasFormFieldDescription'])
+            ->getMockForAbstractClass();
         $admin
             ->method('getClass')
             ->willReturn(Foo::class);
@@ -267,6 +270,12 @@ public function testAppendFormFieldElement(): void
                 'bar' => $fieldDescription,
             ]);
 
+        $admin
+            ->method('hasFormFieldDescription')
+            ->with($associationMapping['fieldName'])
+            ->willReturn(true)
+        ;
+
         $request = $this->createStub(Request::class);
         $request
             ->method('get')
@@ -339,6 +348,218 @@ public function testAppendFormFieldElement(): void
         }
     }
 
+    public function testAppendFormFieldElementWithoutFormFieldDescriptionInAdminAndNoCollectionClass(): void
+    {
+        $propertyAccessor = $this->createMock(PropertyAccessor::class);
+
+        $helper = new AdminHelper($propertyAccessor);
+
+        $admin = $this->getMockBuilder(AdminInterface::class)
+            ->addMethods(['hasFormFieldDescription'])
+            ->getMockForAbstractClass();
+        $admin
+            ->method('getClass')
+            ->willReturn(Foo::class);
+
+        $associationAdmin = $this->createMock(AdminInterface::class);
+        $associationAdmin
+            ->method('getClass')
+            ->willReturn(Bar::class);
+
+        $associationMapping = [
+            'fieldName' => 'bar',
+            'targetEntity' => Foo::class,
+            'sourceEntity' => Foo::class,
+            'isOwningSide' => false,
+        ];
+
+        $fieldDescription = $this->createStub(FieldDescriptionInterface::class);
+        $fieldDescription->method('getAssociationAdmin')->willReturn($associationAdmin);
+        $fieldDescription->method('getAssociationMapping')->willReturn($associationMapping);
+        $fieldDescription->method('getParentAssociationMappings')->willReturn([]);
+
+        $admin
+            ->method('getFormFieldDescription')
+            ->willReturn($fieldDescription);
+
+        $associationAdmin
+            ->method('getFormFieldDescriptions')
+            ->willReturn([
+                'bar' => $fieldDescription,
+            ]);
+
+        $admin
+            ->method('hasFormFieldDescription')
+            ->with($associationMapping['fieldName'])
+            ->willReturn(false)
+        ;
+
+        $request = $this->createStub(Request::class);
+        $request
+            ->method('get')
+            ->willReturn([
+                'bar' => [
+                    [
+                        'baz' => [
+                            'baz' => true,
+                        ],
+                    ],
+                    ['_delete' => true],
+                ],
+            ]);
+
+        $request->request = new ParameterBag();
+
+        $admin
+            ->method('getRequest')
+            ->willReturnOnConsecutiveCalls($request, $request, $request, null, $request, $request, $request, $request, null, $request);
+
+        $foo = $this->createMock(Foo::class);
+        $propertyAccessor
+            ->method('isReadable')
+            ->with($foo, 'bar')
+            ->willReturn(true)
+        ;
+        $admin
+            ->method('hasSubject')
+            ->willReturn(true);
+        $admin
+            ->method('getSubject')
+            ->willReturn($foo);
+
+        $dataMapper = $this->createStub(DataMapperInterface::class);
+        $formFactory = $this->createStub(FormFactoryInterface::class);
+        $eventDispatcher = $this->createStub(EventDispatcherInterface::class);
+        $formBuilder = new FormBuilder('test', \get_class($foo), $eventDispatcher, $formFactory);
+        $childFormBuilder = new FormBuilder('bar', \stdClass::class, $eventDispatcher, $formFactory);
+        $childFormBuilder->setCompound(true);
+        $childFormBuilder->setDataMapper($dataMapper);
+        $subChildFormBuilder = new FormBuilder('baz', \stdClass::class, $eventDispatcher, $formFactory);
+        $subChildFormBuilder->setCompound(true);
+        $subChildFormBuilder->setDataMapper($dataMapper);
+        $childFormBuilder->add($subChildFormBuilder);
+
+        $formBuilder->setCompound(true);
+        $formBuilder->setDataMapper($dataMapper);
+        $formBuilder->add($childFormBuilder);
+
+        $admin->method('getFormBuilder')->willReturn($formBuilder);
+
+        $this->expectException(\Exception::class);
+        $this->expectExceptionMessage('unknown collection class');
+        $helper->appendFormFieldElement($admin, $foo, 'test_bar')[1];
+    }
+
+    public function testAppendFormFieldElementWithCollection(): void
+    {
+        $propertyAccessor = $this->createMock(PropertyAccessor::class);
+
+        $helper = new AdminHelper($propertyAccessor);
+
+        $admin = $this->getMockBuilder(AdminInterface::class)
+            ->addMethods(['hasFormFieldDescription'])
+            ->getMockForAbstractClass();
+        $admin
+            ->method('getClass')
+            ->willReturn(Foo::class);
+
+        $associationAdmin = $this->createMock(AdminInterface::class);
+        $associationAdmin
+            ->method('getClass')
+            ->willReturn(Bar::class);
+
+        $associationMapping = [
+            'fieldName' => 'bar',
+            'targetEntity' => Foo::class,
+            'sourceEntity' => Foo::class,
+            'isOwningSide' => false,
+        ];
+
+        $fieldDescription = $this->createStub(FieldDescriptionInterface::class);
+        $fieldDescription->method('getAssociationAdmin')->willReturn($associationAdmin);
+        $fieldDescription->method('getAssociationMapping')->willReturn($associationMapping);
+        $fieldDescription->method('getParentAssociationMappings')->willReturn([]);
+
+        $admin
+            ->method('getFormFieldDescription')
+            ->willReturn($fieldDescription);
+
+        $associationAdmin
+            ->method('getFormFieldDescriptions')
+            ->willReturn([
+                'bar' => $fieldDescription,
+            ]);
+
+        $admin
+            ->method('hasFormFieldDescription')
+            ->with($associationMapping['fieldName'])
+            ->willReturn(false)
+        ;
+
+        $request = $this->createStub(Request::class);
+        $request
+            ->method('get')
+            ->willReturn([
+                'bar' => [
+                    [
+                        'baz' => [
+                            'baz' => true,
+                        ],
+                    ],
+                    ['_delete' => true],
+                ],
+            ]);
+
+        $request->request = new ParameterBag();
+
+        $admin
+            ->method('getRequest')
+            ->willReturnOnConsecutiveCalls($request, $request, $request, null, $request, $request, $request, $request, null, $request);
+
+        $foo = $this->createMock(Foo::class);
+        $propertyAccessor
+            ->method('isReadable')
+            ->with($foo, 'bar')
+            ->willReturn(true)
+        ;
+        $propertyAccessor
+            ->expects($this->once())
+            ->method('getValue')
+            ->with($foo, 'bar')
+            ->willReturn(new ArrayCollection())
+        ;
+        $admin
+            ->method('hasSubject')
+            ->willReturn(true);
+        $admin
+            ->method('getSubject')
+            ->willReturn($foo);
+
+        $dataMapper = $this->createStub(DataMapperInterface::class);
+        $formFactory = $this->createStub(FormFactoryInterface::class);
+        $eventDispatcher = $this->createStub(EventDispatcherInterface::class);
+        $formBuilder = new FormBuilder('test', \get_class($foo), $eventDispatcher, $formFactory);
+        $childFormBuilder = new FormBuilder('bar', \stdClass::class, $eventDispatcher, $formFactory);
+        $childFormBuilder->setCompound(true);
+        $childFormBuilder->setDataMapper($dataMapper);
+        $subChildFormBuilder = new FormBuilder('baz', \stdClass::class, $eventDispatcher, $formFactory);
+        $subChildFormBuilder->setCompound(true);
+        $subChildFormBuilder->setDataMapper($dataMapper);
+        $childFormBuilder->add($subChildFormBuilder);
+
+        $formBuilder->setCompound(true);
+        $formBuilder->setDataMapper($dataMapper);
+        $formBuilder->add($childFormBuilder);
+
+        $admin->method('getFormBuilder')->willReturn($formBuilder);
+
+        $finalForm = $helper->appendFormFieldElement($admin, $foo, 'test_bar')[1];
+
+        foreach ($finalForm->get($childFormBuilder->getName()) as $childField) {
+            $this->assertFalse($childField->has('_delete'));
+        }
+    }
+
     public function testAppendFormFieldElementNested(): void
     {
         $admin = $this->createMock(AdminInterface::class);