From 2f982e6beaad8263cc6bf9880da78ca2922372bb Mon Sep 17 00:00:00 2001 From: "Quentin (OpenERP)" Date: Fri, 14 Feb 2014 09:46:50 +0100 Subject: [PATCH] [FIX] a lot of changes made during code review bzr revid: qdp-launchpad@openerp.com-20140214084650-8vcy544475urij53 --- addons/mrp/mrp.py | 11 ++++---- addons/mrp/stock.py | 43 +++++++++++++++++-------------- addons/procurement/procurement.py | 1 + addons/stock/procurement.py | 3 ++- addons/stock/stock.py | 34 ++++++++++++++---------- 5 files changed, 53 insertions(+), 39 deletions(-) diff --git a/addons/mrp/mrp.py b/addons/mrp/mrp.py index 59dc7df4ed2..bb50c977a68 100644 --- a/addons/mrp/mrp.py +++ b/addons/mrp/mrp.py @@ -1031,11 +1031,12 @@ class mrp_production(osv.osv): 'location_dest_id': destination_location_id, 'move_dest_id': production.move_prod_id.id, 'company_id': production.company_id.id, + 'production_id': production.id, } move_id = stock_move.create(cr, uid, data, context=context) - production.write({'move_created_ids': [(6, 0, [move_id])]}, context=context) - stock_move.action_confirm(cr, uid, [move_id], context=context) - return move_id + #a phantom bom cannot be used in mrp order so it's ok to assume the list returned by action_confirm + #is 1 element long, so we can take the first. + return stock_move.action_confirm(cr, uid, [move_id], context=context)[0] def _make_production_consume_line(self, cr, uid, production_line, parent_move_id, source_location_id=False, context=None): stock_move = self.pool.get('stock.move') @@ -1058,10 +1059,10 @@ class mrp_production(osv.osv): 'location_dest_id': destination_location_id, 'company_id': production.company_id.id, 'procure_method': 'make_to_order', + 'raw_material_production_id': production.id, }) - production.write({'move_lines': [(4, move_id)]}, context=context) stock_move.action_confirm(cr, uid, [move_id], context=context) - return move_id + return True def action_confirm(self, cr, uid, ids, context=None): """ Confirms production order. diff --git a/addons/mrp/stock.py b/addons/mrp/stock.py index 7ef694f1b17..2d38adf7de4 100644 --- a/addons/mrp/stock.py +++ b/addons/mrp/stock.py @@ -37,9 +37,7 @@ class StockMove(osv.osv): def copy(self, cr, uid, id, default=None, context=None): if not default: default = {} - default.update({ - 'production_id': False, - }) + default['production_id'] = False return super(StockMove, self).copy(cr, uid, id, default, context=context) def check_tracking(self, cr, uid, move, lot_id, context=None): @@ -52,10 +50,12 @@ class StockMove(osv.osv): def _check_phantom_bom(self, cr, uid, move, context=None): """check if product associated to move has a phantom bom return list of ids of mrp.bom for that product """ - return self.pool.get('mrp.bom').search(cr, uid, [ + user_company = self.pool.get('res.users').browse(cr, uid, uid, context=context).company_id.id + return self.pool.get('mrp.bom').search(cr, SUPERUSER_ID, [ ('product_id', '=', move.product_id.id), ('bom_id', '=', False), - ('type', '=', 'phantom')]) + ('type', '=', 'phantom'), + ('company_id', '=', user_company)], context=context) def _action_explode(self, cr, uid, move, context=None): """ Explodes pickings. @@ -66,8 +66,9 @@ class StockMove(osv.osv): move_obj = self.pool.get('stock.move') procurement_obj = self.pool.get('procurement.order') product_obj = self.pool.get('product.product') + to_explode_again_ids = [] processed_ids = [] - bis = self._check_phantom_bom(cr, SUPERUSER_ID, move, context=context) + bis = self._check_phantom_bom(cr, uid, move, context=context) if bis: factor = move.product_qty bom_point = bom_obj.browse(cr, SUPERUSER_ID, bis[0], context=context) @@ -87,33 +88,29 @@ class StockMove(osv.osv): 'name': line['name'], } mid = move_obj.copy(cr, uid, move.id, default=valdef) - processed_ids.append(mid) + to_explode_again_ids.append(mid) + #delete the move with original product which is not relevant anymore move_obj.unlink(cr, SUPERUSER_ID, [move.id], context=context) #check if new moves needs to be exploded - if processed_ids: - for new_move in self.browse(cr, uid, processed_ids, context=context): - exploded_ids = self._action_explode(cr, uid, new_move, context=context) - if not (len(exploded_ids)==1 and exploded_ids[0]==new_move.id): - #we have some exploded move, delete parent move and add new moves - #to the list of processed ones - processed_ids.remove(move.id) - processed_ids.extend(exploded_ids) + if to_explode_again_ids: + for new_move in self.browse(cr, uid, to_explode_again_ids, context=context): + processed_ids.extend(self._action_explode(cr, uid, new_move, context=context)) #return list of newly created move or the move id otherwise return processed_ids or [move.id] def action_confirm(self, cr, uid, ids, context=None): move_ids = [] for move in self.browse(cr, uid, ids, context=context): - #in order to explode a move, we must have a picking_id on that move! - #if action_explode return a list of new move, it means it has exploded - #and that original move has been deleted, so we should remove that id - #from super call to prevent problem + #in order to explode a move, we must have a picking_type_id on that move because otherwise the move + #won't be assigned to a picking and it would be weird to explode a move into several if they aren't + #all grouped in the same picking. if move.picking_type_id: move_ids.extend(self._action_explode(cr, uid, move, context=context)) else: move_ids.append(move.id) + #we go further with the list of ids potentially changed by action_explode return super(StockMove, self).action_confirm(cr, uid, move_ids, context=context) def action_consume(self, cr, uid, ids, product_qty, location_id=False, restrict_lot_id=False, restrict_partner_id=False, @@ -134,9 +131,15 @@ class StockMove(osv.osv): if product_qty <= 0: raise osv.except_osv(_('Warning!'), _('Please provide proper quantity.')) + #because of the action_confirm that can create extra moves in case of phantom bom, we need to make 2 loops + ids2 = [] for move in self.browse(cr, uid, ids, context=context): if move.state == 'draft': - self.action_confirm(cr, uid, [move.id], context=context) + ids2.extend(self.action_confirm(cr, uid, [move.id], context=context)) + else: + ids2.append(move.id) + + for move in self.browse(cr, uid, ids2, context=context): move_qty = move.product_qty uom_qty = uom_obj._compute_qty(cr, uid, move.product_id.uom_id.id, product_qty, move.product_uom.id) if move_qty <= 0: diff --git a/addons/procurement/procurement.py b/addons/procurement/procurement.py index e36e6554b2d..b35a7755b28 100644 --- a/addons/procurement/procurement.py +++ b/addons/procurement/procurement.py @@ -250,6 +250,7 @@ class procurement_order(osv.osv): def _run(self, cr, uid, procurement, context=None): '''This method implements the resolution of the given procurement :param procurement: browse record + :returns: True if the resolution of the procurement was a success, False otherwise to set it in exception ''' return True diff --git a/addons/stock/procurement.py b/addons/stock/procurement.py index 8448ee3c5e1..0ea7d0e13e1 100644 --- a/addons/stock/procurement.py +++ b/addons/stock/procurement.py @@ -216,7 +216,8 @@ class procurement_order(osv.osv): move_obj = self.pool.get('stock.move') move_dict = self._run_move_create(cr, uid, procurement, context=context) move_id = move_obj.create(cr, uid, move_dict, context=context) - return move_obj.action_confirm(cr, uid, [move_id], context=context) + move_obj.action_confirm(cr, uid, [move_id], context=context) + return True return super(procurement_order, self)._run(cr, uid, procurement, context) def _check(self, cr, uid, procurement, context=None): diff --git a/addons/stock/stock.py b/addons/stock/stock.py index 9168f9f8bb9..56c0d3a6f3b 100644 --- a/addons/stock/stock.py +++ b/addons/stock/stock.py @@ -830,6 +830,7 @@ class stock_picking(osv.osv): for pick in self.browse(cr, uid, ids, context=context): if pick.state == 'draft': self.action_confirm(cr, uid, [pick.id], context=context) + pick.refresh() #skip the moves that don't need to be checked move_ids = [x.id for x in pick.move_lines if x.state not in ('draft', 'cancel', 'done')] if not move_ids: @@ -864,8 +865,7 @@ class stock_picking(osv.osv): todo = [] for move in pick.move_lines: if move.state == 'draft': - todo.extend(self.pool.get('stock.move').action_confirm(cr, uid, [move.id], - context=context)) + todo.extend(self.pool.get('stock.move').action_confirm(cr, uid, [move.id], context=context)) elif move.state in ('assigned', 'confirmed'): todo.append(move.id) if len(todo): @@ -911,7 +911,8 @@ class stock_picking(osv.osv): move_obj.write(cr, uid, backorder_move_ids, {'picking_id': backorder_id}, context=context) self.write(cr, uid, [picking.id], {'date_done': time.strftime(DEFAULT_SERVER_DATETIME_FORMAT)}, context=context) - return self.action_confirm(cr, uid, [backorder_id], context=context) + self.action_confirm(cr, uid, [backorder_id], context=context) + return backorder_id return False def recheck_availability(self, cr, uid, picking_ids, context=None): @@ -1084,7 +1085,7 @@ class stock_picking(osv.osv): new_move = stock_move_obj.split(cr, uid, move, move.remaining_qty, context=context) todo_move_ids.append(move.id) #Assign move as it was assigned before - toassign_move_ids.extend(new_move) + toassign_move_ids.append(new_move) elif move.state: #this should never happens raise @@ -1767,7 +1768,7 @@ class stock_move(osv.osv): """ Confirms stock move or put it in waiting if it's linked to another move. @return: List of ids. """ - if type(ids) in ('int', 'float'): + if isinstance(ids, ('int', 'long')): ids = [ids] states = { 'confirmed': [], @@ -2031,9 +2032,18 @@ class stock_move(osv.osv): :param move: browse record :param qty: float. quantity to split (given in product UoM) :param context: dictionay. can contains the special key 'source_location_id' in order to force the source location when copying the move + + returns the ID of the backorder move created """ + if move.state in ('done', 'cancel'): + raise osv.except_osv(_('Error'), _('You cannot split a move done')) + if move.state == 'draft': + #we restrict the split of a draft move because if not confirmed yet, it may be replaced by several other moves in + #case of phantom bom (with mrp module). And we don't want to deal with this complexity by copying the product that will explode. + raise osv.except_osv(_('Error'), _('You cannot split a draft move. It needs to be confirmed first.')) + if move.product_qty <= qty or qty == 0: - return [move.id] + return move.id uom_obj = self.pool.get('product.uom') context = context or {} @@ -2041,9 +2051,6 @@ class stock_move(osv.osv): uom_qty = uom_obj._compute_qty(cr, uid, move.product_id.uom_id.id, qty, move.product_uom.id) uos_qty = uom_qty * move.product_uos_qty / move.product_uom_qty - if move.state in ('done', 'cancel'): - raise osv.except_osv(_('Error'), _('You cannot split a move done')) - defaults = { 'product_uom_qty': uom_qty, 'product_uos_qty': uos_qty, @@ -2070,8 +2077,9 @@ class stock_move(osv.osv): if move.move_dest_id and move.propagate: new_move_prop = self.split(cr, uid, move.move_dest_id, qty, context=context) self.write(cr, uid, [new_move], {'move_dest_id': new_move_prop}, context=context) - - return self.action_confirm(cr, uid, [new_move], context=context) + #returning the first element of list returned by action_confirm is ok because we checked it wouldn't be exploded (and + #thus the result of action_confirm should always be a list of 1 element length) + return self.action_confirm(cr, uid, [new_move], context=context)[0] class stock_inventory(osv.osv): @@ -3085,6 +3093,7 @@ class stock_location_path(osv.osv): 'propagate': True, 'active': True, } + def _apply(self, cr, uid, rule, move, context=None): move_obj = self.pool.get('stock.move') newdate = (datetime.strptime(move.date_expected, DEFAULT_SERVER_DATETIME_FORMAT) + relativedelta.relativedelta(days=rule.delay or 0)).strftime(DEFAULT_SERVER_DATETIME_FORMAT) @@ -3100,7 +3109,6 @@ class stock_location_path(osv.osv): if rule.location_dest_id.id != old_dest_location: #call again push_apply to see if a next step is defined move_obj._push_apply(cr, uid, [move], context=context) - return move.id else: move_id = move_obj.copy(cr, uid, move.id, { 'location_id': move.location_dest_id.id, @@ -3117,7 +3125,7 @@ class stock_location_path(osv.osv): move_obj.write(cr, uid, [move.id], { 'move_dest_id': move_id, }) - return move_obj.action_confirm(cr, uid, [move_id], context=None) + move_obj.action_confirm(cr, uid, [move_id], context=None) class stock_move_putaway(osv.osv): _name = 'stock.move.putaway'