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(medusa): Applying Discounts (with Conditions) on DraftOrders and Carts #3197

Merged
merged 24 commits into from Feb 8, 2023

Conversation

adrien2p
Copy link
Member

@adrien2p adrien2p commented Feb 7, 2023

What

  • During a draft order creation, if a discount of type FIXED on allocated to total is present, the adjustment can't be calculated and throw an error.
  • Fixes cart related issues with applying discount

How

  • Fix the adjustment creation for the draft order creation flow
  • Fix cart discounts management and discount condition repo

Tests

  • Add a new integration test for it
  • Add new tests case

FIXES CORE-1100
FIXES CORE-1109

@changeset-bot
Copy link

changeset-bot bot commented Feb 7, 2023

🦋 Changeset detected

Latest commit: 77d005c

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

This PR includes changesets to release 2 packages
Name Type
@medusajs/medusa Patch
@medusajs/inventory 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

@adrien2p adrien2p force-pushed the fix/create-draft-order-fixed-discount branch from b0fa973 to ed6c9ce Compare February 7, 2023 14:44
@adrien2p adrien2p force-pushed the fix/create-draft-order-fixed-discount branch from ed6c9ce to 3b26228 Compare February 7, 2023 14:45
@adrien2p adrien2p marked this pull request as ready for review February 7, 2023 15:39
@adrien2p adrien2p requested a review from a team as a code owner February 7, 2023 15:39
@adrien2p
Copy link
Member Author

adrien2p commented Feb 7, 2023

/snapshot-this

@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2023

🚀 A snapshot release has been made for this PR

Test the snapshots by updating your package.json with the newly published versions:

yarn add @medusajs/inventory@2.0.0-snapshot-20230207160201
yarn add @medusajs/medusa@1.8.0-snapshot-20230207160201
yarn add @medusajs/medusa-js@2.0.0-snapshot-20230207160201
yarn add medusa-payment-adyen@1.1.42-snapshot-20230207160201
yarn add medusa-payment-klarna@1.3.7-snapshot-20230207160201
yarn add medusa-payment-manual@1.0.21-snapshot-20230207160201
yarn add medusa-payment-paypal@1.2.9-snapshot-20230207160201
yarn add medusa-payment-stripe@1.1.52-snapshot-20230207160201
yarn add medusa-plugin-restock-notification@3.0.0-snapshot-20230207160201
yarn add medusa-plugin-sendgrid@1.4.0-snapshot-20230207160201
yarn add medusa-react@5.0.0-snapshot-20230207160201
yarn add @medusajs/stock-location@2.0.0-snapshot-20230207160201

Latest commit: 6729f9b

@adrien2p
Copy link
Member Author

adrien2p commented Feb 8, 2023

/snapshot-this

@olivermrbl
Copy link
Contributor

We have a similar issue on carts. Try to apply discount with code heyo in staging in region United Kingdom.

It will throw the following:

invalid input syntax for type integer: \"NaN\""

Is this the same? And if so, do you know if this will fix it?

@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 2023

🚀 A snapshot release has been made for this PR

Test the snapshots by updating your package.json with the newly published versions:

yarn add create-medusa-app@0.0.8-snapshot-20230208095134
yarn add @medusajs/inventory@2.0.0-snapshot-20230208095134
yarn add @medusajs/medusa@1.8.0-snapshot-20230208095134
yarn add @medusajs/medusa-cli@1.3.7-snapshot-20230208095134
yarn add medusa-core-utils@1.1.38-snapshot-20230208095134
yarn add medusa-dev-cli@0.0.29-snapshot-20230208095134
yarn add medusa-file-minio@1.1.4-snapshot-20230208095134
yarn add medusa-file-s3@1.1.10-snapshot-20230208095134
yarn add medusa-file-spaces@1.3.4-snapshot-20230208095134
yarn add medusa-fulfillment-manual@1.1.35-snapshot-20230208095134
yarn add medusa-fulfillment-webshipper@1.3.6-snapshot-20230208095134
yarn add medusa-interfaces@1.3.5-snapshot-20230208095134
yarn add @medusajs/medusa-js@2.0.0-snapshot-20230208095134
yarn add medusa-payment-adyen@1.1.42-snapshot-20230208095134
yarn add medusa-payment-klarna@1.3.7-snapshot-20230208095134
yarn add medusa-payment-manual@1.0.21-snapshot-20230208095134
yarn add medusa-payment-paypal@1.2.9-snapshot-20230208095134
yarn add medusa-payment-stripe@1.1.52-snapshot-20230208095134
yarn add medusa-plugin-algolia@0.2.8-snapshot-20230208095134
yarn add medusa-plugin-brightpearl@1.3.5-snapshot-20230208095134
yarn add medusa-plugin-contentful@1.2.6-snapshot-20230208095134
yarn add medusa-plugin-discount-generator@1.1.23-snapshot-20230208095134
yarn add medusa-plugin-economic@1.1.41-snapshot-20230208095134
yarn add medusa-plugin-ip-lookup@1.2.4-snapshot-20230208095134
yarn add medusa-plugin-mailchimp@1.1.45-snapshot-20230208095134
yarn add medusa-plugin-meilisearch@1.0.3-snapshot-20230208095134
yarn add medusa-plugin-restock-notification@3.0.0-snapshot-20230208095134
yarn add medusa-plugin-segment@1.3.3-snapshot-20230208095134
yarn add medusa-plugin-sendgrid@1.4.0-snapshot-20230208095134
yarn add medusa-plugin-slack-notification@1.3.7-snapshot-20230208095134
yarn add medusa-plugin-twilio-sms@1.2.6-snapshot-20230208095134
yarn add medusa-plugin-wishlist@1.1.40-snapshot-20230208095134
yarn add medusa-react@5.0.0-snapshot-20230208095134
yarn add medusa-source-shopify@1.2.6-snapshot-20230208095134
yarn add @medusajs/stock-location@2.0.0-snapshot-20230208095134

Latest commit: 6729f9b

@adrien2p
Copy link
Member Author

adrien2p commented Feb 8, 2023

We have a similar issue on carts. Try to apply discount with code heyo in staging in region United Kingdom.

It will throw the following:

invalid input syntax for type integer: \"NaN\""

Is this the same? And if so, do you know if this will fix it?

looks exactly the same, totals is an empty array and therefore we divide by 0 = NaN

@adrien2p
Copy link
Member Author

adrien2p commented Feb 8, 2023

@olivermrbl do you want me to look into the cart as well. When is the code applied?

@olivermrbl
Copy link
Contributor

That would be great! You should be able to reproduce the error in the staging storefront. Make sure you are in region United Kingdom, add items to your cart, and attempt to apply code heyo in the checkout flow :)

@adrien2p adrien2p changed the title fix(medusa): Creating a draft order with a fixed discount on total fail fix(medusa): Applying discount upon draft order creation or cart update does not work properly Feb 8, 2023
@olivermrbl olivermrbl changed the title fix(medusa): Applying discount upon draft order creation or cart update does not work properly fix(medusa): Applying Discounts (with Conditions) on DraftOrders and Carts Feb 8, 2023
@@ -256,6 +256,10 @@ export class DiscountConditionRepository extends Repository<DiscountCondition> {
// if condition operation is `in` and the query for conditions defined for the given type is empty, the discount is invalid
// if condition operation is `not_in` and the query for conditions defined for the given type is not empty, the discount is invalid
for (const condition of discountConditions) {
if (condition.type === DiscountConditionType.CUSTOMER_GROUPS) {
Copy link
Contributor

@olivermrbl olivermrbl Feb 8, 2023

Choose a reason for hiding this comment

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

comment (for reviewer): This repository method is responsible for checking the validity of a discount in relation to products, types, collections, and tags, which is why we skip the customer groups conditions

Copy link
Contributor

@olivermrbl olivermrbl left a comment

Choose a reason for hiding this comment

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

LGTM, but I would like a second pair of eyes on it, as I took part in some of the changes :)

@adrien2p adrien2p force-pushed the fix/create-draft-order-fixed-discount branch from 4a1182e to 2f4046a Compare February 8, 2023 16:26
Copy link
Contributor

@riqwan riqwan left a comment

Choose a reason for hiding this comment

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

LGTM :)

packages/medusa/src/services/cart.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@olivermrbl olivermrbl left a comment

Choose a reason for hiding this comment

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

LGTM!

@olivermrbl olivermrbl merged commit bfa33f4 into develop Feb 8, 2023
@olivermrbl olivermrbl deleted the fix/create-draft-order-fixed-discount branch February 8, 2023 18:01
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.

None yet

4 participants