[REF] stock_account: move away from the replay everything strategy

The approach we tried with the current implementation lead to a replay
of all the next moves since a move had to be corrected, and posting an
account entry with the valuation difference.

This proved to be slow and complicated, and moreover the resulting
account entry could be very difficult to understand and justify.

So we moved away from this strategy and implemented the following points:

For FIFO
* If a move goes into negative (sent more than availabale, we do not
  counterpart directly at the next in move: we valued it at the last
  fifo price and only when running `_fifo_vacuum` (which is called at
  every openning of the valuation report), we use some units currently
  in stock and post an account entry with the difference between the
  fake value and the real one used.
* When editing a done move and increasing its quantity, the new quantity
  is available now and will be used at the next move out. We do not try
  to replay the history to make the new values go away directly.
* When editing a done move and decreasing its quantity, it acts as a
  return to the supplier at the price unit of the move.

On average price:
* Don't replay anything, if we fall under the negative stock usecase,
  value at the last average price.
We restore the previous code that set the average price on the standard
price and we slightly patch it to continue to update the average price
if quantity in stock is negative.

We also add a link between stock moves and account moves. A stat button
on the stock move's form redirect directly to them. It's therefore a
little easier to understand what happened.

We remove the separate fifo and average price fields that were added in
saas17, we keep the single standard price (that should be renamed
cost_price for consistency). In the same idea, remove the cumulated
value that was stored on the move. If we need it, just recompute it
(it's not a frequent operation). We remove last_done_move_id,
last_done_remaining_qty and last_done_qty that were added in saas17 to
help in the replay method, as we just got rid of it. We also remove the
code that compensate the negative stock directy at the next in.

What the fifo vacuum does is essentially the same thing as what an out
move do: find a candidate and take units from it. In this case, the
candidates are newer than its concerned stock move. We stored on the
concerned stock move the value at which we sent it (remaining_value) and
post the difference with the real value on a real product.
This commit is contained in:
Simon Lejeune
2017-08-25 16:10:19 +02:00
parent afff55e809
commit 9439f6aa55
8 changed files with 1211 additions and 348 deletions
+2
View File
@@ -231,6 +231,8 @@
<field name="state" widget="statusbar" statusbar_visible="draft,confirmed,assigned,done" />
</header>
<sheet>
<div class="oe_button_box" name="button_box">
</div>
<group>
<group name="main_grp" colspan="2">
<group name="main_grp_col1">
+1
View File
@@ -3,6 +3,7 @@
from . import account_chart_template
from . import account_invoice
from . import account_move
from . import product
from . import stock
from . import stock_config_settings
@@ -0,0 +1,10 @@
# -*- coding: utf-8 -*-
from odoo import fields, models
class AccountMove(models.Model):
_inherit = 'account.move'
stock_move_id = fields.Many2one('stock.move', string='Stock Move')
-54
View File
@@ -38,19 +38,6 @@ class ProductTemplate(models.Model):
company_dependent=True, domain=[('deprecated', '=', False)],
help="When doing real-time inventory valuation, counterpart journal items for all outgoing stock moves will be posted in this account, unless "
"there is a specific valuation account set on the destination location. When not set on the product, the one from the product category is used.")
average_price = fields.Float(
'Average Cost', compute='_compute_average_price',
digits=dp.get_precision('Product Price'), groups="base.group_user",
help="Average cost of the product, in the default unit of measure of the product.")
@api.multi
def _compute_average_price(self):
unique_variants = self.filtered(lambda template: len(template.product_variant_ids) == 1)
for template in unique_variants:
template.average_price = template.product_variant_ids.average_price
for template in (self - unique_variants):
template.average_price = 0.0
@api.one
@api.depends('property_valuation', 'categ_id.property_valuation')
@@ -100,12 +87,6 @@ class ProductTemplate(models.Model):
class ProductProduct(models.Model):
_inherit = 'product.product'
average_price = fields.Float(
'Average Cost',
digits=dp.get_precision('Product Price'),
groups="base.group_user",
compute='_compute_average_price',
help="Calculated average cost")
stock_value = fields.Float(
'Value', compute='_compute_stock_value')
@@ -156,24 +137,6 @@ class ProductProduct(models.Model):
self.write({'standard_price': new_price})
return True
def _get_latest_cumulated_value(self, exclude_move=False):
self.ensure_one()
domain = [('product_id', '=', self.id)] + self.env['stock.move']._get_all_base_domain()
if exclude_move:
domain += [('id', '!=', exclude_move.id)]
latest = self.env['stock.move'].search(domain, order='date desc, id desc', limit=1)
if not latest:
return 0.0
return latest.cumulated_value
def _get_fifo_candidates_out_move(self):
""" Find OUT moves that were not valued in time because of negative stock.
"""
self.ensure_one()
domain = [('product_id', '=', self.id), ('remaining_qty', '>', 0.0)] + self.env['stock.move']._get_out_base_domain()
candidates = self.env['stock.move'].search(domain, order='date, id')
return candidates
def _get_fifo_candidates_in_move(self):
""" Find IN moves that can be used to value OUT moves.
"""
@@ -182,23 +145,6 @@ class ProductProduct(models.Model):
candidates = self.env['stock.move'].search(domain, order='date, id')
return candidates
@api.multi
@api.depends('stock_move_ids.product_qty', 'stock_move_ids.state')
def _compute_average_price(self):
""" Compute the average price of a product.product according to the latest cumulated value
and the quantity that is in stock (for all the company owned locations, i.e. including the
warehouses transit ones). If nothing is in stick, we fallback first on the latest average
price on an out move used for this product then on the standard price.
"""
for p_set, product in pycompat.izip(self, self.with_context(company_owned=True)):
if product.qty_available > 0:
last_cumulated_value = product._get_latest_cumulated_value()
p_set.average_price = last_cumulated_value / product.qty_available
else:
domain = [('product_id', '=', product.id), ('last_done_qty', '>', 0.0)] + self.env['stock.move']._get_all_base_domain()
move = self.env['stock.move'].search(domain, order='date desc, id desc', limit=1)
p_set.average_price = move and move.cumulated_value / move.last_done_qty or product.standard_price
@api.multi
def _compute_stock_value(self):
for product in self:
+181 -163
View File
@@ -55,11 +55,25 @@ class StockMoveLine(models.Model):
@api.multi
def write(self, vals):
super(StockMoveLine, self).write(vals)
if 'qty_done' in vals:
for move in self.mapped('move_id').filtered(lambda m: m.state == 'done'):
move.value = move.quantity_done * move.price_unit
move.replay()
# We need to update the `value`, `remaining_value` and `remaining_qty` on the linked
# stock move.
moves_to_update = {}
for move_line in self.filtered(lambda ml: ml.state == 'done'):
moves_to_update[move_line.move_id] = vals['qty_done'] - move_line.qty_done
for move_id, qty_difference in moves_to_update.items():
# more/less units are available, update `value`, `remaining_value` and
# `remaining_qty` on the linked stock move.
move_vals = {
'value': move_id.value + qty_difference * move_id.price_unit,
'remaining_value': move_id.remaining_value + qty_difference * move_id.price_unit,
'remaining_qty': move_id.remaining_qty + qty_difference,
}
move_id.write(move_vals)
if move_id.product_id.valuation == 'real_time':
move_id.with_context(force_valuation_amount=qty_difference*move_id.price_unit)._account_entry_move()
return super(StockMoveLine, self).write(vals)
class StockMove(models.Model):
@@ -67,50 +81,26 @@ class StockMove(models.Model):
to_refund = fields.Boolean(string="To Refund (update SO/PO)",
help='Trigger a decrease of the delivered/received quantity in the associated Sale Order/Purchase Order')
value = fields.Float()
cumulated_value = fields.Float()
remaining_qty = fields.Float()
last_done_move_id = fields.Many2one('stock.move')
last_done_remaining_qty = fields.Float()
last_done_qty = fields.Float()
# TODO: add constraints remaining_qty > 0
# TODO: add constrain price_unit = 0 on done move?
remaining_value = fields.Float()
account_move_ids = fields.One2many('account.move', 'stock_move_id')
@api.multi
def _get_price_unit(self):
def action_get_account_moves(self):
self.ensure_one()
if self.product_id.cost_method == 'average':
return self.product_id.average_price or self.product_id.standard_price
if self.product_id.cost_method == 'fifo':
move = self.search([('product_id', '=', self.product_id.id),
('state', '=', 'done'),
('location_id.usage', '=', 'internal'),
('location_dest_id.usage', '!=', 'internal')], order='date desc', limit=1)
if move:
return move.price_unit or self.product_id.standard_price
return self.product_id.standard_price
action_ref = self.env.ref('account.action_move_journal_line')
if not action_ref:
return False
action_data = action_ref.read()[0]
action_data['domain'] = [('id', 'in', self.account_move_ids.ids)]
return action_data
def _update_fifo_future_cumulated_value(self, value):
""" This method is intended to be called on an OUT move that was not valued in time because
of negative stock with `value` argument representing its new value.
"""
self.ensure_one()
domain = self._get_all_domain()
domain += [
'|',
('date', '>', self.date),
'&',
('date', '=', self.date),
('id', '>', self.id)
]
moves = self.search(domain, order='date, id')
for move in moves:
# As `self` is always an out move, we decrease its value on the future moves.
move.cumulated_value -= value
def get_price_unit(self):
""" Returns the unit price to store on the quant """
return self.price_unit or self.product_id.standard_price
@ api.model
@api.model
def _get_in_base_domain(self, company_id=False):
domain = [
('state', '=', 'done'),
@@ -119,16 +109,7 @@ class StockMove(models.Model):
]
return domain
@ api.model
def _get_out_base_domain(self, company_id=False):
domain = [
('state', '=', 'done'),
('location_id.company_id', '=', company_id or self.env.user.company_id.id),
('location_dest_id.company_id', '=', False)
]
return domain
@ api.model
@api.model
def _get_all_base_domain(self, company_id=False):
domain = [
('state', '=', 'done'),
@@ -145,9 +126,6 @@ class StockMove(models.Model):
def _get_in_domain(self):
return [('product_id', '=', self.product_id.id)] + self._get_in_base_domain(company_id=self.company_id.id)
def _get_out_domain(self):
return [('product_id', '=', self.product_id.id)] + self._get_out_base_domain(company_id=self.company_id.id)
def _get_all_domain(self):
return [('product_id', '=', self.product_id.id)] + self._get_all_base_domain(company_id=self.company_id.id)
@@ -167,133 +145,171 @@ class StockMove(models.Model):
"""
return self.location_id.company_id.id == self.company_id.id and not self.location_dest_id.company_id
# def change_move_value_in_the_past(self, value):
# self.ensure_one()
# if self.product_id.cost_method == 'fifo':
# moves = self.search([('state', '=', 'done'),
# ('date', '>', self.date),
# ('product_id', '=', self.product_id.id)])
# if self.location_id.usage not in ('internal', 'transit'):
# if move.last_done_move_id and move.last_done_remaining_qty:
@api.multi
def replay(self):
# Easy scenario: avergae /done
# search last move before this one
start_move = self.search([('product_id', '=', self.product_id.id),
('state', '=', 'done'),
('date', '<', self.date)], limit=1, order='date desc') #filter on outgoing/incoming moves
next_moves = self.search([('product_id', '=', self.product_id.id),
('state', '=', 'done'),
('date', '>', start_move.date)], order='date') # filter on outgoing/incoming moves
if start_move:
last_cumulated_value = start_move.cumulated_value
last_done_qty_available = start_move.last_done_qty
else:
last_cumulated_value = 0.0
last_done_qty_available = 0.0
if self.product_id.cost_method == 'average':
for move in next_moves:
if move.location_id.usage in ('internal', 'transit') and move.location_dest_id.usage not in ('internal', 'transit'):
if last_done_qty_available:
move.value = - ((last_cumulated_value / last_done_qty_available) * move.product_qty)
last_done_qty_available -= move.product_qty
else:
last_done_qty_available += move.product_qty
move.cumulated_value = last_cumulated_value + move.value
last_cumulated_value = move.cumulated_value
move.last_done_qty = last_done_qty_available
# FIFO: needs dict with qty_remaining to replay algorithm
# update cumulated_value according to value
# update outs according to values
# update
@api.multi
def action_done(self):
average_price = {}
for move in self.filtered(lambda m: m.product_id.cost_method == 'average'):
# For the average costing method, the value depends of the quantity in stock at the
# moment of the transfer, so fetch the value before calling super.
average_price[move.product_id.id] = move.product_id.average_price
self.product_price_update_before_done()
res = super(StockMove, self).action_done()
for move in res:
if move._is_in():
if move.product_id.cost_method in ['fifo', 'average']:
if not move.price_unit:
move.price_unit = move._get_price_unit()
move.value = move.price_unit * move.product_qty
move.cumulated_value = move.product_id._get_latest_cumulated_value(exclude_move=move) + move.value
price_unit = move.price_unit or move.get_price_unit()
value = price_unit * move.product_qty
vals = {
'price_unit': price_unit,
'value': value,
'remaining_value': value,
}
if move.product_id.cost_method == 'fifo':
move.remaining_qty = move.product_qty
# If there are some OUT moves that we could not value in time, find them
# and value them now with this new IN move. After that, update the cumulated
# value along the moves done after the newly valued move.
candidates_out = move.product_id._get_fifo_candidates_out_move()
qty_to_take = move.product_qty
for candidate in candidates_out:
if candidate.remaining_qty < qty_to_take:
qty_taken_for_candidate = candidate.remaining_qty
else:
qty_taken_for_candidate = qty_to_take
candidate.remaining_qty -= qty_taken_for_candidate
move.remaining_qty -= qty_taken_for_candidate
qty_to_take -= qty_taken_for_candidate
candidate.value -= move.price_unit * qty_taken_for_candidate
candidate.cumulated_value -= move.price_unit * qty_taken_for_candidate
candidate._update_fifo_future_cumulated_value(move.price_unit * qty_taken_for_candidate)
candidate.price_unit = candidate.value / candidate.product_qty
move.last_done_qty = move.product_id.qty_available
else:
move.price_unit = move.product_id.standard_price
move.value = move.price_unit * move.product_qty
vals['remaining_qty'] = move.product_qty
move.write(vals)
else: # standard
move.write({
'price_unit': move.product_id.standard_price,
'value': move.product_id.standard_price * move.product_qty,
})
elif move._is_out():
if move.product_id.cost_method == 'fifo':
qty_to_take = move.product_qty
tmp_value = 0
# Find back incoming stock moves (called candidates here) to value this move.
qty_to_take_on_candidates = move.product_qty
candidates = move.product_id._get_fifo_candidates_in_move()
last_candidate = False
new_standard_price = 0
tmp_value = 0 # to accumulate the value taken on the candidates
for candidate in candidates:
if candidate.remaining_qty <= qty_to_take:
new_standard_price = candidate.price_unit
if candidate.remaining_qty <= qty_to_take_on_candidates:
qty_taken_on_candidate = candidate.remaining_qty
else:
qty_taken_on_candidate = qty_to_take
tmp_value += qty_taken_on_candidate * candidate.price_unit
candidate.remaining_qty -= qty_taken_on_candidate
qty_to_take -= qty_taken_on_candidate
if qty_to_take == 0:
qty_taken_on_candidate = qty_to_take_on_candidates
value_taken_on_candidate = qty_taken_on_candidate * candidate.price_unit
candidate_vals = {
'remaining_qty': candidate.remaining_qty - qty_taken_on_candidate,
'remaining_value': candidate.remaining_value - value_taken_on_candidate,
}
candidate.write(candidate_vals)
qty_to_take_on_candidates -= qty_taken_on_candidate
tmp_value += value_taken_on_candidate
if qty_to_take_on_candidates == 0:
break
last_candidate = candidate
if last_candidate:
move.last_done_move_id = last_candidate.id
move.last_done_remaining_qty = last_candidate.remaining_qty
if qty_to_take > 0:
move.remaining_qty = qty_to_take # In case there are no candidates to match, put standard price on it
move.value = -tmp_value
move.cumulated_value = move.product_id._get_latest_cumulated_value(exclude_move=move) + move.value
move.last_done_qty = move.product_id.qty_available
elif move.product_id.cost_method == 'average':
# Update the standard price with the price of the last used candidate, if any.
if new_standard_price:
move.product_id.standard_price = new_standard_price
# If there's still quantity to value but we're out of candidates, we fall in the
# negative stock use case. We chose to value the out move at the price of the
# last out and a correction entry will be made once `_fifo_vacuum` is called.
if qty_to_take_on_candidates == 0:
move.write({
'value': -tmp_value, # outgoing move are valued negatively
'price_unit': -tmp_value / move.product_qty,
})
elif qty_to_take_on_candidates > 0:
last_fifo_price = new_standard_price or move.product_id.standard_price
negative_stock_value = last_fifo_price * -qty_to_take_on_candidates
vals = {
'remaining_qty': -qty_to_take_on_candidates,
'remaining_value': negative_stock_value,
'value': -tmp_value + negative_stock_value,
'price_unit': (-tmp_value + negative_stock_value) / move.product_qty,
}
move.write(vals)
elif move.product_id.cost_method in ['standard', 'average']:
curr_rounding = move.company_id.currency_id.rounding
avg_price_unit = average_price[move.product_id.id]
move.value = float_round(-avg_price_unit * move.product_qty, precision_rounding=curr_rounding)
move.remaining_qty = 0
move.cumulated_value = move.product_id._get_latest_cumulated_value(exclude_move=move) + move.value
move.last_done_qty = move.product_id.qty_available
elif move.product_id.cost_method == 'standard':
move.value = - move.product_id.standard_price * move.product_qty
value = -float_round(move.product_id.standard_price * move.product_qty, precision_rounding=curr_rounding)
move.write({
'value': value,
'price_unit': value / move.product_qty,
})
for move in res.filtered(lambda m: m.product_id.valuation == 'real_time'):
move._account_entry_move()
return res
@api.multi
def product_price_update_before_done(self):
tmpl_dict = defaultdict(lambda: 0.0)
# adapt standard price on incomming moves if the product cost_method is 'average'
std_price_update = {}
for move in self.filtered(lambda move: move.location_id.usage in ('supplier', 'production') and move.product_id.cost_method == 'average'):
product_tot_qty_available = move.product_id.qty_available + tmpl_dict[move.product_id.id]
if product_tot_qty_available == 0:
new_std_price = move.get_price_unit()
else:
# Get the standard price
amount_unit = std_price_update.get((move.company_id.id, move.product_id.id)) or move.product_id.standard_price
new_std_price = ((amount_unit * product_tot_qty_available) + (move.get_price_unit() * move.product_qty)) / (product_tot_qty_available + move.product_qty)
tmpl_dict[move.product_id.id] += move.product_qty
# Write the standard price, as SUPERUSER_ID because a warehouse manager may not have the right to write on products
move.product_id.with_context(force_company=move.company_id.id).sudo().write({'standard_price': new_std_price})
std_price_update[move.company_id.id, move.product_id.id] = new_std_price
@api.model
def _fifo_vacuum(self):
""" Every moves that need to be fixed are identifiable by having a negative `remaining_qty`.
"""
# FIXME: sort by date (does filtered lose the order?)
for move in self.filtered(lambda m: (m._is_in() or m._is_out()) and m.remaining_qty < 0):
domain = [
'|',
('date', '>', move.date),
'&',
('date', '=', move.date),
('id', '>', move.id)
]
domain += self._get_in_domain()
candidates = self.search(domain, order='date, id')
if not candidates:
return
qty_to_take_on_candidates = abs(move.remaining_qty)
tmp_value = 0
for candidate in candidates:
if candidate.remaining_qty <= qty_to_take_on_candidates:
qty_taken_on_candidate = candidate.remaining_qty
else:
qty_taken_on_candidate = qty_to_take_on_candidates
value_taken_on_candidate = qty_taken_on_candidate * candidate.price_unit
candidate_vals = {
'remaining_qty': candidate.remaining_qty - qty_taken_on_candidate,
'remaining_value': candidate.remaining_value - value_taken_on_candidate,
}
candidate.write(candidate_vals)
qty_to_take_on_candidates -= qty_taken_on_candidate
tmp_value += value_taken_on_candidate
if qty_to_take_on_candidates == 0:
break
corrected_value = move.remaining_value + tmp_value
if move.product_id.valuation == 'real_time':
if move._is_in():
# If we just compensated an IN move that has a negative remaining
# quantity, it means the move has returned more items than it received.
# The correction should behave as a return too. As `_account_entry_move`
# will post the natural values for an IN move (credit IN account, debit
# OUT one), we inverse the sign to create the correct entries.
move.with_context(force_valuation_amount=-corrected_value)._account_entry_move()
else:
move.with_context(force_valuation_amount=corrected_value)._account_entry_move()
if qty_to_take_on_candidates == 0:
move.write({
'value': move.value - corrected_value,
'remaining_value': 0,
'remaining_qty': 0,
})
elif qty_to_take_on_candidates > 0:
# It's possible that `remaining_value` is equals to 0 even if the move needs to be
# compensated (negative stock for the first ever out in FIFO).
move.write({
'value': move.value - corrected_value,
'remaining_value': 0 if not move.remaining_value else move.remaining_value + corrected_value,
'remaining_qty': -qty_to_take_on_candidates,
})
@api.multi
def _get_accounting_data_for_valuation(self):
""" Return the accounts and journal to use to post Journal Entries for
@@ -415,7 +431,9 @@ class StockMove(models.Model):
'journal_id': journal_id,
'line_ids': move_lines,
'date': date,
'ref': self.picking_id.name})
'ref': self.picking_id.name,
'stock_move_id': self.id,
})
new_account_move.post()
def _account_entry_move(self):
File diff suppressed because it is too large Load Diff
+1 -4
View File
@@ -39,11 +39,8 @@
</field>
<field name="standard_price" position="replace">
<field name="standard_price"
attrs="{'readonly': [('valuation','=','real_time')]}"
widget='monetary' options="{'currency_field': 'currency_id'}"/>
<field name="average_price" readonly="1"
attrs="{'invisible': [('cost_method','!=', 'average')]}"
widget='monetary' options="{'currency_field': 'currency_id'}"/>
</field>
<div name="standard_price_uom" position="after">
<button string="Update Cost" type="action"
@@ -2,6 +2,17 @@
<odoo>
<data>
<record id="view_move_form_inherit" model="ir.ui.view">
<field name="name">stock.move.form.inherit</field>
<field name="model">stock.move</field>
<field name="inherit_id" ref="stock.view_move_form"/>
<field name="arch" type="xml">
<xpath expr="//div[@name='button_box']" position="inside">
<button name="action_get_account_moves" icon="fa-arrows" class="oe_stat_button" string="Account Moves" type="object"/>
</xpath>
</field>
</record>
<record id="view_inventory_form_inherit" model="ir.ui.view">
<field name="name">stock.inventory.form.inherit</field>
<field name="model">stock.inventory</field>