From e5c187ade8a4ac54f9dc220bb7d2a997e0143871 Mon Sep 17 00:00:00 2001 From: "Anita (anko)" Date: Fri, 21 Oct 2022 08:53:07 +0000 Subject: [PATCH] [IMP] payment(_paypal): UX and payment flow improvements UX was lacking comparing to other payment providers, important fields were not always shown or were checkboxes when they should be automatically true. In order to make payment flow easier and more intitive, unnecessary fields were removed, email is automatically filled. Now when user cancels transaction on paypal before paying, it automatically cancels transaction on Odoo. Additionaly, quick onboarding is only available if user already has paypal account and Stripe no longer installs ond configures paypal if Stripe's onboarding get canceled. task-2854184 closes odoo/odoo#104974 Related: odoo/upgrade#4025 Related: odoo/documentation#3063 Signed-off-by: Antoine Vandevenne (anv) --- addons/payment/models/res_company.py | 16 +-- .../wizards/payment_onboarding_views.xml | 15 +-- .../wizards/payment_onboarding_wizard.py | 17 +-- addons/payment_paypal/__manifest__.py | 1 - addons/payment_paypal/controllers/main.py | 101 ++++++++++-------- addons/payment_paypal/data/neutralize.sql | 1 - .../data/payment_paypal_email_data.xml | 25 ----- .../payment_paypal/models/payment_provider.py | 29 ++--- .../models/payment_transaction.py | 18 ++-- addons/payment_paypal/tests/test_paypal.py | 25 ++++- .../views/payment_paypal_templates.xml | 5 +- .../views/payment_provider_views.xml | 9 +- 12 files changed, 113 insertions(+), 149 deletions(-) delete mode 100644 addons/payment_paypal/data/payment_paypal_email_data.xml diff --git a/addons/payment/models/res_company.py b/addons/payment/models/res_company.py index 64176c4eef5..7ade2105f26 100644 --- a/addons/payment/models/res_company.py +++ b/addons/payment/models/res_company.py @@ -31,29 +31,17 @@ class ResCompany(models.Model): """ self.env.company.get_chart_of_accounts_or_fail() - self._install_modules(['payment_paypal', 'payment_stripe', 'account_payment']) + self._install_modules(['payment_stripe', 'account_payment']) # Create a new env including the freshly installed module(s) new_env = api.Environment(self.env.cr, self.env.uid, self.env.context) + # Configure Stripe default_journal = new_env['account.journal'].search( [('type', '=', 'bank'), ('company_id', '=', new_env.company.id)], limit=1 ) - - # Configure Stripe stripe_provider = new_env.ref('payment.payment_provider_stripe') stripe_provider.journal_id = stripe_provider.journal_id or default_journal - if stripe_provider.state == 'disabled': # The onboarding step has never been run - # Configure PayPal - paypal_provider = new_env.ref( - 'payment.payment_provider_paypal', raise_if_not_found=False - ) - if paypal_provider: - if not paypal_provider.paypal_email_account: - paypal_provider.paypal_email_account = new_env.user.email or new_env.company.email - if paypal_provider.state == 'disabled' and paypal_provider.paypal_email_account: - paypal_provider.state = 'enabled' - paypal_provider.journal_id = paypal_provider.journal_id or default_journal return stripe_provider.action_stripe_connect_account(menu_id=menu_id) diff --git a/addons/payment/wizards/payment_onboarding_views.xml b/addons/payment/wizards/payment_onboarding_views.xml index 0acf01f85a0..5a163f17375 100644 --- a/addons/payment/wizards/payment_onboarding_views.xml +++ b/addons/payment/wizards/payment_onboarding_views.xml @@ -14,19 +14,12 @@
- - - + -

- Start selling directly without an account; an email will be sent by Paypal to create your new account and collect your payments. -

-

- - How to configure your PayPal account - -

+ + How to configure your PayPal account +
diff --git a/addons/payment/wizards/payment_onboarding_wizard.py b/addons/payment/wizards/payment_onboarding_wizard.py index a81d134daa8..331634b9f3c 100644 --- a/addons/payment/wizards/payment_onboarding_wizard.py +++ b/addons/payment/wizards/payment_onboarding_wizard.py @@ -13,12 +13,7 @@ class PaymentWizard(models.TransientModel): ('paypal', "PayPal"), ('manual', "Custom payment instructions"), ], string="Payment Method", default=lambda self: self._get_default_payment_provider_onboarding_value('payment_method')) - - paypal_user_type = fields.Selection([ - ('new_user', "I don't have a Paypal account"), - ('existing_user', 'I have a Paypal account')], string="Paypal User Type", default='new_user') paypal_email_account = fields.Char("Email", default=lambda self: self._get_default_payment_provider_onboarding_value('paypal_email_account')) - paypal_seller_account = fields.Char("Merchant Account ID", default=lambda self: self._get_default_payment_provider_onboarding_value('paypal_seller_account')) paypal_pdt_token = fields.Char("PDT Identity Token", default=lambda self: self._get_default_payment_provider_onboarding_value('paypal_pdt_token')) # Account-specific logic. It's kept here rather than moved in `account_payment` as it's not used by `account` module. @@ -65,9 +60,10 @@ class PaymentWizard(models.TransientModel): if 'payment_paypal' in installed_modules: provider = self.env.ref('payment.payment_provider_paypal') - self._payment_provider_onboarding_cache['paypal_email_account'] = provider['paypal_email_account'] or self.env.user.email or '' - self._payment_provider_onboarding_cache['paypal_seller_account'] = provider['paypal_seller_account'] + self._payment_provider_onboarding_cache['paypal_email_account'] = provider['paypal_email_account'] or self.env.company.email self._payment_provider_onboarding_cache['paypal_pdt_token'] = provider['paypal_pdt_token'] + else: + self._payment_provider_onboarding_cache['paypal_email_account'] = self.env.company.email manual_payment = self._get_manual_payment_provider() journal = manual_payment.journal_id @@ -94,11 +90,16 @@ class PaymentWizard(models.TransientModel): new_env = api.Environment(self.env.cr, self.env.uid, self.env.context) if self.payment_method == 'paypal': + provider = new_env.ref('payment.payment_provider_paypal', raise_if_not_found=False) + default_journal = new_env['account.journal'].search( + [('type', '=', 'bank'), ('company_id', '=', new_env.company.id)], limit=1 + ) new_env.ref('payment.payment_provider_paypal').write({ 'paypal_email_account': self.paypal_email_account, - 'paypal_seller_account': self.paypal_seller_account, 'paypal_pdt_token': self.paypal_pdt_token, 'state': 'enabled', + 'is_published': 'True', + 'journal_id': provider.journal_id or default_journal }) elif self.payment_method == 'manual': manual_provider = self._get_manual_payment_provider(new_env) diff --git a/addons/payment_paypal/__manifest__.py b/addons/payment_paypal/__manifest__.py index 7565eec6128..7a310a375a8 100644 --- a/addons/payment_paypal/__manifest__.py +++ b/addons/payment_paypal/__manifest__.py @@ -13,7 +13,6 @@ 'views/payment_transaction_views.xml', 'data/payment_provider_data.xml', - 'data/payment_paypal_email_data.xml', ], 'post_init_hook': 'post_init_hook', 'uninstall_hook': 'uninstall_hook', diff --git a/addons/payment_paypal/controllers/main.py b/addons/payment_paypal/controllers/main.py index 9897028d783..47292158ebc 100644 --- a/addons/payment_paypal/controllers/main.py +++ b/addons/payment_paypal/controllers/main.py @@ -12,12 +12,15 @@ from odoo.exceptions import ValidationError from odoo.http import request from odoo.tools import html_escape +from odoo.addons.payment import utils as payment_utils + _logger = logging.getLogger(__name__) class PaypalController(http.Controller): _return_url = '/payment/paypal/return/' + _cancel_url = '/payment/paypal/cancel/' _webhook_url = '/payment/paypal/webhook/' @http.route( @@ -33,8 +36,7 @@ class PaypalController(http.Controller): The route accepts both GET and POST requests because PayPal seems to switch between the two depending on whether PDT is enabled, whether the customer pays anonymously (without logging - in on PayPal), whether the customer cancels the payment, whether they click on "Return to - Merchant" after paying, etc. + in on PayPal), whether they click on "Return to Merchant" after paying, etc. The route is flagged with `save_session=False` to prevent Odoo from assigning a new session to the user if they are redirected to this route with a POST request. Indeed, as the session @@ -43,22 +45,43 @@ class PaypalController(http.Controller): request from the payment provider to Odoo. As the redirection to the '/payment/status' page will satisfy any specification of the `SameSite` attribute, the session of the user will be retrieved and with it the transaction which will be immediately post-processed. + + :param dict pdt_data: The PDT notification data send by PayPal. """ - _logger.info("handling redirection from PayPal with data:\n%s", pprint.pformat(pdt_data)) - if not pdt_data: # The customer has canceled or paid then clicked on "Return to Merchant" - pass # Redirect them to the status page to browse the (currently) draft transaction + _logger.info("Handling redirection from PayPal with data:\n%s", pprint.pformat(pdt_data)) + + tx_sudo = request.env['payment.transaction'].sudo()._get_tx_from_notification_data( + 'paypal', pdt_data + ) + try: + notification_data = self._verify_pdt_notification_origin(pdt_data, tx_sudo) + except Forbidden: + _logger.exception("Could not verify the origin of the PDT; discarding it.") else: - # Check the origin of the notification - tx_sudo = request.env['payment.transaction'].sudo()._get_tx_from_notification_data( - 'paypal', pdt_data - ) - try: - notification_data = self._verify_pdt_notification_origin(pdt_data, tx_sudo) - except Forbidden: - _logger.exception("could not verify the origin of the PDT; discarding it") - else: - # Handle the notification data - tx_sudo._handle_notification_data('paypal', notification_data) + tx_sudo._handle_notification_data('paypal', notification_data) + + return request.redirect('/payment/status') + + @http.route( + _cancel_url, type='http', auth='public', methods=['GET'], csrf=False, save_session=False + ) + def paypal_return_from_canceled_checkout(self, tx_ref, access_token): + """ Process the transaction after the customer has canceled the payment. + + :param str tx_ref: The reference of the transaction having been canceled. + :param str access_token: The access token to verify the authenticity of the request. + """ + _logger.info( + "Handling redirection from Paypal for cancellation of transaction with reference %s", + tx_ref, + ) + + tx_sudo = request.env['payment.transaction'].sudo()._get_tx_from_notification_data( + 'paypal', {'item_number': tx_ref} + ) + if not payment_utils.check_access_token(access_token, tx_ref): + raise Forbidden() + tx_sudo._handle_notification_data('paypal', {}) return request.redirect('/payment/status') @@ -77,7 +100,7 @@ class PaypalController(http.Controller): See https://developer.paypal.com/docs/api-basics/notifications/payment-data-transfer/. - :param dict pdt_data: The PDT whose authenticity must be checked. + :param dict pdt_data: The PDT data whose authenticity must be checked. :param recordset tx_sudo: The sudoed transaction referenced in the PDT, as a `payment.transaction` record :return: The retrieved notification data @@ -90,34 +113,24 @@ class PaypalController(http.Controller): ref=tx_sudo.reference, )) raise Forbidden("PayPal: PDT are not enabled; cannot verify data origin") - else: + else: # The PayPal account is configured to send PDT data. + # Request a PDT data authenticity check and the notification data to PayPal. provider_sudo = tx_sudo.provider_id - if not provider_sudo.paypal_pdt_token: # We received PDT data but can't verify them - record_link = f'{html_escape(provider_sudo.name)}' - tx_sudo._log_message_on_linked_documents(_( - "The status of transaction with reference %(ref)s was not synchronized because " - "the PDT Identify Token is not configured on the provider %(record_link)s.", - ref=tx_sudo.reference, record_link=record_link - )) - raise Forbidden("PayPal: The PDT token is not set; cannot verify data origin") - else: # The PayPal account is configured to receive PDT data, and the PDT token is set - # Request a PDT data authenticity check and the notification data to PayPal - url = provider_sudo._paypal_get_api_url() - payload = { - 'cmd': '_notify-synch', - 'tx': pdt_data['tx'], - 'at': tx_sudo.provider_id.paypal_pdt_token, - } - try: - response = requests.post(url, data=payload, timeout=10) - response.raise_for_status() - except (requests.exceptions.ConnectionError, requests.exceptions.HTTPError): - raise Forbidden("PayPal: Encountered an error when verifying PDT origin") - else: - notification_data = self._parse_pdt_validation_response(response.text) - if notification_data is None: - raise Forbidden("PayPal: The PDT origin was not verified by PayPal") + url = provider_sudo._paypal_get_api_url() + payload = { + 'cmd': '_notify-synch', + 'tx': pdt_data['tx'], + 'at': tx_sudo.provider_id.paypal_pdt_token, + } + try: + response = requests.post(url, data=payload, timeout=10) + response.raise_for_status() + except (requests.exceptions.ConnectionError, requests.exceptions.HTTPError): + raise Forbidden("PayPal: Encountered an error when verifying PDT origin") + else: + notification_data = self._parse_pdt_validation_response(response.text) + if notification_data is None: + raise Forbidden("PayPal: The PDT origin was not verified by PayPal") return notification_data diff --git a/addons/payment_paypal/data/neutralize.sql b/addons/payment_paypal/data/neutralize.sql index 5e56d33b333..83c898f0a9a 100644 --- a/addons/payment_paypal/data/neutralize.sql +++ b/addons/payment_paypal/data/neutralize.sql @@ -1,5 +1,4 @@ -- disable paypal payment provider UPDATE payment_provider SET paypal_email_account = NULL, - paypal_seller_account = NULL, paypal_pdt_token = NULL; diff --git a/addons/payment_paypal/data/payment_paypal_email_data.xml b/addons/payment_paypal/data/payment_paypal_email_data.xml deleted file mode 100644 index 83b25b2448a..00000000000 --- a/addons/payment_paypal/data/payment_paypal_email_data.xml +++ /dev/null @@ -1,25 +0,0 @@ - - - - - - diff --git a/addons/payment_paypal/models/payment_provider.py b/addons/payment_paypal/models/payment_provider.py index 0aa4c998e1f..b3a966133dc 100644 --- a/addons/payment_paypal/models/payment_provider.py +++ b/addons/payment_paypal/models/payment_provider.py @@ -6,6 +6,7 @@ from odoo import _, fields, models from odoo.addons.payment_paypal.const import SUPPORTED_CURRENCIES + _logger = logging.getLogger(__name__) @@ -13,16 +14,15 @@ class PaymentProvider(models.Model): _inherit = 'payment.provider' code = fields.Selection( - selection_add=[('paypal', "Paypal")], ondelete={'paypal': 'set default'}) + selection_add=[('paypal', "Paypal")], ondelete={'paypal': 'set default'} + ) paypal_email_account = fields.Char( string="Email", help="The public business email solely used to identify the account with PayPal", - required_if_provider='paypal') - paypal_seller_account = fields.Char( - string="Merchant Account ID", groups='base.group_system') + required_if_provider='paypal', + default=lambda self: self.env.company.email, + ) paypal_pdt_token = fields.Char(string="PDT Identity Token", groups='base.group_system') - paypal_use_ipn = fields.Boolean( - string="Use IPN", help="Paypal Instant Payment Notification", default=True) #=== COMPUTE METHODS ===# @@ -58,20 +58,3 @@ class PaymentProvider(models.Model): return 'https://www.paypal.com/cgi-bin/webscr' else: return 'https://www.sandbox.paypal.com/cgi-bin/webscr' - - def _paypal_send_configuration_reminder(self): - render_template = self.env['ir.qweb']._render( - 'payment_paypal.mail_template_paypal_invite_user_to_configure', - {'provider': self}, - raise_if_not_found=False, - ) - if render_template: - mail_body = self.env['mail.render.mixin']._replace_local_links(render_template) - mail_values = { - 'body_html': mail_body, - 'subject': _("Add your PayPal account to Odoo"), - 'email_to': self.paypal_email_account, - 'email_from': self.create_uid.email_formatted, - 'author_id': self.create_uid.partner_id.id, - } - self.env['mail.mail'].sudo().create(mail_values).send() diff --git a/addons/payment_paypal/models/payment_transaction.py b/addons/payment_paypal/models/payment_transaction.py index 2790ac6507a..e54b673a852 100644 --- a/addons/payment_paypal/models/payment_transaction.py +++ b/addons/payment_paypal/models/payment_transaction.py @@ -35,12 +35,17 @@ class PaymentTransaction(models.Model): return res base_url = self.provider_id.get_base_url() + cancel_url = urls.url_join(base_url, PaypalController._cancel_url) + cancel_url_params = { + 'tx_ref': self.reference, + 'access_token': payment_utils.generate_access_token(self.reference), + } partner_first_name, partner_last_name = payment_utils.split_partner_name(self.partner_name) - webhook_url = urls.url_join(base_url, PaypalController._webhook_url) return { 'address1': self.partner_address, 'amount': self.amount, 'business': self.provider_id.paypal_email_account, + 'cancel_url': f'{cancel_url}?{urls.url_encode(cancel_url_params)}', 'city': self.partner_city, 'country': self.partner_country_id.code, 'currency_code': self.currency_id.name, @@ -51,7 +56,7 @@ class PaymentTransaction(models.Model): 'item_number': self.reference, 'last_name': partner_last_name, 'lc': self.partner_lang, - 'notify_url': webhook_url if self.provider_id.paypal_use_ipn else None, + 'notify_url': urls.url_join(base_url, PaypalController._webhook_url), 'return_url': urls.url_join(base_url, PaypalController._return_url), 'state': self.partner_state_id.name, 'zip_code': self.partner_zip, @@ -92,6 +97,10 @@ class PaymentTransaction(models.Model): if self.provider_code != 'paypal': return + if not notification_data: + self._set_canceled(_("The customer left the payment page.")) + return + txn_id = notification_data.get('txn_id') txn_type = notification_data.get('txn_type') if not all((txn_id, txn_type)): @@ -106,11 +115,6 @@ class PaymentTransaction(models.Model): payment_status = notification_data.get('payment_status') - if payment_status in PAYMENT_STATUS_MAPPING['pending'] + PAYMENT_STATUS_MAPPING['done'] \ - and not (self.provider_id.paypal_pdt_token and self.provider_id.paypal_seller_account): - # If a payment is made on an account waiting for configuration, send a reminder email - self.provider_id._paypal_send_configuration_reminder() - if payment_status in PAYMENT_STATUS_MAPPING['pending']: self._set_pending(state_message=notification_data.get('pending_reason')) elif payment_status in PAYMENT_STATUS_MAPPING['done']: diff --git a/addons/payment_paypal/tests/test_paypal.py b/addons/payment_paypal/tests/test_paypal.py index 5c450fcc2ed..666de434f2e 100644 --- a/addons/payment_paypal/tests/test_paypal.py +++ b/addons/payment_paypal/tests/test_paypal.py @@ -2,6 +2,8 @@ from unittest.mock import patch +from werkzeug import urls + from odoo.exceptions import ValidationError from odoo.tests import tagged from odoo.tools import float_repr, mute_logger @@ -16,11 +18,16 @@ class PaypalTest(PaypalCommon, PaymentHttpCommon): def _get_expected_values(self): return_url = self._build_url(PaypalController._return_url) + cancel_url = self._build_url(PaypalController._cancel_url) + cancel_url_params = { + 'tx_ref': self.reference, + 'access_token': self._generate_test_access_token(self.reference), + } values = { 'address1': 'Huge Street 2/543', 'amount': str(self.amount), 'business': self.paypal.paypal_email_account, - 'cancel_return': return_url, + 'cancel_return': f'{cancel_url}?{urls.url_encode(cancel_url_params)}', 'city': 'Sin City', 'cmd': '_xclick', 'country': 'BE', @@ -45,9 +52,12 @@ class PaypalTest(PaypalCommon, PaymentHttpCommon): return values + @mute_logger('odoo.addons.payment.models.payment_transaction') def test_redirect_form_values(self): tx = self._create_transaction(flow='redirect') - with mute_logger('odoo.addons.payment.models.payment_transaction'): + with patch( + 'odoo.addons.payment.utils.generate_access_token', new=self._generate_test_access_token + ): processing_values = tx._get_processing_values() form_info = self._extract_values_from_html_form(processing_values['redirect_form_html']) @@ -57,9 +67,12 @@ class PaypalTest(PaypalCommon, PaymentHttpCommon): expected_values = self._get_expected_values() self.assertDictEqual( - expected_values, form_info['inputs'], - "Paypal: invalid inputs specified in the redirect form.") + expected_values, + form_info['inputs'], + "Paypal: invalid inputs specified in the redirect form.", + ) + @mute_logger('odoo.addons.payment.models.payment_transaction') def test_redirect_form_with_fees(self): self.paypal.write({ 'fees_active': True, @@ -71,7 +84,9 @@ class PaypalTest(PaypalCommon, PaymentHttpCommon): expected_values = self._get_expected_values() tx = self._create_transaction(flow='redirect') - with mute_logger('odoo.addons.payment.models.payment_transaction'): + with patch( + 'odoo.addons.payment.utils.generate_access_token', new=self._generate_test_access_token + ): processing_values = tx._get_processing_values() form_info = self._extract_values_from_html_form(processing_values['redirect_form_html']) diff --git a/addons/payment_paypal/views/payment_paypal_templates.xml b/addons/payment_paypal/views/payment_paypal_templates.xml index f9fee3fdb46..a9e710dc44d 100644 --- a/addons/payment_paypal/views/payment_paypal_templates.xml +++ b/addons/payment_paypal/views/payment_paypal_templates.xml @@ -7,7 +7,7 @@ - + @@ -20,8 +20,7 @@ - + - - - - +