Repository navigation
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
damaz91
left a comment
There was a problem hiding this comment.
Thanks @arjun2075! Throwing InvalidRequestError inside runInTransaction cleanly rolls back earlier line-item stock reservations without persisting an order or poisoning idempotency_keys, bringing Node.js into full parity with #248. Both new lifecycle tests look great.
Once the commit author email / CLA check is resolved (see details on #273), this is ready to approve and merge!
Completion skipped the stock reservation for a line item whose product no
longer exists (`if (product && !reserveStock(...))`) and still created the
order, so a checkout could complete for a product that was removed from
the catalog, with nothing reserved.
Throw InvalidRequestError ("Product X not found", 400) inside the
completion transaction instead, matching the Python reference (Universal-Commerce-Protocol#248) and
the create/update paths. Earlier reservations roll back and no order is
written. Add lifecycle tests for a single deleted product and for a
deleted product on a later line.
224bbd7 to
68316bf
Compare
Problem
Completing a checkout whose line item's product had been deleted from the catalog returned 200
completedand created an order. No stock was reserved for the missing product.Root cause
The reservation loop in
completeCheckouttreated a missing product as nothing to reserve (if (product && !reserveStock(...))), so the order was still created.Fix
Reject the completion with
InvalidRequestError(Product X not found, HTTP 400INVALID_REQUEST), inside the existing completion transaction. This is the same behavior as the Python reference in #248 and the same message the Node create and update paths already use. Because the throw is insiderunInTransaction, earlier reservations roll back and no order is written.Regression coverage
Two tests in
lifecycle.test.ts, next to the existing rollback tests:ready_for_completewith noorderreference, and it completes once the product exists againready_for_completeBoth fail on
main(200 instead of 400). Valid completion and the 409 out-of-stock behavior are unchanged and covered by the existing tests.Validation
rest/nodejs:npm test, 177 passednpm run build(tsc) cleanFollows the Python fix in #248.