From 24023677ef33580b5a3d685f4e6de43a948e3840 Mon Sep 17 00:00:00 2001 From: Josse Colpaert Date: Thu, 17 Apr 2014 17:24:32 +0200 Subject: [PATCH 1/4] [IMP] Simplify negative quants reconciliation and add extra fallback domain to be able to still take from other moves bzr revid: jco@openerp.com-20140417152432-s12qpgskqvwvyslp --- addons/mrp/mrp.py | 2 +- addons/stock/stock.py | 59 ++++++++++-------------------- addons/stock/test/wiseoperator.yml | 2 +- 3 files changed, 22 insertions(+), 41 deletions(-) diff --git a/addons/mrp/mrp.py b/addons/mrp/mrp.py index b49b99fd6e0..a54f82cf00e 100644 --- a/addons/mrp/mrp.py +++ b/addons/mrp/mrp.py @@ -800,7 +800,7 @@ class mrp_production(osv.osv): q = min(move.product_qty, qty) quants = quant_obj.quants_get_prefered_domain(cr, uid, move.location_id, scheduled.product_id, q, domain=[('qty', '>', 0.0)], - prefered_domain=[('reservation_id', '=', move.id)], fallback_domain=[('reservation_id', '=', False)], context=context) + prefered_domain=[('reservation_id', '=', move.id)], fallback_domain_list=[[('reservation_id', '=', False)],], context=context) for quant, quant_qty in quants: if quant: lot_id = quant.lot_id.id diff --git a/addons/stock/stock.py b/addons/stock/stock.py index 7ed13fe2714..12646dbd2f4 100644 --- a/addons/stock/stock.py +++ b/addons/stock/stock.py @@ -381,32 +381,32 @@ class stock_quant(osv.osv): 'package_id': dest_package_id} self.write(cr, SUPERUSER_ID, [q.id for q in quants], vals, context=context) - def quants_get_prefered_domain(self, cr, uid, location, product, qty, domain=None, prefered_domain=False, fallback_domain=False, restrict_lot_id=False, restrict_partner_id=False, context=None): + def quants_get_prefered_domain(self, cr, uid, location, product, qty, domain=None, prefered_domain=False, fallback_domain_list=[], restrict_lot_id=False, restrict_partner_id=False, context=None): ''' This function tries to find quants in the given location for the given domain, by trying to first limit the choice on the quants that match the prefered_domain as well. But if the qty requested is not reached - it tries to find the remaining quantity by using the fallback_domain. + it tries to find the remaining quantity by using the fallback_domain_list. + Make sure the quants aren't found twice => the prefered_domain and all the fallback_domains should be orthogonal ''' #don't look for quants in location that are of type production, supplier or inventory. if location.usage in ['inventory', 'production', 'supplier']: return [(None, qty)] - if prefered_domain and fallback_domain: + if prefered_domain: if domain is None: domain = [] quants = self.quants_get(cr, uid, location, product, qty, domain=domain + prefered_domain, restrict_lot_id=restrict_lot_id, restrict_partner_id=restrict_partner_id, context=context) res_qty = qty - quant_ids = [] for quant in quants: if quant[0]: - quant_ids.append(quant[0].id) res_qty -= quant[1] - if res_qty > 0: - #try to replace the last tuple (None, res_qty) with something that wasn't chosen at first because of the prefered order - quants.pop() - #make sure the quants aren't found twice (if the prefered_domain and the fallback_domain aren't orthogonal - domain += [('id', 'not in', quant_ids)] - unprefered_quants = self.quants_get(cr, uid, location, product, res_qty, domain=domain + fallback_domain, restrict_lot_id=restrict_lot_id, restrict_partner_id=restrict_partner_id, context=context) - for quant in unprefered_quants: - quants.append(quant) + for fallback_domain in fallback_domain_list: + if res_qty > 0: + #try to replace the last tuple (None, res_qty) with something that wasn't chosen at first because of the prefered order + quants.pop() + unprefered_quants = self.quants_get(cr, uid, location, product, res_qty, domain=domain + fallback_domain, restrict_lot_id=restrict_lot_id, restrict_partner_id=restrict_partner_id, context=context) + for quant in unprefered_quants: + if quant[0]: + res_qty -= quant[1] + quants += unprefered_quants return quants return self.quants_get(cr, uid, location, product, qty, domain=domain, restrict_lot_id=restrict_lot_id, restrict_partner_id=restrict_partner_id, context=context) @@ -510,8 +510,6 @@ class stock_quant(osv.osv): dom += [('lot_id', '=', quant.lot_id.id)] dom += [('owner_id', '=', quant.owner_id.id)] dom += [('package_id', '=', quant.package_id.id)] - if move.move_dest_id: - dom += [('negative_move_id', '=', move.move_dest_id.id)] quants = self.quants_get(cr, uid, quant.location_id, quant.product_id, quant.qty, dom, context=context) for quant_neg, qty in quants: if not quant_neg: @@ -555,10 +553,6 @@ class stock_quant(osv.osv): if move.partially_available: self.pool.get("stock.move").write(cr, uid, [move.id], {'partially_available': False}, context=context) self.write(cr, SUPERUSER_ID, related_quants, {'reservation_id': False}, context=context) - ancestors = self.find_move_ancestors(cr, uid, move, context=context) - if move.state == 'waiting' or ancestors: - for quant in move.reserved_quant_ids: - self._quant_reconcile_negative(cr, uid, quant, move, context=context) def _quants_get_order(self, cr, uid, location, product, quantity, domain=[], orderby='in_date', context=None): ''' Implementation of removal strategies @@ -2030,8 +2024,7 @@ class stock_move(osv.osv): """ Changes the state to assigned. @return: True """ - #when a MTO move availability is forced, it shouldn't be treated anymore as a MTO so we break the link with ancestors (to avoid further problems in re-reservation) - return self.write(cr, uid, ids, {'state': 'assigned', 'procure_method': 'make_to_stock', 'move_orig_ids': [(6, 0, [])]}, context=context) + return self.write(cr, uid, ids, {'state': 'assigned'}, context=context) def check_tracking(self, cr, uid, move, lot_id, context=None): """ Checks if serial number is assigned to stock move or not and raise an error if it had to. @@ -2096,7 +2089,7 @@ class stock_move(osv.osv): domain = main_domain[move.id] + self.pool.get('stock.move.operation.link').get_specific_domain(cr, uid, record, context=context) qty = record.qty if qty: - quants = quant_obj.quants_get_prefered_domain(cr, uid, ops.location_id, move.product_id, qty, domain=domain, prefered_domain=[], fallback_domain=[], restrict_lot_id=move.restrict_lot_id.id, restrict_partner_id=move.restrict_partner_id.id, context=context) + quants = quant_obj.quants_get_prefered_domain(cr, uid, ops.location_id, move.product_id, qty, domain=domain, prefered_domain=[], fallback_domain_list=[], restrict_lot_id=move.restrict_lot_id.id, restrict_partner_id=move.restrict_partner_id.id, context=context) quant_obj.quants_reserve(cr, uid, quants, move, record, context=context) for move in todo_moves: move.refresh() @@ -2104,7 +2097,7 @@ class stock_move(osv.osv): if move.state != 'assigned': qty_already_assigned = move.reserved_availability qty = move.product_qty - qty_already_assigned - quants = quant_obj.quants_get_prefered_domain(cr, uid, move.location_id, move.product_id, qty, domain=main_domain[move.id], prefered_domain=[], fallback_domain=[], restrict_lot_id=move.restrict_lot_id.id, restrict_partner_id=move.restrict_partner_id.id, context=context) + quants = quant_obj.quants_get_prefered_domain(cr, uid, move.location_id, move.product_id, qty, domain=main_domain[move.id], prefered_domain=[], fallback_domain_list=[], restrict_lot_id=move.restrict_lot_id.id, restrict_partner_id=move.restrict_partner_id.id, context=context) quant_obj.quants_reserve(cr, uid, quants, move, context=context) #force assignation of consumable products and incoming from supplier/inventory/production @@ -2152,18 +2145,6 @@ class stock_move(osv.osv): move2 = not move2.move_orig_ids and move2.split_from or False return ancestors - def check_quants_history(self, cr, uid, move, quants, context=None): - #raise an error in the case the move is chained MTO and the selected quants were not properly chained (manual change in pack operations) because: - #1) users must be wanred they break the MTO flow - #2) if left as it, it will create negative quants complex to reconcile as we allow to take quants only from ancestors, - # and we allow reconciliation only of negative quants created by next move when receiving a chained quant. - ancestors = self.find_move_ancestors(cr, uid, move, context=context) - if move.state == 'waiting' and not ancestors: - raise osv.except_osv(_('Error'), _('You cannot the requested operation. As the move is chained you are supposed to take the same products as in the previous step. Please use the values proposed or force the availability of the move to break the link and process it as you like.')) - if ancestors: - for q, dummy in quants: - if not q or not (set(ancestors) & set([m.id for m in q.history_ids])): - raise osv.except_osv(_('Error'), _('You cannot the requested operation. As the move is chained you are supposed to take the same products as in the previous step. Please use the values proposed or force the availability of the move to break the link and process it as you like.')) def action_done(self, cr, uid, ids, context=None): """ Process completly the moves given as ids and if all moves are done, it will finish the picking. @@ -2197,10 +2178,10 @@ class stock_move(osv.osv): self.check_tracking(cr, uid, move, ops.package_id.id or ops.lot_id.id, context=context) prefered_domain = [('reservation_id', '=', move.id)] fallback_domain = [('reservation_id', '=', False)] + fallback_domain2 = ['&', ('reservation_id', '!=', move.id), ('reservation_id', '!=', False)] dom = main_domain + self.pool.get('stock.move.operation.link').get_specific_domain(cr, uid, record, context=context) quants = quant_obj.quants_get_prefered_domain(cr, uid, ops.location_id, move.product_id, record.qty, domain=dom, prefered_domain=prefered_domain, - fallback_domain=fallback_domain, restrict_lot_id=move.restrict_lot_id.id, restrict_partner_id=move.restrict_partner_id.id, context=context) - self.check_quants_history(cr, uid, move, quants, context=context) + fallback_domain_list=[fallback_domain, fallback_domain2], restrict_lot_id=move.restrict_lot_id.id, restrict_partner_id=move.restrict_partner_id.id, context=context) if ops.result_package_id.id: #if a result package is given, all quants go there quant_dest_package_id = ops.result_package_id.id @@ -2221,10 +2202,10 @@ class stock_move(osv.osv): main_domain = [('qty', '>', 0)] prefered_domain = [('reservation_id', '=', move.id)] fallback_domain = [('reservation_id', '=', False)] + fallback_domain2 = ['&', ('reservation_id', '!=', move.id), ('reservation_id', '!=', False)] self.check_tracking(cr, uid, move, move.restrict_lot_id.id, context=context) qty = move_qty[move.id] - quants = quant_obj.quants_get_prefered_domain(cr, uid, move.location_id, move.product_id, qty, domain=main_domain, prefered_domain=prefered_domain, fallback_domain=fallback_domain, restrict_lot_id=move.restrict_lot_id.id, restrict_partner_id=move.restrict_partner_id.id, context=context) - self.check_quants_history(cr, uid, move, quants, context=context) + quants = quant_obj.quants_get_prefered_domain(cr, uid, move.location_id, move.product_id, qty, domain=main_domain, prefered_domain=prefered_domain, fallback_domain_list=[fallback_domain, fallback_domain2], restrict_lot_id=move.restrict_lot_id.id, restrict_partner_id=move.restrict_partner_id.id, context=context) quant_obj.quants_move(cr, uid, quants, move, move.location_dest_id, lot_id=move.restrict_lot_id.id, owner_id=move.restrict_partner_id.id, context=context) #unreserve the quants and make them available for other operations/moves quant_obj.quants_unreserve(cr, uid, move, context=context) diff --git a/addons/stock/test/wiseoperator.yml b/addons/stock/test/wiseoperator.yml index 7b188ba7d67..156174d5ee1 100644 --- a/addons/stock/test/wiseoperator.yml +++ b/addons/stock/test/wiseoperator.yml @@ -106,7 +106,7 @@ - !python {model: stock.quant}: | reco_id = self.search(cr ,uid , [('product_id','=',ref('product_wise')), ('qty', '<', 0.0)], context=context) - assert len(reco_id) > 0, 'This should have created a negative quant' + assert len(reco_id) == 0, 'This should not have created a negative quant' - Process the second picking - From d1cf0c62c44ac2d830b1c81128bfcf512f8206dd Mon Sep 17 00:00:00 2001 From: Josse Colpaert Date: Thu, 17 Apr 2014 17:33:14 +0200 Subject: [PATCH 2/4] [REVERT] Revert extended package constraint bzr revid: jco@openerp.com-20140417153314-jm7sfn5jytyqiqik --- addons/stock/stock.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/addons/stock/stock.py b/addons/stock/stock.py index 12646dbd2f4..dcd4ec887df 100644 --- a/addons/stock/stock.py +++ b/addons/stock/stock.py @@ -3511,13 +3511,8 @@ class stock_package(osv.osv): parent = parent.parent_id quant_ids = self.get_content(cr, uid, [parent.id], context=context) quants = quant_obj.browse(cr, uid, quant_ids, context=context) - normal_quants = [x for x in quants if not x.propagated_from_id if x.qty > 0] - propagated_quants = [x for x in quants if x.propagated_from_id if x.qty > 0] - location_id = normal_quants and normal_quants[0].location_id.id or False - prop_loc_id = propagated_quants and propagated_quants[0].location_id.id or False - all_normal = all([quant.location_id.id == location_id for quant in normal_quants]) - all_propagated = all([quant.location_id.id == prop_loc_id for quant in propagated_quants]) - if not all_normal or not all_propagated: + location_id = quants and quants[0].location_id.id or False + if not [quant.location_id.id == location_id for quant in quants if quant.qty > 0]: raise osv.except_osv(_('Error'), _('Everything inside a package should be in the same location')) return True From 0e387b568383159b2792c6874a2e1228fbc41d38 Mon Sep 17 00:00:00 2001 From: Josse Colpaert Date: Thu, 17 Apr 2014 18:16:22 +0200 Subject: [PATCH 3/4] [IMP] Recalculate states of other moves after taking from other moves bzr revid: jco@openerp.com-20140417161622-qmdjsfg2144jvxkk --- addons/stock/stock.py | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/addons/stock/stock.py b/addons/stock/stock.py index dcd4ec887df..edf1a6080cb 100644 --- a/addons/stock/stock.py +++ b/addons/stock/stock.py @@ -368,6 +368,7 @@ class stock_quant(osv.osv): to_move_quants.append(quant) quants_reconcile.append(quant) if to_move_quants: + self.recalculate_other_move_states(cr, uid, to_move_quants, move, context=context) self.move_quants_write(cr, uid, to_move_quants, move, location_to, dest_package_id, context=context) if location_to.usage == 'internal': if self.search(cr, uid, [('product_id', '=', move.product_id.id), ('qty','<', 0)], limit=1, context=context): @@ -375,6 +376,28 @@ class stock_quant(osv.osv): quant.refresh() self._quant_reconcile_negative(cr, uid, quant, move, context=context) + def recalculate_other_move_states(self, cr, uid, quants, move, context=None): + move_obj = self.pool.get("stock.move") + other_moves = [x.reservation_id for x in quants if x.reservation_id and x.reservation_id.id != move.id] + for o_move in other_moves: + vals = {} + reserved_quant_ids = o_move.reserved_quant_ids + if len(reserved_quant_ids) > 0 and not o_move.partially_available: + # Partially available is true + vals['partially_available'] = True + if len(reserved_quant_ids) == 0 and o_move.partially_available: + vals['partially_available'] = False + if o_move.state == 'assigned': + if move_obj.find_move_ancestors(cr, uid, move, context=context): + vals['state'] = 'waiting' + else: + vals['state'] = 'confirmed' + if vals: + move_obj.write(cr, uid, [o_move.id], vals, context=context) + + + + def move_quants_write(self, cr, uid, quants, move, location_dest_id, dest_package_id, context=None): vals = {'location_id': location_dest_id.id, 'history_ids': [(4, move.id)], @@ -3510,9 +3533,9 @@ class stock_package(osv.osv): while parent.parent_id: parent = parent.parent_id quant_ids = self.get_content(cr, uid, [parent.id], context=context) - quants = quant_obj.browse(cr, uid, quant_ids, context=context) + quants = [x for x in quant_obj.browse(cr, uid, quant_ids, context=context) if x.qty > 0] location_id = quants and quants[0].location_id.id or False - if not [quant.location_id.id == location_id for quant in quants if quant.qty > 0]: + if not [quant.location_id.id == location_id for quant in quants]: raise osv.except_osv(_('Error'), _('Everything inside a package should be in the same location')) return True From 6ea8251efbff94b61bb6836808fb54d525a5dc4f Mon Sep 17 00:00:00 2001 From: Josse Colpaert Date: Fri, 18 Apr 2014 10:35:39 +0200 Subject: [PATCH 4/4] [IMP] Add test and moves state should only be recalculated after the quants have been moved bzr revid: jco@openerp.com-20140418083539-njjh3kqwhdmiqcv5 --- addons/stock/stock.py | 18 +++++++++--------- addons/stock/test/wiseoperator.yml | 8 ++++++-- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/addons/stock/stock.py b/addons/stock/stock.py index edf1a6080cb..046b6a4944e 100644 --- a/addons/stock/stock.py +++ b/addons/stock/stock.py @@ -368,35 +368,32 @@ class stock_quant(osv.osv): to_move_quants.append(quant) quants_reconcile.append(quant) if to_move_quants: - self.recalculate_other_move_states(cr, uid, to_move_quants, move, context=context) + other_moves = [x.reservation_id for x in to_move_quants if x.reservation_id and x.reservation_id.id != move.id] self.move_quants_write(cr, uid, to_move_quants, move, location_to, dest_package_id, context=context) + self.recalculate_other_move_states(cr, uid, other_moves, context=context) if location_to.usage == 'internal': if self.search(cr, uid, [('product_id', '=', move.product_id.id), ('qty','<', 0)], limit=1, context=context): for quant in quants_reconcile: quant.refresh() self._quant_reconcile_negative(cr, uid, quant, move, context=context) - def recalculate_other_move_states(self, cr, uid, quants, move, context=None): + def recalculate_other_move_states(self, cr, uid, other_moves, context=None): move_obj = self.pool.get("stock.move") - other_moves = [x.reservation_id for x in quants if x.reservation_id and x.reservation_id.id != move.id] for o_move in other_moves: vals = {} + o_move.refresh() reserved_quant_ids = o_move.reserved_quant_ids if len(reserved_quant_ids) > 0 and not o_move.partially_available: - # Partially available is true vals['partially_available'] = True if len(reserved_quant_ids) == 0 and o_move.partially_available: vals['partially_available'] = False if o_move.state == 'assigned': - if move_obj.find_move_ancestors(cr, uid, move, context=context): + if move_obj.find_move_ancestors(cr, uid, o_move, context=context): vals['state'] = 'waiting' else: vals['state'] = 'confirmed' if vals: move_obj.write(cr, uid, [o_move.id], vals, context=context) - - - def move_quants_write(self, cr, uid, quants, move, location_dest_id, dest_package_id, context=None): vals = {'location_id': location_dest_id.id, @@ -1746,7 +1743,10 @@ class stock_move(osv.osv): if move.state in ('done', 'cancel'): raise osv.except_osv(_('Operation Forbidden!'), _('Cannot unreserve a done move')) quant_obj.quants_unreserve(cr, uid, move, context=context) - self.write(cr, uid, [move.id], {'state': 'confirmed'}, context=context) + if self.find_move_ancestors(cr, uid, move, context=context): + self.write(cr, uid, [move.id], {'state': 'waiting'}, context=context) + else: + self.write(cr, uid, [move.id], {'state': 'confirmed'}, context=context) def _prepare_procurement_from_move(self, cr, uid, move, context=None): origin = (move.group_id and (move.group_id.name + ":") or "") + (move.rule_id and move.rule_id.name or "/") diff --git a/addons/stock/test/wiseoperator.yml b/addons/stock/test/wiseoperator.yml index 156174d5ee1..cfe1180b545 100644 --- a/addons/stock/test/wiseoperator.yml +++ b/addons/stock/test/wiseoperator.yml @@ -95,7 +95,6 @@ pack_ids2 = [x.id for x in picking2.pack_operation_ids] stock_pack.write(cr, uid, pack_ids1, {'picking_id': picking2.id}) stock_pack.write(cr, uid, pack_ids2, {'picking_id': picking1.id}) - picking1.refresh() - Process this picking - @@ -107,6 +106,11 @@ !python {model: stock.quant}: | reco_id = self.search(cr ,uid , [('product_id','=',ref('product_wise')), ('qty', '<', 0.0)], context=context) assert len(reco_id) == 0, 'This should not have created a negative quant' +- + Check the other delivery order has changed its state back to partially available +- + !python {model: stock.picking}: | + assert self.browse(cr, uid, ref('delivery_order_wise2')).state == 'partially_available', "Delivery order 2 should be back in confirmed state" - Process the second picking - @@ -117,4 +121,4 @@ - !python {model: stock.quant}: | reco_id = self.search(cr ,uid , [('product_id','=',ref('product_wise'))], context=context) - assert all([x.location_id.id==ref('stock_location_customers') and x.qty > 0.0 for x in self.browse(cr, uid, reco_id)]), "Negative quant or wrong location detected" + assert all([x.location_id.id==ref('stock_location_customers') and x.qty > 0.0 for x in self.browse(cr, uid, reco_id)]), "Negative quant or wrong location detected" \ No newline at end of file