From 43eae4d032a3740b2753d8717feaf69481afa291 Mon Sep 17 00:00:00 2001 From: PoMa Date: Mon, 14 Nov 2022 16:32:51 +0000 Subject: [PATCH] [FIX] account: bank statement not ignoring canceled and draft lines Before this commit the canceled lines would not filter out from checks for completeness of the statements and also the corrections in split wizard consider canceled lines as real values. One of the results of this is when the user import statements with some duplicate lines, the duplicates are canceled but the statement stays complete regardless of the missing lines. Also fixes the line date is not auto-filled when creating lines in the list view if they don't have a state Closes PR #105708 closes odoo/odoo#107894 X-original-commit: 2422b2c619209c1ff4171902dd2f90824194366d Signed-off-by: Laurent Smet Signed-off-by: Pouya Malekinejad (poma) --- .../account/models/account_bank_statement.py | 22 ++-- .../models/account_bank_statement_line.py | 36 +++--- .../tests/test_account_bank_statement.py | 110 +++++++++++++++++- 3 files changed, 142 insertions(+), 26 deletions(-) diff --git a/addons/account/models/account_bank_statement.py b/addons/account/models/account_bank_statement.py index 1769f75a973..b12d3f2d372 100644 --- a/addons/account/models/account_bank_statement.py +++ b/addons/account/models/account_bank_statement.py @@ -16,7 +16,7 @@ class AccountBankStatement(models.Model): # create statement on a saved statement line in the tree view if self._context.get('st_line_id'): st_line = self.env['account.bank.statement.line'].browse(self._context['st_line_id']) - defaults['balance_start'] = st_line.running_balance - st_line.amount + defaults['balance_start'] = st_line.running_balance - (st_line.amount if st_line.state == 'posted' else 0) return defaults # create statement from a new line in the tree view, not stored in the db yet if self._context.get('st_line_date'): @@ -61,7 +61,9 @@ class AccountBankStatement(models.Model): raise UserError(_('One or more selected lines already belong to a complete statement.')) if lines: defaults['line_ids'] = [Command.set(lines.ids)] - defaults['balance_start'] = lines[-1:].running_balance - lines[-1:].amount + defaults['balance_start'] = lines[-1:].running_balance - ( + lines[-1:].amount if lines[-1:].state == 'posted' else 0 + ) defaults['balance_end_real'] = lines[:1].running_balance return defaults @@ -154,17 +156,19 @@ class AccountBankStatement(models.Model): # ------------------------------------------------------------------------- # COMPUTE METHODS # ------------------------------------------------------------------------- - @api.depends('line_ids.internal_index') + @api.depends('line_ids.internal_index', 'line_ids.state') def _compute_date_index(self): for stmt in self: sorted_lines = stmt.line_ids.sorted('internal_index') - stmt.date = sorted_lines[-1:].date stmt.first_line_index = sorted_lines[:1].internal_index + stmt.date = sorted_lines.filtered(lambda l: l.state == 'posted')[-1:].date - @api.depends('balance_start', 'line_ids.amount') + @api.depends('balance_start', 'line_ids.amount', 'line_ids.state') def _compute_balance_end(self): for statement in self: - statement.balance_end = statement.balance_start + sum(statement.line_ids.mapped('amount')) + statement.balance_end = statement.balance_start + sum( + statement.line_ids.filtered(lambda l: l.state == 'posted').mapped('amount') + ) @api.depends('journal_id') def _compute_currency_id(self): @@ -179,7 +183,7 @@ class AccountBankStatement(models.Model): @api.depends('balance_end_real', 'balance_end') def _compute_is_complete(self): for stmt in self: - stmt.is_complete = stmt.line_ids and stmt.currency_id.compare_amounts( + stmt.is_complete = stmt.line_ids.filtered(lambda l: l.state == 'posted') and stmt.currency_id.compare_amounts( stmt.balance_end, stmt.balance_end_real) == 0 def _compute_is_valid(self): @@ -205,7 +209,7 @@ class AccountBankStatement(models.Model): def create(self, vals_list): # EXTENDS base # If we are doing a split, we have to correct the split statement's balance to keep both original and new - # statements complete and valid. + # statements valid. if self._context.get('split_line_id'): old_statement = self.env['account.bank.statement.line'].browse(self._context.get('split_line_id')).statement_id old_lines = old_statement.line_ids @@ -213,7 +217,7 @@ class AccountBankStatement(models.Model): if self._context.get('split_line_id'): statements.ensure_one() if old_statement: - net_change = sum((statements.line_ids & old_lines).mapped('amount')) + net_change = sum((statements.line_ids & old_lines).filtered(lambda l: l.state == 'posted').mapped('amount')) old_statement.balance_start += net_change return statements diff --git a/addons/account/models/account_bank_statement_line.py b/addons/account/models/account_bank_statement_line.py index 446af9d130f..37463feb914 100644 --- a/addons/account/models/account_bank_statement_line.py +++ b/addons/account/models/account_bank_statement_line.py @@ -28,16 +28,16 @@ class AccountBankStatementLine(models.Model): # statement is completed. It is only possible if we know the journal that is used, so it can only be done # in a view in which the journal is already set and so is single journal view. if'journal_id' in defaults: - last_line = self.search([('journal_id', '=', defaults.get('journal_id'))], limit=1) + last_line = self.search([ + ('journal_id', '=', defaults.get('journal_id')), + ('state', '=', 'posted'), + ], limit=1) statement = last_line.statement_id if statement and not statement.is_complete: - defaults.setdefault( - 'statement_id', statement.id - ) - if statement.date: - defaults.setdefault( - 'date', statement.date - ) + defaults.setdefault('statement_id', statement.id) + defaults.setdefault('date', statement.date) + elif last_line and not statement: + defaults.setdefault('date', last_line.date) return defaults @@ -155,15 +155,19 @@ class AccountBankStatementLine(models.Model): # that the running balance is always relative to the latest statement. In this way we do not need to calculate # the running balance for all statement lines every time. # If there are statements inside the computed range, their balance_start has priority over calculated balance. + # we have to compute running balance for draft lines because they are visible and also + # the user can split on that lines, but their balance should be the same as previous posted line + # we do the same for the canceled lines, in order to keep using them as anchor points self.statement_id.flush_model(['balance_start', 'first_line_index']) - self.flush_model(['internal_index', 'date', 'journal_id', 'statement_id', 'amount']) + self.flush_model(['internal_index', 'date', 'journal_id', 'statement_id', 'amount', 'state']) record_by_id = {x.id: x for x in self} for journal in self.journal_id: - journal_lines = self.filtered(lambda line: line.journal_id == journal).sorted('internal_index') - max_index = max(journal_lines.mapped('internal_index')) - min_index = min(journal_lines.mapped('internal_index')) + journal_lines_indexes = self.filtered(lambda line: line.journal_id == journal)\ + .sorted('internal_index')\ + .mapped('internal_index') + min_index, max_index = journal_lines_indexes[0], journal_lines_indexes[-1] # Find the oldest index for each journal. self._cr.execute( @@ -193,7 +197,8 @@ class AccountBankStatementLine(models.Model): st_line.id, st_line.amount, st.first_line_index = st_line.internal_index AS is_anchor, - st.balance_start + st.balance_start, + move.state FROM account_bank_statement_line st_line JOIN account_move move ON move.statement_line_id = st_line.id LEFT JOIN account_bank_statement st ON st.id = st_line.statement_id @@ -205,10 +210,11 @@ class AccountBankStatementLine(models.Model): """, [max_index, journal.id] + extra_params, ) - for st_line_id, amount, is_anchor, balance_start in self._cr.fetchall(): + for st_line_id, amount, is_anchor, balance_start, state in self._cr.fetchall(): if is_anchor: current_running_balance = balance_start - current_running_balance += amount + if state == 'posted': + current_running_balance += amount if record_by_id.get(st_line_id): record_by_id[st_line_id].running_balance = current_running_balance diff --git a/addons/account/tests/test_account_bank_statement.py b/addons/account/tests/test_account_bank_statement.py index 8b23c028d00..0ff74f9cac1 100644 --- a/addons/account/tests/test_account_bank_statement.py +++ b/addons/account/tests/test_account_bank_statement.py @@ -716,10 +716,10 @@ class TestAccountBankStatementLine(AccountTestInvoicingCommon): line7 = self.create_bank_transaction(7, '2020-01-10', sequence=1) line8 = self.create_bank_transaction(8, '2020-01-10', sequence=2) line2 = self.create_bank_transaction(2, '2020-01-13') - _line6 = self.create_bank_transaction(6, '2020-01-11') + line6 = self.create_bank_transaction(6, '2020-01-11') line5 = self.create_bank_transaction(5, '2020-01-12', sequence=3) line4 = self.create_bank_transaction(4, '2020-01-12', sequence=2) - _line1 = self.create_bank_transaction(1, '2020-01-13') + line1 = self.create_bank_transaction(1, '2020-01-13') line3 = self.create_bank_transaction(3, '2020-01-12', sequence=1) self.assertRecordValues( @@ -851,6 +851,40 @@ class TestAccountBankStatementLine(AccountTestInvoicingCommon): {'amount': 18, 'running_balance': 18, 'statement_id': False}, ], ) + line1.move_id.button_cancel() + line6.move_id.button_cancel() + line6.move_id.button_draft() + self.env['account.bank.statement.line'].invalidate_model(fnames=['running_balance']) + self.assertRecordValues( + self.env['account.bank.statement.line'].search([('company_id', '=', self.env.company.id)]), + [ + # pylint: disable=C0326 + {'amount': 1, 'running_balance': 45, 'statement_id': False}, + {'amount': 2, 'running_balance': 45, 'statement_id': statement1.id}, + {'amount': 15, 'running_balance': 43, 'statement_id': False}, + {'amount': 3, 'running_balance': 28, 'statement_id': statement1.id}, + {'amount': 4, 'running_balance': 25, 'statement_id': statement1.id}, + {'amount': 6, 'running_balance': 18, 'statement_id': False}, + {'amount': 18, 'running_balance': 18, 'statement_id': False}, + ], + ) + + # remove the anchor point + statement1.line_ids = False + self.env['account.bank.statement.line'].invalidate_model(fnames=['running_balance']) + self.assertRecordValues( + self.env['account.bank.statement.line'].search([('company_id', '=', self.env.company.id)]), + [ + # pylint: disable=C0326 + {'amount': 1, 'running_balance': 42, 'statement_id': False}, + {'amount': 2, 'running_balance': 42, 'statement_id': False}, + {'amount': 15, 'running_balance': 40, 'statement_id': False}, + {'amount': 3, 'running_balance': 25, 'statement_id': False}, + {'amount': 4, 'running_balance': 22, 'statement_id': False}, + {'amount': 6, 'running_balance': 18, 'statement_id': False}, + {'amount': 18, 'running_balance': 18, 'statement_id': False}, + ], + ) def test_statement_split(self): self.env.user.company_id = self.company_data_2['company'] @@ -1064,3 +1098,75 @@ class TestAccountBankStatementLine(AccountTestInvoicingCommon): self.assertEqual(len(all_statements), 6) self.assertEqual(all_statements.mapped('is_valid'), [True] * len(all_statements)) self.assertEqual(all_statements.mapped('is_complete'), [True] * len(all_statements)) + + def test_statement_with_canceled_lines(self): + line1 = self.create_bank_transaction(1, '2020-01-10') + line2 = self.create_bank_transaction(2, '2020-01-11') + statement1 = self.env['account.bank.statement'].create({ + 'line_ids': [Command.set((line1 + line2).ids)], + 'balance_start': 0, + 'balance_end_real': 3, + }) + self.assertRecordValues(statement1, [{ + 'is_complete': True, + 'date': fields.Date.from_string(line2.date), + }]) + # test canceling a line + line2.move_id.button_cancel() + self.assertRecordValues(statement1, [{ + 'is_complete': False, + 'balance_end': 1, + 'date': fields.Date.from_string(line1.date), + }]) + # test add a line with same amount as a canceled line makes statement complete again + line3 = self.create_bank_transaction(2, '2020-01-12', statement=statement1) + self.assertRecordValues(statement1, [{ + 'is_complete': True, + 'balance_end': 3, + 'date': fields.Date.from_string(line3.date), + }]) + # test adding a draft line to a statement, nothing should be changed in statement + line4 = self.create_bank_transaction(4, '2020-01-13') + line4.move_id.button_cancel() + line4.move_id.button_draft() + statement1.line_ids |= line4 + self.assertRecordValues(statement1, [{ + 'is_complete': True, + 'balance_end': 3, + 'date': fields.Date.from_string(line3.date), + }]) + # test split with canceled/draft lines + statement2 = self.env['account.bank.statement'].with_context({'split_line_id': line2.id}).create({}) + self.assertRecordValues(statement1 + statement2, [{ + 'is_complete': True, + 'balance_end': 3, + 'balance_start': 1, + 'line_ids': [line4.id, line3.id], + }, { + 'is_complete': True, + 'balance_start': 0, + 'balance_end': 1, + 'line_ids': [line1.id, line2.id], + }]) + + # test cancel/draft all statement lines + # line 4 is draft, and we cancel line 3 so the statement should be empty + line3.move_id.button_cancel() + self.assertRecordValues(statement1, [{ + 'is_complete': False, + 'balance_start': 1, + 'balance_end': 1, + }]) + # create a statement line with already canceled lines + statement3 = self.env['account.bank.statement'].create({ + 'line_ids': [Command.set((line3 + line4).ids)], + }) + self.assertRecordValues(statement1 + statement3, [{ + 'is_complete': False, + 'balance_start': 1, + 'balance_end': 1, + }, { + 'is_complete': False, + 'balance_start': 0, # no value is given for balance start and it is not a split + 'balance_end': 0, + }])