audrius/stock-logistics-warehouse
1
0
Fork 1
mirror of https://github.com/OCA/stock-logistics-warehouse.git synced 2025-01-21 14:27:28 +02:00
Code Issues Packages Projects Releases Wiki Activity

Fix bug in fallback when no quantity could be reserved

Browse Source 
Before the change, the implementation of the fallback goes like this:

If I reserve a move of 3000 and it finds 600 units, it splits the move
to create a new move of 2400 and pretend to the caller that 3000 was
reserved so the initial move is changed to 'assigned'.

Now, if we have a move of 2400 and finds zero, it still splits the move,
and pretend to the caller that 2400 was reserved → the initial move has
no move line but is assigned. In this case, we should not split the move
but only update the source location of the move.
This commit is contained in:
Guewen Baconnier
2020-05-14 14:27:20 +02:00
committed by Sébastien Alix
parent 42d9c733f0
commit 3d14192088
5 changed files with 112 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
stock_reserve_rule/models/stock_move.py
View File

@@ -114,24 +114,33 @@ class StockMove(models.Model):
owner_id=owner_id,
strict=strict,
)
# 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
still_need = self.product_uom_qty - self.reserved_availability
reserved += reserved_fallback
still_need = self.product_uom_qty - reserved
if still_need:
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 + reserved_fallback + new_move.product_uom_qty
return reserved + reserved_fallback
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

17
stock_reserve_rule/models/stock_reserve_rule.py
View File

@@ -3,10 +3,10 @@
import logging
from odoo import _, api, fields, models
from odoo.exceptions import ValidationError
from odoo.osv import expression
from odoo.tools.float_utils import float_compare
from odoo.tools.safe_eval import safe_eval
from odoo.exceptions import ValidationError
_logger = logging.getLogger(__name__)
@@ -45,8 +45,10 @@ class StockReserveRule(models.Model):
fallback_location_id = fields.Many2one(
comodel_name="stock.location",
help="If all removal rules are exhausted, try to reserve in this "
"location. When empty, the fallback happens in any of the move's "
"source sub-locations.",
"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(
@@ -69,7 +71,8 @@ class StockReserveRule(models.Model):
def _compute_missing_reserve_rule(self):
for rule in self:
rule.missing_reserve_rule = any(
rule.rule_removal_ids.mapped("missing_reserve_rule"))
rule.rule_removal_ids.mapped("missing_reserve_rule")
)
@api.constrains("fallback_location_id")
def _constraint_fallback_location_id(self):
@@ -85,8 +88,7 @@ class StockReserveRule(models.Model):
)
if not is_child:
msg = _(
"Fallback location has to be a child of the "
"location '{}'."
"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)
@@ -212,8 +214,7 @@ class StockReserveRuleRemoval(models.Model):
"Removal rule '{}' location has to be a child "
"of the rule location '{}'."
).format(
removal_rule.name,
removal_rule.rule_id.location_id.display_name,
removal_rule.name, removal_rule.rule_id.location_id.display_name,
)
_logger.error("Rule '%s' - %s", removal_rule.rule_id.name, msg)
raise ValidationError(msg)

8
stock_reserve_rule/readme/CONFIGURE.rst
View File

@@ -5,8 +5,12 @@ 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.
* 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:

91
stock_reserve_rule/tests/test_reserve_rule.py
View File

@@ -1,7 +1,7 @@
# Copyright 2019 Camptocamp (https://www.camptocamp.com)
from odoo.tests import common
from odoo import exceptions
from odoo.tests import common
class TestReserveRule(common.SavepointCase):
@@ -134,39 +134,27 @@ 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_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,
"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}],
)
self._create_rule({}, [{"location_id": self.loc_zone1.id}])
# removal rule location is not a child
with self.assertRaises(exceptions.ValidationError):
self._create_rule(
{},
[
{
"location_id": self.env.ref(
"stock.stock_location_locations").id,
},
],
{}, [{"location_id": self.env.ref("stock.stock_location_locations").id}]
)
def test_rule_fallback_partial_assign(self):
@@ -180,23 +168,64 @@ class TestReserveRule(common.SavepointCase):
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": self.loc_zone2_bin1.id,
},
[
{"location_id": self.loc_zone1_bin1.id, "sequence": 1},
],
{"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.assertEqual(move_assigned.state, "assigned")
self.assertEqual(move_unassigned.state, "confirmed")
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 = (

16
stock_reserve_rule/views/stock_reserve_rule_views.xml
View File

@@ -35,12 +35,15 @@
</group>
<group string="Removal Rules" name="rule" col="1">
<field name="rule_removal_ids" nolabel="1">
<tree string="Removal Rules" decoration-warning="missing_reserve_rule">
<tree
string="Removal Rules"
decoration-warning="missing_reserve_rule"
>
<field name="sequence" widget="handle" />
<field name="name" />
<field name="location_id" />
<field name="removal_strategy" />
<field name="missing_reserve_rule" invisible="1"/>
<field name="missing_reserve_rule" invisible="1" />
</tree>
<form string="Removal Rule">
<group>
@@ -63,8 +66,11 @@
</group>
</form>
</field>
<field name="missing_reserve_rule" invisible="1"/>
<strong attrs="{'invisible': [('missing_reserve_rule', '=', False)]}" style="color:red;">
<field name="missing_reserve_rule" invisible="1" />
<strong
attrs="{'invisible': [('missing_reserve_rule', '=', False)]}"
style="color:red;"
>
WARNING: some of the removal rules are missing a reservation rule with a lower sequence. If it is desirable you should create them to ensure that the reservation on the related locations will use these rules first instead of this one.
</strong>
</group>
@@ -95,7 +101,7 @@
<field name="sequence" widget="handle" />
<field name="name" />
<field name="location_id" />
<field name="rule_domain"/>
<field name="rule_domain" />
</tree>
</field>
</record>