-
Notifications
You must be signed in to change notification settings - Fork 945
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
Show Stripe error message if error contains Stripe error #960
Conversation
83c09b7
to
84c2eee
Compare
src/util/errors.js
Outdated
return errorAPIErrors(error).reduce((messages, apiError) => { | ||
const isPaymentFailedError = | ||
apiError.status === 402 && apiError.code === ERROR_CODE_PAYMENT_FAILED; | ||
let hasMessage = apiError && apiError.meta && apiError.meta.stripeMessage; |
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.
No need to use let
since there's no need to update the variable inside its lifespan (i.e. this function block).
src/util/errors.js
Outdated
apiError.status === 402 && apiError.code === ERROR_CODE_PAYMENT_FAILED; | ||
let hasMessage = apiError && apiError.meta && apiError.meta.stripeMessage; | ||
|
||
return isPaymentFailedError && hasMessage ? [...messages, hasMessage] : messages; |
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.
This could be written as:
const hasStripeError = apiError && apiError.meta && apiError.meta.stripeMessage;
const stripeMessageMaybe = isPaymentFailedError && hasStripeError
? [apiError.meta.stripeMessage]
: [];
return [...messages, ...stripeMessageMaybe];
(And probably isPaymentFailedError
is redundant check.)
src/util/errors.js
Outdated
* Check if the given API error (from `sdk.transaction.initiate()`) is | ||
* due to other error in Stripe. | ||
*/ | ||
export const isTransactionInitiateStripeError = error => { |
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.
Since this is not returning any boolean, the name should be something else.
How about transactionInitiateOrderStripeErrors
or transactionInitiateStripeErrors
?
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.
Two small changes.
a2fea58
to
9892e2f
Compare
9892e2f
to
979eaf2
Compare
@@ -15,7 +15,9 @@ import { | |||
isTransactionInitiateAmountTooLowError, | |||
isTransactionInitiateListingNotFoundError, | |||
isTransactionInitiateMissingStripeAccountError, | |||
transactionInitiateOrderStripeErrors, |
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.
This could be moved to the end of this list. (This just looks funny in the middle of isTransaction...
helper functions.
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.
One extra small change.
979eaf2
to
24eeb90
Compare
No description provided.