[FIX] sale_global_discount: Constraint tax lines combinations

Not all taxes combinations are possible with global discounts when they are later
invoiced and posted, so we need to constraint them early.

OCA/account-invoicing#745 addresses the problem at invoice level, posting correctly
taxes for supported cases, and constraining the rest.

We apply here the same constraints at sales order level. Existing tests have been
changed because they were using one of the forbidden taxes combination, and 2 new
tests have been added for covering the new constraints.

sale_global_discount/README.rst

@@ -64,6 +64,13 @@ To use this module, you need to:
 #. When you create an invoice from the order, the proper global discounts will
    be applied on it.
 
+Known issues / Roadmap
+======================
+
+* Not all the taxes combination can be compatible with global discounts. An
+  error is raised in that cases.
+* Currently, taxes in invoice lines are mandatory with global discounts.
+
 Bug Tracker
 ===========
 

sale_global_discount/__manifest__.py

@@ -2,7 +2,7 @@
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 {
     'name': 'Sale Global Discount',
-    'version': '11.0.1.0.2',
+    'version': '11.0.1.0.3',
     'category': 'Sales Management',
     'author': 'Tecnativa,'
               'Odoo Community Association (OCA)',

sale_global_discount/i18n/es.po

@@ -33,6 +33,12 @@ msgstr "<strong>Subtl. antes desc.</strong>"
 msgid "Amount Untaxed Before Discounts"
 msgstr "Base Imponible sin Descuentos"
 
+#. module: sale_global_discount
+#: code:addons/sale_global_discount/models/sale_order.py:62
+#, python-format
+msgid "Incompatible taxes found for global discounts."
+msgstr ""
+
 #. module: sale_global_discount
 #: model:ir.model,name:sale_global_discount.model_sale_order
 msgid "Quotation"
@@ -48,5 +54,11 @@ msgstr "Descuentos de venta globales"
 msgid "Total Global Discounts"
 msgstr "Total Descuentos Globales"
 
+#. module: sale_global_discount
+#: code:addons/sale_global_discount/models/sale_order.py:55
+#, python-format
+msgid "With global discounts, taxes in lines are required."
+msgstr ""
+
 #~ msgid "<strong>Global Discount</strong>"
 #~ msgstr "<strong>Descuento Global</strong>"

sale_global_discount/i18n/sale_global_discount.pot

@@ -29,6 +29,12 @@ msgstr ""
 msgid "Amount Untaxed Before Discounts"
 msgstr ""
 
+#. module: sale_global_discount
+#: code:addons/sale_global_discount/models/sale_order.py:62
+#, python-format
+msgid "Incompatible taxes found for global discounts."
+msgstr ""
+
 #. module: sale_global_discount
 #: model:ir.model,name:sale_global_discount.model_sale_order
 msgid "Quotation"
@@ -44,3 +50,9 @@ msgstr ""
 msgid "Total Global Discounts"
 msgstr ""
 
+#. module: sale_global_discount
+#: code:addons/sale_global_discount/models/sale_order.py:55
+#, python-format
+msgid "With global discounts, taxes in lines are required."
+msgstr ""
+

sale_global_discount/models/sale_order.py

@@ -1,6 +1,6 @@
 # Copyright 2020 Tecnativa - David Vidal
 # 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
 
 
 class SaleOrder(models.Model):
@@ -42,10 +42,34 @@ def get_discounted_global(self, price=0, discounts=None):
         price *= 1 - (discount / 100)
         return self.get_discounted_global(price, discounts)
 
+    def _check_global_discounts_sanity(self):
+        """Perform a sanity check for discarding cases that will lead to
+        incorrect data in discounts.
+        """
+        self.ensure_one()
+        if not self.global_discount_ids:
+            return True
+        taxes_keys = {}
+        for line in self.order_line:
+            if not line.tax_id:
+                raise exceptions.UserError(_(
+                    "With global discounts, taxes in lines are required."
+                ))
+            for key in taxes_keys:
+                if key == line.tax_id:
+                    break
+                elif key & line.tax_id:
+                    raise exceptions.UserError(_(
+                        "Incompatible taxes found for global discounts."
+                    ))
+            else:
+                taxes_keys[line.tax_id] = True
+
     @api.depends('order_line.price_total', 'global_discount_ids')
     def _amount_all(self):
         res = super()._amount_all()
         for order in self:
+            order._check_global_discounts_sanity()
             amount_untaxed_before_global_discounts = order.amount_untaxed
             amount_total_before_global_discounts = order.amount_total
             discounts = order.global_discount_ids.mapped('discount')
@@ -79,11 +103,10 @@ def _amount_all(self):
     @api.onchange('partner_id')
     def onchange_partner_id(self):
         res = super().onchange_partner_id()
-        if self.partner_id.customer_global_discount_ids:
-            self.global_discount_ids = (
-                self.partner_id.customer_global_discount_ids or
-                self.partner_id.commercial_partner_id
-                .customer_global_discount_ids)
+        self.global_discount_ids = (
+            self.partner_id.customer_global_discount_ids or
+            self.partner_id.commercial_partner_id
+            .customer_global_discount_ids)
         return res
 
     def _prepare_invoice(self):

sale_global_discount/readme/ROADMAP.rst

@@ -0,0 +1,3 @@
+* Not all the taxes combination can be compatible with global discounts. An
+  error is raised in that cases.
+* Currently, taxes in invoice lines are mandatory with global discounts.

sale_global_discount/static/description/index.html

@@ -375,11 +375,12 @@ <h1 class="title">Sale Global Discount</h1>
 <ul class="simple">
 <li><a class="reference internal" href="#configuration" id="id1">Configuration</a></li>
 <li><a class="reference internal" href="#usage" id="id2">Usage</a></li>
-<li><a class="reference internal" href="#bug-tracker" id="id3">Bug Tracker</a></li>
-<li><a class="reference internal" href="#credits" id="id4">Credits</a><ul>
-<li><a class="reference internal" href="#authors" id="id5">Authors</a></li>
-<li><a class="reference internal" href="#contributors" id="id6">Contributors</a></li>
-<li><a class="reference internal" href="#maintainers" id="id7">Maintainers</a></li>
+<li><a class="reference internal" href="#known-issues-roadmap" id="id3">Known issues / Roadmap</a></li>
+<li><a class="reference internal" href="#bug-tracker" id="id4">Bug Tracker</a></li>
+<li><a class="reference internal" href="#credits" id="id5">Credits</a><ul>
+<li><a class="reference internal" href="#authors" id="id6">Authors</a></li>
+<li><a class="reference internal" href="#contributors" id="id7">Contributors</a></li>
+<li><a class="reference internal" href="#maintainers" id="id8">Maintainers</a></li>
 </ul>
 </li>
 </ul>
@@ -415,24 +416,32 @@ <h1><a class="toc-backref" href="#id2">Usage</a></h1>
 be applied on it.</li>
 </ol>
 </div>
+<div class="section" id="known-issues-roadmap">
+<h1><a class="toc-backref" href="#id3">Known issues / Roadmap</a></h1>
+<ul class="simple">
+<li>Not all the taxes combination can be compatible with global discounts. An
+error is raised in that cases.</li>
+<li>Currently, taxes in invoice lines are mandatory with global discounts.</li>
+</ul>
+</div>
 <div class="section" id="bug-tracker">
-<h1><a class="toc-backref" href="#id3">Bug Tracker</a></h1>
+<h1><a class="toc-backref" href="#id4">Bug Tracker</a></h1>
 <p>Bugs are tracked on <a class="reference external" href="https://github.com/OCA/sale-workflow/issues">GitHub Issues</a>.
 In case of trouble, please check there if your issue has already been reported.
 If you spotted it first, help us smashing it by providing a detailed and welcomed
 <a class="reference external" href="https://github.com/OCA/sale-workflow/issues/new?body=module:%20sale_global_discount%0Aversion:%2011.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**">feedback</a>.</p>
 <p>Do not contact contributors directly about support or help with technical issues.</p>
 </div>
 <div class="section" id="credits">
-<h1><a class="toc-backref" href="#id4">Credits</a></h1>
+<h1><a class="toc-backref" href="#id5">Credits</a></h1>
 <div class="section" id="authors">
-<h2><a class="toc-backref" href="#id5">Authors</a></h2>
+<h2><a class="toc-backref" href="#id6">Authors</a></h2>
 <ul class="simple">
 <li>Tecnativa</li>
 </ul>
 </div>
 <div class="section" id="contributors">
-<h2><a class="toc-backref" href="#id6">Contributors</a></h2>
+<h2><a class="toc-backref" href="#id7">Contributors</a></h2>
 <ul class="simple">
 <li><a class="reference external" href="https://www.tecnativa.com">Tecnativa</a><ul>
 <li>David Vidal</li>
@@ -441,7 +450,7 @@ <h2><a class="toc-backref" href="#id6">Contributors</a></h2>
 </ul>
 </div>
 <div class="section" id="maintainers">
-<h2><a class="toc-backref" href="#id7">Maintainers</a></h2>
+<h2><a class="toc-backref" href="#id8">Maintainers</a></h2>
 <p>This module is maintained by the OCA.</p>
 <a class="reference external image-reference" href="https://odoo-community.org"><img alt="Odoo Community Association" src="https://odoo-community.org/logo.png" /></a>
 <p>OCA, or the Odoo Community Association, is a nonprofit organization whose

sale_global_discount/tests/test_sale_global_discount.py

@@ -1,5 +1,6 @@
 # Copyright 2020 Tecnativa - David Vidal
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
+from odoo import exceptions
 from odoo.tests import common
 
 
@@ -90,14 +91,14 @@ def setUpClass(cls):
                 'product_id': cls.product_2.id,
                 'product_uom_qty': 3,
                 'price_unit': 33.33,
-                'tax_id': [(6, 0, cls.tax_1.ids)]
+                'tax_id': [(6, 0, [cls.tax_1.id, cls.tax_2.id])]
             })],
         })
 
     def test_01_global_sale_succesive_discounts(self):
         """Add global discounts to the sale order"""
-        self.assertAlmostEqual(self.sale.amount_total, 294.99)
-        self.assertAlmostEqual(self.sale.amount_tax, 45)
+        self.assertAlmostEqual(self.sale.amount_total, 299.99)
+        self.assertAlmostEqual(self.sale.amount_tax, 50)
         self.assertAlmostEqual(self.sale.amount_untaxed, 249.99)
         # Apply a single 20% global discount
         self.sale.global_discount_ids = self.global_discount_1
@@ -107,38 +108,34 @@ def test_01_global_sale_succesive_discounts(self):
         self.assertAlmostEqual(self.sale.amount_untaxed, 199.99)
         self.assertAlmostEqual(
             self.sale.amount_untaxed_before_global_discounts, 249.99)
-        self.assertAlmostEqual(self.sale.amount_total, 235.99)
+        self.assertAlmostEqual(self.sale.amount_total, 239.99)
         self.assertAlmostEqual(
-            self.sale.amount_total_before_global_discounts, 294.99)
-        self.assertAlmostEqual(self.sale.amount_tax, 36)
+            self.sale.amount_total_before_global_discounts, 299.99)
+        self.assertAlmostEqual(self.sale.amount_tax, 40)
         # Apply an additional 30% global discount
         self.sale.global_discount_ids += self.global_discount_2
         self.assertAlmostEqual(self.sale.amount_global_discount, 110)
         self.assertAlmostEqual(self.sale.amount_untaxed, 139.99)
         self.assertAlmostEqual(
             self.sale.amount_untaxed_before_global_discounts, 249.99)
-        self.assertAlmostEqual(self.sale.amount_total, 165.19)
+        self.assertAlmostEqual(self.sale.amount_total, 167.99)
         self.assertAlmostEqual(
-            self.sale.amount_total_before_global_discounts, 294.99)
-        self.assertAlmostEqual(self.sale.amount_tax, 25.2)
+            self.sale.amount_total_before_global_discounts, 299.99)
+        self.assertAlmostEqual(self.sale.amount_tax, 28)
 
     def test_02_global_sale_discounts_from_partner(self):
         """Change the partner and his global discounts go to the invoice"""
-        self.assertAlmostEqual(self.sale.amount_total, 294.99)
-        self.assertAlmostEqual(self.sale.amount_tax, 45)
-        self.assertAlmostEqual(self.sale.amount_untaxed, 249.99)
-        # Change the partner and his globlal discounts are fetched
         # (30% then 50%)
         self.sale.partner_id = self.partner_2
         self.sale.onchange_partner_id()
         self.assertAlmostEqual(self.sale.amount_global_discount, 162.49)
         self.assertAlmostEqual(self.sale.amount_untaxed, 87.5)
         self.assertAlmostEqual(
             self.sale.amount_untaxed_before_global_discounts, 249.99)
-        self.assertAlmostEqual(self.sale.amount_total, 103.26)
+        self.assertAlmostEqual(self.sale.amount_total, 105.01)
         self.assertAlmostEqual(
-            self.sale.amount_total_before_global_discounts, 294.99)
-        self.assertAlmostEqual(self.sale.amount_tax, 15.76)
+            self.sale.amount_total_before_global_discounts, 299.99)
+        self.assertAlmostEqual(self.sale.amount_tax, 17.51)
 
     def test_03_global_sale_discounts_to_invoice(self):
         """All the discounts go to the invoice"""
@@ -153,25 +150,37 @@ def test_03_global_sale_discounts_to_invoice(self):
         line_tax_2 = invoice.tax_line_ids.filtered(
             lambda x: x.tax_id == self.tax_2)
         self.assertAlmostEqual(line_tax_1.base, 87.5)
-        self.assertAlmostEqual(line_tax_2.base, 52.5)
+        self.assertAlmostEqual(line_tax_2.base, 87.5)
         self.assertAlmostEqual(line_tax_1.amount, 13.13)
-        self.assertAlmostEqual(line_tax_2.amount, 2.63)
+        self.assertAlmostEqual(line_tax_2.amount, 4.38)
         discount_amount = sum(
             invoice.invoice_global_discount_ids.mapped('discount_amount'))
         self.assertAlmostEqual(discount_amount, 162.49)
         self.assertAlmostEqual(
             invoice.amount_untaxed_before_global_discounts, 249.99)
         self.assertAlmostEqual(invoice.amount_untaxed, 87.5)
-        self.assertAlmostEqual(invoice.amount_total, 103.26)
+        self.assertAlmostEqual(invoice.amount_total, 105.01)
 
     def test_04_report_taxes(self):
         """Taxes by group shown in reports"""
         self.sale.partner_id = self.partner_2
         self.sale.onchange_partner_id()
         taxes_groups = self.sale._get_tax_amount_by_group()
         # Taxes
-        self.assertAlmostEqual(taxes_groups[0][1], 2.63)
+        self.assertAlmostEqual(taxes_groups[0][1], 4.38)
         self.assertAlmostEqual(taxes_groups[1][1], 13.13)
         # Bases
-        self.assertAlmostEqual(taxes_groups[0][2], 52.5)
+        self.assertAlmostEqual(taxes_groups[0][2], 87.5)
         self.assertAlmostEqual(taxes_groups[1][2], 87.5)
+
+    def test_05_incompatible_taxes(self):
+        # Line 1 with tax 1 and tax 2
+        # Line 2 with only tax 2
+        self.sale.order_line[1].tax_id = [(6, 0, self.tax_1.ids)]
+        with self.assertRaises(exceptions.UserError):
+            self.sale.global_discount_ids = self.global_discount_1
+
+    def test_06_no_taxes(self):
+        self.sale.order_line[1].tax_id = False
+        with self.assertRaises(exceptions.UserError):
+            self.sale.global_discount_ids = self.global_discount_1