-
Notifications
You must be signed in to change notification settings - Fork 26k
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
[IMP] l10n_latam_check: Allow multiple liquidity lines in own check #190670
base: 18.0
Are you sure you want to change the base?
Conversation
@vbe-odoo @jjscarafia FYI |
This PR introduces an improvement to the account and l10n_latam_check modules that allows for multiple liquidity lines (one per check) to be recorded in a single journal entry. Currently, a split journal entry is created for each liquidity line, which presents several disadvantages: - Incorrect computation of payment and invoice states - Simplified workflow: By using a single journal entry for all liquidity lines, the workflow is significantly simplified and accounting complexity is reduced. - First step towards a refactor: This improvement is a first step towards a broader refactor that will allow adapting the own check flow in payment registration without the need to create journal entries. Changes made: - Enhanced _synchronize_to_moves() method: - Allows for the creation and update of multiple liquidity lines within a single journal entry. - Removes excess liquidity lines if _prepare_move_line_default_vals() returns fewer lines than currently exist. - Account type-based counterparty identification: Replaces the previous position-based identification with an approach based on account types. - The starting index for extra line values is now dynamic, determined by the number of liquidity lines - Removed _l10n_latam_check_split_move method: - This method is no longer necessary given the new implementation that supports multiple liquidity lines in a single journal entry. - Modified _prepare_move_line_default_vals() method: - Now returns one liquidity line for each registered own check, simplifying the generation of journal entries. - Preserved _l10n_latam_check_unlink_split_move() method: - Maintained for backward compatibility purposes, allowing payments containing split moves to be set to draft status.
76a81d0
to
961c3d8
Compare
|
||
counterpart_line_vals = [x for x in line_vals_list if x['account_id'] == pay.company_id.transfer_account_id.id or self.env['account.account'].browse(x['account_id']).account_type in valid_account_types] | ||
line_ids_commands.append( | ||
Command.update(counterpart_lines.id, counterpart_line_vals[0]) if counterpart_lines else Command.create(counterpart_line_vals[0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me, this is completely wrong. A payment is supposed to be a single outstanding transaction. If you want to batch multiple of them, you have to use a batch payment. This is bringing too much cases, too much granularity and complexity in the code: batch -> payment -> liquidity lines. The constraint on the liquidity line has many usage a bit everywhere and you are breaking all those functionalities I think.
If I well understand, you want to use collected checks to pay another invoice? Why not creating a new payment for each check and group them in a batch payment then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @smetl
Thanks for taking the time to review this. This PR aims to correct inconsistencies in the payment status for checks in version 18.0.
In Argentina and other Latin American countries, it's common practice to pay multiple invoices with multiple checks and to deduct two or more withholdings from the total.
The ability to link multiple checks (from third parties and own) to a payment was added in 17.4 to allow for automatic calculation of withholdings, which got mixed up in 18.0.
In the case of own checks, in addition to reconciling them with the bank, they can also be returned or rejected by the bank.
It's common for a payment to be made with checks on different dates (e.g., 30, 60, 90 days) and the moment of reconciliation at the bank is also different.
The current solution creates a split entry that divides the liquidity line into several (each related to a check). This makes it possible to perform operations with each check separately. But it has the disadvantage of creating more entries and the payment status is not recorded correctly.
With the 18.0 payment refactor, all blocks to create multiple liquidity lines related to a payment were removed. In our tests, we didn't find any functionality that breaks with this modification. If there is any, it would be interesting to understand why. The only records that would create more than one liquidity line would be own checks.
Conceptually, this approach is much closer to the operation we want to represent. And I don't see any accounting problems with an entry having more than one liquidity line. The same applies to counterparty lines, though that is a separate matter. :)
On the other hand, batch payment would not solve the problem of calculating withholdings in multiple payments.
In which journal should the withholdings be recorded? In the case of paying a debt with a cash journal and deducting the withholdings from the amount, how would you do it if the payment is confirmed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just as an extra FYI, Standard WTH Module is being developed taking into account the AR approach (using the payment):
https://www.odoo.com/odoo/project.task/3614935
This PR introduces an improvement to the account and l10n_latam_check modules that allows for multiple liquidity lines (one per check) to be recorded in a single journal entry. Currently, a split journal entry is created for each liquidity line, which presents several disadvantages:
Changes made:
Enhanced _synchronize_to_moves() method:
Removed _l10n_latam_check_split_move method:
Modified _prepare_move_line_default_vals() method: - Now returns one liquidity line for each registered own check, simplifying the generation of journal entries.
Preserved _l10n_latam_check_unlink_split_move() method:
Description of the issue/feature this PR addresses:
Current behavior before PR:
Desired behavior after PR is merged:
opw-4236815
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr