Skip to content

Conversation

@adwid
Copy link
Contributor

@adwid adwid commented Aug 8, 2024

When scrapping a kit, it leads to incorrect behaviours

Case 01:

  1. Create a storable kit with one storable component
  2. Validate a scrap order with that kit
  3. Open the product moves

Error: we moved the kit instead of its component

We first create a draft SM with its SML:

move = self.env['stock.move'].create(scrap._prepare_move_values())

def _prepare_move_values(self):

During the SML creation, we check if we should recompute the state
of the SM:

move = move_line.move_id
if move:
reservation = not move._should_bypass_reservation()
else:
reservation = product.type == 'product' and not location.should_bypass_reservation()
if move_line.quantity and reservation:
self.env.context.get('reserved_quant', self.env['stock.quant'])._update_reserved_quantity(
product, location, move_line.quantity_product_uom, lot_id=move_line.lot_id, package_id=move_line.package_id, owner_id=move_line.owner_id)
if move:
move_to_recompute_state |= move
move_to_recompute_state._recompute_state()

reservation is True, the SML has a quantity -> we recompute the
state of the SM -> it is now assigned
Back to the scrap, we now _action_done the kit SM
move.with_context(is_scrap=True)._action_done()

which leads to
moves = self.filtered(
lambda move: move.state == 'draft'
or float_is_zero(move.product_uom_qty, precision_rounding=move.product_uom.rounding)
)._action_confirm(merge=False) # MRP allows scrapping draft moves

Here is the problem: the SM has a demand, its state is not draft
-> we don't confirm it
-> we don't explode it
Hence the error. This is the reason why the commit stops providing
the scrap SM with an initial demand. That way, we will explode the
SM and everything will work as expected

Case 02:

  1. Create a consumable kit with one storable component
  2. Validate a scrap order with that kit

Error: a server error is raised "Missing record [...]"

This time, reservation is False (the diff comes from the kit
type, consu vs stor, c.f. _should_bypass_reservation). Therefore,
we explode it. Since we are in scrap mode, we generate SM with a
zero demand:

for bom_line, line_data in lines:
if float_is_zero(move.product_uom_qty, precision_rounding=move.product_uom.rounding) or self.env.context.get('is_scrap'):
phantom_moves_vals_list += move._generate_move_phantom(bom_line, 0, line_data['qty'])

def _generate_move_phantom(self, bom_line, product_qty, quantity_done):

Back to _action_done, we create the extra moves if needed:
# Create extra moves where necessary
for move in moves:
if move.state == 'cancel' or (move.quantity <= 0 and not move.is_inventory):
continue
if not move.picked:
continue
moves_ids_todo |= move._create_extra_move().ids

Here, our component SM has a done qty greater than its demand
(reminder: it demand is zero), so we will create the extra move,
confirm it and merge it with the initial one:
extra_move_vals = self._prepare_extra_move_vals(extra_move_quantity)
self = self.with_context(avoid_putaway_rules=True, extra_move_mode=True)
extra_move = self.copy(default=extra_move_vals)
return extra_move.with_context(merge_extra=True, do_not_unreserve=True)._action_confirm(merge_into=self)

Buuuuut... Step 2 in the use case, we validate the scrap order.
Since we don't have such product on hand, we trigger a wizard with
some default values:
ctx.update({
'default_product_id': self.product_id.id,
'default_location_id': self.location_id.id,
'default_scrap_id': self.id,
'default_quantity': self.product_uom_id._compute_quantity(self.scrap_qty, self.product_id.uom_id),
'default_product_uom_name': self.product_id.uom_name
})
return {
'name': self.product_id.display_name + _(': Insufficient Quantity To Scrap'),
'view_mode': 'form',
'res_model': 'stock.warn.insufficient.qty.scrap',
'view_id': self.env.ref('stock.stock_warn_insufficient_qty_scrap_form_view').id,
'type': 'ir.actions.act_window',
'context': ctx,
'target': 'new'
}

And... Now you see where I'm going: when we create the extra move,
we still have these default values in the context -> the extra move
has a demand and(!) a done qty to 1. We merge it with the initial
move: we are now scrapping a component with a demand equal to one
and a done qty equal to 2. It will later lead to other
inconsistencies (among them, the server error raised)

opw-4090951

@robodoo
Copy link
Contributor

robodoo commented Aug 8, 2024

Pull request status dashboard

@C3POdoo C3POdoo requested a review from a team August 8, 2024 12:31
@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Aug 8, 2024
@Whenrow
Copy link
Contributor

Whenrow commented Aug 8, 2024

robodoo r+

@robodoo
Copy link
Contributor

robodoo commented Aug 8, 2024

@Whenrow you may want to rebuild or fix this PR as it has failed CI.

@adwid adwid force-pushed the 17.0-OPW-4090951-scrap_kit-awt branch 2 times, most recently from e5ede0e to ecb468c Compare August 9, 2024 10:46
@adwid
Copy link
Contributor Author

adwid commented Aug 9, 2024

robodoo r+

@robodoo
Copy link
Contributor

robodoo commented Aug 9, 2024

@adwid you can't review+. Skill issue.

@adwid
Copy link
Contributor Author

adwid commented Aug 9, 2024

@Whenrow
Copy link
Contributor

Whenrow commented Aug 9, 2024

robodoo r+

@adwid
Copy link
Contributor Author

adwid commented Aug 9, 2024

Trop aimable

@robodoo
Copy link
Contributor

robodoo commented Aug 9, 2024

@adwid @Whenrow 'ci/runbot' failed on this reviewed PR.

When scrapping a kit, it leads to incorrect behaviours

**Case 01:**
1. Create a storable kit with one storable component
2. Validate a scrap order with that kit
3. Open the product moves

Error: we moved the kit instead of its component

We first create a draft SM with its SML:
https://github.com/odoo/odoo/blob/72c4a2352c1184f5e8c1f238d29eb94e37d01115/addons/stock/models/stock_scrap.py#L145
https://github.com/odoo/odoo/blob/72c4a2352c1184f5e8c1f238d29eb94e37d01115/addons/stock/models/stock_scrap.py#L112

During the SML creation, we check if we should recompute the state
of the SM:
https://github.com/odoo/odoo/blob/dc0917d2a55a12e5c30d413a46e2c16190fd8a08/addons/stock/models/stock_move_line.py#L344-L355
`reservation` is `True`, the SML has a quantity -> we recompute the
state of the SM -> it is now assigned
Back to the scrap, we now `_action_done` the kit SM
https://github.com/odoo/odoo/blob/72c4a2352c1184f5e8c1f238d29eb94e37d01115/addons/stock/models/stock_scrap.py#L147
which leads to
https://github.com/odoo/odoo/blob/6ed0d0ca2f90fef8cd020380b194498a9363267c/addons/stock/models/stock_move.py#L1827-L1830
Here is the problem: the SM has a demand, its state is not draft
-> we don't confirm it
-> we don't explode it
Hence the error. This is the reason why the commit stops providing
the scrap SM with an initial demand. That way, we will explode the
SM and everything will work as expected

**Case 02:**
1. Create a consumable kit with one storable component
2. Validate a scrap order with that kit

Error: a server error is raised "Missing record [...]"

This time, `reservation` is `False` (the diff comes from the kit
type, consu vs stor, c.f. `_should_bypass_reservation`). Therefore,
we explode it. Since we are in scrap mode, we generate SM with a
zero demand:
https://github.com/odoo/odoo/blob/68f981d2a690addf0b70ddab498b556986752e49/addons/mrp/models/stock_move.py#L464-L466
https://github.com/odoo/odoo/blob/68f981d2a690addf0b70ddab498b556986752e49/addons/mrp/models/stock_move.py#L527
Back to `_action_done`, we create the extra moves if needed:
https://github.com/odoo/odoo/blob/6ed0d0ca2f90fef8cd020380b194498a9363267c/addons/stock/models/stock_move.py#L1847-L1853
Here, our component SM has a done qty greater than its demand
(reminder: it demand is zero), so we will create the extra move,
confirm it and merge it with the initial one:
https://github.com/odoo/odoo/blob/6ed0d0ca2f90fef8cd020380b194498a9363267c/addons/stock/models/stock_move.py#L1799-L1802
Buuuuut... Step 2 in the use case, we validate the scrap order.
Since we don't have such product on hand, we trigger a wizard with
some default values:
https://github.com/odoo/odoo/blob/72c4a2352c1184f5e8c1f238d29eb94e37d01115/addons/stock/models/stock_scrap.py#L204-L219
And... Now you see where I'm going: when we create the extra move,
we still have these default values in the context -> the extra move
has a demand and(!) a done qty to 1. We merge it with the initial
move: we are now scrapping a component with a demand equal to one
and a done qty equal to 2. It will later lead to other
inconsistencies (among them, the server error raised)

opw-4090951
@adwid adwid force-pushed the 17.0-OPW-4090951-scrap_kit-awt branch from ecb468c to 573680b Compare August 9, 2024 11:57
@adwid
Copy link
Contributor Author

adwid commented Aug 9, 2024

robodoo override=ci/style

@robodoo
Copy link
Contributor

robodoo commented Aug 9, 2024

@adwid you are not allowed to override 'ci/style'.

@adwid
Copy link
Contributor Author

adwid commented Aug 9, 2024

We will never be friends, damn bot

@Whenrow
Copy link
Contributor

Whenrow commented Aug 9, 2024

robodoo override=ci/style

absolutelly not @adwid disguised as @Whenrow

@robodoo
Copy link
Contributor

robodoo commented Aug 9, 2024

@Whenrow you are not allowed to override 'ci/style'.

@agr-odoo
Copy link
Contributor

agr-odoo commented Aug 9, 2024

let's roll
@robodoo override=ci/style

@robodoo
Copy link
Contributor

robodoo commented Aug 9, 2024

@agr-odoo you are not allowed to override 'ci/style'.

@d-fence
Copy link
Contributor

d-fence commented Aug 9, 2024

robodoo override=ci/style

@Whenrow
Copy link
Contributor

Whenrow commented Aug 9, 2024

robodoo r+

@adwid
Copy link
Contributor Author

adwid commented Aug 9, 2024

Odoo 18 will shine thanks to you both

Dasanchez1992 pushed a commit to Nosolotec/odoo that referenced this pull request Aug 22, 2024
When scrapping a kit, it leads to incorrect behaviours

**Case 01:**
1. Create a storable kit with one storable component
2. Validate a scrap order with that kit
3. Open the product moves

Error: we moved the kit instead of its component

We first create a draft SM with its SML:
https://github.com/odoo/odoo/blob/72c4a2352c1184f5e8c1f238d29eb94e37d01115/addons/stock/models/stock_scrap.py#L145
https://github.com/odoo/odoo/blob/72c4a2352c1184f5e8c1f238d29eb94e37d01115/addons/stock/models/stock_scrap.py#L112

During the SML creation, we check if we should recompute the state
of the SM:
https://github.com/odoo/odoo/blob/dc0917d2a55a12e5c30d413a46e2c16190fd8a08/addons/stock/models/stock_move_line.py#L344-L355
`reservation` is `True`, the SML has a quantity -> we recompute the
state of the SM -> it is now assigned
Back to the scrap, we now `_action_done` the kit SM
https://github.com/odoo/odoo/blob/72c4a2352c1184f5e8c1f238d29eb94e37d01115/addons/stock/models/stock_scrap.py#L147
which leads to
https://github.com/odoo/odoo/blob/6ed0d0ca2f90fef8cd020380b194498a9363267c/addons/stock/models/stock_move.py#L1827-L1830
Here is the problem: the SM has a demand, its state is not draft
-> we don't confirm it
-> we don't explode it
Hence the error. This is the reason why the commit stops providing
the scrap SM with an initial demand. That way, we will explode the
SM and everything will work as expected

**Case 02:**
1. Create a consumable kit with one storable component
2. Validate a scrap order with that kit

Error: a server error is raised "Missing record [...]"

This time, `reservation` is `False` (the diff comes from the kit
type, consu vs stor, c.f. `_should_bypass_reservation`). Therefore,
we explode it. Since we are in scrap mode, we generate SM with a
zero demand:
https://github.com/odoo/odoo/blob/68f981d2a690addf0b70ddab498b556986752e49/addons/mrp/models/stock_move.py#L464-L466
https://github.com/odoo/odoo/blob/68f981d2a690addf0b70ddab498b556986752e49/addons/mrp/models/stock_move.py#L527
Back to `_action_done`, we create the extra moves if needed:
https://github.com/odoo/odoo/blob/6ed0d0ca2f90fef8cd020380b194498a9363267c/addons/stock/models/stock_move.py#L1847-L1853
Here, our component SM has a done qty greater than its demand
(reminder: it demand is zero), so we will create the extra move,
confirm it and merge it with the initial one:
https://github.com/odoo/odoo/blob/6ed0d0ca2f90fef8cd020380b194498a9363267c/addons/stock/models/stock_move.py#L1799-L1802
Buuuuut... Step 2 in the use case, we validate the scrap order.
Since we don't have such product on hand, we trigger a wizard with
some default values:
https://github.com/odoo/odoo/blob/72c4a2352c1184f5e8c1f238d29eb94e37d01115/addons/stock/models/stock_scrap.py#L204-L219
And... Now you see where I'm going: when we create the extra move,
we still have these default values in the context -> the extra move
has a demand and(!) a done qty to 1. We merge it with the initial
move: we are now scrapping a component with a demand equal to one
and a done qty equal to 2. It will later lead to other
inconsistencies (among them, the server error raised)

opw-4090951

closes odoo#176186

Signed-off-by: William Henrotin (whe) <whe@odoo.com>
@fw-bot fw-bot deleted the 17.0-OPW-4090951-scrap_kit-awt branch August 23, 2024 16:46
riccardoSANEHO pushed a commit to resultrum/odoo that referenced this pull request Sep 30, 2024
When scrapping a kit, it leads to incorrect behaviours

**Case 01:**
1. Create a storable kit with one storable component
2. Validate a scrap order with that kit
3. Open the product moves

Error: we moved the kit instead of its component

We first create a draft SM with its SML:
https://github.com/odoo/odoo/blob/72c4a2352c1184f5e8c1f238d29eb94e37d01115/addons/stock/models/stock_scrap.py#L145
https://github.com/odoo/odoo/blob/72c4a2352c1184f5e8c1f238d29eb94e37d01115/addons/stock/models/stock_scrap.py#L112

During the SML creation, we check if we should recompute the state
of the SM:
https://github.com/odoo/odoo/blob/dc0917d2a55a12e5c30d413a46e2c16190fd8a08/addons/stock/models/stock_move_line.py#L344-L355
`reservation` is `True`, the SML has a quantity -> we recompute the
state of the SM -> it is now assigned
Back to the scrap, we now `_action_done` the kit SM
https://github.com/odoo/odoo/blob/72c4a2352c1184f5e8c1f238d29eb94e37d01115/addons/stock/models/stock_scrap.py#L147
which leads to
https://github.com/odoo/odoo/blob/6ed0d0ca2f90fef8cd020380b194498a9363267c/addons/stock/models/stock_move.py#L1827-L1830
Here is the problem: the SM has a demand, its state is not draft
-> we don't confirm it
-> we don't explode it
Hence the error. This is the reason why the commit stops providing
the scrap SM with an initial demand. That way, we will explode the
SM and everything will work as expected

**Case 02:**
1. Create a consumable kit with one storable component
2. Validate a scrap order with that kit

Error: a server error is raised "Missing record [...]"

This time, `reservation` is `False` (the diff comes from the kit
type, consu vs stor, c.f. `_should_bypass_reservation`). Therefore,
we explode it. Since we are in scrap mode, we generate SM with a
zero demand:
https://github.com/odoo/odoo/blob/68f981d2a690addf0b70ddab498b556986752e49/addons/mrp/models/stock_move.py#L464-L466
https://github.com/odoo/odoo/blob/68f981d2a690addf0b70ddab498b556986752e49/addons/mrp/models/stock_move.py#L527
Back to `_action_done`, we create the extra moves if needed:
https://github.com/odoo/odoo/blob/6ed0d0ca2f90fef8cd020380b194498a9363267c/addons/stock/models/stock_move.py#L1847-L1853
Here, our component SM has a done qty greater than its demand
(reminder: it demand is zero), so we will create the extra move,
confirm it and merge it with the initial one:
https://github.com/odoo/odoo/blob/6ed0d0ca2f90fef8cd020380b194498a9363267c/addons/stock/models/stock_move.py#L1799-L1802
Buuuuut... Step 2 in the use case, we validate the scrap order.
Since we don't have such product on hand, we trigger a wizard with
some default values:
https://github.com/odoo/odoo/blob/72c4a2352c1184f5e8c1f238d29eb94e37d01115/addons/stock/models/stock_scrap.py#L204-L219
And... Now you see where I'm going: when we create the extra move,
we still have these default values in the context -> the extra move
has a demand and(!) a done qty to 1. We merge it with the initial
move: we are now scrapping a component with a demand equal to one
and a done qty equal to 2. It will later lead to other
inconsistencies (among them, the server error raised)

opw-4090951

closes odoo#176186

Signed-off-by: William Henrotin (whe) <whe@odoo.com>
kawkb added a commit to odoo-dev/odoo that referenced this pull request Dec 5, 2024
Issue:
    In 17.0 and saas-17.2, The Cost of Scrap section does not appear
    in the the Cost Analysis Report despite having scrapped some components
    during manufacturing.

Steps to reproduce:
    1. Create a product with a BoM
    2. Manufacture the product
    3. Scrap some components during manufacturing
    4. print the Cost Analysis Report
    5. Notice the cost of scrap section does not appear in the report.

Solution:
    - In v17.0, the Cost Analysis Report fails to include scrap costs
    because `production_id` is not assigned to scrap stock moves.

    -This issue was introduced by changes in PR odoo#176186,
    which cleared the context, preventing production_id from being set.

    -This commit sets the production_id on scrap stock moves

opw-4183097
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OE the report is linked to a support ticket (opw-...)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants