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) 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 5e322b5037..0687aa532d 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 @@ -248,27 +245,37 @@ 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] - ) - ) - ) - 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) - values = move._prepare_move_line_vals(quantity=qty_todo_remaining) + """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) return self._scan_line__assign_user(picking, line, qty_done) 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): @@ -333,7 +340,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( @@ -687,9 +693,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 @@ -751,7 +754,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 @@ -763,26 +766,30 @@ def _align_product_uom_qties(self, move): # 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"): # 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 - 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 + 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 + ) + # 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): # Used by inherting module see shopfloor_reception_packaging_dimension @@ -791,8 +798,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 +807,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( @@ -845,7 +852,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, @@ -1153,6 +1165,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")) @@ -1222,6 +1250,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 @@ -1243,15 +1272,48 @@ 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 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 with the remaining + # quantity to do, so selected_line is extracted in a new move as expected. + + # 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 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() + 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_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 + 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 + 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() def set_destination( self, picking_id, selected_line_id, location_name, confirmation=False @@ -1361,7 +1423,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 ) @@ -1417,6 +1478,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"}, @@ -1548,6 +1619,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"} @@ -1628,6 +1702,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 { @@ -1705,6 +1789,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/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_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..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: @@ -209,9 +197,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 +232,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_destination.py b/shopfloor_reception/tests/test_set_destination.py index 7a9ec07e35..802f6d4b02 100644 --- a/shopfloor_reception/tests/test_set_destination.py +++ b/shopfloor_reception/tests/test_set_destination.py @@ -145,11 +145,57 @@ 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) 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 23614c1ab8..2c50385543 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 @@ -547,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={ @@ -556,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, ) @@ -582,10 +591,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 +646,195 @@ 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() + 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 = 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) + + # 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 to be done + self.assertEqual(move_line_user_2.state, "assigned") + # As well as the new one + self.assertEqual(len(lines_after), 1) + # 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, 9) + + 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}, + ) 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(