ofbiz-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ki...@objectedge.com
Subject Incorrect use of eca for create/updateShipment
Date Thu, 03 Nov 2011 17:15:55 GMT
Hello Friends,

In case of createShipment, during commit, eca rules are fired. These 
invoke updateShipment(i.e: even before commit on createShipment is 
complete). Update Shipment is called multiple times (You can view the log 
during quickShipOrder/quickDropShipOrder). Also, these rules are fired in 
incorrect order. These rules are updating the same shipment that is being 
committed by the original method. I believe this is incorrect. 

I have verified the functionality of quickShipOrder/quickDropShipOrder 
after the changes. Let me know if there are any testcases that I need to 
run. Please take a look at email thread for more details. Let me know if 
you have questions and concerns. If we integrate the change sooner, we can 
avoid merge conflicts.

PS: Thanks Adrian and Anil for your vote of confidence. As per your 
recommendation, I am posting this to dev mailing list.

Regards,
Kiran Gawde

Senior Software Architect
Object Edge Inc
(925) 943 5558 x108

"There are two kind of people: Those who do the work and those who take 
the credit. Try to be in the first group because there is less competition 
there."
"Never give up on what you really want to do. The person with big dreams 
is more powerful than one with all the facts".




From:   "Adrian Crum (Commented) (JIRA)" <jira@apache.org>
To:     kiran@objectedge.com
Date:   11/03/2011 02:04 AM
Subject:        [jira] [Commented] (OFBIZ-4501) Incorrect use of eca for 
create/updateShipment




    [ 
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

 


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message