Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not having a Facility defined for a Product should not be treated as an error #210

Open
eigood opened this issue Oct 7, 2024 · 2 comments

Comments

@eigood
Copy link
Contributor

eigood commented Oct 7, 2024

<return error="true" message="No facilities ${assetList*.facilityId} found for asset reservations on order ${salesOrderId} for order items ${orderItemList*.orderItemSeqId} with assets ${assetList*.assetId}"/>

If there is no Facility defined for a product, then this line should just return, and not treat it as an error. In this case, drop-ship isn't wanted. This service is called via SECA on all orders, when they transition to Approved status, so needs to handle all cases.

(This actually failed on me, when I attempted to place an order on an existing product that hadn't been matched to a facility yet)

@eigood
Copy link
Contributor Author

eigood commented Oct 7, 2024

This branch also fails if purchasing a digital product, that will never have an asset. Basically, you're adding a forced call out to the create#DropShip service, for all orders being placed, and then returning error when the invariants necessary for drop ship to function are not found. That's going to cause a lot of breakage for existing users.

Don't return error in any case, and ideally, don't spam the log with "can't do drop-ship foo because of bar", for existing flows.

@eigood
Copy link
Contributor Author

eigood commented Oct 7, 2024

Please try this against upstream demo data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant