search

Brille24 / SyliusCustomOptionsPlugin

A Sylius plugin that adds customer options
MIT License
47 stars 34 forks source link

Extending entities breaks backend forms #84

Closed seizan8 closed 1 year ago

seizan8 commented 3 years ago

I extended the CustomerOptionValuePrice entity.

Yet, when I try to use the form in the backend I git this error when saving: Expected value of type "App\Entity\Brille24CustomerOptionsPlugin\CustomerOptionValuePrice" for association field "App\Entity\Brille24CustomerOptionsPlugin\CustomerOptionValue#$prices", got "Brille24\SyliusCustomerOptionsPlugin\Entity\CustomerOptions\CustomerOptionValuePrice" instead.

I think the issue is that Brille24\SyliusCustomerOptionsPlugin\Form\CustomerOptionValuePriceType set CustomerOptionValuePrice as default value for the data_class. So even if I extend the entity, the form data_class is still set to the old entity.

I added a form extension and tried to overwrite the configureOptions method but for some reason my extension doesn't get called. Haven't figured out why yet.

Looking at the formType I noticed that it extends AbstractType. Shouldn't it extend the AbstractResourceType? I think if the formType was structured like the Sylius\Bundle\ProductBundle\Form\Type\ProductType this issue would not be a problem. Sylius passed the entity class as parameter, so after the entity is extended and the sylius_resource config is updated the forms automatically receive the class name of the extended class.

seizan8 commented 3 years ago

Ok, so it seems Brille24\SyliusCustomerOptionsPlugin\EventListener\CustomerOptionValueListener is the problem. The addChannelPricesToNewValue method creates a new CustomerOptionValuePrice instance instead of using the factory. Not sure if the formType can cause problems too. I'll come up with a fix.

mamazu commented 1 year ago

This should be fixed with the MR, right? Closing it, feel free to reopen it if it's still an issue.

seizan8 commented 1 year ago

I don't really have time to test it. The change looks good. Yet, I don't think this will fix it as my problem was mit the form type.

That said, it's been to long. The form type Brille24\SyliusCustomerOptionsPlugin\Form\CustomerOptionValuePriceType should probably refernce the interface and not the class itself.

I quickly looked up other Sylius form types and notices they seem to use the AbstractResourceType. The data_class is set by a value passed to the constructor. So, this way the sylius entities can be extended and once the sylius_resource config is changed, the forms (and everything else) will also use the new entity.

I am not sure how much work it would be to make the customerOption entities sylius resources and I am also not sure if it would make sense. So I think using the interface would probably the easier solution.