From 8e71464ace536bd00e12fc4cf768b5fc282e5d56 Mon Sep 17 00:00:00 2001 From: "Pedram (pebr)" Date: Mon, 30 Sep 2024 08:45:31 +0000 Subject: [PATCH] [FIX] point_of_sale: Remove rescue session creation This commit addresses the issue where orders were created on a device with an open PoS session, even though the session had been closed on another device. Previously, the system would create a "rescue session" for these orders, which led to incorrect cash statements. With this fix, instead of creating a rescue session, the system now attempts to find any available open session. If no open session is found, the system prevents the order sync and raises an error, prompting users to open a new session. The decision to remove rescue sessions was driven by the fact that they caused inaccuracies in cash statements. Orders captured in a rescue session, especially those involving cash payments, would disrupt the cash control process. Furthermore, rescue sessions would initialize with a zero opening balance, which affected the accuracy of closing balances and cash flow. opw-4219284 closes odoo/odoo#187717 X-original-commit: 8be4b82f360dff0b93b37ac896397370c7ddb488 Signed-off-by: Joseph Caburnay (jcb) Signed-off-by: Pedram Bi Ria (pebr) --- addons/point_of_sale/models/pos_order.py | 41 ++---- .../tests/test_point_of_sale_flow.py | 139 ------------------ 2 files changed, 15 insertions(+), 165 deletions(-) diff --git a/addons/point_of_sale/models/pos_order.py b/addons/point_of_sale/models/pos_order.py index 8e4be0816e85a..31ba75604d649 100644 --- a/addons/point_of_sale/models/pos_order.py +++ b/addons/point_of_sale/models/pos_order.py @@ -73,42 +73,30 @@ def _payment_fields(self, order, ui_paymentline): 'pos_order_id': order.id, } - # This deals with orders that belong to a closed session. In order - # to recover from this situation we create a new rescue session, - # making it obvious that something went wrong. - # A new, separate, rescue session is preferred for every such recovery, - # to avoid adding unrelated orders to live sessions. + + # This function deals with orders that belong to a closed session. It attempts to find + # any open session that can be used to capture the order. If no open session is found, + # an error is raised, asking the user to open a session. def _get_valid_session(self, order): PosSession = self.env['pos.session'] closed_session = PosSession.browse(order['pos_session_id']) - _logger.warning('session %s (ID: %s) was closed but received order %s (total: %s) belonging to it', + _logger.warning('Session %s (ID: %s) was closed but received order %s (total: %s) belonging to it', closed_session.name, closed_session.id, order['name'], order['amount_total']) - rescue_session = PosSession.search([ + + open_session = PosSession.search([ ('state', 'not in', ('closed', 'closing_control')), - ('rescue', '=', True), - ('config_id', '=', closed_session.config_id.id), + ('config_id', '=', closed_session.config_id.id) ], limit=1) - if rescue_session: - _logger.warning('reusing recovery session %s for saving order %s', rescue_session.name, order['name']) - return rescue_session - - _logger.warning('attempting to create recovery session for saving order %s', order['name']) - new_session = PosSession.create({ - 'config_id': closed_session.config_id.id, - 'name': _('(RESCUE FOR %(session)s)') % {'session': closed_session.name}, - 'rescue': True, # avoid conflict with live sessions - }) - # bypass opening_control (necessary when using cash control) - new_session.action_pos_session_open() - if new_session.config_id.cash_control and new_session.rescue: - last_session = self.env['pos.session'].search([('config_id', '=', new_session.config_id.id), ('id', '!=', new_session.id)], limit=1) - new_session.cash_register_balance_start = last_session.cash_register_balance_end_real - return new_session + if open_session: + _logger.warning('Using open session %s for saving order %s', open_session.name, order['name']) + return open_session + + raise UserError(_('No open session available. Please open a new session to capture the order.')) @api.model def _process_order(self, order, draft, existing_order): @@ -915,6 +903,7 @@ def create_from_ui(self, orders, draft=False): sync_token = randrange(100000000) # Use to differentiate 2 parallels calls to this function in the logs _logger.info("Start PoS synchronisation #%d for PoS orders references: %s (draft: %s)", sync_token, order_names, draft) order_ids = [] + session_ids = set({order.get('session_id') for order in orders}) for order in orders: order_name = order['data']['name'] existing_draft_order = None @@ -929,7 +918,7 @@ def create_from_ui(self, orders, draft=False): if not existing_draft_order: existing_draft_order = self.env['pos.order'].search(['&', ('pos_reference', '=', order_name), ('state', '=', 'draft')], limit=1) - + try: if existing_draft_order: order_ids.append(self._process_order(order, draft, existing_draft_order)) diff --git a/addons/point_of_sale/tests/test_point_of_sale_flow.py b/addons/point_of_sale/tests/test_point_of_sale_flow.py index 5e53b9f23ea50..d700df043496d 100644 --- a/addons/point_of_sale/tests/test_point_of_sale_flow.py +++ b/addons/point_of_sale/tests/test_point_of_sale_flow.py @@ -817,145 +817,6 @@ def test_order_to_invoice(self): # I close the session to generate the journal entries current_session.action_pos_session_closing_control() - def test_create_from_ui(self): - """ - Simulation of sales coming from the interface, even after closing the session - """ - - # I click on create a new session button - self.pos_config.open_ui() - - current_session = self.pos_config.current_session_id - num_starting_orders = len(current_session.order_ids) - - current_session.set_cashbox_pos(0, None) - - untax, atax = self.compute_tax(self.led_lamp, 0.9) - carrot_order = {'data': - {'amount_paid': untax + atax, - 'amount_return': 0, - 'amount_tax': atax, - 'amount_total': untax + atax, - 'creation_date': fields.Datetime.to_string(fields.Datetime.now()), - 'fiscal_position_id': False, - 'pricelist_id': self.pos_config.available_pricelist_ids[0].id, - 'lines': [[0, - 0, - {'discount': 0, - 'pack_lot_ids': [], - 'price_unit': 0.9, - 'product_id': self.led_lamp.id, - 'price_subtotal': 0.9, - 'price_subtotal_incl': 1.04, - 'qty': 1, - 'tax_ids': [(6, 0, self.led_lamp.taxes_id.ids)]}]], - 'name': 'Order 00042-003-0014', - 'partner_id': False, - 'pos_session_id': current_session.id, - 'sequence_number': 2, - 'statement_ids': [[0, - 0, - {'amount': untax + atax, - 'name': fields.Datetime.now(), - 'payment_method_id': self.cash_payment_method.id}]], - 'uid': '00042-003-0014', - 'user_id': self.env.uid}, - 'to_invoice': False} - - untax, atax = self.compute_tax(self.whiteboard_pen, 1.2) - zucchini_order = {'data': - {'amount_paid': untax + atax, - 'amount_return': 0, - 'amount_tax': atax, - 'amount_total': untax + atax, - 'creation_date': fields.Datetime.to_string(fields.Datetime.now()), - 'fiscal_position_id': False, - 'pricelist_id': self.pos_config.available_pricelist_ids[0].id, - 'lines': [[0, - 0, - {'discount': 0, - 'pack_lot_ids': [], - 'price_unit': 1.2, - 'product_id': self.whiteboard_pen.id, - 'price_subtotal': 1.2, - 'price_subtotal_incl': 1.38, - 'qty': 1, - 'tax_ids': [(6, 0, self.whiteboard_pen.taxes_id.ids)]}]], - 'name': 'Order 00043-003-0014', - 'partner_id': self.partner1.id, - 'pos_session_id': current_session.id, - 'sequence_number': self.pos_config.journal_id.id, - 'statement_ids': [[0, - 0, - {'amount': untax + atax, - 'name': fields.Datetime.now(), - 'payment_method_id': self.credit_payment_method.id}]], - 'uid': '00043-003-0014', - 'user_id': self.env.uid}, - 'to_invoice': False} - - untax, atax = self.compute_tax(self.newspaper_rack, 1.28) - newspaper_rack_order = {'data': - {'amount_paid': untax + atax, - 'amount_return': 0, - 'amount_tax': atax, - 'amount_total': untax + atax, - 'creation_date': fields.Datetime.to_string(fields.Datetime.now()), - 'fiscal_position_id': False, - 'pricelist_id': self.pos_config.available_pricelist_ids[0].id, - 'lines': [[0, - 0, - {'discount': 0, - 'pack_lot_ids': [], - 'price_unit': 1.28, - 'product_id': self.newspaper_rack.id, - 'price_subtotal': 1.28, - 'price_subtotal_incl': 1.47, - 'qty': 1, - 'tax_ids': [[6, False, self.newspaper_rack.taxes_id.ids]]}]], - 'name': 'Order 00044-003-0014', - 'partner_id': False, - 'pos_session_id': current_session.id, - 'sequence_number': self.pos_config.journal_id.id, - 'statement_ids': [[0, - 0, - {'amount': untax + atax, - 'name': fields.Datetime.now(), - 'payment_method_id': self.bank_payment_method.id}]], - 'uid': '00044-003-0014', - 'user_id': self.env.uid}, - 'to_invoice': False} - - # I create an order on an open session - self.PosOrder.create_from_ui([carrot_order]) - self.assertEqual(num_starting_orders + 1, len(current_session.order_ids), "Submitted order not encoded") - - # I close the session - total_cash_payment = sum(current_session.mapped('order_ids.payment_ids').filtered(lambda payment: payment.payment_method_id.type == 'cash').mapped('amount')) - current_session.post_closing_cash_details(total_cash_payment) - current_session.close_session_from_ui() - self.assertEqual(current_session.state, 'closed', "Session was not properly closed") - self.assertFalse(self.pos_config.current_session_id, "Current session not properly recomputed") - - # I keep selling after the session is closed - with mute_logger('odoo.addons.point_of_sale.models.pos_order'): - self.PosOrder.create_from_ui([zucchini_order, newspaper_rack_order]) - rescue_session = self.PosSession.search([ - ('config_id', '=', self.pos_config.id), - ('state', '=', 'opened'), - ('rescue', '=', True) - ]) - self.assertEqual(len(rescue_session), 1, "One (and only one) rescue session should be created for orphan orders") - self.assertIn("(RESCUE FOR %s)" % current_session.name, rescue_session.name, "Rescue session is not linked to the previous one") - self.assertEqual(len(rescue_session.order_ids), 2, "Rescue session does not contain both orders") - - # I close the rescue session - total_cash_payment = sum(rescue_session.mapped('order_ids.payment_ids').filtered(lambda payment: payment.payment_method_id.type == 'cash').mapped('amount')) - rescue_session.post_closing_cash_details(total_cash_payment) - rescue_session.close_session_from_ui() - self.assertEqual(rescue_session.state, 'closed', "Rescue session was not properly closed") - self.assertEqual(rescue_session.cash_register_balance_start, current_session.cash_register_balance_end_real, "Rescue session does not start with the same amount as the previous session") - def test_order_to_payment_currency(self): """ In order to test the Point of Sale in module, I will do a full flow from the sale to the payment and invoicing.