From d2dd13e8b14f846b3d850649b22b4edfe0aeeccd Mon Sep 17 00:00:00 2001 From: Akim Juillerat Date: Thu, 18 Jan 2024 19:36:24 +0100 Subject: [PATCH] Refactor mto_route_product_variant Define route_ids as computed on the product variant, in order to reuse route_ids from its template and add or remove MTO route according to the setting on the variant. In case MTO route is changed on the template, it must reset any variant specific setting. --- .../models/product_product.py | 68 ++++++++----------- .../models/product_template.py | 40 ++++++++--- .../readme/CONFIGURATION.rst | 2 + .../readme/CONTRIBUTORS.rst | 1 + .../readme/DESCRIPTION.rst | 4 +- stock_product_variant_mto/readme/USAGE.rst | 4 -- stock_product_variant_mto/tests/common.py | 4 ++ .../tests/test_mto_variant.py | 56 +++++++++++++-- 8 files changed, 119 insertions(+), 60 deletions(-) create mode 100644 stock_product_variant_mto/readme/CONFIGURATION.rst delete mode 100644 stock_product_variant_mto/readme/USAGE.rst diff --git a/stock_product_variant_mto/models/product_product.py b/stock_product_variant_mto/models/product_product.py index cbfc09b1a3fb..d3bdc05354bf 100644 --- a/stock_product_variant_mto/models/product_product.py +++ b/stock_product_variant_mto/models/product_product.py @@ -5,7 +5,9 @@ IS_MTO_HELP = """ Check or Uncheck this field to enable the Make To Order on the variant, - independantly from its template configuration. + independantly from its template configuration.\n + Please note that activating or deactivating Make To Order on the template, + will reset this setting on its variants. """ class ProductProduct(models.Model): @@ -14,49 +16,35 @@ class ProductProduct(models.Model): is_mto = fields.Boolean( string="Variant is MTO", compute="_compute_is_mto", - inverse="_inverse_is_mto", store=True, + readonly=False, help=IS_MTO_HELP, ) - @api.depends("product_tmpl_id.route_ids") + route_ids = fields.Many2many( + "stock.location.route", + compute="_compute_route_ids", + store=False + ) + def _compute_is_mto(self): - # We only want to force all variants `is_mto` to True when - # the mto route is explicitely set on its template. - # Watching the mto route is not enough, since we might - # have a template with the mto route, and disabled the mto route - # for a few of its variants - # If a user sets another route on the variant, we do not want the - # mto disabled variants to be updated. - # To ensure that, the created `has_mto_route_changed` boolean field - # is set to True only when the MTO route is set on a template. mto_route = self.env.ref("stock.route_warehouse0_mto", raise_if_not_found=False) - templates = self.product_tmpl_id - # Only variants with a template with the MTO route and has_mto_route_changed - # should be updated. - mto_templates = templates.filtered( - lambda t: mto_route in t.route_ids and t.has_mto_route_changed - ) - mto_variants = self.filtered(lambda p: p.product_tmpl_id in mto_templates) - mto_variants.is_mto = True - # For the other variants, keep their current value. - other_variants = self - mto_variants - for variant in other_variants: - variant.is_mto = variant.is_mto - # Then set template's has_mto_route_changed to False, as it - # has been handled above - templates.has_mto_route_changed = False - - def _inverse_is_mto(self): - # When all variants of a template are `is_mto == False`, drop the MTO route - # from the template, otherwise do nothing + for product in self: + if not mto_route: + product.is_mto = False + continue + product.is_mto = mto_route in product.product_tmpl_id.route_ids + + @api.depends("is_mto", "product_tmpl_id.route_ids") + def _compute_route_ids(self): mto_route = self.env.ref("stock.route_warehouse0_mto", raise_if_not_found=False) - for template in self.product_tmpl_id: - is_mto = False - for variant in template.product_variant_ids: - if variant.is_mto: - is_mto = True - break - # If no variant is mto, then drop the route of the template. - if not is_mto: - template.route_ids = [(3, mto_route.id, 0)] + for product in self: + if mto_route and mto_route in product.product_tmpl_id.route_ids: + if not product.is_mto: + product.route_ids = product.product_tmpl_id.route_ids - mto_route + continue + else: + if mto_route and product.is_mto: + product.route_ids = product.product_tmpl_id.route_ids + mto_route + continue + product.route_ids = product.product_tmpl_id.route_ids diff --git a/stock_product_variant_mto/models/product_template.py b/stock_product_variant_mto/models/product_template.py index 2c24693adad2..4c2baebb9b72 100644 --- a/stock_product_variant_mto/models/product_template.py +++ b/stock_product_variant_mto/models/product_template.py @@ -1,24 +1,42 @@ # Copyright 2023 Camptocamp SA # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) -from odoo import models, fields +from odoo import _, api, models, fields class ProductTemplate(models.Model): _inherit = "product.template" - has_mto_route_changed = fields.Boolean() - def write(self, values): if not "route_ids" in values: return super().write(values) - # This route_ids change will trigger variant's _compute_is_mto. - # We want to set variant's is_mto to True only if their - # template has been set to True here ↓ + # As _compute_is_mto cannot use api.depends (or it would reset MTO + # route on variants as soon as there is a change on the template routes), + # we need to check which template in self had MTO route activated + # or deactivated to force the recomputation of is_mto on variants mto_route = self.env.ref("stock.route_warehouse0_mto") template_not_mto_before = self.filtered(lambda t: mto_route not in t.route_ids) res = super().write(values) - template_mto_after = self.filtered(lambda t: mto_route in t.route_ids) - # Templates where mto route has changed are those where - # the mto route has been set - templates_mto_set = template_not_mto_before & template_mto_after - templates_mto_set.has_mto_route_changed = True + templates_mto_after = self.filtered(lambda t: mto_route in t.route_ids) + templates_mto_added = template_not_mto_before & templates_mto_after + templates_mto_removed = (self - template_not_mto_before) & (self - templates_mto_after) + (templates_mto_added | templates_mto_removed).product_variant_ids._compute_is_mto() + + @api.onchange("route_ids") + def onchange_route_ids(self): + mto_route = self.env.ref("stock.route_warehouse0_mto", raise_if_not_found=False) + if mto_route not in self._origin.route_ids and mto_route in self.route_ids._origin: + # Return warning activating MTO route + return { + "warning": { + "title": _("Warning"), + "message": _("Activating MTO route will reset `Variant is MTO` setting on the variants.") + } + } + if mto_route in self._origin.route_ids and mto_route not in self.route_ids._origin: + # Return warning deactivating MTO route + return { + "warning": { + "title": _("Warning"), + "message": _("Deactivating MTO route will reset `Variant is MTO` setting on the variants.") + } + } diff --git a/stock_product_variant_mto/readme/CONFIGURATION.rst b/stock_product_variant_mto/readme/CONFIGURATION.rst new file mode 100644 index 000000000000..616f736ea524 --- /dev/null +++ b/stock_product_variant_mto/readme/CONFIGURATION.rst @@ -0,0 +1,2 @@ +The checkbox `Variant is MTO` on the product variant allows +to force usage or non-usage of the MTO route for the variant. diff --git a/stock_product_variant_mto/readme/CONTRIBUTORS.rst b/stock_product_variant_mto/readme/CONTRIBUTORS.rst index bca4ee0cadbc..00d1de9cd1af 100644 --- a/stock_product_variant_mto/readme/CONTRIBUTORS.rst +++ b/stock_product_variant_mto/readme/CONTRIBUTORS.rst @@ -1 +1,2 @@ * Matthieu Méquignon +* Akim Juillerat diff --git a/stock_product_variant_mto/readme/DESCRIPTION.rst b/stock_product_variant_mto/readme/DESCRIPTION.rst index 5d3195d67b91..cca2497c4da5 100644 --- a/stock_product_variant_mto/readme/DESCRIPTION.rst +++ b/stock_product_variant_mto/readme/DESCRIPTION.rst @@ -1 +1,3 @@ -Allow to individually set variants as Make To Order +This module allows to define if a product variant can use the +Make To Order route without any dependency on its template routes +settings. diff --git a/stock_product_variant_mto/readme/USAGE.rst b/stock_product_variant_mto/readme/USAGE.rst deleted file mode 100644 index 956d094848b2..000000000000 --- a/stock_product_variant_mto/readme/USAGE.rst +++ /dev/null @@ -1,4 +0,0 @@ -This module is useless on its own. - -However, it is meant to give the ability to check if a product variant is MTO, -rather than checking its template's routes. diff --git a/stock_product_variant_mto/tests/common.py b/stock_product_variant_mto/tests/common.py index 6ad89d7821b7..4e9975da0b70 100644 --- a/stock_product_variant_mto/tests/common.py +++ b/stock_product_variant_mto/tests/common.py @@ -70,7 +70,11 @@ def toggle_is_mto(self, records): def assertVariantsMTO(self, records): records.invalidate_cache(["is_mto"]) self.assertTrue(all([record.is_mto for record in records])) + for rec in records: + self.assertIn(self.mto_route, rec.route_ids) def assertVariantsNotMTO(self, records): records.invalidate_cache(["is_mto"]) self.assertFalse(any([record.is_mto for record in records])) + for rec in records: + self.assertNotIn(self.mto_route, rec.route_ids) diff --git a/stock_product_variant_mto/tests/test_mto_variant.py b/stock_product_variant_mto/tests/test_mto_variant.py index e67ac9511c04..98c4490f9024 100644 --- a/stock_product_variant_mto/tests/test_mto_variant.py +++ b/stock_product_variant_mto/tests/test_mto_variant.py @@ -1,5 +1,6 @@ # Copyright 2023 Camptocamp SA # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) +import logging from .common import TestMTOVariantCommon @@ -30,11 +31,9 @@ def test_variants_mto(self): self.toggle_is_mto(black_pen) self.assertVariantsNotMTO(black_pen) self.assertVariantsMTO(blue_pen | green_pen | red_pen) - # Disable mto route on the template, variants is_mto is kept + # Disable mto route on the template, reset is_mto on variants self.remove_route(pen_template, self.mto_route) - # is_mto is unchanged - self.assertVariantsNotMTO(black_pen) - self.assertVariantsMTO(blue_pen | green_pen | red_pen) + self.assertVariantsNotMTO(pens) def test_template_routes_updated(self): # instanciate variables @@ -58,3 +57,52 @@ def test_template_routes_updated(self): # Template is still MTO, but variants is_mto shouldn't have changed self.assertVariantsMTO(green_pen | red_pen) self.assertVariantsNotMTO(black_pen | blue_pen) + + def test_template_warnings(self): + # instanciate variables + pen_template = self.template_pen + pens = self.variants_pen + blue_pen = self.blue_pen + red_pen = self.red_pen + green_pen = self.green_pen + black_pen = self.black_pen + onchange_logger = logging.getLogger("odoo.tests.common.onchange") + self.assertVariantsNotMTO(pens) + # enable mto route for black pen + self.toggle_is_mto(black_pen) + self.assertVariantsMTO(black_pen) + # Enable mto route on the template, raise warning as is_mto is reset on variants + with self.assertLogs(onchange_logger) as log: + self.add_route(pen_template, self.mto_route) + self.assertIn("WARNING", log.output[0]) + self.assertIn("Activating MTO route will reset", log.output[0]) + self.assertVariantsMTO(pens) + # Disable mto route for black pen + self.toggle_is_mto(black_pen) + self.assertVariantsNotMTO(black_pen) + self.assertVariantsMTO(blue_pen | green_pen | red_pen) + # Enable unrelated route does not raise warning nor reset + random_route = self.mto_route.create({"name": "loutourout de la vit"}) + with self.assertLogs(onchange_logger) as log: + self.add_route(pen_template, random_route) + onchange_logger.info("No warning raised") + self.assertNotIn("WARNING", log.output[0]) + self.assertVariantsNotMTO(black_pen) + self.assertVariantsMTO(blue_pen | green_pen | red_pen) + # Disable mto route on the template, raise warning as is_mto is reset on variants + with self.assertLogs(onchange_logger) as log: + self.remove_route(pen_template, self.mto_route) + self.assertIn("WARNING", log.output[0]) + self.assertIn("Deactivating MTO route will reset", log.output[0]) + self.assertVariantsNotMTO(pens) + # Enable mto route for black pen + self.toggle_is_mto(black_pen) + self.assertVariantsMTO(black_pen) + self.assertVariantsNotMTO(blue_pen | green_pen | red_pen) + # Disable unrelated route does not raise warning nor reset + with self.assertLogs(onchange_logger) as log: + self.remove_route(pen_template, random_route) + onchange_logger.info("No warning raised") + self.assertNotIn("WARNING", log.output[0]) + self.assertVariantsMTO(black_pen) + self.assertVariantsNotMTO(blue_pen | green_pen | red_pen)