[ApplePay] Take name and address from shippingAddress if present

[davidrothera]

At present if a PKPayment object contains a shippingAddress but no billingAddress then no name/address details are extracted and passed along to Stripe.

This means that when looking on the Stripe dashboard at ApplePay charges with no billingAddress that the name/address fields are empty which is still valid.

This diff changes the behaviour so that:

• If billingAddress and shippingAddress exist then we take the billingAddress
• If billingAddress is set and shippingAddress is not set then we take the billingAddress
• If shippingAddress is set and billingAddress is not set then we take the shippingAddress
• If neither are set then we don't do anything

I tested this and checked the generated URL (from the formEncodedDataForPayment method) and checked that only one copy of the name/address data was present when both shipping & billing were set.

[0thernet]

There's a lot of repeated code here. It'd be better to extract this into a function:

- (NSString *)queryStringWithAddress:(ABRecord *)record {
...
}

[davidrothera]

Agreed @benzguo, I have changed the code now to re-use a string generator method which trims down the code a little.

[0thernet]

No need to expose this in the header (I believe formEncodedDataForPayment was exposed for testing).

[davidrothera]

• Updated to re-use existing NSCharacterSet
• Removed method declaration from public header

[0thernet]

sorry, one more nit :)
you can consolidate this into two cases:

if (payment.billingAddress) {
    NSString *addressString = [self queryStringForAddress:payment.billingAddress characterSet:set];
    payloadString = [payloadString stringByAppendingString:addressString];
}
else if (payment.shippingAddress) {
    NSString *addressString = [self queryStringForAddress:payment.shippingAddress characterSet:set];
    payloadString = [payloadString stringByAppendingString:addressString];
}

thanks for catching and fixing this!

[davidrothera]

@benzguo my only reason for keeping all 3 options was so that if both the shippingAddress and the billingAddress were set then we would pick the billingAddress, doing this keeps with the same behaviour that is seen at present.

[0thernet]

whoops, just noticed that i'd swapped billingAddress and shippingAddress above (fixed now).

the if-else if above is equivalent to your three ifs. if both billingAddress and shippingAddress are non-nil, the first branch will be used – no need for a separate case.

[davidrothera]

Got it, amending now 👍

[0thernet]

🎉 thanks!