oroinc / customer-portal

OroCommerce package with customer portal and non authenticated visitor website base features.
Other
12 stars 15 forks source link

CustomerUser validating Customer validating all attached CustomerUser #9

Closed NicolasCayet closed 4 years ago

NicolasCayet commented 4 years ago

Hello,

I'm facing performance issue regarding CustomerUser validations when updating or creating.

It happens for CustomerUser linked to a Customer which have many CustomerUsers (more than 4500). I could face what was happening : when validating a CustomerUser, it validates its Customer, which itself checks all attached CustomerUsers validations.

I may find what was causing this : in https://github.com/oroinc/customer-portal/blob/2.6/src/Oro/Bundle/CustomerBundle/Resources/config/validation.yml, we can see for CustomerUser entity

customer:
            - NotBlank: ~
            - Valid: ~

and for Customer entity we can see

users:
            - Valid: ~

My questions are : is it really necessary to validate all CustomerUsers when validating a Customer ? Same question about validating Customer when validating a CustomerUser ? I was thinking about overriding this validation definition by removing both of them.

We are using oro/customer-portal v2.6.30 with oro/platform v2.6.38 + enterprise edition. Does this behavior change on oro/platform v3.1 ? We are planning to upgrade soon.

BR

anyt commented 4 years ago

Hi Nicolas,

Thanks for the report, obviously there is a performance issue. The @Valid constrains were added in the scope of the bugfix related to customer users import. So just dropping them is not safe in case you are using customer users import. And as I see it’s not fixed in 3.1 or master branches as you are the first one who reported it.

After quick investigation it looks like this validation must be applied only for the import context and we should consider changing @Valid constraint to a custom implementation with a better performance.

Please report it to an enterprise customer portal to track the progress, and link this issue there. Thank you

anyt commented 4 years ago

Internal ticket id #BB-18704

NicolasCayet commented 4 years ago

Thanks a lot for this quick answer.

As we have many issues around it, and we're not using and not planning to use the import functionality, I really would like to disable these two @Valid constraints on Customer and CustomerUser.

Symfony doc explains the only way to remove a constraint of a property defined in a 3rd party bundle is to define another validation for a ACME validation group, then modify the 3rd party bundle global configuration to use by default this new validation group. Could you tell me which configuration from OroCustomerBundle I should change in order to use my ACME validation group ? Is there any single config allowing this ? Or maybe there is another way to do so without any changes from the 3rd party bundle ?

anyt commented 4 years ago

This is a good question, Nicolas.

Unfortunately, for now there is no single configuration where you can change validation groups for existing entities.

So to change the group you have to override all the places where the validation is triggered. In a simple case, it's enough to create a form extension, and depending on your needs you can also need to update API endpoints configuration, override some services and so on.

NicolasCayet commented 4 years ago

So i decided to apply custom validation group in form extensions for the default types relating CustomerUser + our custom form types.

I'll report this issue to enterprise Oro Github later, linking to the current one.

Best solution could be to add specific validation groups only on @Valid constraints and then use these validations groups for import behavior to keep the bugfix you mentioned. For me, it looks weird to validate by default Customer + all attached CustomerUsers when editing one.

Thank you for your help Andrey

anyt commented 4 years ago

The fix has been merged and it will be available in the next 3.1 EE release and in 4.1.1.

rey0bs commented 4 years ago

Hi @anyt , is the fix released ? Where can I find it ? Thanks

anyt commented 4 years ago

The fix was released in 3.1.20.

NicolasCayet commented 4 years ago

FYI , using the obsolete version of customer-portal bundle, i ended up by applying patch after any composer install, removing the @Valid lines from the vendor's validation.yml

XaBe20 commented 4 years ago

Hi @anyt I have the same issue, do you have a patch ? I don't found v3.1.20

anyt commented 4 years ago

Mi miss, 3.1.20 hasn't been published yet. So the fix will be available in 3.1.20.

If you are an enterprise customer, please wait, otherwise, consider upgrading to 4.1.

rey0bs commented 4 years ago

Ok thank you

anyt commented 4 years ago

The issue was also fixed in 4.1.1.