[16.0][IMP] account_reconcile_oca: Improve multicurrency management #694

etobella · 2024-09-13T08:34:54Z

Tries to fix #692 #693
@florian-dacosta Can you check it please?

florian-dacosta · 2024-09-13T08:50:48Z

@etobella Thanks for the PR!
Well, it fix the example I gave in the issue yes, thanks, but it does not fix all cases, like the one in the second issue.
Let me show you what I have with the same setup from #693

Default screen when going to reconcile UI, we have a rounding issue

After entering the 121000 account in manual tab :

etobella · 2024-09-13T13:55:02Z

@florian-dacosta Round 2

I added a rounding factor in order to remove this possible error (the rounding should only be used on comparison)

florian-dacosta · 2024-09-13T14:26:17Z

@etobella I still don't get why there is a rounding to find an amount that is written in the amount field of the account.bank.statement.line. Why the 259 200 needs to be computed and not read for the line ?

Anyway, I've tested the later version
I still get this rounding issue before chosing the invoice.
And once validated, the state /payment on the invoice is not ok.

Go to reconcile UI - Not OK

Choose the invoice - OK

Reconciled invoice after validation - Not OK

The bank accounting entry is missing the currency on the 121000 line

etobella · 2024-09-13T14:51:53Z

We have a rounding issue because the reconcile is done on the currency of the company. Also we cannot use that amount, because we might have previous lines.

The only option I can see is to do the reconcile in the amount of the statement line.

This is more work to do and I just wanted to add a patch, but seems we need to do a real refactoring (it is good to have so many tests)

PaulGoubert · 2024-09-13T15:27:15Z

Thank you for your help with these issues, @etobella.
As previously mentioned by @florian-dacosta, we're in a rush on this topic due to our customer's situation—they've been expecting to use it properly for 6 months now. I hope you can tackle these topics and help us resolve the issues.

etobella · 2024-09-13T22:37:49Z

@PaulGoubert @florian-dacosta Round 3

I have changed the flow in order to compare using the currency of the line. It has no sense to move from EUR to USD to EUR, so it should remove some of this errors.

etobella · 2024-09-13T22:46:17Z

The only issue I found is that it is not showing properly when reconciling with the Foraign Exchange Loss

Before Reconciling

After reconciling

The foreign loss has been lost, but it is correctly reconciled

etobella · 2024-09-14T21:09:58Z

I applied a fix for this. I am not sure if it gets all the possible cases, but at least it is an approach

florian-dacosta · 2024-09-15T14:56:32Z

@etobella Thanks for your work, it is really helpull !
About the rounding case, with last version of the code, here is what I have :

First click on the reconcile button on journal
It give me the reconciliation screen with the rounding error in the suspense account :

If I choose the counter part, all is fine, then when I reconcile, as you saif, all is fine, the invoice is reconciled correctly, the accounting entries are correct.

Right after, the view of the statement line in the reconciliation UI was correct. I am not sure how it work, but with a new tab/after restarting the server and going back on the statement line, I have really strange data :

For now the most annoying for me is the display in 1. Even if all goes fine after, it is very confusing for the user.

I applied a fix for this. I am not sure if it gets all the possible cases, but at least it is an approach

I am not sure what you refer to ?
If it is about the last created issue (695), I have just updated it because I had made a mistake in the screen shot of the use case.
I have tested again right now, and it is still actual.

etobella · 2024-09-15T15:16:20Z

1st point should be solved now.

florian-dacosta · 2024-09-18T09:20:58Z

1st point should be solved now.

It is, thanks
I have made a PR on yours to fix different other issues, Take a look when you can

florian-dacosta · 2024-09-23T08:32:10Z

@etobella Could you rebase please ?

yvaucher · 2024-10-01T13:54:10Z

account_reconcile_oca/models/account_bank_statement_line.py

-                self.manual_partner_id and self.manual_partner_id.name_get()[0] or False
+                self.manual_partner_id
+                and self.manual_partner_id.name_get()[0]
+                or [False, False]


bool([False, False]) => True

Are you sure you want this?

Sorry for the noise here.
I'll make it short here. When there are not partner at all, in the data, the value [False, False] is set here :
https://github.com/OCA/account-reconcile/blob/16.0/account_reconcile_oca/models/account_bank_statement_line.py#L405
and here :
https://github.com/OCA/account-reconcile/blob/16.0/account_reconcile_oca/models/account_bank_statement_line.py#L896

So when checking if the line has changed, and there is no partner at all, comparing the reconcile_data partner part ([False, False])with False will always be considered different while no changed occured (no partner was set).

So I think we could leave this as is. (meaning comparing [False, False] with line.get("partner_id")

PabloMartFL

Tested the PR on runboat and it resolves the problems in the issues:

For the test, I created a company in EUR and created a Bank in EUR.
The currency rate are the following:

The suspended balance apears correct and when you select the invoices that fully match with the statement, it doesnt correct the invoice amount.
Also, in the account move, the amount in currency is displayed correcly

https://www.awesomescreenshot.com/video/32216430?key=6538e7535b46717a2905cfcb08d9a052

ljsalvatierra-factorlibre · 2024-11-27T11:18:45Z

Hi @etobella can you do a rebase?

It has conflicts with #758

We will use currency of the line in order to get the suspense and max line value

Fix the case of payment with a foreign currency set on bank statement line improve tests around multi-currency

…rrency rate It is possible that the statement line in foreign currency is created before the rate of the day is updated in Odoo. In this case we need to take the real rate of the statement line to comput the exchange rate

…wanted currency conversion

…ount If currency amount is not 0, the suspense line will have wrong amount

ljsalvatierra-factorlibre · 2024-11-28T14:14:47Z

account_reconcile_oca/models/account_bank_statement_line.py

+                        == self.company_id.currency_exchange_journal_id
+                    ):
+                        reconcile_auxiliary_id, lines = self._get_reconcile_line(
+                            reconciled_line.move_id.line_ids - reconciled_line,


Expected singleton.

>>> reconcile_auxiliary_id, lines = self._get_reconcile_line( >>> new_vals = super()._get_reconcile_line( >>> original_amount = amount = net_amount = line.debit - line.credit >>> record.ensure_one()

etobella force-pushed the 16.0-fix-mcurr branch from 3db40d1 to 0e1b9bb Compare September 13, 2024 08:46

This was linked to issues Sep 13, 2024

account_reconcile_oca : wrong reconciliation of account if foreign currency #692

Open

[16] account_reconcile_oca : bank account in foreign currency, rounding error for full reconcile #693

Open

etobella force-pushed the 16.0-fix-mcurr branch from 0e1b9bb to 1be98ea Compare September 13, 2024 13:53

etobella force-pushed the 16.0-fix-mcurr branch from 1be98ea to 7018d67 Compare September 13, 2024 22:36

etobella force-pushed the 16.0-fix-mcurr branch from 7018d67 to d7fd2bb Compare September 13, 2024 22:49

etobella linked an issue Sep 14, 2024 that may be closed by this pull request

[16] account_reconcile_oca : foreign currency in company currency account #695

Open

etobella force-pushed the 16.0-fix-mcurr branch from 43080b7 to 7be86a9 Compare September 23, 2024 09:48

florian-dacosta mentioned this pull request Sep 30, 2024

16.0 fix mcurr tests dixmit/account-reconcile#2

Merged

etobella force-pushed the 16.0-fix-mcurr branch from cc242ac to 0553397 Compare October 1, 2024 13:11

OCA deleted a comment from OCA-git-bot Oct 1, 2024

yvaucher reviewed Oct 1, 2024

View reviewed changes

PabloMartFL approved these changes Oct 4, 2024

View reviewed changes

etobella and others added 4 commits November 27, 2024 13:04

[IMP] account_reconcile_oca: Improve multicurrency management.

e3bd65e

We will use currency of the line in order to get the suspense and max line value

[FIX] account_reconcile_oca : Fix multi currency management

06c1dd9

Fix the case of payment with a foreign currency set on bank statement line improve tests around multi-currency

[FIX] account_reconcile_oca : max_amount rounding error leading to un…

023e810

…wanted currency conversion

[FIX] account_reconcile_oca : exchange rate gain/loss currency and am…

2edf543

…ount If currency amount is not 0, the suspense line will have wrong amount

etobella force-pushed the 16.0-fix-mcurr branch from 32421da to 2edf543 Compare November 27, 2024 12:08

ljsalvatierra-factorlibre suggested changes Nov 28, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[16.0][IMP] account_reconcile_oca: Improve multicurrency management #694

[16.0][IMP] account_reconcile_oca: Improve multicurrency management #694

etobella commented Sep 13, 2024 •

edited

Loading

florian-dacosta commented Sep 13, 2024

etobella commented Sep 13, 2024

florian-dacosta commented Sep 13, 2024 •

edited

Loading

etobella commented Sep 13, 2024

PaulGoubert commented Sep 13, 2024

etobella commented Sep 13, 2024

etobella commented Sep 13, 2024

etobella commented Sep 14, 2024

florian-dacosta commented Sep 15, 2024 •

edited

Loading

etobella commented Sep 15, 2024

florian-dacosta commented Sep 18, 2024

florian-dacosta commented Sep 23, 2024

yvaucher Oct 1, 2024

florian-dacosta Oct 7, 2024

PabloMartFL left a comment

ljsalvatierra-factorlibre commented Nov 27, 2024 •

edited

Loading

ljsalvatierra-factorlibre Nov 28, 2024

[16.0][IMP] account_reconcile_oca: Improve multicurrency management #694

Are you sure you want to change the base?

[16.0][IMP] account_reconcile_oca: Improve multicurrency management #694

Conversation

etobella commented Sep 13, 2024 • edited Loading

florian-dacosta commented Sep 13, 2024

etobella commented Sep 13, 2024

florian-dacosta commented Sep 13, 2024 • edited Loading

etobella commented Sep 13, 2024

PaulGoubert commented Sep 13, 2024

etobella commented Sep 13, 2024

etobella commented Sep 13, 2024

etobella commented Sep 14, 2024

florian-dacosta commented Sep 15, 2024 • edited Loading

etobella commented Sep 15, 2024

florian-dacosta commented Sep 18, 2024

florian-dacosta commented Sep 23, 2024

yvaucher Oct 1, 2024

Choose a reason for hiding this comment

florian-dacosta Oct 7, 2024

Choose a reason for hiding this comment

PabloMartFL left a comment

Choose a reason for hiding this comment

ljsalvatierra-factorlibre commented Nov 27, 2024 • edited Loading

ljsalvatierra-factorlibre Nov 28, 2024

Choose a reason for hiding this comment

etobella commented Sep 13, 2024 •

edited

Loading

florian-dacosta commented Sep 13, 2024 •

edited

Loading

florian-dacosta commented Sep 15, 2024 •

edited

Loading

ljsalvatierra-factorlibre commented Nov 27, 2024 •

edited

Loading