Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Handle change to logged-in status in checkout API AJAX requests #3429

Merged
merged 10 commits into from Dec 2, 2020

Conversation

haszari
Copy link
Member

@haszari haszari commented Nov 19, 2020

Related #3363 #3423

The checkout API can sometimes create a new user account and log that user in – the Create an account? option.

If there are other errors that prevent checkout from succeeding (e.g. invalid coupon - see #3363), this can leave the checkout in a slightly broken state. The core of the issue is that the checkout block and state isn't updated to reflect the fact that the user is logged in. Also the API response includes a stale nonce, because the nonce is generated early in request handling, before account creation.

The goal of this PR is to fix these issues, and generally make the checkout UX robust to user login each time the checkout is submitted (i.e. after any AJAX request).

  • Ensure the response includes a valid nonce.
  • Trigger changes to state/props so the UX updates appropriately. For example:
    • Hide the Create an account? checkbox if user is now logged in.
    • Show a notice informing the user that their account was created, even though the order submission failed.

This PR is currently based off #3454. They are closely related; this PR depends on the updated nonce from #3454 when a new account is created. When #3454 is merged, this should be rebased to trunk (and retested).

How to test the changes in this Pull Request:

  1. Make a coupon with an email restriction e.g. *@legit.com.
  2. In logged-out browser/user, add a product to cart, proceed to checkout.
  3. Enter coupon code.
  4. Enter an invalid email for coupon e.g. try@sneaky.com
  5. Check Create an account? option and complete checkout fields.
  6. Submit checkout.
  7. Confirm that checkout shows appropriate feedback:
  • Invalid coupon has been removed.
  • New user account created - Create an account checkbox is gone + notice.
  1. Submit checkout again - should succeed.

"New account" notice

Note - I couldn't get a regular notice working, it seems to only display one at a time. Snackbar style works, and maybe is a good fit for this notice. @LevinMedia can you review this notice UX and wording and advise? I don't think the notice is essential (could be handled as an improvement later), so we could back out of this if needed.

Here are some ideas for wording for the notice:

  • Congratulations! You have signed up for a user account.
  • Welcome! Your user account has been created.
  • variations of the above with email / username

Screenshot walkthrough

Screen Capture on 2020-11-27 at 14-05-21

Changelog

Fix – Checkout block: Correctly handle cases where the order fails with an error (e.g. invalid coupon) and a new user account is created.

@haszari
Copy link
Member Author

haszari commented Nov 19, 2020

Hide the Create an account? checkbox if user is now logged in.

Started looking at this and got stuck. Here's what's happening:

  • Shopper adds an invalid coupon to checkout.
  • Shopper checks Create an account? = YES.
  • Shopper submits order - POST /checkout.
    • Checkout::get_route_post_response():
    • Server creates user account, logs user in. ($create_account->from_order_request())
    • Coupon fails validation, error thrown, error response returned. (OrderController::validate_order_before_payment())

(Note I haven't pushed any code related to this.)

I think what we need to do is return the new customer user ID in the checkout response, alongside the coupon error. But that seems problematic - it's a sign that the request has potential side effects, and this complicates response handling.

FYI @nerrad @mikejolley for thoughts - maybe we need to circle back to the issue and decide how best to handle this (my initial attempt doesn't feel like a good solution).

@mikejolley
Copy link
Member

I think what we need to do is return the new customer user ID in the checkout response, alongside the coupon error. But that seems problematic - it's a sign that the request has potential side effects, and this complicates response handling.

I think this sounds like the thing to do; it would be handled kind of like the cart updates. Once the customer ID is added in state/context, the rest of the UI can react and hide things like login prompts etc.

@haszari haszari force-pushed the fix/checkout-handle-ajax-login-change branch from 3df268c to 0743162 Compare November 20, 2020 00:40
@haszari
Copy link
Member Author

haszari commented Nov 20, 2020

I think what we need to do is return the new customer user ID in the checkout response, alongside the coupon error. But that seems problematic - it's a sign that the request has potential side effects, and this complicates response handling.
I think this sounds like the thing to do; it would be handled kind of like the cart updates. Once the customer ID is added in state/context, the rest of the UI can react and hide things like login prompts etc.

Thanks for the hint @mikejolley – I can see now that cart API does something similar, and checkout supports updating the cart alongside an error response.

I've got a rough prototype working in this PR now, but there's a bit of work to do to get it polished. Feel free to give me an early code review if you like.

The challenge for the checkout API (as I see it) is that the current logic allows anything to throw a RouteException to stop processing and return an error. Now we need to change things so there's one main processing path (e.g. get_route_update_response()), which checks for errors along the way, and controls whether subsequent processing is skipped. I've currently special-cased this for coupons, but if we like this approach I think we might need to generalise so all the other sub-routines return errors instead of throwing. Totally open to ideas for how best to organise this. The crux of it is that the state we need to return will be built up along the way in get_route_update_response(), and if an error happens, we need to return the state we have, and the error info.

@haszari haszari force-pushed the fix/checkout-handle-ajax-login-change branch from 6f20b7b to 59f9ba5 Compare November 26, 2020 21:20
@haszari haszari changed the base branch from trunk to fix/checkout-signup-order-processing-logic November 26, 2020 21:47
@haszari
Copy link
Member Author

haszari commented Nov 26, 2020

Rebased and retargeted this off #3454 for now, so this can use the customer ID and nonce returned with error responses.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 26, 2020

Size Change: +534 B (0%)

Total Size: 1.12 MB

Filename Size Change
build/cart-frontend.js 71.8 kB +116 B (0%)
build/cart.js 40.3 kB +113 B (0%)
build/checkout-frontend.js 86.9 kB +117 B (0%)
build/checkout.js 42.7 kB +112 B (0%)
build/vendors.js 410 kB +76 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/active-filters-frontend.js 8.89 kB 0 B
build/active-filters.js 8.94 kB 0 B
build/all-products-frontend.js 31.3 kB 0 B
build/all-products.js 36.3 kB 0 B
build/all-reviews.js 9.79 kB 0 B
build/atomic-block-components/add-to-cart--atomic-block-components/button.js 3.29 kB 0 B
build/atomic-block-components/add-to-cart--atomic-block-components/image--atomic-block-components/title.js 335 B 0 B
build/atomic-block-components/add-to-cart-frontend.js 9.03 kB 0 B
build/atomic-block-components/add-to-cart.js 7.53 kB 0 B
build/atomic-block-components/button-frontend.js 2.31 kB 0 B
build/atomic-block-components/button.js 837 B 0 B
build/atomic-block-components/category-list-frontend.js 468 B 0 B
build/atomic-block-components/category-list.js 477 B 0 B
build/atomic-block-components/image-frontend.js 1.7 kB 0 B
build/atomic-block-components/image.js 1.15 kB 0 B
build/atomic-block-components/price-frontend.js 2.29 kB 0 B
build/atomic-block-components/price.js 2.32 kB 0 B
build/atomic-block-components/rating-frontend.js 523 B 0 B
build/atomic-block-components/rating.js 527 B 0 B
build/atomic-block-components/sale-badge-frontend.js 858 B 0 B
build/atomic-block-components/sale-badge.js 863 B 0 B
build/atomic-block-components/sku-frontend.js 389 B 0 B
build/atomic-block-components/sku.js 395 B 0 B
build/atomic-block-components/stock-indicator-frontend.js 568 B 0 B
build/atomic-block-components/stock-indicator.js 573 B 0 B
build/atomic-block-components/summary-frontend.js 916 B 0 B
build/atomic-block-components/summary.js 926 B 0 B
build/atomic-block-components/tag-list-frontend.js 467 B 0 B
build/atomic-block-components/tag-list.js 474 B 0 B
build/atomic-block-components/title-frontend.js 1.23 kB 0 B
build/atomic-block-components/title.js 1.06 kB 0 B
build/attribute-filter-frontend.js 18.2 kB 0 B
build/attribute-filter.js 12.5 kB 0 B
build/blocks.js 3.54 kB 0 B
build/editor-rtl.css 13.9 kB 0 B
build/editor.css 13.9 kB 0 B
build/featured-category.js 7.73 kB 0 B
build/featured-product.js 9.97 kB 0 B
build/handpicked-products.js 7.36 kB 0 B
build/price-filter-frontend.js 14.9 kB 0 B
build/price-filter.js 10.4 kB 0 B
build/product-best-sellers.js 7.44 kB 0 B
build/product-categories.js 3.23 kB 0 B
build/product-category.js 8.38 kB 0 B
build/product-new.js 7.61 kB 0 B
build/product-on-sale.js 7.99 kB 0 B
build/product-search.js 3.56 kB 0 B
build/product-tag.js 6.53 kB 0 B
build/product-top-rated.js 7.58 kB 0 B
build/products-by-attribute.js 8.34 kB 0 B
build/reviews-by-category.js 11.8 kB 0 B
build/reviews-by-product.js 13.4 kB 0 B
build/reviews-frontend.js 9.38 kB 0 B
build/single-product-frontend.js 33.8 kB 0 B
build/single-product.js 10.1 kB 0 B
build/style-rtl.css 18.4 kB 0 B
build/style.css 18.4 kB 0 B
build/vendors--atomic-block-components/price-frontend.js 5.65 kB 0 B
build/vendors-style-rtl.css 1.03 kB 0 B
build/vendors-style.css 1.03 kB 0 B
build/wc-blocks-data.js 7.1 kB 0 B
build/wc-blocks-middleware.js 931 B 0 B
build/wc-blocks-registry.js 2.39 kB 0 B
build/wc-payment-method-bacs.js 775 B 0 B
build/wc-payment-method-cheque.js 771 B 0 B
build/wc-payment-method-cod.js 866 B 0 B
build/wc-payment-method-paypal.js 813 B 0 B
build/wc-payment-method-stripe.js 12.1 kB 0 B
build/wc-settings.js 2.59 kB 0 B
build/wc-shared-context.js 1.53 kB 0 B
build/wc-shared-hocs.js 1.66 kB 0 B

compressed-size-action

@mikejolley mikejolley self-assigned this Dec 2, 2020
Base automatically changed from fix/checkout-signup-order-processing-logic to trunk December 2, 2020 12:22
- return coupon errors in a property
- don't throw from coupon validation:
  - return info about errors to route handler
  - tweak logic in route handler to prevent subsequent processing
  - default payment result to fail to avoid accidental successful checkout
- in client, catch errors and new customer id:
  - render any errors as notices - i.e. coupon error
  - if a customer ID is included, push into store (so UI updates)
- reinstate thrown exception when validating order
- return exception was an experiment, now solved in #3454
…response:

- update store with new user id
- remove stale response.errors handling;
  - current approach (#3454) is to add data to error response
@mikejolley mikejolley force-pushed the fix/checkout-handle-ajax-login-change branch from b6a1d86 to 0d1f403 Compare December 2, 2020 12:51
@mikejolley
Copy link
Member

This PR introduced a snack bar notice and "errors" array in responses (unused?). I'm removing this code so as not to block it being merged. So long as we ensure nonces and user IDs are updated in the client, the checkout can react to these changes accordingly.

Copy link
Member

@mikejolley mikejolley left a comment

Choose a reason for hiding this comment

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

I've removed the code that wasn't needed and swapped the user_id in body for the new header. Tested with a coupon that failed after account creation and it showed the notice, updated the checkout view (hiding the account signup box), and updated nonces. Resubmission worked as normal.

@mikejolley mikejolley merged commit 4e25cfd into trunk Dec 2, 2020
@mikejolley mikejolley deleted the fix/checkout-handle-ajax-login-change branch December 2, 2020 14:27
senadir pushed a commit that referenced this pull request Dec 2, 2020
* refactor and reorder checkout processing

* improve handling of checkout POST with mixed success:
- return coupon errors in a property
- don't throw from coupon validation:
  - return info about errors to route handler
  - tweak logic in route handler to prevent subsequent processing
  - default payment result to fail to avoid accidental successful checkout
- in client, catch errors and new customer id:
  - render any errors as notices - i.e. coupon error
  - if a customer ID is included, push into store (so UI updates)

* fix linter whitespace issue from rebase merge

* fix MIA order validation/errors (due to rebase):
- reinstate thrown exception when validating order
- return exception was an experiment, now solved in #3454

* hide "Create account" checkbox if account is created during an error response:
- update store with new user id
- remove stale response.errors handling;
  - current approach (#3454) is to add data to error response

* show a notice informing user that they have signed up

* white space

* Handle header and update typedef

* Remove "errors" schema

* remove errors

Co-authored-by: Mike Jolley <mike.jolley@me.com>
@mikejolley mikejolley added this to the 4.0.0 milestone Dec 7, 2020
@mikejolley mikejolley added the type: bug The issue/PR concerns a confirmed bug. label Dec 7, 2020
@mikejolley mikejolley mentioned this pull request Dec 7, 2020
20 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug The issue/PR concerns a confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants