Copied to clipboard

Flag this post as spam?

This post will be reported to the moderators as potential spam to be looked at


  • Kenn Jacobsen 132 posts 763 karma points MVP c-trib
    Jul 25, 2011 @ 13:22
    Kenn Jacobsen
    0

    A bug in the checkout flow creates errornous copies of orders

    Hi...

    It looks like there's something fishy going on with orders during the checkout flow in the starter kit (1.2.0.0).

    I am doing a dynamic calculation of shipping cost and other calculations to orders during the checkout (e.g. discount handling). Under specific circumstances, Tea Commerce adds new entries in the Order table, causing the current order to change.

    The problem is easy to recreate:

    1. Create an event handler using the code below
    2. Open the starter kit webshop, add an item to the basket and go through the checkout flow - stop at the "Accept" page. Notice that the shipping cost for the order is shown as the value calculated by the UpdateShippingFee() method in the event handler.
    3. Access the database from SQL Management Studio and execute "SELECT TOP 10 o.Id, o.ManipulatedShippingFeeWithoutVAT, o.CreatedDate FROM TeaCommerce_Order o ORDER BY o.Id DESC". Note the Id of the order (the first result). Also notice that the colunm ManipulatedShippingFeeWithoutVAT has the correct value set by the UpdateShippingFee() method in the event handler.
    4. In the webshop, flip backwards through the checkout flow until you reach "Customer information" (Click "< Back to Shipping and Payment", then "< Back to Customer information")
    5. Change country for the order.
    6. In SQL Management Studio, refresh the query. For some reason what looks like a copy of the order has been added to the Order table. Why that happens is beyond me, but at least it does contain a correct, updated value in the ManipulatedShippingFeeWithoutVAT column - so far so good.
    7. In the webshop, go to the next step in the checkout flow (click "Shipping and Payment >")
    8. In SQL Management Studio, refresh the query. Yet another order has been added to the Order table - this time with a NULL value in the ManipulatedShippingFeeWithoutVAT column.
    9. In the webshop, go to the next step in the checkout flow (click "Next >")
    10. As one might expect, the shipping cost for the order is now listed as the default shipping price for the chosen provider.

    I'm not entirely certain why Tea Commerce would create a copy of the order in step 5. It might be a feature, it might be a bug - I don't know. 

    The order copy created in step 7 seems most definitively to be a bug. It happens when the OriginalUnitPriceWithoutVAT property on the orderlines is read by the GetOriginalPriceWithoutVAT() method in the event handler. Read - not written to. Comment out the line and the order is not copied in step 7. And what's even more weird; if you attach a debugger, you'll see that the GetOriginalPriceWithoutVAT() method is not even invoked in step 7!

    Any ideas?

    ~Kenn

        public class TestEventHandler : ITeaCommerceExtension {
            public void Initialize() {
                WebshopEvents.OrderLineChanged += WebshopEvents_OrderLineChanged;
                WebshopEvents.ShippingMethodChanged += WebshopEvents_ShippingMethodChanged;
                WebshopEvents.CountryChanged += WebshopEvents_CountryChanged;
            }
    
            private bool UpdateShippingFee(Order order) {
                // get the shipping fee per country per currency (simplified...)
                var shippingFees = new Dictionary<stringDictionary<stringdecimal>> {
                             {"dk"new Dictionary<stringdecimal> {{"dkk", 10.00m}, {"eur", 1.33m}, {"usd", 1.80m}}},
                             {"de"new Dictionary<stringdecimal> {{"dkk", 20.00m}, {"eur", 2.66m}, {"usd", 3.60m}}},
                             {"us"new Dictionary<stringdecimal> {{"dkk", 50.00m}, {"eur", 6.65m}, {"usd", 9.00m}}}
                };
                var fee = shippingFees[order.Country.CountryCode.ToLowerInvariant()][order.CurrencyISOCode.ToLowerInvariant()];
    
                // update the shipping fee as applicable
                if(order.ShippingFee == fee) {
                    return false;
                }
                order.ShippingFee = fee;
                return true;
            }
    
            private decimal GetOriginalPriceWithoutVAT(Order order) {
                // this code also triggers the error... but the code below is better for showing where the error occurs :-)
                //return order.OrderLines.Sum(o => o.OriginalUnitPriceWithoutVAT);
    
                // sum the total price without VAT
                var total = 0m;
                foreach(var item in order.OrderLines) {
                    // access the OriginalUnitPriceWithoutVAT property to trigger the error
                    // (the original code uses OriginalUnitPriceWithoutVAT for something else, this code is just for triggering the error)
                    total += item.OriginalUnitPriceWithoutVAT;
    
                    // accessing other properties instead - e.g. Quantity - does not seem to trigger the error
                    //total += item.Quantity;
                }
                return total;
            }
    
            private void WebshopEvents_OrderLineChanged(Order order, OrderLine orderLine) {
                // trigger the error
                // note: the code does not change anything in the order, and we don't even call order.Save() - but still the error occurs
                var priceWithoutVAT = GetOriginalPriceWithoutVAT(order);
            }
    
            private void WebshopEvents_CountryChanged(Order order, Country country) {
                if(UpdateShippingFee(order)) {
                    order.Save();
                }
            }
    
            private void WebshopEvents_ShippingMethodChanged(Order order, ShippingMethod shippingMethod) {
                if(UpdateShippingFee(order)) {
                    order.Save();
                }
            }
        }

     

  • Rune Grønkjær 1296 posts 2863 karma points
    Jul 25, 2011 @ 21:01
    Rune Grønkjær
    0

    Hi Kenn,

    The reason we clone the orders is the payment provider model we are using. When a user is going to payment we mark the order as "Initial" which means that this specific order number may no longer be manipulated. If the order is being manipulated by the Tea Commerce base it will be cloned and the old order is no longer ind the Current order session. This is done as most Payment providers will not allow the same order number to be pay twice.

    It does look like we are not cloning the manipulated values, which would be a bug ofcause. We will have a look at that, but have a slow turn around time at the moment due to the holidays, as you might already have noticed. Until then you might want to recalc your value, which you could do in your OrderChanged event, which is fired all the time.

    /Rune

  • Kenn Jacobsen 132 posts 763 karma points MVP c-trib
    Jul 26, 2011 @ 07:05
    Kenn Jacobsen
    0

    Hi Rune,

    I totally understand with the holidays :-)

    It looks like Tea Commerce is cloning the manipulated values correctly when changing country on the order, but when the 2nd copy is created out of the blue, the values are not cloned.

    I've tested it with the original event handler, that does discounts on order lines, and those discount manipulations seem to be cloned correctly on both occasions.

    I have also tried the OrderChanged event, but apparently the ShippingFee for the order is not loaded correctly from the database when using this event, causing the code to be stuck in an infinite loop (code below). Also, moving the logic to the OrderChanged event does not stop Tea Commerce from creating the 2nd clone.

    ~Kenn

        public class TestEventHandler : ITeaCommerceExtension {
            public void Initialize() {
                WebshopEvents.OrderChanged += WebshopEvents_OrderChanged;
            }
    
            void WebshopEvents_OrderChanged(Order order) {
                // trigger the error
                // note: the code does not change anything in the order, and we don't even call order.Save() - but still the error occurs
                var priceWithoutVAT = GetOriginalPriceWithoutVAT(order);
    
                if(UpdateShippingFee(order)) {
                    // WARNING: 
                    // the code is stuck in an infinite loop here since it believes to have changed 
                    // the shipping fee because it is not loaded correctly from the DB (?)
                    order.Save();
                }
            }
    
            private bool UpdateShippingFee(Order order) {
                // get the shipping fee per country per currency (simplified...)
                var shippingFees = new Dictionary<stringDictionary<stringdecimal>> {
                             {"dk"new Dictionary<stringdecimal> {{"dkk", 10.00m}, {"eur", 1.33m}, {"usd", 1.80m}}},
                             {"de"new Dictionary<stringdecimal> {{"dkk", 20.00m}, {"eur", 2.66m}, {"usd", 3.60m}}},
                             {"us"new Dictionary<stringdecimal> {{"dkk", 50.00m}, {"eur", 6.65m}, {"usd", 9.00m}}}
                };
                var fee = shippingFees[order.Country.CountryCode.ToLowerInvariant()][order.CurrencyISOCode.ToLowerInvariant()];
    
                // update the shipping fee as applicable
                if(order.ShippingFee == fee) {
                    return false;
                }
                order.ShippingFee = fee;
                return true;
            }
    
            private decimal GetOriginalPriceWithoutVAT(Order order) {
                // this code also triggers the error... but the code below is better for showing where the error occurs :-)
                //return order.OrderLines.Sum(o => o.OriginalUnitPriceWithoutVAT);
    
                // sum the total price without VAT
                var total = 0m;
                foreach(var item in order.OrderLines) {
                    // access the OriginalUnitPriceWithoutVAT property to trigger the error
                    // (the original code uses OriginalUnitPriceWithoutVAT for something else, this code is just for triggering the error)
                    total += item.OriginalUnitPriceWithoutVAT;
    
                    // accessing other properties instead - e.g. Quantity - does not seem to trigger the error
                    //total += item.Quantity;
                }
                return total;
            }
        }
    
  • Rune Grønkjær 1296 posts 2863 karma points
    Jul 27, 2011 @ 20:55
    Rune Grønkjær
    0

    Hi Kenn,

    I have tried testing it here, but it will have to wait until next week. I just can't get anything to work properly here. Hope you can wait, and we will find a solution.

    /Rune

  • Kenn Jacobsen 132 posts 763 karma points MVP c-trib
    Jul 28, 2011 @ 07:47
    Kenn Jacobsen
    0

    Hi again,

    That's OK. 

    I'll be on vacation myself the next two weeks, so I won't be here to test any solutions until mid August. But I hope you can reproduce it with the posted code... it happens without fail on my solution.

    ~Kenn

  • Rune Grønkjær 1296 posts 2863 karma points
    Aug 08, 2011 @ 11:08
    Rune Grønkjær
    0

    Hi Kenn,

    Finally I found the problem, which was a bug in our cloning method. I have uploaded a new beta package for you with a fix.

    /Rune

  • Kenn Jacobsen 132 posts 763 karma points MVP c-trib
    Aug 16, 2011 @ 14:59
    Kenn Jacobsen
    0

    Hi Rune,

    The new beta seems to solve the problem. I'll test it more thoroughly tomorrow and mark the issue as solved :-)

    ~Kenn

  • Kenn Jacobsen 132 posts 763 karma points MVP c-trib
    Aug 17, 2011 @ 13:47
    Kenn Jacobsen
    0

    It works :-)

    Thank you for your help...!

  • Rune Grønkjær 1296 posts 2863 karma points
    Aug 17, 2011 @ 13:48
    Rune Grønkjær
    0

    Great. The fix will be public with our next release.

    /Rune

Please Sign in or register to post replies

Write your reply to:

Draft