From 74ebc34fea162b680081164867348d4085943c1c Mon Sep 17 00:00:00 2001 From: Yannick Tivisse Date: Wed, 1 Feb 2017 17:25:42 +0100 Subject: [PATCH] [FIX] sale_coupon: Trigger correctly the reward recomputation methods Use Case ======== In the coupon module, a call to a write, a create or an unlink on a sales order line may trigger a write, a create or an unlink on another line. Example: I have a promotional offer "If you buy for more than 100 euros, you got a 10 percent discount on the sales order. On my sales order, I buy 2 imac at 999 euros. At that moment my total amount is over 1000 euros and I got another line, the discount. I write on the sales order line that I only buy one imac, a unlink is trigger on the reward line to remove it from the sales order. Current Solution ================ At the beginning, I was overwriting the write, the create and the unlink methods on the sales order line to perform the correct actions according to the promotional offers that were still applicable, not applicable anymore of newly applicable. This solution was followed by side effects on the computed fields. For example, a write on the SO line that was triggering an unlink on another line was triggering a computed field that depended on the sales order line. In that case, the environment I received on the compute method was the environment before the line unlink. As the compute method was looping on the sales order line, it was taking values on a line that wasn't existing anymore and that way it was triggering an 'This record doesn't exist or has been deleted' error. After a discussion with @rco-odoo, we realized that the only way to achieve this was to override the write method on a sales order to avoid quadratic computation loop when creating/editing/deleting sale order lines. As coupon are recorded as sale.order.line, it may effectively blow up. We also overrode the create method on a sales order line to make an ORM command on the sales order instead of a simple create on the sales order. See: https://github.com/odoo/enterprise/blob/saas-14/sale_coupon/models/sale_order.py#L253 For the write and the unlink, this is a little more complicated because a write may trigger a write and an unlink may trigger an unlink, resulting in an infinite loop. New Solution ============ Having a method _recompute_coupon_lines on sale.order. This method do nothing if there is a key coupon_recompute in the context. ```python @multi def _recompute_coupon_lines(self): if self.env.context.get('coupon_recompute'): return self = self.with_context(coupon_recompute=True) # avoid loop [...do actual stuff...] ``` The create/write/unlink method on sale.order.line call super before calling the method _recompute_coupon_lines ```python @multi def write(self, values): result = super(SaleOrderLine, self).write(values) self.mapped('order_id')._recompute_coupon_lines() return result ``` In order (pun intended) to accelerate mass creation/edition of lines, the method create/write/unlink of sale.order should call recompute themself ```python @mutli def write(self, values): result = super(SaleOrder, self.with_context(coupon_recompute=True)).write(values) self._recompute_coupon_lines() return result ``` --- models/sale_order.py | 54 ++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/models/sale_order.py b/models/sale_order.py index 37a4a3fcf29..209bc7ecffc 100644 --- a/models/sale_order.py +++ b/models/sale_order.py @@ -21,6 +21,15 @@ class SaleOrder(models.Model): for order in self: order.reward_amount = sum([line.price_subtotal for line in order.order_line.filtered(lambda line: line.is_reward_line)]) + @api.multi + def _recompute_coupon_lines(self): + if self.env.context.get('coupon_recompute', False): + return + for order in self.with_context(coupon_recompute=True): + order._remove_invalid_reward_lines() + order._create_new_no_code_promo_reward_lines() + order._update_existing_reward_lines() + def copy(self, default=None): order = super(SaleOrder, self).copy(dict(default or {}, order_line=False)) for line in self.order_line.filtered(lambda line: not line.is_reward_line): @@ -30,18 +39,14 @@ class SaleOrder(models.Model): @api.model def create(self, vals): - res = super(SaleOrder, self).create(vals) - res._create_new_no_code_promo_reward_lines() - res._update_existing_reward_lines() + res = super(SaleOrder, self.with_context(coupon_recompute=True)).create(vals) + res = res.with_context(coupon_recompute=False) + res._recompute_coupon_lines() return res def write(self, vals): - res = super(SaleOrder, self).write(vals) - if 'order_line' in vals and self.env.context.get('sale_coupon_no_loop', True): - for order in self.with_context(sale_coupon_no_loop=False): - order._remove_invalid_reward_lines() - order._create_new_no_code_promo_reward_lines() - order._update_existing_reward_lines() + res = super(SaleOrder, self.with_context(coupon_recompute=True)).write(vals) + self._recompute_coupon_lines() return res def action_confirm(self): @@ -251,25 +256,14 @@ class SaleOrderLine(models.Model): @api.model def create(self, vals): - """ - Override the sale order line creation to transform the line creation into a ORM request - This horrible hack is due to the fact that with the coupons mechanism, a line creation may - lead to the creation, the modification or the deletion of another line. If we override the - create, write and unlink methods on the sale.order.line model, we may have issues like - cache mismatch on computed fields and so on. Due to this reason, we override the write - method on the sale.order and do the actions according to the situation. A write is done on - the sales order if we create, write or unlink a sales order line with an ORM command. To avoid - having places where creating a sales order doens't activate the coupon mechanism, (It is the - case in website_sale or website_quote for example) we do this hack here where we create the line - on the order with an ORM command and return the newly created order line. I hope that you'll - find in your heart enough faith to forgive me. - PS: RCO told me it was ok - """ - order = self.env['sale.order'].browse(vals['order_id']) - if not self.env.context.get('sale_coupon_no_loop', False): - order.with_context(sale_coupon_no_loop=True).write({'order_line': [(0, False, vals)]}) - return order.order_line.sorted('create_date', reverse=True)[0] - return super(SaleOrderLine, self).create(vals) + res = super(SaleOrderLine, self).create(vals) + self.order_id._recompute_coupon_lines() + return res + + def write(self, vals): + res = super(SaleOrderLine, self).write(vals) + self.mapped('order_id')._recompute_coupon_lines() + return res def unlink(self): # Reactivate coupons related to unlinked reward line @@ -284,5 +278,7 @@ class SaleOrderLine(models.Model): if related_program: self.order_id.no_code_promo_program_ids -= related_program self.order_id.code_promo_program_id -= related_program - res = super(SaleOrderLine, self.with_context(sale_coupon_no_loop=False)).unlink() + orders = self.mapped('order_id') + res = super(SaleOrderLine, self).unlink() + orders._recompute_coupon_lines() return res