From 7e6b875d1e4dbe11472d55f71b4a6f54c312f9d6 Mon Sep 17 00:00:00 2001 From: "Tiffany Chang (tic)" Date: Fri, 29 Dec 2023 17:40:48 +0100 Subject: [PATCH] [FIX] stock: ensure reception report always uses quantity field Following quantitypocalypse (odoo/odoo#137864), a couple of `reserved_uom_qty` and `reserved_qty` references were left over. This was partially due to no tests for these tricky parts of the code, therefore we both fix these references to use the newer `quantity`/`quantity_product_uom` fields and add/update tests to ensure we avoid breaking these 2 use cases again. Also discovered during this fix, the remaining assigned qtys when unassigning done incoming moves from already reserved outgoing moves (i.e. the "1. batch reserved + individual picking unreserved" use case in the code comments) was NOT being calculated correctly. This also fixes that + adds a test for it. closes odoo/odoo#147815 Related: odoo/enterprise#53536 Signed-off-by: Steve Van Essche --- addons/stock/report/report_stock_reception.py | 26 ++-- addons/stock/tests/test_report.py | 144 +++++++++++++++++- 2 files changed, 150 insertions(+), 20 deletions(-) diff --git a/addons/stock/report/report_stock_reception.py b/addons/stock/report/report_stock_reception.py index 8f47a39c02d6a..a96abf03d9316 100644 --- a/addons/stock/report/report_stock_reception.py +++ b/addons/stock/report/report_stock_reception.py @@ -247,13 +247,13 @@ def action_assign(self, move_ids, qtys, in_ids): out.move_line_ids.move_id = new_out assigned_amount = 0 for move_line_id in new_out.move_line_ids: - if assigned_amount + move_line_id.reserved_qty > qty_to_link: - new_move_line = move_line_id.copy({'reserved_uom_qty': 0, 'quantity': 0}) - new_move_line.reserved_uom_qty = move_line_id.reserved_uom_qty - move_line_id.reserved_uom_qty = out.product_id.uom_id._compute_quantity(qty_to_link - assigned_amount, out.product_uom, rounding_method='HALF-UP') - new_move_line.reserved_uom_qty -= out.product_id.uom_id._compute_quantity(move_line_id.reserved_qty, out.product_uom, rounding_method='HALF-UP') + if assigned_amount + move_line_id.quantity_product_uom > qty_to_link: + new_move_line = move_line_id.copy({'quantity': 0}) + new_move_line.quantity = move_line_id.quantity + move_line_id.quantity = out.product_id.uom_id._compute_quantity(qty_to_link - assigned_amount, out.product_uom, rounding_method='HALF-UP') + new_move_line.quantity -= out.product_id.uom_id._compute_quantity(move_line_id.quantity_product_uom, out.product_uom, rounding_method='HALF-UP') move_line_id.move_id = out - assigned_amount += move_line_id.reserved_qty + assigned_amount += move_line_id.quantity_product_uom if float_compare(assigned_amount, qty_to_link, precision_rounding=out.product_id.uom_id.rounding) == 0: break @@ -301,7 +301,7 @@ def action_unassign(self, move_id, qty, in_ids): # 1. batch reserved + individual picking unreserved # 2. moves linked from backorder generation total_still_linked = sum(out.move_orig_ids.mapped('product_qty')) - new_move_vals = out._split(out.product_qty - total_still_linked) + new_move_vals = out._split(total_still_linked) if new_move_vals: new_move_vals[0]['procure_method'] = 'make_to_order' new_move_vals[0]['reservation_date'] = out.reservation_date @@ -316,15 +316,15 @@ def action_unassign(self, move_id, qty, in_ids): for move_line_id in new_out.move_line_ids: if reserved_amount_to_remain <= 0: break - if move_line_id.reserved_qty > reserved_amount_to_remain: - new_move_line = move_line_id.copy({'reserved_uom_qty': 0, 'quantity': 0}) - new_move_line.reserved_uom_qty = out.product_id.uom_id._compute_quantity(move_line_id.reserved_qty - reserved_amount_to_remain, move_line_id.product_uom_id, rounding_method='HALF-UP') - move_line_id.reserved_uom_qty -= new_move_line.reserved_uom_qty - new_move_line.move_id = out + if move_line_id.quantity_product_uom > reserved_amount_to_remain: + new_move_line = move_line_id.copy({'quantity': 0}) + new_move_line.quantity = out.product_id.uom_id._compute_quantity(move_line_id.quantity_product_uom - reserved_amount_to_remain, move_line_id.product_uom_id, rounding_method='HALF-UP') + move_line_id.quantity -= new_move_line.quantity + move_line_id.move_id = out break else: move_line_id.move_id = out - reserved_amount_to_remain -= move_line_id.reserved_qty + reserved_amount_to_remain -= move_line_id.quantity_product_uom (out | new_out)._compute_quantity() out.move_orig_ids = False new_out._recompute_state() diff --git a/addons/stock/tests/test_report.py b/addons/stock/tests/test_report.py index ba428cd6ff64c..7b7e0b9409c0a 100644 --- a/addons/stock/tests/test_report.py +++ b/addons/stock/tests/test_report.py @@ -1322,22 +1322,26 @@ def test_report_reception_2_two_receipts(self): shows corresponding potential allocations when receipts have differing states. """ # Creates delivery for reception report to match against + outgoing_qty = 100 delivery_form = Form(self.env['stock.picking'], view='stock.view_picking_form') delivery_form.partner_id = self.partner delivery_form.picking_type_id = self.picking_type_out with delivery_form.move_ids_without_package.new() as move_line: move_line.product_id = self.product - move_line.product_uom_qty = 100 + move_line.product_uom_qty = outgoing_qty delivery = delivery_form.save() delivery.action_confirm() # Create 2 receipts and check its reception report values + receipt1_qty = 5 + receipt2_qty = 3 + incoming_qty = receipt1_qty + receipt2_qty receipt_form = Form(self.env['stock.picking'], view='stock.view_picking_form') receipt_form.partner_id = self.partner receipt_form.picking_type_id = self.picking_type_in with receipt_form.move_ids_without_package.new() as move_line: move_line.product_id = self.product - move_line.product_uom_qty = 5 + move_line.product_uom_qty = receipt1_qty receipt1 = receipt_form.save() receipt_form = Form(self.env['stock.picking'], view='stock.view_picking_form') @@ -1345,10 +1349,10 @@ def test_report_reception_2_two_receipts(self): receipt_form.picking_type_id = self.picking_type_in with receipt_form.move_ids_without_package.new() as move_line: move_line.product_id = self.product - move_line.product_uom_qty = 3 + move_line.product_uom_qty = receipt2_qty receipt2 = receipt_form.save() - # check that report correctly merges not draft incoming quantities + # check that report correctly merges draft incoming quantities report = self.env['report.stock.report_reception'] report_values = report._get_report_values(docids=[receipt1.id, receipt2.id]) self.assertEqual(len(report_values['docs']), 2, "There should be 2 receipts to assign from in this report") @@ -1370,16 +1374,73 @@ def test_report_reception_2_two_receipts(self): all_lines = list(sources_to_lines.values())[0] # line quantities depends on done vs not done incoming quantities => should be 2 lines now self.assertEqual(len(all_lines), 2, "The report has wrong number of lines (1 assignable + 1 not).") - self.assertEqual(all_lines[0]['quantity'], 5, "The first move has wrong incoming qty to assign.") - self.assertTrue(all_lines[0]['is_qty_assignable'], "1 receipt is done => should have 1 reservable move.") - self.assertEqual(all_lines[1]['quantity'], 3, "The second move has wrong (expected) incoming qty.") + self.assertEqual(all_lines[0]['quantity'], receipt1_qty, "The first move has wrong incoming qty to assign.") + self.assertTrue(all_lines[0]['is_qty_assignable'], "1 receipt is confirmed => should have 1 reservable move.") + self.assertEqual(all_lines[1]['quantity'], receipt2_qty, "The second move has wrong (expected) incoming qty.") self.assertFalse(all_lines[1]['is_qty_assignable'], "1 receipt is draft => should have 1 non-assignable move.") + # check that we can assign incoming quantities from 2 different CONFIRMED receipts and then unassign just 1 of them afterwards + receipt2.action_confirm() + report_values = report._get_report_values(docids=[receipt1.id, receipt2.id]) + sources_to_lines = report_values['sources_to_lines'] + all_lines = list(sources_to_lines.values())[0] + self.assertEqual(len(all_lines), 1, "The report has wrong number of lines (1 outgoing move they are assignable to).") + self.assertEqual(all_lines[0]['quantity'], incoming_qty, "The total amount of incoming qty to assign should be receipt1 + receipt2's qties.") + self.assertTrue(all_lines[0]['is_qty_assignable'], "receipts are confirmed, incoming moves should be assignable.") + report.action_assign(delivery.move_ids_without_package.ids, [incoming_qty], (receipt1 | receipt2).move_ids_without_package.ids) + mto_move = delivery.move_ids_without_package.filtered(lambda m: m.procure_method == 'make_to_order') + non_mto_move = delivery.move_ids_without_package - mto_move + # check that assigned (MTO) move is correctly created + self.assertEqual(len(mto_move), 1, "Only 1 delivery move should be MTO") + self.assertEqual(len(non_mto_move), 1, "Remaining not-assigned outgoing qty should have split into separate move") + self.assertEqual(mto_move.product_uom_qty, incoming_qty, "Incorrect quantity split for MTO move") + self.assertEqual(mto_move.state, 'waiting', "MTO move state not correctly set") + # unassign only 1 of the incoming moves + report.action_unassign([mto_move.id], receipt2_qty, receipt2.move_ids_without_package.ids) + mto_move = delivery.move_ids_without_package.filtered(lambda m: m.procure_method == 'make_to_order') + non_mto_moves = delivery.move_ids_without_package - mto_move + self.assertEqual(len(mto_move), 1, "Only 1 delivery move should be MTO") + self.assertEqual(len(non_mto_moves), 2, "Original split not-assigned outgoing qty should still exist + new move of unassigned qty") + self.assertEqual(mto_move.product_uom_qty, receipt1_qty, "Incorrect quantity split for remaining MTO move qty") + self.assertEqual(mto_move.state, 'waiting', "MTO move state shouldn't have changed") + # check that report doesn't allow done and non-done moves at same time receipt1.button_validate() reason = report._get_report_values(docids=[receipt1.id, receipt2.id])['reason'] self.assertEqual(reason, "This report cannot be used for done and not done %s at the same time" % report._get_doc_types(), "empty report reason not shown") + # check that we can assign incoming quantities from 2 different DONE receipts and then unassign just 1 of them afterwards when reserved amounts in delivery + receipt2.button_validate() + # create clean delivery since moves are split in original delivery + new delivery will auto merge the moves + delivery.action_cancel() + delivery2 = delivery.copy() + self.env['stock.quant'].with_context(inventory_mode=True).create({ + 'product_id': self.product.id, + 'location_id': self.stock_location.id, + 'inventory_quantity': outgoing_qty + }).action_apply_inventory() + delivery2.action_confirm() + self.assertEqual(delivery2.move_ids_without_package.quantity, outgoing_qty, "Delivery move should already be reserved") + report.action_assign(delivery2.move_ids_without_package.ids, [incoming_qty], (receipt1 | receipt2).move_ids_without_package.ids) + mto_move = delivery2.move_ids_without_package.filtered(lambda m: m.procure_method == 'make_to_order') + non_mto_move = delivery2.move_ids_without_package - mto_move + # check that assigned (MTO) move is correctly created + self.assertEqual(len(mto_move), 1, "Only 1 delivery move should be MTO") + self.assertEqual(len(non_mto_move), 1, "Remaining not-assigned outgoing qty should have split into separate move") + self.assertEqual(mto_move.product_uom_qty, incoming_qty, "Incorrect quantity split for MTO move") + self.assertEqual(mto_move.state, 'assigned', "MTO move should still be reserved") + # unassign only 1 of the incoming moves + report.action_unassign([mto_move.id], receipt2_qty, receipt2.move_ids_without_package.ids) + mto_move = delivery2.move_ids_without_package.filtered(lambda m: m.procure_method == 'make_to_order') + non_mto_moves = delivery2.move_ids_without_package - mto_move + self.assertEqual(len(mto_move), 1, "Only 1 delivery move should be MTO") + self.assertEqual(len(non_mto_moves), 2, "Original split not-assigned outgoing qty should still exist + new move of unassigned qty") + self.assertEqual(mto_move.product_uom_qty, receipt1_qty, "Incorrect quantity split for remaining MTO move qty") + self.assertEqual(mto_move.quantity, receipt1_qty, "Incorrect reserved amount split for remaining MTO move qty") + self.assertEqual(mto_move.state, 'assigned', "MTO move state shouldn't have changed") + for move in non_mto_moves: + self.assertEqual(move.quantity, move.product_uom_qty, "Incorrect reserved amount split for remaining MTO move qty") + def test_report_reception_3_multiwarehouse(self): """ Check that reception report respects same warehouse for receipts and deliveries. @@ -1462,6 +1523,7 @@ def test_report_reception_5_move_splitting(self): """ Check the complicated use cases of correct move splitting when assigning/unassigning when: 1. Qty to assign is less than delivery qty demand 2. Delivery already has some reserved quants + 3. Receipt and delivery are not yet 'done' at time of assign/unassign """ incoming_qty = 4 outgoing_qty = 10 @@ -1596,3 +1658,71 @@ def test_report_reception_6_backorders(self): backorder.button_validate() for move in delivery.move_ids_without_package: self.assertEqual(move.state, 'assigned', "All delivery moves should be fully reserved now") + + def test_report_reception_7_done_receipt(self): + """ Check the complicated use cases of correct move splitting when assigning when: + 1. Outgoing qty is greater than incoming qty + total outgoing qty is already reserved + 2. Receipt is already done and then assigned + """ + + incoming_qty = 4 + outgoing_qty = 10 + self.env['stock.quant'].with_context(inventory_mode=True).create({ + 'product_id': self.product.id, + 'location_id': self.stock_location.id, + 'inventory_quantity': outgoing_qty + }).action_apply_inventory() + + # create delivery + receipt + delivery_form = Form(self.env['stock.picking'], view='stock.view_picking_form') + delivery_form.picking_type_id = self.picking_type_out + with delivery_form.move_ids_without_package.new() as move_line: + move_line.product_id = self.product + move_line.product_uom_qty = outgoing_qty + delivery = delivery_form.save() + delivery.action_confirm() + + receipt_form = Form(self.env['stock.picking'], view='stock.view_picking_form') + receipt_form.partner_id = self.partner + receipt_form.picking_type_id = self.picking_type_in + with receipt_form.move_ids_without_package.new() as move_line: + move_line.product_id = self.product + move_line.product_uom_qty = incoming_qty + receipt = receipt_form.save() + receipt.action_confirm() + receipt.button_validate() + + self.assertEqual(len(delivery.move_ids_without_package), 1) + self.assertEqual(delivery.move_ids_without_package.quantity, outgoing_qty, "Delivery move should already be reserved") + report = self.env['report.stock.report_reception'] + + # ------------------- + # check report assign + # ------------------- + report.action_assign(delivery.move_ids_without_package.ids, [incoming_qty], receipt.move_ids_without_package.ids) + mto_move = delivery.move_ids_without_package.filtered(lambda m: m.procure_method == 'make_to_order') + non_mto_move = delivery.move_ids_without_package - mto_move + + # check that delivery move splits correctly when receipt move is assigned to it, done receipt = can be assigned to reserved outs + self.assertEqual(len(delivery.move_ids_without_package), 2, "Delivery moves should have split into assigned + not assigned") + self.assertEqual(len(delivery.move_ids_without_package.move_orig_ids), 1, "Only 1 delivery + 1 receipt move should be assigned") + self.assertEqual(len(receipt.move_ids_without_package.move_dest_ids), 1, "Receipt move should remain unsplit") + + # check that assigned (MTO) move is correctly created + self.assertEqual(len(mto_move), 1, "Only 1 delivery move should be MTO") + self.assertEqual(mto_move.product_uom_qty, incoming_qty, "Incorrect quantity split for MTO move") + self.assertEqual(mto_move.quantity, incoming_qty, "Receipt IS done => assigned pre-reserved move reserved_qty = assigned receipt move qty") + self.assertEqual(mto_move.state, 'assigned', "MTO move state not correctly set") + + # check that non-assigned move has correct values + self.assertEqual(non_mto_move.product_uom_qty, outgoing_qty - incoming_qty, "Incorrect quantity split for non-MTO move") + self.assertEqual(non_mto_move.quantity, outgoing_qty - incoming_qty, "Remaining reserved qty not correctly linked to non-MTO move") + self.assertEqual(non_mto_move.state, 'assigned', "Remaining non-MTO reserved move should stay reserved") + + # --------------------- + # check report unassign + # --------------------- + report.action_unassign([mto_move.id], incoming_qty, receipt.move_ids_without_package.ids) + self.assertEqual(mto_move.product_uom_qty, incoming_qty, "Move quantities should be unchanged") + self.assertEqual(mto_move.procure_method, 'make_to_stock', "Procure method not correctly reset") + self.assertEqual(mto_move.state, 'assigned', "Unassigning receipt move shouldn't affect the out move reservation")