-
Notifications
You must be signed in to change notification settings - Fork 295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[V7] Update BTCard
#1443
Merged
Merged
[V7] Update BTCard
#1443
Changes from 10 commits
Commits
Show all changes
31 commits
Select commit
Hold shift + click to select a range
096741e
Include all properties in public init
richherrera d4db0c9
Make all properties internal
richherrera 56f49c8
Update BTCard tests
richherrera 862763a
Update BTVardClient tests
richherrera 2aa76c8
Update Integration tests
richherrera bab98ba
Update CardHelper newCard method
richherrera 54a08af
Update demo app Amex View Controller
richherrera 5cd4420
Update CHANGELOG
richherrera 36dd53f
Update V7 readme
richherrera 0e4df32
Move property docstrings to the init method
richherrera 8724b64
Add optional on postalCode docstring
richherrera f0351a3
Merge branch 'v7' into update-btcard-parameters
richherrera 6b1c65d
Fix code snippet
richherrera 7f6c766
Merge branch 'v7' into update-btcard-parameters
richherrera bc42546
Add the missing commas in the documentation example
richherrera 618dc37
Remove optional for required properties, change boolean to let
richherrera d62f43d
Make return optional for static method card helper
richherrera 727ce54
Add message when user uses optional card helper method
richherrera c96278d
Fix BTCardTests
richherrera f4628db
Fix BTCardClient tests
richherrera a578f4a
Fix BTCardClient integration tests
richherrera c127063
Remove unnecessary code sample
richherrera dfb1c5e
Address PR feedback
richherrera 8d74df5
Merge branch 'v7' into update-btcard-parameters
richherrera 759bfc6
Remove cvv note
richherrera ecd23fc
Merge branch 'v7' into update-btcard-parameters
richherrera bb9cccb
Merge branch 'v7' into update-btcard-parameters
richherrera b9744d3
Add init with cvv
richherrera 8e192df
Update amex integration test
richherrera 6ae1adb
Update docstring and empty validation
richherrera 65cadd5
Update UTs
richherrera File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on this doc string I wonder if we should have 2 inits:
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though I don't think postal code is actually required. We can confirm with the gateway and if not we should update the docstring to denote optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After confirming with the Cards team, the only required parameters are
number
,expirationMonth
,expirationYear
, andcvv
, the others are optional. Regarding overloading initializers, it’s not necessary, after the latest changes I madeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, does this mean that we no longer support CVV only nonces for validation? Based on this docs string
If you wish to create a CVV-only payment method nonce to verify a card already stored in your Vault, omit all other properties to only collect CVV.
was the intent for an overload. If this is no longer supported we should remove that note since currently this would not be possible.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Jax, for the heads up! I’ve added a new initializer with CVV as the only parameter. As you mentioned, this is only to verify cards that are already stored, let me know what do you think.