From 74b6f84cc83a9adee7d78e77a5e28465a30ce03a Mon Sep 17 00:00:00 2001 From: Olivier Colson Date: Fri, 10 Aug 2018 16:21:39 +0200 Subject: [PATCH] [IMP] account: make partner account selection easier for vendor bills payment This is done by displaying this field on top of the vendor bills form view and using it as a default value when registering the payment. The use of a sequence on res.partner.bank also allows giving priorities to partner bank accounts directly from the tree view that is contained in res.partner form. In case payments are generated for multiple invoices at once, their account is set as the partner's most prioritary account. [IMP] account: payment for multiple invoices: option to group + also group on account + hide memo Add an option in the multiple invoices payment wizard to choose whether or not to group the generated payments. This is useful in case we want to keep one payment by invoice (even if they share the same partner, etc) so that each of them corresponds to a single communication. The memo field of this wizard is now also hidden in some cases, when displaying it would allow entering wrong communication values (for example, when paying invoices from different partners ; it makes no sense that all the generated payments share the same memo). [IMP] account: option to sanize invoice communications --- addons/account/models/account_invoice.py | 16 ++- .../models/account_journal_dashboard.py | 2 + addons/account/models/account_payment.py | 97 ++++++++++++++++--- addons/account/models/company.py | 1 + addons/account/models/res_config_settings.py | 1 + addons/account/tests/test_payment.py | 70 ++++++++----- addons/account/views/account_invoice_view.xml | 3 +- addons/account/views/account_payment_view.xml | 5 +- addons/account/views/partner_view.xml | 3 +- .../views/res_config_settings_views.xml | 11 +++ 10 files changed, 165 insertions(+), 44 deletions(-) diff --git a/addons/account/models/account_invoice.py b/addons/account/models/account_invoice.py index 58711dfb92f..785159896fd 100644 --- a/addons/account/models/account_invoice.py +++ b/addons/account/models/account_invoice.py @@ -508,6 +508,15 @@ class AccountInvoice(models.Model): return invoice + @api.constrains('partner_id', 'partner_bank_id') + def validate_partner_bank_id(self): + for record in self: + if record.partner_bank_id: + if record.type in ('in_invoice', 'out_refund') and record.partner_bank_id.partner_id != record.partner_id.commercial_partner_id: + raise ValidationError(_("Commercial partner and vendor account owners must be identical.")) + elif record.type in ('out_invoice', 'in_refund') and not record.company_id in record.partner_bank_id.partner_id.ref_company_ids: + raise ValidationError(_("The account selected for payment does not belong to the same company as this invoice.")) + @api.multi def _write(self, vals): pre_not_reconciled = self.filtered(lambda invoice: not invoice.reconciled) @@ -526,7 +535,7 @@ class AccountInvoice(models.Model): """ res = super(AccountInvoice, self).default_get(default_fields) - if not res.get('type', False) == 'out_invoice' or not 'company_id' in res: + if res.get('type', False) not in ('out_invoice', 'in_refund') or not 'company_id' in res: return res company = self.env['res.company'].browse(res['company_id']) @@ -874,7 +883,7 @@ class AccountInvoice(models.Model): def action_invoice_open(self): # lots of duplicate calls to action_invoice_open, so we remove those already open to_open_invoices = self.filtered(lambda inv: inv.state != 'open') - for inv in to_open_invoices.filtered(lambda inv: not inv.partner_id): + if to_open_invoices.filtered(lambda inv: not inv.partner_id): raise UserError(_("The field Vendor is required, please complete it to validate the Vendor Bill.")) if to_open_invoices.filtered(lambda inv: inv.state != 'draft'): raise UserError(_("Invoice must be in draft state in order to validate it.")) @@ -882,6 +891,9 @@ class AccountInvoice(models.Model): raise UserError(_("You cannot validate an invoice with a negative total amount. You should create a credit note instead.")) if to_open_invoices.filtered(lambda inv: not inv.account_id): raise UserError(_('No account was found to create the invoice, be sure you have installed a chart of account.')) + for record in to_open_invoices: + if record.company_id.account_sanitize_invoice_ref and record.reference: + record.reference = self.env['account.payment']._sanitize_communication(record.reference) to_open_invoices.action_date_assign() to_open_invoices.action_move_create() return to_open_invoices.invoice_validate() diff --git a/addons/account/models/account_journal_dashboard.py b/addons/account/models/account_journal_dashboard.py index da0c1dbb960..56338b54efb 100644 --- a/addons/account/models/account_journal_dashboard.py +++ b/addons/account/models/account_journal_dashboard.py @@ -320,8 +320,10 @@ class account_journal(models.Model): action_name = 'action_view_bank_statement_tree' elif self.type == 'sale': action_name = 'action_invoice_tree1' + self = self.with_context(use_domain=[('type', '=', 'out_invoice')]) elif self.type == 'purchase': action_name = 'action_vendor_bill_template' + self = self.with_context(use_domain=[('type', '=', 'in_invoice')]) else: action_name = 'action_move_journal_line' diff --git a/addons/account/models/account_payment.py b/addons/account/models/account_payment.py index 1976912110c..121c5a38d11 100644 --- a/addons/account/models/account_payment.py +++ b/addons/account/models/account_payment.py @@ -5,6 +5,7 @@ from odoo.exceptions import UserError, ValidationError from odoo.tools import float_compare from itertools import groupby +import re MAP_INVOICE_TYPE_PARTNER_TYPE = { @@ -92,6 +93,7 @@ class account_abstract_payment(models.AbstractModel): multi = any(inv.commercial_partner_id != invoices[0].commercial_partner_id or MAP_INVOICE_TYPE_PARTNER_TYPE[inv.type] != MAP_INVOICE_TYPE_PARTNER_TYPE[invoices[0].type] or inv.account_id != invoices[0].account_id + or inv.partner_bank_id != invoices[0].partner_bank_id for inv in invoices) currency = invoices[0].currency_id @@ -125,7 +127,7 @@ class account_abstract_payment(models.AbstractModel): """ Computes if the destination bank account must be displayed in the payment form view. By default, it won't be displayed but some modules might change that, depending on the payment type.""" for payment in self: - payment.show_partner_bank_account = payment.payment_method_code in self._get_method_codes_using_bank_account() + payment.show_partner_bank_account = payment.payment_method_code in self._get_method_codes_using_bank_account() and not self.multi @api.multi @api.depends('payment_type', 'journal_id') @@ -166,12 +168,19 @@ class account_abstract_payment(models.AbstractModel): @api.onchange('partner_id') def _onchange_partner_id(self): - if self.partner_id and len(self.partner_id.bank_ids) > 0: - self.partner_bank_account_id = self.partner_id.bank_ids[0] - elif self.partner_id and len(self.partner_id.commercial_partner_id.bank_ids) > 0: - self.partner_bank_account_id = self.partner_id.commercial_partner_id.bank_ids[0] - else: - self.partner_bank_account_id = False + if not self.multi and self.invoice_ids and self.invoice_ids[0].partner_bank_id: + self.partner_bank_account_id = self.invoice_ids[0].partner_bank_id + elif self.partner_id != self.partner_bank_account_id.partner_id: + # This condition ensures we use the default value provided into + # context for partner_bank_account_id properly when provided with a + # default partner_id. Without it, the onchange recomputes the bank account + # uselessly and might assign a different value to it. + if self.partner_id and len(self.partner_id.bank_ids) > 0: + self.partner_bank_account_id = self.partner_id.bank_ids[0] + elif self.partner_id and len(self.partner_id.commercial_partner_id.bank_ids) > 0: + self.partner_bank_account_id = self.partner_id.commercial_partner_id.bank_ids[0] + else: + self.partner_bank_account_id = False return {'domain': {'partner_bank_account_id': [('partner_id', 'in', [self.partner_id.id, self.partner_id.commercial_partner_id.id])]}} def _compute_journal_domain_and_types(self): @@ -242,6 +251,21 @@ class account_register_payments(models.TransientModel): _inherit = 'account.abstract.payment' _description = "Register payments on multiple invoices" + group_invoices = fields.Boolean(string="Group Invoices", help="""If enabled, groups invoices by commercial partner, invoice account, + type and recipient bank account in the generated payments. If disabled, + a distinct payment will be generated for each invoice.""") + show_communication_field = fields.Boolean(compute='_compute_show_communication_field') + + @api.depends('invoice_ids.partner_id', 'group_invoices') + def _compute_show_communication_field(self): + """ We allow choosing a common communication for payments if the group + option has been activated, and all the invoices relate to the same + partner. + """ + for record in self: + record.show_communication_field = len(record.invoice_ids) == 1 \ + or record.group_invoices and len(record.mapped('invoice_ids.partner_id.commercial_partner_id')) == 1 + @api.onchange('journal_id') def _onchange_journal(self): res = super(account_register_payments, self)._onchange_journal() @@ -262,18 +286,27 @@ class account_register_payments(models.TransientModel): @api.multi def _groupby_invoices(self): - '''Split the invoices linked to the wizard according to their commercial partner, - their account and their type. + '''Groups the invoices linked to the wizard. - :return: a dictionary mapping (partner_id, account_id, invoice_type) => invoices recordset. + If the group_invoices option is activated, invoices will be grouped + according to their commercial partner, their account, their type and + the account where the payment they expect should end up. Otherwise, + invoices will be grouped so that each of them belongs to a + distinct group. + + :return: a dictionary mapping, grouping invoices as a recordset under each of its keys. ''' + if not self.group_invoices: + return {inv.id: inv for inv in self.invoice_ids} + results = {} - # Create a dict dispatching invoices according to their commercial_partner_id and type + # Create a dict dispatching invoices according to their commercial_partner_id, account_id, invoice_type and partner_bank_id for inv in self.invoice_ids: partner_id = inv.commercial_partner_id.id account_id = inv.account_id.id invoice_type = MAP_INVOICE_TYPE_PARTNER_TYPE[inv.type] - key = (partner_id, account_id, invoice_type) + recipient_account = inv.partner_bank_id + key = (partner_id, account_id, invoice_type, recipient_account) if not key in results: results[key] = self.env['account.invoice'] results[key] += inv @@ -288,18 +321,22 @@ class account_register_payments(models.TransientModel): ''' amount = self._compute_payment_amount(invoices=invoices) if self.multi else self.amount payment_type = ('inbound' if amount > 0 else 'outbound') if self.multi else self.payment_type + bank_account = self.multi and invoices[0].partner_bank_id or self.partner_bank_account_id + pmt_communication = self.show_communication_field and self.communication \ + or self.group_invoices and ' '.join([inv.reference or inv.number for inv in invoices]) \ + or invoices[0].reference # in this case, invoices contains only one element, since group_invoices is False return { 'journal_id': self.journal_id.id, 'payment_method_id': self.payment_method_id.id, 'payment_date': self.payment_date, - 'communication': self.communication, + 'communication': pmt_communication, 'invoice_ids': [(6, 0, invoices.ids)], 'payment_type': payment_type, 'amount': abs(amount), 'currency_id': self.currency_id.id, 'partner_id': invoices[0].commercial_partner_id.id, 'partner_type': MAP_INVOICE_TYPE_PARTNER_TYPE[invoices[0].type], - 'partner_bank_account_id': self.partner_bank_account_id.id, + 'partner_bank_account_id': bank_account.id, 'multi': False, } @@ -475,6 +512,18 @@ class account_payment(models.Model): rec['amount'] = invoice['residual'] return rec + @api.model + def create(self, vals): + rslt = super(account_payment, self).create(vals) + # When a payment is created by the multi payments wizard in 'multi' mode, + # its partner_bank_account_id will never be displayed, and hence stay empty, + # even if the payment method requires it. This condition ensures we set + # the first (and thus most prioritary) account of the partner in this field + # in that situation. + if not rslt.partner_bank_account_id and rslt.show_partner_bank_account and rslt.partner_id.bank_ids: + rslt.partner_bank_account_id = rslt.partner_id.bank_ids[0] + return rslt + @api.multi def button_journal_entries(self): return { @@ -761,3 +810,23 @@ class account_payment(models.Model): }) return vals + + @api.model + def _sanitize_communication(self, communication): + """ Returns a sanitized version of the communication given in parameter, + so that: + - it contains only latin characters + - it does not contain any // + - it does not start or end with / + - it is maximum 140 characters long + (these are the SEPA compliance criteria) + """ + communication = communication[:140] + while '//' in communication: + communication = communication.replace('//', '/') + if communication.startswith('/'): + communication = communication[1:] + if communication.endswith('/'): + communication = communication[:-1] + communication = re.sub('[^-A-Za-z0-9/?:().,\'+ ]', '', communication) + return communication diff --git a/addons/account/models/company.py b/addons/account/models/company.py index b659d0ad00e..2122d9b1f4d 100644 --- a/addons/account/models/company.py +++ b/addons/account/models/company.py @@ -65,6 +65,7 @@ Best Regards,''')) help='International Commercial Terms are a series of predefined commercial terms used in international transactions.') invoice_reference_type = fields.Selection(string='Default Communication Type', selection='_get_invoice_reference_types', default='none', help='You can set here the default communication that will appear on customer invoices, once validated, to help the customer to refer to that particular invoice when making the payment.') + account_sanitize_invoice_ref = fields.Boolean(string="Sanitize Invoice References", default=True, help="Whether or not customer invoices and vendor bills should automatically correct their reference they are maximum 140 characters long, consist only of latin characters, contain no '//' sequence, and have no leading or trailing /. (these are the SEPA criteria for payment communications)") #Fields of the setup step for opening move account_opening_move_id = fields.Many2one(string='Opening Journal Entry', comodel_name='account.move', help="The journal entry containing the initial balance of all this company's accounts.") diff --git a/addons/account/models/res_config_settings.py b/addons/account/models/res_config_settings.py index d9e08ca9bbf..5a4b7f19e03 100644 --- a/addons/account/models/res_config_settings.py +++ b/addons/account/models/res_config_settings.py @@ -81,6 +81,7 @@ class ResConfigSettings(models.TransientModel): help="""The bank reconciliation widget won't ask to reconcile payments older than this date. This is useful if you install accounting after having used invoicing for some time and don't want to reconcile all the past payments with bank statements.""") + account_sanitize_invoice_ref = fields.Boolean(string="Sanitize Invoice References", related='company_id.account_sanitize_invoice_ref', help="If checked, customer invoices' and vendor bills' referneces will automatically correct their reference so that they are maximum 140 characters long, consist only of latin characters, contain no '//' sequence, and have no leading or trailing /.") @api.multi def set_values(self): diff --git a/addons/account/tests/test_payment.py b/addons/account/tests/test_payment.py index bf547de6bf6..9c86d9ba56c 100644 --- a/addons/account/tests/test_payment.py +++ b/addons/account/tests/test_payment.py @@ -168,6 +168,7 @@ class TestPayment(AccountingTestCase): 'payment_date': time.strftime('%Y') + '-07-15', 'journal_id': self.bank_journal_euro.id, 'payment_method_id': self.payment_method_manual_in.id, + 'group_invoices': True, }) register_payments.create_payments() payment_ids = self.payment_model.search([('invoice_ids', 'in', ids)], order="id desc") @@ -214,31 +215,6 @@ class TestPayment(AccountingTestCase): ]) self.assertEqual(inv_4.state, 'paid') - def test_multiple_payments_01(self): - """ Create test to pay several invoices/refunds at once """ - # One payment for inv_1 and inv_2 (same partner) but inv_2 is refund - inv_1 = self.create_invoice(amount=550) - inv_2 = self.create_invoice(amount=100, type='out_refund') - - ids = [inv_1.id, inv_2.id] - register_payments = self.register_payments_model.with_context(active_ids=ids).create({ - 'payment_date': time.strftime('%Y') + '-07-15', - 'journal_id': self.bank_journal_euro.id, - 'payment_method_id': self.payment_method_manual_in.id, - }) - register_payments.create_payments() - payment_id = self.payment_model.search([('invoice_ids', 'in', ids)], order="id desc") - - self.assertEqual(len(payment_id), 1) - self.assertAlmostEquals(register_payments.amount, 450) - - self.assertEqual(payment_id.state, 'posted') - - self.assertRecordValues(payment_id.move_line_ids, [ - {'account_id': self.account_eur.id, 'debit': 450.0, 'credit': 0.0, 'amount_currency': 0.0, 'currency_id': False}, - {'account_id': inv_1.account_id.id, 'debit': 0.0, 'credit': 450.0, 'amount_currency': 0.0, 'currency_id': False}, - ]) - def test_partial_payment(self): """ Create test to pay invoices (cust. inv + vendor bill) with partial payment """ # Test Customer Invoice @@ -309,6 +285,7 @@ class TestPayment(AccountingTestCase): 'payment_date': time.strftime('%Y') + '-07-15', 'journal_id': self.bank_journal_euro.id, 'payment_method_id': self.payment_method_manual_in.id, + 'group_invoices': True, }) register_payments.create_payments() payment_ids = self.payment_model.search([('invoice_ids', 'in', ids)], order="id desc") @@ -332,6 +309,49 @@ class TestPayment(AccountingTestCase): {'account_id': inv_3.account_id.id, 'debit': 0.0, 'credit': 300.0, 'amount_currency': 0.0, 'currency_id': False}, ]) + def test_register_payment_group_invoices(self): + """ Tests the 'group invoices' option of the payment registering wizard + """ + + account_receivable_id_1 = self.account_receivable.id + account_receivable_id_2 = self.account_receivable.copy(default={ + 'code': '%s (%s)' % (self.account_receivable.code, 'duplicate 1') + }).id + + inv_1 = self.create_invoice(amount=100, account_id=account_receivable_id_1) + inv_2 = self.create_invoice(amount=150, account_id=account_receivable_id_1) + inv_3 = self.create_invoice(amount=300, account_id=account_receivable_id_2) + inv_4 = self.create_invoice(amount=300, account_id=account_receivable_id_2, partner=self.partner_china_exp.id) + + inv_5 = self.create_invoice(amount=42, account_id=account_receivable_id_1) + inv_6 = self.create_invoice(amount=1111, account_id=account_receivable_id_1) + inv_7 = self.create_invoice(amount=666, account_id=account_receivable_id_2) + inv_8 = self.create_invoice(amount=300, account_id=account_receivable_id_2, partner=self.partner_china_exp.id) + + # When grouping invoices, we should have one payment per receivable account + ids1 = [inv_1.id, inv_2.id, inv_3.id, inv_4.id] + register_payments1 = self.register_payments_model.with_context(active_ids=ids1).create({ + 'payment_date': time.strftime('%Y') + '-07-15', + 'journal_id': self.bank_journal_euro.id, + 'payment_method_id': self.payment_method_manual_in.id, + 'group_invoices': True, + }) + register_payments1.create_payments() + payment_ids1 = self.payment_model.search([('invoice_ids', 'in', ids1)], order="id desc") + self.assertEqual(len(payment_ids1), 3, "3 payments should have been created, one fo each (partner, receivable account).") + + # When not grouping, we should have one payment per invoice + ids2 = [inv_5.id, inv_6.id, inv_7.id, inv_8.id] + register_payments2 = self.register_payments_model.with_context(active_ids=ids2).create({ + 'payment_date': time.strftime('%Y') + '-07-15', + 'journal_id': self.bank_journal_euro.id, + 'payment_method_id': self.payment_method_manual_in.id, + 'group_invoices': False, + }) + register_payments2.create_payments() + payment_ids2 = self.payment_model.search([('invoice_ids', 'in', ids2)], order="id desc") + self.assertEqual(len(payment_ids2), 4, "Not grouping payments should always create a distinct payment per invoice.") + def test_payment_and_writeoff_in_other_currency_1(self): # Use case: # Company is in EUR, create a customer invoice for 25 EUR and register payment of 25 USD. diff --git a/addons/account/views/account_invoice_view.xml b/addons/account/views/account_invoice_view.xml index 5dbd235cf28..5e908ce7339 100644 --- a/addons/account/views/account_invoice_view.xml +++ b/addons/account/views/account_invoice_view.xml @@ -299,6 +299,8 @@ + + @@ -366,7 +368,6 @@ - diff --git a/addons/account/views/account_payment_view.xml b/addons/account/views/account_payment_view.xml index 51c26e49583..318844bc247 100644 --- a/addons/account/views/account_payment_view.xml +++ b/addons/account/views/account_payment_view.xml @@ -335,7 +335,10 @@ - + + + +

Memo will be computed from invoices