[jira] [Created] (OFBIZ-4501) Incorrect use of eca for create/updateShipment

[Nicolas Malin (Jira)]

Incorrect use of eca for create/updateShipment
----------------------------------------------

                 Key: OFBIZ-4501
                 URL: https://issues.apache.org/jira/browse/OFBIZ-4501
             Project: OFBiz
          Issue Type: Bug
          Components: product
    Affects Versions: Release Branch 11.04, SVN trunk
            Reporter: Kiran Gawde


createShipment service doesn't populate the facility and order info into shipment. Instead it is handled by eca rules. This is wrong. ECA rules should be used to update other objects or cause other actions and not update the object that is being committed. This makes it difficult to traverse the code. Can also cause bugs that are difficult troubleshoot. e.g: In this case, facilities are populated in shipment by method setShipmentSettingsFromPrimaryOrder, but eca rule checking for originFacilityId gets executed before it is populated. Following eca rules should be removed and instead the code should be added to create/updateshipment methods.


    <!-- if new originFacilityId or destinationFacilityId, get settings from facilities -->
    <eca service="createShipment" event="commit">
        <condition field-name="originFacilityId" operator="is-not-empty"/>
        <action service="setShipmentSettingsFromFacilities" mode="sync"/>
    </eca>
    <eca service="createShipment" event="commit">
        <condition field-name="destinationFacilityId" operator="is-not-empty"/>
        <action service="setShipmentSettingsFromFacilities" mode="sync"/>
    </eca>
    <eca service="updateShipment" event="commit">
        <condition-field field-name="originFacilityId" operator="not-equals" to-field-name="oldOriginFacilityId"/>
        <condition field-name="originFacilityId" operator="is-not-empty"/>
        <action service="setShipmentSettingsFromFacilities" mode="sync"/>
    </eca>
    <eca service="updateShipment" event="commit">
        <condition-field field-name="destinationFacilityId" operator="not-equals" to-field-name="oldDestinationFacilityId"/>
        <condition field-name="destinationFacilityId" operator="is-not-empty"/>
        <action service="setShipmentSettingsFromFacilities" mode="sync"/>
    </eca>

    <!-- if new primaryOrderId, get settings from order -->
    <eca service="createShipment" event="commit">
        <condition field-name="primaryOrderId" operator="is-not-empty"/>
        <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
    </eca>

    <eca service="updateShipment" event="commit">
        <condition-field field-name="primaryOrderId" operator="not-equals" to-field-name="oldPrimaryOrderId"/>
        <condition field-name="primaryOrderId" operator="is-not-empty"/>
        <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
    </eca>

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-4501?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kiran Gawde updated OFBIZ-4501:
-------------------------------

    Attachment: OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch

Changes are done to 4 files:
controller.xml, services_shipping.xml, secas_shipment.xml(Removed reference to setShipmentSettingsFromPrimaryOrder, setShipmentSettingsFromFacilities)

ShipmentServices.xml(Changes look a lot, but most are just renaming of newEntity and lookedUpValue to shipment)
               

> Incorrect use of eca for create/updateShipment
> ----------------------------------------------
>
>                 Key: OFBIZ-4501
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-4501
>             Project: OFBiz
>          Issue Type: Bug
>          Components: product
>    Affects Versions: Release Branch 11.04, SVN trunk
>            Reporter: Kiran Gawde
>         Attachments: OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch
>
>
> createShipment service doesn't populate the facility and order info into shipment. Instead it is handled by eca rules. This is wrong. ECA rules should be used to update other objects or cause other actions and not update the object that is being committed. This makes it difficult to traverse the code. Can also cause bugs that are difficult troubleshoot. e.g: In this case, facilities are populated in shipment by method setShipmentSettingsFromPrimaryOrder, but eca rule checking for originFacilityId gets executed before it is populated. Following eca rules should be removed and instead the code should be added to create/updateshipment methods.
>     <!-- if new originFacilityId or destinationFacilityId, get settings from facilities -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="createShipment" event="commit">
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="originFacilityId" operator="not-equals" to-field-name="oldOriginFacilityId"/>
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="destinationFacilityId" operator="not-equals" to-field-name="oldDestinationFacilityId"/>
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <!-- if new primaryOrderId, get settings from order -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="primaryOrderId" operator="not-equals" to-field-name="oldPrimaryOrderId"/>
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-4501?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kiran Gawde updated OFBIZ-4501:
-------------------------------

    Attachment: OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch
                OFBIZ-4501-ShipmentServiceXml.patch

Moved ShipmentService.xml changes in separate file for clarity. Also fixed the facilityId for DROP_SHIPMENT order
               

> Incorrect use of eca for create/updateShipment
> ----------------------------------------------
>
>                 Key: OFBIZ-4501
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-4501
>             Project: OFBiz
>          Issue Type: Bug
>          Components: product
>    Affects Versions: Release Branch 11.04, SVN trunk
>            Reporter: Kiran Gawde
>         Attachments: OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ShipmentServiceXml.patch
>
>
> createShipment service doesn't populate the facility and order info into shipment. Instead it is handled by eca rules. This is wrong. ECA rules should be used to update other objects or cause other actions and not update the object that is being committed. This makes it difficult to traverse the code. Can also cause bugs that are difficult troubleshoot. e.g: In this case, facilities are populated in shipment by method setShipmentSettingsFromPrimaryOrder, but eca rule checking for originFacilityId gets executed before it is populated. Following eca rules should be removed and instead the code should be added to create/updateshipment methods.
>     <!-- if new originFacilityId or destinationFacilityId, get settings from facilities -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="createShipment" event="commit">
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="originFacilityId" operator="not-equals" to-field-name="oldOriginFacilityId"/>
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="destinationFacilityId" operator="not-equals" to-field-name="oldDestinationFacilityId"/>
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <!-- if new primaryOrderId, get settings from order -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="primaryOrderId" operator="not-equals" to-field-name="oldPrimaryOrderId"/>
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-4501?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kiran Gawde updated OFBIZ-4501:
-------------------------------

    Attachment: OFBIZ-4501-ShipmentServiceXml.patch

Replaced tabs with spaces. And added following before calling setShipmentSettingsFromPrimaryOrder:

<set from-field="shipment.shipmentId" field="parameters.shipmentId"/>
               

> Incorrect use of eca for create/updateShipment
> ----------------------------------------------
>
>                 Key: OFBIZ-4501
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-4501
>             Project: OFBiz
>          Issue Type: Bug
>          Components: product
>    Affects Versions: Release Branch 11.04, SVN trunk
>            Reporter: Kiran Gawde
>         Attachments: OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch
>
>
> createShipment service doesn't populate the facility and order info into shipment. Instead it is handled by eca rules. This is wrong. ECA rules should be used to update other objects or cause other actions and not update the object that is being committed. This makes it difficult to traverse the code. Can also cause bugs that are difficult troubleshoot. e.g: In this case, facilities are populated in shipment by method setShipmentSettingsFromPrimaryOrder, but eca rule checking for originFacilityId gets executed before it is populated. Following eca rules should be removed and instead the code should be added to create/updateshipment methods.
>     <!-- if new originFacilityId or destinationFacilityId, get settings from facilities -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="createShipment" event="commit">
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="originFacilityId" operator="not-equals" to-field-name="oldOriginFacilityId"/>
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="destinationFacilityId" operator="not-equals" to-field-name="oldDestinationFacilityId"/>
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <!-- if new primaryOrderId, get settings from order -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="primaryOrderId" operator="not-equals" to-field-name="oldPrimaryOrderId"/>
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-4501?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kiran Gawde updated OFBIZ-4501:
-------------------------------

    Attachment: OFBIZ-4501-ShipmentServiceXml.patch

Separated setShipmentPartiesAndContacts & createShipmentRouteSegments.

Now following sequence is followed while creating/updating the shipment:
setShipmentSettingsFromPrimaryOrder
setShipmentSettingsFromFacilities
setShipmentPartiesAndContacts
createShipmentRouteSegments
               

> Incorrect use of eca for create/updateShipment
> ----------------------------------------------
>
>                 Key: OFBIZ-4501
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-4501
>             Project: OFBiz
>          Issue Type: Bug
>          Components: product
>    Affects Versions: Release Branch 11.04, SVN trunk
>            Reporter: Kiran Gawde
>         Attachments: OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch
>
>
> createShipment service doesn't populate the facility and order info into shipment. Instead it is handled by eca rules. This is wrong. ECA rules should be used to update other objects or cause other actions and not update the object that is being committed. This makes it difficult to traverse the code. Can also cause bugs that are difficult troubleshoot. e.g: In this case, facilities are populated in shipment by method setShipmentSettingsFromPrimaryOrder, but eca rule checking for originFacilityId gets executed before it is populated. Following eca rules should be removed and instead the code should be added to create/updateshipment methods.
>     <!-- if new originFacilityId or destinationFacilityId, get settings from facilities -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="createShipment" event="commit">
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="originFacilityId" operator="not-equals" to-field-name="oldOriginFacilityId"/>
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="destinationFacilityId" operator="not-equals" to-field-name="oldDestinationFacilityId"/>
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <!-- if new primaryOrderId, get settings from order -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="primaryOrderId" operator="not-equals" to-field-name="oldPrimaryOrderId"/>
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-4501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13142850#comment-13142850 ]

Kiran Gawde commented on OFBIZ-4501:
------------------------------------

Hello Friends,

Any ETA on getting this integrated on 11.04 branch? I already have this fix in my vendor branch, so I am not dependent. But in case, there are any questions or suggestions, I can alter the code while the change is fresh in mind. Also, it would avoid future merge issues.

Cheers,
Kiran
               

> Incorrect use of eca for create/updateShipment
> ----------------------------------------------
>
>                 Key: OFBIZ-4501
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-4501
>             Project: OFBiz
>          Issue Type: Bug
>          Components: product
>    Affects Versions: Release Branch 11.04, SVN trunk
>            Reporter: Kiran Gawde
>         Attachments: OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch
>
>
> createShipment service doesn't populate the facility and order info into shipment. Instead it is handled by eca rules. This is wrong. ECA rules should be used to update other objects or cause other actions and not update the object that is being committed. This makes it difficult to traverse the code. Can also cause bugs that are difficult troubleshoot. e.g: In this case, facilities are populated in shipment by method setShipmentSettingsFromPrimaryOrder, but eca rule checking for originFacilityId gets executed before it is populated. Following eca rules should be removed and instead the code should be added to create/updateshipment methods.
>     <!-- if new originFacilityId or destinationFacilityId, get settings from facilities -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="createShipment" event="commit">
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="originFacilityId" operator="not-equals" to-field-name="oldOriginFacilityId"/>
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="destinationFacilityId" operator="not-equals" to-field-name="oldDestinationFacilityId"/>
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <!-- if new primaryOrderId, get settings from order -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="primaryOrderId" operator="not-equals" to-field-name="oldPrimaryOrderId"/>
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-4501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13142972#comment-13142972 ]

Adrian Crum commented on OFBIZ-4501:
------------------------------------

Kiran,

Thank you for working on this. I agree that the overuse of ECAs causes problems and makes the services difficult to troubleshoot. But removing them is going to be a tough sell because many in the community see it as a feature - they see it as a crude form of a workflow implementation. I have added my vote to this issue because I believe a lot of the ECAs should go away. It might help your cause to start a discussion on the dev mailing list and see if you can rally some more support for ECA removal.

               

> Incorrect use of eca for create/updateShipment
> ----------------------------------------------
>
>                 Key: OFBIZ-4501
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-4501
>             Project: OFBiz
>          Issue Type: Bug
>          Components: product
>    Affects Versions: Release Branch 11.04, SVN trunk
>            Reporter: Kiran Gawde
>         Attachments: OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch
>
>
> createShipment service doesn't populate the facility and order info into shipment. Instead it is handled by eca rules. This is wrong. ECA rules should be used to update other objects or cause other actions and not update the object that is being committed. This makes it difficult to traverse the code. Can also cause bugs that are difficult troubleshoot. e.g: In this case, facilities are populated in shipment by method setShipmentSettingsFromPrimaryOrder, but eca rule checking for originFacilityId gets executed before it is populated. Following eca rules should be removed and instead the code should be added to create/updateshipment methods.
>     <!-- if new originFacilityId or destinationFacilityId, get settings from facilities -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="createShipment" event="commit">
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="originFacilityId" operator="not-equals" to-field-name="oldOriginFacilityId"/>
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="destinationFacilityId" operator="not-equals" to-field-name="oldDestinationFacilityId"/>
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <!-- if new primaryOrderId, get settings from order -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="primaryOrderId" operator="not-equals" to-field-name="oldPrimaryOrderId"/>
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-4501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13143075#comment-13143075 ]

Anil K Patel commented on OFBIZ-4501:
-------------------------------------

Thanks for inputs.
I like the theme of this thread and plan to spend sometime review the code and test it. Don't know how soon it might be so did not assign task to myself.
               

> Incorrect use of eca for create/updateShipment
> ----------------------------------------------
>
>                 Key: OFBIZ-4501
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-4501
>             Project: OFBiz
>          Issue Type: Bug
>          Components: product
>    Affects Versions: Release Branch 11.04, SVN trunk
>            Reporter: Kiran Gawde
>         Attachments: OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch
>
>
> createShipment service doesn't populate the facility and order info into shipment. Instead it is handled by eca rules. This is wrong. ECA rules should be used to update other objects or cause other actions and not update the object that is being committed. This makes it difficult to traverse the code. Can also cause bugs that are difficult troubleshoot. e.g: In this case, facilities are populated in shipment by method setShipmentSettingsFromPrimaryOrder, but eca rule checking for originFacilityId gets executed before it is populated. Following eca rules should be removed and instead the code should be added to create/updateshipment methods.
>     <!-- if new originFacilityId or destinationFacilityId, get settings from facilities -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="createShipment" event="commit">
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="originFacilityId" operator="not-equals" to-field-name="oldOriginFacilityId"/>
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="destinationFacilityId" operator="not-equals" to-field-name="oldDestinationFacilityId"/>
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <!-- if new primaryOrderId, get settings from order -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="primaryOrderId" operator="not-equals" to-field-name="oldPrimaryOrderId"/>
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-4501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13144402#comment-13144402 ]

Jacques Le Roux commented on OFBIZ-4501:
----------------------------------------

Just FYI: part of this code is pretty old: http://svn.ofbiz.org/viewcvs/trunk/components/product/servicedef/secas_shipment.xml?rev=82&view=markup
At this time maybe things were far less complicated...
               

> Incorrect use of eca for create/updateShipment
> ----------------------------------------------
>
>                 Key: OFBIZ-4501
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-4501
>             Project: OFBiz
>          Issue Type: Bug
>          Components: product
>    Affects Versions: Release Branch 11.04, SVN trunk
>            Reporter: Kiran Gawde
>         Attachments: OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch
>
>
> createShipment service doesn't populate the facility and order info into shipment. Instead it is handled by eca rules. This is wrong. ECA rules should be used to update other objects or cause other actions and not update the object that is being committed. This makes it difficult to traverse the code. Can also cause bugs that are difficult troubleshoot. e.g: In this case, facilities are populated in shipment by method setShipmentSettingsFromPrimaryOrder, but eca rule checking for originFacilityId gets executed before it is populated. Following eca rules should be removed and instead the code should be added to create/updateshipment methods.
>     <!-- if new originFacilityId or destinationFacilityId, get settings from facilities -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="createShipment" event="commit">
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="originFacilityId" operator="not-equals" to-field-name="oldOriginFacilityId"/>
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="destinationFacilityId" operator="not-equals" to-field-name="oldDestinationFacilityId"/>
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <!-- if new primaryOrderId, get settings from order -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="primaryOrderId" operator="not-equals" to-field-name="oldPrimaryOrderId"/>
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

[Nicolas Malin (Jira)]

     [ https://issues.apache.org/jira/browse/OFBIZ-4501?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jacques Le Roux reassigned OFBIZ-4501:
--------------------------------------

    Assignee: Jacques Le Roux
   

> Incorrect use of eca for create/updateShipment
> ----------------------------------------------
>
>                 Key: OFBIZ-4501
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-4501
>             Project: OFBiz
>          Issue Type: Bug
>          Components: product
>    Affects Versions: Release Branch 11.04, SVN trunk
>            Reporter: Kiran Gawde
>            Assignee: Jacques Le Roux
>         Attachments: OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch
>
>
> createShipment service doesn't populate the facility and order info into shipment. Instead it is handled by eca rules. This is wrong. ECA rules should be used to update other objects or cause other actions and not update the object that is being committed. This makes it difficult to traverse the code. Can also cause bugs that are difficult troubleshoot. e.g: In this case, facilities are populated in shipment by method setShipmentSettingsFromPrimaryOrder, but eca rule checking for originFacilityId gets executed before it is populated. Following eca rules should be removed and instead the code should be added to create/updateshipment methods.
>     <!-- if new originFacilityId or destinationFacilityId, get settings from facilities -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="createShipment" event="commit">
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="originFacilityId" operator="not-equals" to-field-name="oldOriginFacilityId"/>
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="destinationFacilityId" operator="not-equals" to-field-name="oldDestinationFacilityId"/>
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <!-- if new primaryOrderId, get settings from order -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="primaryOrderId" operator="not-equals" to-field-name="oldPrimaryOrderId"/>
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-4501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13236743#comment-13236743 ]

Jacques Le Roux commented on OFBIZ-4501:
----------------------------------------

Hi Anil,

Did you get chances to review and test?
               

> Incorrect use of eca for create/updateShipment
> ----------------------------------------------
>
>                 Key: OFBIZ-4501
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-4501
>             Project: OFBiz
>          Issue Type: Bug
>          Components: product
>    Affects Versions: Release Branch 11.04, SVN trunk
>            Reporter: Kiran Gawde
>            Assignee: Jacques Le Roux
>         Attachments: OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch
>
>
> createShipment service doesn't populate the facility and order info into shipment. Instead it is handled by eca rules. This is wrong. ECA rules should be used to update other objects or cause other actions and not update the object that is being committed. This makes it difficult to traverse the code. Can also cause bugs that are difficult troubleshoot. e.g: In this case, facilities are populated in shipment by method setShipmentSettingsFromPrimaryOrder, but eca rule checking for originFacilityId gets executed before it is populated. Following eca rules should be removed and instead the code should be added to create/updateshipment methods.
>     <!-- if new originFacilityId or destinationFacilityId, get settings from facilities -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="createShipment" event="commit">
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="originFacilityId" operator="not-equals" to-field-name="oldOriginFacilityId"/>
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="destinationFacilityId" operator="not-equals" to-field-name="oldDestinationFacilityId"/>
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <!-- if new primaryOrderId, get settings from order -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="primaryOrderId" operator="not-equals" to-field-name="oldPrimaryOrderId"/>
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-4501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13259409#comment-13259409 ]

Jacques Le Roux commented on OFBIZ-4501:
----------------------------------------

One month, still on it? ;)
               

> Incorrect use of eca for create/updateShipment
> ----------------------------------------------
>
>                 Key: OFBIZ-4501
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-4501
>             Project: OFBiz
>          Issue Type: Bug
>          Components: product
>    Affects Versions: Release Branch 11.04, SVN trunk
>            Reporter: Kiran Gawde
>            Assignee: Jacques Le Roux
>         Attachments: OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch
>
>
> createShipment service doesn't populate the facility and order info into shipment. Instead it is handled by eca rules. This is wrong. ECA rules should be used to update other objects or cause other actions and not update the object that is being committed. This makes it difficult to traverse the code. Can also cause bugs that are difficult troubleshoot. e.g: In this case, facilities are populated in shipment by method setShipmentSettingsFromPrimaryOrder, but eca rule checking for originFacilityId gets executed before it is populated. Following eca rules should be removed and instead the code should be added to create/updateshipment methods.
>     <!-- if new originFacilityId or destinationFacilityId, get settings from facilities -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="createShipment" event="commit">
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="originFacilityId" operator="not-equals" to-field-name="oldOriginFacilityId"/>
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="destinationFacilityId" operator="not-equals" to-field-name="oldDestinationFacilityId"/>
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <!-- if new primaryOrderId, get settings from order -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="primaryOrderId" operator="not-equals" to-field-name="oldPrimaryOrderId"/>
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-4501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13464499#comment-13464499 ]

Jacques Le Roux commented on OFBIZ-4501:
----------------------------------------

Hi Anil, did you get a chance to review?

I don't know if it's the case but [it was blocking Kiran to contribute more|http://markmail.org/message/qyqrcbxm2p4xtdhk], Kiran?
               

> Incorrect use of eca for create/updateShipment
> ----------------------------------------------
>
>                 Key: OFBIZ-4501
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-4501
>             Project: OFBiz
>          Issue Type: Bug
>          Components: product
>    Affects Versions: Release Branch 11.04, SVN trunk
>            Reporter: Kiran Gawde
>            Assignee: Jacques Le Roux
>         Attachments: OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ModifiedCreateUpdateShipmentService.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch, OFBIZ-4501-ShipmentServiceXml.patch
>
>
> createShipment service doesn't populate the facility and order info into shipment. Instead it is handled by eca rules. This is wrong. ECA rules should be used to update other objects or cause other actions and not update the object that is being committed. This makes it difficult to traverse the code. Can also cause bugs that are difficult troubleshoot. e.g: In this case, facilities are populated in shipment by method setShipmentSettingsFromPrimaryOrder, but eca rule checking for originFacilityId gets executed before it is populated. Following eca rules should be removed and instead the code should be added to create/updateshipment methods.
>     <!-- if new originFacilityId or destinationFacilityId, get settings from facilities -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="createShipment" event="commit">
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="originFacilityId" operator="not-equals" to-field-name="oldOriginFacilityId"/>
>         <condition field-name="originFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="destinationFacilityId" operator="not-equals" to-field-name="oldDestinationFacilityId"/>
>         <condition field-name="destinationFacilityId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromFacilities" mode="sync"/>
>     </eca>
>     <!-- if new primaryOrderId, get settings from order -->
>     <eca service="createShipment" event="commit">
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>
>     <eca service="updateShipment" event="commit">
>         <condition-field field-name="primaryOrderId" operator="not-equals" to-field-name="oldPrimaryOrderId"/>
>         <condition field-name="primaryOrderId" operator="is-not-empty"/>
>         <action service="setShipmentSettingsFromPrimaryOrder" mode="sync"/>
>     </eca>

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira