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

FIX Accountancy - Display good amount for situation line #18652

Closed
wants to merge 2 commits into from

Conversation

aspangaro
Copy link
Member

@aspangaro aspangaro commented Sep 8, 2021

Hello,

When we have a situation line from a S2 to bind, amount displayed is the total amount of the situation line without advancement.

Exemple :

Advancement : S1 (30% * 50000 €) = 15000 € | S2 (50% * 50000 €) = 25000 €

Reported sales displayed = 40 000 € (15000 € + 25000 €)

Reported sales real = 25 000 € (15000 € + (25000 € - 15000 €))

No problem when you register in accountancy, we work with the total fields of the invoice, not the lines. It's just a display problem on the binding pages

Before :
image

With this invoice :
image

After :
image


Strictly the same problem on customer index :

Before :
image

After :
image

@aspangaro aspangaro changed the title FIX Accountancy - Show good amount for situation line FIX Accountancy - Display good amount for situation line Sep 9, 2021
@eldy
Copy link
Member

eldy commented Sep 9, 2021

Here the bug is not into accounting feature but into the way the situation invoice into this experimental features is currently store. The situation invoice can not be supported currently into accountancy because there is a serious bug in the way the data is stored into the invoice table: Sometimes it is cumulative, sometimes it contains only amount of invoice.
We must first fix the way the amount of situation invoices are stored into the invoice table and this will fix automatically the accounting feature too.

Currently for situation invoice, into table llx_facture
If invoice is standard:
total = total of all previous situation invoice instead of the total of THE situation invoice
If invoice is a credit note
total = total of THE credit note

What we must have
If invoice is standard:
total = total of all THE situation invoice (the field total for all situation invoices must be stored into a new field).
If invoice is a credit note
total = total of THE credit note

@aspangaro
Copy link
Member Author

aspangaro commented Sep 9, 2021

We must first fix the way the amount of situation invoices are stored into the invoice table and this will fix automatically the accounting feature too.

We totally agree. The way the data is recorded is bad for situation invoices. The accounting is impacted because of this.

Multi-currency is impacted equally...

I proposed to work for correct the data stored in table facturedet by difference, not in advancement

@eldy
Copy link
Member

eldy commented Sep 9, 2021

We must first fix the way the amount of situation invoices are stored into the invoice table and this will fix automatically the accounting feature too.

We totally agree. The way the data is recorded is bad for situation invoices. The accounting is impacted because of this.

Multi-currency is impacted equally...

I proposed to work for correct the data stored in table invoice by difference, not in advancement

And all reporting is wrong too. Even total into list. It just can't work like this.
To avoid a big mess with current code and make things easier to understand, I suggest to introduce a constant :

if ($conf->global->INVOICE_USE_SITUATION == 'new') {
        new code to store the values correctly
} else {
        current existing code.
}

@aspangaro
Copy link
Member Author

Ok for the constant.

Do you think it's possible to propose a fix on "old" version 12 ? Or only on develop branch ? I think it's better if we correct the data as far back in time as possible

@eldy
Copy link
Member

eldy commented Sep 9, 2021

We will have a lot of change. Doing this into an old version is simply not possible. Automatic merge will be broken and it won't be "clear" in which version data is good and which version data are wrong. So it must be done on develop.
A migration tool will be possible to fix data for people who enabled this feature. And it will be necessary to launch this migration only "once" and with the good version at the good time.
This will not be supported by Dolibarr core because the feature is still not yet flagged as stable/ready to use (note that there was already different changes and fixes in the way data were store from version to version around this features, with no care if it was used or not and how it was used). So for me data is already corrupted now with different storage rules with no way to know which one was used. No migration process will be provided by the core (like for any develop feature. We can take care of data only when structure is clear and for this one, it is not). A tool/page can however be provided as an external tool but i really don't see how such a tool would be able to work with data that was saved differently after each past fix.

@aspangaro
Copy link
Member Author

Thanks for your answer @eldy

It's clear. I try to work on a total review to integrate in v15 or v16

@eldy eldy added the PR not qualified PR is not qualified (feature not enough requested, duplicate feature or other reason) label Sep 10, 2021
@aspangaro aspangaro closed this Sep 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR not qualified PR is not qualified (feature not enough requested, duplicate feature or other reason)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants