search

Dolibarr / dolibarr

Dolibarr ERP CRM is a modern software package to manage your company or foundation's activity (contacts, suppliers, invoices, orders, stocks, agenda, accounting, ...). it's an open source Web application (written in PHP) designed for businesses of any sizes, foundations and freelancers.
https://www.dolibarr.org
GNU General Public License v3.0
5.46k stars 2.79k forks source link

BUGFIX: Copied Invoices Fail to Copy Local Tax 3 #24748

Closed qaOYR closed 1 month ago

qaOYR commented 1 year ago

Bug

Scenario: An invoice contains both VAT and an addiontal tax (tax 3). We use this additional tax in Spain for income taxes applied to invoices (IRPF). The rate of Tax 3 has been setup in the dictionary setup for VAT tax.

Environment Version

16.05 and 17.01

Environment OS

Ubuntu Server 22.04

Environment Web server

Apache

Environment PHP

PHP 7.4.29 and PHP 8.1

Environment Database

MySQL 7.4.29

Environment URL(s)

No response

Expected and actual behavior

Behavior: When copying an invoice that contains a VAT Tax and an additional Tax 3 , Tax 3 is only applied to the generated PDF file when you update individually each line on an invoice. No actual changes need to be made to each invoiced service line, just click on edit without changing anything and save.

Once this is done, the tax is correctly applied to that line in your invoice.

My guess is that the right tax value exists in the database, but it is not applied to each item of an invoice until the invoiced is reloaded for editing

Expected Behavior: when an invoice is copied all attributes and taxes should be preserved.

Steps to reproduce the behavior

Copy any existing invoice that has both a VAT and Local Tax 3 (IRPF in Spain) and local tax 3 will not be applied until you edit each service line on the invoice.

Attached files

No response

Daviid-P commented 1 year ago

To clarify, Tax 3 is localtax_2 in code/DB.

Tested on 16.0.3, locatax2 gets copied to the new invoice with out needing to update a line.

qaOYR commented 1 year ago

Hi Daviid-P,

Thanks for responding. We are testing and using Dolibarr 16.05 and can reproduce the above-mentioned bug in more than one install. We have resolved the issue by adding the following type definitions before line 3744 in file facture.class.php

Path: /htdocs/compta/facture/class

File: facture.class.php

Insert the following code before line 3744:

        $localtaxes_type[2] = 5;
        $localtaxes_type[3] =  $txlocaltax2;

Line: 3744

     $tabprice = calcul_price_total($qty, $pu, $remise_percent, $txtva, $txlocaltax1, $txlocaltax2, 0, $price_base_type, $info_bits, $product_type, $mysoc, $localtaxes_type, $situation_percent, $this->multicurrency_tx, $pu_ht_devise);

Do you see any downsides or possible side-effects to what we are proposing?

Thanks,

qaOYR

Daviid-P commented 1 year ago

How does your 

function getLocalTaxesFromRate($vatrate, $local, $buyer, $seller, $firstparamisid = 0)
{
    global $db, $mysoc;

    dol_syslog("getLocalTaxesFromRate vatrate=".$vatrate." local=".$local);

    // Search local taxes
    $sql  = "SELECT t.taux as rate, t.code, t.localtax1, t.localtax1_type, t.localtax2, t.localtax2_type, t.accountancy_code_sell, t.accountancy_code_buy";
    $sql .= " FROM ".MAIN_DB_PREFIX."c_tva as t";
    if ($firstparamisid) {
        $sql .= " WHERE t.rowid = ".(int) $vatrate;
    } else {
        $vatratecleaned = $vatrate;
        $vatratecode = '';
        $reg = array();
        if (preg_match('/^(.*)\s*\((.*)\)$/', $vatrate, $reg)) {     // If vat is "x.x (yy)"
            $vatratecleaned = $reg[1];
            $vatratecode = $reg[2];
        }

        $sql .= ", ".MAIN_DB_PREFIX."c_country as c";
        if ($mysoc->country_code == 'ES') {
            $sql .= " WHERE t.fk_pays = c.rowid AND c.code = '".$db->escape($buyer->country_code)."'"; // local tax in spain use the buyer country ??
        } else {
            $sql .= " WHERE t.fk_pays = c.rowid AND c.code = '".$db->escape(empty($seller->country_code) ? $mysoc->country_code : $seller->country_code)."'";
        }
        $sql .= " AND t.taux = ".((float) $vatratecleaned)." AND t.active = 1";
        if ($vatratecode) {
            $sql .= " AND t.code = '".$db->escape($vatratecode)."'";
        }
    }

    $resql = $db->query($sql);
    if ($resql) {
        $obj = $db->fetch_object($resql);

        if ($obj) {
            $vateratestring = $obj->rate.($obj->code ? ' ('.$obj->code.')' : '');

            if ($local == 1) {
                return array($obj->localtax1_type, get_localtax($vateratestring, $local, $buyer, $seller), $obj->accountancy_code_sell, $obj->accountancy_code_buy);
            } elseif ($local == 2) {
                return array($obj->localtax2_type, get_localtax($vateratestring, $local, $buyer, $seller), $obj->accountancy_code_sell, $obj->accountancy_code_buy);
            } else {
                return array($obj->localtax1_type, get_localtax($vateratestring, 1, $buyer, $seller), $obj->localtax2_type, get_localtax($vateratestring, 2, $buyer, $seller), $obj->accountancy_code_sell, $obj->accountancy_code_buy);
            }
        }
    }

    return array();
}

and

function get_localtax($vatrate, $local, $thirdparty_buyer = "", $thirdparty_seller = "", $vatnpr = 0)
{
    global $db, $conf, $mysoc;

    if (empty($thirdparty_seller) || !is_object($thirdparty_seller)) {
        $thirdparty_seller = $mysoc;
    }

    dol_syslog("get_localtax tva=".$vatrate." local=".$local." thirdparty_buyer id=".(is_object($thirdparty_buyer) ? $thirdparty_buyer->id : '')."/country_code=".(is_object($thirdparty_buyer) ? $thirdparty_buyer->country_code : '')." thirdparty_seller id=".$thirdparty_seller->id."/country_code=".$thirdparty_seller->country_code." thirdparty_seller localtax1_assuj=".$thirdparty_seller->localtax1_assuj."  thirdparty_seller localtax2_assuj=".$thirdparty_seller->localtax2_assuj);

    $vatratecleaned = $vatrate;
    $reg = array();
    if (preg_match('/^(.*)\s*\((.*)\)$/', $vatrate, $reg)) {     // If vat is "xx (yy)"
        $vatratecleaned = trim($reg[1]);
        $vatratecode = $reg[2];
    }

    /*if ($thirdparty_buyer->country_code != $thirdparty_seller->country_code)
    {
        return 0;
    }*/

    // Some test to guess with no need to make database access
    if ($mysoc->country_code == 'ES') { // For spain localtaxes 1 and 2, tax is qualified if buyer use local tax
        if ($local == 1) {
            if (!$mysoc->localtax1_assuj || (string) $vatratecleaned == "0") {
                return 0;
            }
            if ($thirdparty_seller->id == $mysoc->id) {
                if (!$thirdparty_buyer->localtax1_assuj) {
                    return 0;
                }
            } else {
                if (!$thirdparty_seller->localtax1_assuj) {
                    return 0;
                }
            }
        }

        if ($local == 2) {
            //if (! $mysoc->localtax2_assuj || (string) $vatratecleaned == "0") return 0;
            //if (!$mysoc->localtax2_assuj) {
            //  return 0; // If main vat is 0, IRPF may be different than 0.
            //}
            if ($thirdparty_seller->id == $mysoc->id) {
                // if (!$thirdparty_buyer->localtax2_assuj) {
                //  return 0;
                // }
                // customCode
                if (
                    $conf->global->FACTURE_LOCAL_TAX2_OPTION != "localtax2off" &&
                    isset($conf->global->MAIN_INFO_VALUE_LOCALTAX2)
                ) {
                    return $conf->global->MAIN_INFO_VALUE_LOCALTAX2;
                } elseif ($thirdparty_seller->localtax2_assuj) {
                    return $thirdparty_seller->localtax2_value;
                }
                // customCode
            } else {
                if (!$thirdparty_seller->localtax2_assuj) {
                    return 0;
                }
            }
        }
    } else {
        if ($local == 1 && !$thirdparty_seller->localtax1_assuj) {
            return 0;
        }
        if ($local == 2 && !$thirdparty_seller->localtax2_assuj) {
            return 0;
        }
    }

    // For some country MAIN_GET_LOCALTAXES_VALUES_FROM_THIRDPARTY is forced to on.
    if (in_array($mysoc->country_code, array('ES'))) {
        $conf->global->MAIN_GET_LOCALTAXES_VALUES_FROM_THIRDPARTY = 1;
    }

    // Search local taxes
    if (!empty($conf->global->MAIN_GET_LOCALTAXES_VALUES_FROM_THIRDPARTY)) {
        if ($local == 1) {
            if ($thirdparty_seller != $mysoc) {
                if (!isOnlyOneLocalTax($local)) {  // TODO We should provide $vatrate to search on correct line and not always on line with highest vat rate
                    return $thirdparty_seller->localtax1_value;
                }
            } else { // i am the seller
                if (!isOnlyOneLocalTax($local)) { // TODO If seller is me, why not always returning this, even if there is only one locatax vat.
                    return $conf->global->MAIN_INFO_VALUE_LOCALTAX1;
                }
            }
        }
        if ($local == 2) {
            if ($thirdparty_seller != $mysoc) {
                if (!isOnlyOneLocalTax($local)) {  // TODO We should provide $vatrate to search on correct line and not always on line with highest vat rate
                    // TODO We should also return value defined on thirdparty only if defined
                    return $thirdparty_seller->localtax2_value;
                }
            } else { // i am the seller
                // customCode
                if ($conf->global->FACTURE_LOCAL_TAX2_OPTION != "localtax2off" &&
                    isset($conf->global->MAIN_INFO_VALUE_LOCALTAX2)) {
                    return $conf->global->MAIN_INFO_VALUE_LOCALTAX2;
                } elseif ($thirdparty_buyer->localtax2_value > 0) {
                    return $thirdparty_buyer->localtax2_value;
                }
                // customCode
                //if (in_array($mysoc->country_code, array('ES'))) {
                //  return $thirdparty_buyer->localtax2_value;
                //} else {
                //  return $conf->global->MAIN_INFO_VALUE_LOCALTAX2;
                //}
            }
        }
    }

    // By default, search value of local tax on line of common tax
    $sql = "SELECT t.localtax1, t.localtax2, t.localtax1_type, t.localtax2_type";
    $sql .= " FROM ".MAIN_DB_PREFIX."c_tva as t, ".MAIN_DB_PREFIX."c_country as c";
    $sql .= " WHERE t.fk_pays = c.rowid AND c.code = '".$db->escape($thirdparty_seller->country_code)."'";
    $sql .= " AND t.taux = ".((float) $vatratecleaned)." AND t.active = 1";
    if (!empty($vatratecode)) {
        $sql .= " AND t.code ='".$db->escape($vatratecode)."'"; // If we have the code, we use it in priority
    } else {
        $sql .= " AND t.recuperableonly = '".$db->escape($vatnpr)."'";
    }

    $resql = $db->query($sql);

    if ($resql) {
        $obj = $db->fetch_object($resql);
        if ($obj) {
            if ($local == 1) {
                return $obj->localtax1;
            } elseif ($local == 2) {
                return $obj->localtax2;
            }
        }
    }

    return 0;
}

look like?

I think I've modified mine becuase of some problems I had so maybe mine is working because of that.

Also how is your tva dictionary? /htdocs/admin/dict.php in "Tasa de IVA"

qaOYR commented 1 year ago

@simnandez - Could you please review our proposed fix as I believe it is a bug that is easy to reproduce?

This issue affects many users of Dolibarr in Spain and we belive that our proposed fix addresses the issue. It has been in production for some time without any visible downsides.

Look forward to your response,

qaOYR.

Daviid-P commented 1 year ago

@simnandez - Could you please review our proposed fix as I believe it is a bug that is easy to reproduce?

This issue affects many users of Dolibarr in Spain and we belive that our proposed fix addresses the issue. It has been in production for some time without any visible downsides.

Look forward to your response,

qaOYR.

I doubt the suggested fix will be imp,emented, you're hardcoding values here.

Assuming localtax2, getLocalTaxesFromRate returns:

array(
    0 $obj->localtax2_type, 
    1 get_localtax($vateratestring, $local, $buyer, $seller), 
    2 $obj->accountancy_code_sell, 
    3 $obj->accountancy_code_buy
)

Are you sure accountancy_code_sell should be always 5 and accountancy_code_buy always $txlocaltax2

And that's v16 and maybe v17

Now in v18 it returns:

array(
    'rowid'=>$obj->rowid,
    'code'=>$obj->code,
    'rate'=>$obj->rate,
    'localtax1'=>$obj->localtax1,
    'localtax1_type'=>$obj->localtax1_type,
    'localtax2'=>$obj->localtax2,
    'localtax2_type'=>$obj->localtax2_type,
    'npr'=>$obj->npr,
    'accountancy_code_sell'=>$obj->accountancy_code_sell,
    'accountancy_code_buy'=>$obj->accountancy_code_buy
);

So no integer index.

github-actions[bot] commented 1 month ago

This issue is stale because it has been open 1 year with no activity. If this is a bug, please comment to confirm it is still present on latest stable version. if this is a feature request, please comment to notify the request is still relevant and not yet covered by latest stable version. This issue may be closed automatically by stale bot in 10 days (you should still be able to re-open it if required).