From ea00318e9ef6fa6a38b8edffd5a61f69894e6e93 Mon Sep 17 00:00:00 2001 From: "Lucas Perais (lpe)" Date: Thu, 21 Mar 2019 15:39:32 +0000 Subject: [PATCH 1/4] [FIX] account: revert move subjected to cash basis Before this commit, when reverting a move that will be subjected to the creation of a cash basis move the process failed saying that some entries were already reconciled That was because the process tried to create the cash basis move during the revert. This is wrong, because no real cash is dealt with. After this fix, the move lines of the original entry are reconciled only with their revert counterpart OPW 1938809 closes #30972 --- addons/account/models/account_move.py | 3 +- addons/account/tests/test_reconciliation.py | 63 +++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py index 3a16deb625b..d57dbce741a 100644 --- a/addons/account/models/account_move.py +++ b/addons/account/models/account_move.py @@ -887,7 +887,8 @@ class AccountMoveLine(models.Model): for after_rec_dict in cash_basis_subjected: new_rec = part_rec.create(after_rec_dict) - if cash_basis: + # if the pair belongs to move being reverted, do not create CABA entry + if cash_basis and not (new_rec.debit_move_id + new_rec.credit_move_id).mapped('move_id').mapped('reverse_entry_id'): new_rec.create_tax_cash_basis_entry(cash_basis_percentage_before_rec) self.recompute() diff --git a/addons/account/tests/test_reconciliation.py b/addons/account/tests/test_reconciliation.py index e9be12bcefb..4ca4a94404c 100644 --- a/addons/account/tests/test_reconciliation.py +++ b/addons/account/tests/test_reconciliation.py @@ -1876,3 +1876,66 @@ class TestReconciliation(AccountingTestCase): (move_lines - base_amount_tax_lines) .filtered(lambda l: l.account_id == self.tax_final_account) .debit, 17094.66) + + def test_reconciliation_cash_basis_revert(self): + company = self.env.ref('base.main_company') + company.tax_cash_basis_journal_id = self.cash_basis_journal + tax_cash_basis10percent = self.tax_cash_basis.copy({'amount': 10}) + self.tax_waiting_account.reconcile = True + tax_waiting_account10 = self.tax_waiting_account.copy({ + 'name': 'TAX WAIT 10', + 'code': 'TWAIT1', + }) + + AccountMoveLine = self.env['account.move.line'].with_context(check_move_validity=False) + + # Purchase + purchase_move = self.env['account.move'].create({ + 'name': 'invoice', + 'journal_id': self.purchase_journal.id, + }) + + purchase_payable_line0 = AccountMoveLine.create({ + 'account_id': self.account_rsa.id, + 'credit': 175, + 'move_id': purchase_move.id, + }) + + AccountMoveLine.create({ + 'name': 'expenseTaxed 10%', + 'account_id': self.expense_account.id, + 'debit': 50, + 'move_id': purchase_move.id, + 'tax_ids': [(4, tax_cash_basis10percent.id, False)], + }) + tax_line0 = AccountMoveLine.create({ + 'name': 'TaxLine0', + 'account_id': tax_waiting_account10.id, + 'debit': 5, + 'move_id': purchase_move.id, + 'tax_line_id': tax_cash_basis10percent.id, + }) + AccountMoveLine.create({ + 'name': 'expenseTaxed 20%', + 'account_id': self.expense_account.id, + 'debit': 100, + 'move_id': purchase_move.id, + 'tax_ids': [(4, self.tax_cash_basis.id, False)], + }) + tax_line1 = AccountMoveLine.create({ + 'name': 'TaxLine1', + 'account_id': self.tax_waiting_account.id, + 'debit': 20, + 'move_id': purchase_move.id, + 'tax_line_id': self.tax_cash_basis.id, + }) + purchase_move.post() + + reverted = self.env['account.move'].browse(purchase_move.reverse_moves()) + self.assertTrue(reverted.exists()) + + for inv_line in [purchase_payable_line0, tax_line0, tax_line1]: + self.assertTrue(inv_line.full_reconcile_id.exists()) + reverted_expected = reverted.line_ids.filtered(lambda l: l.account_id == inv_line.account_id) + self.assertEqual(len(reverted_expected), 1) + self.assertEqual(reverted_expected.full_reconcile_id, inv_line.full_reconcile_id) From 30f7c0ea0e6cfaaadbe3ba4e4237895253bc8f20 Mon Sep 17 00:00:00 2001 From: "Lucas Perais (lpe)" Date: Mon, 25 Mar 2019 08:54:03 +0000 Subject: [PATCH 2/4] [FIX] account: tests reconciliation helper class Before this commit, test reconciliation was inherited by test reconciliation widget. It made the actual test_xx functions of the former being executed twice After this commit, we create an intermediary test class for setUp and helpers And the tests for each class execute only once --- addons/account/tests/test_reconciliation.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/addons/account/tests/test_reconciliation.py b/addons/account/tests/test_reconciliation.py index 4ca4a94404c..ba4c148fc66 100644 --- a/addons/account/tests/test_reconciliation.py +++ b/addons/account/tests/test_reconciliation.py @@ -5,7 +5,8 @@ import time import unittest -@tagged('post_install', '-at_install') +# TODO in master +# The name of this class should be TestReconciliationHelpers class TestReconciliation(AccountingTestCase): """Tests for reconciliation (account.tax) @@ -166,6 +167,10 @@ class TestReconciliation(AccountingTestCase): supplier_move_lines = bank_stmt.move_line_ids return customer_move_lines, supplier_move_lines + +@tagged('post_install', '-at_install') +class TestReconciliationExec(TestReconciliation): + def test_statement_usd_invoice_eur_transaction_eur(self): customer_move_lines, supplier_move_lines = self.make_customer_and_supplier_flows(self.currency_euro_id, 30, self.bank_journal_usd, 42, 30, self.currency_euro_id) self.assertRecordValues(customer_move_lines, [ From c43acc34b873dc70cf792675fe1d58d8b0e116b1 Mon Sep 17 00:00:00 2001 From: "Lucas Perais (lpe)" Date: Mon, 25 Mar 2019 09:43:31 +0000 Subject: [PATCH 3/4] [FIX] account: cash basis reconciliation almost all amount In multicurrency Do an invoice with cash basis lines in the foreign currency Make a payment for almost all the invoice "almost" refers to the point where virtually all taxes will be paid i.e., paying 90 over 100, that contains a tax of 5% The tax paid will be an amount almost equal (to a few cents) to the total amount of the tax It is not negligible, but if the currency rates are in the right configuration (i.e. 0.005888) Before this commit: the Cash Basis reconciliation will be considered as full and will trigger the creation of the Exchange Diff Entry. In turn, paying the rest of the invoice will pop up an error saying that some entries are already reconciled (with the Exchange Diff entry) It is not *that* that an exchange rate entry has been created the first time after all, a percentage sufficiently close to 100 has been paid But it blocks subsequent reconciliation, hence this fix After this commit, if the cash basis entry doesn't match at least 100% of the paid move then, we force to not check for full reconciliation OPW 1953027 closes #31168 closes odoo/odoo#32023 Signed-off-by: Lucas Perais (lpe) --- addons/account/models/account_move.py | 19 ++++++- addons/account/tests/test_reconciliation.py | 55 +++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py index d57dbce741a..3ebb1702d0e 100644 --- a/addons/account/models/account_move.py +++ b/addons/account/models/account_move.py @@ -770,7 +770,9 @@ class AccountMoveLine(models.Model): total_amount_currency = 0 maxdate = date.min to_balance = {} + cash_basis_partial = self.env['account.partial.reconcile'] for aml in amls: + cash_basis_partial |= aml.move_id.tax_cash_basis_rec_id total_debit += aml.debit total_credit += aml.credit maxdate = max(aml.date, maxdate) @@ -785,15 +787,28 @@ class AccountMoveLine(models.Model): to_balance[aml.currency_id] = [self.env['account.move.line'], 0] to_balance[aml.currency_id][0] += aml to_balance[aml.currency_id][1] += aml.amount_residual != 0 and aml.amount_residual or aml.amount_residual_currency + # Check if reconciliation is total # To check if reconciliation is total we have 3 differents use case: # 1) There are multiple currency different than company currency, in that case we check using debit-credit # 2) We only have one currency which is different than company currency, in that case we check using amount_currency # 3) We have only one currency and some entries that don't have a secundary currency, in that case we check debit-credit # or amount_currency. + # 4) Cash basis full reconciliation + # - either none of the moves are cash basis reconciled, and we proceed + # - or some moves are cash basis reconciled and we make sure they are all fully reconciled + digits_rounding_precision = amls[0].company_id.currency_id.rounding - if (currency and float_is_zero(total_amount_currency, precision_rounding=currency.rounding)) or \ - (multiple_currency and float_compare(total_debit, total_credit, precision_rounding=digits_rounding_precision) == 0): + if ( + ( + not cash_basis_partial or (cash_basis_partial and all([p >= 1.0 for p in amls._get_matched_percentage().values()])) + ) and + ( + currency and float_is_zero(total_amount_currency, precision_rounding=currency.rounding) or + multiple_currency and float_compare(total_debit, total_credit, precision_rounding=digits_rounding_precision) == 0 + ) + ): + exchange_move_id = False # Eventually create a journal entry to book the difference due to foreign currency's exchange rate that fluctuates if to_balance and any([not float_is_zero(residual, precision_rounding=digits_rounding_precision) for aml, residual in to_balance.values()]): diff --git a/addons/account/tests/test_reconciliation.py b/addons/account/tests/test_reconciliation.py index ba4c148fc66..0226a0f1e94 100644 --- a/addons/account/tests/test_reconciliation.py +++ b/addons/account/tests/test_reconciliation.py @@ -1944,3 +1944,58 @@ class TestReconciliationExec(TestReconciliation): reverted_expected = reverted.line_ids.filtered(lambda l: l.account_id == inv_line.account_id) self.assertEqual(len(reverted_expected), 1) self.assertEqual(reverted_expected.full_reconcile_id, inv_line.full_reconcile_id) + + def test_reconciliation_cash_basis_foreign_currency_low_values(self): + journal = self.env['account.journal'].create({ + 'name': 'Bank', 'type': 'bank', 'code': 'THE', + 'currency_id': self.currency_usd_id, + }) + usd = self.env['res.currency'].browse(self.currency_usd_id) + usd.rate_ids.unlink() + self.env['res.currency.rate'].create({ + 'name': time.strftime('%Y-01-01'), + 'rate': 1/17.0, + 'currency_id': self.currency_usd_id, + 'company_id': self.env.ref('base.main_company').id, + }) + invoice = self.create_invoice( + type='out_invoice', invoice_amount=50, + currency_id=self.currency_usd_id) + invoice.journal_id.update_posted = True + invoice.action_cancel() + invoice.state = 'draft' + invoice.invoice_line_ids.write({ + 'invoice_line_tax_ids': [(6, 0, [self.tax_cash_basis.id])]}) + invoice.compute_taxes() + invoice.action_invoice_open() + + self.assertTrue(invoice.currency_id != self.env.user.company_id.currency_id) + + # First Payment + payment0 = self.make_payment(invoice, journal, invoice.amount_total - 0.01) + self.assertEqual(invoice.residual, 0.01) + + tax_waiting_line = invoice.move_id.line_ids.filtered(lambda l: l.account_id == self.tax_waiting_account) + self.assertFalse(tax_waiting_line.reconciled) + + move_caba0 = tax_waiting_line.matched_debit_ids.debit_move_id.move_id + self.assertTrue(move_caba0.exists()) + self.assertEqual(move_caba0.journal_id, self.env.user.company_id.tax_cash_basis_journal_id) + + pay_receivable_line0 = payment0.move_line_ids.filtered(lambda l: l.account_id == self.account_rcv) + self.assertTrue(pay_receivable_line0.reconciled) + self.assertEqual(pay_receivable_line0.matched_debit_ids, move_caba0.tax_cash_basis_rec_id) + + # Second Payment + payment1 = self.make_payment(invoice, journal, 0.01) + self.assertEqual(invoice.residual, 0) + self.assertEqual(invoice.state, 'paid') + + self.assertTrue(tax_waiting_line.reconciled) + move_caba1 = tax_waiting_line.matched_debit_ids.mapped('debit_move_id').mapped('move_id').filtered(lambda m: m != move_caba0) + self.assertEqual(len(move_caba1.exists()), 1) + self.assertEqual(move_caba1.journal_id, self.env.user.company_id.tax_cash_basis_journal_id) + + pay_receivable_line1 = payment1.move_line_ids.filtered(lambda l: l.account_id == self.account_rcv) + self.assertTrue(pay_receivable_line1.reconciled) + self.assertEqual(pay_receivable_line1.matched_debit_ids, move_caba1.tax_cash_basis_rec_id) From 81215120afffe54b17be3f38bbc2ac292452c0c4 Mon Sep 17 00:00:00 2001 From: Nans Lefebvre Date: Wed, 10 Apr 2019 07:55:59 +0000 Subject: [PATCH 4/4] Revert "[FIX] mail: remove attachment as main at unlink" This reverts commit abc45b1 Since by default the ondelete attribute of a many2one is `set null`, this was completely unnecessary to begin with. Bug caused by this commit: Unlink a record that has some attachments. The unlink first removes the record, then its related attachments. It calls remove_as_main_attachment, which reads the attachment res_model and res_id. This triggers a check that the related record can be read. However the related record has already been removed, an exception is raised. It is thus impossible to unlink a record. Closes #32563 closes odoo/odoo#32572 Signed-off-by: Raphael Collet (rco) --- addons/mail/models/ir_attachment.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/addons/mail/models/ir_attachment.py b/addons/mail/models/ir_attachment.py index 527b22409d6..ee51ed8382e 100644 --- a/addons/mail/models/ir_attachment.py +++ b/addons/mail/models/ir_attachment.py @@ -15,19 +15,6 @@ class IrAttachment(models.Model): for record in self: record.register_as_main_attachment(force=False) - @api.multi - def unlink(self): - self.remove_as_main_attachment() - super(IrAttachment, self).unlink() - - @api.multi - def remove_as_main_attachment(self): - for attachment in self: - related_record = self.env[attachment.res_model].browse(attachment.res_id) - if related_record and hasattr(related_record, 'message_main_attachment_id'): - if related_record.message_main_attachment_id == attachment: - related_record.message_main_attachment_id = False - def register_as_main_attachment(self, force=True): """ Registers this attachment as the main one of the model it is attached to.