Money cannot represent non numberic value NaN and discount for order line with 0 quantity

[TeikSean]

Describe the bug
When try to modify orderline quantity to 0, it show error Money cannot represent non numberic value NaN

Demo Video:

Screen.Recording.2024-08-13.at.12.52.06.PM.1.mov

To Reproduce
Steps to reproduce the behavior:

1. Go to Promotion, create a discount where the condition is 'If order total is greater than 0' with amount 0 and actions 'Discount order by 15%' and value is 15%
2. Create an order with 2 order line and make payment
3. Modify the order and change the quantity of the lines to become 0, click preview changes

Expected behavior

1. Error will be thrown 'Money cannot represent non numberic value NaN'

Environment (please complete the following information):

• @vendure/core version: 3.0.0
• Nodejs version: 18.20
• Database (mysql/postgres etc):

Additional context
I am writing my own resolver that remove certain items when some condition are met (like, customer want to remove the item and the item is yet to be fulfilled). When doing this flow without any promotion that’s on the order level, it works as expected, the error arise when we want to remove the item. Which causes an error of NaN because i am running the modifyOrder without the dryRun.

I have discovered a potential root cause that might solve this issue. In the applyOrderPromotion in OrderCalculator, there’s a code to get the share of amount of discount that each order line get which is the const shareOfAmount = distribution[i]; However, I noticed that for order line that dont have any quantity, the distribution will result in the value of undefined, to solve this, I found out that adding ?? 0 after the const shareOfAmount = distribution[i]; which becomes const shareOfAmount = distribution[i] ?? 0; solves the issue.

[michaelbromley]

Hi,

Thanks for the detailed report. Are you interested in contributing a fix for this? If so, that would be very welcome. Let me know also if you need any help putting together an e2e test for this scenario.

btw I noticed you are based in Penang. I'm writing this reply on the train from Penang -> Ipoh! Greetings!

[jyling]

Hi,

Thanks for the detailed report. Are you interested in contributing a fix for this? If so, that would be very welcome. Let me know also if you need any help putting together an e2e test for this scenario.

btw I noticed you are based in Penang. I'm writing this reply on the train from Penang -> Ipoh! Greetings!

Hi @michaelbromley, Welcome to Malaysia, I hope you enjoy your stay in Malaysia, I and @LavaSean are colleagues, and we want to contribute a fix for this issue, We will let you know once we have a pr ready for this.

[jyling]

Hi, I have created a pull request for this issue over here, #3009

I do however encounter a weird error that happens for the custom field validation, I'm wondering if this is an issue that only appears on my machine

[michaelbromley]

All the e2e tests are passing in CI so it seems something local to you. I'll review in more detail later just to check but so far it looks good 👍

[jyling]

All the e2e tests are passing in CI so it seems something local to you. I'll review in more detail later just to check but so far it looks good 👍

Thank you 😄