OFBiz › OFBiz - Dev

[jira] [Created] (OFBIZ-4519) Order adjustment prorating does not work as expected for when creating an invoice for an order with partially canceled items

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

7 messages Options

Nicolas Malin (Jira)

[jira] [Created] (OFBIZ-4519) Order adjustment prorating does not work as expected for when creating an invoice for an order with partially canceled items

Order adjustment prorating does not work as expected for when creating an invoice for an order with partially canceled items
----------------------------------------------------------------------------------------------------------------------------

Key: OFBIZ-4519
URL: https://issues.apache.org/jira/browse/OFBIZ-4519
Project: OFBiz
Issue Type: Bug
Components: order
Affects Versions: SVN trunk
Reporter: Martin Kreidenweis

We had a problem with creating invoices for orders with partially canceled items (OrderItem.cancelQuantity > 0). The pro-rating of the order adjustments doesn't work as expected.

In the createInvoiceForOrder the billingQuantity passed to the service takes into account the cancelQuantity, whereas the quantity from the original order item used to prorate the order adjustment amount does not. The attached patch changes this by calling OrderReadHelper.getOrderItemQuantity() like done for the billItem orderItem, instead of reading the quantity from the original orderItem directly.
I'm not 100% sure this doesn't break anything else. Maybe there was a reason not to take the cancelQuantity into account here. It does not break any OFBiz tests though.

--
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)

[jira] [Updated] (OFBIZ-4519) Order adjustment prorating does not work as expected for when creating an invoice for an order with partially canceled items

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

Martin Kreidenweis updated OFBIZ-4519:
--------------------------------------

Attachment: OFBIZ-4519.patch

> Order adjustment prorating does not work as expected for when creating an invoice for an order with partially canceled items
> ----------------------------------------------------------------------------------------------------------------------------
>
> Key: OFBIZ-4519
> URL: https://issues.apache.org/jira/browse/OFBIZ-4519
> Project: OFBiz
> Issue Type: Bug
> Components: order
> Affects Versions: SVN trunk
> Reporter: Martin Kreidenweis
> Attachments: OFBIZ-4519.patch
>
>
> We had a problem with creating invoices for orders with partially canceled items (OrderItem.cancelQuantity > 0). The pro-rating of the order adjustments doesn't work as expected.
> In the createInvoiceForOrder the billingQuantity passed to the service takes into account the cancelQuantity, whereas the quantity from the original order item used to prorate the order adjustment amount does not. The attached patch changes this by calling OrderReadHelper.getOrderItemQuantity() like done for the billItem orderItem, instead of reading the quantity from the original orderItem directly.
> I'm not 100% sure this doesn't break anything else. Maybe there was a reason not to take the cancelQuantity into account here. It does not break any OFBiz tests though.

--
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)

[jira] [Commented] (OFBIZ-4519) Order adjustment prorating does not work as expected for when creating an invoice for an order with partially canceled items

In reply to this post by Nicolas Malin (Jira)

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

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

Martin,
Can you please check again? Current implementation looks correct. OrderReadHelper.getOrderItemQuantity() looks like following,
public static BigDecimal getOrderItemQuantity(GenericValue orderItem) {

BigDecimal cancelQty = orderItem.getBigDecimal("cancelQuantity");
BigDecimal orderQty = orderItem.getBigDecimal("quantity");

if (cancelQty == null) cancelQty = ZERO;
if (orderQty == null) orderQty = ZERO;

return orderQty.subtract(cancelQty).setScale(scale, rounding);
}

So what we are getting is Quantity billed and not original quantity, but we need original order item quantity to do the math in creating invoice process. Does this sound right or I miss something?

Nicolas Malin (Jira)

[jira] [Commented] (OFBIZ-4519) Order adjustment prorating does not work as expected for when creating an invoice for an order with partially canceled items

In reply to this post by Nicolas Malin (Jira)

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

Martin Kreidenweis commented on OFBIZ-4519:
-------------------------------------------

OK, I looked into it in more detail now. Let me try to explain:

The problem I see is not in {{OrderReadHelper.getOrderItemQuantity()}} -- this method works all right.

The problem seems to be in the {{InvoiceServices.createInvoiceForOrder()}}: In line 401 it calculates the {{billingQuantity}} using the {{orderedQuantity}} provided by {{OrderReadHelper}} as a base:
{code}
BigDecimal orderedQuantity = OrderReadHelper.getOrderItemQuantity(orderItem);
BigDecimal invoicedQuantity = OrderReadHelper.getOrderItemInvoicedQuantity(orderItem);
billingQuantity = orderedQuantity.subtract(invoicedQuantity);
{code}
So the in essence
*billingQuantity = originallyOrderedQuantity -- cancelQuantity -- alreadyInvoicedQuantity*

Later, the pro-rating of the item adjustments is done, starting at line 525, because if we split the invoicing of an order item over mutltiple invoices, we also want to split the related order adjustments. The amount to invoice for the adjustment is calculated like that:
*invoicedAdjustmentAmount = adjustmentAmount / originallyOrderedQuantity * billingQuantity*

The problem now is this: if we canceled an item partially (using the {{cancelOrderItem}} service) before creating the invoice by setting cancelQuantity, the calculation is done wrong. Let's assume we have unitPrice=100, originallyOrderedQuantity=10 and cancelQuantity=4 and alreadyInvoicedQuantity=0 because no invoice was created yet, and the sales tax adjustment adjustmentAmount=100 (10% tax).
Then if we want to create an invoice for all items,
*billingQuantity = 10 -- 4 -- 0 = 6*
*invoicedAdjustmentAmount = 100 / 10 * 6 = 60*
This might seem right at first sight, but the {{recalcTaxTotal}} has been run on the order, triggered by some SECA. And it created a compensating "Tax adjustment due to order change" to reduce the order total by the sales tax for the 4 canceled items.
This leads to a wrong invoice, because now the taxes for the canceled item are subtracted twice: once by the tax adjustment pro-rating and once by the negative order header tax adjustment item created by tax recalculation (orderHeaderAdjustmentAmount).
*invoiceTotal = (billingQuantity * unitPrice) + invoicedAdjustmentAmount -- orderHeaderAdjustmentAmount)*
In our example:
*invoiceTotal = 6 * 100 + 60 - 40 = 620*
So the $40 sales tax for canceled items are incorrectly subtracted twice.

What our patch does is changing the calculation of the invoicedAdjustmentAmount as follows:
*invoicedAdjustmentAmount = adjustmentAmount / (originallyOrderedQuantity -- cancelQuantity) * billingQuantity*

In our example that would be
*invoicedAdjustmentAmount = 100 / (10 -- 4) * 6 = 100 / 6 * 6 = 100*
*invoiceTotal = 6 * 100 + 100 - 40 = 660*
which is the correct total for 10% tax on 6 $100 items.

--

While investigating this I found another issue:
The {{recalcTaxTotal}} service is triggered by {{changeOrderItemStatus}} when an order item changes its status to {{ITEM_CANCELLED}}. This happens only when an item is completely canceled. It is not triggered by the {{cancelOrderItem}} service when partially canceling an order item (setting cancelQuantity). (The {{recreateOrderAdjustments}} service that is triggered here seems to affect promo adjustments only, somehow misleadingly.) So in the case where I cancel one order item partially and another one completely, the {{recalcTaxTotal}} service is run, but when I only cancel an item partially, it is not run. This is inconsistent, as the {{recalcTaxTotal}} actually changes the order total also in the case of partially canceled items. So I think it should be triggered in this case as well.

--

(In our installation we actually changed the the behavior of tax recalculation to not create a negative order header tax adjustments like the OFBiz {{recalcTaxTotal}} currently does, but to create separate, negative order item tax adjustments instead. This has the advantage that the current order item amount including adjustments can be easily displayed, e.g. in order PDF documents. This code cannot be easily contributed though, because we implemented this service as a Groovy class, which is not supported by the stock OFBiz build system currently.)

Nicolas Malin (Jira)

[jira] [Assigned] (OFBIZ-4519) Order adjustment prorating does not work as expected for when creating an invoice for an order with partially canceled items

In reply to this post by Nicolas Malin (Jira)

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

Anil K Patel reassigned OFBIZ-4519:
-----------------------------------

Assignee: Anil K Patel

> Order adjustment prorating does not work as expected for when creating an invoice for an order with partially canceled items
> ----------------------------------------------------------------------------------------------------------------------------
>
> Key: OFBIZ-4519
> URL: https://issues.apache.org/jira/browse/OFBIZ-4519
> Project: OFBiz
> Issue Type: Bug
> Components: order
> Affects Versions: SVN trunk
> Reporter: Martin Kreidenweis
> Assignee: Anil K Patel
> Attachments: OFBIZ-4519.patch
>
>
> We had a problem with creating invoices for orders with partially canceled items (OrderItem.cancelQuantity > 0). The pro-rating of the order adjustments doesn't work as expected.
> In the createInvoiceForOrder the billingQuantity passed to the service takes into account the cancelQuantity, whereas the quantity from the original order item used to prorate the order adjustment amount does not. The attached patch changes this by calling OrderReadHelper.getOrderItemQuantity() like done for the billItem orderItem, instead of reading the quantity from the original orderItem directly.
> I'm not 100% sure this doesn't break anything else. Maybe there was a reason not to take the cancelQuantity into account here. It does not break any OFBiz tests though.

Nicolas Malin (Jira)

[jira] [Resolved] (OFBIZ-4519) Order adjustment prorating does not work as expected for when creating an invoice for an order with partially canceled items

In reply to this post by Nicolas Malin (Jira)

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

Anil K Patel resolved OFBIZ-4519.
---------------------------------

Resolution: Fixed

Fix applied to trunk r1190134 and 11.0 r 1190135. Martin, thanks for contribution.

> Order adjustment prorating does not work as expected for when creating an invoice for an order with partially canceled items
> ----------------------------------------------------------------------------------------------------------------------------
>
> Key: OFBIZ-4519
> URL: https://issues.apache.org/jira/browse/OFBIZ-4519
> Project: OFBiz
> Issue Type: Bug
> Components: order
> Affects Versions: SVN trunk
> Reporter: Martin Kreidenweis
> Assignee: Anil K Patel
> Attachments: OFBIZ-4519.patch
>
>
> We had a problem with creating invoices for orders with partially canceled items (OrderItem.cancelQuantity > 0). The pro-rating of the order adjustments doesn't work as expected.
> In the createInvoiceForOrder the billingQuantity passed to the service takes into account the cancelQuantity, whereas the quantity from the original order item used to prorate the order adjustment amount does not. The attached patch changes this by calling OrderReadHelper.getOrderItemQuantity() like done for the billItem orderItem, instead of reading the quantity from the original orderItem directly.
> I'm not 100% sure this doesn't break anything else. Maybe there was a reason not to take the cancelQuantity into account here. It does not break any OFBiz tests though.

Nicolas Malin (Jira)

[jira] [Closed] (OFBIZ-4519) Order adjustment prorating does not work as expected for when creating an invoice for an order with partially canceled items

In reply to this post by Nicolas Malin (Jira)

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

Anil K Patel closed OFBIZ-4519.
-------------------------------

> Order adjustment prorating does not work as expected for when creating an invoice for an order with partially canceled items
> ----------------------------------------------------------------------------------------------------------------------------
>
> Key: OFBIZ-4519
> URL: https://issues.apache.org/jira/browse/OFBIZ-4519
> Project: OFBiz
> Issue Type: Bug
> Components: order
> Affects Versions: SVN trunk
> Reporter: Martin Kreidenweis
> Assignee: Anil K Patel
> Attachments: OFBIZ-4519.patch
>
>
> We had a problem with creating invoices for orders with partially canceled items (OrderItem.cancelQuantity > 0). The pro-rating of the order adjustments doesn't work as expected.
> In the createInvoiceForOrder the billingQuantity passed to the service takes into account the cancelQuantity, whereas the quantity from the original order item used to prorate the order adjustment amount does not. The attached patch changes this by calling OrderReadHelper.getOrderItemQuantity() like done for the billItem orderItem, instead of reading the quantity from the original orderItem directly.
> I'm not 100% sure this doesn't break anything else. Maybe there was a reason not to take the cancelQuantity into account here. It does not break any OFBiz tests though.