From 0d9b82f4f9aeef02fa0f615b0c8432d791990f25 Mon Sep 17 00:00:00 2001 From: Andrea Date: Tue, 4 Jun 2019 13:38:00 +0200 Subject: [PATCH] Code review + add coverage --- .../models/account_invoice.py | 21 +++---- .../models/account_spread.py | 63 +++++++++---------- .../models/account_spread_line.py | 8 +-- .../tests/test_account_invoice_spread.py | 22 +++++++ 4 files changed, 65 insertions(+), 49 deletions(-) diff --git a/account_spread_cost_revenue/models/account_invoice.py b/account_spread_cost_revenue/models/account_invoice.py index 48790cbd..53bba642 100644 --- a/account_spread_cost_revenue/models/account_invoice.py +++ b/account_spread_cost_revenue/models/account_invoice.py @@ -11,10 +11,9 @@ class AccountInvoice(models.Model): def action_move_create(self): """Invoked when validating the invoices.""" res = super().action_move_create() - for invoice in self: - spreads = invoice.invoice_line_ids.mapped('spread_id') - spreads.compute_spread_board() - spreads.reconcile_spread_moves() + spreads = self.mapped('invoice_line_ids.spread_id') + spreads.compute_spread_board() + spreads.reconcile_spread_moves() return res @api.multi @@ -37,18 +36,18 @@ class AccountInvoice(models.Model): """Cancel the spread lines and their related moves when the invoice is canceled.""" res = super().action_cancel() - for invoice_line in self.mapped('invoice_line_ids'): - moves = invoice_line.spread_id.line_ids.mapped('move_id') - moves.button_cancel() - moves.unlink() - invoice_line.spread_id.line_ids.unlink() + spread_lines = self.mapped('invoice_line_ids.spread_id.line_ids') + moves = spread_lines.mapped('move_id') + moves.button_cancel() + moves.unlink() + spread_lines.unlink() return res @api.model def _refund_cleanup_lines(self, lines): - result = super(AccountInvoice, self)._refund_cleanup_lines(lines) + result = super()._refund_cleanup_lines(lines) for i, line in enumerate(lines): - for name, field in line._fields.items(): + for name in line._fields.keys(): if name == 'spread_id': result[i][2][name] = False break diff --git a/account_spread_cost_revenue/models/account_spread.py b/account_spread_cost_revenue/models/account_spread.py index 05d0df57..46b80692 100644 --- a/account_spread_cost_revenue/models/account_spread.py +++ b/account_spread_cost_revenue/models/account_spread.py @@ -402,9 +402,8 @@ class AccountSpread(models.Model): """Checks whether the spread lines should be calculated. In case checks pass, invoke "def _compute_spread_board()" method. """ - for spread in self: - if spread.total_amount: - spread._compute_spread_board() + for spread in self.filtered(lambda s: s.total_amount): + spread._compute_spread_board() @api.multi def action_recalculate_spread(self): @@ -434,38 +433,36 @@ class AccountSpread(models.Model): @api.multi def _action_unlink_invoice_line(self): - for spread in self: - spread_mls = spread.line_ids.mapped('move_id.line_ids') - spread_mls.remove_move_reconcile() - spread._message_post_unlink_invoice_line() - spread.write({'invoice_line_ids': [(5, 0, 0)]}) + spread_mls = self.mapped('line_ids.move_id.line_ids') + spread_mls.remove_move_reconcile() + self._message_post_unlink_invoice_line() + self.write({'invoice_line_ids': [(5, 0, 0)]}) def _message_post_unlink_invoice_line(self): - self.ensure_one() - inv_link = '%s' % (self.invoice_id.id, _("Invoice")) - msg_body = _("Unlinked invoice line '%s' (view %s).") % ( - self.invoice_line_id.name, inv_link) - self.message_post(body=msg_body) - spread_link = '%s' % (self.id, _("Spread Board")) - msg_body = _("Unlinked '%s' (invoice line %s).") % ( - spread_link, self.invoice_line_id.name) - self.invoice_id.message_post(body=msg_body) + for spread in self: + invoice_id = spread.invoice_id.id + inv_link = '%s' % (invoice_id, _("Invoice")) + msg_body = _("Unlinked invoice line '%s' (view %s).") % ( + spread.invoice_line_id.name, inv_link) + spread.message_post(body=msg_body) + spread_link = '%s' % (spread.id, _("Spread")) + msg_body = _("Unlinked '%s' (invoice line %s).") % ( + spread_link, spread.invoice_line_id.name) + spread.invoice_id.message_post(body=msg_body) @api.multi def unlink(self): - for spread in self: - if spread.invoice_line_id: - raise UserError( - _('Cannot delete spread(s) that are linked ' - 'to an invoice line.')) - posted_line_ids = self.line_ids.filtered( - lambda x: x.move_id.state == 'posted') - if posted_line_ids: - raise ValidationError( - _('Cannot delete spread(s): there are ' - 'posted Journal Entries.')) + if self.filtered(lambda s: s.invoice_line_id): + raise UserError( + _('Cannot delete spread(s) that are linked ' + 'to an invoice line.')) + if self.mapped('line_ids.move_id').filtered( + lambda m: m.state == 'posted'): + raise ValidationError( + _('Cannot delete spread(s): there are ' + 'posted Journal Entries.')) return super().unlink() @api.multi @@ -528,10 +525,8 @@ class AccountSpread(models.Model): @api.multi def create_all_moves(self): - for spread in self: - for line in spread.line_ids: - if not line.move_id: - line.create_move() + for line in self.mapped('line_ids').filtered(lambda l: not l.move_id): + line.create_move() @api.depends( 'debit_account_id.deprecated', 'credit_account_id.deprecated') diff --git a/account_spread_cost_revenue/models/account_spread_line.py b/account_spread_cost_revenue/models/account_spread_line.py index f3a7650d..caba7743 100644 --- a/account_spread_cost_revenue/models/account_spread_line.py +++ b/account_spread_cost_revenue/models/account_spread_line.py @@ -61,12 +61,12 @@ class AccountInvoiceSpreadLine(models.Model): @api.multi def _create_moves(self): + if self.filtered(lambda l: l.move_id): + raise UserError(_('This spread line is already linked to a ' + 'journal entry! Please post or delete it.')) + created_moves = self.env['account.move'] for line in self: - if line.move_id: - raise UserError(_('This spread line is already linked to a ' - 'journal entry! Please post or delete it.')) - move_vals = line._prepare_move() move = self.env['account.move'].create(move_vals) diff --git a/account_spread_cost_revenue/tests/test_account_invoice_spread.py b/account_spread_cost_revenue/tests/test_account_invoice_spread.py index 9e33a529..e89f91f4 100644 --- a/account_spread_cost_revenue/tests/test_account_invoice_spread.py +++ b/account_spread_cost_revenue/tests/test_account_invoice_spread.py @@ -746,3 +746,25 @@ class TestAccountInvoiceSpread(common.TransactionCase): with self.assertRaises(ValidationError): self.spread.unlink() + + def test_15_invoice_refund(self): + + self.invoice_line.spread_id = self.spread + + # Validate invoice + self.invoice.action_invoice_open() + self.assertTrue(self.invoice.invoice_line_ids.mapped('spread_id')) + + # Create a refund for invoice. + self.env['account.invoice.refund'].with_context({ + 'active_model': 'account.invoice', + 'active_ids': [self.invoice.id], + 'active_id': self.invoice.id + }).create(dict( + description='Invoice Refund', + filter_refund='refund', + )).invoice_refund() + + # Invoice lines do not contain the lint to the spread. + refund = self.invoice.refund_invoice_ids[0] + self.assertFalse(refund.invoice_line_ids.mapped('spread_id'))