[FIX] loyalty,pos_*: reward program loading issue
Previously, when a reward was applied on a product, the product was checked against the reward_product_ids field, which is a computed m2m field that contains all the IDS of the products on which this reward is available. In many cases, rewards are available on all products, causing the computation of the m2m to fill it with the ids of all products. This caused performance issues on all DBs with lots of products (~200k) in all flows involving rewards. We can't remove reward_product_ids from the data loaded in the frontend in stable because existing JS customisations might crash if they depend on its presence. As such, to keep compatibility with existing databases, an ir.config_parameter has been introduced to opt into the new behaviour. This parameter is set when creating a database so that new databases don't suffer from this performance penalty. For existing databases, the parameter can be set by hand if the old behaviour is not necessary and the performance penalty is an issue in practice, but is unset by default. When opting into the new behaviour, the reward_product_ids field now always evaluates to an empty recordset, and the desired behaviour should be achieved by evaluating records against the reward_product_domain instead. In the point_of_sale, the products available on rewards are calculated by evaluating each product against the reward when it is loaded. In the loyalty modules, instead of using the in operator on the reward_product_ids field, we instead evaluate the reward product domain against the product, which is much faster. This is always done even when not opting into the new behaviour as the change in implementation cannot be observed outside of timing. closes odoo/odoo#120074 X-original-commit: 6f72d053a31aca520cbfbba2d1257568b7f7c0cb Related: odoo/enterprise#40503 Signed-off-by: Joseph Caburnay (jcb) <jcb@odoo.com>
This commit is contained in:
@@ -15,4 +15,11 @@
|
||||
<field name="detailed_type">service</field>
|
||||
<field name="purchase_ok" eval="False"/>
|
||||
</record>
|
||||
|
||||
<data noupdate="1">
|
||||
<record forcecreate="0" id="config_online_sync_proxy_mode" model="ir.config_parameter">
|
||||
<field name="key">loyalty.compute_all_discount_product_ids</field>
|
||||
<field name="value">False</field>
|
||||
</record>
|
||||
</data>
|
||||
</odoo>
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
|
||||
import ast
|
||||
import json
|
||||
|
||||
from odoo import _, api, fields, models
|
||||
from odoo.osv import expression
|
||||
@@ -69,6 +70,7 @@ class LoyaltyReward(models.Model):
|
||||
discount_product_category_id = fields.Many2one('product.category', string="Discounted Prod. Categories")
|
||||
discount_product_tag_id = fields.Many2one('product.tag', string="Discounted Prod. Tag")
|
||||
all_discount_product_ids = fields.Many2many('product.product', compute='_compute_all_discount_product_ids')
|
||||
reward_product_domain = fields.Char(compute='_compute_reward_product_domain', store=False)
|
||||
discount_max_amount = fields.Monetary('Max Discount', 'currency_id',
|
||||
help="This is the max amount this reward may discount, leave to 0 for no limit.")
|
||||
discount_line_product_id = fields.Many2one('product.product', copy=False, ondelete='restrict',
|
||||
@@ -103,23 +105,45 @@ class LoyaltyReward(models.Model):
|
||||
for reward in self:
|
||||
reward.reward_product_uom_id = reward.reward_product_ids.product_tmpl_id.uom_id[:1]
|
||||
|
||||
def _find_all_category_children(self, category_id, child_ids):
|
||||
if len(category_id.child_id) > 0:
|
||||
for child_id in category_id.child_id:
|
||||
child_ids.append(child_id.id)
|
||||
self._find_all_category_children(child_id, child_ids)
|
||||
return child_ids
|
||||
|
||||
def _get_discount_product_domain(self):
|
||||
self.ensure_one()
|
||||
domain = []
|
||||
if self.discount_product_ids:
|
||||
domain = [('id', 'in', self.discount_product_ids.ids)]
|
||||
if self.discount_product_category_id:
|
||||
domain = expression.OR([domain, [('categ_id', 'child_of', self.discount_product_category_id.id)]])
|
||||
product_category_ids = self._find_all_category_children(self.discount_product_category_id, [])
|
||||
product_category_ids.append(self.discount_product_category_id.id)
|
||||
domain = expression.OR([domain, [('categ_id', 'in', product_category_ids)]])
|
||||
if self.discount_product_tag_id:
|
||||
domain = expression.OR([domain, [('all_product_tag_ids', 'in', self.discount_product_tag_id.id)]])
|
||||
if self.discount_product_domain and self.discount_product_domain != '[]':
|
||||
domain = expression.AND([domain, ast.literal_eval(self.discount_product_domain)])
|
||||
return domain
|
||||
|
||||
@api.depends('discount_product_domain')
|
||||
def _compute_reward_product_domain(self):
|
||||
compute_all_discount_product = self.env['ir.config_parameter'].get_param('loyalty.compute_all_discount_product_ids', 'enabled')
|
||||
for reward in self:
|
||||
if compute_all_discount_product == 'enabled':
|
||||
reward.reward_product_domain = "null"
|
||||
else:
|
||||
reward.reward_product_domain = json.dumps(reward._get_discount_product_domain())
|
||||
|
||||
@api.depends('discount_product_ids', 'discount_product_category_id', 'discount_product_tag_id', 'discount_product_domain')
|
||||
def _compute_all_discount_product_ids(self):
|
||||
compute_all_discount_product = self.env['ir.config_parameter'].get_param('loyalty.compute_all_discount_product_ids', 'enabled')
|
||||
for reward in self:
|
||||
reward.all_discount_product_ids = self.env['product.product'].search(reward._get_discount_product_domain())
|
||||
if compute_all_discount_product == 'enabled':
|
||||
reward.all_discount_product_ids = self.env['product.product'].search(reward._get_discount_product_domain())
|
||||
else:
|
||||
reward.all_discount_product_ids = self.env['product.product']
|
||||
|
||||
@api.depends('reward_product_id', 'reward_product_tag_id', 'reward_type')
|
||||
def _compute_multi_product(self):
|
||||
@@ -161,8 +185,9 @@ class LoyaltyReward(models.Model):
|
||||
elif reward.discount_applicability == 'cheapest':
|
||||
reward_string += _('the cheapest product')
|
||||
elif reward.discount_applicability == 'specific':
|
||||
if len(reward.all_discount_product_ids) == 1:
|
||||
reward_string += reward.all_discount_product_ids.name
|
||||
product_available = self.env['product.product'].search(reward._get_discount_product_domain(), limit=2)
|
||||
if len(product_available) == 1:
|
||||
reward_string += product_available.name
|
||||
else:
|
||||
reward_string += _('specific products')
|
||||
if reward.discount_max_amount:
|
||||
|
||||
@@ -43,7 +43,7 @@ class PosSession(models.Model):
|
||||
'fields': ['description', 'program_id', 'reward_type', 'required_points', 'clear_wallet', 'currency_id',
|
||||
'discount', 'discount_mode', 'discount_applicability', 'all_discount_product_ids', 'is_global_discount',
|
||||
'discount_max_amount', 'discount_line_product_id',
|
||||
'multi_product', 'reward_product_ids', 'reward_product_qty', 'reward_product_uom_id'],
|
||||
'multi_product', 'reward_product_ids', 'reward_product_qty', 'reward_product_uom_id', 'reward_product_domain'],
|
||||
}
|
||||
}
|
||||
|
||||
@@ -112,3 +112,9 @@ class PosSession(models.Model):
|
||||
product_id_to_program_ids[product['id']].append(program['id'])
|
||||
|
||||
loaded_data['product_id_to_program_ids'] = product_id_to_program_ids
|
||||
|
||||
def _loader_params_product_product(self):
|
||||
params = super()._loader_params_product_product()
|
||||
# this is usefull to evaluate reward domain in frontend
|
||||
params['search_params']['fields'].append('all_product_tag_ids')
|
||||
return params
|
||||
|
||||
@@ -7,7 +7,9 @@ import { roundDecimals, roundPrecision } from "@web/core/utils/numbers";
|
||||
import { _t } from "@web/core/l10n/translation";
|
||||
import { patch } from "@web/core/utils/patch";
|
||||
import { ConfirmPopup } from "@point_of_sale/js/Popups/ConfirmPopup";
|
||||
import { Domain, InvalidDomainError } from "@web/core/domain";
|
||||
import { sprintf } from "@web/core/utils/strings";
|
||||
import { ErrorPopup } from "@point_of_sale/js/Popups/ErrorPopup";
|
||||
|
||||
// FIXME: Perhaps MutexedDropPrevious can be replaced by the new KeepLast.
|
||||
// > This might require thorough investigation on how _updateRewards work.
|
||||
@@ -83,13 +85,61 @@ patch(PosGlobalState.prototype, "pos_loyalty.PosGlobalState", {
|
||||
async _processData(loadedData) {
|
||||
this.couponCache = {};
|
||||
this.partnerId2CouponIds = {};
|
||||
this.rewards = loadedData["loyalty.reward"] || [];
|
||||
|
||||
for (const reward of this.rewards) {
|
||||
reward.all_discount_product_ids = new Set(reward.all_discount_product_ids);
|
||||
}
|
||||
|
||||
await this._super(loadedData);
|
||||
this.productId2ProgramIds = loadedData["product_id_to_program_ids"];
|
||||
this.programs = loadedData["loyalty.program"] || []; //TODO: rename to `loyaltyPrograms` etc
|
||||
this.rules = loadedData["loyalty.rule"] || [];
|
||||
this.rewards = loadedData["loyalty.reward"] || [];
|
||||
this._loadLoyaltyData();
|
||||
},
|
||||
|
||||
_loadProductProduct(products) {
|
||||
this._super(...arguments);
|
||||
|
||||
for (const reward of this.rewards) {
|
||||
this.compute_discount_product_ids(reward, products);
|
||||
}
|
||||
|
||||
this.rewards = this.rewards.filter(Boolean);
|
||||
},
|
||||
|
||||
compute_discount_product_ids(reward, products) {
|
||||
const reward_product_domain = JSON.parse(reward.reward_product_domain);
|
||||
if (!reward_product_domain) {
|
||||
return;
|
||||
}
|
||||
|
||||
const domain = new Domain(reward_product_domain);
|
||||
|
||||
try {
|
||||
products
|
||||
.filter((product) => domain.contains(product))
|
||||
.forEach((product) => reward.all_discount_product_ids.add(product.id));
|
||||
} catch (error) {
|
||||
if (!(error instanceof InvalidDomainError)) {
|
||||
throw error;
|
||||
}
|
||||
const index = this.rewards.indexOf(reward);
|
||||
if (index != -1) {
|
||||
this.pos.env.services.popup.add(ErrorPopup, {
|
||||
title: _t("A reward could not be loaded"),
|
||||
body: sprintf(
|
||||
_t(
|
||||
'The reward "%s" contain an error in its domain, your domain must be compatible with the PoS client'
|
||||
),
|
||||
this.rewards[index].description
|
||||
),
|
||||
});
|
||||
this.rewards[index] = null;
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
_loadLoyaltyData() {
|
||||
this.program_by_id = {};
|
||||
this.reward_by_id = {};
|
||||
|
||||
@@ -200,7 +200,8 @@ class SaleOrder(models.Model):
|
||||
|
||||
discountable_lines = self.env['sale.order.line']
|
||||
for line in (self.order_line - self._get_no_effect_on_threshold_lines()):
|
||||
if not line.reward_id and line.product_id in reward.all_discount_product_ids:
|
||||
domain = reward._get_discount_product_domain()
|
||||
if not line.reward_id and line.product_id.filtered_domain(domain):
|
||||
discountable_lines |= line
|
||||
return discountable_lines
|
||||
|
||||
@@ -223,7 +224,8 @@ class SaleOrder(models.Model):
|
||||
if not line.product_uom_qty or not line.price_unit:
|
||||
continue
|
||||
remaining_amount_per_line[line] = line.price_total
|
||||
if not line.reward_id and line.product_id in reward.all_discount_product_ids:
|
||||
domain = reward._get_discount_product_domain()
|
||||
if not line.reward_id and line.product_id.filtered_domain(domain):
|
||||
lines_to_discount |= line
|
||||
elif line.reward_id.reward_type == 'discount':
|
||||
discount_lines[line.reward_identifier_code] |= line
|
||||
|
||||
@@ -200,6 +200,7 @@ class TestSaleCouponProgramNumbers(TestSaleCouponNumbersCommon):
|
||||
|
||||
def test_program_numbers_one_discount_line_per_tax(self):
|
||||
order = self.empty_order
|
||||
self.env['ir.config_parameter'].set_param('loyalty.compute_all_discount_product_ids', 'enabled')
|
||||
# Create taxes
|
||||
self.tax_15pc_excl = self.env['account.tax'].create({
|
||||
'name': "15% Tax excl",
|
||||
|
||||
Reference in New Issue
Block a user