[11.0][FIX] sale_global_discount: Constraint tax lines combinations

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/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