Remove response parameter from STPCustomer method to update Sources with/without Apple Pay
#1020
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Looks like a copy/paste error from f3b2d72 in #864. `STPCustomerContext` holds onto a `STPCustomer`. There are three places where it retrieves a new `STPCustomer` - and when it does it needs to update the `sources` of that customer based on the `STPCustomerContext.includeApplePaySources` boolean. The bug is that we were using the *old* `STPCustomer.allResponseFields` (via `self.customer`) instead of the `allResponseFields` property on the newly retrieved `STPCustomer` object. In isolation, it's a pretty obvious bug (once you know to look for it). There's no reason we'd want to retrieve a Customer, and then use the response data for the previous object to update the `sources`/`defaultSource` properties of the new one. There is a fourth place that updates the sources, and that's inside `setIncludeApplePaySources:`. In that case, we're updating the existing customer, and using the existing customer's `allResponseFields`, so `self.customer.allResponseFields` is correct there.
csabol-stripe
approved these changes
Sep 5, 2018
…d, use `allResponseFields` directly Better fix than the previous commit. Instead of allowing the caller to pass in the response dictionary (it might be the wrong one!) use the one in `allResponseFields`.
danj-stripe
changed the title
STPCustomer object instead of previous oneresponse parameter from STPCustomer method to update Sources with/without Apple Pay
csabol-stripe
approved these changes
Sep 18, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Looks like a copy/paste error from f3b2d72 in #864.
STPCustomerContextholds onto aSTPCustomer. There are three placeswhere it retrieves a new
STPCustomer- and when it does it needs toupdate the
sourcesof that customer based on theSTPCustomerContext.includeApplePaySourcesboolean.The bug is that we were using the old
STPCustomer.allResponseFields(via
self.customer) instead of theallResponseFieldsproperty on the newlyretrieved
STPCustomerobject.So, instead of allowing the caller to pass in the
NSDictionaryrepresenting the response, just use the one that's already on ourSTPCustomerobject that's being updated viaself.allResponseFieldsinside the method.Motivation
IOS-870
Testing
Extended the existing unit test to catch this failure, and then fixed it.