Skip to content
New issue

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

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

Already on GitHub? # to your account

[17] account_reconcile_oca : Forward port multi currency management from 16 #788

Merged
merged 13 commits into from
Feb 11, 2025

Conversation

florian-dacosta
Copy link
Contributor

@florian-dacosta florian-dacosta commented Jan 22, 2025

@OCA-git-bot
Copy link
Contributor

Hi @etobella,
some modules you are maintaining are being modified, check this out!

Copy link
Member

@victoralmau victoralmau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review OK, please, fix pre-commit errors.,

@florian-dacosta
Copy link
Contributor Author

Yes, I am waiting @etobella On this one!

Comment on lines 655 to 692
partial = partial_lines.filtered(
lambda r: r.debit_move_id == reconciled_line
or r.credit_move_id == reconciled_line
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
partial = partial_lines.filtered(
lambda r: r.debit_move_id == reconciled_line
or r.credit_move_id == reconciled_line
)
partial = partial_lines.filtered(
lambda r, line=reconciled_line: r.debit_move_id == line
or r.credit_move_id == line
)

Doing something like this will fix the pre-commit

@victoralmau
Copy link
Member

Can you also add the changes from #792?

@florian-dacosta
Copy link
Contributor Author

Pre-commit is fixed and and #792 added.

@pedrobaeza
Copy link
Member

Please squash some commits together like the linter fix with the code that originates the linter the error for merging this.

etobella and others added 11 commits February 10, 2025 09:23
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
…ount

If currency amount is not 0, the suspense line will have wrong amount
…er_id without onchange or subsequent changes with the _synchronize_to_moves() method.

We do not define partner_id with the value of manual_partner_id to prevent _synchronize_to_moves()
from making changes to the account.move.line leaving unintended values and/or data.

Re-define the value of reconcile_data_info if the _synchronize_to_moves() method has changed
anything on the lines.

Related to OCA#779

TT52634
We need to look at the reconcile_data to find the partner as the manual_partner_id can contain the information of an auxiliary line
We cant to avoid to change amounts on accounting entries during the reconciliation process. One of the goal is to avoid a following case :
The statement line is created in a foreign currency journal, later, the rate is updated in Odoo.
During reconciliation process, the partner is set on liquidity line, the accounting entries are synchronized and the balance changes.
@florian-dacosta
Copy link
Contributor Author

Please squash some commits together like the linter fix with the code that originates the linter the error for merging this.

I have squashed this one.
I have also added #784
and fix tests with last commit (unrelated to any other commit, it is coming from the migration of the module I think)

@pedrobaeza pedrobaeza added this to the 17.0 milestone Feb 11, 2025
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 17.0-ocabot-merge-pr-788-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 3606a9a into OCA:17.0 Feb 11, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 5a0cfa1. Thanks a lot for contributing to OCA. ❤️

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants