Simple Payments: update error message handling by jhnstn · Pull Request #7503 · Automattic/jetpack

jhnstn · 2017-07-23T23:12:54Z

This updates how we handle error messaging from the wpcom REST api.
It replaces paypal.request.post with JQuery.post. The change allows better api error handling such as paypal REST api errors or disabled buttons. The way paypal.request.post handles response errors is too specific and only exposes errors as strings ( including stringifying json data).

Screenshots
Custom error message from wpcom api ( in this case , the button did not have a valid currency )

Displaying multiple errors from calling WP_Error::add

Default error message. The message is from the client

Testing

Apply D6514-code , sandbox
Verify no regression errors for valid payment flows
Introduce a paypal api error ( suggestion: set the currency code on a button to null )
The error message displayed should be from the wpcom rest api.
Try buying with a free JP site as well

replaces paypal.request.post

artpi · 2017-07-24T10:28:15Z

I:

made backend for this backend for this D6514-code
Checked plan
moved success message to server
stoped sending payload
fixed some linting

lamosty · 2017-07-24T11:54:24Z

modules/simple-payments/paypal-express-checkout.js

+		if ( error.additional_errors ) {
+			var messages = [];
+			error.additional_errors.forEach( function( error ) {
+				messages.push( '<p>' + error.message.toString() + '</p>' );


We should probably guard against undefined message since that will error if we call toString() on that.

lamosty · 2017-07-24T11:55:40Z

modules/simple-payments/paypal-express-checkout.js

 		if ( ! paypal ) {
 			throw new Error( 'PayPal module is required by PaypalExpressCheckout' );
 		}
+		if ( ! jQuery ) {


Hmm, this should normally not happen. We require jQuery as a dependency of this script. Also, not sure we should error the whole page if jQuery is not defined.

lamosty · 2017-07-24T12:01:03Z

modules/simple-payments/paypal-express-checkout.js

+						.fail( function( paymentError ) {
+							var errorMessage = PaypalExpressCheckout.processErrorMessage( paymentError );
+							PaypalExpressCheckout.showError( errorMessage, buttonDomId );
+							reject( new Error( paymentError.responseJSON.code ) );


I would not throw a new JS error. If we get some error on backend, we should inform the user by a front-end message, not a JS error I think.

I would say throwing JS errors should be used for showing some issues with the JS code itself.

We cannot get rid of those since Paypal promise system needs an error

Yeah, that's also how they do it in the library itself: https://github.com/paypal/paypal-checkout/blob/9a3cd0d068a77f1ca7baa5a132b32ceb4248fabb/test/tests/button/error.js#L108

Let's keep it for now.

I added the error specifically for what Artur noticed in the paypal script. It's useless IMO but it's better than the "Expected reject to be called with Error" bit. I think that is actually coming from the Promise library that the paypal script is using.

This reverts commit 3765425.

This reverts commit b692df6.

lamosty

Working for me.

- display success message from backend ( D6514-code ) - display all error messages

@iamtakashi

* Introduce Jetpack_Simple_Payments class, load it * Add doc about format to CPT * Add more docs * Add enabled / disabled status * Add shortcode * reg shortcode * Add shortcode logic * Fix func * fix class * rename to product * Better content passing * Pass proper JS * Add comments * Rename to "paypal-express-checkout" * Add price * Label change * jshint fixes * filter * items * Throw error if paypal not defined * Info about script hosted with PP * Get rid of static keyword * Fix some lint issues * proper header * Adjust classes to WP coding standards * Change header * Change comment * add payload for number of items * Make ids work for paypal code * repair init code * Introduce ID for form * Add endpoint urls * Better domain * Endpoint urls with blog_id * Fix id * Add version * Change endpoint * Make sandbox * Add post meta syncing * Sandbox management * Review fixes * remove console Attempt to make test pass in JS * Fir for missing product * simple-payments: print message when the transaction ends. * simple-payments: clean message placeholder * simple-payments: implement styles to the block * some change * simple-payments: improve showing transaction messages * Put the purchase message into HTML, hidden by default * Toggle purchase messages with either success/error and message * Use p tag instead of div with margin Props @iamtakashi for the suggestion. * Remove unneeded code * Fix typo * Fix paypal checkout.js script error * Use .html instead of text to append html content * Removing one error message: there's no error to show, uneeded * Fix toggle logic of the purchase message * fix linting issue * update success copy * simple-payments: small coding improvements in output_shortcode() * simple-payments: do not use underscore in css class https://make.wordpress.org/core/handbook/best-practices/coding-standards/css/#selectors * simple-payments: minor improvements defining endpoints * simple-payments: use `<p>` tag for description and price #7500 (comment) * simple-payments: minor improvements * simple-payments: add purchase-box container. improve paypal button styles. * Simple Payments: update error message handling (#7503) - display success message from backend ( D6514-code ) - display all error messages * simple-pyaments: wrap all text into `<p>` elems * simple-payments: move globals as PaypalExpressCheckout properties * simple-payments: coding improvementes. * simple-payments: minor code improvements * rebase: squash -> "simple-payments: move globals as PaypalExpressCheckout properties" * Remove wpautop for Simple Payments product content/description (#7507) * Simple payments: image (#7508) Introduces an image to a payment button * Disable default sandboxing * simple-payments: user camel-case convention * reenable sandbox * Add env handling * Simple Payments: Style shortcode * simple-payments: inject msgs in the right place * simple-payments: show error in onAuthorize callback * fix additional error messages * Simple Payments: Modify message style * fix empty join call * Simple Payments: Reset iframe margin * wrap default error message * Implement review feedback * Add min field * SimplePayments: add formatted price meta data (#7521) Output `spay_formatted_price` if present * somple-payments: improve-error-handling * simple-payments: check processErrorMessage() param * Fixed lint errors. * Simple Payments: return wpcom blog_id if code is running on wpcom (#7524) * Simple Payments: add method to process success messages (#7528) Fixes styling of success messages.

jhnstn added 2 commits July 23, 2017 12:10

add error processing helper

33b2679

update to use jQuery post

464cb10

replaces paypal.request.post

jhnstn requested review from artpi, jsnajdr, lamosty and retrofox July 23, 2017 23:13

jhnstn added the [Status] Needs Review This PR is ready for review. label Jul 23, 2017

artpi added 2 commits July 24, 2017 10:36

small fixes

219e395

Move message to server

c46005a

lamosty reviewed Jul 24, 2017

View reviewed changes

artpi added 8 commits July 24, 2017 14:23

Review fixes

f643fc8

More review fixes

c1a5b9c

bring back errors

91de0fc

Add catching errors log method

b692df6

promise try 2

3765425

Revert "promise try 2"

4327ed4

This reverts commit 3765425.

Revert "Add catching errors log method"

a2ca3f1

This reverts commit b692df6.

Make minimal number 1

4ea73eb

lamosty approved these changes Jul 24, 2017

View reviewed changes

artpi merged commit e88ea38 into feature/simple-payments Jul 24, 2017

matticbot removed the [Status] Needs Review This PR is ready for review. label Jul 24, 2017

artpi deleted the simple-payments/error-messages branch July 24, 2017 13:39

artpi pushed a commit that referenced this pull request Jul 24, 2017

Simple Payments: update error message handling (#7503)

5e0d810

- display success message from backend ( D6514-code ) - display all error messages

jeherve added the Simple Payments label Sep 7, 2017

jeherve added the [Feature] Pay with PayPal aka Simple Payments label Aug 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simple Payments: update error message handling#7503

Simple Payments: update error message handling#7503
artpi merged 12 commits intofeature/simple-paymentsfrom
simple-payments/error-messages

jhnstn commented Jul 23, 2017 •

edited by lancewillett

Loading

Uh oh!

artpi commented Jul 24, 2017

Uh oh!

lamosty Jul 24, 2017

Uh oh!

lamosty Jul 24, 2017

Uh oh!

lamosty Jul 24, 2017

Uh oh!

artpi Jul 24, 2017

Uh oh!

artpi Jul 24, 2017

Uh oh!

lamosty Jul 24, 2017

Uh oh!

jhnstn Jul 24, 2017

Uh oh!

lamosty left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

jhnstn commented Jul 23, 2017 • edited by lancewillett Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

artpi commented Jul 24, 2017

Uh oh!

lamosty Jul 24, 2017

Choose a reason for hiding this comment

Uh oh!

lamosty Jul 24, 2017

Choose a reason for hiding this comment

Uh oh!

lamosty Jul 24, 2017

Choose a reason for hiding this comment

Uh oh!

artpi Jul 24, 2017

Choose a reason for hiding this comment

Uh oh!

artpi Jul 24, 2017

Choose a reason for hiding this comment

Uh oh!

lamosty Jul 24, 2017

Choose a reason for hiding this comment

Uh oh!

jhnstn Jul 24, 2017

Choose a reason for hiding this comment

Uh oh!

lamosty left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jhnstn commented Jul 23, 2017 •

edited by lancewillett

Loading