From 69908089454b1e592177f93c667dffa8a5516c70 Mon Sep 17 00:00:00 2001 From: "Lucas Perais (lpe)" Date: Fri, 13 Sep 2019 10:08:59 +0000 Subject: [PATCH] [FIX] account: payment keep move names on cancel Reverts d91b1e5bd56373f869bdc0db2215818e9ce11cbc But still handles the use case (cancelling transfer payment) Make a payment, confirm it The payment's move has a name, i.e. constructed from its sequence Cancel it the payment has no move anymore Re-confirm the payment Before this commit, the new move had a new name from a new sequence number This is problematic to ensure accounting consistency Besides, it was not the way it works for invoices' moves After this commit, the former name of the move is taken to build the new move of the re-confirmed payment OPW 2046412 OPW 2070167 closes odoo/odoo#36868 closes odoo/odoo#36922 Signed-off-by: Lucas Perais (lpe) Signed-off-by: Lucas Perais (lpe) --- addons/account/models/account_payment.py | 27 +++++- addons/account/tests/test_payment.py | 118 +++++++++++++++++++++++ 2 files changed, 142 insertions(+), 3 deletions(-) diff --git a/addons/account/models/account_payment.py b/addons/account/models/account_payment.py index 45a3970552d..fc9d38beabc 100644 --- a/addons/account/models/account_payment.py +++ b/addons/account/models/account_payment.py @@ -323,6 +323,10 @@ class account_payment(models.Model): def name_get(self): return [(payment.id, payment.name or _('Draft Payment')) for payment in self] + @api.model + def _get_move_name_transfer_separator(self): + return 'ยงยง' + @api.multi @api.depends('move_line_ids.reconciled') def _get_move_reconciled(self): @@ -457,7 +461,6 @@ class account_payment(models.Model): move.unlink() rec.write({ 'state': 'cancelled', - 'move_name': False, }) @api.multi @@ -507,6 +510,7 @@ class account_payment(models.Model): # Create the journal entry amount = rec.amount * (rec.payment_type in ('outbound', 'transfer') and 1 or -1) move = rec._create_payment_entry(amount) + persist_move_name = move.name # In case of a transfer, the first journal entry created debited the source liquidity account and credited # the transfer account. Now we debit the transfer account and credit the destination liquidity account. @@ -514,8 +518,9 @@ class account_payment(models.Model): transfer_credit_aml = move.line_ids.filtered(lambda r: r.account_id == rec.company_id.transfer_account_id) transfer_debit_aml = rec._create_transfer_entry(amount) (transfer_credit_aml + transfer_debit_aml).reconcile() + persist_move_name += self._get_move_name_transfer_separator() + transfer_debit_aml.move_id.name - rec.write({'state': 'posted', 'move_name': move.name}) + rec.write({'state': 'posted', 'move_name': persist_move_name}) return True @api.multi @@ -608,14 +613,30 @@ class account_payment(models.Model): """ Return dict to create the payment move """ journal = journal or self.journal_id + move_vals = { 'date': self.payment_date, 'ref': self.communication or '', 'company_id': self.company_id.id, 'journal_id': journal.id, } + + name = False if self.move_name: - move_vals['name'] = self.move_name + names = self.move_name.split(self._get_move_name_transfer_separator()) + if self.payment_type == 'transfer': + if journal == self.destination_journal_id and len(names) == 2: + name = names[1] + elif journal == self.destination_journal_id and len(names) != 2: + # We are probably transforming a classical payment into a transfer + name = False + else: + name = names[0] + else: + name = names[0] + + if name: + move_vals['name'] = name return move_vals def _get_shared_move_line_vals(self, debit, credit, amount_currency, move_id, invoice_id=False): diff --git a/addons/account/tests/test_payment.py b/addons/account/tests/test_payment.py index 808393d89a5..4fd92d22207 100644 --- a/addons/account/tests/test_payment.py +++ b/addons/account/tests/test_payment.py @@ -35,6 +35,8 @@ class TestPayment(AccountingTestCase): self.bank_journal_euro = self.env['account.journal'].create({'name': 'Bank', 'type': 'bank', 'code': 'BNK67'}) self.account_eur = self.bank_journal_euro.default_debit_account_id + self.cash_journal_euro = self.env['account.journal'].create({'name': 'Cash', 'type': 'cash', 'code': 'CASH'}) + self.bank_journal_usd = self.env['account.journal'].create({'name': 'Bank US', 'type': 'bank', 'code': 'BNK68', 'currency_id': self.currency_usd_id}) self.account_usd = self.bank_journal_usd.default_debit_account_id @@ -506,3 +508,119 @@ class TestPayment(AccountingTestCase): # The invoice should now be paid self.assertEqual(invoice.state, 'paid', "Invoice should be in 'paid' state after having reconciled the two payments with a bank statement") + + def test_payment_cancel_keep_name(self): + self.bank_journal_euro.update_posted = True + + payment = self.payment_model.create({ + 'payment_type': 'inbound', + 'payment_method_id': self.payment_method_manual_in.id, + 'partner_type': 'customer', + 'partner_id': self.partner_agrolait.id, + 'amount': 90, + 'payment_date': time.strftime('%Y') + '-07-15', + 'payment_difference_handling': 'reconcile', + 'journal_id': self.bank_journal_euro.id, + }) + + payment.post() + self.assertEqual(len(payment.move_line_ids.mapped('move_id')), 1) + name = payment.move_line_ids.mapped('move_id').name + self.assertTrue(name) + + payment.cancel() + self.assertFalse(payment.move_line_ids.mapped('move_id')) + payment.action_draft() + + payment.post() + self.assertEqual(len(payment.move_line_ids.mapped('move_id')), 1) + self.assertEqual(name, payment.move_line_ids.mapped('move_id').name) + + def test_payment_transfer_cancel_keep_names(self): + self.bank_journal_euro.update_posted = True + self.cash_journal_euro.update_posted = True + + payment = self.payment_model.create({ + 'payment_type': 'transfer', + 'payment_method_id': self.payment_method_manual_out.id, + 'amount': 90, + 'payment_date': time.strftime('%Y') + '-07-15', + 'journal_id': self.bank_journal_euro.id, + 'destination_journal_id': self.cash_journal_euro.id, + }) + + payment.post() + self.assertEqual(len(payment.move_line_ids.mapped('move_id')), 2) + + all_moves = payment.move_line_ids.mapped('move_id') + move = all_moves.filtered(lambda m: m.journal_id == self.bank_journal_euro) + transfer_move = all_moves - move + self.assertEqual(transfer_move.journal_id, self.cash_journal_euro) + + name = move.name + transfer_name = transfer_move.name + self.assertTrue(name) + self.assertTrue(transfer_name) + self.assertNotEqual(name, transfer_name) + + reconciled_lines = payment.move_line_ids.filtered(lambda l: l.reconciled) + self.assertEqual(len(reconciled_lines), 2) + self.assertEqual(reconciled_lines.mapped('move_id'), all_moves) + + reconciled_lines.remove_move_reconcile() + payment.cancel() + self.assertFalse(payment.move_line_ids.mapped('move_id')) + payment.action_draft() + + payment.post() + self.assertEqual(len(payment.move_line_ids.mapped('move_id')), 2) + + all_moves = payment.move_line_ids.mapped('move_id') + move = all_moves.filtered(lambda m: m.journal_id == self.bank_journal_euro) + transfer_move = all_moves - move + self.assertEqual(transfer_move.journal_id, self.cash_journal_euro) + + self.assertEqual(name, move.name) + self.assertEqual(transfer_name, transfer_move.name) + + def test_payment_cancel_to_transfer(self): + self.bank_journal_euro.update_posted = True + + payment = self.payment_model.create({ + 'payment_type': 'inbound', + 'payment_method_id': self.payment_method_manual_in.id, + 'partner_type': 'customer', + 'partner_id': self.partner_agrolait.id, + 'amount': 90, + 'payment_date': time.strftime('%Y') + '-07-15', + 'payment_difference_handling': 'reconcile', + 'journal_id': self.bank_journal_euro.id, + }) + + payment.post() + self.assertEqual(len(payment.move_line_ids.mapped('move_id')), 1) + name = payment.move_line_ids.mapped('move_id').name + self.assertTrue(name) + + payment.cancel() + self.assertFalse(payment.move_line_ids.mapped('move_id')) + payment.action_draft() + + payment.write({ + 'payment_type': 'transfer', + 'payment_method_id': self.payment_method_manual_out.id, + 'partner_id': False, + 'destination_journal_id': self.cash_journal_euro.id, + }) + + payment.post() + self.assertEqual(len(payment.move_line_ids.mapped('move_id')), 2) + + all_moves = payment.move_line_ids.mapped('move_id') + move = all_moves.filtered(lambda m: m.journal_id == self.bank_journal_euro) + transfer_move = all_moves - move + self.assertEqual(transfer_move.journal_id, self.cash_journal_euro) + + self.assertEqual(name, move.name) + self.assertTrue(transfer_move.name) + self.assertNotEqual(name, transfer_move.name)