From 8be4b82f360dff0b93b37ac896397370c7ddb488 Mon Sep 17 00:00:00 2001 From: "Pedram (pebr)" Date: Mon, 30 Sep 2024 10:45:31 +0200 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#182058 Signed-off-by: Joseph Caburnay (jcb) --- addons/point_of_sale/models/pos_order.py | 41 ++---- .../tests/test_point_of_sale_flow.py | 133 ------------------ 2 files changed, 14 insertions(+), 160 deletions(-) diff --git a/addons/point_of_sale/models/pos_order.py b/addons/point_of_sale/models/pos_order.py index 38f4aed918cda..f9f4118ee6ba1 100644 --- a/addons/point_of_sale/models/pos_order.py +++ b/addons/point_of_sale/models/pos_order.py @@ -32,42 +32,29 @@ def _amount_line_tax(self, line, fiscal_position_id): taxes = taxes.compute_all(price, line.order_id.currency_id, line.qty, product=line.product_id, partner=line.order_id.partner_id or False)['taxes'] return sum(tax.get('amount', 0.0) for tax in taxes) - # 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['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.depends('sequence_number', 'session_id') def _compute_tracking_number(self): @@ -966,10 +953,10 @@ def sync_from_ui(self, orders): order._ensure_access_token() # If the previous session is closed, the order will get a new session_id due to _get_valid_session in _process_order - is_rescue_session = any(order.get('session_id') not in session_ids for order in orders) + is_new_session = any(order.get('session_id') not in session_ids for order in orders) return { 'pos.order': pos_order_ids.read(pos_order_ids._load_pos_data_fields(config_id), load=False) if config_id else [], - 'pos.session': pos_order_ids.session_id._load_pos_data({})['data'] if config_id and is_rescue_session else [], + 'pos.session': pos_order_ids.session_id._load_pos_data({})['data'] if config_id and is_new_session else [], 'pos.payment': pos_order_ids.payment_ids.read(pos_order_ids.payment_ids._load_pos_data_fields(config_id), load=False) if config_id else [], 'pos.order.line': pos_order_ids.lines.read(pos_order_ids.lines._load_pos_data_fields(config_id), load=False) if config_id else [], 'pos.pack.operation.lot': pos_order_ids.lines.pack_lot_ids.read(pos_order_ids.lines.pack_lot_ids._load_pos_data_fields(config_id), load=False) if config_id else [], 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 7e22787ef7a50..476e174c503af 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 @@ -824,139 +824,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_sync_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 = { - 'amount_paid': untax + atax, - 'amount_return': 0, - 'amount_tax': atax, - 'amount_total': untax + atax, - 'date_order': fields.Datetime.to_string(fields.Datetime.now()), - 'fiscal_position_id': False, - '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.filtered(lambda t: t.company_id.id == self.env.company.id).ids)]}]], - 'name': 'Order 00042-003-0014', - 'partner_id': False, - 'session_id': current_session.id, - 'sequence_number': 2, - 'payment_ids': [[0, 0, - {'amount': untax + atax, - 'name': fields.Datetime.now(), - 'payment_method_id': self.cash_payment_method.id}]], - 'uuid': '00042-003-0014', - 'last_order_preparation_change': '{}', - 'user_id': self.env.uid - } - - untax, atax = self.compute_tax(self.whiteboard_pen, 1.2) - zucchini_order = { - 'amount_paid': untax + atax, - 'amount_return': 0, - 'amount_tax': atax, - 'amount_total': untax + atax, - 'date_order': fields.Datetime.to_string(fields.Datetime.now()), - 'fiscal_position_id': False, - '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.filtered(lambda t: t.company_id.id == self.env.company.id).ids)]}]], - 'name': 'Order 00043-003-0014', - 'partner_id': self.partner1.id, - 'session_id': current_session.id, - 'sequence_number': self.pos_config.journal_id.id, - 'payment_ids': [[0, 0, - {'amount': untax + atax, - 'name': fields.Datetime.now(), - 'payment_method_id': self.credit_payment_method.id}]], - 'uuid': '00043-003-0014', - 'last_order_preparation_change': '{}', - 'user_id': self.env.uid - } - - untax, atax = self.compute_tax(self.newspaper_rack, 1.28) - newspaper_rack_order = { - 'amount_paid': untax + atax, - 'amount_return': 0, - 'amount_tax': atax, - 'amount_total': untax + atax, - 'date_order': fields.Datetime.to_string(fields.Datetime.now()), - 'fiscal_position_id': False, - '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.filtered(lambda t: t.company_id.id == self.env.company.id).ids]]}]], - 'name': 'Order 00044-003-0014', - 'partner_id': False, - 'session_id': current_session.id, - 'sequence_number': self.pos_config.journal_id.id, - 'payment_ids': [[0, 0, - {'amount': untax + atax, - 'name': fields.Datetime.now(), - 'payment_method_id': self.bank_payment_method.id}]], - 'uuid': '00044-003-0014', - 'last_order_preparation_change': '{}', - 'user_id': self.env.uid - } - - # I create an order on an open session - self.PosOrder.sync_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.sync_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.