Skip to content

SimplePayments: improve-error-handling#7525

Merged
retrofox merged 2 commits intofeature/simple-paymentsfrom
simple-payments/error-handling-client-side
Jul 25, 2017
Merged

SimplePayments: improve-error-handling#7525
retrofox merged 2 commits intofeature/simple-paymentsfrom
simple-payments/error-handling-client-side

Conversation

@retrofox
Copy link
Copy Markdown
Contributor

This PR request adds some checks when the response is processed in order to avoid javascript errors.

@retrofox retrofox requested a review from a team as a code owner July 25, 2017 15:18
@retrofox retrofox requested review from artpi, jhnstn, jsnajdr and lamosty July 25, 2017 15:18
@retrofox retrofox added the [Status] Needs Review This PR is ready for review. label Jul 25, 2017
@retrofox retrofox removed the request for review from a team July 25, 2017 15:19
Copy link
Copy Markdown
Contributor

@lamosty lamosty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

I would change it to server_error from unknown_error since it should only show if our server endpoints don't return proper data (ie paymentError.responseJSON or paymentResponse).

jQuery.post( PaypalExpressCheckout.getCreatePaymentEndpoint( blogId ), payload )
.done( function( paymentResponse ) {
if ( ! paymentResponse ) {
PaypalExpressCheckout.showError( PaypalExpressCheckout.processErrorMessage(), domId );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cause an error :(
PaypalExpressCheckout.processErrorMessage currently expects the first argument to be an object: https://github.com/Automattic/jetpack/pull/7525/files#diff-ad5f66f7ce71d416f68444397b6e59cdR91

Just need to update processErrorMessage. This should work var error = errorResponse && errorResponse.responseJSON;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice catch
454ce2d

@retrofox
Copy link
Copy Markdown
Contributor Author

I would change it to server_error from unknown_error since it should only show if our server endpoints don't return proper data (ie paymentError.responseJSON or paymentResponse).

done

@retrofox retrofox merged commit f472eda into feature/simple-payments Jul 25, 2017
@retrofox retrofox deleted the simple-payments/error-handling-client-side branch July 25, 2017 16:34
@matticbot matticbot removed the [Status] Needs Review This PR is ready for review. label Jul 25, 2017
@jeherve jeherve added the [Feature] Pay with PayPal aka Simple Payments label Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Pay with PayPal aka Simple Payments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants