allow zero value item in basket test for discounts and escaped html c…

[dignat]

…haracters in xml basket
we added code to allow zero value items to be included in basket XML and fixed a bug , as add->Child was not escaping html characters (&). Also added tests fro no discount, for one and more discounts, and test for escaped html charaters

[coatesap]

+1 This is actually quite serious, as it prevents transactions that contain items with certain characters in the description from being processed. When this is merged, if we can get a composer version bump as well, then that would be really helpful.

[judgej]

Was going to go over it this weekend, but ran out of time. Will go through it in the next few days and give it a patch release.

Note though, that this does not remove characters that are not supported in the description field of the basket. It only turns a few characters (<>&") into entities to ensure the basket is valid XML.

The whole validation and character replacement/removal thing is a wider issue. There are validation rules on all the fields - postcodes, city, name etc. that are not caught or escaped/replaced, and those rules will differ between all the different gateway drivers. The difference, I suppose, between the address details and the basket, is that the user can be presented with the address again to amend, but they have no control over the basket at all.

[coatesap]

Hi Jason, sorry to chase (I wouldn't normally!), but is there any chance we could expediate this one? Happy to fork to my own branch / add any additional tests etc if needed - it's just I've got this running on a production environment where certain item names (which I can't change) are blocking sales entirely...

That being said - if you're stacked - just let me know and I'll point our composer at dignat's copy of the repo.

[judgej]

Yes, end of January is a busy time - tax forms and all sorts. Just use your branch for now, and I'll get to this as quickly as I can, unless someone else wants to step in?

[delatbabel]

I have recently merged a PR on omnipay-common to fix a bug that prevented 0 values from passing validation checks. That may go part way towards fixing related issues around 0 value amounts.

[coatesap]

@judgej - no worries, I've done that now, and the errors on production have cleared up.

Denise has also added me as a collaborator on her fork, so I've tweaked some of the tests, and also addressed (at least as far as the basket goes) the concerns you had about filtering out disallowed characters.

[judgej]

Brilliant - all looks great. The discounts with the quantities > 1 is a fun one. Well spotted. I'll get this released as soon as I can (assuming you can't), as soon as all my awful paperwork here is out the way.

[judgej]

Merged in - works in my tests. The preg_replace delimiter probably needs changing, but that can be done separately. I did not find any issues putting either "/" or "`" into a basket item name, so it's not looking like a serious issue.

[dignat]

Thank you so much.