From 51a08bbfa35b9101bc715de326bcd1626d26fde7 Mon Sep 17 00:00:00 2001 From: Joseph Caburnay Date: Fri, 20 Nov 2020 15:56:14 +0000 Subject: [PATCH] [IMP] point_of_sale: show wizard when closing pos.session failed When closing pos.session and it turned out that during the creation of journal entry the move lines are not balanced, pos automatically balances the lines by crediting the amount to balance and recording it to the default pos receivable account. In this commit, we are changing the behavior by allowing the user to choose which account the imbalance amount be recorded. So the workflow becomes: 1. Close pos session fails. 2. Wizard shows the amount to balance and asks which account to record it. 3. Confirm the wizard to successfully close the session. closes odoo/odoo#62425 Signed-off-by: pimodoo --- addons/account/models/account_move.py | 11 ++- addons/point_of_sale/__manifest__.py | 1 + addons/point_of_sale/models/pos_session.py | 98 ++++++++++++------- .../security/ir.model.access.csv | 1 + .../tests/test_point_of_sale_flow.py | 8 +- addons/point_of_sale/wizard/__init__.py | 1 + .../wizard/pos_close_session_wizard.py | 20 ++++ .../wizard/pos_close_session_wizard.xml | 21 ++++ 8 files changed, 126 insertions(+), 35 deletions(-) create mode 100644 addons/point_of_sale/wizard/pos_close_session_wizard.py create mode 100644 addons/point_of_sale/wizard/pos_close_session_wizard.xml diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py index 8b49cf154ad..4b89de2566c 100644 --- a/addons/account/models/account_move.py +++ b/addons/account/models/account_move.py @@ -16,6 +16,15 @@ import json import re import warnings +class ImbalanceJournalEntryError(UserError): + """Specialized UserError raised by `_check_balanced` method.""" + + def __init__(self, message, move_ids, imbalance_amounts): + super().__init__(message) + self.move_ids = move_ids + self.imbalance_amounts = imbalance_amounts + + #forbidden fields INTEGRITY_HASH_MOVE_FIELDS = ('date', 'journal_id', 'company_id') INTEGRITY_HASH_LINE_FIELDS = ('debit', 'credit', 'account_id', 'partner_id') @@ -1671,7 +1680,7 @@ class AccountMove(models.Model): if query_res: ids = [res[0] for res in query_res] sums = [res[1] for res in query_res] - raise UserError(_("Cannot create unbalanced journal entry. Ids: %s\nDifferences debit - credit: %s") % (ids, sums)) + raise ImbalanceJournalEntryError(_("Cannot create unbalanced journal entry. Ids: %s\nDifferences debit - credit: %s") % (ids, sums), ids, sums) def _check_fiscalyear_lock_date(self): for move in self: diff --git a/addons/point_of_sale/__manifest__.py b/addons/point_of_sale/__manifest__.py index 938ebae549b..a317df78906 100644 --- a/addons/point_of_sale/__manifest__.py +++ b/addons/point_of_sale/__manifest__.py @@ -17,6 +17,7 @@ 'wizard/pos_box.xml', 'wizard/pos_details.xml', 'wizard/pos_payment.xml', + 'wizard/pos_close_session_wizard.xml', 'views/pos_assets_common.xml', 'views/pos_assets_index.xml', 'views/pos_assets_qunit.xml', diff --git a/addons/point_of_sale/models/pos_session.py b/addons/point_of_sale/models/pos_session.py index 6b902eae18b..b8a3f5e8895 100644 --- a/addons/point_of_sale/models/pos_session.py +++ b/addons/point_of_sale/models/pos_session.py @@ -8,6 +8,7 @@ from odoo import api, fields, models, _ from odoo.exceptions import UserError, ValidationError from odoo.tools import float_is_zero, float_compare +from odoo.addons.account.models.account_move import ImbalanceJournalEntryError class PosSession(models.Model): _name = 'pos.session' @@ -256,14 +257,14 @@ class PosSession(models.Model): session.write(values) return True - def action_pos_session_closing_control(self): + def action_pos_session_closing_control(self, balancing_account=False, amount_to_balance=0): self._check_pos_session_balance() for session in self: if session.state == 'closed': raise UserError(_('This session is already closed.')) session.write({'state': 'closing_control', 'stop_at': fields.Datetime.now()}) if not session.config_id.cash_control: - session.action_pos_session_close() + return session.action_pos_session_close(balancing_account, amount_to_balance) def _check_pos_session_balance(self): for session in self: @@ -275,12 +276,12 @@ class PosSession(models.Model): self._check_pos_session_balance() return self.action_pos_session_close() - def action_pos_session_close(self): + def action_pos_session_close(self, balancing_account=False, amount_to_balance=0): # Session without cash payment method will not have a cash register. # However, there could be other payment methods, thus, session still # needs to be validated. if not self.cash_register_id: - return self._validate_session() + return self._validate_session(balancing_account, amount_to_balance) if self.cash_control and abs(self.cash_register_difference) > self.config_id.amount_authorized_diff: # Only pos manager can close statements with cash_register_difference greater than amount_authorized_diff. @@ -292,9 +293,9 @@ class PosSession(models.Model): else: return self._warning_balance_closing() else: - return self._validate_session() + return self._validate_session(balancing_account, amount_to_balance) - def _validate_session(self): + def _validate_session(self, balancing_account=False, amount_to_balance=0): self.ensure_one() if self.order_ids: self.cash_real_transaction = self.cash_register_total_entry_encoding @@ -308,19 +309,37 @@ class PosSession(models.Model): # Users without any accounting rights won't be able to create the journal entry. If this # case, switch to sudo for creation and posting. sudo = False - if ( - not self.env['account.move'].check_access_rights('create', raise_exception=False) - and self.user_has_groups('point_of_sale.group_pos_user') - ): - sudo = True - self.sudo().with_company(self.company_id)._create_account_move() - else: - self.with_company(self.company_id)._create_account_move() - if self.move_id.line_ids: - # Set the uninvoiced orders' state to 'done' - self.env['pos.order'].search([('session_id', '=', self.id), ('state', '=', 'paid')]).write({'state': 'done'}) - else: - self.move_id.unlink() + try: + if ( + not self.env['account.move'].check_access_rights('create', raise_exception=False) + and self.user_has_groups('point_of_sale.group_pos_user') + ): + sudo = True + self.sudo().with_company(self.company_id)._create_account_move(balancing_account, amount_to_balance) + else: + self.with_company(self.company_id)._create_account_move(balancing_account, amount_to_balance) + if self.move_id.line_ids: + # Set the uninvoiced orders' state to 'done' + self.env['pos.order'].search([('session_id', '=', self.id), ('state', '=', 'paid')]).write({'state': 'done'}) + else: + self.move_id.unlink() + + except ImbalanceJournalEntryError as error: + # Creating the account move is just part of a big database transaction + # when closing a session. There are other database changes that will happen + # before attempting to create the account move, such as, creating the picking + # records. + # We don't, however, want them to be committed when the account move creation + # failed; therefore, we need to roll back this transaction before showing the + # close session wizard. + self.env.cr.rollback() + # When closing a session, single account move is checked which resulted to + # `ImbalanceJournalEntryError`. Thus, there should only be one value in imbalance_amounts + # which is what we need. + if len(error.imbalance_amounts) != 1: + raise UserError(_("Single imbalance amount that matches the pos session's account move is expected.")) + return self._close_session_action(error.imbalance_amounts[0]) + self.write({'state': 'closed'}) return { 'type': 'ir.actions.client', @@ -329,6 +348,25 @@ class PosSession(models.Model): 'params': {'menu_id': self.env.ref('point_of_sale.menu_point_root').id}, } + def _close_session_action(self, amount_to_balance): + default_account = self._get_balancing_account() + wizard = self.env['pos.close.session.wizard'].create({ + 'amount_to_balance': amount_to_balance, + 'account_id': default_account.id, + 'account_readonly': not self.env.user.has_group('account.group_account_readonly'), + 'message': _("There is a difference between the amounts to post and the amounts of the orders, it is probably caused by taxes or accounting configurations changes.") + }) + return { + 'name': _("Force Close Session"), + 'type': 'ir.actions.act_window', + 'view_type': 'form', + 'view_mode': 'form', + 'res_model': 'pos.close.session.wizard', + 'res_id': wizard.id, + 'target': 'new', + 'context': {**self.env.context, 'active_ids': self.ids, 'active_model': 'pos.session'}, + } + def _create_picking_at_end_of_session(self): self.ensure_one() lines_grouped_by_dest_location = {} @@ -352,24 +390,17 @@ class PosSession(models.Model): pickings = self.env['stock.picking']._create_picking_from_pos_order_lines(location_dest_id, lines, picking_type) pickings.write({'pos_session_id': self.id, 'origin': self.name}) - def _create_balancing_line(self, data): - imbalance_amount = 0 - for line in self.move_id.line_ids: - # it is an excess debit so it should be credited - imbalance_amount += line.debit - line.credit - - if (not float_is_zero(imbalance_amount, precision_rounding=self.currency_id.rounding)): - balancing_vals = self._prepare_balancing_line_vals(imbalance_amount, self.move_id) + def _create_balancing_line(self, data, balancing_account, amount_to_balance): + if (not float_is_zero(amount_to_balance, precision_rounding=self.currency_id.rounding)): + balancing_vals = self._prepare_balancing_line_vals(amount_to_balance, self.move_id, balancing_account) MoveLine = data.get('MoveLine') MoveLine.create(balancing_vals) - return data - def _prepare_balancing_line_vals(self, imbalance_amount, move): - account = self._get_balancing_account() + def _prepare_balancing_line_vals(self, imbalance_amount, move, balancing_account): partial_vals = { 'name': _('Difference at closing PoS session'), - 'account_id': account.id, + 'account_id': balancing_account.id, 'move_id': move.id, 'partner_id': False, } @@ -384,7 +415,7 @@ class PosSession(models.Model): def _get_balancing_account(self): return self.company_id.account_default_pos_receivable_account_id or self.env['ir.property'].get('property_account_receivable_id', 'res.partner') - def _create_account_move(self): + def _create_account_move(self, balancing_account=False, amount_to_balance=0): """ Create account.move and account.move.line records for this session. Side-effects include: @@ -408,7 +439,8 @@ class PosSession(models.Model): data = self._create_cash_statement_lines_and_cash_move_lines(data) data = self._create_invoice_receivable_lines(data) data = self._create_stock_output_lines(data) - data = self._create_balancing_line(data) + if balancing_account and amount_to_balance: + data = self._create_balancing_line(data, balancing_account, amount_to_balance) if account_move.line_ids: account_move._post() diff --git a/addons/point_of_sale/security/ir.model.access.csv b/addons/point_of_sale/security/ir.model.access.csv index 35f4d181775..aed5f96ce96 100644 --- a/addons/point_of_sale/security/ir.model.access.csv +++ b/addons/point_of_sale/security/ir.model.access.csv @@ -50,3 +50,4 @@ access_closing_balance_confirm_wizard,access.closing.balance.confirm.wizard,mode access_pos_details_wizard,access.pos.details.wizard,model_pos_details_wizard,point_of_sale.group_pos_manager,1,1,1,0 access_pos_make_payment,access.pos.make.payment,model_pos_make_payment,point_of_sale.group_pos_manager,1,1,1,0 access_money_in_out_wizard,access.money.in.out.wizard,model_cash_box_out,point_of_sale.group_pos_user,1,1,1,0 +access_pos_close_session_wizard,access.pos.close.session.wizard,model_pos_close_session_wizard,point_of_sale.group_pos_user,1,1,1,0 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 04b9b952725..c99231c780a 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 @@ -913,7 +913,13 @@ class TestPointOfSaleFlow(TestPointOfSaleCommon): dummy_50_perc_tax.unlink() # close session (should not fail here) - pos_session.action_pos_session_closing_control() + # We don't call `action_pos_session_closing_control` to force the failed + # closing which will return the action because the internal rollback call messes + # with the rollback of the test runner. So instead, we directly call the method + # that returns the action by specifying the imbalance amount. + action = pos_session._close_session_action(5.0) + wizard = self.env['pos.close.session.wizard'].browse(action['res_id']) + wizard.with_context(action['context']).close_session() # check the difference line diff_line = pos_session.move_id.line_ids.filtered(lambda line: line.name == 'Difference at closing PoS session') diff --git a/addons/point_of_sale/wizard/__init__.py b/addons/point_of_sale/wizard/__init__.py index 16789e5d2ae..bd556f7e4cf 100644 --- a/addons/point_of_sale/wizard/__init__.py +++ b/addons/point_of_sale/wizard/__init__.py @@ -4,3 +4,4 @@ from . import pos_box from . import pos_details from . import pos_payment +from . import pos_close_session_wizard diff --git a/addons/point_of_sale/wizard/pos_close_session_wizard.py b/addons/point_of_sale/wizard/pos_close_session_wizard.py new file mode 100644 index 00000000000..d42ee72c761 --- /dev/null +++ b/addons/point_of_sale/wizard/pos_close_session_wizard.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo import fields, models + + +class PosCloseSessionWizard(models.TransientModel): + _name = "pos.close.session.wizard" + _description = "Close Session Wizard" + + amount_to_balance = fields.Float("Amount to balance") + account_id = fields.Many2one("account.account", "Destination account") + account_readonly = fields.Boolean("Destination account is readonly") + message = fields.Text("Information message") + + def close_session(self): + session = self.env["pos.session"].browse(self.env.context["active_ids"]) + return session.action_pos_session_closing_control( + self.account_id, self.amount_to_balance + ) diff --git a/addons/point_of_sale/wizard/pos_close_session_wizard.xml b/addons/point_of_sale/wizard/pos_close_session_wizard.xml new file mode 100644 index 00000000000..b424fe2a9bd --- /dev/null +++ b/addons/point_of_sale/wizard/pos_close_session_wizard.xml @@ -0,0 +1,21 @@ + + + + pos.close.session.wizard.form + pos.close.session.wizard + +
+

+ + + + + + +
+ + +