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

REST API: Checkout/Order API #1322

Closed
mikejolley opened this issue Dec 4, 2019 · 8 comments · Fixed by #1425
Closed

REST API: Checkout/Order API #1322

mikejolley opened this issue Dec 4, 2019 · 8 comments · Fixed by #1425
Assignees
Labels
focus: rest api Work impacting REST api routes.

Comments

@mikejolley
Copy link
Member

This issue needs more in-depth exploration and planning.

We need to allow the new checkout block to create orders in WooCommerce. This is probably the most complicated endpoint as it handles data, customers, and payments. This needs additional exploration.

We need to decide when orders are created, either:

  1. When the user goes to the checkout. If we created a draft order, we'd have an order ID to reference and update throughout the process, and the final 'place order' step would be faster. We could also store data to persist it between checkouts (i.e. if the customer leaves the page). Tracking abandonment would be possible.
  2. When the customer places the order. This is how core does it now. We'd have to hold all customer data in memory, but it could be lost after page refresh unless persistent to a session somewhere.

Unanswered Questions/Notes:

  • When do we want to create the order?
  • How, or do we, protect this endpoint from tampering? What validation is needed server side?
  • Are there concurrency issues to worry about server side if orders are created in parallel by different users?
  • When is payment taken/attempted, and what context is given to the payment gateway? Currently they require an order ID, and if payment fails the order is rolled back.
  • Is the 'order summary' powered by the cart API, or the order API? (dependent on point 1)
  • When is stock reduced? Can core handle this currently, or do we want to do something ourselves to ensure stock levels are managed correctly.
@mikejolley mikejolley added the focus: rest api Work impacting REST api routes. label Dec 4, 2019
@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the label type: feature request to this issue, with a confidence of 0.95. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@nerrad
Copy link
Contributor

nerrad commented Dec 17, 2019

We talked about this in slack a bit today (p1576592217032800-slack-C8X6Q7XQU) and I'll document some answers to questions here:

When do we want to create the order?

As noted in our discussion, we felt that it would be best to create the order when we have a key piece of information in the checkout (i.e. email). It can be created in the background as the user is filling out the rest of the information.

How, or do we, protect this endpoint from tampering? What validation is needed server side?

This was not discussed (yet) but at a minimum I think the following should exist in order to create the order:

  • A valid cart session id.
  • Email (or other id that we've decided will be required before creating the order).

Some other thoughts/ideas:

  • should we implement some sort of honeypot trap in the checkout? What existing mechanisms does woo core have for preventing spam checkouts?
  • rate-limiting is usually a great mechanism as well. With the rest endpoint this may even be more necessary (but perhaps something left to server environments?)

Are there concurrency issues to worry about server side if orders are created in parallel by different users?

If orders are tied to cart/session id is this still a potential issue? Is this a problem that has some history in Woo Core already? If so, what mechanisms were introduced to help mitigate it?

When is payment taken/attempted, and what context is given to the payment gateway? Currently they require an order ID, and if payment fails the order is rolled back.

This has bearing on the the reference pull I created in #1349 (which is now linked). I think this should actually simplify things because now we can pass along the order ID along with the updated (and locked in) checkout data to the payment method for completing the payment. The payment would then notify the checkout when payment is successfully completed or not and the checkout would take care of any further processing after that. Since payment method logic is being left up to the payment method themselves, I'm hoping much of the existing logic in payment methods/core can be preserved for this stage of the process.

Is the 'order summary' powered by the cart API, or the order API? (dependent on point 1)

As soon as the order is created (after email is entered) then order summary can be updated. If there are any changes, we should alert the user. A good question from a design standpoint though, should we differentiate between the two states in the ui: Example transition from "Pending Order Summary" to "Order Summary" in the label for the order summary are in the existing mockups? (cc @garymurray for thoughts).

When is stock reduced? Can core handle this currently, or do we want to do something ourselves to ensure stock levels are managed correctly.

I guess this depends somewhat on how this happens now. Does core currently reduce stock on successful payment? Is this configured by the store owner? Are there enhancements to this that we want to implement regardless of how core does it currently (and what would those be)? I'm assuming for MVP we want to mostly mirror core?

@pmcpinto
Copy link

pmcpinto commented Dec 17, 2019

If we created a draft order, we'd have an order ID to reference and update throughout the process, and the final 'place order' step would be faster.

If we opt for creating a draft order, is this order going to be displayed in the store orders' dashboard before the shopper finishes the checkout?

And I a shopper adds a coupon code after the draft order it's created, it will be required to update that draft order, correct?

@mikejolley mikejolley self-assigned this Dec 19, 2019
@mikejolley
Copy link
Member Author

mikejolley commented Dec 19, 2019

@nerrad I'm going to start this one next and I think the way we should approach it is this..

Rather than treating it like the core REST APIs where the consumer/client passes props to create an order, we should keep the creation logic out of reach of clients and instead make it more a Cart to order conversion, only exposing things like addresses for editing.

So:

  1. /create which creates a draft order from a cart.
  2. PUT will allow editing customer data e.g. address, chosen shipping, chosen payment.

PUT is only available for draft orders. Once the order progresses, we shouldn't allow edits.

Calculations are off limits.

Status updates are off limits too. We don't want the client to be allowed to update order status as that could be abused.

The payment API (#1321?) should be passed an order ID and do updates to the order (status/payment) on the server side. Our client can poll for updates if needed.

@garymurray
Copy link

As soon as the order is created (after email is entered) then order summary can be updated. If there are any changes, we should alert the user. A good question from a design standpoint though, should we differentiate between the two states in the ui: Example transition from "Pending Order Summary" to "Order Summary" in the label for the order summary are in the existing mockups? (cc @garymurray for thoughts).

I think we can just stick with Order Summary

@nerrad
Copy link
Contributor

nerrad commented Dec 19, 2019

Rather than treating it like the core REST APIs where the consumer/client passes props to create an order, we should keep the creation logic out of reach of clients and instead make it more a Cart to order conversion, only exposing things like addresses for editing.

Agreed, writes should be strictly limited. I also agree the server should take care of converting from cart to order 👍

PUT is only available for draft orders. Once the order progresses, we shouldn't allow edits.

Hmm. Does the order data model include billing information and shipping information or are those things coupled to their own data models and just linked via order id?

Calculations are off limits.

What about coupon entry in the checkout though? What about quantity changes? What about variation selection (might not be MVP but I wonder if we should consider/think through the impact).

If you're mostly meaning by the statement that the server is the source of truth for calculations and the client doesn't do calculations and send to the server, then yes I agree.

Status updates are off limits too. We don't want the client to be allowed to update order status as that could be abused.

Agreed. Any status changes for order should only happen server-side or via authed endpoint int he backend, not through the public store/order endpoint.

The payment API (#1321?) should be passed an order ID and do updates to the order (status/payment) on the server side. Our client can poll for updates if needed.

Ya that's mostly what I thought would be happening. Receive the order ID plus any other data already in state from the order (totals, line items etc). The goal with the current work I'm doing in #1349 is to basically have the checkout pass off to the payment method what it needs to process the payment and hopefully re-use as much of the existing logic in current payment methods as there is currently (including what ajax requests they might make etc). The focus I think we should have is providing an interface that payment methods will use to communicate with the checkout (and I think I'm pretty close to cracking that nut).

@mikejolley
Copy link
Member Author

Hmm. Does the order data model include billing information and shipping information or are those things coupled to their own data models and just linked via order id?

Those would be editable and form part of the order data. I was thinking we'd just accept those as params during POST to the endpoint. That data is duplicated to the customer object also (used for defaults on next checkout).

What about coupon entry in the checkout though? What about quantity changes? What about variation selection (might not be MVP but I wonder if we should consider/think through the impact)

I don't consider those calculations. Obviously the client needs to be able to set these things, but calculation logic (i.e. totals) should be server side. This differs from the core REST API where everything is writeable.

If we need to be able to track changes to order totals as a result of these updates, we can store and return a hash based on order contents + total amounts.

@nerrad
Copy link
Contributor

nerrad commented Dec 19, 2019

Ya your current approach sounds great to me Mike! Thanks for thinking through this. I anticipate we may have to fine tune things once we get to the implementation stage, but I think you've identified some key architectural boundaries we have to keep.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
focus: rest api Work impacting REST api routes.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants