Sage Pay Form integration - case mis-match for the SendEmail parameter?

[davesmall9xb]

I've come across a possible issue with the SendEmail option in Omnipay\SagePay\Message\Form\AuthorizeRequest

At https://github.com/thephpleague/omnipay-sagepay/blob/master/src/Message/Form/AuthorizeRequest.php#L127 the SendEmail data is set up, but it's then filtered out because of the mis-match in case sensitivity at https://github.com/thephpleague/omnipay-sagepay/blob/master/src/Message/Form/AuthorizeRequest.php#L29

So $data['SendEmail'] is set up properly but then filtered out because it's not in the $validFields array (the $validFields array contains SendEMail but not SendEmail).

I'd propose updating $validFields to allow 'SendEmail' rather than 'SendEMail', but I wanted to check this with the creators of this useful project :)

I have created a PR for this change at https://github.com/thephpleague/omnipay-sagepay/pull/140

[judgej]

Hi @davesmall9xb thanks for the heads up and the fix.

Looking at the specs (for Sage Pay Form), the correct field name to send is SendEMail. I'm guessing it is derived from "send e-mail" and converted into upper camel case, and that's how it comes out.

The Sage Pay servers may be case-insensitive to these names, so it may not ultimately matter what case is used, but that is out of our control and so we should stick to the spec.

So, I actually thinking it is this line that needs to change. And I guess we need a test too, to prove that the flag gets through to the request message.

[judgej]

It looks like the same fix is also needed here:

https://github.com/thephpleague/omnipay-sagepay/blob/master/src/Message/Form/AuthorizeRequest.php#L103

But not here:

https://github.com/thephpleague/omnipay-sagepay/blob/master/src/Message/Form/AuthorizeRequest.php#L107

Consistency, eh?

Do you fancy making that change to your fork, so it can be merged in?

[davesmall9xb]

Thanks @judgej - I have updated the PR for this change at https://github.com/thephpleague/omnipay-sagepay/pull/140

Can this be merged in, please?

[judgej]

Merged - that fix works great. Thank you!

I tried it out setting the SendEMail option to 1 (both vendor and customer emails) and received both. The customer email used the language set for the customer and referred to "your order". The email for the vendor used the language set for the vendor account and referred to the order for "a customer".

I'll try to get it tagged for release in a bit.

[judgej]

Now tagged for release:

https://github.com/thephpleague/omnipay-sagepay/releases/tag/3.2.3

I've tried it out locally, and works fine. I will work some tests in soon, when I have a little more time, and expand those tests to cover every field that can be sent over the API.

[judgej]

I'm closing this - works for me in production.