diff --git a/addons/account/models/account_payment.py b/addons/account/models/account_payment.py index bfbe5ecfc7c..25061f929c4 100644 --- a/addons/account/models/account_payment.py +++ b/addons/account/models/account_payment.py @@ -1,5 +1,4 @@ # -*- coding: utf-8 -*- - from odoo import models, fields, api, _ from odoo.exceptions import UserError, ValidationError @@ -33,10 +32,14 @@ class AccountPayment(models.Model): is_matched = fields.Boolean(string="Is Matched With a Bank Statement", store=True, compute='_compute_reconciliation_status', help="Technical field indicating if the payment has been matched with a statement line.") + available_partner_bank_ids = fields.Many2many( + comodel_name='res.partner.bank', + compute='_compute_available_partner_bank_ids', + ) partner_bank_id = fields.Many2one('res.partner.bank', string="Recipient Bank Account", readonly=False, store=True, tracking=True, compute='_compute_partner_bank_id', - domain="[('partner_id', '=', partner_id)]", + domain="[('id', 'in', available_partner_bank_ids)]", check_company=True) is_internal_transfer = fields.Boolean(string="Internal Transfer", readonly=False, store=True, @@ -369,21 +372,20 @@ class AccountPayment(models.Model): else: payment.amount_signed = payment.amount - @api.depends('partner_id', 'company_id', 'payment_type', 'destination_journal_id', 'is_internal_transfer') + @api.depends('partner_id', 'company_id', 'payment_type') + def _compute_available_partner_bank_ids(self): + for pay in self: + if pay.payment_type == 'inbound': + pay.available_partner_bank_ids = pay.journal_id.bank_account_id + else: + pay.available_partner_bank_ids = pay.partner_id.bank_ids\ + .filtered(lambda x: x.company_id.id in (False, pay.company_id.id))._origin + + @api.depends('available_partner_bank_ids', 'journal_id') 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.payment_type == 'inbound': - bank_partner = pay.company_id.partner_id - else: - bank_partner = pay.partner_id - - available_partner_bank_accounts = bank_partner.bank_ids.filtered(lambda x: x.company_id.id in (False, pay.company_id.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 + pay.partner_bank_id = pay.available_partner_bank_ids[:1]._origin @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 427ab9bde8a..b7edcb1fc2c 100644 --- a/addons/account/tests/test_account_payment.py +++ b/addons/account/tests/test_account_payment.py @@ -11,10 +11,13 @@ class TestAccountPayment(AccountTestInvoicingCommon): def setUpClass(cls, chart_template_ref=None): super().setUpClass(chart_template_ref=chart_template_ref) - company_id = cls.company_data['default_journal_bank'].company_id + company = cls.company_data['default_journal_bank'].company_id - 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.payment_debit_account_id = cls.copy_account(company.account_journal_payment_debit_account_id) + cls.payment_credit_account_id = cls.copy_account(company.account_journal_payment_credit_account_id) + + cls.bank_journal_1 = cls.company_data['default_journal_bank'] + cls.bank_journal_2 = cls.company_data['default_journal_bank'].copy() cls.partner_bank_account1 = cls.env['res.partner.bank'].create({ 'acc_number': "0123456789", @@ -26,13 +29,18 @@ class TestAccountPayment(AccountTestInvoicingCommon): 'partner_id': cls.partner_a.id, 'acc_type': 'bank', }) - cls.comp_bank_account = cls.env['res.partner.bank'].create({ + 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', + }) - company_id.write({ + company.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 }) @@ -813,17 +821,34 @@ class TestAccountPayment(AccountTestInvoicingCommon): 'payment_method_line_id': self.outbound_payment_method_line.id, }]) - 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. + def test_suggested_default_partner_bank(self): + """ Ensure the 'partner_bank_id' is well computed on payments. When the payment is inbound, the money must be + received by a bank account linked to the company. In case of outbound payment, the bank account must be found + on the partner. """ payment = self.env['account.payment'].create({ + 'journal_id': self.bank_journal_1.id, '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}]) + self.assertRecordValues(payment, [{ + 'available_partner_bank_ids': self.partner_a.bank_ids.ids, + 'partner_bank_id': self.partner_bank_account1.id, + }]) payment.payment_type = 'inbound' - self.assertRecordValues(payment, [{'partner_bank_id': self.comp_bank_account.id}]) + self.assertRecordValues(payment, [{ + 'available_partner_bank_ids': [], + 'partner_bank_id': False, + }]) + + self.bank_journal_2.bank_account_id = self.comp_bank_account2 + # A sequence is automatically added on the first move. We need to clean it before changing the journal. + payment.name = False + payment.journal_id = self.bank_journal_2 + self.assertRecordValues(payment, [{ + 'available_partner_bank_ids': self.comp_bank_account2.ids, + 'partner_bank_id': self.comp_bank_account2.id, + }]) diff --git a/addons/account/tests/test_account_payment_register.py b/addons/account/tests/test_account_payment_register.py index 62d9dab723c..18abfbfa36b 100644 --- a/addons/account/tests/test_account_payment_register.py +++ b/addons/account/tests/test_account_payment_register.py @@ -21,11 +21,19 @@ 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({ + cls.bank_journal_1 = cls.company_data['default_journal_bank'] + cls.bank_journal_2 = cls.company_data['default_journal_bank'].copy() + + 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_account1 = cls.env['res.partner.bank'].create({ 'acc_number': "985632147", 'partner_id': cls.env.company.partner_id.id, @@ -575,6 +583,9 @@ class TestAccountPaymentRegister(AccountTestInvoicingCommon): ''' Choose to pay multiple batches, one with two customer invoices (1000 + 2000) and one with a vendor bill of 600, by splitting payments. ''' + self.in_invoice_1.partner_bank_id = self.partner_bank_account1 + self.in_invoice_2.partner_bank_id = self.partner_bank_account2 + 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': False, @@ -582,16 +593,22 @@ class TestAccountPaymentRegister(AccountTestInvoicingCommon): self.assertRecordValues(payments, [ { + 'journal_id': self.bank_journal_1.id, 'ref': 'BILL/2017/01/0001', 'payment_method_line_id': self.outbound_payment_method_line.id, + 'partner_bank_id': self.partner_bank_account1.id, }, { + 'journal_id': self.bank_journal_1.id, 'ref': 'BILL/2017/01/0002', 'payment_method_line_id': self.outbound_payment_method_line.id, + 'partner_bank_id': self.partner_bank_account2.id, }, { + 'journal_id': self.bank_journal_1.id, 'ref': 'BILL/2017/01/0003', 'payment_method_line_id': self.outbound_payment_method_line.id, + 'partner_bank_id': False, }, ]) self.assertRecordValues(payments[0].line_ids.sorted('balance') + payments[1].line_ids.sorted('balance') + payments[2].line_ids.sorted('balance'), [ @@ -648,27 +665,6 @@ 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() @@ -933,3 +929,80 @@ class TestAccountPaymentRegister(AccountTestInvoicingCommon): 'reconciled': False, }, ]) + + def test_suggested_default_partner_bank_inbound_payment(self): + """ Test the suggested bank account on the wizard for inbound payment. """ + self.out_invoice_1.partner_bank_id = False + + ctx = {'active_model': 'account.move', 'active_ids': self.out_invoice_1.ids} + wizard = self.env['account.payment.register'].with_context(**ctx).create({}) + self.assertRecordValues(wizard, [{ + 'journal_id': self.bank_journal_1.id, + 'available_partner_bank_ids': [], + 'partner_bank_id': False, + }]) + + self.bank_journal_2.bank_account_id = self.out_invoice_1.partner_bank_id = self.comp_bank_account2 + wizard = self.env['account.payment.register'].with_context(**ctx).create({}) + self.assertRecordValues(wizard, [{ + 'journal_id': self.bank_journal_2.id, + 'available_partner_bank_ids': self.comp_bank_account2.ids, + 'partner_bank_id': self.comp_bank_account2.id, + }]) + + wizard.journal_id = self.bank_journal_1 + self.assertRecordValues(wizard, [{ + 'journal_id': self.bank_journal_1.id, + 'available_partner_bank_ids': [], + 'partner_bank_id': False, + }]) + + def test_suggested_default_partner_bank_outbound_payment(self): + """ Test the suggested bank account on the wizard for outbound payment. """ + self.in_invoice_1.partner_bank_id = False + + ctx = {'active_model': 'account.move', 'active_ids': self.in_invoice_1.ids} + wizard = self.env['account.payment.register'].with_context(**ctx).create({}) + self.assertRecordValues(wizard, [{ + 'journal_id': self.bank_journal_1.id, + 'available_partner_bank_ids': self.partner_a.bank_ids.ids, + 'partner_bank_id': self.partner_bank_account1.id, + }]) + + self.in_invoice_1.partner_bank_id = self.partner_bank_account2 + wizard = self.env['account.payment.register'].with_context(**ctx).create({}) + self.assertRecordValues(wizard, [{ + 'journal_id': self.bank_journal_1.id, + 'available_partner_bank_ids': self.partner_a.bank_ids.ids, + 'partner_bank_id': self.partner_bank_account2.id, + }]) + + wizard.journal_id = self.bank_journal_2 + self.assertRecordValues(wizard, [{ + 'journal_id': self.bank_journal_2.id, + 'available_partner_bank_ids': self.partner_a.bank_ids.ids, + 'partner_bank_id': self.partner_bank_account2.id, + }]) + + def test_register_payment_inbound_multiple_bank_account(self): + """ Pay customer invoices with different bank accounts. """ + self.out_invoice_1.partner_bank_id = self.comp_bank_account1 + self.out_invoice_2.partner_bank_id = self.comp_bank_account2 + self.bank_journal_2.bank_account_id = self.comp_bank_account2 + + ctx = {'active_model': 'account.move', 'active_ids': (self.out_invoice_1 + self.out_invoice_2).ids} + wizard = self.env['account.payment.register'].with_context(**ctx).create({'journal_id': self.bank_journal_2.id}) + payments = wizard._create_payments() + + self.assertRecordValues(payments, [ + { + 'journal_id': self.bank_journal_2.id, + 'ref': 'INV/2017/00001', + 'partner_bank_id': self.comp_bank_account2.id, + }, + { + 'journal_id': self.bank_journal_2.id, + 'ref': 'INV/2017/00002', + 'partner_bank_id': self.comp_bank_account2.id, + }, + ]) diff --git a/addons/account/views/account_payment_view.xml b/addons/account/views/account_payment_view.xml index 44021749081..c4dd944e321 100644 --- a/addons/account/views/account_payment_view.xml +++ b/addons/account/views/account_payment_view.xml @@ -178,6 +178,7 @@ + diff --git a/addons/account/wizard/account_payment_register.py b/addons/account/wizard/account_payment_register.py index 35a144d778f..61fe60a8549 100644 --- a/addons/account/wizard/account_payment_register.py +++ b/addons/account/wizard/account_payment_register.py @@ -129,6 +129,73 @@ class AccountPaymentRegister(models.TransientModel): labels = set(line.name or line.move_id.ref or line.move_id.name for line in batch_result['lines']) return ' '.join(sorted(labels)) + @api.model + def _get_batch_available_journals(self, batch_result): + """ Helper to compute the available journals based on the batch. + + :param batch_result: A batch returned by '_get_batches'. + :return: A recordset of account.journal. + """ + payment_type = batch_result['payment_values']['payment_type'] + company = batch_result['lines'].company_id + journals = self.env['account.journal'].search([('company_id', '=', company.id), ('type', 'in', ('bank', 'cash'))]) + if payment_type == 'inbound': + return journals.filtered('inbound_payment_method_line_ids') + else: + return journals.filtered('outbound_payment_method_line_ids') + + @api.model + def _get_batch_journal(self, batch_result): + """ Helper to compute the journal based on the batch. + + :param batch_result: A batch returned by '_get_batches'. + :return: An account.journal record. + """ + payment_values = batch_result['payment_values'] + foreign_currency_id = payment_values['currency_id'] + partner_bank_id = payment_values['partner_bank_id'] + + currency_domain = [('currency_id', '=', foreign_currency_id)] + partner_bank_domain = [('bank_account_id', '=', partner_bank_id)] + + default_domain = [ + ('type', 'in', ('bank', 'cash')), + ('company_id', '=', batch_result['lines'].company_id.id), + ] + + if partner_bank_id: + extra_domains = ( + currency_domain + partner_bank_domain, + partner_bank_domain, + currency_domain, + [], + ) + else: + extra_domains = ( + currency_domain, + [], + ) + + for extra_domain in extra_domains: + journal = self.env['account.journal'].search(default_domain + extra_domain, limit=1) + if journal: + return journal + + return self.env['account.journal'] + + @api.model + def _get_batch_available_partner_banks(self, batch_result, journal): + payment_values = batch_result['payment_values'] + company = batch_result['lines'].company_id + + # A specific bank account is set on the journal. The user must use this one. + if payment_values['payment_type'] == 'inbound': + # Receiving money on a bank account linked to the journal. + return journal.bank_account_id + else: + # Sending money to a bank account owned by a partner. + return batch_result['lines'].partner_id.bank_ids.filtered(lambda x: x.company_id.id in (False, company.id))._origin + @api.model def _get_line_batch_key(self, line): ''' Turn the line passed as parameter to a dictionary defining on which way the lines @@ -136,15 +203,15 @@ class AccountPaymentRegister(models.TransientModel): :return: A python dictionary. ''' move = line.move_id - partner_bank_account = self.env['res.partner.bank'] + 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, + 'currency_id': line.currency_id.id, 'partner_bank_id': partner_bank_account.id, 'partner_type': 'customer' if line.account_internal_type == 'receivable' else 'supplier', } @@ -234,7 +301,6 @@ 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, @@ -263,74 +329,57 @@ class AccountPaymentRegister(models.TransientModel): else: wizard.group_payment = False - @api.depends('company_id', 'source_currency_id', 'line_ids') - def _compute_journal_id(self): - for wizard in self: - if wizard.journal_id: - wizard.journal_id = wizard.journal_id - else: - partner_bank_id = wizard.line_ids.move_id.mapped('partner_bank_id') - - company_domain = [('company_id', '=', wizard.company_id.id), ('id', 'in', wizard.available_journal_ids.ids)] - bank_domain = [('bank_account_id', '=', partner_bank_id.id), ('type', '=', 'bank')] if len(partner_bank_id) == 1 else None - no_bank_domain = [('type', 'in', ('bank', 'cash'))] - - journal = None - if wizard.source_currency_id: - currency_domain = [('currency_id', '=', wizard.source_currency_id.id)] - if bank_domain: - journal = self.env['account.journal'].search(company_domain + currency_domain + bank_domain, limit=1) - if not journal: - journal = self.env['account.journal'].search(company_domain + currency_domain + no_bank_domain, limit=1) - if not journal and bank_domain: - journal = self.env['account.journal'].search(company_domain + bank_domain, limit=1) - if not journal: - journal = self.env['account.journal'].search(company_domain + no_bank_domain, limit=1) - - wizard.journal_id = journal - - @api.depends('payment_type') - def _compute_available_journal_ids(self): - """ - Get all journals having at least one payment method for inbound/outbound depending on the payment_type. - """ - journals = self.env['account.journal'].search([ - ('company_id', 'in', self.company_id.ids), ('type', 'in', ('bank', 'cash')) - ]) - for pay in self: - if pay.payment_type == 'inbound': - pay.available_journal_ids = journals.filtered( - lambda j: j.company_id == pay.company_id and j.inbound_payment_method_line_ids.ids != [] - ) - else: - pay.available_journal_ids = journals.filtered( - lambda j: j.company_id == pay.company_id and j.outbound_payment_method_line_ids.ids != [] - ) - @api.depends('journal_id') def _compute_currency_id(self): 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('company_id', 'can_edit_wizard') + @api.depends('payment_type', 'company_id', 'can_edit_wizard') + def _compute_available_journal_ids(self): + for wizard in self: + if wizard.can_edit_wizard: + batch = wizard._get_batches()[0] + wizard.available_journal_ids = wizard._get_batch_available_journals(batch) + else: + wizard.available_journal_ids = self.env['account.journal'].search([ + ('company_id', '=', wizard.company_id.id), + ('type', 'in', ('bank', 'cash')), + ]) + + @api.depends('available_journal_ids') + def _compute_journal_id(self): + for wizard in self: + if wizard.can_edit_wizard: + batch = wizard._get_batches()[0] + wizard.journal_id = wizard._get_batch_journal(batch) + else: + wizard.journal_id = self.env['account.journal'].search([ + ('type', 'in', ('bank', 'cash')), + ('company_id', '=', wizard.company_id.id), + ], limit=1) + + @api.depends('can_edit_wizard', 'journal_id') def _compute_available_partner_bank_ids(self): for wizard in self: 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.id in (False, wizard.company_id.id))._origin + batch = wizard._get_batches()[0] + wizard.available_partner_bank_ids = wizard._get_batch_available_partner_banks(batch, wizard.journal_id) else: - wizard.available_partner_bank_ids = False + wizard.available_partner_bank_ids = None - @api.depends('available_partner_bank_ids') + @api.depends('journal_id', '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']) + batch = wizard._get_batches()[0] + partner_bank_id = batch['payment_values']['partner_bank_id'] + available_partner_banks = wizard.available_partner_bank_ids._origin + if partner_bank_id and partner_bank_id in available_partner_banks.ids: + wizard.partner_bank_id = self.env['res.partner.bank'].browse(partner_bank_id) + else: + wizard.partner_bank_id = available_partner_banks[:1] else: - wizard.partner_bank_id = False + wizard.partner_bank_id = None @api.depends('payment_type', 'journal_id') def _compute_payment_method_line_fields(self): @@ -469,6 +518,12 @@ class AccountPaymentRegister(models.TransientModel): def _create_payment_vals_from_batch(self, batch_result): batch_values = self._get_wizard_values_from_batch(batch_result) + + if batch_values['payment_type'] == 'inbound': + partner_bank_id = self.journal_id.bank_account_id.id + else: + partner_bank_id = batch_result['payment_values']['partner_bank_id'] + return { 'date': self.payment_date, 'amount': batch_values['source_amount_currency'], @@ -478,7 +533,7 @@ class AccountPaymentRegister(models.TransientModel): 'journal_id': self.journal_id.id, 'currency_id': batch_values['source_currency_id'], 'partner_id': batch_values['partner_id'], - 'partner_bank_id': batch_result['payment_values']['partner_bank_id'], + 'partner_bank_id': partner_bank_id, 'payment_method_line_id': self.payment_method_line_id.id, 'destination_account_id': batch_result['lines'][0].account_id.id }