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

Add adders and remover callbacks #66

Open
zmitic opened this issue Apr 24, 2019 · 2 comments
Open

Add adders and remover callbacks #66

zmitic opened this issue Apr 24, 2019 · 2 comments
Labels

Comments

@zmitic
Copy link

zmitic commented Apr 24, 2019

When dealing with collections, using write_property_path can be problematic when dealing with non-direct relations.

Simple example; please note I am using new arrow function for readibility, recent github changes for code blocks are problematic:


Lets say we have Category<->Product many2many but with association class. But our form don't need to display assoc class, it can be much simpler:

// CategoryType
public function buildForm(FormBuilderInterface $builder, array $options): void
{
    $builder->add('products', EntityType::class, ...options...];
}

And Category entity:

public function getProducts(): array
{
    return $this->association
        ->map(fn(CategoryProductAssociation $assoc) => $assoc->getProduct())
        ->toArray();
}

public function addProduct(Product $product): void
{
    $this->assoc->add(
        new ProductCategoryAssociation($this, $product,... some extra param...)
    );
}

public function removeProduct(Product $product): void
{
    if ($assoc = $this->findAssociationToProduct($product)) {
        $this->association->removeElement($assoc);
    }
}

But when there is only setProducts(array $products)

'write_property_path' => fn($category, array $products) => $category->setProducts($products)

code would be much more complex. User would need to manually compare existing vs new values, something that Symfony already does perfectly.

Usage

New options would be:

$builder->add('products', EntityType::class, [
    'read_property_path' => fn(Category $category) => $category->getProducts(),
    // removed typehints for readibility
    'adder' => fn($category, $product) => $category->addProduct($product),
    'remover' => fn($category, $product) => $category->removeProduct($product),
]);

If adder and remover are set, then write_property_path is not required.


This has more usages when there is extra param needed, something I really need in my current app and the reason why I have to use DTO. If interested, I will put the problem here.

But in short; I have m2m relation between Client and CustomValue. CustomValue has discriminator string column type.
Entire ClientType form is made of 11 pages (flow bundle) and most of fields are working with that m2m relation but depend on value type column.

So for each value of type, I would need to write getter, adder and remover methods and make my entity totally unreadable.

With this RFC, I could change that to simple:

// ClientType.php
'read_property_path' => fn($client) => $client->getValuesOfType('xxx'),
'adder' => fn($client, $value) => $client->addValueOfType('xxx', $value),
'remover' => fn($client, $value) => $client->removeValueOfType('xxx', $value),

This way, I would need only 3 methods.

@zmitic
Copy link
Author

zmitic commented Apr 24, 2019

Just to add that I made numerous attempts of implementing it. One of them was new form-type extension like this, with bigger or lesser priority:

Note:

I removed lots of code in this example but you get the idea.

public function configureOptions(OptionsResolver $resolver): void
{
    $resolver->setDefault('adder', null);
    $resolver->setAllowedTypes('adder', ['null', Closure::class]);
    $resolver->setNormalizer('write_property_path', function (Options $options) {
        return function ($data, array $values) {
            // get original values via $option['read_property_path']
            // compare and call $options['adder'] and $options['remover']
            foreach($newValues as $newValue) { 
                $adder($data, $newValue);    
            }
            foreach($removedValues as $removedValue) { 
                $remover($data, $removedValue);    
            }
        }    
    // 
});

Using setDefault didn't work, decorating RichModelFormsTypeExtension service also failed (I had to turn-off autoconfigure for my decorator so it is not double-registered) etc.

The errors I got was either failure at this point:

throw new InvalidConfigurationException(
    'Cannot use "read_property_path" without "write_property_path".'
);

or circular dependency error by OptionsResolver. So none of my attempts can work, it needs changes in resolver.

@xabbuh xabbuh added the feature label Apr 24, 2019
@zmitic
Copy link
Author

zmitic commented Apr 24, 2019

Sorry for making a sale, but there is one more use-case I forgot but can be used in implementation.

It is when when $data is not an entity with adder and remover methods, but when I can work only with array of entities.

This happens when I have unidirectional Many2One relation. In this situation, each new entity must be manually $em->persist($newEntity) and removed entities $em->remove($removedEntity).

Example:

// ComboController.php
$data = [
    'awards' => $awardRepository->findAll(),
    // can have other entities here
];

and form, removed irrelevant parts:

$builder
    ->add('awards', BootstrapCollectionType::class, [
		// ... other options ...
        'read_property_path' => function ($data) {
            return $data['awards'];
        },
        'write_property_path' => function ($data, array $newValues) {
            $oldValues = $data['awards'];
            $this->collectionFormWriter->diff($data, $oldValues, $newValues,
					// $data is not used in this example but will be in other cases
                    function ($data, Award $award) {$this->em->persist($award);},
                    function ($data, Award $award) {$this->em->remove($award);}
                );
        },
    ]);

And this writer:

class CollectionFormWriter
{
    public function diff($data, array $oldValues, array $newValues, callable $adder, callable $remover): void
    {
        $addedValues = $this->getDiff($oldValues, $newValues);
        $removedValues = $this->getDiff($newValues, $oldValues);
        foreach ($addedValues as $addedValue) {
            $adder($data, $addedValue);
        }
        foreach ($removedValues as $removedValue) {
            $remover($data, $removedValue);
        }
    }

    private function getDiff(array $oldValues, array $newValues): array
    {
        return array_filter($newValues, static function ($newValue) use ($oldValues) {
            return !in_array($newValue, $oldValues, true);
        });
    }

This is something I am using right now, looks like everything is working. My example deals only with arrays; iterables are easy to convert.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants