Add customer_validate event

[davefreiman]

Intended to try to minimize need for rewrites when custom validation is required for the customer.

Motivation
Analyzed 5996 Magento 1.x Extensions and determined the most common rewrites and hope to add events to Magento2 to make these rewrites unnecessary (https://gist.github.com/dfry22/474f727495228baccf28).
Nearly 1 in 100 extensions was rewriting Mage_Customer_Model_Customer, and after analyzing a large sample, found that many were rewriting the validation.

Solution
Dispatch customer_validate event and pass the errors array so it can be modified by custom validators and passed back into the function. This allows for localized validation and any other custom customer validation

[alankent]

Just wanted to drop a note of thanks for all your efforts here tracking down good events to add.

[kandy]

It's so simple add events in any place of code, but generally better to refactor code. For example in this case will be better to extract validate method in separate class (ex. CustomerValidator { validate(Customer); })

[fooman]

@kandy being simple is the point here. In a round about way you just confirmed that adding the event is a good thing.

We are at developer RC now, I have also heard that Magento considers the Customer module "done". If someone had implemented an interceptor and you went ahead with the better refactor to CustomerValidator { validate(Customer) the developer's code would likely break. If the developer had used the above new event, chances are high that during the refactor the event will survive in the new location.

As you said adding events is simple, refactoring not so much. So with the code being what it is at now and how much time there is available the reality might be the added event vs. a refactor that did not happen.

[kandy]

@fooman, We promise to be backward compatible for classes and methods annotated with @api.
If we add event now, we cannot remove it after refactoring.
Also this event no need at all, because you can use interceptor for same purpose.

[davefreiman]

@kandy The inspiration behind the event here is to avoid the need for interception. If I understand correctly, it is still possible (though less likely) for plugins to conflict. The event triggered here was designed to remove the need for interception here. Since there were many extensions that rewrote this class in Magento 1.x, according to our research here: https://gist.github.com/dfry22/474f727495228baccf28, I thought that triggering an event would help remove the potential for conflicts down the road.

[fooman]

@kandy in addition to what David mentioned we are at developer rc and I don't see an @api

Agreed that once the event is added it shouldn't be removed.

[kandy]

@dfry22, Is no difference between plugin and events from "module conflict" point of view. Moreover I believe plugins should replace events at all (except place where introduced new SPI).

[vpelipenko]

CR: passed
Builds: green
Resolution: OK to merge
Internal ticket: MAGETWO-36734

[maksek]

@dfry22, @fooman Thanks for contribution. Hope it will resolve pain points from M1 :)

[davefreiman]

@maksek Thanks for all your help!

[orlangur]

@dfry22,

@kandy The inspiration behind the event here is to avoid the need for interception. If I understand correctly, it is still possible (though less likely) for plugins to conflict. The event triggered here was designed to remove the need for interception here.

Interception was introduced, among other benefits, to avoid extension conflict by design. Manual clearly states that

A plug-in changes behavior of an original class, but does not change a class itself. Because they can be called sequentially, according to a configured sort order, these plug-ins do not conflict.
Interception ensures that conflicting extensions run without intervention.

Are there any other downsides in using interceptors instead of adding events besides conflicting aspect? As to me, there is no need to avoid interception at all.

In general, it's much easier to add dispatch event in whatever place instead of refactoring of code so that interceptor could be written to extend behavior. In this particular place on the first glance it looks like no refactoring needed at all, after plugin could be already written for that method to achieve the same goal.

Thus, in my humble opinion, no new events must be added at all as everything what could be achieved by new event introduction can be accomplished via interception.

[fooman]

@orlangur Don't get me wrong I think Interception will be of great benefit to Magento however I believe interception will really only come into its own once the promised @api notices start appearing. Currently while refactoring code changes still happen on a big scale I think an explicit event has a higher probability to still be around after a refactor.

In this particular example how would you unset an error that you don't want to have triggered with a plugin (for example you don't care if you have the first name or not)? Without an event you currently would need to duplicate code to work out why a previous validate in the chain failed.

I have some concerns around this comment

In general, it's much easier to add dispatch event in whatever place instead of refactoring of code so that interceptor could be written to extend behavior.

The dispatched event is a lot easier to read for a future developer looking at the code to understand the intend that this is place for extension. Again the @api annotations will help but I think using events+interception will allow you to say "hey here is a piece of code where we like to allow developers to extend from" + also allowing you to possibly refactor this code (ie not having to use @api on every public method).