REST API - Cart Order API #1425
Conversation
mikejolley
force-pushed
the
add/checkout-api
branch
from
877d3a3 to
1b3a7c3
Compare
mikejolley
changed the title
mikejolley
changed the title
There was a problem hiding this comment.
Alrighty, overall, I think this is a great and addresses the tricky problems identified, namely:
- creating an order securely.
- allowing for stock level checks and updating those temporarily.
I've got a few comments sprinkled through the code that I think should be addressed.
Also:
- will you take care of creating followup issues for what you identified in the pull description (plus any others that might be in inline todos and should be addressed as part of the cart/checkout work)?
- we probably should take note of things to surface for wider awareness as a part of this work. In this case the new table and the introduction of a draft status. I think it'd be worthwhile to publish a p2 early about this and invite feedback from impacted teams on the approach we're taking here. We don't need to wait for feedback before merging the pull.
Co-Authored-By: Darren Ethier <[email protected]>
…lobal cart properties
There was a problem hiding this comment.
This looks great! I didn't test - just reviewed the code. I have a few comments but pre-approving. I think you mentioned you were going to add tests and update documentation?
I really liked how you broke out the ReserveStock logic into its own class. Great idea there!
| $items = array_filter( | ||
| $order->get_items(), | ||
| function( $item ) { | ||
| return $item->is_type( 'line_item' ) && $item->get_product() instanceof \WC_Product; | ||
| } | ||
| ); |
There was a problem hiding this comment.
noice! I like how this verifies all the items.
| * @param \WC_Order $order Order object. | ||
| */ | ||
| protected function reserve_stock( \WC_Order $order ) { | ||
| $reserve_stock_helper = new ReserveStock(); |
There was a problem hiding this comment.
Is it okay that this is created every time the method is called? I wonder if this should be moved to a property on the CartOrder and instantiate it on creating that instance? That way if we eventually refactor things it can be injected via the constructor. As far as I can tell ReserveStock doesn't keep state so it would likely be a single instance kept in the DIC.
There was a problem hiding this comment.
I think the overhead here is pretty low, and it's likely only going to be used/called once per request.
I see how dependency injection patterns can help in some places but I'm not fully on board with injecting things such as utility classes. Also, what happens if a dependency changes? The constructor would then change too right? Maybe a discussion outside of this :)
| class ReserveStock { | ||
| /** |
There was a problem hiding this comment.
Great idea to break this out into it's own utility class!
| if ( ! $product->is_in_stock() ) { | ||
| return new WP_Error( | ||
| 'product_out_of_stock', | ||
| sprintf( | ||
| /* translators: %s: product name */ | ||
| __( '%s is out of stock and cannot be purchased.', 'woo-gutenberg-products-block' ), | ||
| $product->get_name() | ||
| ), | ||
| [ 'status' => 403 ] | ||
| ); | ||
| } |
There was a problem hiding this comment.
I see the value here in using WP_Error because it allows for easier return of custom data. However, I'd love it if we could move to using exceptions more. We could create our own named exceptions. For instance you could have a OutOfStockException (and for use later a NotEnoughStockException) that receives the product name and status code to return. I think error handling is much more straightforward with exceptions.
With that said, I don't want to block this pull. If you agree, just comment and I'll create an issue for considering implementing custom exceptions at some point.
There was a problem hiding this comment.
Maybe worth a shot. Only concern would be over-engineering when something can be handled by a single existing error class. Might just be set in my ways.
| // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQL.NotPrepared | ||
| $wpdb->query( "REPLACE INTO {$wpdb->wc_reserved_stock} ( order_id, product_id, stock_quantity ) VALUES {$values};" ); |
There was a problem hiding this comment.
I appreciate this! Thanks :)
|
Merging this now. The only follow up I think that needs tracking is draft cleanup - Ill log it. |
Implements REST endpoints to create orders from carts. The response heavily mimics the cart endpoint meaning the client can show details from the cart or the order in the same manner.
Closes #1322
Cart Order API
Create a new order from the items in the cart.
billing_addressshipping_addresscustomer_noteshipping_ratesrate_idof selected shipping methods to add to the order.curl --request POST https://example-store.com/wp-json/wc/store/cart/orderExample response:
{ "id": 149, "number": "149", "status": "draft", "order_key": "wc_order_9falc306dOkWb", "created_via": "store-api", "prices_include_tax": true, "events": { "date_created": "2020-01-07T12:33:23", "date_created_gmt": "2020-01-07T12:33:23", "date_modified": "2020-01-07T12:33:23", "date_modified_gmt": "2020-01-07T12:33:23", "date_completed": null, "date_completed_gmt": null, "date_paid": null, "date_paid_gmt": null }, "customer": { "customer_id": 1, "customer_ip_address": "192.168.50.1", "customer_user_agent": "insomnia\/7.0.5" }, "customer_note": "This is a customer note.", "billing_address": { "first_name": "Margaret", "last_name": "Thatchcroft", "company": "", "address_1": "123 South Street", "address_2": "Apt 1", "city": "Philadelphia", "state": "PA", "postcode": "19123", "country": "US", "email": "[email protected]", "phone": "" }, "shipping_address": { "first_name": "Margaret", "last_name": "Thatchcroft", "company": "", "address_1": "123 South Street", "address_2": "Apt 1", "city": "Philadelphia", "state": "PA", "postcode": "19123", "country": "US" }, "items": [ { "id": 12, "quantity": 1, "name": "Belt", "sku": "woo-belt", "permalink": "http:\/\/local.wordpress.test\/product\/belt\/", "images": [ { "id": "41", "src": "http:\/\/local.wordpress.test\/wp-content\/uploads\/2019\/12\/belt-2.jpg", "thumbnail": "http:\/\/local.wordpress.test\/wp-content\/uploads\/2019\/12\/belt-2-300x300.jpg", "srcset": "http:\/\/local.wordpress.test\/wp-content\/uploads\/2019\/12\/belt-2.jpg 801w, http:\/\/local.wordpress.test\/wp-content\/uploads\/2019\/12\/belt-2-300x300.jpg 300w, http:\/\/local.wordpress.test\/wp-content\/uploads\/2019\/12\/belt-2-100x100.jpg 100w, http:\/\/local.wordpress.test\/wp-content\/uploads\/2019\/12\/belt-2-450x450.jpg 450w, http:\/\/local.wordpress.test\/wp-content\/uploads\/2019\/12\/belt-2-150x150.jpg 150w, http:\/\/local.wordpress.test\/wp-content\/uploads\/2019\/12\/belt-2-768x768.jpg 768w", "sizes": "(max-width: 801px) 100vw, 801px", "name": "belt-2.jpg", "alt": "" } ], "variation": [], "totals": { "currency_code": "GBP", "currency_symbol": "£", "currency_minor_unit": 2, "currency_decimal_separator": ".", "currency_thousand_separator": ",", "currency_prefix": "£", "currency_suffix": "", "line_subtotal": "4583", "line_subtotal_tax": "917", "line_total": "4583", "line_total_tax": "917" } } ], "totals": { "currency_code": "GBP", "currency_symbol": "£", "currency_minor_unit": 2, "currency_decimal_separator": ".", "currency_thousand_separator": ",", "currency_prefix": "£", "currency_suffix": "", "total_items": "4583", "total_items_tax": "917", "total_fees": "0", "total_fees_tax": "0", "total_discount": "0", "total_discount_tax": "0", "total_shipping": "499", "total_shipping_tax": "100", "total_price": "6099", "total_tax": "1017", "tax_lines": [ { "name": "Tax", "price": "1017" } ] } }Stock handling
Since we're moving to a system where orders are created upon checkout entry, we need to ensure stock is reserved during the checkout process. For this I've included a new table named
wc_reserved_stockwhich stores order id, product id, and stock quantity. This table is then referenced during order creation to ensure there is enough stock to fulfil an order.Stock reservations older than 10 mins or for non-draft orders are ignored. If this endpoint is called multiple times, stock reservations are renewed.
If there is insufficient stock available, the endpoint returns an error message and code
403. We can use this to show a message and/or redirect back to the cart.We could look into having a cleanup method for draft orders but that would likely fit into a followup. Right now drafts are harmless, but we'll need to chat about what we want doing with them if abandoned.
To test
/store/cart/order.There are some follow-up items from this that likely need looking at during the build of checkout: