From dfd20da44f6f7834ff7772a5b33828dea0d72ced Mon Sep 17 00:00:00 2001 From: Mmequignon Date: Thu, 10 Aug 2023 09:28:21 +0200 Subject: [PATCH 01/13] shopfloor_reception: Fix qty_done computation --- shopfloor_reception/services/reception.py | 33 +++-- .../tests/test_return_scan_line.py | 7 +- shopfloor_reception/tests/test_select_move.py | 21 ++- .../tests/test_set_quantity.py | 124 ++++++++++++++++-- 4 files changed, 151 insertions(+), 34 deletions(-) diff --git a/shopfloor_reception/services/reception.py b/shopfloor_reception/services/reception.py index 5e322b5037..347868d6d7 100644 --- a/shopfloor_reception/services/reception.py +++ b/shopfloor_reception/services/reception.py @@ -256,12 +256,12 @@ def _scan_line__find_or_create_line(self, picking, move, qty_done=1): ) ) ) - if line: - # The line quantity to do needs to correspond to - # the remaining quantity to do of its move. - line.product_uom_qty = move.product_uom_qty - move.quantity_done - else: - qty_todo_remaining = max(0, move.product_uom_qty - move.quantity_done) + if not line: + qty_todo_remaining = max( + 0, + move.product_uom_qty + - sum(move.move_line_ids.mapped("product_uom_qty")), + ) values = move._prepare_move_line_vals(quantity=qty_todo_remaining) line = self.env["stock.move.line"].create(values) return self._scan_line__assign_user(picking, line, qty_done) @@ -751,7 +751,7 @@ def _response_for_set_lot(self, picking, line, message=None): message=message, ) - def _align_product_uom_qties(self, move): + def _align_display_product_uom_qty(self, line, response): # This method aligns product uom qties on move lines. # In the shopfloor context, we might have multiple users working at # the same time on the same move. This is done by creating one move line @@ -770,19 +770,18 @@ def _align_product_uom_qties(self, move): # If move is already done, do not update lines qties # if move.state in ("done", "cancel"): # return - + move = line.move_id qty_todo = move.product_uom_qty qty_done = sum(move.move_line_ids.mapped("qty_done")) rounding = move.product_id.uom_id.rounding compare = float_compare(qty_done, qty_todo, precision_rounding=rounding) - if compare < 1: # If qty done <= qty todo, align qty todo on move lines + if compare < 1: # If qty done < qty todo, align qty todo in the response remaining_todo = qty_todo - qty_done - # if we didn't bypass reservation update, the quant reservation - # would be reduced as much as the deduced quantity, which is wrong - # as we only moved the quantity to a new move line - lines = move.move_line_ids.with_context(bypass_reservation_update=True) - for line in lines: - line.product_uom_qty = line.qty_done + remaining_todo + line_todo = line.qty_done + remaining_todo + response["data"]["set_quantity"]["selected_move_line"][0][ + "quantity" + ] = line_todo + return response def _before_state__set_quantity(self, picking, line, message=None): # Used by inherting module see shopfloor_reception_packaging_dimension @@ -791,8 +790,7 @@ def _before_state__set_quantity(self, picking, line, message=None): def _response_for_set_quantity( self, picking, line, message=None, asking_confirmation=None ): - self._align_product_uom_qties(line.move_id) - return self._response( + response = self._response( next_state="set_quantity", data={ "selected_move_line": self._data_for_move_lines(line), @@ -801,6 +799,7 @@ def _response_for_set_quantity( }, message=message, ) + return self._align_display_product_uom_qty(line, response) def _response_for_set_destination(self, picking, line, message=None): return self._response( diff --git a/shopfloor_reception/tests/test_return_scan_line.py b/shopfloor_reception/tests/test_return_scan_line.py index 9f94e79dc8..9575ebdbf1 100644 --- a/shopfloor_reception/tests/test_return_scan_line.py +++ b/shopfloor_reception/tests/test_return_scan_line.py @@ -88,15 +88,18 @@ def test_scan_packaging_in_delivery(self): "barcode": self.product_a_packaging.barcode, }, ) - data = self.data.picking(return_picking) selected_move_line = self.get_new_move_lines() + move_line_data = self.data.move_lines(selected_move_line) + move_line_data[0]["quantity"] = 20.0 + # Displayed qtu todo is modified by _align_display_product_uom_qty + data = self.data.picking(return_picking) self.assert_response( response, next_state="set_quantity", data={ "confirmation_required": None, "picking": data, - "selected_move_line": self.data.move_lines(selected_move_line), + "selected_move_line": move_line_data, }, ) self.assertEqual(selected_move_line.qty_done, self.product_a_packaging.qty) diff --git a/shopfloor_reception/tests/test_select_move.py b/shopfloor_reception/tests/test_select_move.py index db35bc9fcd..4f0126dc99 100644 --- a/shopfloor_reception/tests/test_select_move.py +++ b/shopfloor_reception/tests/test_select_move.py @@ -209,9 +209,20 @@ def test_assign_shopfloor_user_to_line(self): self.assertEqual(other_move_line.shopfloor_user_id.id, False) def test_create_new_line_none_available(self): - # If all lines for a product are already assigned to a different user - # and there's still qty todo remaining - # a new line will be created for that qty todo. + # If there's already a move line for a given incoming move, + # we assigned the whole move's product_uom_qty to it. + # The reason for that is that when recomputing states for a given move + # if sum(move.move_line_ids.product_uom_qty) != move.product_uom_qty, + # then it's state won't be assigned. + # For instance: + # - user 1 selects line1 + # - user 2 selected line1 too + # - user 1 posts 20/40 goods + # - user 2 tries to process any qty, and it fails, because posting + # a move triggers the recompute of move's state + # To avoid that, the first created line gets + # product_uom_qty = move.product_uom_qty + # The next ones are getting 0. picking = self._create_picking() self.assertEqual(len(picking.move_line_ids), 2) selected_move_line = picking.move_line_ids.filtered( @@ -233,9 +244,11 @@ def test_create_new_line_none_available(self): "barcode": self.product_a.barcode, }, ) + # A new line has been created self.assertEqual(len(picking.move_line_ids), 3) created_line = picking.move_line_ids[2] - self.assertEqual(created_line.product_uom_qty, 7) + # And its product_uom_qty is 0 + self.assertEqual(created_line.product_uom_qty, 0.0) self.assertEqual(created_line.shopfloor_user_id.id, self.env.uid) def test_done_action(self): diff --git a/shopfloor_reception/tests/test_set_quantity.py b/shopfloor_reception/tests/test_set_quantity.py index 23614c1ab8..d090b5751d 100644 --- a/shopfloor_reception/tests/test_set_quantity.py +++ b/shopfloor_reception/tests/test_set_quantity.py @@ -454,6 +454,10 @@ def test_concurrent_update(self): ) self.assertEqual(len(selected_move_line), 1) self.assertEqual(selected_move_line.qty_done, 1.0) + self.assertEqual( + selected_move_line.product_uom_qty, + selected_move_line.move_id.product_uom_qty, + ) # Let's make the first user work a little bit, and pick a total of 4 units for __ in range(4): @@ -477,10 +481,13 @@ def test_concurrent_update(self): "scan_line", params={"picking_id": picking.id, "barcode": self.product_a.barcode}, ) + # The whole move's product_uom_qty has been assigned to the first created line. + # The new one gets 0.0 new_line = picking.move_line_ids.filtered( lambda l: l.product_id == self.product_a and l.shopfloor_user_id == manager_user ) + self.assertEqual(new_line.product_uom_qty, 0.0) move_lines = selected_move_line | new_line line_service_mapping = [ @@ -494,10 +501,6 @@ def test_concurrent_update(self): self.assertEqual(lines_qty_done, 6.0) # should be equal to the moves quantity_done self.assertEqual(lines_qty_done, move_lines.move_id.quantity_done) - # And also, the remaining qty is 4.0, then for each move line, - # product_uom_qty = line.qty_done + move's remaining_qty - for line in move_lines: - self.assertEqual(line.product_uom_qty, line.qty_done + 4.0) # Now, let the new user finish its work for __ in range(4): @@ -516,10 +519,12 @@ def test_concurrent_update(self): self.assertEqual(lines_qty_done, 10.0) self.assertEqual(lines_qty_done, move_lines.move_id.quantity_done) - # And also, the product_uom_qty should be == qty_done == 5.0 - for line in move_lines: - self.assertEqual(line.product_uom_qty, 5.0) - self.assertEqual(line.product_uom_qty, line.qty_done) + # However, product_uom_qty hasn't changed + self.assertEqual(selected_move_line.product_uom_qty, 10.0) + self.assertEqual(new_line.product_uom_qty, 0.0) + # And what's important is that the sum of lines's product_uom_qty is + # always == move's product_uom_qty + self.assertEqual(sum(move_lines.mapped("product_uom_qty")), 10.0) # However, if we pick more than move's product_uom_qty, then lines # product_uom_qty isn't updated, in order to be able to display an error @@ -582,10 +587,11 @@ def test_concurrent_update(self): ) self.assertEqual(selected_move_line.qty_done, 3.0) - self.assertEqual(selected_move_line.product_uom_qty, 3.0) - self.assertEqual(new_line.qty_done, 7.0) - self.assertEqual(new_line.product_uom_qty, 7.0) + + self.assertEqual(selected_move_line.product_uom_qty, 10.0) + self.assertEqual(new_line.product_uom_qty, 0.0) + self.assertEqual(sum(move_lines.mapped("product_uom_qty")), 10.0) # And everything's fine on the move move = move_lines.move_id @@ -636,3 +642,99 @@ def test_split_move_line(self): lambda l: l.product_id == self.product_a ) self.assertEqual(len(move_lines), 3) + + def test_concurrent_update_2(self): + self.menu.sudo().auto_post_line = True + self.input_location.sudo().active = True + # Test related to picking being set to "ready" once the first user posts + # its move line, hence the picking being not available in shopfloor afterwards. + + # The reason for that is that `_post_line` calls `_recompute_state`. + # If at this point there's more or less reserved qty than what's been ordered + # then state isn't computed as assigned. + + # This test ensure that this isn't the case anymore. + + # Creating the picking, selecting the move line. + picking = self._create_picking() + service_user_1 = self.service + service_user_1.dispatch("scan_document", params={"barcode": picking.name}) + service_user_1.dispatch( + "scan_line", + params={"picking_id": picking.id, "barcode": self.product_a.barcode}, + ) + move_line_user_1 = picking.move_line_ids.filtered( + lambda l: l.product_id == self.product_a + ) + # The only move line should have qty_done = 1 + self.assertEqual(move_line_user_1.qty_done, 1.0) + self.assertEqual(move_line_user_1.product_uom_qty, 10.0) + + # Now, concurrently pick products with another user for the same move + manager_user = self.shopfloor_manager + service_user_2 = self._get_service_for_user(manager_user) + service_user_2.dispatch("scan_document", params={"barcode": picking.name}) + service_user_2.dispatch( + "scan_line", + params={"picking_id": picking.id, "barcode": self.product_a.barcode}, + ) + # The whole move's product_uom_qty has been assigned to the first created line. + # The new one gets 0.0 + move_line_user_2 = picking.move_line_ids.filtered( + lambda l: l.product_id == self.product_a + and l.shopfloor_user_id == manager_user + ) + self.assertEqual(move_line_user_2.product_uom_qty, 0.0) + self.assertEqual(move_line_user_2.qty_done, 1.0) + + # At this point, both lines are referencing the same move + self.assertEqual(move_line_user_2.move_id, move_line_user_1.move_id) + + # A new move / picking will be created after it is posted. + # store the list of pickings to find it out after it is posted + # moves before + lines_before = self.env["stock.move.line"].search([]) + + # Now, post user_1 move line + response = service_user_1.dispatch( + "process_with_new_pack", + params={ + "picking_id": picking.id, + "selected_line_id": move_line_user_1.id, + "quantity": move_line_user_1.qty_done, + }, + ) + picking_data = self.data.picking(picking) + self.assert_response( + response, + next_state="set_destination", + data={ + "picking": picking_data, + "selected_move_line": self.data.move_lines(move_line_user_1), + }, + ) + + response = self.service.dispatch( + "set_destination", + params={ + "picking_id": picking.id, + "selected_line_id": move_line_user_1.id, + "location_name": self.input_location.name, + }, + ) + lines_after = self.env["stock.move.line"].search( + [("id", "not in", lines_before.ids)] + ) + + # After move_line is posted, its state is done, and its qty_done is 1.0 + self.assertEqual(move_line_user_1.state, "done") + + # The remaining one is still assigned + self.assertEqual(move_line_user_2.state, "assigned") + # As well as the new one + self.assertEqual(len(lines_after), 1) + + # And the total remaining qty to be done is 9.0 (10.0 - 1.0) + self.assertEqual( + lines_after.product_uom_qty + move_line_user_2.product_uom_qty, 9.0 + ) From f49eb1c26166ffc7d686290b38cbc85dea2e9c77 Mon Sep 17 00:00:00 2001 From: Mmequignon Date: Mon, 4 Sep 2023 10:55:48 +0200 Subject: [PATCH 02/13] shopfloor_reception: Split moves before posting --- shopfloor_reception/services/reception.py | 29 ++++++++++++++++------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/shopfloor_reception/services/reception.py b/shopfloor_reception/services/reception.py index 347868d6d7..fd651fba7e 100644 --- a/shopfloor_reception/services/reception.py +++ b/shopfloor_reception/services/reception.py @@ -1242,15 +1242,26 @@ def _post_shopfloor_created_line(self, selected_line): ) def _auto_post_line(self, selected_line): - new_move = selected_line.move_id.split_other_move_lines( - selected_line, intersection=True - ) - if new_move: - # A new move is created in case of partial quantity - new_move.extract_and_action_done() - return - # In case of full quantity, post the initial move - selected_line.move_id.extract_and_action_done() + # If user only processed 1/5 and is the only one working on the move, + # then selected_line is the only one related to this move. + # In such case, we must ensure there's another move line with the remaining + # quantity to do, so selected_line is extracted in a new move as expected. + if selected_line.product_uom_qty: + selected_line.product_uom_qty = 0 + move = selected_line.move_id + if selected_line.qty_done == move.product_uom_qty: + # In case of full quantity, post the initial move + selected_line.move_id.extract_and_action_done() + split_move_vals = move._split(selected_line.qty_done) + new_move = move.create(split_move_vals) + new_move.move_line_ids = selected_line + new_move._action_confirm(merge=False) + new_move._recompute_state() + new_move._action_assign() + lock = self._actions_for("lock") + lock.for_update(move) + move._recompute_state() + new_move.extract_and_action_done() def set_destination( self, picking_id, selected_line_id, location_name, confirmation=False From 74692b49863acbaea071a051052fe13aaef0b2d3 Mon Sep 17 00:00:00 2001 From: Mmequignon Date: Mon, 4 Sep 2023 11:19:51 +0200 Subject: [PATCH 03/13] shopfloor_reception: Fix uom computation --- shopfloor_reception/services/reception.py | 34 +++++++++++++++-------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/shopfloor_reception/services/reception.py b/shopfloor_reception/services/reception.py index fd651fba7e..a597c06d6a 100644 --- a/shopfloor_reception/services/reception.py +++ b/shopfloor_reception/services/reception.py @@ -772,15 +772,22 @@ def _align_display_product_uom_qty(self, line, response): # return move = line.move_id qty_todo = move.product_uom_qty - qty_done = sum(move.move_line_ids.mapped("qty_done")) - rounding = move.product_id.uom_id.rounding - compare = float_compare(qty_done, qty_todo, precision_rounding=rounding) - if compare < 1: # If qty done < qty todo, align qty todo in the response - remaining_todo = qty_todo - qty_done - line_todo = line.qty_done + remaining_todo - response["data"]["set_quantity"]["selected_move_line"][0][ - "quantity" - ] = line_todo + other_lines_qty_done = 0.0 + move_uom = move.product_uom + for move_line in (move.move_line_ids - line): + # Use move's uom + line_uom = move_line.product_uom_id + other_lines_qty_done += line_uom._compute_quantity( + move_line.qty_done, move_line.product_uom_id, round=False + ) + remaining_todo = qty_todo - other_lines_qty_done + # Change back to line uom + line_todo = line.product_uom_id._compute_quantity( + remaining_todo, move_uom, round=False + ) + response["data"]["set_quantity"]["selected_move_line"][0][ + "quantity" + ] = line_todo return response def _before_state__set_quantity(self, picking, line, message=None): @@ -1244,14 +1251,17 @@ def _post_shopfloor_created_line(self, selected_line): def _auto_post_line(self, selected_line): # If user only processed 1/5 and is the only one working on the move, # then selected_line is the only one related to this move. - # In such case, we must ensure there's another move line with the remaining + # In such case, we must ensure there's another move with the remaining # quantity to do, so selected_line is extracted in a new move as expected. if selected_line.product_uom_qty: selected_line.product_uom_qty = 0 move = selected_line.move_id - if selected_line.qty_done == move.product_uom_qty: + move_quantity = move.product_uom._compute_quantity( + move.product_uom_qty, selected_line.product_uom_id + ) + if selected_line.qty_done == move_quantity: # In case of full quantity, post the initial move - selected_line.move_id.extract_and_action_done() + return selected_line.move_id.extract_and_action_done() split_move_vals = move._split(selected_line.qty_done) new_move = move.create(split_move_vals) new_move.move_line_ids = selected_line From 7e9aaa13253878c7ade00be5f5bc69b81fca8df9 Mon Sep 17 00:00:00 2001 From: Mmequignon Date: Wed, 13 Sep 2023 13:42:26 +0200 Subject: [PATCH 04/13] shopfloor_reception: Make select picking by product optional --- shopfloor_reception/data/shopfloor_scenario_data.xml | 3 ++- shopfloor_reception/services/reception.py | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/shopfloor_reception/data/shopfloor_scenario_data.xml b/shopfloor_reception/data/shopfloor_scenario_data.xml index d9ef7cf264..67b04aa823 100644 --- a/shopfloor_reception/data/shopfloor_scenario_data.xml +++ b/shopfloor_reception/data/shopfloor_scenario_data.xml @@ -9,7 +9,8 @@ { "auto_post_line": true, - "allow_return": true + "allow_return": true, + "scan_location_or_pack_first": true } diff --git a/shopfloor_reception/services/reception.py b/shopfloor_reception/services/reception.py index a597c06d6a..4647550f31 100644 --- a/shopfloor_reception/services/reception.py +++ b/shopfloor_reception/services/reception.py @@ -851,7 +851,12 @@ def start(self): def _scan_document__get_handlers_by_type(self): return { "picking": self._scan_document__by_picking, - "product": self._scan_document__by_product, + # only add the handler if scan_location_or_pack_first is disabled + "product": ( + self._scan_document__by_product + if not self.work.menu.scan_location_or_pack_first + else None + ), "packaging": self._scan_document__by_packaging, "lot": self._scan_document__by_lot, "origin_move": self._scan_document__by_origin_move, From 6133aa7fafaecb92910db549d974412eaa40ecaf Mon Sep 17 00:00:00 2001 From: Mmequignon Date: Thu, 14 Sep 2023 16:56:30 +0200 Subject: [PATCH 05/13] shopfloor_reception: set_quantity - Add cancel button --- shopfloor_reception/services/reception.py | 44 +++++++++ .../tests/test_set_quantity_action.py | 90 +++++++++++++++++++ .../static/src/scenario/reception.js | 3 + .../static/src/scenario/reception_states.js | 15 ++-- 4 files changed, 144 insertions(+), 8 deletions(-) diff --git a/shopfloor_reception/services/reception.py b/shopfloor_reception/services/reception.py index 4647550f31..d9b1511046 100644 --- a/shopfloor_reception/services/reception.py +++ b/shopfloor_reception/services/reception.py @@ -1164,6 +1164,22 @@ def set_quantity( ) return self._response_for_set_quantity(picking, selected_line) + def set_quantity__cancel_action(self, picking_id, selected_line_id): + picking = self.env["stock.picking"].browse(picking_id) + selected_line = self.env["stock.move.line"].browse(selected_line_id) + message = self._check_picking_status(picking) + if message: + return self._response_for_set_quantity( + picking, selected_line, message=message + ) + if selected_line.exists(): + if selected_line.product_uom_qty: + stock = self._actions_for("stock") + stock.unmark_move_line_as_picked(selected_line) + else: + selected_line.unlink() + return self._response_for_select_move(picking) + def _set_quantity__process__set_qty_and_split(self, picking, line, quantity): move = line.move_id sum(move.move_line_ids.mapped("qty_done")) @@ -1442,6 +1458,16 @@ def set_quantity(self): "confirmation": {"type": "string", "nullable": True}, } + def set_quantity__cancel_action(self): + return { + "picking_id": {"coerce": to_int, "required": True, "type": "integer"}, + "selected_line_id": { + "coerce": to_int, + "type": "integer", + "required": True, + }, + } + def process_with_existing_pack(self): return { "picking_id": {"coerce": to_int, "required": True, "type": "integer"}, @@ -1573,6 +1599,9 @@ def _set_lot_next_states(self): def _set_quantity_next_states(self): return {"set_quantity", "select_move", "set_destination"} + def _set_quantity__cancel_action_next_states(self): + return {"set_quantity", "select_move"} + def _set_destination_next_states(self): return {"set_destination", "select_move"} @@ -1653,6 +1682,16 @@ def _schema_set_quantity(self): }, } + @property + def _schema_set_quantity__cancel_action(self): + return { + "selected_move_line": { + "type": "list", + "schema": {"type": "dict", "schema": self.schemas.move_line()}, + }, + "picking": {"type": "dict", "schema": self.schemas.picking()}, + } + @property def _schema_set_destination(self): return { @@ -1730,6 +1769,11 @@ def set_lot_confirm_action(self): def set_quantity(self): return self._response_schema(next_states=self._set_quantity_next_states()) + def set_quantity__cancel_action(self): + return self._response_schema( + next_states=self._set_quantity__cancel_action_next_states() + ) + def process_with_existing_pack(self): return self._response_schema( next_states=self._process_with_existing_pack_next_states() diff --git a/shopfloor_reception/tests/test_set_quantity_action.py b/shopfloor_reception/tests/test_set_quantity_action.py index 7cf981eacc..8881e64a5f 100644 --- a/shopfloor_reception/tests/test_set_quantity_action.py +++ b/shopfloor_reception/tests/test_set_quantity_action.py @@ -84,3 +84,93 @@ def test_process_without_package(self): }, ) self.assertFalse(self.selected_move_line.result_package_id) + + def test_cancel_action(self): + picking = self._create_picking() + move_product_a = picking.move_lines.filtered( + lambda l: l.product_id == self.product_a + ) + # User 1 and 2 selects the same picking + service_user_1 = self.service + service_user_1.dispatch("scan_document", params={"barcode": picking.name}) + user2 = self.shopfloor_manager + service_user_2 = self._get_service_for_user(user2) + response = service_user_2.dispatch( + "scan_document", params={"barcode": picking.name} + ) + # both users selects the same move + service_user_1.dispatch( + "scan_line", + params={"picking_id": picking.id, "barcode": self.product_a.barcode}, + ) + move_line_user_1 = move_product_a.move_line_ids + service_user_2.dispatch( + "scan_line", + params={"picking_id": picking.id, "barcode": self.product_a.barcode}, + ) + move_line_user_2 = move_product_a.move_line_ids - move_line_user_1 + # And both sets the qty done to 10 + service_user_1.dispatch( + "set_quantity", + params={ + "picking_id": picking.id, + "selected_line_id": move_line_user_1.id, + "quantity": 10, + }, + ) + service_user_2.dispatch( + "set_quantity", + params={ + "picking_id": picking.id, + "selected_line_id": move_line_user_2.id, + "quantity": 10, + }, + ) + # Users are blocked, product_uom_qty is 10, but both users have qty_done=10 + # on their move line, therefore, none of them can confirm + expected_message = { + "body": "You cannot process that much units.", + "message_type": "error", + } + response = service_user_1.dispatch( + "process_with_new_pack", + params={ + "picking_id": picking.id, + "selected_line_id": move_line_user_1.id, + "quantity": 10.0, + }, + ) + self.assertMessage(response, expected_message) + response = service_user_2.dispatch( + "process_with_new_pack", + params={ + "picking_id": picking.id, + "selected_line_id": move_line_user_2.id, + "quantity": 10.0, + }, + ) + self.assertMessage(response, expected_message) + # make user1 cancel + service_user_1.dispatch( + "set_quantity__cancel_action", + params={ + "picking_id": picking.id, + "selected_line_id": move_line_user_1.id, + }, + ) + # Since we reused the move line created by odoo for the first user, we only + # reset the line + self.assertTrue(move_line_user_1.exists()) + self.assertFalse(move_line_user_1.shopfloor_user_id) + self.assertEqual(move_line_user_1.qty_done, 0) + self.assertEqual(move_line_user_1.product_uom_qty, 10) + # make user cancel + service_user_2.dispatch( + "set_quantity__cancel_action", + params={ + "picking_id": picking.id, + "selected_line_id": move_line_user_2.id, + }, + ) + # This line has been created by shopfloor, therefore, we unlinked it + self.assertFalse(move_line_user_2.exists()) diff --git a/shopfloor_reception_mobile/static/src/scenario/reception.js b/shopfloor_reception_mobile/static/src/scenario/reception.js index f366d422d9..f921e21a35 100644 --- a/shopfloor_reception_mobile/static/src/scenario/reception.js +++ b/shopfloor_reception_mobile/static/src/scenario/reception.js @@ -154,6 +154,9 @@ const Reception = { + + + diff --git a/shopfloor_reception_mobile/static/src/scenario/reception_states.js b/shopfloor_reception_mobile/static/src/scenario/reception_states.js index 67e1c269fd..9ff37f360a 100644 --- a/shopfloor_reception_mobile/static/src/scenario/reception_states.js +++ b/shopfloor_reception_mobile/static/src/scenario/reception_states.js @@ -171,6 +171,7 @@ export const reception_states = function () { events: { qty_edit: "on_qty_edit", go_back: "on_back", + cancel: "on_cancel", }, on_qty_edit: (qty) => { this.scan_destination_qty = parseInt(qty, 10); @@ -188,14 +189,12 @@ export const reception_states = function () { ); }, on_cancel: () => { - // TODO: this endpoing is currently missing in the backend, - // and it's currently in the roadmap. - // Once it's implemented, uncomment this call. - // this.wait_call( - // this.odoo.call("cancel", { - // package_level_id: this.state.data.id, - // }) - // ); + this.wait_call( + this.odoo.call("set_quantity__cancel_action", { + picking_id: this.state.data.picking.id, + selected_line_id: this.line_being_handled.id, + }) + ); }, on_add_to_existing_pack: () => { this.wait_call( From 9cb142570c46fba7895fea7932bd3906983f7e3b Mon Sep 17 00:00:00 2001 From: Mmequignon Date: Fri, 15 Sep 2023 12:08:57 +0200 Subject: [PATCH 06/13] shopfloor_reception: fix product_uom_qty at move line create When creating a move line, set product_uom_qty = 0. Only when such line is posted this value will be aligned with qty_done. --- shopfloor_reception/services/reception.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/shopfloor_reception/services/reception.py b/shopfloor_reception/services/reception.py index d9b1511046..1b524f41f1 100644 --- a/shopfloor_reception/services/reception.py +++ b/shopfloor_reception/services/reception.py @@ -257,12 +257,7 @@ def _scan_line__find_or_create_line(self, picking, move, qty_done=1): ) ) if not line: - qty_todo_remaining = max( - 0, - move.product_uom_qty - - sum(move.move_line_ids.mapped("product_uom_qty")), - ) - values = move._prepare_move_line_vals(quantity=qty_todo_remaining) + values = move._prepare_move_line_vals() line = self.env["stock.move.line"].create(values) return self._scan_line__assign_user(picking, line, qty_done) @@ -1249,6 +1244,7 @@ def process_without_pack(self, picking_id, selected_line_id, quantity): return self._response_for_set_destination(picking, selected_line) def _post_line(self, selected_line): + selected_line.product_uom_qty = selected_line.qty_done if ( selected_line.picking_id.is_shopfloor_created and self.work.menu.allow_return From 285a68a791791998aa5da50a0685c0549c52cec0 Mon Sep 17 00:00:00 2001 From: Mmequignon Date: Fri, 15 Sep 2023 12:10:55 +0200 Subject: [PATCH 07/13] shopfloor_reception: more integration tests --- shopfloor_reception/tests/common.py | 27 +++++ .../tests/test_set_quantity.py | 108 +++++++++++++++++- 2 files changed, 133 insertions(+), 2 deletions(-) diff --git a/shopfloor_reception/tests/common.py b/shopfloor_reception/tests/common.py index 73da099b67..56e812ddb0 100644 --- a/shopfloor_reception/tests/common.py +++ b/shopfloor_reception/tests/common.py @@ -138,3 +138,30 @@ def _get_today_pickings(self): ], order="scheduled_date ASC", ) + + def assertMessage(self, response, expected_message): + message = response.get("message") + for key, value in expected_message.items(): + self.assertEqual(message.get(key), value) + + @classmethod + def _get_move_ids_from_response(cls, response): + state = response.get("next_state") + data = response["data"][state] + picking_data = data.get("pickings") or [data.get("picking")] + moves_data = [] + for picking in picking_data: + moves_data.extend(picking["moves"]) + return [move["id"] for move in moves_data] + + def _get_service_for_user(self, user): + user_env = self.env(user=user) + return self.get_service( + "reception", menu=self.menu, profile=self.profile, env=user_env + ) + + @classmethod + def _shopfloor_manager_values(cls): + vals = super()._shopfloor_manager_values() + vals["groups_id"] = [(6, 0, [cls.env.ref("stock.group_stock_user").id])] + return vals diff --git a/shopfloor_reception/tests/test_set_quantity.py b/shopfloor_reception/tests/test_set_quantity.py index d090b5751d..42039b4aff 100644 --- a/shopfloor_reception/tests/test_set_quantity.py +++ b/shopfloor_reception/tests/test_set_quantity.py @@ -552,7 +552,9 @@ def test_concurrent_update(self): "body": "You cannot process that much units.", } picking_data = self.data.picking(picking) + quantity_done_by_user = 1 for line, service in line_service_mapping: + quantity_done_by_user += 2 response = service.dispatch( "process_with_new_pack", params={ @@ -561,13 +563,15 @@ def test_concurrent_update(self): "quantity": line.qty_done, }, ) + line_data = self.data.move_lines(line) + line_data[0]["quantity"] = quantity_done_by_user self.assert_response( response, next_state="set_quantity", data={ "picking": picking_data, - "selected_move_line": self.data.move_lines(line), "confirmation_required": None, + "selected_move_line": line_data, }, message=error_msg, ) @@ -725,7 +729,6 @@ def test_concurrent_update_2(self): lines_after = self.env["stock.move.line"].search( [("id", "not in", lines_before.ids)] ) - # After move_line is posted, its state is done, and its qty_done is 1.0 self.assertEqual(move_line_user_1.state, "done") @@ -738,3 +741,104 @@ def test_concurrent_update_2(self): self.assertEqual( lines_after.product_uom_qty + move_line_user_2.product_uom_qty, 9.0 ) + + def test_move_states(self): + # as only assigned moves can be posted, we need to ensure that + # we got the right states in any case, especially when users are working + # concurrently + picking = self._create_picking() + move_product_a = picking.move_lines.filtered( + lambda l: l.product_id == self.product_a + ) + # user1 processes 10 units + move_line_user_1 = move_product_a.move_line_ids + service_user_1 = self.service + service_user_1.dispatch("scan_document", params={"barcode": picking.name}) + service_user_1.dispatch( + "scan_line", + params={"picking_id": picking.id, "barcode": self.product_a.barcode}, + ) + response = service_user_1.dispatch( + "set_quantity", + params={ + "picking_id": picking.id, + "selected_line_id": move_line_user_1.id, + "quantity": move_product_a.product_qty - 1, + "barcode": self.product_a.barcode, + }, + ) + # user2 selects the same picking + user2 = self.shopfloor_manager + service_user_2 = self._get_service_for_user(user2) + response = service_user_2.dispatch( + "scan_document", params={"barcode": picking.name} + ) + # And the same line + service_user_2.dispatch( + "scan_line", + params={"picking_id": picking.id, "barcode": self.product_a.barcode}, + ) + move_line_user_2 = move_product_a.move_line_ids - move_line_user_1 + # user1 shouldn't be able to process his move, since + # move qty_done > move product_qty + response = service_user_1.dispatch( + "process_with_new_pack", + params={ + "picking_id": picking.id, + "selected_line_id": move_line_user_1.id, + "quantity": 10.0, + }, + ) + # + expected_message = { + "body": "You cannot process that much units.", + "message_type": "error", + } + self.assertMessage(response, expected_message) + # user1 cancels the operation + service_user_1.dispatch( + "set_quantity__cancel_action", + params={ + "picking_id": picking.id, + "selected_line_id": move_line_user_1.id, + }, + ) + self.assertFalse(move_line_user_1.shopfloor_user_id) + self.assertEqual(move_line_user_1.qty_done, 0) + # User2 should be able to process 1 unit + response = service_user_2.dispatch( + "process_with_new_pack", + params={ + "picking_id": picking.id, + "selected_line_id": move_line_user_2.id, + "quantity": 1.0, + }, + ) + data = self.data.picking(picking) + self.assert_response( + response, + next_state="set_destination", + data={ + "picking": data, + "selected_move_line": self.data.move_lines(move_line_user_2), + }, + ) + self.assertEqual(move_product_a.quantity_done, 1.0) + response = service_user_2.dispatch( + "set_destination", + params={ + "picking_id": picking.id, + "selected_line_id": move_line_user_2.id, + "location_name": self.dispatch_location.name, + }, + ) + # When posted, the move line product_uom_qty has been set to qty_done + self.assertEqual(move_line_user_2.qty_done, move_line_user_2.product_qty) + self.assert_response( + response, next_state="select_move", data=self._data_for_select_move(picking) + ) + # Now, user1 can start working on this again + service_user_1.dispatch( + "scan_line", + params={"picking_id": picking.id, "barcode": self.product_a.barcode}, + ) From 36c795c9958b25d16ffd68573dd63c84dccfa010 Mon Sep 17 00:00:00 2001 From: Thierry Ducrest Date: Fri, 20 Oct 2023 11:15:32 +0200 Subject: [PATCH 08/13] shopfloor_reception: Fix product_uom_qty on move line --- shopfloor_reception/services/reception.py | 24 ++++++++++++------- .../tests/test_set_destination.py | 5 ++-- .../tests/test_set_quantity.py | 18 +++++++------- 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/shopfloor_reception/services/reception.py b/shopfloor_reception/services/reception.py index 1b524f41f1..45349e73ec 100644 --- a/shopfloor_reception/services/reception.py +++ b/shopfloor_reception/services/reception.py @@ -758,9 +758,6 @@ def _align_display_product_uom_qty(self, line, response): # remaining_todo = move.product_uom_qty - move.quantity_done # line.product_uom_qty = line.qty_done + remaining_todo - # However, if the overall qty_done is > to the move's product_uom_qty, - # then we're not updating the line's product_uom_qty. - # TODO, do we need to check move's state? # If move is already done, do not update lines qties # if move.state in ("done", "cancel"): @@ -769,7 +766,7 @@ def _align_display_product_uom_qty(self, line, response): qty_todo = move.product_uom_qty other_lines_qty_done = 0.0 move_uom = move.product_uom - for move_line in (move.move_line_ids - line): + for move_line in move.move_line_ids - line: # Use move's uom line_uom = move_line.product_uom_id other_lines_qty_done += line_uom._compute_quantity( @@ -780,9 +777,10 @@ def _align_display_product_uom_qty(self, line, response): line_todo = line.product_uom_id._compute_quantity( remaining_todo, move_uom, round=False ) - response["data"]["set_quantity"]["selected_move_line"][0][ - "quantity" - ] = line_todo + # If more has been done keep the quantity to zero + response["data"]["set_quantity"]["selected_move_line"][0]["quantity"] = max( + line_todo, 0 + ) return response def _before_state__set_quantity(self, picking, line, message=None): @@ -1270,8 +1268,16 @@ def _auto_post_line(self, selected_line): # then selected_line is the only one related to this move. # In such case, we must ensure there's another move with the remaining # quantity to do, so selected_line is extracted in a new move as expected. - if selected_line.product_uom_qty: - selected_line.product_uom_qty = 0 + + # Always keep the quantity todo at zero, the same is done + # in Odoo when move lines are created manually (setting) + lines_with_qty_todo = selected_line.move_id.move_line_ids.filtered( + lambda line: line.state not in ("cancel", "done") + and line.product_uom_qty > 0 + ) + if lines_with_qty_todo: + lines_with_qty_todo.product_uom_qty = 0 + move = selected_line.move_id move_quantity = move.product_uom._compute_quantity( move.product_uom_qty, selected_line.product_uom_id diff --git a/shopfloor_reception/tests/test_set_destination.py b/shopfloor_reception/tests/test_set_destination.py index 7a9ec07e35..dc4a20e02e 100644 --- a/shopfloor_reception/tests/test_set_destination.py +++ b/shopfloor_reception/tests/test_set_destination.py @@ -145,11 +145,10 @@ def test_auto_posting(self): self.assertEqual(selected_move_line.picking_id.state, "done") # The line that remained in the original picking - # for that product has a product_uom_qty of 7 - # and a qty_done of 0. line_in_picking = picking.move_line_ids.filtered( lambda l: l.product_id == selected_move_line.product_id ) - self.assertEqual(line_in_picking.product_uom_qty, 7) + # New created line always quantity to do at zero + self.assertEqual(line_in_picking.product_uom_qty, 0) self.assertEqual(line_in_picking.qty_done, 0) self.assertEqual(picking.state, "assigned") diff --git a/shopfloor_reception/tests/test_set_quantity.py b/shopfloor_reception/tests/test_set_quantity.py index 42039b4aff..98fbdc5373 100644 --- a/shopfloor_reception/tests/test_set_quantity.py +++ b/shopfloor_reception/tests/test_set_quantity.py @@ -661,15 +661,14 @@ def test_concurrent_update_2(self): # Creating the picking, selecting the move line. picking = self._create_picking() + move = picking.move_lines.filtered(lambda l: l.product_id == self.product_a) service_user_1 = self.service service_user_1.dispatch("scan_document", params={"barcode": picking.name}) service_user_1.dispatch( "scan_line", params={"picking_id": picking.id, "barcode": self.product_a.barcode}, ) - move_line_user_1 = picking.move_line_ids.filtered( - lambda l: l.product_id == self.product_a - ) + move_line_user_1 = move.move_line_ids # The only move line should have qty_done = 1 self.assertEqual(move_line_user_1.qty_done, 1.0) self.assertEqual(move_line_user_1.product_uom_qty, 10.0) @@ -732,15 +731,14 @@ def test_concurrent_update_2(self): # After move_line is posted, its state is done, and its qty_done is 1.0 self.assertEqual(move_line_user_1.state, "done") - # The remaining one is still assigned - self.assertEqual(move_line_user_2.state, "assigned") + # The remaining one is still to be done + self.assertEqual(move_line_user_2.state, "confirmed") # As well as the new one self.assertEqual(len(lines_after), 1) - - # And the total remaining qty to be done is 9.0 (10.0 - 1.0) - self.assertEqual( - lines_after.product_uom_qty + move_line_user_2.product_uom_qty, 9.0 - ) + # The total remaining qty to be done on line is always zero + # Because it is computed in the frontend + self.assertEqual(lines_after.product_uom_qty, 0) + self.assertEqual(move_line_user_2.product_uom_qty, 0) def test_move_states(self): # as only assigned moves can be posted, we need to ensure that From 90ee4016a349ae9549c0d6f56d3ed40ff39be616 Mon Sep 17 00:00:00 2001 From: Thierry Ducrest Date: Wed, 8 Nov 2023 11:22:32 +0100 Subject: [PATCH 09/13] sf_reception: remove assigning user to picking Because many users can work on the same picking at the same time and the scenario never uses this information. It is not needed. --- shopfloor_reception/services/reception.py | 9 --------- shopfloor_reception/tests/test_select_move.py | 12 ------------ 2 files changed, 21 deletions(-) diff --git a/shopfloor_reception/services/reception.py b/shopfloor_reception/services/reception.py index 45349e73ec..6de458e2cc 100644 --- a/shopfloor_reception/services/reception.py +++ b/shopfloor_reception/services/reception.py @@ -137,12 +137,10 @@ def _select_picking(self, picking): return self._response_for_select_move(picking) def _response_for_select_move(self, picking, message=None): - self._assign_user_to_picking(picking) data = {"picking": self._data_for_stock_picking(picking, with_lines=True)} return self._response(next_state="select_move", data=data, message=message) def _response_for_confirm_done(self, picking, message=None): - self._assign_user_to_picking(picking) data = {"picking": self._data_for_stock_picking(picking, with_lines=True)} return self._response(next_state="confirm_done", data=data, message=message) @@ -161,7 +159,6 @@ def _response_for_confirm_new_package( def _select_document_from_move_lines(self, move_lines, msg_func): pickings = move_lines.move_id.picking_id if len(pickings) == 1: - self._assign_user_to_picking(pickings) if ( move_lines.product_id.tracking not in ("lot", "serial") or move_lines.lot_id @@ -263,7 +260,6 @@ def _scan_line__find_or_create_line(self, picking, move, qty_done=1): def _scan_line__assign_user(self, picking, line, qty_done): product = line.product_id - self._assign_user_to_picking(picking) self._assign_user_to_line(line) line.qty_done += qty_done if product.tracking not in ("lot", "serial") or (line.lot_id or line.lot_name): @@ -328,7 +324,6 @@ def _scan_document__by_picking(self, pickings, barcode): < today_end ) if len(picking_filter_result_due_today) == 1: - self._assign_user_to_picking(picking_filter_result_due_today) return self._select_picking(picking_filter_result_due_today) if len(picking_filter_result) > 1: return self._response_for_select_document( @@ -682,9 +677,6 @@ def _use_handlers(self, handlers, *args, **kwargs): if response: return response - def _assign_user_to_picking(self, picking): - picking.user_id = self.env.user - def _assign_user_to_line(self, line): line.shopfloor_user_id = self.env.user @@ -1404,7 +1396,6 @@ def select_dest_package( return response return self._response_for_select_move(picking) message = self.msg_store.create_new_pack_ask_confirmation(barcode) - self._assign_user_to_picking(picking) return self._response_for_confirm_new_package( picking, selected_line, new_package_name=barcode, message=message ) diff --git a/shopfloor_reception/tests/test_select_move.py b/shopfloor_reception/tests/test_select_move.py index 4f0126dc99..091ae3f60c 100644 --- a/shopfloor_reception/tests/test_select_move.py +++ b/shopfloor_reception/tests/test_select_move.py @@ -176,18 +176,6 @@ def test_scan_packaging_not_found(self): message={"message_type": "warning", "body": error_msg}, ) - def test_assign_user_to_picking(self): - picking = self._create_picking() - self.assertEqual(picking.user_id.id, False) - self.service.dispatch( - "scan_line", - params={ - "picking_id": picking.id, - "barcode": self.product_a.barcode, - }, - ) - self.assertEqual(picking.user_id.id, self.env.uid) - def test_assign_shopfloor_user_to_line(self): picking = self._create_picking() for line in picking.move_line_ids: From 6d381aa99d1ec4bd02975ac6d1a5b9e2478471db Mon Sep 17 00:00:00 2001 From: Thierry Ducrest Date: Wed, 8 Nov 2023 18:15:54 +0100 Subject: [PATCH 10/13] shopfloor_base: improve lock for update action Add the skip locked option. --- shopfloor_base/actions/lock.py | 25 +++++++++++++++---- shopfloor_base/tests/__init__.py | 1 + shopfloor_base/tests/test_actions_lock.py | 29 +++++++++++++++++++++++ 3 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 shopfloor_base/tests/test_actions_lock.py diff --git a/shopfloor_base/actions/lock.py b/shopfloor_base/actions/lock.py index 6c5148ada7..f74fa68a3e 100644 --- a/shopfloor_base/actions/lock.py +++ b/shopfloor_base/actions/lock.py @@ -1,4 +1,5 @@ # Copyright 2022 Michael Tietz (MT Software) +# Copyright 2023 Camptocamp SA (http://www.camptocamp.com) # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html). import hashlib import struct @@ -26,7 +27,23 @@ def advisory(self, name): self.env.cr.execute("SELECT pg_advisory_xact_lock(%s);", (int_lock,)) self.env.cr.fetchone()[0] - def for_update(self, records, log_exceptions=False): - """Lock a table FOR UPDATE""" - sql = "SELECT id FROM %s WHERE ID IN %%s FOR UPDATE" % records._table - self.env.cr.execute(sql, (tuple(records.ids),), log_exceptions=False) + def for_update(self, records, log_exceptions=False, skip_locked=False): + """Lock rows for update on a specific table. + + This function will try to obtain a lock on the rows (records parameter) and + wait until they are available for update. + + Using the SKIP LOCKED parameter (better used with only one record), it will + not wait for the row to be available but return False if the lock could not + be obtained. + + """ + query = "SELECT id FROM %s WHERE ID IN %%s FOR UPDATE" + if skip_locked: + query += " SKIP LOCKED" + sql = query % records._table + self.env.cr.execute(sql, (tuple(records.ids),), log_exceptions=log_exceptions) + if skip_locked: + rows = self.env.cr.fetchall() + return len(rows) == len(records) + return True diff --git a/shopfloor_base/tests/__init__.py b/shopfloor_base/tests/__init__.py index a82e511759..5936ceae86 100644 --- a/shopfloor_base/tests/__init__.py +++ b/shopfloor_base/tests/__init__.py @@ -1,4 +1,5 @@ from . import test_actions_data +from . import test_actions_lock from . import test_menu_service from . import test_profile_service from . import test_scan_anything_service diff --git a/shopfloor_base/tests/test_actions_lock.py b/shopfloor_base/tests/test_actions_lock.py new file mode 100644 index 0000000000..ee5a7cba7e --- /dev/null +++ b/shopfloor_base/tests/test_actions_lock.py @@ -0,0 +1,29 @@ +# Copyright 2023 Camptocamp SA (http://www.camptocamp.com) +# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html). +from contextlib import closing + +from .common import CommonCase + + +class ActionsLockCase(CommonCase): + @classmethod + def setUpClassBaseData(cls): + super().setUpClassBaseData() + cls.partner = cls.env.ref("base.res_partner_12") + with cls.work_on_actions(cls) as work: + cls.lock = work.component(usage="lock") + + def test_select_for_update_skip_locked_ok(self): + """Check the lock is obtained and True is returned.""" + result = self.lock.for_update(self.partner, skip_locked=True) + self.assertTrue(result) + + def test_select_for_update_skip_locked_not_ok(self): + """Check the lock is NOT obtained and False is returned.""" + with closing(self.registry.cursor()) as cr: + # Simulate another user locked a row + cr.execute( + "SELECT id FROM res_partner WHERE id=%s FOR UPDATE", (self.partner.id,) + ) + result = self.lock.for_update(self.partner, skip_locked=True) + self.assertFalse(result) From ed5e320eb3d528c39c2bd9b4ad53de2dc5e2fe02 Mon Sep 17 00:00:00 2001 From: Thierry Ducrest Date: Wed, 8 Nov 2023 18:35:54 +0100 Subject: [PATCH 11/13] shopfloor_reception: improve move line assignation to user --- shopfloor_reception/services/reception.py | 32 +++++++++++++++++------ 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/shopfloor_reception/services/reception.py b/shopfloor_reception/services/reception.py index 6de458e2cc..45fa68d969 100644 --- a/shopfloor_reception/services/reception.py +++ b/shopfloor_reception/services/reception.py @@ -245,14 +245,30 @@ def _select_document_from_lot(self, lot): ) def _scan_line__find_or_create_line(self, picking, move, qty_done=1): - line = fields.first( - move.move_line_ids.filtered( - lambda l: ( - not l.result_package_id - and l.shopfloor_user_id.id in [False, self.env.uid] - ) - ) - ) + """Find or create a line on a move for the user to work on. + + First try to find a line already assigned to the user. + Then a line that is not yet assigned to any users (locking the line + to avoid concurent access.) + If none are found create a new line. + + """ + line = None + unassigned_lines = self.env["stock.move.line"] + for move_line in move.move_line_ids: + if move_line.result_package_id: + continue + if move_line.shopfloor_user_id.id == self.env.uid: + line = move_line + break + elif not move_line.shopfloor_user_id: + unassigned_lines |= move_line + if not line and unassigned_lines: + lock = self._actions_for("lock") + for move_line in unassigned_lines: + if lock.for_update(move_line, skip_locked=True): + line = move_line + break if not line: values = move._prepare_move_line_vals() line = self.env["stock.move.line"].create(values) From 20560a6dad4c673a8b6ecb7012d620317b7962d7 Mon Sep 17 00:00:00 2001 From: Thierry Ducrest Date: Wed, 15 Nov 2023 11:34:22 +0100 Subject: [PATCH 12/13] shopfloor_reception: fix lock on move when done --- shopfloor_reception/services/reception.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shopfloor_reception/services/reception.py b/shopfloor_reception/services/reception.py index 45fa68d969..c1ebb4959e 100644 --- a/shopfloor_reception/services/reception.py +++ b/shopfloor_reception/services/reception.py @@ -1283,10 +1283,12 @@ def _auto_post_line(self, selected_line): lambda line: line.state not in ("cancel", "done") and line.product_uom_qty > 0 ) + move = selected_line.move_id + lock = self._actions_for("lock") + lock.for_update(move) if lines_with_qty_todo: lines_with_qty_todo.product_uom_qty = 0 - move = selected_line.move_id move_quantity = move.product_uom._compute_quantity( move.product_uom_qty, selected_line.product_uom_id ) @@ -1299,8 +1301,6 @@ def _auto_post_line(self, selected_line): new_move._action_confirm(merge=False) new_move._recompute_state() new_move._action_assign() - lock = self._actions_for("lock") - lock.for_update(move) move._recompute_state() new_move.extract_and_action_done() From 8e35df0038bffa9674060f7a5cb7067066e8c15d Mon Sep 17 00:00:00 2001 From: Thierry Ducrest Date: Thu, 16 Nov 2023 15:07:27 +0100 Subject: [PATCH 13/13] shopfloor_reception: fix auto post option When 2 users work on the same move and one user completes his picking, The move containing the quantity left to do could not be switch back to assigned. --- shopfloor_reception/services/reception.py | 11 ++++ .../tests/test_set_destination.py | 51 ++++++++++++++++++- .../tests/test_set_quantity.py | 8 ++- 3 files changed, 63 insertions(+), 7 deletions(-) diff --git a/shopfloor_reception/services/reception.py b/shopfloor_reception/services/reception.py index c1ebb4959e..0687aa532d 100644 --- a/shopfloor_reception/services/reception.py +++ b/shopfloor_reception/services/reception.py @@ -1301,6 +1301,17 @@ def _auto_post_line(self, selected_line): new_move._action_confirm(merge=False) new_move._recompute_state() new_move._action_assign() + # Set back the quantity to do on one of the lines + line = fields.first( + move.move_line_ids.filtered( + lambda line: line.state not in ("cancel", "done") + ) + ) + if line: + move_quantity = move.product_uom._compute_quantity( + move.product_uom_qty, line[0].product_uom_id + ) + line.product_uom_qty = move_quantity move._recompute_state() new_move.extract_and_action_done() diff --git a/shopfloor_reception/tests/test_set_destination.py b/shopfloor_reception/tests/test_set_destination.py index dc4a20e02e..802f6d4b02 100644 --- a/shopfloor_reception/tests/test_set_destination.py +++ b/shopfloor_reception/tests/test_set_destination.py @@ -148,7 +148,54 @@ def test_auto_posting(self): line_in_picking = picking.move_line_ids.filtered( lambda l: l.product_id == selected_move_line.product_id ) - # New created line always quantity to do at zero - self.assertEqual(line_in_picking.product_uom_qty, 0) + self.assertEqual(line_in_picking.product_uom_qty, 7) self.assertEqual(line_in_picking.qty_done, 0) self.assertEqual(picking.state, "assigned") + + def test_auto_posting_concurent_work(self): + """Check 2 users working on the same move. + + With the auto post line option On. + + """ + self.menu.sudo().auto_post_line = True + picking = self._create_picking(lines=[(self.product_a, 10)]) + move = picking.move_lines + # User 1 starts working + service_u1 = self.service + res_u1 = service_u1.dispatch( + "manual_select_move", + params={"move_id": move.id}, + ) + # User 2 starts working on the same move + service_u2 = self._get_service_for_user(self.shopfloor_manager) + service_u2.dispatch( + "manual_select_move", + params={"move_id": move.id}, + ) + self.assertEqual(len(move.move_line_ids), 2) + # User 1 finishes his work + move_line_data = res_u1["data"]["set_quantity"]["selected_move_line"][0] + line_id_u1 = move_line_data["id"] + qty_done_u1 = move_line_data["qty_done"] + res_u1 = service_u1.dispatch( + "process_without_pack", + params={ + "picking_id": picking.id, + "selected_line_id": line_id_u1, + "quantity": qty_done_u1, + }, + ) + res_u1 = service_u1.dispatch( + "set_destination", + params={ + "picking_id": picking.id, + "selected_line_id": line_id_u1, + "location_name": self.dispatch_location.name, + }, + ) + # With the auto post line option + # The work done is moved and done in a specific transfer + self.assertEqual(picking.state, "assigned") + # So the quantity left to do on the current move has decreased + self.assertEqual(move.product_uom_qty, 9) diff --git a/shopfloor_reception/tests/test_set_quantity.py b/shopfloor_reception/tests/test_set_quantity.py index 98fbdc5373..2c50385543 100644 --- a/shopfloor_reception/tests/test_set_quantity.py +++ b/shopfloor_reception/tests/test_set_quantity.py @@ -730,15 +730,13 @@ def test_concurrent_update_2(self): ) # After move_line is posted, its state is done, and its qty_done is 1.0 self.assertEqual(move_line_user_1.state, "done") - # The remaining one is still to be done - self.assertEqual(move_line_user_2.state, "confirmed") + self.assertEqual(move_line_user_2.state, "assigned") # As well as the new one self.assertEqual(len(lines_after), 1) - # The total remaining qty to be done on line is always zero - # Because it is computed in the frontend + # The quantity to do is set on 1 of the lines self.assertEqual(lines_after.product_uom_qty, 0) - self.assertEqual(move_line_user_2.product_uom_qty, 0) + self.assertEqual(move_line_user_2.product_uom_qty, 9) def test_move_states(self): # as only assigned moves can be posted, we need to ensure that