From 6da353d66f42965f83db87caee3d1ab468b556af Mon Sep 17 00:00:00 2001 From: pedrambiria Date: Fri, 19 Aug 2022 08:55:52 +0000 Subject: [PATCH] [FIX] point_of_sale: remove "willstart" for a performance issue Before this commit: if the number of contacts is enormous, and the "Limited Partners Loading" with "Load all remaining partners in the background" is enabled, clicking on the product information button would take lots of time to reload. Also, it would cause the same issue for the "ClosePoSPopup". The problem is that when "willstart" takes time more than the background process it would call it again, and for every call. The solution is to prepare the "willstart" function's data and pass it to the constructor. opw-2956104 closes odoo/odoo#100177 X-original-commit: 940c72d0a058240f75d3f2673a4145e8c1a0921a Signed-off-by: Masereel Pierre --- .../l10n_fr_pos_cert/static/src/js/Chrome.js | 3 +- .../src/js/ChromeWidgets/HeaderButton.js | 5 +- .../static/src/js/Popups/ClosePosPopup.js | 45 +----------- .../static/src/js/Popups/ProductInfoPopup.js | 44 +---------- .../ControlButtons/ProductInfoButton.js | 5 +- .../js/Screens/ProductScreen/ProductItem.js | 5 +- addons/point_of_sale/static/src/js/models.js | 73 +++++++++++++++++++ .../static/src/js/models.js | 4 +- 8 files changed, 90 insertions(+), 94 deletions(-) diff --git a/addons/l10n_fr_pos_cert/static/src/js/Chrome.js b/addons/l10n_fr_pos_cert/static/src/js/Chrome.js index f68a7e7f912..f3ba3ba8ed7 100644 --- a/addons/l10n_fr_pos_cert/static/src/js/Chrome.js +++ b/addons/l10n_fr_pos_cert/static/src/js/Chrome.js @@ -13,7 +13,8 @@ odoo.define('l10n_fr_pos_cert.Chrome', function (require) { let limitDate = new Date(this.env.pos.pos_session.start_at); limitDate.setDate(limitDate.getDate() + 1); if (limitDate < now) { - this.showPopup('ClosePosPopup'); + const info = await this.env.pos.getClosePosInfo(); + this.showPopup('ClosePosPopup', { info: info }); } } } diff --git a/addons/point_of_sale/static/src/js/ChromeWidgets/HeaderButton.js b/addons/point_of_sale/static/src/js/ChromeWidgets/HeaderButton.js index 7dbb5efd3a2..cbd725aab5d 100644 --- a/addons/point_of_sale/static/src/js/ChromeWidgets/HeaderButton.js +++ b/addons/point_of_sale/static/src/js/ChromeWidgets/HeaderButton.js @@ -7,8 +7,9 @@ odoo.define('point_of_sale.HeaderButton', function(require) { // Previously HeaderButtonWidget // This is the close session button class HeaderButton extends PosComponent { - onClick() { - this.showPopup('ClosePosPopup', { keepBehind: true }); + async onClick() { + const info = await this.env.pos.getClosePosInfo(); + this.showPopup('ClosePosPopup', { info: info, keepBehind: true }); } } HeaderButton.template = 'HeaderButton'; diff --git a/addons/point_of_sale/static/src/js/Popups/ClosePosPopup.js b/addons/point_of_sale/static/src/js/Popups/ClosePosPopup.js index 63419c95f0a..0fd3e38fc42 100644 --- a/addons/point_of_sale/static/src/js/Popups/ClosePosPopup.js +++ b/addons/point_of_sale/static/src/js/Popups/ClosePosPopup.js @@ -6,7 +6,7 @@ odoo.define('point_of_sale.ClosePosPopup', function(require) { const { identifyError } = require('point_of_sale.utils'); const { ConnectionLostError, ConnectionAbortedError} = require('@web/core/network/rpc_service') - const { onWillStart, useState } = owl; + const { useState } = owl; /** * This popup needs to be self-dependent because it needs to be called from different place. @@ -21,48 +21,7 @@ odoo.define('point_of_sale.ClosePosPopup', function(require) { this.state = useState({ displayMoneyDetailsPopup: false, }); - onWillStart(this.onWillStart); - } - async onWillStart() { - try { - const closingData = await this.rpc({ - model: 'pos.session', - method: 'get_closing_control_data', - args: [[this.env.pos.pos_session.id]] - }); - this.ordersDetails = closingData.orders_details; - this.paymentsAmount = closingData.payments_amount; - this.payLaterAmount = closingData.pay_later_amount; - this.openingNotes = closingData.opening_notes; - this.defaultCashDetails = closingData.default_cash_details; - this.otherPaymentMethods = closingData.other_payment_methods; - this.isManager = closingData.is_manager; - this.amountAuthorizedDiff = closingData.amount_authorized_diff; - - // component state and refs definition - const state = {notes: '', payments: {}}; - if (this.cashControl) { - state.payments[this.defaultCashDetails.id] = {counted: 0, difference: -this.defaultCashDetails.amount, number: 0}; - } - if (this.otherPaymentMethods.length > 0) { - this.otherPaymentMethods.forEach(pm => { - if (pm.type === 'bank') { - state.payments[pm.id] = {counted: this.env.pos.round_decimals_currency(pm.amount), difference: 0, number: pm.number} - } - }) - } - Object.assign(this.state, state); - } catch (error) { - super.cancel(); - if (identifyError(error) instanceof ConnectionLostError) { - this.showPopup('ErrorPopup', { - title: this.env._t('Network Error'), - body: this.env._t('Please check your internet connection and try again.'), - }); - } else { - throw error; - } - } + Object.assign(this, this.props.info); } //@override async confirm() { diff --git a/addons/point_of_sale/static/src/js/Popups/ProductInfoPopup.js b/addons/point_of_sale/static/src/js/Popups/ProductInfoPopup.js index ceaadff49b3..76abf73eaaa 100644 --- a/addons/point_of_sale/static/src/js/Popups/ProductInfoPopup.js +++ b/addons/point_of_sale/static/src/js/Popups/ProductInfoPopup.js @@ -3,8 +3,6 @@ odoo.define('point_of_sale.ProductInfoPopup', function(require) { const AbstractAwaitablePopup = require('point_of_sale.AbstractAwaitablePopup'); const Registries = require('point_of_sale.Registries'); - const { identifyError } = require('point_of_sale.utils'); - const { ConnectionLostError } = require('@web/core/network/rpc_service') /** * This popup needs to be self-dependent because it needs to be called from different place. In order to avoid code @@ -17,47 +15,7 @@ odoo.define('point_of_sale.ProductInfoPopup', function(require) { class ProductInfoPopup extends AbstractAwaitablePopup { setup() { super.setup(); - owl.onWillStart(this.onWillStart); - } - async onWillStart() { - const order = this.env.pos.get_order(); - try { - // check back-end method `get_product_info_pos` to see what it returns - // We do this so it's easier to override the value returned and use it in the component template later - this.productInfo = await this.rpc({ - model: 'product.product', - method: 'get_product_info_pos', - args: [[this.props.product.id], - this.props.product.get_price(order.pricelist, this.props.quantity), - this.props.quantity, - this.env.pos.config.id], - kwargs: {context: this.env.session.user_context}, - }); - - const priceWithoutTax = this.productInfo['all_prices']['price_without_tax']; - const margin = priceWithoutTax - this.props.product.standard_price; - const orderPriceWithoutTax = order.get_total_without_tax(); - const orderCost = order.get_total_cost(); - const orderMargin = orderPriceWithoutTax - orderCost; - - this.costCurrency = this.env.pos.format_currency(this.props.product.standard_price); - this.marginCurrency = this.env.pos.format_currency(margin); - this.marginPercent = priceWithoutTax ? Math.round(margin/priceWithoutTax * 10000) / 100 : 0; - this.orderPriceWithoutTaxCurrency = this.env.pos.format_currency(orderPriceWithoutTax); - this.orderCostCurrency = this.env.pos.format_currency(orderCost); - this.orderMarginCurrency = this.env.pos.format_currency(orderMargin); - this.orderMarginPercent = orderPriceWithoutTax ? Math.round(orderMargin/orderPriceWithoutTax * 10000) / 100 : 0; - } catch (error) { - this.cancel(); - if (identifyError(error) instanceof ConnectionLostError) { - this.showPopup('ErrorPopup', { - title: this.env._t('Network Error'), - body: this.env._t('Cannot access product information screen if offline.'), - }); - } else { - throw error; - } - } + Object.assign(this, this.props.info); } searchProduct(productName) { this.env.posbus.trigger('search-product-from-info-popup', productName); diff --git a/addons/point_of_sale/static/src/js/Screens/ProductScreen/ControlButtons/ProductInfoButton.js b/addons/point_of_sale/static/src/js/Screens/ProductScreen/ControlButtons/ProductInfoButton.js index d34f576bfb7..d79753df21b 100644 --- a/addons/point_of_sale/static/src/js/Screens/ProductScreen/ControlButtons/ProductInfoButton.js +++ b/addons/point_of_sale/static/src/js/Screens/ProductScreen/ControlButtons/ProductInfoButton.js @@ -11,12 +11,13 @@ odoo.define('point_of_sale.ProductInfoButton', function(require) { super.setup(); useListener('click', this.onClick); } - onClick() { + async onClick() { const orderline = this.env.pos.get_order().get_selected_orderline(); if (orderline) { const product = orderline.get_product(); const quantity = orderline.get_quantity(); - this.showPopup('ProductInfoPopup', { product, quantity }); + const info = await this.env.pos.getProductInfo(product, quantity); + this.showPopup('ProductInfoPopup', { info: info , product: product }); } } } diff --git a/addons/point_of_sale/static/src/js/Screens/ProductScreen/ProductItem.js b/addons/point_of_sale/static/src/js/Screens/ProductScreen/ProductItem.js index 37ff4c0e930..afb658c24c7 100644 --- a/addons/point_of_sale/static/src/js/Screens/ProductScreen/ProductItem.js +++ b/addons/point_of_sale/static/src/js/Screens/ProductScreen/ProductItem.js @@ -40,8 +40,9 @@ odoo.define('point_of_sale.ProductItem', function(require) { return formattedUnitPrice; } } - onProductInfoClick() { - this.showPopup('ProductInfoPopup', { product: this.props.product, quantity: 1 }); + async onProductInfoClick() { + const info = await this.env.pos.getProductInfo(this.props.product, 1); + this.showPopup('ProductInfoPopup', { info: info , product: this.props.product }); } } ProductItem.template = 'ProductItem'; diff --git a/addons/point_of_sale/static/src/js/models.js b/addons/point_of_sale/static/src/js/models.js index 12a0ae365d4..65a722e0aef 100644 --- a/addons/point_of_sale/static/src/js/models.js +++ b/addons/point_of_sale/static/src/js/models.js @@ -545,6 +545,79 @@ class PosGlobalState extends PosModel { i += 1; } while(partners.length); } + async getProductInfo(product, quantity) { + const order = this.get_order(); + try { + // check back-end method `get_product_info_pos` to see what it returns + // We do this so it's easier to override the value returned and use it in the component template later + const productInfo = await this.env.services.rpc({ + model: 'product.product', + method: 'get_product_info_pos', + args: [[product.id], + product.get_price(order.pricelist, quantity), + quantity, + this.config.id], + kwargs: {context: this.env.session.user_context}, + }); + + const priceWithoutTax = productInfo['all_prices']['price_without_tax']; + const margin = priceWithoutTax - product.standard_price; + const orderPriceWithoutTax = order.get_total_without_tax(); + const orderCost = order.get_total_cost(); + const orderMargin = orderPriceWithoutTax - orderCost; + + const costCurrency = this.format_currency(product.standard_price); + const marginCurrency = this.format_currency(margin); + const marginPercent = priceWithoutTax ? Math.round(margin/priceWithoutTax * 10000) / 100 : 0; + const orderPriceWithoutTaxCurrency = this.format_currency(orderPriceWithoutTax); + const orderCostCurrency = this.format_currency(orderCost); + const orderMarginCurrency = this.format_currency(orderMargin); + const orderMarginPercent = orderPriceWithoutTax ? Math.round(orderMargin/orderPriceWithoutTax * 10000) / 100 : 0; + return { + costCurrency, marginCurrency, marginPercent, orderPriceWithoutTaxCurrency, + orderCostCurrency, orderMarginCurrency, orderMarginPercent,productInfo + } + } catch (error) { + return { error } + } + } + async getClosePosInfo() { + try { + const closingData = await this.env.services.rpc({ + model: 'pos.session', + method: 'get_closing_control_data', + args: [[this.pos_session.id]] + }); + const ordersDetails = closingData.orders_details; + const paymentsAmount = closingData.payments_amount; + const payLaterAmount = closingData.pay_later_amount; + const openingNotes = closingData.opening_notes; + const defaultCashDetails = closingData.default_cash_details; + const otherPaymentMethods = closingData.other_payment_methods; + const isManager = closingData.is_manager; + const amountAuthorizedDiff = closingData.amount_authorized_diff; + const cashControl = this.config.cash_control; + + // component state and refs definition + const state = {notes: '', payments: {}}; + if (cashControl) { + state.payments[defaultCashDetails.id] = {counted: 0, difference: -defaultCashDetails.amount, number: 0}; + } + if (otherPaymentMethods.length > 0) { + otherPaymentMethods.forEach(pm => { + if (pm.type === 'bank') { + state.payments[pm.id] = {counted: this.round_decimals_currency(pm.amount), difference: 0, number: pm.number} + } + }) + } + return { + ordersDetails, paymentsAmount, payLaterAmount, openingNotes, defaultCashDetails, otherPaymentMethods, + isManager, amountAuthorizedDiff, state, cashControl + } + } catch (error) { + return { error } + } + } set_start_order(){ if (this.orders.length && !this.selectedOrder) { this.selectedOrder = this.orders[0] diff --git a/addons/pos_sale_product_configurator/static/src/js/models.js b/addons/pos_sale_product_configurator/static/src/js/models.js index 68fef964602..5c84ea98855 100644 --- a/addons/pos_sale_product_configurator/static/src/js/models.js +++ b/addons/pos_sale_product_configurator/static/src/js/models.js @@ -21,7 +21,9 @@ odoo.define('pos_sale_product_configurator.models', function (require) { } ); if (isProductLoaded) { - Gui.showPopup('ProductInfoPopup', {product, quantity: this.get_selected_orderline().get_quantity()}); + const quantity = this.get_selected_orderline().get_quantity(); + const info = await this.env.pos.getProductInfo(product, quantity); + Gui.showPopup('ProductInfoPopup', {info: info , product: product}); } } }