Reaction: Discussion: How to handle catalog item deletions in Cart

[aldeed]

Problem

6077 describes an issue which I've been attempting to fix in #6089. I've worked through the simple solutions but I'm left at a dead end on at least one aspect. So it's time to step back, discuss, and determine how we really want all of this to work.

A summary of the problem is:

When a shop product manager deletes or hides any product, variant, or option and then publishes that change to the catalog, that product configuration disappears from the catalog. This causes various errors when someone attempts to add an item with that product configuration to any cart, or when someone does any action with a cart that already contains an item with that product configuration.

This is really a series of problems, some more serious than others, and some easier to solve than others. Let me try to break it up into individual questions.

Business Questions

• When you add an item to a cart and that item refers to a product configuration (variant or option ID) that is currently not in the catalog, what should happen?
• When a product manager publishes a product change that makes a previously visible product, variant, or option now hidden, what should happen to cart items that reference this product configuration?
• Same as previous question, but for deleting.
• Is visibility different from deletion with regard to cart items? For example, should we allow hidden variants and options to be added to a cart and ordered, but just not show them in catalog search/browse?
• If items remain in a cart after their product configuration is no longer in the catalog, how do we determine their current price? (Currently this tries to look at catalog, causing some of the errors.)
• If items are automatically removed from a cart, how would we make this not confusing for the shopper?

Those are the business logic questions. The main related technical question that I need to answer is as follows.

Technical Question

(xformCartGroupToCommonOrder) All calculations on cart items (such as shipping rates, taxes, surcharges, etc.) rely on knowing the current price of the product configuration. The simple-pricing plugin looks to Catalog to find this, but this fails when the product config is no longer in the catalog. Which price do we use as current/active for an item that has been removed from the catalog? If we use the price directly, it may confuse product managers because usually price is something that you need to publish before it takes effect.

Possible Solution to Technical Question 1

I'm tempted to say we should avoid this question altogether by deleting all cart items for a product configuration at the same time we remove that option or variant from the catalog. But this brings up a number of other issues:

• will this confuse shoppers? / how can we not confuse them?
• what if you accidentally publish a deletion/hide, or temporarily hide something? Now it's also gone from all those carts that had it, and you may lose those sales even if you immediately put it back in the catalog?
• in a streaming world we'd probably queue removal from all carts, or somehow do it async for performance reasons. If we delete the catalog variant and there's a delay in deleting related cart items, we'd have a period of time in which we'd continue to have the same problems we currently have. What's the best approach here?

Possible Solution to Technical Question 2

If we allow cart items to exist after their related variant is removed from the Catalog, then our only choice is to rethink how simple-pricing works. It would likely need its own concept of a "current" published price, separate from the act of publishing to the catalog. (This would be similar to how the more advanced pricing engine plugin works.)

Versions

3.2.0

[aldeed]

@kieckhafer @ticean @focusaurus @mikemurray @rosshadden

[rosshadden]

We could leave items in the cart and throw a visible error on the cart page "This item is no longer available", which prevents checkout until removed. That way it wouldn't be confusing for users.

My worry with supporting hidden products is I feel like it precludes us from intentionally removing items from a catalog, unless we had both "hidden" and "no really, not there" states.

[mikemurray]

Do we have to physically remove the items, or can we just ignore them until we make the order, and then just omit them? Perhaps displaying a message, and/or locking checkout until they're removed. It would be better to let this be eventually consistent when the user takes an action on their cart.

I feel like, telling the user something is no longer available, and keeping a lightweight record around, like the original name, would help them find a replacement if one is available.

---

As for accidentally publishing a deletion, other than being more careful with the publish button, we could better confirm when products/variants are going to be archived.

If you want an undo, then it could be as simple as, just undo the archive and republish, or more complicated like a full revert to a previous version.

[aldeed]

If we want to do some sort of there-but-not-there state as @rosshadden and @mikemurray suggest, I think it would require adding a completely separate Cart.removedItems array to avoid breaking changes. Because CartItem.price is currently a required field, and this is the piece of info we would no longer know.

Two other benefits of Cart.removedItems:

• if they unhide/undelete, we could optionally move items back from Cart.removedItems to Cart.items list
• using a separate list rather than a flag on existing items means that all of the shipping, tax, surcharge, etc. calculation code would not need to be updated to filter out or otherwise handle such items.

And I agree this may be better for the shopper versus just plain deleting the item, but it isn't any different in how and where we'd have to implement it. We'd still need to either synchronously or asynchronously find and adjust every cart during or right after a catalog publish that hides or deletes.

I think it almost certainly has to be async, which means there will still be errors of some sort trying to retrieve a price during the lag time. My thought at the moment is to work around this by falling back to priceWhenAdded, which is stored directly on the items, rather than throwing an error when price calculation fails. This can lead to incorrect calculations during checkout, but they'd be caught by the placeOrder code.

[ticean]

I don't think shoppers should be able to checkout or maintain items in the cart if they can't view the catalog data for those items, for any reason, hidden or deleted. I'll call these "invalid" items now.

• Adding an invalid item should fail.
• When invalid items are detected in the cart, then they should be removed immediately. It's OK to lazily detect them, though. From what I understand this is the case that led to #6077? When this happens, we could remove the item and an error/msg could be returned or queued so that the user can be notified. Are there snags that I'm not considering that make this difficult? It sounds like the main one is user experience? If the item has been removed or hidden then I think the best we can do is tell the user what happened.
• Alternatively, we could do what @rosshadden suggested. Make the cart invalid and display an error to the user until they fix or acknowledge it. This wouldn't necessarily need to be something in the cart. It may be better to build a simple notifications system than to bend up the cart as a workaround.

[focusaurus]

This is tricky stuff but after a first read through my inclination would be toward @aldeed's suggestion of 'cart.removedItems' array. I do think it's more pragmatic to wait until issues are detected to do that transformation of the cart. So abandoned carts never need to be changed, and carts that try to proceed toward checkout get updated on the fly during price recalculation or whatever step of the process first detects invalid items in the cart, and temporary hide/show operations have no customer impact.

[aldeed]

This has been a helpful discussion so far. Feel free to continue posting thoughts if you have them, but for now I'll take a stab at the "lazy detect" suggested by @ticean, which can either straight up remove them or move them to removedItems (or maybe missingItems is clearer?) array. I'll see how that goes and then post here if I encounter other unforeseen problems.

[aldeed]

Closing because the proposed solution is merged now.