Shipping discount coupon resulting in negative value exception

Vote:
 

Hi,

I am using Commerce 12.10.0 and below is the scenario:

I added an item into the cart and then applied a shipping coupon (Free shipping). Upon applying the coupon I get the error mentioned below (step 2) 

1) Added a weight based shipping method: https://www.screencast.com/t/trWQxz7zJ

2) Below is the stack trace for the error: https://www.screencast.com/t/TRpqw9MgFR6D

I also had a look into the reflector based on the stack trace and I see the code mentioned below:

public Money GetDiscountedShippingAmount(IShipment shipment, IMarket market, Mediachase.Commerce.Currency currency)
    {
      Validator.ThrowIfNull(nameof (shipment), (object) shipment);
      Validator.ThrowIfNull(nameof (market), (object) market);
      Money money = this.GetShippingCost(shipment, market, currency) - shipment.GetShipmentDiscountPrice(currency);
      this.ValidateShippingCostForShipment(money);
      return money;
    }

Looking at this I feel the issue is where you subtract GetShippingCost with GetShipmentDiscountPrice. And GetShipmentDiscountPrice being the culprit which leads to a negative value and then throws the error. 

#198790
Nov 06, 2018 23:00
Vote:
 

Right before running the calculators in your own code.

If you do:

shipment.GetShipmentDiscount()

What does that give you?

#198807
Edited, Nov 07, 2018 8:23
Vote:
 
#198868
Nov 07, 2018 23:08
Vote:
 

Yep, as I suspected. The dreaded rounding error. :P 

The underlying cause is that the PromotionEngine rounds the resulting saved amount before setting it on the Shipment.SavedAmount field.

So either you avoid using shipment costs with more than X number of decimal places (X in your case being 2).

Or......

... if you can't do that I'm hard pressed to find a good solution to this.

But technically you can modify to how many decimal places the promotion engine rounds off to. You can try doing this before running the promotion engine. Like so:

var oldCurrencyDecimalDigits = orderGroup.Currency.Format.CurrencyDecimalDigits,
orderGroup.Currency.Format.CurrencyDecimalDigits = 5;
// Run PromotionEngine
orderGroup.Currency.Format.CurrencyDecimalDigits = oldCurrencyDecimalDigits;

This should (I hope, I haven't tested it :D) mean that the PromotionEngine rounds off correctly in your case.

But I'm assuming that this approach will come with other issues since all calculations made based on the ordergroup in the Promotion Engine will be rounded off the same way, which may be incorrect in another case.

#198911
Edited, Nov 08, 2018 12:55
Vote:
 

@Jafet Valdez: Thank you for the response, I see this as a bug with the promotions engine then. It should not round the saved amount  right? I appreciate your suggestion to try and modify the number of decimal places rounding. But since the effect of this change might cause other issues I dont want to move forward with this. Would you suggest to file a bug in this case then? The project I am working on is approaching the go-live date.

#198915
Nov 08, 2018 16:06
Vote:
 

I'm not so sure that it can be considered a bug per sé. It's rounding according to the defaults for currencies in C#. Like Episerver Commerce does in most other places.

It needs to round to be able to work with various types of percentages or you'd have other calculation issues instead.

Personally I'd say that doing the same rounding in your ShippingMethod would be the best solution in general, but I understand that this might not be possible for business reasons.

So contact Developer Support to get a path forward on this.

#198916
Nov 08, 2018 17:25
Vote:
 

It looks like modifying Currency.Format.CurrencyDecimalDigit is the way to go for now until Episerver gets a fix out in the next major release

Take a look at this blog post:

https://world.episerver.com/blogs/Quan-Mai/Dates/2018/11/control-the-rounding-level-of-currencies-/

#199076
Edited, Nov 14, 2018 20:32
Vote:
 

@Jafet Valdez: Thanks the response. Marking your reply as the answer :)

#199078
Nov 14, 2018 20:53
This topic was created over six months ago and has been resolved. If you have a similar question, please create a new topic and refer to this one.
* You are NOT allowed to include any hyperlinks in the post because your account hasn't associated to your company. User profile should be updated.