pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] mrp_subcontracting_dropshipping,*: dropship valued product

Browse Source 
*:purchase_stock,stock_account

To reproduce the issue:
1. In Settings, enable 'Multi-Step Routes'
2. Create a product category PC:
   - Method: FIFO
   - Valuation: Auto
3. Create two products P_compo, P_finished
   - Both:
     - Type: Storable
     - Category: PC
     - With a positive cost
   - P_finished:
     - Vendors: a new vendor V
4. Create and confirm a SO with one order line:
   - Product: P_finished
   - Route: Dropship
5. Confirm the PO and process the dropship
6. Accounting > Accounting > Miscellaneous > Journal Items

Error: There are a lot of lines and most are incorrect

When marking the dropship SM as done, we check how we should
value it:
https://github.com/odoo/odoo/blob/18450973eaf3ca048cdcd2046a8cba5f5c0f98b0/addons/stock_account/models/stock_move.py#L244-L252
Here is one issue: `_is_dropshipped` returns `True`, which is
correct because the source is the subcontractor location and the
destination a customer:
https://github.com/odoo/odoo/blob/8276a9b51b7c5fc0ae6dd557ae88a9c31f1d1962/addons/mrp_subcontracting_dropshipping/models/stock_move.py#L14-L20
However, `_is_out` will return `True` too. To get this value, we
check if the source and destination locations have to be valued:
https://github.com/odoo/odoo/blob/18450973eaf3ca048cdcd2046a8cba5f5c0f98b0/addons/stock_account/models/stock_move.py#L99-L100
Again, since the source location is the subcontractor one (which is
an internal one) and the destination location is the customer one,
we will consider that we have to value this SM. As a result, we will
apply `_create_out_svl` and `_create_dropshipped_svl` on the same SM
(this explains some incorrect AMLs are generated).

Then, there is another issue. In `_create_dropshipped_svl`, we
create an in-SVL and an out-SVL for P_finished. However, when
processing the MO, an in-SVL will also be generated for P_finished.
The issue comes from `_create_dropshipped_svl`: it creates an in-SVL
but the source location is not outside from the company, so it
should not create the incoming layer.

Finally, when creating the SVL in `_create_dropshipped_svl`, we use
the unit price of the SM to define the value of the SVL. This is an
issue: suppose the unit price of the POL is $10 and the costs of
P_compo is $1, the unit price of the SM is then $10 (and so does the
value of the SVL) but the actual value of the delivered P_finished
is $11. Howver, when calling `_action_done` on a SM, we first
process it and only then we will process the MO:
https://github.com/odoo/odoo/blob/829369d3ca0530f1aad0599fbb1598810a928cc3/addons/mrp_subcontracting/models/stock_picking.py#L33-L36
So, in `_create_dropshipped_svl`, we can't know the actual cost of
P_finished. This is the reason why we have to create a compensation
layer once the MO is processed (i.e. in the override from
`mrp_subcontracting_dropshipping`)

Similar issues will happen if we return the
dropshipped/subcontracted product.

OPW-3078499

closes odoo/odoo#110390

X-original-commit: 200aac56771fd1ef759f73b277a6808d96ee5f31
Signed-off-by: William Henrotin (whe) <whe@odoo.com>
Signed-off-by: Adrien Widart <awt@odoo.com>
This commit is contained in:
Adrien Widart (awt)
2023-01-19 14:01:27 +01:00
parent 39d2723b84
commit e39af99476
5 changed files with 190 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mrp_subcontracting_dropshipping/models/stock_picking.py
+32
View File
@@ -2,6 +2,7 @@
# Part of Odoo. See LICENSE file for full copyright and licensing details.
from odoo import models
from odoo.tools import float_compare
class StockPicking(models.Model):
@@ -20,6 +21,37 @@ class StockPicking(models.Model):
def _action_done(self):
res = super()._action_done()
self.move_ids.move_dest_ids._action_assign()
# If needed, create a compensation layer, so we add the MO cost to the dropship one
svls = self.env['stock.valuation.layer']
for move in self.move_ids:
if not (move.is_subcontract and move._is_dropshipped() and move.state == 'done'):
continue
dropship_svls = move.stock_valuation_layer_ids
if not dropship_svls:
continue
subcontract_svls = move.move_orig_ids.stock_valuation_layer_ids
subcontract_value = sum(subcontract_svls.mapped('value'))
dropship_value = abs(sum(dropship_svls.mapped('value')))
diff = subcontract_value - dropship_value
if float_compare(diff, 0, precision_rounding=move.company_id.currency_id.rounding) <= 0:
continue
svl_vals = move._prepare_common_svl_vals()
svl_vals.update({
'remaining_value': 0,
'remaining_qty': 0,
'value': -diff,
'quantity': 0,
'unit_cost': 0,
'stock_valuation_layer_id': dropship_svls[0].id,
'stock_move_id': move.id,
})
svls |= self.env['stock.valuation.layer'].create(svl_vals)
svls._validate_accounting_entries()
return res
def _prepare_subcontract_mo_vals(self, subcontract_move, bom):
addons/mrp_subcontracting_dropshipping/tests/__init__.py
+1
View File
@@ -3,3 +3,4 @@
from . import test_purchase_subcontracting
from . import test_sale_dropshipping
from . import test_anglo_saxon_valuation
addons/mrp_subcontracting_dropshipping/tests/test_anglo_saxon_valuation.py
+134
View File
@@ -0,0 +1,134 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.
from odoo.addons.stock_account.tests.test_anglo_saxon_valuation_reconciliation_common import ValuationReconciliationTestCommon
from odoo.tests import tagged, Form
@tagged('post_install', '-at_install')
class TestSubcontractingDropshippingValuation(ValuationReconciliationTestCommon):
@classmethod
def setUpClass(cls, chart_template_ref=None):
super().setUpClass(chart_template_ref=chart_template_ref)
categ_form = Form(cls.env['product.category'])
categ_form.name = 'fifo auto'
categ_form.parent_id = cls.env.ref('product.product_category_all')
categ_form.property_cost_method = 'fifo'
categ_form.property_valuation = 'real_time'
cls.categ_fifo_auto = categ_form.save()
(cls.product_a | cls.product_b).type = 'product'
cls.bom_a = cls.env['mrp.bom'].create({
'product_tmpl_id': cls.product_a.product_tmpl_id.id,
'type': 'subcontract',
'subcontractor_ids': [(6, 0, cls.partner_a.ids)],
'bom_line_ids': [
(0, 0, {'product_id': cls.product_b.id, 'product_qty': 1.0}),
],
})
def test_valuation_subcontracted_and_dropshipped(self):
"""
Product:
- FIFO + Auto
- Subcontracted
Purchase 2 from Subcontractor to a customer (dropship).
Then return 1 to subcontractor and one to stock
It should generate the correct valuations AMLs
"""
# pylint: disable=bad-whitespace
all_amls_ids = self.env['account.move.line'].search_read([], ['id'])
grp_multi_loc = self.env.ref('stock.group_stock_multi_locations')
self.env.user.write({'groups_id': [(4, grp_multi_loc.id)]})
(self.product_a | self.product_b).categ_id = self.categ_fifo_auto
self.product_b.standard_price = 10
dropship_picking_type = self.env['stock.picking.type'].search([
('company_id', '=', self.env.company.id),
('default_location_src_id.usage', '=', 'supplier'),
('default_location_dest_id.usage', '=', 'customer'),
], limit=1, order='sequence')
po = self.env['purchase.order'].create({
"partner_id": self.partner_a.id,
"picking_type_id": dropship_picking_type.id,
"dest_address_id": self.partner_b.id,
"order_line": [(0, 0, {
'product_id': self.product_a.id,
'name': self.product_a.name,
'product_qty': 2.0,
'price_unit': 100,
'taxes_id': False,
})],
})
po.button_confirm()
delivery = po.picking_ids
res = delivery.button_validate()
Form(self.env['stock.immediate.transfer'].with_context(res['context'])).save().process()
stock_in_acc_id = self.categ_fifo_auto.property_stock_account_input_categ_id.id
stock_out_acc_id = self.categ_fifo_auto.property_stock_account_output_categ_id.id
stock_valu_acc_id = self.categ_fifo_auto.property_stock_valuation_account_id.id
amls = self.env['account.move.line'].search([('id', 'not in', all_amls_ids)])
all_amls_ids += amls.ids
self.assertRecordValues(amls, [
# Compensation of dropshipping value
{'account_id': stock_valu_acc_id, 'product_id': self.product_a.id, 'debit': 0.0, 'credit': 20.0},
{'account_id': stock_out_acc_id, 'product_id': self.product_a.id, 'debit': 20.0, 'credit': 0.0},
# Receipt from subcontractor
{'account_id': stock_in_acc_id, 'product_id': self.product_a.id, 'debit': 0.0, 'credit': 220.0},
{'account_id': stock_valu_acc_id, 'product_id': self.product_a.id, 'debit': 220.0, 'credit': 0.0},
# Delivery to subcontractor
{'account_id': stock_valu_acc_id, 'product_id': self.product_b.id, 'debit': 0.0, 'credit': 20.0},
{'account_id': stock_out_acc_id, 'product_id': self.product_b.id, 'debit': 20.0, 'credit': 0.0},
# Initial dropshipped value
{'account_id': stock_valu_acc_id, 'product_id': self.product_a.id, 'debit': 0.0, 'credit': 200.0},
{'account_id': stock_out_acc_id, 'product_id': self.product_a.id, 'debit': 200.0, 'credit': 0.0},
])
# return to subcontracting location
sbc_location = self.env.company.subcontracting_location_id
return_form = Form(self.env['stock.return.picking'].with_context(active_id=delivery.id, active_model='stock.picking'))
return_form.location_id = sbc_location
with return_form.product_return_moves.edit(0) as line:
line.quantity = 1
return_wizard = return_form.save()
return_id, _ = return_wizard._create_returns()
return_picking = self.env['stock.picking'].browse(return_id)
return_picking.move_ids.quantity_done = 1
return_picking.button_validate()
amls = self.env['account.move.line'].search([('id', 'not in', all_amls_ids)])
all_amls_ids += amls.ids
self.assertRecordValues(amls, [
{'account_id': stock_valu_acc_id, 'product_id': self.product_a.id, 'debit': 0.0, 'credit': 110.0},
{'account_id': stock_in_acc_id, 'product_id': self.product_a.id, 'debit': 110.0, 'credit': 0.0},
])
# return to stock location
warehouse = self.env['stock.warehouse'].search([('company_id', '=', self.env.company.id)], limit=1)
stock_location = warehouse.lot_stock_id
stock_location.return_location = True
return_form = Form(self.env['stock.return.picking'].with_context(active_id=delivery.id, active_model='stock.picking'))
return_form.location_id = stock_location
with return_form.product_return_moves.edit(0) as line:
line.quantity = 1
return_wizard = return_form.save()
return_id, _ = return_wizard._create_returns()
return_picking = self.env['stock.picking'].browse(return_id)
return_picking.move_ids.quantity_done = 1
return_picking.button_validate()
amls = self.env['account.move.line'].search([('id', 'not in', all_amls_ids)])
all_amls_ids += amls.ids
self.assertRecordValues(amls, [
{'account_id': stock_out_acc_id, 'product_id': self.product_a.id, 'debit': 0.0, 'credit': 110.0},
{'account_id': stock_valu_acc_id, 'product_id': self.product_a.id, 'debit': 110.0, 'credit': 0.0},
])
addons/purchase_stock/models/stock_move.py
+1 -1
View File
@@ -34,7 +34,7 @@ class StockMove(models.Model):
def _get_price_unit(self):
""" Returns the unit price for the move"""
self.ensure_one()
if not self.purchase_line_id or not self.product_id.id:
if self.origin_returned_move_id or not self.purchase_line_id or not self.product_id.id:
return super(StockMove, self)._get_price_unit()
price_unit_prec = self.env['decimal.precision'].precision_get('Product Price')
line = self.purchase_line_id
addons/stock_account/models/stock_move.py
+22 -18
View File
@@ -46,6 +46,7 @@ class StockMove(models.Model):
# If the move is a return, use the original move's price unit.
if self.origin_returned_move_id and self.origin_returned_move_id.sudo().stock_valuation_layer_ids:
layers = self.origin_returned_move_id.sudo().stock_valuation_layer_ids
layers |= layers.stock_valuation_layer_ids
quantity = sum(layers.mapped("quantity"))
return layers.currency_id.round(sum(layers.mapped("value")) / quantity) if not float_is_zero(quantity, precision_rounding=layers.uom_id.rounding) else 0
return price_unit if not float_is_zero(price_unit, precision) or self._should_force_price_unit() else self.product_id.standard_price
@@ -86,7 +87,7 @@ class StockMove(models.Model):
:rtype: bool
"""
self.ensure_one()
if self._get_in_move_lines():
if self._get_in_move_lines() and not self._is_dropshipped_returned():
return True
return False
@@ -114,7 +115,7 @@ class StockMove(models.Model):
:rtype: bool
"""
self.ensure_one()
if self._get_out_move_lines():
if self._get_out_move_lines() and not self._is_dropshipped():
return True
return False
@@ -206,23 +207,26 @@ class StockMove(models.Model):
common_vals = dict(move._prepare_common_svl_vals(), remaining_qty=0)
# create the in
in_vals = {
'unit_cost': unit_cost,
'value': unit_cost * quantity,
'quantity': quantity,
}
in_vals.update(common_vals)
svl_vals_list.append(in_vals)
# create the in if it does not come from a valued location (eg subcontract -> customer)
if not move.location_id._should_be_valued():
in_vals = {
'unit_cost': unit_cost,
'value': unit_cost * quantity,
'quantity': quantity,
}
in_vals.update(common_vals)
svl_vals_list.append(in_vals)
# create the out if it does not go to a valued location (eg customer -> subcontract)
if not move.location_dest_id._should_be_valued():
out_vals = {
'unit_cost': unit_cost,
'value': unit_cost * quantity * -1,
'quantity': quantity * -1,
}
out_vals.update(common_vals)
svl_vals_list.append(out_vals)
# create the out
out_vals = {
'unit_cost': unit_cost,
'value': unit_cost * quantity * -1,
'quantity': quantity * -1,
}
out_vals.update(common_vals)
svl_vals_list.append(out_vals)
return self.env['stock.valuation.layer'].sudo().create(svl_vals_list)
def _create_dropshipped_returned_svl(self, forced_quantity=None):