academe / SagePay-Integration

HTTP Messages for the Sage Pay REST (Pi) gateway.
GNU General Public License v3.0
9 stars 5 forks source link

Use SagePay field names in validation exceptions #8

Closed judgej closed 8 years ago

judgej commented 9 years ago

Not sure how this would work, but it's worth looking at.

Each field submitted to SagePay has a unique name. Those names are used to link any validation errors to the correct fields. That's find for data that has been submitted to SagePay.

Data submitted to the user needs to be put into models before it can be sent to SagePay. Sometimes the validation could fail at that point, such as a missing postcode with a country other than "IE" put into an Address object. In this case an exception is thrown. Ideally we would be able to catch the field name from that exception, e.g. billing.postalCode. These fields can map the exception to errors in the relevant fields using the same logic as for mapping SagePay validation errors.

It's a thought anyway, and I can see this type of thing already being done in the likes of Guzzle, where a HTTP exception prevents the response being set, but the response object is still available in the exception handler, somehow attached to the exception itself.

judgej commented 9 years ago

In summary this is a double-edged sword.

In previous Sage Pay APIs, in order to provide meaningful validation messages to the end user, it was necessary to catch those errors at the front end before it was sent to Sage Pay. The reason was that SagePay could only return once message for a submission.

Now Sage Pay can returns multiple validation error messages, not only for the submission as a whole, but also for each submitted field. So catching these errors too early may be less useful to the end user.

But on the other hand, these exceptions (e.g. if no state is provided when "US" is the country) are something the developer should be preventing from happening in the first place. So perhaps these validation exceptions should be treated as a developer tool with the assumption that the messages and models will always be loaded with roughly valid data in production.

judgej commented 8 years ago

Kind of dealt with under some specific actions in Issue #38