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

[FIX] stock_account, sale_stock: reverse AML using the returned SM value

Browse Source 
When reversing an invoice linked to a sale order, the value of the new
anglo saxon AML should be based on the value of the returned product

To reproduce the issue:
(Need account_accountant,purchase)
1. Create a product category PC:
    - Costing Method: FIFO
    - Inventory Valuation: Automated
2. Create a product P:
    - Type: Storable
    - Product Category: PC
3. For C in [10, 20, 30]:
    - Create a purchase order with 1 x P at cost C
    - Receive P
4. Create a SO with 3 x P
5. Deliver the products one by one
6. Create and Post the invoice INV linked to SO
7. Return the second delivery
8. Add a credit note to INV01:
    - Credit Method: Partial Refund
9. Set the quantity to 1
10. Post the new invoice INV02

Error: The anglo saxon lines are listed in the journal items, which is
correct, but their value is $30 while it should be $20 (i.e., the value
of the returned product)

When delivering the last product, its standard price is updated with the
new value ($30):
https://github.com/odoo/odoo/blob/6b96ed418cb626678f4fa5baec25a903d7a74eda/addons/stock_account/models/product.py#L305-L307
Later on, when posting the credit note, the module gets the anglo saxon
unit price (`_stock_account_get_anglo_saxon_price_unit`). However, it
does not consider that the current account move line is reversing
another one. In the reversing process, the computation of the invoiced
quantity should be based on the invoices lines that are reversing too.
Moreover, when computing the average unit price of the returned product,
the module should use the stock moves that are returning the product.
In the use case, because of the two issued noted above,
`_compute_average_price` does not find any stock valuation layer to
compute the average unit price and uses the fallback, i.e. the standard
price of the product:
https://github.com/odoo/odoo/blob/6b96ed418cb626678f4fa5baec25a903d7a74eda/addons/stock_account/models/product.py#L652-L656
That's the reason why, in the above case, the value is $30 instead of
$20.

Note: a similar use case can be reproduced with a kit

OPW-2646926
OPW-2628215

closes odoo/odoo#82978

X-original-commit: 524d0d5e8817e1c7bcc99204c4cbb90fcfb6074c
Signed-off-by: William Henrotin (whe) <whe@odoo.com>
Signed-off-by: Adrien Widart <awt@odoo.com>
This commit is contained in:
Adrien Widart
2022-01-18 15:25:51 +00:00
parent 483609c479
commit 87dc2bc614
6 changed files with 227 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mrp_account/models/product.py
+4 -4
View File
@@ -46,13 +46,13 @@ class ProductProduct(models.Model):
if price:
self.standard_price = price
def _compute_average_price(self, qty_invoiced, qty_to_invoice, stock_moves):
def _compute_average_price(self, qty_invoiced, qty_to_invoice, stock_moves, is_returned=False):
self.ensure_one()
if stock_moves.product_id == self:
return super()._compute_average_price(qty_invoiced, qty_to_invoice, stock_moves)
return super()._compute_average_price(qty_invoiced, qty_to_invoice, stock_moves, is_returned=is_returned)
bom = self.env['mrp.bom']._bom_find(self, company_id=stock_moves.company_id.id, bom_type='phantom')[self]
if not bom:
return super()._compute_average_price(qty_invoiced, qty_to_invoice, stock_moves)
return super()._compute_average_price(qty_invoiced, qty_to_invoice, stock_moves, is_returned=is_returned)
dummy, bom_lines = bom.explode(self, 1)
bom_lines = {line: data for line, data in bom_lines}
value = 0
@@ -66,7 +66,7 @@ class ProductProduct(models.Model):
else:
# bom was altered (i.e. bom line removed) after being used
line_qty = move.product_qty
value += line_qty * move.product_id._compute_average_price(qty_invoiced * line_qty, qty_to_invoice * line_qty, move)
value += line_qty * move.product_id._compute_average_price(qty_invoiced * line_qty, qty_to_invoice * line_qty, move, is_returned=is_returned)
return value
def _compute_bom_price(self, bom, boms_to_recompute=False, byproduct_bom=False):
addons/sale_mrp/models/account_move.py
+5 -2
View File
@@ -15,8 +15,10 @@ class AccountMoveLine(models.Model):
lambda b: not b.company_id or b.company_id == so_line.company_id
)[:1]
if bom and bom.type == 'phantom':
is_line_reversing = bool(self.move_id.reversed_entry_id)
qty_to_invoice = self.product_uom_id._compute_quantity(self.quantity, self.product_id.uom_id)
qty_invoiced = sum([x.product_uom_id._compute_quantity(x.quantity, x.product_id.uom_id) for x in so_line.invoice_lines if x.move_id.state == 'posted'])
posted_invoice_lines = so_line.invoice_lines.filtered(lambda l: l.move_id.state == 'posted' and bool(l.move_id.reversed_entry_id) == is_line_reversing)
qty_invoiced = sum([x.product_uom_id._compute_quantity(x.quantity, x.product_id.uom_id) for x in posted_invoice_lines])
moves = so_line.move_ids
average_price_unit = 0
components_qty = so_line._get_bom_component_qty(bom)
@@ -26,7 +28,8 @@ class AccountMoveLine(models.Model):
prod_moves = moves.filtered(lambda m: m.product_id == product)
prod_qty_invoiced = factor * qty_invoiced
prod_qty_to_invoice = factor * qty_to_invoice
average_price_unit += factor * product.with_company(self.company_id)._compute_average_price(prod_qty_invoiced, prod_qty_to_invoice, prod_moves)
product = product.with_company(self.company_id)
average_price_unit += factor * product._compute_average_price(prod_qty_invoiced, prod_qty_to_invoice, prod_moves, is_returned=is_line_reversing)
price_unit = average_price_unit / bom.product_qty or price_unit
price_unit = self.product_id.uom_id._compute_price(price_unit, self.product_uom_id)
return price_unit
addons/sale_mrp/tests/test_sale_mrp_flow.py
+115
View File
@@ -5,6 +5,7 @@ from odoo.addons.stock_account.tests.test_anglo_saxon_valuation_reconciliation_c
from odoo.tests import common, Form
from odoo.exceptions import UserError
from odoo.tools import mute_logger, float_compare
from odoo.addons.stock_account.tests.test_stockvaluation import _create_accounting_data
# these tests create accounting entries, and therefore need a chart of accounts
@@ -1926,3 +1927,117 @@ class TestSaleMrpFlow(ValuationReconciliationTestCommon):
so.action_draft()
so.action_confirm()
self.assertEqual(len(so.picking_ids), 1, "The product was already delivered, no need to re-create a delivery order")
def test_anglo_saxo_return_and_credit_note(self):
"""
When posting a credit note for a returned kit, the value of the anglo-saxo lines
should be based on the returned component's value
"""
stock_input_account, stock_output_account, stock_valuation_account, expense_account, stock_journal = _create_accounting_data(self.env)
fifo = self.env['product.category'].create({
'name': 'FIFO',
'property_valuation': 'real_time',
'property_cost_method': 'fifo',
'property_stock_account_input_categ_id': stock_input_account.id,
'property_stock_account_output_categ_id': stock_output_account.id,
'property_stock_valuation_account_id': stock_valuation_account.id,
'property_stock_journal': stock_journal.id,
})
kit = self._create_product('Simple Kit', self.uom_unit)
(kit + self.component_a).categ_id = fifo
kit.property_account_expense_id = expense_account
self.env['mrp.bom'].create({
'product_tmpl_id': kit.product_tmpl_id.id,
'product_qty': 1.0,
'type': 'phantom',
'bom_line_ids': [(0, 0, {'product_id': self.component_a.id, 'product_qty': 1.0})]
})
# Receive 3 components: one @10, one @20 and one @60
in_moves = self.env['stock.move'].create([{
'name': 'IN move @%s' % p,
'product_id': self.component_a.id,
'location_id': self.env.ref('stock.stock_location_suppliers').id,
'location_dest_id': self.company_data['default_warehouse'].lot_stock_id.id,
'product_uom': self.component_a.uom_id.id,
'product_uom_qty': 1,
'price_unit': p,
} for p in [10, 20, 60]])
in_moves._action_confirm()
in_moves.quantity_done = 1
in_moves._action_done()
# Sell 3 kits
so = self.env['sale.order'].create({
'partner_id': self.env.ref('base.res_partner_1').id,
'order_line': [
(0, 0, {
'name': kit.name,
'product_id': kit.id,
'product_uom_qty': 3.0,
'product_uom': kit.uom_id.id,
'price_unit': 100,
'tax_id': False,
})],
})
so.action_confirm()
# Deliver the components: 1@10, then 1@20 and then 1@60
pickings = []
picking = so.picking_ids
while picking:
pickings.append(picking)
picking.move_ids.quantity_done = 1
action = picking.button_validate()
if isinstance(action, dict):
wizard = Form(self.env[action['res_model']].with_context(action['context'])).save()
wizard.process()
picking = picking.backorder_ids
invoice = so._create_invoices()
invoice.action_post()
# Receive one @100
in_moves = self.env['stock.move'].create({
'name': 'IN move @100',
'product_id': self.component_a.id,
'location_id': self.env.ref('stock.stock_location_suppliers').id,
'location_dest_id': self.company_data['default_warehouse'].lot_stock_id.id,
'product_uom': self.component_a.uom_id.id,
'product_uom_qty': 1,
'price_unit': 100,
})
in_moves._action_confirm()
in_moves.quantity_done = 1
in_moves._action_done()
# Return the second picking (i.e. one component @20)
ctx = {'active_id': pickings[1].id, 'active_model': 'stock.picking'}
return_wizard = Form(self.env['stock.return.picking'].with_context(ctx)).save()
return_picking_id, dummy = return_wizard._create_returns()
return_picking = self.env['stock.picking'].browse(return_picking_id)
return_picking.move_ids.quantity_done = 1
return_picking.button_validate()
# Add a credit note for the returned kit
ctx = {'active_model': 'account.move', 'active_ids': invoice.ids}
refund_wizard = self.env['account.move.reversal'].with_context(ctx).create({
'refund_method': 'refund',
'journal_id': invoice.journal_id.id,
})
action = refund_wizard.reverse_moves()
reverse_invoice = self.env['account.move'].browse(action['res_id'])
with Form(reverse_invoice) as reverse_invoice_form:
with reverse_invoice_form.invoice_line_ids.edit(0) as line:
line.quantity = 1
reverse_invoice.action_post()
amls = reverse_invoice.line_ids
stock_out_aml = amls.filtered(lambda aml: aml.account_id == stock_output_account)
self.assertEqual(stock_out_aml.debit, 20, 'Should be to the value of the returned component')
self.assertEqual(stock_out_aml.credit, 0)
cogs_aml = amls.filtered(lambda aml: aml.account_id == expense_account)
self.assertEqual(cogs_aml.debit, 0)
self.assertEqual(cogs_aml.credit, 20, 'Should be to the value of the returned component')
addons/sale_stock/models/account_move.py
+6 -3
View File
@@ -116,10 +116,13 @@ class AccountMoveLine(models.Model):
so_line = self.sale_line_ids and self.sale_line_ids[-1] or False
if so_line:
is_line_reversing = bool(self.move_id.reversed_entry_id)
qty_to_invoice = self.product_uom_id._compute_quantity(self.quantity, self.product_id.uom_id)
qty_invoiced = sum([x.product_uom_id._compute_quantity(x.quantity, x.product_id.uom_id) for x in so_line.invoice_lines if x.move_id.state == 'posted'])
average_price_unit = self.product_id.with_company(self.company_id)._compute_average_price(qty_invoiced, qty_to_invoice, so_line.move_ids)
posted_invoice_lines = so_line.invoice_lines.filtered(lambda l: l.move_id.state == 'posted' and bool(l.move_id.reversed_entry_id) == is_line_reversing)
qty_invoiced = sum([x.product_uom_id._compute_quantity(x.quantity, x.product_id.uom_id) for x in posted_invoice_lines])
product = self.product_id.with_company(self.company_id)
average_price_unit = product._compute_average_price(qty_invoiced, qty_to_invoice, so_line.move_ids, is_returned=is_line_reversing)
if average_price_unit:
price_unit = self.product_id.uom_id.with_company(self.company_id)._compute_price(average_price_unit, self.product_uom_id)
return price_unit
addons/sale_stock/tests/test_anglo_saxon_valuation.py
+94
View File
@@ -1296,3 +1296,97 @@ class TestAngloSaxonValuation(ValuationReconciliationTestCommon):
# Expenses
self.assertEqual(aml[3].debit, 12.0)
self.assertEqual(aml[3].credit, 0.0)
def test_fifo_return_and_credit_note(self):
"""
When posting a credit note for a returned product, the value of the anglo-saxo lines
should be based on the returned product's value
"""
self.product.categ_id.property_cost_method = 'fifo'
# Receive one @10, one @20 and one @60
in_moves = self.env['stock.move'].create([{
'name': 'IN move @%s' % p,
'product_id': self.product.id,
'location_id': self.env.ref('stock.stock_location_suppliers').id,
'location_dest_id': self.company_data['default_warehouse'].lot_stock_id.id,
'product_uom': self.product.uom_id.id,
'product_uom_qty': 1,
'price_unit': p,
} for p in [10, 20, 60]])
in_moves._action_confirm()
in_moves.quantity_done = 1
in_moves._action_done()
# Sell 3 units
so = self.env['sale.order'].create({
'partner_id': self.partner_a.id,
'order_line': [
(0, 0, {
'name': self.product.name,
'product_id': self.product.id,
'product_uom_qty': 3.0,
'product_uom': self.product.uom_id.id,
'price_unit': 100,
'tax_id': False,
})],
})
so.action_confirm()
# Deliver 1@10, then 1@20 and then 1@60
pickings = []
picking = so.picking_ids
while picking:
pickings.append(picking)
picking.move_ids.quantity_done = 1
action = picking.button_validate()
if isinstance(action, dict):
wizard = Form(self.env[action['res_model']].with_context(action['context'])).save()
wizard.process()
picking = picking.backorder_ids
invoice = so._create_invoices()
invoice.action_post()
# Receive one @100
in_moves = self.env['stock.move'].create({
'name': 'IN move @100',
'product_id': self.product.id,
'location_id': self.env.ref('stock.stock_location_suppliers').id,
'location_dest_id': self.company_data['default_warehouse'].lot_stock_id.id,
'product_uom': self.product.uom_id.id,
'product_uom_qty': 1,
'price_unit': 100,
})
in_moves._action_confirm()
in_moves.quantity_done = 1
in_moves._action_done()
# Return the second picking (i.e. 1@20)
ctx = {'active_id': pickings[1].id, 'active_model': 'stock.picking'}
return_wizard = Form(self.env['stock.return.picking'].with_context(ctx)).save()
return_picking_id, dummy = return_wizard._create_returns()
return_picking = self.env['stock.picking'].browse(return_picking_id)
return_picking.move_ids.quantity_done = 1
return_picking.button_validate()
# Add a credit note for the returned product
ctx = {'active_model': 'account.move', 'active_ids': invoice.ids}
refund_wizard = self.env['account.move.reversal'].with_context(ctx).create({
'refund_method': 'refund',
'journal_id': invoice.journal_id.id,
})
action = refund_wizard.reverse_moves()
reverse_invoice = self.env['account.move'].browse(action['res_id'])
with Form(reverse_invoice) as reverse_invoice_form:
with reverse_invoice_form.invoice_line_ids.edit(0) as line:
line.quantity = 1
reverse_invoice.action_post()
amls = reverse_invoice.line_ids
stock_out_aml = amls.filtered(lambda aml: aml.account_id == self.company_data['default_account_stock_out'])
self.assertEqual(stock_out_aml.debit, 20, 'Should be to the value of the returned product')
self.assertEqual(stock_out_aml.credit, 0)
cogs_aml = amls.filtered(lambda aml: aml.account_id == self.company_data['default_account_expense'])
self.assertEqual(cogs_aml.debit, 0)
self.assertEqual(cogs_aml.credit, 20, 'Should be to the value of the returned product')
addons/stock_account/models/product.py
+3 -2
View File
@@ -646,7 +646,7 @@ class ProductProduct(models.Model):
return price or 0.0
return self.uom_id._compute_price(price, uom)
def _compute_average_price(self, qty_invoiced, qty_to_invoice, stock_moves):
def _compute_average_price(self, qty_invoiced, qty_to_invoice, stock_moves, is_returned=False):
"""Go over the valuation layers of `stock_moves` to value `qty_to_invoice` while taking
care of ignoring `qty_invoiced`. If `qty_to_invoice` is greater than what's possible to
value with the valuation layers, use the product's standard price.
@@ -654,6 +654,7 @@ class ProductProduct(models.Model):
:param qty_invoiced: quantity already invoiced
:param qty_to_invoice: quantity to invoice
:param stock_moves: recordset of `stock.move`
:param is_returned: if True, consider the incoming moves
:returns: the anglo saxon price unit
:rtype: float
"""
@@ -667,7 +668,7 @@ class ProductProduct(models.Model):
returned_quantities[move.origin_returned_move_id.id] += abs(sum(move.sudo().stock_valuation_layer_ids.mapped('quantity')))
candidates = stock_moves\
.sudo()\
.filtered(lambda m: not (m.origin_returned_move_id and sum(m.stock_valuation_layer_ids.mapped('quantity')) >= 0))\
.filtered(lambda m: is_returned == bool(m.origin_returned_move_id and sum(m.stock_valuation_layer_ids.mapped('quantity')) >= 0))\
.mapped('stock_valuation_layer_ids')\
.sorted()
qty_to_take_on_candidates = qty_to_invoice