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

The drop-ship process behaves incorrectly when a combination of drop-ship and non-drop-ship products are added into the cart

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • Release Branch 18.12, Trunk
    • 18.12.01
    • order
    • None
    • OFBiz Community Day (May 2019)

    Description

      The drop-ship process behaves incorrectly when a combination of drop-ship and non-drop-ship products are added into the cart.
       
      Suppose we have two items in the cart:

      1. The first item is drop-shippable.
      2. The second item is not drop-shippable.

      For the first item, the implementation works fine.
      For the second item, this is not working correctly.
       
      For drop shipment process, cart method 'createDropShipGroups' decides RequirementMethodEnumId in the order of property set at ProductStore -> ProductFacility -> Product which work fine in case of all drop-ship products and if the drop-ship product(s) is added last in the cart.
       
      The issue I found is that if a combination of a dropship and non-drop ship products are added into the cart and the non-drop shippable product(s) is added after the drop shippable product(s) then the non-drop-shippable products also considered as drop-shippable.

      Attachments

        1. OFBIZ-11021.patch
          2 kB
          Pawan Verma

        Activity

          pawan Pawan Verma added a comment -

          Attached patch with the proper implementation.

          pawan Pawan Verma added a comment - Attached patch with the proper implementation.
          surajk Suraj Khurana added a comment -

          +1.
          Patch looks good to me.

          surajk Suraj Khurana added a comment - +1. Patch looks good to me.

          +1 sounds good to me.

          Thanks Pawan, well spotted and fixed, but would be easier for reviewers if you explained in a sentence or two what you did in your patch in functional terms

          jleroux Jacques Le Roux added a comment - +1 sounds good to me. Thanks Pawan, well spotted and fixed, but would be easier for reviewers if you explained in a sentence or two what you did in your patch in functional terms
          pawan Pawan Verma added a comment - - edited

          Sure jacques.le.roux,

          RequirementMethodEnumId should be decided in the order of property set at ProductStore -> Facility -> Product for each order item which was not working if the non-drop shippable product(s) is added after the drop shippable product(s).

          Original Flow: requirementMethodEnumId was initialized outside the loop and once it is initialized with Product level information it never gets re-initialized if Product is non-drop shippable.

          I have updated the code to use the storeRequirementMethodEnumId variable for store level information which is defined outside the loop and this will be used to initialize requirementMethodEnumId for each order item and then product level information can override it. That means for each product ProductStore -> Facility -> Product this rule will work properly.

          Hope It is more clear now. Thanks!

          pawan Pawan Verma added a comment - - edited Sure jacques.le.roux , RequirementMethodEnumId should be decided in the order of property set at ProductStore -> Facility -> Product for each order item which was not working if the non-drop shippable product(s) is added after the drop shippable product(s). Original Flow: requirementMethodEnumId was initialized outside the loop and once it is initialized with Product level information it never gets re-initialized if Product is non-drop shippable. I have updated the code to use the storeRequirementMethodEnumId variable for store level information which is defined outside the loop and this will be used to initialize requirementMethodEnumId for each order item and then product level information can override it. That means for each product  ProductStore -> Facility -> Product this rule will work properly. Hope It is more clear now. Thanks!
          jleroux Jacques Le Roux added a comment - - edited

          Thanks,

          It would be hard to make it more clear

          BTW my comment was more for new issues to come

          jleroux Jacques Le Roux added a comment - - edited Thanks, It would be hard to make it more clear BTW my comment was more for new issues to come
          surajk Suraj Khurana added a comment -

          This has been committed at:

          Trunk at rev#1859909

          Release 18.12 at rev #1859910

          surajk Suraj Khurana added a comment - This has been committed at: Trunk at rev#1859909 Release 18.12 at rev #1859910
          surajk Suraj Khurana added a comment -

          Thanks Pawan Verma for reporting the issue and providing the patch and Jacques Le Roux for review.

          surajk Suraj Khurana added a comment - Thanks Pawan Verma for reporting the issue and providing the patch and Jacques Le Roux for review.

          People

            surajk Suraj Khurana
            pawan Pawan Verma
            Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: