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: correct calculations of subtotal for shipping option requirements #7089

Conversation

kevinfurmanski
Copy link
Contributor

When using min/max subtotal for shipping options, there is a case when the listCartOptions method in medusa-js will return the wrong option. If I have a "Paid shipping" option between 0-999 SEK costing 29 SEK and a "Free shipping" option of orders above 999 SEK and the line items amount to for example 980 SEK, then when you select the "Paid shipping" option for 29 SEK the order total goes over 999 SEK and the "Paid shipping" option is hidden and instead the "Free shipping" option is shown. When you then press the "Free shipping" option the order total goes under 999 SEK and the "Free shipping" option is hidden and the "Paid shipping" option is shown again. It's an endless cycle that you can't get out of due to the fact that the shipping cost is included in the total. This should be modified to not include the shipping price.

Copy link

changeset-bot bot commented Apr 17, 2024

🦋 Changeset detected

Latest commit: c12f410

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@medusajs/medusa Patch

Not sure what this means? Click here to learn what changesets are.

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

Copy link

vercel bot commented Apr 17, 2024

@kevinfurmanski is attempting to deploy a commit to the medusajs Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Member

@sradevski sradevski left a comment

Choose a reason for hiding this comment

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

LGTM! Can you just please add a changeset, and rebase? We can merge it right after. Adding a test would also be appreciated

@kevinfurmanski kevinfurmanski force-pushed the fix/shipping-option-requirement-basis branch from c7fd37e to c12f410 Compare April 27, 2024 18:10
@kevinfurmanski
Copy link
Contributor Author

@sradevski I added a changeset and rebased from develop

@sradevski sradevski merged commit 82cb6cf into medusajs:develop Apr 29, 2024
19 of 23 checks passed
@sradevski
Copy link
Member

@olivermrbl Just a heads up, maybe we should also include this in today's release.

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

Successfully merging this pull request may close these issues.

None yet

2 participants