[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) <anv@odoo.com>
2023-02-17 14:09:43 +01:00
parent 5a5a00a352
commit e5c187ade8
12 changed files with 113 additions and 149 deletions
@@ -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)

@@ -14,19 +14,12 @@
                        <div class="col" name="right-column">
                            <div attrs="{'invisible': [('payment_method', '!=', 'paypal')]}">
                                <group>
-                                    <field name="paypal_user_type" widget="radio" nolabel="1" colspan="2"/>
                                    <field name="paypal_email_account" attrs="{'required': [('payment_method', '=', 'paypal')]}" string="Email"/>
-                                    <field name="paypal_seller_account" attrs="{'invisible': [('paypal_user_type', '=', 'new_user')], 'required': [('paypal_user_type', '!=', 'new_user'), ('payment_method', '=', 'paypal')]}" />
-                                    <field name="paypal_pdt_token" password="True" attrs="{'invisible': [('paypal_user_type', '=', 'new_user')], 'required': [('paypal_user_type', '!=', 'new_user'), ('payment_method', '=', 'paypal')]}" />
+                                    <field name="paypal_pdt_token" password="True" attrs="{'required': [('payment_method', '=', 'paypal')]}" />
                                </group>
-                                <p attrs="{'invisible': [('paypal_user_type', '!=', 'new_user')]}">
-                                    <span>Start selling directly without an account; an email will be sent by Paypal to create your new account and collect your payments.</span>
-                                </p>
-                                <p attrs="{'invisible': [('paypal_user_type', '=', 'new_user')]}">
-                                    <a href="https://www.odoo.com/documentation/master/applications/finance/payment_providers/paypal.html" target="_blank">
-                                        <span><i class="fa fa-arrow-right"/> How to configure your PayPal account</span>
-                                    </a>
-                                </p>
+                                <a href="https://www.odoo.com/documentation/master/applications/finance/payment_providers/paypal.html" target="_blank">
+                                    <span><i class="fa fa-arrow-right"/> How to configure your PayPal account</span>
+                                </a>
                            </div>

                            <div attrs="{'invisible': [('payment_method', '!=', 'manual')]}">
@@ -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)
@@ -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',
@@ -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'<a href=# data-oe-model=payment.provider ' \
-                              f'data-oe-id={provider_sudo.id}>{html_escape(provider_sudo.name)}</a>'
-                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

@@ -1,5 +1,4 @@
 -- disable paypal payment provider
 UPDATE payment_provider
   SET paypal_email_account = NULL,
-       paypal_seller_account = NULL,
       paypal_pdt_token = NULL;
@@ -1,25 +0,0 @@
-<?xml version="1.0" ?>
-<odoo>
-
-    <template id="mail_template_paypal_invite_user_to_configure">
-        <div>
-            <p>
-                Hello,
-                <br/><br/>
-                You have received a payment through PayPal.<br/>
-                Kindly follow the instructions given by PayPal to create your account.<br/>
-                Then, help us complete your Paypal credentials in Odoo.<br/><br/>
-            </p>
-            <a t-attf-href="/web#id=#{provider.id}&amp;model=payment.provider&amp;view_type=form"
-               style="background-color: #875A7B; padding: 10px; text-decoration: none; color: #fff; border-radius: 5px; font-size: 12px;">
-                Set Paypal credentials
-            </a>
-            <p>
-                <br/><br/>
-                Thanks,<br/>
-                <b>The Odoo Team</b>
-            </p>
-        </div>
-    </template>
-
-</odoo>
@@ -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()
@@ -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']:
@@ -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'])

@@ -7,7 +7,7 @@
            <input type="hidden" name="address1" t-att-value="address1"/>
            <input type="hidden" name="amount" t-att-value="amount"/>
            <input type="hidden" name="business" t-att-value="business"/>
-            <input type="hidden" name="cancel_return" t-att-value="return_url"/>
+            <input type="hidden" name="cancel_return" t-att-value="cancel_url"/>
            <input type="hidden" name="city" t-att-value="city"/>
            <input type="hidden" name="cmd" value="_xclick"/>
            <input type="hidden" name="country" t-att-value="country"/>
@@ -20,8 +20,7 @@
            <input type="hidden" name="item_number" t-att-value="item_number"/>
            <input type="hidden" name="last_name" t-att-value="last_name"/>
            <input type="hidden" name="lc" t-att-value="lc"/>
-            <input t-if="notify_url"
-                   type="hidden" name="notify_url" t-att-value="notify_url"/>
+            <input type="hidden" name="notify_url" t-att-value="notify_url"/>
            <input type="hidden" name="return" t-att-value="return_url"/>
            <input type="hidden" name="rm" value="2"/>
            <input t-if="state"
@@ -10,13 +10,8 @@
                <group attrs="{'invisible': [('code', '!=', 'paypal')]}">
                    <field name="paypal_email_account"
                           attrs="{'required':[('code', '=', 'paypal'), ('state', '!=', 'disabled')]}"/>
-                    <field name="paypal_seller_account" password="True"/>
-                    <!-- This field should no longer be used but is kept in debug mode for the time
-                         being, until we are sure that the verification protocol of IPN can be used
-                         for DPT notifications -->
-                    <field name="paypal_pdt_token" password="True"/>
-                    <field name="paypal_use_ipn"
-                           attrs="{'required':[('code', '=', 'paypal'), ('state', '!=', 'disabled')]}"/>
+                    <field name="paypal_pdt_token"  password="True"
+                           attrs="{'required': [('code', '=', 'paypal'), ('state', '!=', 'disabled')]}"/>
                    <a href="https://www.odoo.com/documentation/master/applications/finance/payment_providers/paypal.html"
                       target="_blank"
                       colspan="2">