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

fix(core-flows): item id in reservations #9097

Merged
merged 5 commits into from
Sep 16, 2024
Merged

Conversation

fPolic
Copy link
Contributor

@fPolic fPolic commented Sep 11, 2024

What

  • on cart completion, when creating reservations, use line item id instead of cart item id
  • NOTE: inventory reservation is now done after we create the order

FIXES CC-448

Copy link

vercel bot commented Sep 11, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
medusa-dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 13, 2024 3:55pm
6 Skipped Deployments
Name Status Preview Comments Updated (UTC)
api-reference ⬜️ Ignored (Inspect) Sep 13, 2024 3:55pm
api-reference-v2 ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2024 3:55pm
docs-ui ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2024 3:55pm
docs-v2 ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2024 3:55pm
medusa-docs ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2024 3:55pm
resources-docs ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2024 3:55pm

Copy link

changeset-bot bot commented Sep 11, 2024

⚠️ No Changeset found

Latest commit: 3686c13

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

@carlos-r-l-rodrigues carlos-r-l-rodrigues left a comment

Choose a reason for hiding this comment

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

LGTM

@srindom
Copy link
Collaborator

srindom commented Sep 11, 2024

thought(non-blocking): could it make sense to reserve in the same way we are doing it now, but then move the reservation from the cart line item to the order line item as the final step? I don't have strong opinions on this just thinking that it would be beneficial to terminate the workflow early if the reservation fails to happen.

(this could be something to consider at some other point, just wanted to put the idea out there)

@fPolic
Copy link
Contributor Author

fPolic commented Sep 11, 2024

thought(non-blocking): could it make sense to reserve in the same way we are doing it now, but then move the reservation from the cart line item to the order line item as the final step? I don't have strong opinions on this just thinking that it would be beneficial to terminate the workflow early if the reservation fails to happen.

(this could be something to consider at some other point, just wanted to put the idea out there)

I was thinking of taking this route initially but we don't have a link from which we would know to match order_line_item <> cart_line_item.

@kodiakhq kodiakhq bot merged commit 9db3345 into develop Sep 16, 2024
23 checks passed
@kodiakhq kodiakhq bot deleted the fix/reservation-item-id branch September 16, 2024 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants