Uploaded image for project: 'OFBiz'
  1. OFBiz
  2. OFBIZ-5430

"Please Select Your Shipping Method" error sometimes occurs when updating order items

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: Trunk
    • Fix Version/s: 14.12.01, 12.04.06, 13.07.02
    • Component/s: order
    • Labels:
      None

      Description

      Sometimes when an order item is appended or existing order items are updated, a "Please Select Your Shipment Method" error occurs. I'm not exactly sure what causes this but it seems to be intermittent because sometimes I'm able to perform both of the operations without the error occur.

      The problem seems to be that an extra empty ship group is added to the cart and takes the index position of first ship group. Because the empty ship group contains no shipmentMethodTypeId or carrierPartyId the error occurs.

      Logic needs to exist to prevent the empty ship group that causes this issue from ever being added to the cart.

        Issue Links

          Activity

          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Yes I saw that also, I found a workaround for the extra empty ship group, but was really not satisfied with it. Then I was unable to reproduce.

          It could be related to ShoppingCart.java.getShipInfo() where surprinsingly a shipInfo is created.

          Show
          jacques.le.roux Jacques Le Roux added a comment - Yes I saw that also, I found a workaround for the extra empty ship group, but was really not satisfied with it. Then I was unable to reproduce. It could be related to ShoppingCart.java.getShipInfo() where surprinsingly a shipInfo is created.
          Hide
          ofbizzer Christian Carlow added a comment -

          Hey Jacques,

          I've just determined that I'm not receiving the error on the demo site. Its probably occurring because of the recent changes I made for handling complex ship groups better. I guess whatever solution I find should apply to that issue instead of this one. I'll leave this issue open for now.

          Show
          ofbizzer Christian Carlow added a comment - Hey Jacques, I've just determined that I'm not receiving the error on the demo site. Its probably occurring because of the recent changes I made for handling complex ship groups better. I guess whatever solution I find should apply to that issue instead of this one. I'll leave this issue open for now.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Actually I believe I have seen this error on the demo site and locally w/o your changes as well. It's there but randomly...

          Show
          jacques.le.roux Jacques Le Roux added a comment - Actually I believe I have seen this error on the demo site and locally w/o your changes as well. It's there but randomly...
          Hide
          ofbizzer Christian Carlow added a comment -

          Hey Jacques,

          I think you are right. I restarted my local dev server and tried it again and it worked fine. Before the restart I managed to debug to determine that the new empty ship group was added at index=0 when cart.setUserLogin(userLogin, dispatcher); is called on line 208 of ShoppingCartServices.loadCartFromOrder(). Here is the chain of function calls I followed that was causing the empty ship group to be added (sorry meant to copy call stack):

          ShoppingCartServices.loadCartFromOrder()
          setUserLogin()
          doPromotions()
          runProductPromos()
          doPromotions()
          runProductPromoRules()
          performAction()

          Within the ProductPromoWorker.performAction() that was eventually called line 1531 was responsible for adding the empty ship group:
          gwpItem = ShoppingCartItem.makeItem(null, product, null, quantity, null, null, null, null, null, null, null, null, prodCatalogId, null, null, null, dispatcher, cart, Boolean.FALSE, Boolean.TRUE, null, Boolean.FALSE, Boolean.FALSE);

          The line is called as part of the productPromoActionEnumId="PROMO_GWP" logic. So it seems that somehow the cart ship groups are not yet set when a promo is added so a new empty one is created to support the promo item to be added.

          I'm still unable to reproduce intentionally.

          Show
          ofbizzer Christian Carlow added a comment - Hey Jacques, I think you are right. I restarted my local dev server and tried it again and it worked fine. Before the restart I managed to debug to determine that the new empty ship group was added at index=0 when cart.setUserLogin(userLogin, dispatcher); is called on line 208 of ShoppingCartServices.loadCartFromOrder(). Here is the chain of function calls I followed that was causing the empty ship group to be added (sorry meant to copy call stack): ShoppingCartServices.loadCartFromOrder() setUserLogin() doPromotions() runProductPromos() doPromotions() runProductPromoRules() performAction() Within the ProductPromoWorker.performAction() that was eventually called line 1531 was responsible for adding the empty ship group: gwpItem = ShoppingCartItem.makeItem(null, product, null, quantity, null, null, null, null, null, null, null, null, prodCatalogId, null, null, null, dispatcher, cart, Boolean.FALSE, Boolean.TRUE, null, Boolean.FALSE, Boolean.FALSE); The line is called as part of the productPromoActionEnumId="PROMO_GWP" logic. So it seems that somehow the cart ship groups are not yet set when a promo is added so a new empty one is created to support the promo item to be added. I'm still unable to reproduce intentionally.
          Hide
          ofbizzer Christian Carlow added a comment -

          I just encountered this error again on the Demo Site upon trying to update the price of the order item for orderId DEMO10090. I'm pretty sure the error is due to setUserLogin() being called before the ship groups get created for the cart in loadCartFromOrder(). setUserLogin() expects the ShipGroups to have already been created. To resolve the issue, orderItemShipGroup handling statements performed later in the function should be changed to be performed before setUserLogin() is called.

          I'm almost done with OFBIZ-5440 which required this issue to be fixed so it will be included for the patch for that issue.

          Show
          ofbizzer Christian Carlow added a comment - I just encountered this error again on the Demo Site upon trying to update the price of the order item for orderId DEMO10090. I'm pretty sure the error is due to setUserLogin() being called before the ship groups get created for the cart in loadCartFromOrder(). setUserLogin() expects the ShipGroups to have already been created. To resolve the issue, orderItemShipGroup handling statements performed later in the function should be changed to be performed before setUserLogin() is called. I'm almost done with OFBIZ-5440 which required this issue to be fixed so it will be included for the patch for that issue.
          Hide
          ofbizzer Christian Carlow added a comment -

          Patches in OFBIZ-5420 included changes to resolve this issue and will be extracted as a separate patch to resolve this issue.

          Show
          ofbizzer Christian Carlow added a comment - Patches in OFBIZ-5420 included changes to resolve this issue and will be extracted as a separate patch to resolve this issue.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          I got this again today using a local instance. I could not reproduce on a custom test server but I could OOTB on trunk demo adding a GZ-1005 to the DEMO10090 order I could reproduce. I will investigate this again because it's blocking me on OFBIZ-5761. Even if I'm now sure it's unrelated to pending work for OFBIZ-5761 (pfew, I prefer that!) I'm using locally (Nicolas's patch with minor unrelated changes), I want to be sure I dont mix things. Because the custom test server uses a slightly different code (my last patch for OFBIZ-5761) and I can't reproduce there. We will get it!

          Show
          jacques.le.roux Jacques Le Roux added a comment - I got this again today using a local instance. I could not reproduce on a custom test server but I could OOTB on trunk demo adding a GZ-1005 to the DEMO10090 order I could reproduce. I will investigate this again because it's blocking me on OFBIZ-5761 . Even if I'm now sure it's unrelated to pending work for OFBIZ-5761 (pfew, I prefer that!) I'm using locally (Nicolas's patch with minor unrelated changes), I want to be sure I dont mix things. Because the custom test server uses a slightly different code (my last patch for OFBIZ-5761 ) and I can't reproduce there. We will get it!
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          BTW Christian, did you get a chance to extract the relevant fixing changes?

          Show
          jacques.le.roux Jacques Le Roux added a comment - BTW Christian, did you get a chance to extract the relevant fixing changes?
          Hide
          jacques.le.roux Jacques Le Roux added a comment - - edited

          I found the case when this happens. It's when you add a new ship group. Then even if you use the 1st ship group, which has a shipping method, the error appears. The good news is the change in OFBIZ-5761 will fix this issue

          Show
          jacques.le.roux Jacques Le Roux added a comment - - edited I found the case when this happens. It's when you add a new ship group. Then even if you use the 1st ship group, which has a shipping method, the error appears. The good news is the change in OFBIZ-5761 will fix this issue
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Actually the problem is still there. I will give it a chance today!

          Show
          jacques.le.roux Jacques Le Roux added a comment - Actually the problem is still there. I will give it a chance today!
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Fixed in
          trunk r1643341
          R13.07 r1643342
          R12.04 r1643343

          Show
          jacques.le.roux Jacques Le Roux added a comment - Fixed in trunk r1643341 R13.07 r1643342 R12.04 r1643343
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          For the relases branches you still need to update the 2nd (etc.) created ship group shipping method by hand...

          Show
          jacques.le.roux Jacques Le Roux added a comment - For the relases branches you still need to update the 2nd (etc.) created ship group shipping method by hand...

            People

            • Assignee:
              jacques.le.roux Jacques Le Roux
              Reporter:
              ofbizzer Christian Carlow
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development