Remove fallback location

It could not work properly here as we need the "fallback" to apply
even if there is no quantity at all in the stock. As we hook the
reservation rules in StockMove._update_reserved_quantity(), and
this method is called only if we have at least 1 product in qty,
the fallback was not applied with zero qty.

A new module will handle this concept: https://github.com/OCA/wms/pull/28

stock_reserve_rule/README.rst

@@ -23,7 +23,7 @@ Stock Reservation Rules
     :target: https://runbot.odoo-community.org/runbot/153/12.0
     :alt: Try me on Runbot
 
-|badge1| |badge2| |badge3| |badge4| |badge5| 
+|badge1| |badge2| |badge3| |badge4| |badge5|
 
 This module adds rules for advanced reservation / removal strategies.
 
@@ -86,8 +86,6 @@ Creation of a rule:
 Properties that define where the rule will be applied:
 
 * Location: Define where the rule will look for goods (a parent of the move's source location).
-* Fallback Location: Define where the goods are reserved if none of the removal rule could reserve
-  the goods. If left empty, the goods are reserved in the move's source location / sub-locations.
 * Rule Domain: The rule is used only if the Stock Move matches the domain.
 
 Removal rules for the locations:

stock_reserve_rule/models/stock_move.py

@@ -103,68 +103,16 @@ class StockMove(models.Model):
                     break
 
             reserved = need - still_need
-            if rule.fallback_location_id:
-                quants = self.env["stock.quant"]._gather(
-                    self.product_id,
-                    rule.fallback_location_id,
-                    lot_id=lot_id,
-                    package_id=forced_package_id,
-                    owner_id=owner_id,
-                    strict=strict,
-                )
-                fallback_quantity = sum(quants.mapped("quantity")) - sum(
-                    quants.mapped("reserved_quantity")
-                )
-                # If there is some qties to reserve in the fallback location,
-                # reserve them
-                reserved_fallback = super()._update_reserved_quantity(
-                    still_need,
-                    fallback_quantity,
-                    location_id=rule.fallback_location_id,
-                    lot_id=lot_id,
-                    package_id=package_id,
-                    owner_id=owner_id,
-                    strict=strict,
-                )
-                reserved += reserved_fallback
-                still_need = self.product_uom_qty - reserved
-                if still_need:
-                    if not reserved:
-                        # nothing could be reserved, however, we want to source
-                        # the move on the specific fallback location (for
-                        # replenishment), so update it's origin and return 0
-                        # reserved to leave the move confirmed
-                        self.location_id = rule.fallback_location_id
-                        return 0
-                    else:
-                        # Then if there is still a need, we split the current move to
-                        # get a new one targetting the fallback location with the
-                        # remaining qties for replenishment
-                        qty_split = self.product_uom._compute_quantity(
-                            still_need,
-                            self.product_id.uom_id,
-                            rounding_method="HALF-UP",
-                        )
-                        new_move_id = self._split(qty_split)
-                        new_move = self.browse(new_move_id)
-                        new_move.location_id = rule.fallback_location_id
-                        # Shunt the caller '_action_assign' by telling that all
-                        # the need has been reserved to get the current move
-                        # updated to the state 'assigned'
-                        return reserved + new_move.product_uom_qty
-                return reserved
-
-            else:
-                # Implicit fallback on the original location
-                return reserved + super()._update_reserved_quantity(
-                    still_need,
-                    available_quantity - reserved,
-                    location_id=location_id,
-                    lot_id=lot_id,
-                    package_id=package_id,
-                    owner_id=owner_id,
-                    strict=strict,
-                )
+            # Implicit fallback on the original location
+            return reserved + super()._update_reserved_quantity(
+                still_need,
+                available_quantity - reserved,
+                location_id=location_id,
+                lot_id=lot_id,
+                package_id=package_id,
+                owner_id=owner_id,
+                strict=strict,
+            )
 
         # We fall here if there is no rule or they have all been
         # excluded by 'rule._is_rule_applicable'

stock_reserve_rule/models/stock_reserve_rule.py

@@ -42,14 +42,6 @@ class StockReserveRule(models.Model):
     )
 
     location_id = fields.Many2one(comodel_name="stock.location", required=True)
-    fallback_location_id = fields.Many2one(
-        comodel_name="stock.location",
-        help="If all removal rules are exhausted, try to reserve in this "
-        "location. Use it for replenishment. The source location move will be "
-        "changed to this location if the move is not available. If the move is "
-        "partially available, it is split and the unavailable quantity is sourced "
-        "in this location for replenishment.",
-    )
 
     rule_removal_ids = fields.One2many(
         comodel_name="stock.reserve.rule.removal", inverse_name="rule_id"
@@ -62,25 +54,6 @@ class StockReserveRule(models.Model):
         "rule is applicable or not.",
     )
 
-    @api.constrains("fallback_location_id")
-    def _constraint_fallback_location_id(self):
-        """The fallback location has to be a child of the main location."""
-        location_model = self.env["stock.location"]
-        for rule in self:
-            if rule.fallback_location_id:
-                is_child = location_model.search_count(
-                    [
-                        ("id", "=", rule.fallback_location_id.id),
-                        ("id", "child_of", rule.location_id.id),
-                    ],
-                )
-                if not is_child:
-                    msg = _(
-                        "Fallback location has to be a child of the location '{}'."
-                    ).format(rule.location_id.display_name)
-                    _logger.error("Rule '%s' - %s", rule.name, msg)
-                    raise ValidationError(msg)
-
     def _rules_for_location(self, location):
         # We'll typically have a handful of rules, so reading all of them then
         # checking if they are a parent location of the location is pretty

stock_reserve_rule/readme/CONFIGURE.rst

@@ -5,12 +5,6 @@ Creation of a rule:
 Properties that define where the rule will be applied:
 
 * Location: Define where the rule will look for goods (a parent of the move's source location).
-* Fallback Location: Define where the goods are reserved if none of the removal
-  rule could reserve the goods. Use it for replenishment. The source location
-  move will be changed to this location if the move is not available. If the
-  move is partially available, it is split and the unavailable quantity is
-  sourced in this location for replenishment. If left empty, the goods are
-  reserved in the move's source location / sub-locations.
 * Rule Domain: The rule is used only if the Stock Move matches the domain.
 
 Removal rules for the locations:

stock_reserve_rule/static/description/index.html

@@ -429,8 +429,6 @@ Only for development or testing purpose, do not use in production.
 <p>Properties that define where the rule will be applied:</p>
 <ul class="simple">
 <li>Location: Define where the rule will look for goods (a parent of the move’s source location).</li>
-<li>Fallback Location: Define where the goods are reserved if none of the removal rule could reserve
-the goods. If left empty, the goods are reserved in the move’s source location / sub-locations.</li>
 <li>Rule Domain: The rule is used only if the Stock Move matches the domain.</li>
 </ul>
 <p>Removal rules for the locations:</p>

stock_reserve_rule/tests/test_reserve_rule.py

@@ -131,23 +131,6 @@ class TestReserveRule(common.SavepointCase):
             ]
         )
 
-    def test_rule_fallback_child_of_location(self):
-        # fallback is a child
-        self._create_rule(
-            {"fallback_location_id": self.loc_zone1.id},
-            [{"location_id": self.loc_zone1.id}],
-        )
-        # fallback is not a child
-        with self.assertRaises(exceptions.ValidationError):
-            self._create_rule(
-                {
-                    "fallback_location_id": self.env.ref(
-                        "stock.stock_location_locations"
-                    ).id
-                },
-                [{"location_id": self.loc_zone1.id}],
-            )
-
     def test_removal_rule_location_child_of_rule_location(self):
         # removal rule location is a child
         self._create_rule({}, [{"location_id": self.loc_zone1.id}])
@@ -157,82 +140,6 @@ class TestReserveRule(common.SavepointCase):
                 {}, [{"location_id": self.env.ref("stock.stock_location_locations").id}]
             )
 
-    def test_rule_fallback_partial_assign(self):
-        """Assign move partially available.
-
-        The move should be splitted in two:
-            - one move assigned with reserved goods
-            - one move for remaining goods targetting the fallback location
-        """
-        # Need 150 and 120 available => new move with 30 waiting qties
-        self._update_qty_in_location(self.loc_zone1_bin1, self.product1, 100)
-        self._update_qty_in_location(self.loc_zone2_bin1, self.product1, 20)
-        picking = self._create_picking(self.wh, [(self.product1, 150)])
-        fallback = self.loc_zone2_bin1
-        self._create_rule(
-            {"fallback_location_id": fallback.id},
-            [{"location_id": self.loc_zone1_bin1.id, "sequence": 1}],
-        )
-        self.assertEqual(len(picking.move_lines), 1)
-        picking.action_assign()
-        self.assertEqual(len(picking.move_lines), 2)
-        move_assigned = picking.move_lines.filtered(lambda m: m.state == "assigned")
-        move_unassigned = picking.move_lines.filtered(lambda m: m.state == "confirmed")
-        self.assertRecordValues(
-            move_assigned,
-            [
-                {
-                    "state": "assigned",
-                    "location_id": picking.location_id.id,
-                    "product_uom_qty": 120,
-                }
-            ],
-        )
-        self.assertRecordValues(
-            move_unassigned,
-            [{"state": "confirmed", "location_id": fallback.id, "product_uom_qty": 30}],
-        )
-
-    def test_rule_fallback_unavailable(self):
-        """Assign move unavailable.
-
-        The move source location should be changed to be the fallback location.
-        """
-        self._update_qty_in_location(self.loc_zone1_bin1, self.product1, 100)
-        picking = self._create_picking(self.wh, [(self.product1, 150)])
-        fallback = self.loc_zone2_bin1
-        self._create_rule(
-            {"fallback_location_id": fallback.id},
-            [
-                {
-                    "location_id": self.loc_zone1_bin1.id,
-                    "sequence": 1,
-                    # FIXME check if this isn't an issue?
-                    # for the test: StockQuant._get_available_quantity should
-                    # return something otherwise we won't enter in
-                    # StockMove._update_reserved_quantity
-                    # and the fallback will not be applied, so to reproduce a
-                    # case where the quants are not allowed to be taken,
-                    # use a domain that always resolves to false
-                    "quant_domain": [("id", "=", 0)],
-                }
-            ],
-        )
-        self.assertEqual(len(picking.move_lines), 1)
-        picking.action_assign()
-        self.assertEqual(len(picking.move_lines), 1)
-        move = picking.move_lines
-        self.assertRecordValues(
-            move,
-            [
-                {
-                    "state": "confirmed",
-                    "location_id": fallback.id,
-                    "product_uom_qty": 150,
-                }
-            ],
-        )
-
     def test_rule_take_all_in_2(self):
         all_locs = (
             self.loc_zone1_bin1,
@@ -324,41 +231,6 @@ class TestReserveRule(common.SavepointCase):
         self.assertEqual(move.state, "partially_available")
         self.assertEqual(move.reserved_availability, 300.0)
 
-    def test_rule_fallback(self):
-        reserve = self.env["stock.location"].create(
-            {"name": "Reserve", "location_id": self.wh.lot_stock_id.id}
-        )
-
-        self._update_qty_in_location(self.loc_zone1_bin1, self.product1, 100)
-        self._update_qty_in_location(self.loc_zone2_bin1, self.product1, 100)
-        self._update_qty_in_location(self.loc_zone3_bin1, self.product1, 100)
-        self._update_qty_in_location(reserve, self.product1, 300)
-        picking = self._create_picking(self.wh, [(self.product1, 400)])
-
-        self._create_rule(
-            {"fallback_location_id": reserve.id},
-            [
-                {"location_id": self.loc_zone1.id, "sequence": 3},
-                {"location_id": self.loc_zone2.id, "sequence": 1},
-                {"location_id": self.loc_zone3.id, "sequence": 2},
-            ],
-        )
-
-        picking.action_assign()
-        move = picking.move_lines
-        ml = move.move_line_ids
-        self.assertRecordValues(
-            ml,
-            [
-                {"location_id": self.loc_zone2_bin1.id, "product_qty": 100},
-                {"location_id": self.loc_zone3_bin1.id, "product_qty": 100},
-                {"location_id": self.loc_zone1_bin1.id, "product_qty": 100},
-                {"location_id": reserve.id, "product_qty": 100},
-            ],
-        )
-        self.assertEqual(move.state, "assigned")
-        self.assertEqual(move.reserved_availability, 400.0)
-
     def test_rule_domain(self):
         self._update_qty_in_location(self.loc_zone1_bin1, self.product1, 100)
         self._update_qty_in_location(self.loc_zone2_bin1, self.product1, 100)

stock_reserve_rule/views/stock_reserve_rule_views.xml

@@ -21,7 +21,6 @@
                     <group>
                         <field name="active" invisible="1" />
                         <field name="location_id" />
-                        <field name="fallback_location_id" />
                         <field name="sequence" />
                     </group>
                     <group>