[FIX] account_global_discount: More on global discount/taxes link

- We need to take into account invoice lines with multiple taxes, so the link should
  be m2m.
- Migration scripts for preserving the best information on existing installations.
- Tests for checking new conditions.
- Perform sanity check for not ending in an incompatible situation.
- Some refactor done on onchanges for avoiding duplicating operations.
- Adjust UI for not allowing to edit computed invoice global discounts.

account_global_discount/README.rst

@@ -53,9 +53,10 @@ You can assign global discounts to partners as well:
 Known issues / Roadmap
 ======================
 
-* Global Discount move lines are created for a common base amount. If that
-  wasn't the case, we should split discount move lines between bases and
-  assign proper taxes accordingly.
+* Not all the taxes combination can be compatible with global discounts, as
+  the generated journal items won't be correct for taxes declarations. An error
+  is raised in that cases.
+* Currently, taxes in invoice lines are mandatory with global discounts.
 
 Bug Tracker
 ===========

account_global_discount/__manifest__.py

@@ -1,8 +1,9 @@
 # Copyright 2019 Tecnativa S.L. - David Vidal
+# Copyright 2020 Tecnativa - Pedro M. Baeza
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 {
     'name': 'Account Global Discount',
-    'version': '11.0.1.1.0',
+    'version': '11.0.2.0.0',
     'category': 'Accounting',
     'author': 'Tecnativa,'
               'Odoo Community Association (OCA)',

account_global_discount/migrations/11.0.2.0.0/post-migration.py

@@ -0,0 +1,55 @@
+# Copyright 2020 Tecnativa - Pedro M. Baeza
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
+
+from openupgradelib import openupgrade  # pylint: disable=W7936
+from psycopg2 import sql
+
+
+@openupgrade.migrate()
+def migrate(env, version):
+    # Link the new field that points to the invoice global discount instead
+    # of the global discount definition
+    openupgrade.logged_query(
+        env.cr, sql.SQL("""
+        UPDATE account_move_line aml
+        SET invoice_global_discount_id = aigd.id
+        FROM account_invoice_global_discount aigd
+        WHERE aigd.invoice_id = aml.invoice_id
+            AND aigd.global_discount_id = aml.{}
+        """).format(
+            sql.Identifier(openupgrade.get_legacy_name("global_discount_id"))
+        )
+    )
+    # Link to existing global discount records, all the invoice taxes as best
+    # effort
+    openupgrade.logged_query(
+        env.cr, """
+        INSERT INTO account_invoice_global_discount_account_tax_rel
+        (account_invoice_global_discount_id, account_tax_id)
+        SELECT aigd.id, ailt.tax_id
+        FROM account_invoice_global_discount aigd
+        JOIN account_invoice_line ail ON aigd.invoice_id = ail.invoice_id
+        JOIN account_invoice_line_tax ailt ON ailt.invoice_line_id = ail.id
+        GROUP BY aigd.id, ailt.tax_id"""
+    )
+    # Delete in prevention of manual manipulations existing tax lines linked
+    # to global discount journal items
+    openupgrade.logged_query(
+        env.cr, """
+        DELETE FROM account_move_line_account_tax_rel rel
+            USING account_move_line aml
+        WHERE rel.account_move_line_id = aml.id
+            AND aml.invoice_global_discount_id IS NOT NULL"""
+    )
+    # Link all invoice taxes in global discount existing journal items as best
+    # effort
+    openupgrade.logged_query(
+        env.cr, """
+        INSERT INTO account_move_line_account_tax_rel
+        (account_move_line_id, account_tax_id)
+        SELECT aml.id, rel.account_tax_id
+        FROM account_move_line aml
+        JOIN account_invoice_global_discount_account_tax_rel rel
+            ON rel.account_invoice_global_discount_id =
+                aml.invoice_global_discount_id"""
+    )

account_global_discount/migrations/11.0.2.0.0/pre-migration.py

@@ -0,0 +1,11 @@
+# Copyright 2020 Tecnativa - Pedro M. Baeza
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
+
+from openupgradelib import openupgrade  # pylint: disable=W7936
+
+
+@openupgrade.migrate()
+def migrate(env, version):
+    openupgrade.rename_columns(
+        env.cr, {"account_move_line": [("global_discount_id", None)]}
+    )

account_global_discount/models/account_invoice.py

@@ -1,6 +1,7 @@
 # Copyright 2019 Tecnativa - David Vidal
+# Copyright 2020 Tecnativa - Pedro M. Baeza
 # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
-from odoo import api, fields, models
+from odoo import _, api, exceptions, fields, models
 from odoo.addons import decimal_precision as dp
 
 
@@ -35,14 +36,45 @@ class AccountInvoice(models.Model):
     invoice_global_discount_ids = fields.One2many(
         comodel_name='account.invoice.global.discount',
         inverse_name='invoice_id',
+        readonly=True,
     )
 
     def _set_global_discounts_by_tax(self):
-        """Create invoice global discount lines by tax and discount"""
+        """Create invoice global discount lines by taxes combinations and
+        discounts.
+        """
         self.ensure_one()
+        if not self.global_discount_ids:
+            return
         invoice_global_discounts = self.env['account.invoice.global.discount']
+        taxes_keys = {}
+        # Perform a sanity check for discarding cases that will lead to
+        # incorrect data in discounts
+        for inv_line in self.invoice_line_ids:
+            if not inv_line.invoice_line_tax_ids:
+                raise exceptions.UserError(_(
+                    "With global discounts, taxes in lines are required."
+                ))
+            for key in taxes_keys:
+                if key == inv_line.invoice_line_tax_ids:
+                    break
+                elif key & inv_line.invoice_line_tax_ids:
+                    raise exceptions.UserError(_(
+                        "Incompatible taxes found for global discounts."
+                    ))
+            else:
+                taxes_keys[inv_line.invoice_line_tax_ids] = True
         for tax_line in self.tax_line_ids:
-            base = tax_line.base
+            key = []
+            to_create = True
+            for key in taxes_keys:
+                if tax_line.tax_id in key:
+                    to_create = taxes_keys[key]
+                    taxes_keys[key] = False  # mark for not duplicating
+                    break  # we leave in key variable the proper taxes value
+            if not to_create:
+                continue
+            base = tax_line.base_before_global_discounts or tax_line.base
             for global_discount in self.global_discount_ids:
                 discount = global_discount._get_global_discount_vals(base)
                 invoice_global_discounts += invoice_global_discounts.new({
@@ -53,7 +85,7 @@ def _set_global_discounts_by_tax(self):
                     'base': base,
                     'base_discounted': discount['base_discounted'],
                     'account_id': global_discount.account_id.id,
-                    'tax_id': tax_line.tax_id.id,
+                    'tax_ids': [(4, x.id) for x in key],
                 })
                 base = discount['base_discounted']
         self.invoice_global_discount_ids = invoice_global_discounts
@@ -63,17 +95,12 @@ def _set_global_discounts(self):
         fetched in their sequence order """
         for inv in self:
             inv._set_global_discounts_by_tax()
-            # Recompute line taxes according to global discounts
-            taxes_grouped = inv.get_taxes_values()
-            tax_lines = inv.tax_line_ids.filtered('manual')
-            for tax in taxes_grouped.values():
-                tax_lines += tax_lines.new(tax)
-            inv.tax_line_ids = tax_lines
 
     @api.onchange('invoice_line_ids')
     def _onchange_invoice_line_ids(self):
+        res = super()._onchange_invoice_line_ids()
         self._set_global_discounts()
-        return super()._onchange_invoice_line_ids()
+        return res
 
     @api.onchange('partner_id', 'company_id')
     def _onchange_partner_id(self):
@@ -86,14 +113,12 @@ def _onchange_partner_id(self):
                 self.partner_id.supplier_global_discount_ids):
             self.global_discount_ids = (
                 self.partner_id.supplier_global_discount_ids)
-        self._set_global_discounts()
         return res
 
     @api.onchange('global_discount_ids')
     def _onchange_global_discount_ids(self):
         """Trigger global discount lines to recompute all"""
-        self._set_global_discounts()
-        return
+        return self._onchange_invoice_line_ids()
 
     @api.depends('invoice_line_ids.price_subtotal', 'tax_line_ids.amount',
                  'tax_line_ids.amount_rounding', 'currency_id', 'company_id',
@@ -126,10 +151,12 @@ def _compute_amount(self):
         self.amount_untaxed_signed = amount_untaxed_signed * sign
 
     def get_taxes_values(self):
+        """Override this computation for adding global discount to taxes."""
         round_curr = self.currency_id.round
         tax_grouped = super().get_taxes_values()
         for key in tax_grouped.keys():
             base = tax_grouped[key]['base']
+            tax_grouped[key]['base_before_global_discounts'] = base
             amount = tax_grouped[key]['amount']
             for discount in self.global_discount_ids:
                 base = discount._get_global_discount_vals(
@@ -149,20 +176,29 @@ def invoice_line_move_line_get(self):
                 continue
             res.append({
                 'invoice_global_discount_id': discount.id,
-                'global_discount_id': discount.global_discount_id.id,
                 'type': 'global_discount',
-                'name': discount.name,
+                'name': "%s - %s" % (
+                    discount.name, ", ".join(discount.tax_ids.mapped("name"))),
                 'price_unit': discount.discount_amount * -1,
                 'quantity': 1,
                 'price': discount.discount_amount * -1,
                 'account_id': discount.account_id.id,
                 'account_analytic_id': discount.account_analytic_id.id,
-                'tax_ids': discount.tax_id.id,
+                'tax_ids': [(4, x.id) for x in discount.tax_ids],
                 'invoice_id': self.id,
             })
         return res
 
 
+class AccountInvoiceTax(models.Model):
+    _inherit = "account.invoice.tax"
+
+    base_before_global_discounts = fields.Monetary(
+        string='Amount Untaxed Before Discounts',
+        readonly=True,
+    )
+
+
 class AccountInvoiceGlobalDiscount(models.Model):
     _name = "account.invoice.global.discount"
     _description = "Invoice Global Discount"
@@ -212,7 +248,7 @@ class AccountInvoiceGlobalDiscount(models.Model):
         currency_field='currency_id',
         readonly=True,
     )
-    tax_id = fields.Many2one(
+    tax_ids = fields.Many2many(
         comodel_name='account.tax',
     )
     account_id = fields.Many2one(

account_global_discount/models/account_move_line.py

@@ -6,11 +6,6 @@
 class AccountMoveLine(models.Model):
     _inherit = 'account.move.line'
 
-    global_discount_id = fields.Many2one(
-        comodel_name='global.discount',
-        string='Global Discount',
-        ondelete='restrict',
-    )
     invoice_global_discount_id = fields.Many2one(
         comodel_name='account.invoice.global.discount',
         string='Invoice Global Discount',

account_global_discount/readme/ROADMAP.rst

@@ -1,3 +1,4 @@
-* Global Discount move lines are created for a common base amount. If that
-  wasn't the case, we should split discount move lines between bases and
-  assign proper taxes accordingly.
+* Not all the taxes combination can be compatible with global discounts, as
+  the generated journal items won't be correct for taxes declarations. An error
+  is raised in that cases.
+* Currently, taxes in invoice lines are mandatory with global discounts.

account_global_discount/tests/test_global_discount.py

@@ -1,5 +1,7 @@
 # Copyright 2019 Tecnativa - David Vidal
+# Copyright 2020 Tecnativa - Pedro M. Baeza
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
+from odoo import exceptions
 from odoo.tests import common
 
 
@@ -123,9 +125,113 @@ def test_02_global_invoice_discounts_from_partner(self):
         # on the type of the invoice. In this case, we fetch the supplier
         # global discounts
         self.invoice.partner_id = self.partner_2
+        # trigger onchanges mimicking UI
         self.invoice._onchange_partner_id()
+        self.invoice._onchange_global_discount_ids()
         self.assertAlmostEqual(self.invoice.tax_line_ids.base, 140.0)
         self.assertAlmostEqual(self.invoice.tax_line_ids.amount, 21.0)
         self.assertAlmostEqual(self.invoice.amount_untaxed, 140.0)
         self.assertAlmostEqual(self.invoice.amount_total, 161.0)
         self.assertAlmostEqual(self.invoice.amount_global_discount, -60.0)
+
+    def test_03_multiple_taxes_multi_line(self):
+        tax2 = self.env['account.tax'].create({
+            'name': 'TAX 20%',
+            'amount_type': 'percent',
+            'type_tax_use': 'purchase',
+            'amount': 20.0,
+        })
+        self.invoice_line.create({
+            'invoice_id': self.invoice.id,
+            'name': 'Line 2',
+            'price_unit': 100.0,
+            'account_id': self.account.id,
+            'invoice_line_tax_ids': [(6, 0, [tax2.id])],
+            'quantity': 1,
+        })
+        self.invoice.global_discount_ids = self.global_discount_1
+        self.invoice._onchange_global_discount_ids()
+        # Global discounts are applied to the base and taxes are recomputed:
+        # 300 - 20% (global disc. 1) =  240
+        self.assertEqual(len(self.invoice.invoice_global_discount_ids), 2)
+        discount_tax_15 = self.invoice.invoice_global_discount_ids.filtered(
+            lambda x: x.tax_ids == self.tax)
+        discount_tax_20 = self.invoice.invoice_global_discount_ids.filtered(
+            lambda x: x.tax_ids == tax2)
+        self.assertAlmostEqual(discount_tax_15.discount_amount, 40)
+        self.assertAlmostEqual(discount_tax_20.discount_amount, 20)
+        tax_line_15 = self.invoice.tax_line_ids.filtered(
+            lambda x: x.tax_id == self.tax)
+        tax_line_20 = self.invoice.tax_line_ids.filtered(
+            lambda x: x.tax_id == tax2)
+        self.assertAlmostEqual(tax_line_15.base, 160)
+        self.assertAlmostEqual(tax_line_15.amount, 24)
+        self.assertAlmostEqual(tax_line_20.base, 80.0)
+        self.assertAlmostEqual(tax_line_20.amount, 16)
+        self.assertAlmostEqual(self.invoice.amount_untaxed, 240.0)
+        self.assertAlmostEqual(self.invoice.amount_total, 280)
+        self.assertAlmostEqual(self.invoice.amount_global_discount, -60.0)
+        # Validate invoice for seeing result
+        self.invoice.action_invoice_open()
+        self.assertEqual(len(self.invoice.move_id.line_ids), 7)
+
+    def test_04_multiple_taxes_same_line(self):
+        tax2 = self.env['account.tax'].create({
+            'name': 'Retention 20%',
+            'amount_type': 'percent',
+            'type_tax_use': 'purchase',
+            'amount': -20.0,  # negative for testing more use cases
+        })
+        self.invoice_line1.invoice_line_tax_ids = [(4, tax2.id)]
+        self.invoice.global_discount_ids = self.global_discount_1
+        self.invoice._onchange_global_discount_ids()
+        # Global discounts are applied to the base and taxes are recomputed:
+        # 300 - 20% (global disc. 1) =  240
+        self.assertEqual(len(self.invoice.invoice_global_discount_ids), 1)
+        self.assertAlmostEqual(
+            self.invoice.invoice_global_discount_ids.discount_amount, 40)
+        self.assertEqual(
+            self.invoice.invoice_global_discount_ids.tax_ids, self.tax + tax2)
+        tax_line_15 = self.invoice.tax_line_ids.filtered(
+            lambda x: x.tax_id == self.tax)
+        tax_line_20 = self.invoice.tax_line_ids.filtered(
+            lambda x: x.tax_id == tax2)
+        self.assertAlmostEqual(tax_line_15.base, 160)
+        self.assertAlmostEqual(tax_line_15.amount, 24)
+        self.assertAlmostEqual(tax_line_20.base, 160.0)
+        self.assertAlmostEqual(tax_line_20.amount, -32)
+        self.assertAlmostEqual(self.invoice.amount_untaxed, 160.0)
+        self.assertAlmostEqual(self.invoice.amount_total, 152)
+        self.assertAlmostEqual(self.invoice.amount_global_discount, -40.0)
+        # Validate invoice for seeing result
+        self.invoice.action_invoice_open()
+        self.assertEqual(len(self.invoice.move_id.line_ids), 5)
+
+    def test_05_incompatible_taxes(self):
+        # Line 1 with tax and tax2
+        # Line 2 with only tax2
+        tax2 = self.env['account.tax'].create({
+            'name': 'Retention 20%',
+            'amount_type': 'percent',
+            'type_tax_use': 'purchase',
+            'amount': -20.0,  # negative for testing more use cases
+        })
+        self.invoice_line1.invoice_line_tax_ids = [
+            (4, tax2.id), (4, self.tax.id)]
+        self.invoice_line.create({
+            'invoice_id': self.invoice.id,
+            'name': 'Line 2',
+            'price_unit': 100.0,
+            'account_id': self.account.id,
+            'invoice_line_tax_ids': [(6, 0, [tax2.id])],
+            'quantity': 1,
+        })
+        self.invoice.global_discount_ids = self.global_discount_1
+        with self.assertRaises(exceptions.UserError):
+            self.invoice._onchange_global_discount_ids()
+
+    def test_06_no_taxes(self):
+        self.invoice_line1.invoice_line_tax_ids = False
+        self.invoice.global_discount_ids = self.global_discount_1
+        with self.assertRaises(exceptions.UserError):
+            self.invoice._onchange_global_discount_ids()