Merge pull request #8 from factorlibre/16.0-fix-changes-sale_invoice_…

…policy

[16.0][FIX] sale_invoice_policy: changes after review

sale_invoice_policy/models/product_template.py

@@ -1,29 +1,18 @@
 # Copyright 2017 ACSONE SA/NV (<http://acsone.eu>)
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 
-from odoo import api, models
+from odoo import fields, models
 
 
 class ProductTemplate(models.Model):
     _inherit = "product.template"
 
-    @api.depends("detailed_type")
-    def _compute_invoice_policy(self):
-        """
-        Apply the invoice_policy given by context (if exist) otherwise use the
-        default invoice policy given by variable with this same name.
-        If the product is type = 'service', we don't have to apply the invoice
-        policy given by the context.
-        """
-        invoice_policy = self.env.context.get("invoice_policy")
-        if invoice_policy:
-            non_service_products = self.filtered(lambda p: p.type != "service")
-            non_service_products.invoice_policy = invoice_policy
-        else:
-            default_invoice_policy = (
-                self.env["res.config.settings"]
-                .sudo()
-                .default_get(["default_invoice_policy"])
-                .get("default_invoice_policy", False)
-            )
-            self.invoice_policy = default_invoice_policy
+    def _default_invoice_policy(self):
+        return (
+            self.env["res.config.settings"]
+            .sudo()
+            .default_get(["default_invoice_policy"])
+            .get("default_invoice_policy", False)
+        )
+
+    invoice_policy = fields.Selection(default=_default_invoice_policy)

sale_invoice_policy/models/sale_order_line.py

@@ -7,6 +7,20 @@
 class SaleOrderLine(models.Model):
     _inherit = "sale.order.line"
 
+    def _get_order_lines_by_invoice_policy(self, invoice_policy):
+        so_lines = self.filtered(
+            lambda x, p=invoice_policy: x.order_id.invoice_policy == p
+        ).with_prefetch()
+        return so_lines
+
+    def _set_invoice_policy_on_product_templates(self, invoice_policy):
+        product_templates = self.mapped("product_template_id").filtered(
+            lambda p: p.invoice_policy != invoice_policy
+            and p.detailed_type != "service"
+        )
+        if product_templates:
+            product_templates.write({"invoice_policy": invoice_policy})
+
     @api.depends(
         "qty_invoiced",
         "qty_delivered",
@@ -19,16 +33,8 @@ def _compute_qty_to_invoice(self):
         line_by_id = {line.id: line for line in self}
         done_lines = self.env["sale.order.line"]
         for invoice_policy in invoice_policies:
-            so_lines = self.filtered(
-                lambda x, p=invoice_policy: x.order_id.invoice_policy == p
-            ).with_prefetch()
-            product_templates = so_lines.mapped("product_template_id").filtered(
-                lambda p: p.invoice_policy != invoice_policy
-            )
-            if product_templates:
-                product_templates.with_context(
-                    invoice_policy=invoice_policy
-                )._compute_invoice_policy()
+            so_lines = self._get_order_lines_by_invoice_policy(invoice_policy)
+            so_lines._set_invoice_policy_on_product_templates(invoice_policy)
             if so_lines:
                 done_lines |= so_lines
                 super(SaleOrderLine, so_lines)._compute_qty_to_invoice()
@@ -54,16 +60,8 @@ def _compute_invoice_status(self):
         line_by_id = {line.id: line for line in self}
         done_lines = self.env["sale.order.line"]
         for invoice_policy in invoice_policies:
-            so_lines = self.filtered(
-                lambda x, p=invoice_policy: x.order_id.invoice_policy == p
-            ).with_prefetch()
-            product_templates = so_lines.mapped("product_template_id").filtered(
-                lambda p: p.invoice_policy != invoice_policy
-            )
-            if product_templates:
-                product_templates.with_context(
-                    invoice_policy=invoice_policy
-                )._compute_invoice_policy()
+            so_lines = self._get_order_lines_by_invoice_policy(invoice_policy)
+            so_lines._set_invoice_policy_on_product_templates(invoice_policy)
             done_lines |= so_lines
             if so_lines:
                 super(SaleOrderLine, so_lines)._compute_invoice_status()

sale_invoice_policy/tests/test_sale_invoice_policy.py

@@ -47,9 +47,12 @@ def test_sale_order_invoice_order(self):
         so_line = so.order_line[0]
         self.assertEqual(so_line.qty_to_invoice, 2)
         self.assertEqual(so_line.invoice_status, "to invoice")
+        self.assertEqual(so_line.product_id.invoice_policy, "order")
+
         so_line = so.order_line[1]
         self.assertEqual(so_line.qty_to_invoice, 3)
         self.assertEqual(so_line.invoice_status, "to invoice")
+        self.assertEqual(so_line.product_id.invoice_policy, "order")
 
     def test_sale_order_invoice_deliver(self):
         """Test invoicing based on delivered quantities"""
@@ -88,82 +91,12 @@ def test_sale_order_invoice_deliver(self):
         so_line = so.order_line[0]
         self.assertEqual(so_line.qty_to_invoice, 2)
         self.assertEqual(so_line.invoice_status, "to invoice")
+        self.assertEqual(so_line.product_id.invoice_policy, "delivery")
 
         so_line = so.order_line[1]
         self.assertEqual(so_line.qty_to_invoice, 3)
         self.assertEqual(so_line.invoice_status, "to invoice")
-
-    def test_sale_order_invoice_policy_service1(self):
-        """
-        For this test, we check if the invoice policy is correctly updated
-        (into the product) when the type is 'service'.
-        The behaviour should be:
-        - Get the value of the context but if the type is 'service': use the
-        default_invoice_policy value from res.config.settings
-        :return: bool
-        """
-        product = self.product3.product_tmpl_id
-        product.write({"invoice_policy": "delivery"})
-        self.assertEqual(product.invoice_policy, "delivery")
-        product.with_context(invoice_policy="order")._compute_invoice_policy()
-        # Shouldn't be impacted by the context because the type is service
-        self.assertEqual(product.detailed_type, "service")
-        self.assertEqual(product.invoice_policy, "delivery")
-        return True
-
-    def test_sale_order_invoice_policy_service2(self):
-        """
-        For this test, we check if the invoice policy is correctly updated
-        (into the product) when the type is 'service'.
-        The behaviour should be:
-        - Get the value of the context but if the type is 'service': use the
-        default_invoice_policy value from res.config.settings
-        :return: bool
-        """
-        product = self.product3.product_tmpl_id
-        product.write({"invoice_policy": "order"})
-        self.assertEqual(product.invoice_policy, "order")
-        product.with_context(invoice_policy="delivery")._compute_invoice_policy()
-        # Shouldn't be impacted by the context because the type is service
-        self.assertEqual(product.detailed_type, "service")
-        self.assertEqual(product.invoice_policy, "order")
-        return True
-
-    def test_sale_order_invoice_policy_service3(self):
-        """
-        For this test, we check if the invoice policy is correctly updated
-        (into the product) when the type is 'service'.
-        The behaviour should be:
-        - Get the value of the context but if the type is 'service': use the
-        default_invoice_policy value from res.config.settings
-        :return: bool
-        """
-        product = self.product3
-        product2 = self.product2
-        products = product
-        products |= product2
-        invoice_policy = "order"
-        products.write({"invoice_policy": invoice_policy})
-        self.assertEqual(product.invoice_policy, invoice_policy)
-        self.assertEqual(product2.invoice_policy, invoice_policy)
-        new_invoice_policy = "delivery"
-        products.mapped("product_tmpl_id").with_context(
-            invoice_policy=new_invoice_policy,
-        )._compute_invoice_policy()
-        # Shouldn't be impacted by the context because the type is service
-        self.assertEqual(product.detailed_type, "service")
-        self.assertEqual(product.invoice_policy, invoice_policy)
-        # This one is not a service, so it must be impacted by the context
-        self.assertEqual(product2.invoice_policy, new_invoice_policy)
-        products.mapped("product_tmpl_id").with_context(
-            invoice_policy=invoice_policy,
-        )._compute_invoice_policy()
-        # Shouldn't be impacted by the context because the type is service
-        self.assertEqual(product.detailed_type, "service")
-        self.assertEqual(product.invoice_policy, invoice_policy)
-        # This one is not a service, so it must be impacted by the context
-        self.assertEqual(product2.invoice_policy, invoice_policy)
-        return True
+        self.assertEqual(so_line.product_id.invoice_policy, "delivery")
 
     def test_settings(self):
         # delivery policy is the default