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

fix nested admin (one-to-one-to-many relation) that opens inline (fixes #6777) #6778

Merged
merged 1 commit into from
Jan 21, 2021
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
31 changes: 0 additions & 31 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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\\.$#"
Expand Down
53 changes: 29 additions & 24 deletions src/Admin/AdminHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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());

Expand Down Expand Up @@ -252,6 +230,33 @@ public function appendFormFieldElement(AdminInterface $admin, $subject, $element
$newInstance = ObjectManipulator::addInstance($form->getData(), $associationAdmin->getNewInstance(), $fieldDescription);

$associationAdmin->setSubject($newInstance);
} else {
// The `else` branch is executed when child form was 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 the admin object does not have a formFieldDescription with
// same name as `$childFormBuilder`.

$path = $this->getElementAccessPath($elementId, $subject);

$collection = $this->propertyAccessor->getValue($subject, $path);

if (!($collection instanceof Collection)) {
phansys marked this conversation as resolved.
Show resolved Hide resolved
throw new \TypeError(sprintf(
'Collection must be an instance of %s, %s given.',
Collection::class,
\is_object($collection) ? 'instance of "'.\get_class($collection).'"' : '"'.\gettype($collection).'"'
));
}

$modelClassName = $this->getEntityClassName(
$admin,
explode('.', preg_replace('#\[\d*?]#', '', $path))
);

$collection->add(new $modelClassName());
$this->propertyAccessor->setValue($subject, $path, $collection);

$fieldDescription = null;
}

$finalForm = $admin->getFormBuilder()->getForm();
Expand Down
228 changes: 225 additions & 3 deletions tests/Admin/AdminHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

namespace Sonata\AdminBundle\Tests\Admin;

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Collection;
use PHPUnit\Framework\TestCase;
use Sonata\AdminBundle\Admin\AdminHelper;
use Sonata\AdminBundle\Admin\AdminInterface;
Expand Down Expand Up @@ -235,7 +237,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);
Expand Down Expand Up @@ -267,6 +271,12 @@ public function testAppendFormFieldElement(): void
'bar' => $fieldDescription,
]);

$admin
->method('hasFormFieldDescription')
->with($associationMapping['fieldName'])
->willReturn(true)
;

$request = $this->createStub(Request::class);
$request
->method('get')
Expand Down Expand Up @@ -339,6 +349,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(\TypeError::class);
$this->expectExceptionMessage(sprintf('Collection must be an instance of %s, "%s" given.', Collection::class, \gettype(null)));
$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);
Expand Down Expand Up @@ -374,8 +596,8 @@ public function testAppendFormFieldElementNested(): void
$admin->method('getSubject')->willReturn($object);
$admin->expects($this->once())->method('getFormBuilder')->willReturn($formBuilder);

$this->expectException(\Exception::class);
$this->expectExceptionMessage('unknown collection class');
$this->expectException(\TypeError::class);
$this->expectExceptionMessage(sprintf('Collection must be an instance of %s, "string" given.', Collection::class));

$this->helper->appendFormFieldElement($admin, $object, 'uniquePartOfId_sub_object_0_and_more_0_final_data');
}
Expand Down