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

Ensure shipment gets updated when an order is updated #12954

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rioug
Copy link
Collaborator

@rioug rioug commented Oct 30, 2024

What? Why?

I came across this while looking at #12907
Given an order up to "Order Summary" step, if a customer then update the cart by going back to the shop page via the link next to the cart, the order shipment won't be updated.
It results in :

  • the data on the order detail page in the back office being wrong
  • the stock level not being updated properly

To fix this, we make sure we delete existing shipment and restart the checkout flow.

Screenshot Capture - 2024-10-30 - 21-22-06
Order confirmation started with 2 potatoes test , was updated to 4 potatoes test and 3 potatoes

Screenshot Capture - 2024-10-30 - 21-22-41
Admin screen showing only the original items/quantity , 2 potatoes test

Screenshot Capture - 2024-10-30 - 21-22-57
Potatoes test stock reduced from 50 to 48, and Potatoes stock stayed at 20

What should we test?

As an enterprise manager:

  • set up a product with some stock, ie not "on demand"

As a customer

  • Start an order (use a product with stock) , proceed to checkout and progress to "Order confirmation" step
  • Click on the shop link next to the cart
  • Add a new item or update quantity of an item already in the cart, it should be product with a stock set
  • Proceed to checkout again
    --> the order should have the correct items and quantity
  • Complete the order
    As an enterprise manager
  • open the order just created
    --> The items should all be there and have the correct quantity
  • Go to the Bulk Edit product page
    --> verify that the stock of the products in the order have been decreased by the correct number

Release notes

Changelog Category (reviewers may add a label for the release notes):

  • User facing changes
  • API changes (V0, V1, DFC or Webhook)
  • Technical changes only
  • Feature toggled

The title of the pull request will be included in the release notes.

`Spree::OrderContents#update_or_create` is used to update the cart when
on the /shop page. If you start an order and proceed to the "Order
summary" step, and then decide to update your order by using the shop
link next to the cart, such update wouldn't update the shipment.
This result in the order page in the backoffice displaying the wrong data,
and more importantly, in the stock not being updated.
So now we ensure shipment will be updated, which result in the checkout
flow being restarted, thus making sure the shipment is updated.
@rioug rioug marked this pull request as ready for review October 30, 2024 10:35
Copy link
Member

@dacook dacook left a comment

Choose a reason for hiding this comment

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

Nice followup!

I wonder if we can do it more efficiently? And I'm unsure about the spec.

@@ -38,6 +38,7 @@ def update_or_create(variant, attributes)
line_item.price = variant.price
order.line_items << line_item
end
update_shipment
Copy link
Member

Choose a reason for hiding this comment

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

Is it relevant for both create and update? I thought it might only need to be called on update.


subject.update_or_create(variant, { quantity: 2, max_quantity: 3 })
end

Copy link
Member

Choose a reason for hiding this comment

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

Looks like this tests creating mode, but not updating an existing line item (Which I think is what it should be testing)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress ⚙
Development

Successfully merging this pull request may close these issues.

2 participants