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

Taxes are not handled correctly when creating accounting transactions from purchase invoices

    Details

    • Type: Bug
    • Status: Patch Available
    • Priority: Trivial
    • Resolution: Unresolved
    • Affects Version/s: Trunk
    • Fix Version/s: None
    • Component/s: accounting
    • Labels:
    • Sprint:
      Bug Crush Event - 21/2/2015

      Description

      Taxes are not handled correctly when creating accounting transactions from purchase invoices in GeneralLedgerServices#createAcctgTransForPurchaseInvoice

      1. OFBIZ-4514_combined.patch
        4 kB
        Deepak Nigam
      2. OFBIZ-4514.patch
        8 kB
        Martin Kreidenweis

        Issue Links

          Activity

          Hide
          mkreidenweis Martin Kreidenweis added a comment -

          patch to handle taxes in purchase invoices similar to those in sales invoices

          Show
          mkreidenweis Martin Kreidenweis added a comment - patch to handle taxes in purchase invoices similar to those in sales invoices
          Hide
          anilpatel Anil K Patel added a comment -

          Martin,
          I played a little bit with GL Posting of Purchase Invoice that has Sales tax items. The accounting transaction entries did not look right to me, and now I am confused about handling sales tax (or any other taxes) on purchase invoice. I'll try to find time to learn more about it, so I can be confident about the expected behavior of system. In the mean time if you have any information that can help then please share it.

          Show
          anilpatel Anil K Patel added a comment - Martin, I played a little bit with GL Posting of Purchase Invoice that has Sales tax items. The accounting transaction entries did not look right to me, and now I am confused about handling sales tax (or any other taxes) on purchase invoice. I'll try to find time to learn more about it, so I can be confident about the expected behavior of system. In the mean time if you have any information that can help then please share it.
          Hide
          anilpatel Anil K Patel added a comment -

          This Process does not seem correct to me. Please post more information.

          Show
          anilpatel Anil K Patel added a comment - This Process does not seem correct to me. Please post more information.
          Hide
          jacopoc Jacopo Cappellato added a comment -

          With the premise that I didn't test the patch or recreate the bug reported:

          • the code change at @@ -2165,6 +2155,7 @@ seems wrong to me and should be probably suppressed.
          • the code changes at @@ -1841,28 +1841,6 @@ and at @@ -2359,39 +2354,6 @@ are irrelevant because they delete only commented out code.
          • the code changes at @@ -2173,6 +2164,10 @@ and @@ -2144,6 +2122,18 @@ seem actually relevant and a valid bug fix
          Show
          jacopoc Jacopo Cappellato added a comment - With the premise that I didn't test the patch or recreate the bug reported: the code change at @@ -2165,6 +2155,7 @@ seems wrong to me and should be probably suppressed. the code changes at @@ -1841,28 +1841,6 @@ and at @@ -2359,39 +2354,6 @@ are irrelevant because they delete only commented out code. the code changes at @@ -2173,6 +2164,10 @@ and @@ -2144,6 +2122,18 @@ seem actually relevant and a valid bug fix
          Hide
          mkreidenweis Martin Kreidenweis added a comment -

          Hi,

          I've found some time now to explain what our patch is doing.

          The changes are actually in production in our system and generate correct accounting for the German tax system.
          When we have sales tax in a purchase invoice it means that we actually get a tax refund/rebate ("Vorsteuer").

          The change at @@ -2144,6 +2122,18 @@ sets the glAccountId in the acctgTransEntry according to the tax authority party and geo id found in the invoice items. This is necessary so that the right account for the tax authority is used in the posting of the transaction.
          The changes were basically copied from the createAcctgTransForSalesInvoice method, where the same logic is implemented. (For sales invoices we have to pay taxes, so it is a creditEntry. For purchase invoices we get a tax rebate, so it is a debitEntry.)
          The change at @@ -2165,6 +2155,7 @@ also just makes the purchase invoice code more like the sales invoice code. The unattributedTaxTotal has to be posted to a tax account. In the sales invoice this is already set, in the purchase invoice it isn't – the patch fixes that. This is basically like the fallback in the change @@ -2144,6 +2122,18 @@, where we set the same value when we can't find a matching configuration for the tax authority. In case we have unattributed tax entries, we don't even need to try to find a tax authority configuration and simply set the fallback account type, so it gets posted to the default tax account.

          Regarding @@ -2173,6 +2164,10 @@: The call to getInvoiceTaxTotal was simply missing here. A few lines down the variable invoiceTaxTotal is accessed. Without applying the patch it is never set.

          Jacopo is right for @@ -1841,28 +1841,6 @@ and @@ -2359,39 +2354,6 @@ – they are just (unrelated) cleanup. Like the comment says, the functionality has moved to the createAcctgTransAndEntriesForPaymentApplication method, and as far as I can tell is working correctly.

          I hope the changes make more sense now.

          Best Regards,
          Martin

          Show
          mkreidenweis Martin Kreidenweis added a comment - Hi, I've found some time now to explain what our patch is doing. The changes are actually in production in our system and generate correct accounting for the German tax system. When we have sales tax in a purchase invoice it means that we actually get a tax refund/rebate ("Vorsteuer"). The change at @@ -2144,6 +2122,18 @@ sets the glAccountId in the acctgTransEntry according to the tax authority party and geo id found in the invoice items. This is necessary so that the right account for the tax authority is used in the posting of the transaction. The changes were basically copied from the createAcctgTransForSalesInvoice method, where the same logic is implemented. (For sales invoices we have to pay taxes, so it is a creditEntry. For purchase invoices we get a tax rebate, so it is a debitEntry.) The change at @@ -2165,6 +2155,7 @@ also just makes the purchase invoice code more like the sales invoice code. The unattributedTaxTotal has to be posted to a tax account. In the sales invoice this is already set, in the purchase invoice it isn't – the patch fixes that. This is basically like the fallback in the change @@ -2144,6 +2122,18 @@, where we set the same value when we can't find a matching configuration for the tax authority. In case we have unattributed tax entries, we don't even need to try to find a tax authority configuration and simply set the fallback account type, so it gets posted to the default tax account. Regarding @@ -2173,6 +2164,10 @@: The call to getInvoiceTaxTotal was simply missing here. A few lines down the variable invoiceTaxTotal is accessed. Without applying the patch it is never set. Jacopo is right for @@ -1841,28 +1841,6 @@ and @@ -2359,39 +2354,6 @@ – they are just (unrelated) cleanup. Like the comment says, the functionality has moved to the createAcctgTransAndEntriesForPaymentApplication method, and as far as I can tell is working correctly. I hope the changes make more sense now. Best Regards, Martin
          Hide
          mkreidenweis Martin Kreidenweis added a comment -

          I still think at least part of this issue is definitely a bug. (@@ -2173,6 +2164,10 @@)

          I'd also consider the other issues fixed by the patch a bug from the German point of view, but I cannot speak for other tax systems.

          Show
          mkreidenweis Martin Kreidenweis added a comment - I still think at least part of this issue is definitely a bug. (@@ -2173,6 +2164,10 @@) I'd also consider the other issues fixed by the patch a bug from the German point of view, but I cannot speak for other tax systems.
          Hide
          jacopoc Jacopo Cappellato added a comment -

          Paul Foxworthy we should also commit part of this patch with OFBIZ-6330

          Show
          jacopoc Jacopo Cappellato added a comment - Paul Foxworthy we should also commit part of this patch with OFBIZ-6330
          Hide
          paul_foxworthy Paul Foxworthy added a comment -

          OK will put together a combined patch

          Show
          paul_foxworthy Paul Foxworthy added a comment - OK will put together a combined patch
          Hide
          deepak.nigam Deepak Nigam added a comment -

          Attaching the combined patch for OFBIZ-4514 and OFBIZ-6330.

          Show
          deepak.nigam Deepak Nigam added a comment - Attaching the combined patch for OFBIZ-4514 and OFBIZ-6330 .
          Hide
          swash78 Swapnil Shah added a comment -

          Deepak Nigam
          Its nice that patch has been submitted, Can we please now summarize what was the "Before Behavior" and what would be "After Behavior" of system so everyone can refer to that and understand easily what changes have been made instead of digging into code/patch every time.

          It would be nice if we can follow this as a practice to put a very quick summary of before and after behavior on every ticket if code changes have been made before closing it.

          Show
          swash78 Swapnil Shah added a comment - Deepak Nigam Its nice that patch has been submitted, Can we please now summarize what was the "Before Behavior" and what would be "After Behavior" of system so everyone can refer to that and understand easily what changes have been made instead of digging into code/patch every time. It would be nice if we can follow this as a practice to put a very quick summary of before and after behavior on every ticket if code changes have been made before closing it.
          Hide
          deepak.nigam Deepak Nigam added a comment -

          In the case of Purchase Invoice,a prominent feature of Sales Tax calculation was missing from the systems,which leads towards invalid Account Postings of General Ledger,also there were no provision of calculation of freight charges.
          Now after the patch has submitted,the system comes to a stage where the sales taxes at Purchase invoice is now being taking into the calculation.

          Show
          deepak.nigam Deepak Nigam added a comment - In the case of Purchase Invoice,a prominent feature of Sales Tax calculation was missing from the systems,which leads towards invalid Account Postings of General Ledger,also there were no provision of calculation of freight charges. Now after the patch has submitted,the system comes to a stage where the sales taxes at Purchase invoice is now being taking into the calculation.

            People

            • Assignee:
              paul_foxworthy Paul Foxworthy
              Reporter:
              mkreidenweis Martin Kreidenweis
            • Votes:
              1 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:

                Development

                  Agile