Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VAT code for all products which is removed from a document (proposal / invoices / command ) when adding a global reduction #29192

Open
donowr opened this issue Apr 2, 2024 · 14 comments
Labels
Bug This is a bug (something does not work as expected)

Comments

@donowr
Copy link

donowr commented Apr 2, 2024

Bug

Good morning,

I have a problem with the VAT accounting codes on the lines of a document (invoice/proposal or other).

When I add a global reduction to a document it removes the VAT code from each line.

I tried it on a blank Dolibarr.

I also found how to fix the problem by adding a small piece of code in each card.php of a document.

Here is the line that I added temporarily in all card.php:

$line->tva_tx= $line->tva_tx.' ('.$line->vat_src_code.')'; 

This line was added in the following condition in each in card.php:
I dont think it is the best way to patch that.

} elseif ($action == 'addline' && GETPOST('submitforalllines', 'alpha') && GETPOST('remiseforalllines', 'alpha') !== '' && $usercancreate) {
		// Define a discount for all lines
		$remise_percent = (GETPOST('remiseforalllines') ? GETPOST('remiseforalllines') : 0);
		$remise_percent = str_replace('*', '', $remise_percent);
		foreach ($object->lines as $line) {
			$line->tva_tx= $line->tva_tx.' ('.$line->vat_src_code.')'; // temp FIX
			$result = $object->updateline($line->id, $line->subprice, $line->qty, $remise_percent, $line->tva_tx, $line->localtax1_tx, $line->localtax2_tx, $line->desc, 'HT', $line->info_bits, $line->special_code, $line->fk_parent_line, 0, $line->fk_fournprice, $line->pa_ht, $line->label, $line->product_type, $line->date_start, $line->date_end, $line->array_options, $line->fk_unit, $line->multicurrency_subprice);
		}
} elseif ($action == 'addline' && GETPOST('submitforallmargins', 'alpha') && GETPOST('marginforalllines') !== '' && $usercancreate) {
   

Dolibarr Version

18.0.5

Environment PHP

8.1.27

Environment Database

Mysql 8.3.0

Steps to reproduce the behavior and expected behavior

Apply a discount to all lines of a document:

image_2024_04_02T07_02_50_280Z

Output:

image_2024_04_02T07_03_15_261Z

Attached files

No response

@donowr donowr added the Bug This is a bug (something does not work as expected) label Apr 2, 2024
@donowr donowr changed the title VAT code for all products which is removed from a document (proposal / invoices ) when adding a global reduction VAT code for all products which is removed from a document (proposal / invoices / command ) when adding a global reduction Apr 2, 2024
@JonBendtsen
Copy link
Contributor

@donowr why don't you think it is the best way to patch it?

@donowr
Copy link
Author

donowr commented Apr 8, 2024

@JonBendtsen

Because it does not solve the problem at its source. Instead of fixing the code that handles the application of global discounts so that it correctly preserves the VAT discount codes, my solution is to manually reinsert the VAT codes after they have been deleted.

While this may temporarily resolve the issue, it does not resolve the underlying cause of the issue.

A more robust approach would be to identify and fix the part of Dolibarr's code that removes VAT codes when applying global discounts.

This would ensure correct and consistent behavior in all situations, without requiring additional fixes in the future. In summary, my solution is a temporary solution that works around the problem, but the real solution would be to modify the Dolibarr code so that it works correctly from the start.

@donowr
Copy link
Author

donowr commented Apr 8, 2024

I found where the problem comes from.

The $object->updateline() function uses the content of the $txtva parameter to extract the VAT code:

 // Clean vat code $vat_src_code = ''; 
$reg = array(); 
if (preg_match('/\((.*)\)/', $txtva, $reg)) {
   $vat_src_code = $reg[1]; 
   $txtva = preg_replace('/\s*\(.*\)/', '', $txtva); // Remove code into vatrate. 
} 

However, I looked in $object and it turns out that not all the lines in the documents contain the VAT code when the object is instantiated:
php $object->line[x]->tva_tx .

I tried to add the VAT code when the object retrieved the document lines but this is not the right solution because the frontend does not handle it when the VAT code is in $line->tva_tx .

My first solution remains valid but I have a second solution to propose.

We can modify the $object->updateline() function so that it takes the VAT code as a parameter. By default the parameter would be False, this will allow the basic logic to be preserved in the case where the parameter was not provided.

@JonBendtsen
Copy link
Contributor

We can modify the $object->updateline() function so that it takes the VAT code as a parameter. By default the parameter would be False, this will allow the basic logic to be preserved in the case where the parameter was not provided.

@donowr I like that, but I have no authority to say

I wonder if there are more places than proposal/order/invoice (propal/commande/facture) where it needs to be fixed

JonSweet16:dolibarr jonbendtsen$ grep -irl '$object->updateline('
./htdocs/variants/card.php
./htdocs/bom/bom_card.php
./htdocs/fourn/commande/card.php
./htdocs/fourn/facture/card.php
./htdocs/fourn/facture/card-rec.php
./htdocs/commande/card.php
./htdocs/compta/facture/card.php
./htdocs/compta/facture/card-rec.php
./htdocs/expensereport/card.php
./htdocs/comm/propal/card.php
./htdocs/supplier_proposal/card.php

JonSweet16:dolibarr jonbendtsen$ grep -irl 'function updateline('
./htdocs/contrat/class/contrat.class.php
./htdocs/variants/class/ProductAttribute.class.php
./htdocs/bom/class/bom.class.php
./htdocs/fourn/class/fournisseur.commande.class.php
./htdocs/fourn/class/fournisseur.facture.class.php
./htdocs/fourn/class/fournisseur.facture-rec.class.php
./htdocs/commande/class/commande.class.php
./htdocs/compta/bank/class/api_bankaccounts.class.php
./htdocs/compta/facture/class/facture-rec.class.php
./htdocs/compta/facture/class/facture.class.php
./htdocs/expensereport/class/expensereport.class.php
./htdocs/comm/propal/class/propal.class.php
./htdocs/supplier_proposal/class/supplier_proposal.class.php

@donowr
Copy link
Author

donowr commented Apr 8, 2024

@JonBendtsen

Who has the authority to tell me what to do?
This is my first contribution and I would like to go through to the end, I don't know the process either...

Concerning the places where the modification must be made I cannot answer you with certainty but I think you have listed them all.

I'll ask a colleague this tomorrow

@JonBendtsen
Copy link
Contributor

@JonBendtsen

Who has the authority to tell me what to do? This is my first contribution and I would like to go through to the end, I don't know the process either...

@donowr I think that you can definitely get a final decisive answer from @eldy

@donowr
Copy link
Author

donowr commented Apr 19, 2024

@JonBendtsen I have to wait waitfor @eldy ​​to give his answer here?

@JonBendtsen
Copy link
Contributor

@JonBendtsen I have to wait waitfor @eldy ​​to give his answer here?

I think it is either that or submit a pull request

@eldy
Copy link
Member

eldy commented Apr 19, 2024

It is better to add the
' ('.$line->vat_src_code.')'
In the 5th parameter of update method, concatenated to line->tva_src, instead of modifying the value $line->tva_tx.
So line->tva_tx keeps its numeric value.

Can you submit a Pull Request in this way ?

@donowr
Copy link
Author

donowr commented Apr 26, 2024

hello, I did a pr but the phan test failed, is this disturbing?

How do I resolve the problem so that my pr passes all the tests?

@JonBendtsen
Copy link
Contributor

hello, I did a pr but the phan test failed, is this disturbing?

How do I resolve the problem so that my pr passes all the tests?

that depends - was it your fault or was it something that you did not change?

@JonBendtsen
Copy link
Contributor

@donowr if you can update your merge request to a newer base version (there should be a button in the bottom of the merge request page if possible) then use that possibility. Even if there is not such a thing, come back regularly and update which triggers a new build and maybe someone else fixed the phan error.

But it might just be your code that has the issue, so check where you edited and what, and correlate with the error message in phan.

@donowr
Copy link
Author

donowr commented Apr 26, 2024

@JonBendtsen
here is the phan error, unless I'm mistaken I don't think it refers to one of my modifications. Additionally I found a similar error on another pull request : #29481 (comment)

image

@JonBendtsen
Copy link
Contributor

@JonBendtsen here is the phan error, unless I'm mistaken I don't think it refers to one of my modifications. Additionally I found a similar error on another pull request : #29481 (comment)

image

good, then i'd suggest that you keep updating when possible until all checks are clean, then you leave it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This is a bug (something does not work as expected)
Projects
None yet
Development

No branches or pull requests

3 participants