[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(
@@ -147,22 +174,38 @@ def invoice_line_move_line_get(self):
         for discount in self.invoice_global_discount_ids:
             if not discount.discount:
                 continue
-            res.append({
+            # Traverse upstream result for taking existing dictionary vals
+            inv_lines = self.invoice_line_ids.filtered(
+                lambda x: x.invoice_line_tax_ids == discount.tax_ids)
+            discount_dict = {}
+            for move_line_dict in res:
+                if move_line_dict.get("invl_id", 0) in inv_lines.ids:
+                    discount_dict.update(move_line_dict)
+            # Change needed values for the global discount
+            discount_dict.update({
                 '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,
-                'invoice_id': self.id,
             })
+            res.append(discount_dict)
         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 +255,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.