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

Cart with soft-deletion #5361

Merged

Conversation

Matt-Yorkley
Copy link
Contributor

@Matt-Yorkley Matt-Yorkley commented May 4, 2020

What? Why?

Closes #5358

Adds various missing specs for variant soft-deletion issues and adjusts CartService logic so that if a variant is soft-deleted whilst it's in an active cart or is added to a cart, the user gets useful feedback (and no fatal error problems), the item is removed and acts as if it has gone out of stock.

What should we test?

Load a shop page, then delete a variant that's already shown on the page, then add that variant to the cart. It should show an out-of-stock modal and remove it from the cart.

Release notes

Fixed issues around adding deleted variants to cart.

@Matt-Yorkley Matt-Yorkley self-assigned this May 4, 2020
Copy link
Contributor

@luisramos0 luisramos0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very good!! 👏

app/models/spree/stock/quantifier.rb Show resolved Hide resolved
@Matt-Yorkley Matt-Yorkley force-pushed the cart-with-soft-deletion branch from 2938c9b to 8b0ef5f Compare May 5, 2020 10:20
@Matt-Yorkley Matt-Yorkley force-pushed the cart-with-soft-deletion branch from 8b0ef5f to 6afda87 Compare May 5, 2020 11:58
@Matt-Yorkley Matt-Yorkley marked this pull request as ready for review May 5, 2020 11:58
@Matt-Yorkley
Copy link
Contributor Author

Rebased 👍

Copy link
Contributor

@luisramos0 luisramos0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome 👌

app/models/spree/stock/quantifier.rb Show resolved Hide resolved
@Matt-Yorkley Matt-Yorkley mentioned this pull request May 5, 2020
2 tasks
@Matt-Yorkley Matt-Yorkley added the bug-s1 The bug is stopping the platform from working, and there is no workaround. Impact of lot of users. label May 5, 2020
includes(:default_price, :stock_items, :product).
index_by(&:id)
end
end

def remove_deleted_variant(variant)
line_item_for_variant(variant).andand.destroy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in the long term we should soft-delete the line item upon soft-deletion of the variant, ala ON DELETE CASCADE only that it'll be an UPDATE statement in this case.

@filipefurtad0
Copy link
Contributor

Awesome @Matt-Yorkley ,

Checked two scenarios here:

User: Loads /shopfront (variant x is visible)
Admin: Deletes variant x
User: Adds variant x to the cart (pressing the up arrow a couple of times)
-> the "Reduced stock available" modal kicks in 🎉 and the variant x becomes greyed-out.

I think this is the main aim of this PR, this is ready to go.

The other scenario I checked is:

User: Loads /shopfront (variant x is visible)
User: Adds variant x to the cart (pressing the up arrow a couple of times)
Admin: Deletes variant x
-> In this case, the modal is not shown.
User: pressing Cart or Edit your cart will lead to the situation described on issue #5239

While this could be improved it surely introduces a huge improvement. Should I open a separate issue for the second test-case?

@sauloperez sauloperez merged commit 194c87e into openfoodfoundation:master May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-s1 The bug is stopping the platform from working, and there is no workaround. Impact of lot of users.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cart Problems in Production
4 participants