From 22ea3519d72391dbc9e2602b1c586d592e9f8ac6 Mon Sep 17 00:00:00 2001 From: "Antoine Vandevenne (anv)" Date: Wed, 3 May 2023 18:22:12 +0200 Subject: [PATCH] [FIX] payment(_*): make a proper usage of async and promises Several functions were: - called with `await` while there are synchronous; - declared as synchronous while they should have been asynchronous; - declared as synchronous but their overrides were async; - explicitly encapsulating their return values in a `Promise` when it was unnecessary. This commit also cleans up a few mistakes in comments and docstrings. task-2882677 Part-of: odoo/odoo#120446 --- addons/payment/static/src/js/checkout_form.js | 6 +-- .../static/src/js/express_checkout_form.js | 8 ++- addons/payment/static/src/js/manage_form.js | 12 ++--- .../static/src/js/payment_form_mixin.js | 52 ++++++++----------- .../static/src/js/payment_form.js | 26 +++++----- .../static/src/js/payment_form.js | 30 ++++++----- .../static/src/js/payment_form.js | 15 +++--- .../static/src/js/express_checkout_form.js | 7 +-- .../static/src/js/website_payment_form.js | 9 ++-- .../static/src/js/website_sale_payment.js | 14 ++--- 10 files changed, 89 insertions(+), 90 deletions(-) diff --git a/addons/payment/static/src/js/checkout_form.js b/addons/payment/static/src/js/checkout_form.js index 32302fbc754..b0db9ec5265 100644 --- a/addons/payment/static/src/js/checkout_form.js +++ b/addons/payment/static/src/js/checkout_form.js @@ -42,9 +42,9 @@ publicWidget.registry.PaymentCheckoutForm = publicWidget.Widget.extend(paymentFo * * @private * @param {Event} ev - * @return {undefined} + * @return {void} */ - _onClickPay: async function (ev) { + _onClickPay: function (ev) { ev.stopPropagation(); ev.preventDefault(); @@ -84,7 +84,7 @@ publicWidget.registry.PaymentCheckoutForm = publicWidget.Widget.extend(paymentFo * * @private * @param {Event} ev - * @return {undefined} + * @return {void} */ _onSubmit: function (ev) { ev.stopPropagation(); diff --git a/addons/payment/static/src/js/express_checkout_form.js b/addons/payment/static/src/js/express_checkout_form.js index 7d4e86b03ac..b9198e249a4 100644 --- a/addons/payment/static/src/js/express_checkout_form.js +++ b/addons/payment/static/src/js/express_checkout_form.js @@ -45,11 +45,9 @@ publicWidget.registry.PaymentExpressCheckoutForm = publicWidget.Widget.extend({ * * @private * @param {Object} providerData - The provider-specific data. - * @return {Promise} + * @return {void} */ - async _prepareExpressCheckoutForm(providerData) { - return Promise.resolve(); - }, + async _prepareExpressCheckoutForm(providerData) {}, /** * Prepare the params to send to the transaction route. @@ -85,7 +83,7 @@ publicWidget.registry.PaymentExpressCheckoutForm = publicWidget.Widget.extend({ * @private * @param {number} newAmount - The new amount. * @param {number} newMinorAmount - The new minor amount. - * @return {undefined} + * @return {void} */ _updateAmount(newAmount, newMinorAmount) { this.txContext.amount = parseFloat(newAmount); diff --git a/addons/payment/static/src/js/manage_form.js b/addons/payment/static/src/js/manage_form.js index ace737e3a38..3c1ad48144f 100644 --- a/addons/payment/static/src/js/manage_form.js +++ b/addons/payment/static/src/js/manage_form.js @@ -46,7 +46,7 @@ publicWidget.registry.PaymentManageForm = publicWidget.Widget.extend(paymentForm * * @private * @param {number} tokenId - The id of the token to assign - * @return {undefined} + * @return {void} */ _assignToken: function (tokenId) { // Call the assign route to assign the token to a record @@ -95,7 +95,7 @@ publicWidget.registry.PaymentManageForm = publicWidget.Widget.extend(paymentForm * * @private * @param {number} tokenId - The id of the token to delete - * @return {undefined} + * @return {void} */ _deleteToken: function (tokenId) { const execute = () => { @@ -151,7 +151,7 @@ publicWidget.registry.PaymentManageForm = publicWidget.Widget.extend(paymentForm * * @private * @param {Event} ev - * @return {undefined} + * @return {void} */ _onClickDeleteToken: function (ev) { ev.preventDefault(); @@ -171,9 +171,9 @@ publicWidget.registry.PaymentManageForm = publicWidget.Widget.extend(paymentForm * * @private * @param {Event} ev - * @return {undefined} + * @return {void} */ - _onClickSaveToken: async function (ev) { + _onClickSaveToken: function (ev) { ev.stopPropagation(); ev.preventDefault(); @@ -209,7 +209,7 @@ publicWidget.registry.PaymentManageForm = publicWidget.Widget.extend(paymentForm * * @private * @param {Event} ev - * @return {undefined} + * @return {void} */ _onSubmit: function (ev) { ev.stopPropagation(); diff --git a/addons/payment/static/src/js/payment_form_mixin.js b/addons/payment/static/src/js/payment_form_mixin.js index a58a50f26c7..bb840a1532b 100644 --- a/addons/payment/static/src/js/payment_form_mixin.js +++ b/addons/payment/static/src/js/payment_form_mixin.js @@ -45,7 +45,7 @@ import { _t } from '@web/core/l10n/translation'; * * @private * @param {boolean} showLoadingAnimation - Whether a spinning loader should be shown - * @return {undefined} + * @return {void} */ _disableButton(showLoadingAnimation = true) { const $submitButton = $('button[name="o_payment_submit_button"]'); @@ -109,7 +109,7 @@ import { _t } from '@web/core/l10n/translation'; * * @private * @param {HTMLInputElement} radio - The radio button linked to the payment option - * @return {undefined} + * @return {void} */ _displayInlineForm: function (radio) { this._hideInlineForms(); // Collapse previously opened inline forms @@ -252,10 +252,10 @@ import { _t } from '@web/core/l10n/translation'; * Collapse all inline forms of the current widget. * * @private - * @return {undefined}. + * @return {void}. */ _hideInlineForms() { - return this.$('[name="o_payment_inline_form"]').addClass('d-none'); + this.$('[name="o_payment_inline_form"]').addClass('d-none'); }, /** @@ -266,7 +266,7 @@ import { _t } from '@web/core/l10n/translation'; * another inline form. * * @private - * @return {undefined} + * @return {void} */ _hideInputs: function () { const $submitButton = this.$('button[name="o_payment_submit_button"]'); @@ -333,18 +333,16 @@ import { _t } from '@web/core/l10n/translation'; * Prepare the provider-specific inline form of the selected payment option. * * For a provider to manage an inline form, it must override this method. When the override - * is called, it must lookup the parameters to decide whether it is necessary to prepare its - * inline form. Otherwise, the call must be sent back to the parent method. + * is called, it must determine whether it is necessary to prepare its inline form. Otherwise, + * the call must be sent back to the parent method. * * @private * @param {string} code - The code of the selected payment option's provider * @param {number} paymentOptionId - The id of the selected payment option * @param {string} flow - The online payment flow of the selected payment option - * @return {Promise} + * @return {void} */ - _prepareInlineForm(code, paymentOptionId, flow) { - return Promise.resolve(); - }, + _prepareInlineForm(code, paymentOptionId, flow) {}, /** * Process the payment. @@ -359,22 +357,20 @@ import { _t } from '@web/core/l10n/translation'; * @param {string} code - The code of the payment option's provider * @param {number} paymentOptionId - The id of the payment option handling the transaction * @param {string} flow - The online payment flow of the transaction - * @return {Promise} + * @return {void} */ _processPayment: function (code, paymentOptionId, flow) { // Call the transaction route to create a tx and retrieve the processing values - return this._rpc({ + this._rpc({ route: this.txContext['transactionRoute'], params: this._prepareTransactionRouteParams(code, paymentOptionId, flow), }).then(processingValues => { if (flow === 'redirect') { - return this._processRedirectPayment( - code, paymentOptionId, processingValues - ); + this._processRedirectPayment(code, paymentOptionId, processingValues); } else if (flow === 'direct') { - return this._processDirectPayment(code, paymentOptionId, processingValues); + this._processDirectPayment(code, paymentOptionId, processingValues); } else if (flow === 'token') { - return this._processTokenPayment(code, paymentOptionId, processingValues); + this._processTokenPayment(code, paymentOptionId, processingValues); } }).guardedCatch(error => { error.event.preventDefault(); @@ -396,11 +392,9 @@ import { _t } from '@web/core/l10n/translation'; * @param {string} code - The code of the provider * @param {number} providerId - The id of the provider handling the transaction * @param {object} processingValues - The processing values of the transaction - * @return {Promise} + * @return {void} */ - _processDirectPayment(code, providerId, processingValues) { - return Promise.resolve(); - }, + _processDirectPayment(code, providerId, processingValues) {}, /** * Redirect the customer by submitting the redirect form included in the processing values. @@ -412,7 +406,7 @@ import { _t } from '@web/core/l10n/translation'; * @param {string} code - The code of the provider * @param {number} providerId - The id of the provider handling the transaction * @param {object} processingValues - The processing values of the transaction - * @return {undefined} + * @return {void} */ _processRedirectPayment(code, providerId, processingValues) { // Append the redirect form to the body @@ -437,7 +431,7 @@ import { _t } from '@web/core/l10n/translation'; * @param {string} provider_code - The code of the token's provider * @param {number} tokenId - The id of the token handling the transaction * @param {object} processingValues - The processing values of the transaction - * @return {undefined} + * @return {void} */ _processTokenPayment(provider_code, tokenId, processingValues) { // The flow is already completed as payments by tokens are immediately processed @@ -454,7 +448,7 @@ import { _t } from '@web/core/l10n/translation'; * @private * @param {string} flow - The flow for the selected payment option. Either 'redirect', * 'direct' or 'token' - * @return {undefined} + * @return {void} */ _setPaymentFlow: function (flow = 'redirect') { if (flow !== 'redirect' && flow !== 'direct' && flow !== 'token') { @@ -472,7 +466,7 @@ import { _t } from '@web/core/l10n/translation'; * Show the "Save my payment details" label and checkbox, and the submit button. * * @private - * @return {undefined}. + * @return {void}. */ _showInputs: function () { const $submitButton = this.$('button[name="o_payment_submit_button"]'); @@ -492,7 +486,7 @@ import { _t } from '@web/core/l10n/translation'; * * @private * @param {Event} ev - * @return {undefined} + * @return {void} */ _onClickLessPaymentIcons(ev) { ev.preventDefault(); @@ -512,7 +506,7 @@ import { _t } from '@web/core/l10n/translation'; * * @private * @param {Event} ev - * @return {undefined} + * @return {void} */ _onClickMorePaymentIcons(ev) { ev.preventDefault(); @@ -530,7 +524,7 @@ import { _t } from '@web/core/l10n/translation'; * * @private * @param {Event} ev - * @return {undefined} + * @return {void} */ _onClickPaymentOption: function (ev) { // Uncheck all radio buttons diff --git a/addons/payment_adyen/static/src/js/payment_form.js b/addons/payment_adyen/static/src/js/payment_form.js index 4a4160fc993..6cafd22bf61 100644 --- a/addons/payment_adyen/static/src/js/payment_form.js +++ b/addons/payment_adyen/static/src/js/payment_form.js @@ -17,7 +17,7 @@ const adyenMixin = { * @private * @param {object} state - The state of the drop-in * @param {object} dropin - The drop-in - * @return {Promise} + * @return {void} */ _dropinOnAdditionalDetails: function (state, dropin) { return this._rpc({ @@ -48,7 +48,7 @@ const adyenMixin = { * * @private * @param {object} error - The error in the drop-in - * @return {undefined} + * @return {void} */ _dropinOnError: function (error) { if (!this.$('div[name="o_payment_error"]')) { // Don't replace a specific server error. @@ -68,7 +68,7 @@ const adyenMixin = { * @private * @param {object} state - The state of the drop-in * @param {object} dropin - The drop-in - * @return {Promise} + * @return {void} */ _dropinOnSubmit: function (state, dropin) { // Create the transaction and retrieve the processing values @@ -117,26 +117,27 @@ const adyenMixin = { * @param {string} code - The code of the selected payment option's provider * @param {number} paymentOptionId - The id of the selected payment option * @param {string} flow - The online payment flow of the selected payment option - * @return {Promise} + * @return {void} */ _prepareInlineForm: function (code, paymentOptionId, flow) { if (code !== 'adyen') { - return this._super(...arguments); + this._super(...arguments); + return; } // Check if instantiation of the drop-in is needed if (flow === 'token') { - return Promise.resolve(); // No drop-in for tokens + return; // No drop-in for tokens } else if (this.adyenDropin && this.adyenDropin.providerId === paymentOptionId) { this._setPaymentFlow('direct'); // Overwrite the flow even if no re-instantiation - return Promise.resolve(); // Don't re-instantiate if already done for this provider + return; // Don't re-instantiate if already done for this provider } // Overwrite the flow of the select payment option this._setPaymentFlow('direct'); // Get public information on the provider (state, client_key) - return this._rpc({ + this._rpc({ route: '/payment/adyen/provider_info', params: { 'provider_id': paymentOptionId, @@ -197,18 +198,19 @@ const adyenMixin = { * @param {string} provider - The provider of the payment option's provider * @param {number} paymentOptionId - The id of the payment option handling the transaction * @param {string} flow - The online payment flow of the transaction - * @return {Promise} + * @return {void} */ - async _processPayment(provider, paymentOptionId, flow) { + _processPayment(provider, paymentOptionId, flow) { if (provider !== 'adyen' || flow === 'token') { - return this._super(...arguments); // Tokens are handled by the generic flow + this._super(...arguments); // Tokens are handled by the generic flow + return; } if (this.adyenDropin === undefined) { // The drop-in has not been properly instantiated this._displayError( _t("Server Error"), _t("We are not able to process your payment.") ); } else { - return await this.adyenDropin.submit(); + this.adyenDropin.submit(); } }, diff --git a/addons/payment_authorize/static/src/js/payment_form.js b/addons/payment_authorize/static/src/js/payment_form.js index d36f53a442b..c7306d190c9 100644 --- a/addons/payment_authorize/static/src/js/payment_form.js +++ b/addons/payment_authorize/static/src/js/payment_form.js @@ -71,24 +71,25 @@ const authorizeMixin = { * * @override method from @payment/js/payment_form_mixin * @private - * @param {string} provider - The provider of the selected payment option's provider + * @param {string} code - The code of the selected payment option's provider * @param {number} paymentOptionId - The id of the selected payment option * @param {string} flow - The online payment flow of the selected payment option - * @return {Promise} + * @return {void} */ _prepareInlineForm: function (code, paymentOptionId, flow) { if (code !== 'authorize') { - return this._super(...arguments); + this._super(...arguments); + return; } if (flow === 'token') { - return Promise.resolve(); // Don't show the form for tokens + return; // Don't show the form for tokens } this._setPaymentFlow('direct'); let acceptJSUrl = 'https://js.authorize.net/v1/Accept.js'; - return this._rpc({ + this._rpc({ route: '/payment/authorize/get_provider_info', params: { 'provider_id': paymentOptionId, @@ -118,17 +119,18 @@ const authorizeMixin = { * @param {string} code - The code of the payment option's provider * @param {number} paymentOptionId - The id of the payment option handling the transaction * @param {string} flow - The online payment flow of the transaction - * @return {Promise} + * @return {void} */ _processPayment: function (code, paymentOptionId, flow) { if (code !== 'authorize' || flow === 'token') { - return this._super(...arguments); // Tokens are handled by the generic flow + this._super(...arguments); // Tokens are handled by the generic flow + return; } if (!this._validateFormInputs(paymentOptionId)) { this._enableButton(); // The submit button is disabled at this point, enable it this.call('ui', 'unblock'); // The page is blocked at this point, unblock it - return Promise.resolve(); + return; } // Build the authentication and card data objects to be dispatched to Authorized.Net @@ -141,7 +143,7 @@ const authorizeMixin = { }; // Dispatch secure data to Authorize.Net to get a payment nonce in return - return Accept.dispatchData( + Accept.dispatchData( secureData, response => this._responseHandler(paymentOptionId, response) ); }, @@ -152,7 +154,7 @@ const authorizeMixin = { * @private * @param {number} providerId - The id of the selected provider * @param {object} response - The payment nonce returned by Authorized.Net - * @return {Promise} + * @return {void} */ _responseHandler: function (providerId, response) { if (response.messages.resultCode === 'Error') { @@ -163,11 +165,11 @@ const authorizeMixin = { _t("We are not able to process your payment."), error ); - return Promise.resolve(); + return; } // Create the transaction and retrieve the processing values - return this._rpc({ + this._rpc({ route: this.txContext.transactionRoute, params: this._prepareTransactionRouteParams('authorize', providerId, 'direct'), }).then(processingValues => { @@ -180,7 +182,9 @@ const authorizeMixin = { 'opaque_data': response.opaqueData, 'access_token': processingValues.access_token, } - }).then(() => window.location = '/payment/status'); + }); + }).then(() => { + window.location = '/payment/status'; }).guardedCatch((error) => { error.event.preventDefault(); this._displayError( diff --git a/addons/payment_demo/static/src/js/payment_form.js b/addons/payment_demo/static/src/js/payment_form.js index e0d15b5a160..3a02f45593a 100644 --- a/addons/payment_demo/static/src/js/payment_form.js +++ b/addons/payment_demo/static/src/js/payment_form.js @@ -17,16 +17,17 @@ const paymentDemoMixin = { * @param {string} code - The code of the provider * @param {number} providerId - The id of the provider handling the transaction * @param {object} processingValues - The processing values of the transaction - * @return {Promise} + * @return {void} */ _processDirectPayment: function (code, providerId, processingValues) { if (code !== 'demo') { - return this._super(...arguments); + this._super(...arguments); + return; } const customerInput = document.getElementById('customer_input').value; const simulatedPaymentState = document.getElementById('simulated_payment_state').value; - return this._rpc({ + this._rpc({ route: '/payment/demo/simulate_payment', params: { 'reference': processingValues.reference, @@ -46,16 +47,16 @@ const paymentDemoMixin = { * @param {string} code - The code of the selected payment option's provider * @param {integer} paymentOptionId - The id of the selected payment option * @param {string} flow - The online payment flow of the selected payment option - * @return {Promise} + * @return {void} */ _prepareInlineForm: function (code, paymentOptionId, flow) { if (code !== 'demo') { - return this._super(...arguments); + this._super(...arguments); + return; } else if (flow === 'token') { - return Promise.resolve(); + return; } this._setPaymentFlow('direct'); - return Promise.resolve(); }, }; diff --git a/addons/payment_stripe/static/src/js/express_checkout_form.js b/addons/payment_stripe/static/src/js/express_checkout_form.js index 1282ebf95d0..73d44c5463c 100644 --- a/addons/payment_stripe/static/src/js/express_checkout_form.js +++ b/addons/payment_stripe/static/src/js/express_checkout_form.js @@ -47,7 +47,7 @@ paymentExpressCheckoutForm.include({ * @override method from payment.express_form * @private * @param {Object} providerData - The provider-specific data. - * @return {Promise} + * @return {void} */ async _prepareExpressCheckoutForm(providerData) { /* @@ -56,7 +56,8 @@ paymentExpressCheckoutForm.include({ * the value when it equals '0'. */ if (providerData.providerCode !== 'stripe' || !this.txContext.amount) { - return this._super(...arguments); + this._super(...arguments); + return; } const stripeJS = Stripe( @@ -200,7 +201,7 @@ paymentExpressCheckoutForm.include({ * @private * @param {number} newAmount - The new amount. * @param {number} newMinorAmount - The new minor amount. - * @return {undefined} + * @return {void} */ _updateAmount(newAmount, newMinorAmount) { this._super(...arguments); diff --git a/addons/website_payment/static/src/js/website_payment_form.js b/addons/website_payment/static/src/js/website_payment_form.js index 1ec0f3078ef..6196f81ce19 100644 --- a/addons/website_payment/static/src/js/website_payment_form.js +++ b/addons/website_payment/static/src/js/website_payment_form.js @@ -9,9 +9,9 @@ checkoutForm.include({ /** * @override */ - start: function () { + async start() { core.bus.on('update_shipping_cost', this, this._updateShippingCost); - return this._super.apply(this, arguments); + return await this._super.apply(this, arguments); }, //-------------------------------------------------------------------------- @@ -26,7 +26,7 @@ checkoutForm.include({ * @param {string} code - The code of the payment option's provider * @param {number} paymentOptionId - The id of the payment option handling the transaction * @param {string} flow - The online payment flow of the transaction - * @return {Promise} + * @return {void} */ _processPayment: function (code, paymentOptionId, flow) { if ($('.o_donation_payment_form').length) { @@ -56,10 +56,9 @@ checkoutForm.include({ _t("Validation Error"), _t("Some information is missing to process your payment.") ); - return Promise.resolve(); } } - return this._super(...arguments); + this._super(...arguments); }, /** diff --git a/addons/website_sale/static/src/js/website_sale_payment.js b/addons/website_sale/static/src/js/website_sale_payment.js index b03869a5bee..954aa97e0e0 100644 --- a/addons/website_sale/static/src/js/website_sale_payment.js +++ b/addons/website_sale/static/src/js/website_sale_payment.js @@ -17,11 +17,11 @@ const websiteSalePaymentMixin = { /** * @override */ - start: function () { + start: async function () { this.$checkbox = this.$('#checkbox_tc'); this.$submitButton = this.$('button[name="o_payment_submit_button"]'); this._adaptConfirmButton(); - return this._super(...arguments); + return await this._super(...arguments); }, //-------------------------------------------------------------------------- @@ -32,7 +32,7 @@ const websiteSalePaymentMixin = { * Update the data on the submit button with the status of the Terms and Conditions input. * * @private - * @return {undefined} + * @return {void} */ _adaptConfirmButton: function () { if (this.$checkbox.length > 0) { @@ -75,7 +75,7 @@ checkoutForm.include(Object.assign({}, websiteSalePaymentMixin, { * Enable the submit button if it all conditions are met. * * @private - * @return {undefined} + * @return {void} */ _onClickTCCheckbox: function () { this._adaptConfirmButton(); @@ -97,11 +97,11 @@ publicWidget.registry.WebsiteSalePayment = publicWidget.Widget.extend( /** * @override */ - start: function () { + start: async function () { this.$checkbox = this.$('#checkbox_tc'); this.$submitButton = this.$('button[name="o_payment_submit_button"]'); this._onClickTCCheckbox(); - return this._super(...arguments); + return await this._super(...arguments); }, //-------------------------------------------------------------------------- @@ -112,7 +112,7 @@ publicWidget.registry.WebsiteSalePayment = publicWidget.Widget.extend( * Enable the submit button if it all conditions are met. * * @private - * @return {undefined} + * @return {void} */ _onClickTCCheckbox: function () { this._adaptConfirmButton();