Simplify Cancel Errors #296
Merged
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
sshropshire
reviewed
Nov 7, 2024
| @@ -39,7 +39,7 @@ enum CardClientError { | |||
| case malformedDeeplinkURLError | |||
|
|
|||
| /// 10. Cancellation from 3DS verification | |||
| case threeDSCancellation | |||
| case canceledError | |||
There was a problem hiding this comment.
nit: For this error threeDSecureCanceled makes sense to me, since CardClient won't always perform 3DS verification. It like how it implicitly implies that 3DS was attempted.
There was a problem hiding this comment.
I thought about this and I was opting for brevity instead of the error name being more descriptive.
The only way card processing can be canceled is in 3DS. I wanted to keep it as simple as possible
since cancellation as an error is new.
But I see your point that it is not clear that this is only for 3DS.
sshropshire
approved these changes
Nov 7, 2024
scannillo
approved these changes
Nov 7, 2024
scannillo
reviewed
Nov 7, 2024
| @@ -22,7 +23,8 @@ | |||
| * Add `approveOrder(request:completion:(CardResult?, Error?) -> Void)` to `CardClient` | |||
| * Add `vault(request:completion:(CardVaultResult?, Error?) -> Void)` to `CardClient` | |||
| * Add `approveOrder(request:) async throws -> CardResult` | |||
| * Add `vault(vaultRequest:) async throws -> CardVaultResult` | |||
| * Add `vault(vaultRequest:) async throws -> CardVaultResult` | |||
| * Add `.threeDSecureCanceled` to `CardClientError` | |||
There was a problem hiding this comment.
Nit, these are a rename vs an add. In case a merchant expects the old ones to still be there
There was a problem hiding this comment.
I wanted to show that a new async/await function was added.
I think rename might confuse them because it really is a new function.
KunJeongPark
added a commit
that referenced
this pull request
Nov 19, 2024
* Remove MXO (#291) * Remove MXO * remove Package.resolved file containing incompatible pinstorage version * CHANGELOG entry * Jax feedback and docStrings correction * Remove PatchOrder for MXO, unused files in demo app * Remove more classes used exclusively for MXO * Convert Delegation Pattern to Completion Handler (#293) * PayPal remove delgation pattern for completion handler function for checkout * lint error for paypal start * completion and analytics into notify functions * paypal vault completion, unit tests * remove PayPal delegates, references * Card vault to completion, unit tests * Card approve to completion, unit tests * docstrings for approve completion param * wrap paypal functions in async await * async await wrapper for CardClient functions * remove CardDelegate and references * fix error in notifyCheckoutFailure * Make error names and messages payPal caps consistent * changelog entires * Steven PR feedback: typo in Chagelog * Simplify Cancel Errors (#296) * Simplify cancel errors * CHANGELOG for the cancel errors * Steven PR feedback: change back CardClientError.canceled to .threeDSecureCanceled * CHANGELOG update * Cancel error handling (#297) * http performRequest returns NetworkingClientErrors * CardClient helper function for threeDSCancel, demo app cancel, demo app minor fixes * PayPalClient cancel helper functions and demo app changes * Steven PR feedback: move static helper functions to error enums * Rename CardClientError -> CardError, PayPalWebCheckoutError -> PayPalError * Steven PR feedback: return CoreSDKError in merchant completion handler * CHANGELOG and analytics typo and fix wrong code in graphql error * v2 migration guide (#295) * v2 migration guide * just cocoapods or SPM * fix typos * minor spacing changes * Update with simplified cancel errors * Steven PR feedback - diff to render green/red * include removal of delegate methods in delete block * update with cardClient threeDSecureCanceled error * change to threeDSecureCanceled in migration steps * add comment highlighting cancellation errors * typo fix * clarify separating cancel cases in errors * revert cancel handling instructions * add changes for cancellation helper methods * fix typo in PayPalError.isCheckoutCanceled * Steven PR feedback * Make PayPalError and CardError Code enum consistent (#298) * Changelog: move additive changes from breaking section (#299) * Changelog: move additive changes from breaking section * remove duplicate line for PayPal cancel errors
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 of changes
Checklist
Authors