You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2011/10/17 16:03:30 UTC
svn commit: r1185179 - in /ofbiz/trunk/applications/product:
script/org/ofbiz/shipment/issuance/IssuanceServices.xml
src/org/ofbiz/shipment/test/
Author: jleroux
Date: Mon Oct 17 14:03:30 2011
New Revision: 1185179
URL: http://svn.apache.org/viewvc?rev=1185179&view=rev
Log:
A patch from Paul Foxworthy "Order not completed when filled from more than one InventoryItem" https://issues.apache.org/jira/browse/OFBIZ-4386
There is a nasty bug in IssuanceServices in the situation where the order is filled from more than one InventoryItem. When the order is created, there are OrderItemShipGrpInvRes reservations created for all the inventory items needed to fill the order.
In the simple method issueOrderItemShipGrpInvResToShipment in IssuanceServices, at line 173 (https://fisheye6.atlassian.com/browse/ofbiz/trunk/applications/product/script/org/ofbiz/shipment/issuance/IssuanceServices.xml?hb=true#to173), there is an assumption that if the OrderShipment already exists, we are some sort of adjustment, and we should adjust the OrderShipment quantity by the difference between the old OrderShipment quantity and the new one. This all works fine in the situation where there is only one InventoryItem needed to fill the order. However, if there are two or more InventoryItems, the simple method is called twice, and an incorrect quantity is calculated.
The crux of the problem is that OrderShipment doesn't care about InventoryItems, so there will be only one OrderShipment row for a given product. In contrast, ItemIssuance does care about InventoryItems, so there will be more than one if the product is supplied from more than one InventoryItem. The ItemIssuance code doesn't take account of this distinction.
I have provided two patches. One is a new unit test that exemplifies the bug. If you just apply this patch to trunk, the unit test will fail with an OrderShipment quantity of 4 when it ought to be 6.
The other patch is my fix for IssuanceServices, which now looks for a relevant ItemIssuance to decide if we are adjusting quantities, or creating a new ItemIssuance (and possibly a new OrderShipment).
jleroux: I commit patch by patch to be able to backport the fix, here only the fix
Added:
ofbiz/trunk/applications/product/src/org/ofbiz/shipment/test/ (with props)
Modified:
ofbiz/trunk/applications/product/script/org/ofbiz/shipment/issuance/IssuanceServices.xml
Modified: ofbiz/trunk/applications/product/script/org/ofbiz/shipment/issuance/IssuanceServices.xml
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/product/script/org/ofbiz/shipment/issuance/IssuanceServices.xml?rev=1185179&r1=1185178&r2=1185179&view=diff
==============================================================================
--- ofbiz/trunk/applications/product/script/org/ofbiz/shipment/issuance/IssuanceServices.xml (original)
+++ ofbiz/trunk/applications/product/script/org/ofbiz/shipment/issuance/IssuanceServices.xml Mon Oct 17 14:03:30 2011
@@ -185,23 +185,18 @@ under the License.
<set-pk-fields value-field="orderShipmentLookupPk" map="parameters"/>
<find-by-and entity-name="OrderShipment" map="orderShipmentLookupPk" list="orderShipments"/>
<first-from-list entry="orderShipment" list="orderShipments"/>
- <!-- qtyForShipmentItem is the quantity we will add to the ShipmentItem -->
- <!-- OrderShipment.quantity stores the OrderItem.quantity planned in the shipment -->
- <calculate field="qtyForShipmentItem">
- <calcop field="parameters.quantity" operator="subtract">
- <calcop operator="get" field="orderShipment.quantity"/>
- </calcop>
- </calculate>
+
+ <!-- will create qtyForShipmentItem -->
+ <call-simple-method method-name="calcQtyForShipmentItemInline"/>
+ <log level="info" message="qtyForShipmentItem: ${qtyForShipmentItem}"/>
<if-compare value="0" operator="greater-equals" field="qtyForShipmentItem" type="BigDecimal">
- <!-- remove the orderShipment.quantity -->
<if-not-empty field="orderShipment">
<field-to-result field="orderShipment.shipmentItemSeqId" result-name="shipmentItemSeqId"/>
<make-value entity-name="ShipmentItem" value-field="shipmentItemLookupPk"/>
<set-pk-fields value-field="shipmentItemLookupPk" map="parameters"/>
<set from-field="orderShipment.shipmentItemSeqId" field="shipmentItemLookupPk.shipmentItemSeqId"/>
<find-by-primary-key map="shipmentItemLookupPk" value-field="shipmentItem"/>
- <remove-value value-field="orderShipment"/>
</if-not-empty>
<if-compare value="0" operator="not-equals" field="qtyForShipmentItem" type="BigDecimal">
<!-- add the qtyForShipmentItem to the shipmentItem -->
@@ -211,7 +206,7 @@ under the License.
<set from-field="originalQuantity" field="parameters.quantity"/>
</if-compare>
<else>
- <!-- decrement the orderShipment.quantity -->
+ <!-- A reduction in the quantity, so OrderShipment must exist. -->
<calculate field="orderShipment.quantity">
<calcop field="orderShipment.quantity" operator="subtract">
<calcop operator="get" field="parameters.quantity"/>
@@ -226,7 +221,6 @@ under the License.
</else>
</if-compare>
- <!--<call-simple-method method-name="findCreateIssueShipmentItem"/>-->
<set field="eventDate" from-field="parameters.eventDate"/>
<set field="shipmentId" from-field="parameters.shipmentId"/>
<call-simple-method method-name="findCreateItemIssuance"/>
@@ -251,6 +245,10 @@ under the License.
<set from-field="orderItem.orderItemSeqId" field="changeOrderItemStatusMap.orderItemSeqId"/>
<call-service service-name="changeOrderItemStatus" in-map-name="changeOrderItemStatusMap"/>
</if-empty>
+ <else>
+ <log level="info" message="orderId: ${orderItem.orderId} orderItemSeqId: ${orderItem.orderItemSeqId}"/>
+ <log level="info" message="Items issued but can't set order item status to ITEM_COMPLETED because shipment status is SHIPMENT_SCHEDULED" />
+ </else>
</if-compare>
<else>
<store-value value-field="orderItemShipGrpInvRes"/>
@@ -274,6 +272,51 @@ under the License.
</simple-method>
<!-- some inline methods for the issuance process -->
+
+ <simple-method method-name="calcQtyForShipmentItemInline" short-description="Calculate quantity for a shipment item - meant to be called in-line">
+
+ <!-- If our order has reserved a particular inventoryItemId, other InventoryItemIds
+ should not contribute to the adjustment calculation here
+ -->
+ <if-not-empty field="parameters.inventoryItemId">
+ <entity-and list="itemIssuances" entity-name="ItemIssuance">
+ <field-map field-name="orderId" from-field="parameters.orderId"/>
+ <field-map field-name="orderItemSeqId" from-field="parameters.orderItemSeqId"/>
+ <field-map field-name="shipGroupSeqId" from-field="parameters.shipGroupSeqId"/>
+ <field-map field-name="shipmentId" from-field="parameters.shipmentId"/>
+ <order-by field-name="-issuedDateTime"/>
+ </entity-and>
+
+ <set field="otherInventoryItemQuantity" value="0" />
+ <iterate list="itemIssuances" entry="itemIssuance">
+ <if-compare-field field="itemIssuance.inventoryItemId" operator="not-equals" to-field="parameters.inventoryItemId">
+ <calculate field="otherInventoryItemQuantity">
+ <calcop field="otherInventoryItemQuantity" operator="add">
+ <calcop operator="get" field="itemIssuance.quantity"/>
+ </calcop>
+ </calculate>
+ </if-compare-field>
+ </iterate>
+ </if-not-empty>
+
+ <!-- If the shipmentItem includes products from more than one inventoryItemId, any items that came from a different inventoryItemId
+ from the current one should be ignored as we calculate the adjustment to make.
+ -->
+ <calculate field="orderShipmentAmount">
+ <calcop field="orderShipment.quantity" operator="subtract">
+ <calcop operator="get" field="otherInventoryItemQuantity"/>
+ </calcop>
+ </calculate>
+
+ <!-- qtyForShipmentItem is the quantity we will add to the ShipmentItem -->
+ <calculate field="qtyForShipmentItem">
+ <calcop field="parameters.quantity" operator="subtract">
+ <calcop operator="get" field="orderShipmentAmount"/>
+ </calcop>
+ </calculate>
+
+ </simple-method>
+
<simple-method method-name="findCreateIssueShipmentItem" short-description="Find or Create ShipmentItem to Issue To - meant to be called in-line">
<!-- try to find an existing shipmentItem and attach to it, if none found create a new shipmentItem -->
<!-- if there is NO productId on the orderItem, ALWAYS create a new shipmentItem -->
@@ -300,27 +343,52 @@ under the License.
<find-by-primary-key entity-name="ShipmentItem" map="shipmentItemLookupPk" value-field="shipmentItem"/>
<else>
<calculate field="shipmentItem.quantity">
- <calcop operator="get" field="shipmentItem.quantity"/>
- <calcop operator="get" field="parameters.quantity"/>
+ <calcop operator="add" field="shipmentItem.quantity">
+ <calcop operator="get" field="parameters.quantity"/>
+ </calcop>
</calculate>
<store-value value-field="shipmentItem"/>
</else>
</if-empty>
+
+ <call-simple-method method-name="createOrUpdateOrderShipmentInline" />
+
+ <field-to-result field="shipmentItem.shipmentItemSeqId" result-name="shipmentItemSeqId"/>
+ </simple-method>
+
+ <simple-method method-name="createOrUpdateOrderShipmentInline" short-description="Create or update the OrderShipment - meant to be called in-line">
<set from-field="parameters.shipmentId" field="orderShipmentCreate.shipmentId"/>
<set from-field="shipmentItem.shipmentItemSeqId" field="orderShipmentCreate.shipmentItemSeqId"/>
<set from-field="orderItem.orderId" field="orderShipmentCreate.orderId"/>
<set from-field="orderItem.orderItemSeqId" field="orderShipmentCreate.orderItemSeqId"/>
+
<if-not-empty field="orderItemShipGroupAssoc">
<set from-field="orderItemShipGroupAssoc.shipGroupSeqId" field="orderShipmentCreate.shipGroupSeqId"/>
</if-not-empty>
<if-not-empty field="orderItemShipGrpInvRes">
<set from-field="orderItemShipGrpInvRes.shipGroupSeqId" field="orderShipmentCreate.shipGroupSeqId"/>
</if-not-empty>
- <set from-field="parameters.quantity" field="orderShipmentCreate.quantity"/>
- <call-service service-name="createOrderShipment" in-map-name="orderShipmentCreate"/>
+
- <field-to-result field="shipmentItem.shipmentItemSeqId" result-name="shipmentItemSeqId"/>
+ <make-value entity-name="OrderShipment" value-field="orderShipmentLookupPk"/>
+ <set-pk-fields value-field="orderShipmentLookupPk" map="orderShipmentCreate"/>
+ <find-by-and entity-name="OrderShipment" map="orderShipmentLookupPk" list="orderShipments"/>
+ <first-from-list entry="orderShipment" list="orderShipments"/>
+
+ <if-empty field="orderShipment">
+ <set from-field="parameters.quantity" field="orderShipmentCreate.quantity"/>
+ <call-service service-name="createOrderShipment" in-map-name="orderShipmentCreate"/>
+ <else>
+ <calculate field="orderShipment.quantity">
+ <calcop field="orderShipment.quantity" operator="add">
+ <calcop operator="get" field="parameters.quantity"/>
+ </calcop>
+ </calculate>
+ <store-value value-field="orderShipment"/>
+ </else>
+ </if-empty>
</simple-method>
+
<simple-method method-name="findCreateItemIssuance" short-description="Find Create ItemIssuance - meant to be called in-line">
<!-- If a non-sales order find ItemIssuance for orderItemSeqId-shimentItemSeqId-shipGroupSeqId pair, update it and return -->
<if-compare field="orderHeader.orderTypeId" operator="not-equals" value="SALES_ORDER">
Propchange: ofbiz/trunk/applications/product/src/org/ofbiz/shipment/test/
------------------------------------------------------------------------------
bugtraq:number = true