From 6227007b7a988bc6325a6749db87cea6ff5c9b82 Mon Sep 17 00:00:00 2001 From: Laurent Smet Date: Wed, 10 Nov 2021 19:51:29 +0000 Subject: [PATCH] [FIX] account: Fix Receive bank account on register payment wizard - Create a partner with 2 bank accounts: BNK1 and BNK2 Wrong flow with vendor bill: - Create an vendor bill => BNK1 is set by default - Change BNK1 to BNK2 - Register a payment => BNK1 is proposed by default instead of BNK2 Wrong flow with customer invoice: - Set BNK3 on your company - Create a customer invoice - Register a payment => BNK1 is suggested instead of BNK3 closes odoo/odoo#80175 Issue: 2683197 X-original-commit: ec0d807b2b6ff3596d9b591bd8ec7d8889d41d8a Signed-off-by: Olivier Colson Signed-off-by: Laurent Smet --- addons/account/models/account_payment.py | 17 ++- addons/account/tests/test_account_payment.py | 40 ++++-- .../tests/test_account_payment_register.py | 52 ++++++-- .../wizard/account_payment_register.py | 121 +++++++----------- .../wizard/account_payment_register_views.xml | 1 + 5 files changed, 131 insertions(+), 100 deletions(-) diff --git a/addons/account/models/account_payment.py b/addons/account/models/account_payment.py index 9fd78ae4fb1..b982858af56 100644 --- a/addons/account/models/account_payment.py +++ b/addons/account/models/account_payment.py @@ -369,18 +369,21 @@ class AccountPayment(models.Model): else: payment.amount_signed = payment.amount - @api.depends('partner_id', 'destination_journal_id', 'is_internal_transfer') + @api.depends('partner_id', 'company_id', 'payment_type', 'destination_journal_id', 'is_internal_transfer') def _compute_partner_bank_id(self): ''' The default partner_bank_id will be the first available on the partner. ''' for pay in self: - if pay.is_internal_transfer: - pay.partner_bank_id = self.destination_journal_id.bank_account_id + if pay.payment_type == 'inbound': + bank_partner = pay.company_id.partner_id else: - available_partner_bank_accounts = pay.partner_id.bank_ids.filtered(lambda x: x.company_id in (False, pay.company_id)) - if available_partner_bank_accounts: + bank_partner = pay.partner_id + + available_partner_bank_accounts = bank_partner.bank_ids.filtered(lambda x: x.company_id in (False, pay.company_id)) + if available_partner_bank_accounts: + if pay.partner_bank_id not in available_partner_bank_accounts: pay.partner_bank_id = available_partner_bank_accounts[0]._origin - else: - pay.partner_bank_id = False + else: + pay.partner_bank_id = False @api.depends('partner_id', 'destination_account_id', 'journal_id') def _compute_is_internal_transfer(self): diff --git a/addons/account/tests/test_account_payment.py b/addons/account/tests/test_account_payment.py index 772ed6dd159..427ab9bde8a 100644 --- a/addons/account/tests/test_account_payment.py +++ b/addons/account/tests/test_account_payment.py @@ -16,21 +16,27 @@ class TestAccountPayment(AccountTestInvoicingCommon): cls.payment_debit_account_id = cls.copy_account(company_id.account_journal_payment_debit_account_id) cls.payment_credit_account_id = cls.copy_account(company_id.account_journal_payment_credit_account_id) - cls.partner_bank_account = cls.env['res.partner.bank'].create({ - 'acc_number': 'BE32707171912447', + cls.partner_bank_account1 = cls.env['res.partner.bank'].create({ + 'acc_number': "0123456789", 'partner_id': cls.partner_a.id, 'acc_type': 'bank', }) + cls.partner_bank_account2 = cls.env['res.partner.bank'].create({ + 'acc_number': "9876543210", + 'partner_id': cls.partner_a.id, + 'acc_type': 'bank', + }) + cls.comp_bank_account = cls.env['res.partner.bank'].create({ + 'acc_number': "985632147", + 'partner_id': cls.env.company.partner_id.id, + 'acc_type': 'bank', + }) company_id.write({ 'account_journal_payment_debit_account_id': cls.payment_debit_account_id.id, 'account_journal_payment_credit_account_id': cls.payment_credit_account_id.id }) - cls.partner_a.write({ - 'bank_ids': [(6, 0, cls.partner_bank_account.ids)], - }) - def test_payment_move_sync_create_write(self): copy_receivable = self.copy_account(self.company_data['default_account_receivable']) @@ -51,12 +57,10 @@ class TestAccountPayment(AccountTestInvoicingCommon): 'partner_id': False, 'destination_account_id': copy_receivable.id, 'payment_method_line_id': self.inbound_payment_method_line.id, - 'partner_bank_id': False, } expected_move_values = { 'currency_id': self.company_data['currency'].id, 'partner_id': False, - 'partner_bank_id': False, } expected_liquidity_line = { 'debit': 50.0, @@ -94,13 +98,11 @@ class TestAccountPayment(AccountTestInvoicingCommon): 'destination_account_id': self.partner_a.property_account_payable_id.id, 'currency_id': self.currency_data['currency'].id, 'partner_id': self.partner_a.id, - 'partner_bank_id': self.partner_bank_account.id, }]) self.assertRecordValues(payment.move_id, [{ **expected_move_values, 'currency_id': self.currency_data['currency'].id, 'partner_id': self.partner_a.id, - 'partner_bank_id': self.partner_bank_account.id, }]) self.assertRecordValues(payment.line_ids.sorted('balance'), [ { @@ -124,7 +126,6 @@ class TestAccountPayment(AccountTestInvoicingCommon): liquidity_lines, counterpart_lines, writeoff_lines = payment._seek_for_lines() payment.move_id.write({ - 'partner_bank_id': False, 'line_ids': [ (1, counterpart_lines.id, { 'debit': 0.0, @@ -798,7 +799,6 @@ class TestAccountPayment(AccountTestInvoicingCommon): 'currency_id': self.company_data['currency'].id, 'partner_id': False, 'payment_method_line_id': self.inbound_payment_method_line.id, - 'partner_bank_id': False, }]) payment.payment_type = 'outbound' @@ -811,5 +811,19 @@ class TestAccountPayment(AccountTestInvoicingCommon): 'currency_id': self.company_data['currency'].id, 'partner_id': False, 'payment_method_line_id': self.outbound_payment_method_line.id, - 'partner_bank_id': False, }]) + + def test_payment_partner_bank_inbound(self): + """ Test the bank account is well recomputed for inbound payments. In that case, the recipient + bank account must be the one set on the company. + """ + payment = self.env['account.payment'].create({ + 'amount': 50.0, + 'payment_type': 'outbound', + 'partner_type': 'supplier', + 'partner_id': self.partner_a.id, + }) + self.assertRecordValues(payment, [{'partner_bank_id': self.partner_bank_account1.id}]) + + payment.payment_type = 'inbound' + self.assertRecordValues(payment, [{'partner_bank_id': self.comp_bank_account.id}]) diff --git a/addons/account/tests/test_account_payment_register.py b/addons/account/tests/test_account_payment_register.py index 0d4c50c6a6f..62d9dab723c 100644 --- a/addons/account/tests/test_account_payment_register.py +++ b/addons/account/tests/test_account_payment_register.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- from odoo.addons.account.tests.common import AccountTestInvoicingCommon from odoo.exceptions import UserError -from odoo.tests import tagged +from odoo.tests import tagged, Form @tagged('post_install', '-at_install') @@ -21,6 +21,22 @@ class TestAccountPaymentRegister(AccountTestInvoicingCommon): cls.payment_debit_account_id = cls.company_data['default_journal_bank'].company_id.account_journal_payment_debit_account_id.copy() cls.payment_credit_account_id = cls.company_data['default_journal_bank'].company_id.account_journal_payment_credit_account_id.copy() + cls.partner_bank_account = cls.env['res.partner.bank'].create({ + 'acc_number': "0123456789", + 'partner_id': cls.partner_a.id, + 'acc_type': 'bank', + }) + cls.comp_bank_account1 = cls.env['res.partner.bank'].create({ + 'acc_number': "985632147", + 'partner_id': cls.env.company.partner_id.id, + 'acc_type': 'bank', + }) + cls.comp_bank_account2 = cls.env['res.partner.bank'].create({ + 'acc_number': "741258963", + 'partner_id': cls.env.company.partner_id.id, + 'acc_type': 'bank', + }) + # Customer invoices sharing the same batch. cls.out_invoice_1 = cls.env['account.move'].create({ 'move_type': 'out_invoice', @@ -389,6 +405,7 @@ class TestAccountPaymentRegister(AccountTestInvoicingCommon): payments = self.env['account.payment.register'].with_context(active_model='account.move', active_ids=active_ids).create({ 'group_payment': True, })._create_payments() + self.assertRecordValues(payments, [ { 'ref': 'BILL/2017/01/0001 BILL/2017/01/0002 RBILL/2017/01/0001', @@ -416,23 +433,23 @@ class TestAccountPaymentRegister(AccountTestInvoicingCommon): def test_register_payment_multiple_batch_grouped_with_credit_note(self): ''' Do not batch payments if multiple partner_bank_id ''' - test_bank = self.env['res.bank'].create({'name': 'test'}) bank1 = self.env['res.partner.bank'].create({ 'acc_number': 'BE43798822936101', 'partner_id': self.partner_a.id, - 'bank_id': test_bank.id, }) bank2 = self.env['res.partner.bank'].create({ 'acc_number': 'BE85812541345906', 'partner_id': self.partner_a.id, - 'bank_id': test_bank.id, }) + self.in_invoice_1.partner_bank_id = bank1 self.in_invoice_2.partner_bank_id = bank2 + active_ids = (self.in_invoice_1 + self.in_invoice_2 + self.in_refund_1).ids payments = self.env['account.payment.register'].with_context(active_model='account.move', active_ids=active_ids).create({ 'group_payment': True, })._create_payments() + self.assertRecordValues(payments, [ { 'ref': 'BILL/2017/01/0001', @@ -497,13 +514,11 @@ class TestAccountPaymentRegister(AccountTestInvoicingCommon): 'reconciled': False, }, ]) - self.in_invoice_1.partner_bank_id = None - self.in_invoice_2.partner_bank_id = None def test_register_payment_multi_batches_grouped(self): ''' Choose to pay multiple batches, one with two customer invoices (1000 + 2000) - and one with a vendor bill of 600, by grouping payments. - ''' + and one with a vendor bill of 600, by grouping payments. + ''' active_ids = (self.in_invoice_1 + self.in_invoice_2 + self.in_invoice_3).ids payments = self.env['account.payment.register'].with_context(active_model='account.move', active_ids=active_ids).create({ 'group_payment': True, @@ -633,6 +648,27 @@ class TestAccountPaymentRegister(AccountTestInvoicingCommon): }, ]) + def test_register_payment_custom_bank_account(self): + """ Ensure the user is able to select a custom bank account when registering a payment and this bank account + lands correctly on the generated payment. + """ + self.out_invoice_1.partner_bank_id = self.comp_bank_account1 + + ctx = {'active_model': 'account.move', 'active_ids': self.out_invoice_1.ids} + wizard_form = Form(self.env['account.payment.register'].with_context(**ctx)) + wizard = wizard_form.save() + + # The bank account set on the invoice must be the default suggested value. + self.assertRecordValues(wizard, [{'partner_bank_id': self.comp_bank_account1.id}]) + + wizard_form = Form(wizard) + wizard_form.partner_bank_id = self.comp_bank_account2 + wizard = wizard_form.save() + payments = wizard._create_payments() + + # The user should be able to set a custom bank account. + self.assertRecordValues(payments, [{'partner_bank_id': self.comp_bank_account2.id}]) + def test_register_payment_constraints(self): # Test to register a payment for a draft journal entry. self.out_invoice_1.button_draft() diff --git a/addons/account/wizard/account_payment_register.py b/addons/account/wizard/account_payment_register.py index 66a6d277198..a71e22d5ad0 100644 --- a/addons/account/wizard/account_payment_register.py +++ b/addons/account/wizard/account_payment_register.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- +from collections import defaultdict from odoo import models, fields, api, _ from odoo.exceptions import UserError @@ -29,10 +30,18 @@ class AccountPaymentRegister(models.TransientModel): comodel_name='account.journal', compute='_compute_available_journal_ids' ) - partner_bank_id = fields.Many2one('res.partner.bank', string="Recipient Bank Account", - readonly=False, store=True, + available_partner_bank_ids = fields.Many2many( + comodel_name='res.partner.bank', + compute='_compute_available_partner_bank_ids', + ) + partner_bank_id = fields.Many2one( + comodel_name='res.partner.bank', + string="Recipient Bank Account", + readonly=False, + store=True, compute='_compute_partner_bank_id', - domain="['|', ('company_id', '=', False), ('company_id', '=', company_id), ('partner_id', '=', partner_id)]") + domain="[('id', 'in', available_partner_bank_ids)]", + ) company_currency_id = fields.Many2one('res.currency', string="Company Currency", related='company_id.currency_id') @@ -126,18 +135,23 @@ class AccountPaymentRegister(models.TransientModel): will be grouped together. :return: A python dictionary. ''' - res = { + move = line.move_id + partner_bank_account = self.env['res.partner.bank'] + + if move.is_invoice(include_receipts=True): + partner_bank_account = move.partner_bank_id._origin + + return { 'partner_id': line.partner_id.id, 'account_id': line.account_id.id, 'currency_id': (line.currency_id or line.company_currency_id).id, + 'partner_bank_id': partner_bank_account.id, 'partner_type': 'customer' if line.account_internal_type == 'receivable' else 'supplier', } - return res - def _get_batches(self): ''' Group the account.move.line linked to the wizard together. - Lines are groupes if they share 'partner_id','account_id','currency_id' & 'partner_type' and if + Lines are grouped if they share 'partner_id','account_id','currency_id' & 'partner_type' and if 0 or 1 partner_bank_id can be determined for the group. :return: A list of batches, each one containing: * payment_values: A dictionary of payment values. @@ -152,69 +166,21 @@ class AccountPaymentRegister(models.TransientModel): if not lines: raise UserError(_("You can't open the register payment wizard without at least one receivable/payable line.")) - # Lines are grouped first on 'partner_id','account_id','currency_id' & 'partner_type' and - # then on 'partner_bank_id' & 'payment_type'. 'batches' data structure : - # {common_pay_val_key:{ - # part_bank_pay_type_key : lines, - # part_bank_pay_type_key : lines, - # }} - batches = {} - + batches = defaultdict(lambda: {'lines': self.env['account.move.line']}) for line in lines: batch_key = self._get_line_batch_key(line) - common_pay_val_key = frozendict(batch_key) - part_bank_pay_type_key = frozendict( - payment_type='inbound' if line.balance > 0.0 else 'outbound', - partner_bank_id=line.move_id.partner_bank_id.id, - ) - batches.setdefault(common_pay_val_key, {}) - batches[common_pay_val_key].setdefault(part_bank_pay_type_key, self.env['account.move.line']) - batches[common_pay_val_key][part_bank_pay_type_key] += line + serialized_key = '-'.join(str(v) for v in batch_key.values()) + vals = batches[serialized_key] + vals['payment_values'] = batch_key + vals['lines'] += line - res = [] + # Compute 'payment_type'. + for vals in batches.values(): + lines = vals['lines'] + balance = sum(lines.mapped('balance')) + vals['payment_values']['payment_type'] = 'inbound' if balance > 0.0 else 'outbound' - for common_pay_val_key, common_params_group in batches.items(): - # Group all lines with same 'partner_id','account_id','currency_id' & 'partner_type' in one batch if: - # - the resulting batch payment is outbound and there is max 1 partner_bank_id for all outbound lines - # or - # - the resulting batch payment is inbound and there is max 1 partner_bank_id for all inbound lines - all_batch_aml = self.env['account.move.line'].browse( - id - for lines in common_params_group.values() - for id in lines.ids - ) - total = sum(all_batch_aml.mapped('balance')) - if float_compare(total, 0.0, precision_digits=line.move_id.currency_id.decimal_places) > 0.0: - if_grouped_payment_type = 'inbound' - else: - if_grouped_payment_type = 'outbound' - partner_bank_id_candidates = { - key['partner_bank_id'] - for key in common_params_group - if key['partner_bank_id'] and key['payment_type'] == if_grouped_payment_type - } - if len(partner_bank_id_candidates) < 2: - # condition is met, grouping in 1 batch - partner_bank_id = partner_bank_id_candidates and partner_bank_id_candidates.pop() or None - payment_values = dict(common_pay_val_key) - payment_values['partner_bank_id'] = partner_bank_id - payment_values['payment_type'] = if_grouped_payment_type - res.append({ - 'payment_values': payment_values, - 'lines': all_batch_aml, - }) - else: - # no grouping - for part_bank_pay_type_key, lines in common_params_group.items(): - payment_values = dict(common_pay_val_key) - payment_values['payment_type'] = part_bank_pay_type_key['payment_type'] - payment_values['partner_bank_id'] = part_bank_pay_type_key['partner_bank_id'] - res.append({ - 'payment_values': payment_values, - 'lines': lines, - }) - - return res + return list(batches.values()) @api.model def _get_wizard_values_from_batch(self, batch_result): @@ -268,6 +234,7 @@ class AccountPaymentRegister(models.TransientModel): 'partner_id': False, 'partner_type': False, 'payment_type': wizard_values_from_batch['payment_type'], + 'partner_bank_id': False, 'source_currency_id': False, 'source_amount': False, 'source_amount_currency': False, @@ -345,13 +312,23 @@ class AccountPaymentRegister(models.TransientModel): for wizard in self: wizard.currency_id = wizard.journal_id.currency_id or wizard.source_currency_id or wizard.company_id.currency_id - @api.depends('partner_id') - def _compute_partner_bank_id(self): - ''' The default partner_bank_id will be the first available on the partner. ''' + @api.depends('company_id', 'can_edit_wizard') + def _compute_available_partner_bank_ids(self): for wizard in self: - available_partner_bank_accounts = wizard.partner_id.bank_ids.filtered(lambda x: x.company_id in (False, wizard.company_id)) - if available_partner_bank_accounts: - wizard.partner_bank_id = available_partner_bank_accounts[0]._origin + if wizard.can_edit_wizard: + batches = wizard._get_batches() + bank_partners = batches[0]['lines'].move_id.bank_partner_id + wizard.available_partner_bank_ids = bank_partners.bank_ids\ + .filtered(lambda x: x.company_id in (False, wizard.company_id))._origin + else: + wizard.available_partner_bank_ids = False + + @api.depends('available_partner_bank_ids') + def _compute_partner_bank_id(self): + for wizard in self: + if wizard.can_edit_wizard: + batches = wizard._get_batches() + wizard.partner_bank_id = self.env['res.partner.bank'].browse(batches[0]['payment_values']['partner_bank_id']) else: wizard.partner_bank_id = False diff --git a/addons/account/wizard/account_payment_register_views.xml b/addons/account/wizard/account_payment_register_views.xml index e85e07efe77..0c929dc199e 100644 --- a/addons/account/wizard/account_payment_register_views.xml +++ b/addons/account/wizard/account_payment_register_views.xml @@ -24,6 +24,7 @@ +