Code review + add coverage

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

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 = '<a href=# data-oe-model=account.invoice ' \
-                   'data-oe-id=%d>%s</a>' % (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 = '<a href=# data-oe-model=account.spread ' \
-                      'data-oe-id=%d>%s</a>' % (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 = '<a href=# data-oe-model=account.invoice ' \
+                       'data-oe-id=%d>%s</a>' % (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 = '<a href=# data-oe-model=account.spread ' \
+                          'data-oe-id=%d>%s</a>' % (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')

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)
 

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'))