From 0fc6fe8653952b359aa16f79eb22129217dbd101 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Thu, 19 Oct 2017 14:15:32 +0200 Subject: [PATCH] [FIX] payment_stripe: fix multiple usability problems - There was no feedback to the user when executing the Charge transaction in server-to-server mode, while it could take several seconds, with the normal UI/action buttons still available. - Stripe integration was almost working along with `website_quote` payment, but entirely broken with `website_payment`, and partially broken with `website_sale`. Fixing it required: + More leniency in processing optional transaction parameters, which may or may not be present in the various payment flows. + It also required more precautions when locating the transaction for which the Stripe Charge was to be created, which passed in different manners in the session. The route now supports an explicit `tx_id` to allow forcing the transaction without risk of mixing different payment flows. + FIXME: There is still some amount of duplication and bad modularity in the handling of the various payment flows in relation with Stripe. - We provided very little metadata to the Customer and Charge APIs of Stripe. We now pass more names and references to make Stripe payments easier to manage in the Stripe dashboard. - In some cases, selecting Stripe as payment method caused a second inclusion of `stripe.js`, raising a JS error because of the duplication. - Strip whitespace in emails: the Stripe API raises an error for transactions done with invalid emails, including with leading/trailing whitespace. Customers will have a hard time figuring out the problem by themselves, so we should at least strip whitespaces. --- addons/payment_stripe/controllers/main.py | 13 ++- addons/payment_stripe/models/payment.py | 41 ++++++--- addons/payment_stripe/static/src/js/stripe.js | 92 ++++++++++++------- 3 files changed, 93 insertions(+), 53 deletions(-) diff --git a/addons/payment_stripe/controllers/main.py b/addons/payment_stripe/controllers/main.py index b4627157a9e..bc9a775a330 100644 --- a/addons/payment_stripe/controllers/main.py +++ b/addons/payment_stripe/controllers/main.py @@ -29,9 +29,16 @@ class StripeController(http.Controller): """ Create a payment transaction Expects the result from the user input from checkout.js popup""" - tx = request.env['payment.transaction'].sudo().browse( - int(request.session.get('sale_transaction_id') or request.session.get('website_payment_tx_id', False)) - ) + TX = request.env['payment.transaction'] + tx = None + if post.get('tx_ref'): + tx = TX.sudo().search([('reference', '=', post['tx_ref'])]) + if not tx: + tx_id = (post.get('tx_id') or request.session.get('sale_transaction_id') or + request.session.get('website_payment_tx_id')) + tx = TX.sudo().browse(int(tx_id)) + if not tx: + raise werkzeug.exceptions.NotFound() response = tx._create_stripe_charge(tokenid=post['tokenid'], email=post['email']) _logger.info('Stripe: entering form_feedback with post data %s', pprint.pformat(response)) if response: diff --git a/addons/payment_stripe/models/payment.py b/addons/payment_stripe/models/payment.py index 0fe3dc746d9..c7dfc226512 100644 --- a/addons/payment_stripe/models/payment.py +++ b/addons/payment_stripe/models/payment.py @@ -43,13 +43,13 @@ class PaymentAcquirerStripe(models.Model): 'amount': tx_values.get('amount'), 'currency': tx_values.get('currency') and tx_values.get('currency').name or '', 'currency_id': tx_values.get('currency') and tx_values.get('currency').id or '', - 'address_line1': tx_values['partner_address'], - 'address_city': tx_values['partner_city'], - 'address_country': tx_values['partner_country'] and tx_values['partner_country'].name or '', - 'email': tx_values['partner_email'], - 'address_zip': tx_values['partner_zip'], - 'name': tx_values['partner_name'], - 'phone': tx_values['partner_phone'], + 'address_line1': tx_values.get('partner_address'), + 'address_city': tx_values.get('partner_city'), + 'address_country': tx_values.get('partner_country') and tx_values['partner_country'].name or '', + 'email': tx_values.get('partner_email'), + 'address_zip': tx_values.get('partner_zip'), + 'name': tx_values.get('partner_name'), + 'phone': tx_values.get('partner_phone'), } temp_stripe_tx_values['returndata'] = stripe_tx_values.pop('return_url', '') @@ -92,14 +92,15 @@ class PaymentTransactionStripe(models.Model): charge_params = { 'amount': int(self.amount if self.currency_id.name in INT_CURRENCIES else self.amount*100), 'currency': self.currency_id.name, - 'metadata[reference]': self.reference + 'metadata[reference]': self.reference, + 'description': self.reference, } if acquirer_ref: charge_params['customer'] = acquirer_ref if tokenid: charge_params['card'] = str(tokenid) if email: - charge_params['receipt_email'] = email + charge_params['receipt_email'] = email.strip() r = requests.post(api_url_charge, auth=(self.acquirer_id.stripe_secret_key, ''), params=charge_params, @@ -118,12 +119,17 @@ class PaymentTransactionStripe(models.Model): transaction record. """ reference = data.get('metadata', {}).get('reference') if not reference: - error_msg = _( - 'Stripe: invalid reply received from provider, missing reference. Additional message: %s' - % data.get('error', {}).get('message', '') - ) - _logger.error(error_msg) + stripe_error = data.get('error', {}).get('message', '') + _logger.error('Stripe: invalid reply received from stripe API, looks like ' + 'the transaction failed. (error: %s)', stripe_error or 'n/a') + error_msg = _("We're sorry to report that the transaction has failed.") + if stripe_error: + error_msg += " " + (_("Stripe gave us the following info about the problem: '%s'") % + stripe_error) + error_msg += " " + _("Perhaps the problem can be solved by double-checking your " + "credit card details, or contacting your bank?") raise ValidationError(error_msg) + tx = self.search([('reference', '=', reference)]) if not tx: error_msg = (_('Stripe: no order found for reference %s') % reference) @@ -190,6 +196,7 @@ class PaymentTokenStripe(models.Model): 'card[exp_month]': str(values['cc_expiry'][:2]), 'card[exp_year]': str(values['cc_expiry'][-2:]), 'card[cvc]': values['cvc'], + 'card[name]': values['cc_holder_name'], } r = requests.post(url_token, auth=(payment_acquirer.stripe_secret_key, ''), @@ -198,8 +205,12 @@ class PaymentTokenStripe(models.Model): token = r.json() if token.get('id'): customer_params = { - 'source': token['id'] + 'source': token['id'], + 'description': values['cc_holder_name'] } + if values.get('partner_id'): + partner = self.env['res.partner'].browse(values['partner_id']) + customer_params['email'] = partner.email and partner.email.strip() r = requests.post(url_customer, auth=(payment_acquirer.stripe_secret_key, ''), params=customer_params, diff --git a/addons/payment_stripe/static/src/js/stripe.js b/addons/payment_stripe/static/src/js/stripe.js index 14e19775747..b74601a172b 100644 --- a/addons/payment_stripe/static/src/js/stripe.js +++ b/addons/payment_stripe/static/src/js/stripe.js @@ -14,6 +14,14 @@ odoo.define('payment_stripe.stripe', function(require) { 'RWF', 'KRW', 'VUV', 'VND', 'XOF' ]; + if ($.blockUI) { + // our message needs to appear above the modal dialog + $.blockUI.defaults.baseZ = 2147483647; //same z-index as StripeCheckout + $.blockUI.defaults.css.border = '0'; + $.blockUI.defaults.css["background-color"] = ''; + $.blockUI.defaults.overlayCSS["opacity"] = '0.9'; + } + var handler = StripeCheckout.configure({ key: $("input[name='stripe_key']").val(), image: $("input[name='stripe_image']").val(), @@ -27,6 +35,14 @@ odoo.define('payment_stripe.stripe', function(require) { }, token: function(token, args) { handler.isTokenGenerate = true; + if ($.blockUI) { + var msg = _t("Just one more second, confirming your payment..."); + $.blockUI({ + 'message': '

' + + '
' + msg + + '

' + }); + } ajax.jsonRpc("/payment/stripe/create_charge", 'call', { tokenid: token.id, email: token.email, @@ -34,12 +50,17 @@ odoo.define('payment_stripe.stripe', function(require) { acquirer_id: $("#acquirer_stripe").val(), currency: $("input[name='currency']").val(), invoice_num: $("input[name='invoice_num']").val(), + tx_ref: $("input[name='invoice_num']").val(), return_url: $("input[name='return_url']").val() + }).always(function(){ + if ($.blockUI) { + $.unblockUI(); + } }).done(function(data){ handler.isTokenGenerate = false; window.location.href = data; }).fail(function(){ - var msg = arguments && arguments[1] && arguments[1].data && arguments[1].data.message; + var msg = arguments && arguments[1] && arguments[1].data && arguments[1].data.arguments && arguments[1].data.arguments[0]; var wizard = $(qweb.render('stripe.error', {'msg': msg || _t('Payment error')})); wizard.appendTo($('body')).modal({'keyboard': true}); }); @@ -66,44 +87,45 @@ odoo.define('payment_stripe.stripe', function(require) { } e.preventDefault(); + + var currency = $("input[name='currency']").val(); + var currency_id = $("input[name='currency_id']").val(); + var amount = parseFloat($("input[name='amount']").val() || '0.0'); + + if ($('.o_website_payment').length !== 0) { - var currency = $("input[name='currency']").val(); - var amount = parseFloat($("input[name='amount']").val() || '0.0'); - if (!_.contains(int_currencies, currency)) { - amount = amount*100; - } - - ajax.jsonRpc('/website_payment/transaction', 'call', { + var create_tx = ajax.jsonRpc('/website_payment/transaction', 'call', { reference: $("input[name='invoice_num']").val(), - amount: amount, - currency_id: currency, + amount: amount, // exact amount, not stripe cents + currency_id: currency_id, acquirer_id: acquirer_id - }) - handler.open({ - name: $("input[name='merchant']").val(), - description: $("input[name='invoice_num']").val(), - currency: currency, - amount: amount, - }); - } else { - var currency = $("input[name='currency']").val(); - var amount = parseFloat($("input[name='amount']").val() || '0.0'); - if (!_.contains(int_currencies, currency)) { - amount = amount*100; - } - - ajax.jsonRpc('/shop/payment/transaction/' + acquirer_id, 'call', { - so_id: so_id, - so_token: so_token - }, {'async': false}).then(function (data) { - $form.html(data); - handler.open({ - name: $("input[name='merchant']").val(), - description: $("input[name='invoice_num']").val(), - currency: currency, - amount: amount, - }); }); } + else if ($('.o_website_quote').length !== 0) { + var url = _.str.sprintf("/quote/%s/transaction/%s/%s", so_id, acquirer_id, so_token); + var create_tx = ajax.jsonRpc(url, 'call', {}).then(function (data) { + try { $form.html(data); } catch (e) {}; + }); + } + else { + var create_tx = ajax.jsonRpc('/shop/payment/transaction/' + acquirer_id, 'call', { + so_id: so_id, + so_token: so_token + }).then(function (data) { + try { $form.html(data); } catch (e) {}; + }); + } + create_tx.done(function () { + if (!_.contains(int_currencies, currency)) { + amount = amount*100; + } + handler.open({ + name: $("input[name='merchant']").val(), + description: $("input[name='invoice_num']").val(), + email: $("input[name='email']").val(), + currency: currency, + amount: amount, + }); + }); }); });