From 1d846c36d913fad6be55740b01d965a0adbc48f5 Mon Sep 17 00:00:00 2001 From: "Anita (anko)" Date: Thu, 24 Nov 2022 13:50:45 +0000 Subject: [PATCH] [IMP] payment(_*): allow specifying extra allowed states Some providers require additional allowed states due to their refund or transaction process justifying it. Until now, these extra states were specified in the `payment` module, which was not ideal as it allowed every provider in every flow to accept these additional states. With this commit, additional states are now specified only in the coresponding flow of a provider that requires them. task-2869678 closes odoo/odoo#107110 Signed-off-by: Antoine Vandevenne (anv) --- .../models/payment_transaction.py | 4 +- addons/payment/models/payment_transaction.py | 46 +++++++++++++------ .../payment/tests/test_payment_transaction.py | 14 ++++++ .../models/payment_transaction.py | 2 +- .../models/payment_transaction.py | 2 +- .../models/payment_transaction.py | 2 +- addons/sale/models/payment_transaction.py | 8 ++-- 7 files changed, 56 insertions(+), 22 deletions(-) diff --git a/addons/account_payment/models/payment_transaction.py b/addons/account_payment/models/payment_transaction.py index b4b5e082d80..d3e68f326db 100644 --- a/addons/account_payment/models/payment_transaction.py +++ b/addons/account_payment/models/payment_transaction.py @@ -89,14 +89,14 @@ class PaymentTransaction(models.Model): return separator.join(invoices.mapped('name')) return super()._compute_reference_prefix(provider_code, separator, **values) - def _set_canceled(self, state_message=None): + def _set_canceled(self, state_message=None, **kwargs): """ Update the transactions' state to 'cancel'. :param str state_message: The reason for which the transaction is set in 'cancel' state :return: updated transactions :rtype: `payment.transaction` recordset """ - processed_txs = super()._set_canceled(state_message) + processed_txs = super()._set_canceled(state_message, **kwargs) # Cancel the existing payments processed_txs.payment_id.action_cancel() return processed_txs diff --git a/addons/payment/models/payment_transaction.py b/addons/payment/models/payment_transaction.py index 300d1466001..866060f1bf9 100644 --- a/addons/payment/models/payment_transaction.py +++ b/addons/payment/models/payment_transaction.py @@ -631,69 +631,89 @@ class PaymentTransaction(models.Model): """ self.ensure_one() - def _set_pending(self, state_message=None): + def _set_pending(self, state_message=None, extra_allowed_states=()): """ Update the transactions' state to `pending`. :param str state_message: The reason for setting the transactions in the state `pending`. + :param tuple[str] extra_allowed_states: The extra states that should be considered allowed + target states for the source state 'pending'. :return: The updated transactions. :rtype: recordset of `payment.transaction` """ allowed_states = ('draft',) target_state = 'pending' - txs_to_process = self._update_state(allowed_states, target_state, state_message) + txs_to_process = self._update_state( + allowed_states + extra_allowed_states, target_state, state_message + ) txs_to_process._log_received_message() return txs_to_process - def _set_authorized(self, state_message=None): + def _set_authorized(self, state_message=None, extra_allowed_states=()): """ Update the transactions' state to `authorized`. :param str state_message: The reason for setting the transactions in the state `authorized`. + :param tuple[str] extra_allowed_states: The extra states that should be considered allowed + target states for the source state 'authorized'. :return: The updated transactions. :rtype: recordset of `payment.transaction` """ allowed_states = ('draft', 'pending') target_state = 'authorized' - txs_to_process = self._update_state(allowed_states, target_state, state_message) + txs_to_process = self._update_state( + allowed_states + extra_allowed_states, target_state, state_message + ) txs_to_process._log_received_message() return txs_to_process - def _set_done(self, state_message=None): + def _set_done(self, state_message=None, extra_allowed_states=()): """ Update the transactions' state to `done`. :param str state_message: The reason for setting the transactions in the state `done`. + :param tuple[str] extra_allowed_states: The extra states that should be considered allowed + target states for the source state 'done'. :return: The updated transactions. :rtype: recordset of `payment.transaction` """ - allowed_states = ('draft', 'pending', 'authorized', 'error', 'cancel') # 'cancel' for Payulatam + allowed_states = ('draft', 'pending', 'authorized', 'error') target_state = 'done' - txs_to_process = self._update_state(allowed_states, target_state, state_message) + txs_to_process = self._update_state( + allowed_states + extra_allowed_states, target_state, state_message + ) txs_to_process._log_received_message() return txs_to_process - def _set_canceled(self, state_message=None): + def _set_canceled(self, state_message=None, extra_allowed_states=()): """ Update the transactions' state to `cancel`. :param str state_message: The reason for setting the transactions in the state `cancel`. + :param tuple[str] extra_allowed_states: The extra states that should be considered allowed + target states for the source state 'canceled'. :return: The updated transactions. :rtype: recordset of `payment.transaction` """ - allowed_states = ('draft', 'pending', 'authorized', 'done') # 'done' for Authorize refunds. + allowed_states = ('draft', 'pending', 'authorized') target_state = 'cancel' - txs_to_process = self._update_state(allowed_states, target_state, state_message) + txs_to_process = self._update_state( + allowed_states + extra_allowed_states, target_state, state_message + ) # Cancel the existing payments. txs_to_process._log_received_message() return txs_to_process - def _set_error(self, state_message): + def _set_error(self, state_message, extra_allowed_states=()): """ Update the transactions' state to `error`. :param str state_message: The reason for setting the transactions in the state `error`. + :param tuple[str] extra_allowed_states: The extra states that should be considered allowed + target states for the source state 'error'. :return: The updated transactions. :rtype: recordset of `payment.transaction` """ - allowed_states = ('draft', 'pending', 'authorized', 'done') # 'done' for Stripe refunds. + allowed_states = ('draft', 'pending', 'authorized') target_state = 'error' - txs_to_process = self._update_state(allowed_states, target_state, state_message) + txs_to_process = self._update_state( + allowed_states + extra_allowed_states, target_state, state_message + ) txs_to_process._log_received_message() return txs_to_process diff --git a/addons/payment/tests/test_payment_transaction.py b/addons/payment/tests/test_payment_transaction.py index 1f1976570af..1e21d317c03 100644 --- a/addons/payment/tests/test_payment_transaction.py +++ b/addons/payment/tests/test_payment_transaction.py @@ -1,6 +1,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from odoo.tests import tagged +from odoo.tools import mute_logger from odoo.addons.payment.tests.common import PaymentCommon @@ -80,3 +81,16 @@ class TestPaymentTransaction(PaymentCommon): msg="The amount of the refund transaction should be the negative value of the amount " "to refund." ) + + @mute_logger('odoo.addons.payment.models.payment_transaction') + def test_update_state_to_illegal_target_state(self): + tx = self._create_transaction('redirect', state='done') + tx._update_state(['draft', 'pending', 'authorized'], 'cancel', None) + self.assertEqual(tx.state, 'done') + + def test_update_state_to_extra_allowed_state(self): + tx = self._create_transaction('redirect', state='done') + tx._update_state( + ['draft', 'pending', 'authorized', 'done'], 'cancel', None + ) + self.assertEqual(tx.state, 'cancel') diff --git a/addons/payment_authorize/models/payment_transaction.py b/addons/payment_authorize/models/payment_transaction.py index b729adfa7b1..759565191c6 100644 --- a/addons/payment_authorize/models/payment_transaction.py +++ b/addons/payment_authorize/models/payment_transaction.py @@ -108,7 +108,7 @@ class PaymentTransaction(models.Model): tx_status = tx_details.get('transaction', {}).get('transactionStatus') if tx_status in TRANSACTION_STATUS_MAPPING['voided']: # The payment has been voided from Authorize.net side before we could refund it. - self._set_canceled() + self._set_canceled(extra_allowed_states=('done',)) elif tx_status in TRANSACTION_STATUS_MAPPING['refunded']: # The payment has been refunded from Authorize.net side before we could refund it. We # create a refund tx on Odoo to reflect the move of the funds. diff --git a/addons/payment_payulatam/models/payment_transaction.py b/addons/payment_payulatam/models/payment_transaction.py index e6ca8e34346..6f7cb39b1b7 100644 --- a/addons/payment_payulatam/models/payment_transaction.py +++ b/addons/payment_payulatam/models/payment_transaction.py @@ -129,7 +129,7 @@ class PaymentTransaction(models.Model): if status == 'PENDING': self._set_pending(state_message=state_message) elif status == 'APPROVED': - self._set_done(state_message=state_message) + self._set_done(state_message=state_message, extra_allowed_states=('cancel',)) elif status in ('EXPIRED', 'DECLINED'): self._set_canceled(state_message=state_message) else: diff --git a/addons/payment_stripe/models/payment_transaction.py b/addons/payment_stripe/models/payment_transaction.py index 4fc26b1c152..f79bca53c83 100644 --- a/addons/payment_stripe/models/payment_transaction.py +++ b/addons/payment_stripe/models/payment_transaction.py @@ -452,7 +452,7 @@ class PaymentTransaction(models.Model): self._set_error(_( "The refund did not go through. Please log into your Stripe Dashboard to get " "more information on that matter, and address any accounting discrepancies." - )) + ), extra_allowed_states=('done',)) else: # Classify unknown intent statuses as `error` tx state _logger.warning( "received invalid payment status (%s) for transaction with reference %s", diff --git a/addons/sale/models/payment_transaction.py b/addons/sale/models/payment_transaction.py index 8ff6a297d15..6728db36be6 100644 --- a/addons/sale/models/payment_transaction.py +++ b/addons/sale/models/payment_transaction.py @@ -31,14 +31,14 @@ class PaymentTransaction(models.Model): for trans in self: trans.sale_order_ids_nbr = len(trans.sale_order_ids) - def _set_pending(self, state_message=None): + def _set_pending(self, state_message=None, **kwargs): """ Override of `payment` to send the quotations automatically. :param str state_message: The reason for which the transaction is set in 'pending' state. :return: updated transactions. :rtype: `payment.transaction` recordset. """ - txs_to_process = super()._set_pending(state_message=state_message) + txs_to_process = super()._set_pending(state_message=state_message, **kwargs) for tx in txs_to_process: # Consider only transactions that are indeed set pending. sales_orders = tx.sale_order_ids.filtered(lambda so: so.state in ['draft', 'sent']) @@ -92,9 +92,9 @@ class PaymentTransaction(models.Model): ) return confirmed_orders - def _set_authorized(self, state_message=None): + def _set_authorized(self, state_message=None, **kwargs): """ Override of payment to confirm the quotations automatically. """ - super()._set_authorized(state_message=state_message) + super()._set_authorized(state_message=state_message, **kwargs) confirmed_orders = self._check_amount_and_confirm_order() confirmed_orders._send_order_confirmation_mail()