Shipping discount coupon resulting in negative value exception

Siddharth Gupta
Member since: 2018
 

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
  • Jafet Valdez
    Member since: 2016
     

    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
  • Siddharth Gupta
    Member since: 2018
     

    Right before running the calculators in your own code.

    If you do:

    shipment.GetShipmentDiscount()

    What does that give you?

    @Jafet : Here you go: https://www.screencast.com/t/gt9z0j8d

    I basically casted shipment to IShipmentDiscountAmount, if you see the screencast; ShippingDiscountAmount is 440.65 and ShippingSubTotal is 440.648000000. So if you apply a free shipping coupon then you do (ShippingSubTotal - ShippingDiscountAmount) which is -0.002 and hence the Invalid shipping cost exception.

    #198868 Nov 07, 2018 23:08
  • Jafet Valdez
    Member since: 2016
     

    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
  • Siddharth Gupta
    Member since: 2018
     

    @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
  • Jafet Valdez
    Member since: 2016
     

    @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.

    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
  • Jafet Valdez
    Member since: 2016
     

    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
  • Siddharth Gupta
    Member since: 2018
     

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

    #199078 Nov 14, 2018 20:53