From e227e7c3a24415f1d44cdbb055da4e6a22c10bb3 Mon Sep 17 00:00:00 2001 From: Joseph Caburnay Date: Mon, 18 Sep 2023 12:53:50 +0200 Subject: [PATCH] [IMP] point_of_sale: consistent price on the combo lines **ISSUE** - Given a combo product with 2 combo_ids (pos.combo). - Combo A - A1, A2 - Combo B - B1, B2 - When adding this combo product, it's possible that the user selects the combination A1 & B1. - And in another order, A2 & B1. - ISSUE: Between those two orders, prices of first line (component A) will be different. What we want is that the price of line corresponding to a component (pos.combo.line) is consistent between orders. **SOLUTION** This can be achieved by establishing a constant in each pos.combo.line. We choose the minimum lst_price of the components as this constant which we call as `base_price` field. The calculation will be practically the same as before, except unit price is now based on the `base_price` field: - Compute the original price - which will be based on the `base_price`. - Recall: `base_price` is the minimum price among the `combo_line_ids` of a pos.combo. - We then divide the original price to the target price (which is the price of the combo product) which we'll call the "prorateFactor". - The price of the components are computed by multiplying the "prorateFactor" to the `base_price`. **Rounding error:** Both the original procedure and the new strategy introduced in this PR suffers from rounding error. To prevent the rounding error, we compute the error and distribute it to the last line of the combo. **Combo (Extra) Price:** The `pos.combo.line.combo_price` is added on top of the prorated price. closes odoo/odoo#138352 Signed-off-by: Vlad Stroia (vlst) --- addons/point_of_sale/models/pos_combo.py | 11 +++++++ addons/point_of_sale/models/pos_session.py | 4 +-- .../static/src/app/store/models.js | 31 ++++++++++++++++--- .../static/src/app/store/pos_store.js | 2 +- .../static/tests/tours/PosComboTour.js | 10 +++--- addons/point_of_sale/views/pos_combo_view.xml | 1 + addons/point_of_sale/views/product_view.xml | 1 + .../tests/tours/SplitBillScreen.tour.js | 8 ++--- 8 files changed, 51 insertions(+), 17 deletions(-) diff --git a/addons/point_of_sale/models/pos_combo.py b/addons/point_of_sale/models/pos_combo.py index 116ec5b66e8..936ab90832e 100644 --- a/addons/point_of_sale/models/pos_combo.py +++ b/addons/point_of_sale/models/pos_combo.py @@ -23,6 +23,11 @@ class PosCombo(models.Model): combo_line_ids = fields.One2many("pos.combo.line", "combo_id", string="Products in Combo", copy=True) num_of_products = fields.Integer("No of Products", compute="_compute_num_of_products") sequence = fields.Integer(copy=False) + base_price = fields.Float( + compute="_compute_base_price", + string="Product Price", + help="The value from which pro-rating of the component price is based. This is to ensure that whatever product the user chooses for a component, it will always be they same price." + ) @api.depends("combo_line_ids") def _compute_num_of_products(self): @@ -44,3 +49,9 @@ class PosCombo(models.Model): def _check_combo_line_ids_is_not_null(self): if any(not rec.combo_line_ids for rec in self): raise ValidationError(_("Please add products in combo.")) + + @api.depends("combo_line_ids") + def _compute_base_price(self): + for rec in self: + # Use the lowest price of the combo lines as the base price + rec.base_price = min(rec.combo_line_ids.mapped("product_id.lst_price")) diff --git a/addons/point_of_sale/models/pos_session.py b/addons/point_of_sale/models/pos_session.py index 596d8fd599a..b17e6ebccdd 100644 --- a/addons/point_of_sale/models/pos_session.py +++ b/addons/point_of_sale/models/pos_session.py @@ -1990,7 +1990,7 @@ class PosSession(models.Model): def _loader_params_pos_combo(self): products = self._context.get('loaded_data')['product.product'] combo_ids = set().union(*[product.get('combo_ids') for product in products]) - return {'search_params': {'fields': ['id', 'name', 'combo_line_ids']}, 'ids': combo_ids} + return {'search_params': {'fields': ['id', 'name', 'combo_line_ids', 'base_price']}, 'ids': combo_ids} def _get_pos_ui_pos_combo(self, params): return self.env['pos.combo'].browse(params['ids']).read(**params['search_params']) @@ -1998,7 +1998,7 @@ class PosSession(models.Model): def _loader_params_pos_combo_line(self): combo_ids = self._context.get('loaded_data')['pos.combo'] combo_line_ids = set().union(*[combo.get('combo_line_ids') for combo in combo_ids]) - return {'search_params': {'fields': ['id', 'product_id', 'combo_price']}, 'ids': combo_line_ids} + return {'search_params': {'fields': ['id', 'product_id', 'combo_price', 'combo_id']}, 'ids': combo_line_ids} def _get_pos_ui_pos_combo_line(self, params): return self.env['pos.combo.line'].browse(params['ids']).read(**params['search_params']) diff --git a/addons/point_of_sale/static/src/app/store/models.js b/addons/point_of_sale/static/src/app/store/models.js index b58004dd6cd..ca08c4a561c 100644 --- a/addons/point_of_sale/static/src/app/store/models.js +++ b/addons/point_of_sale/static/src/app/store/models.js @@ -2138,21 +2138,19 @@ export class Order extends PosModel { } } async addComboLines(comboParent, options) { - const pricelist = this.pos.getDefaultPricelist(); const originalPrices = {}; - let [originalTotal, targetExtra] = [0, 0]; + let originalTotal = 0; for (const comboLine of options.comboLines) { const product = this.pos.db.product_by_id[comboLine.product_id[0]]; - const originalPrice = product.get_price(pricelist, 1, comboLine.combo_price); + const originalPrice = this.pos.db.combo_by_id[comboLine.combo_id[0]].base_price; originalTotal += product.get_display_price({ price: originalPrice }); - targetExtra += product.get_display_price({ price: comboLine.combo_price }); // Keep track of the original price of each product for the subsequent for loop. originalPrices[product.id] = originalPrice; } - const targetPrice = comboParent.product.lst_price + targetExtra; + const targetPrice = comboParent.product.lst_price; const childPriceFactor = targetPrice / originalTotal; for (const comboLine of options.comboLines) { const product = this.pos.db.product_by_id[comboLine.product_id[0]]; @@ -2162,6 +2160,29 @@ export class Order extends PosModel { comboParent, }); } + + // Adjust the price of the last combo line to make sure the total price of the combo + // is the same as the target price. + const childLines = this.get_orderlines().filter( + (l) => l.comboParent?.uuid === comboParent.uuid + ); + const totalComboPrice = childLines.reduce((acc, l) => acc + l.get_display_price(), 0); + const diff = targetPrice - totalComboPrice; + if (!this.env.utils.floatIsZero(diff)) { + const lastLine = childLines[childLines.length - 1]; + lastLine.set_unit_price(lastLine.get_unit_price() + diff); + } + + // Finally, take into account the combo price for each combo line. + for (const comboLine of options.comboLines) { + if (this.env.utils.floatIsZero(comboLine.combo_price)) { + continue; + } + const presentLine = childLines.find((l) => l.product.id === comboLine.product_id[0]); + if (presentLine) { + presentLine.set_unit_price(presentLine.get_unit_price() + comboLine.combo_price); + } + } } set_orderline_options(orderline, options) { if (options.comboLines?.length) { diff --git a/addons/point_of_sale/static/src/app/store/pos_store.js b/addons/point_of_sale/static/src/app/store/pos_store.js index 73e7909c669..81e69f8d990 100644 --- a/addons/point_of_sale/static/src/app/store/pos_store.js +++ b/addons/point_of_sale/static/src/app/store/pos_store.js @@ -1797,7 +1797,7 @@ export class PosStore extends Reactive { } // FIXME: POSREF, method exist only to be overrided async addProductFromUi(product, options) { - this.get_order().add_product(product, options); + return this.get_order().add_product(product, options); } async addProductToCurrentOrder(product, options = {}) { if (Number.isInteger(product)) { diff --git a/addons/point_of_sale/static/tests/tours/PosComboTour.js b/addons/point_of_sale/static/tests/tours/PosComboTour.js index 1fc028d7141..74bd7f8351b 100644 --- a/addons/point_of_sale/static/tests/tours/PosComboTour.js +++ b/addons/point_of_sale/static/tests/tours/PosComboTour.js @@ -43,21 +43,21 @@ registry.category("web_tour.tours").add("PosComboPriceTaxIncludedTour", { ...ProductScreen.check.selectedOrderlineHas( "Combo Product 3", "1.0", - "14.63", + "12.92", "Office Combo" ), ...ProductScreen.do.clickOrderline("Combo Product 5"), ...ProductScreen.check.selectedOrderlineHas( "Combo Product 5", "1.0", - "16.87", + "17.87", "Office Combo" ), ...ProductScreen.do.clickOrderline("Combo Product 8"), ...ProductScreen.check.selectedOrderlineHas( "Combo Product 8", "1.0", - "28.11", + "28.81", "Office Combo" ), @@ -74,7 +74,7 @@ registry.category("web_tour.tours").add("PosComboPriceTaxIncludedTour", { combo.select("Combo Product 5"), combo.select("Combo Product 8"), combo.confirm(), - ...ProductScreen.check.totalAmountIs("59.61"), + ...ProductScreen.check.totalAmountIs("59.60"), ...ProductScreen.do.clickPayButton(), ...PaymentScreen.do.clickPaymentMethod("Bank"), ...PaymentScreen.do.clickValidate(), @@ -88,7 +88,7 @@ registry.category("web_tour.tours").add("PosComboPriceTaxIncludedTour", { combo.select("Combo Product 6"), combo.confirm(), ...ProductScreen.check.totalAmountIs("50.00"), - ...ProductScreen.check.totalTaxIs("8.91"), + ...ProductScreen.check.totalTaxIs("8.92"), // the split screen is tested in `pos_restaurant` ], diff --git a/addons/point_of_sale/views/pos_combo_view.xml b/addons/point_of_sale/views/pos_combo_view.xml index efe96fa3f1e..4a1dd55e840 100644 --- a/addons/point_of_sale/views/pos_combo_view.xml +++ b/addons/point_of_sale/views/pos_combo_view.xml @@ -28,6 +28,7 @@ + diff --git a/addons/point_of_sale/views/product_view.xml b/addons/point_of_sale/views/product_view.xml index c8b903db57e..b2b492ca5d6 100644 --- a/addons/point_of_sale/views/product_view.xml +++ b/addons/point_of_sale/views/product_view.xml @@ -80,6 +80,7 @@ + diff --git a/addons/pos_restaurant/static/tests/tours/SplitBillScreen.tour.js b/addons/pos_restaurant/static/tests/tours/SplitBillScreen.tour.js index 55c8c807f85..d153f3b835d 100644 --- a/addons/pos_restaurant/static/tests/tours/SplitBillScreen.tour.js +++ b/addons/pos_restaurant/static/tests/tours/SplitBillScreen.tour.js @@ -177,7 +177,7 @@ registry.category("web_tour.tours").add("SplitBillScreenTour4PosCombo", { ...SplitBillScreen.check.orderlineHas("Combo Product 4", "1", "0"), ...SplitBillScreen.check.orderlineHas("Combo Product 7", "1", "0"), - ...SplitBillScreen.check.subtotalIs("52.14"), + ...SplitBillScreen.check.subtotalIs("52.13"), ...SplitBillScreen.do.clickPay(), ...PaymentScreen.do.clickPaymentMethod("Bank"), ...PaymentScreen.do.clickValidate(), @@ -191,21 +191,21 @@ registry.category("web_tour.tours").add("SplitBillScreenTour4PosCombo", { ...ProductScreen.check.selectedOrderlineHas( "Combo Product 2", "1.0", - "6.45", + "6.15", "Office Combo" ), ...ProductScreen.do.clickOrderline("Combo Product 4"), ...ProductScreen.check.selectedOrderlineHas( "Combo Product 4", "1.0", - "12.90", + "13.54", "Office Combo" ), ...ProductScreen.do.clickOrderline("Combo Product 7"), ...ProductScreen.check.selectedOrderlineHas( "Combo Product 7", "1.0", - "20.65", + "20.31", "Office Combo" ), ...ProductScreen.check.totalAmountIs("42.53"),