From ea50cb3806a1cd336e67928814479bcc97b0b9d7 Mon Sep 17 00:00:00 2001 From: Martin Trigaux Date: Mon, 7 Mar 2016 13:57:27 +0100 Subject: [PATCH] [IMP] website_sale: clean cart update methods * _cart_find_product_line The method _cart_find_product_line is always called with only one record. The fact that the return statement was inside the loop confirms the for loop is not useful. The method will now return recordsets instead of ids. Convert the overrides to the new API for cleaner code and avoid old-new api headhache. Side note: the website_sale_option override logic is very counter intuitive and should probably be refactored. * _cart_update Add ensure_one to be sure working on only one sale.order. The method is always called on only one order and does not make sense to be used on multiple (is the action that is done on one line of the cart). The return format is the updated line without link to multiple orders. Remove the for loop and assume self is only one order. --- .../website_event_sale/models/sale_order.py | 19 +++--- addons/website_sale/models/sale_order.py | 63 +++++++++---------- .../website_sale_options/models/sale_order.py | 30 ++++----- 3 files changed, 57 insertions(+), 55 deletions(-) diff --git a/addons/website_event_sale/models/sale_order.py b/addons/website_event_sale/models/sale_order.py index facd88bb351..39875af682e 100644 --- a/addons/website_event_sale/models/sale_order.py +++ b/addons/website_event_sale/models/sale_order.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -from openerp import SUPERUSER_ID +from openerp import SUPERUSER_ID, api from openerp.osv import osv from openerp.tools.translate import _ from openerp.exceptions import UserError @@ -8,15 +8,16 @@ from openerp.exceptions import UserError class sale_order(osv.Model): _inherit = "sale.order" - def _cart_find_product_line(self, cr, uid, ids, product_id=None, line_id=None, context=None, **kwargs): - line_ids = super(sale_order, self)._cart_find_product_line(cr, uid, ids, product_id, line_id, context=context) + @api.multi + def _cart_find_product_line(self, product_id=None, line_id=None, **kwargs): + self.ensure_one() + lines = super(sale_order, self)._cart_find_product_line(product_id, line_id) if line_id: - return line_ids - for so in self.browse(cr, uid, ids, context=context): - domain = [('id', 'in', line_ids)] - if context.get("event_ticket_id"): - domain += [('event_ticket_id', '=', context.get("event_ticket_id"))] - return self.pool.get('sale.order.line').search(cr, SUPERUSER_ID, domain, context=context) + return lines + + if self.env.context.get("event_ticket_id"): + lines = lines.filtered(lambda line: line.event_ticket_id.id == self.env.context["event_ticket_id"]) + return lines def _website_product_id_change(self, cr, uid, ids, order_id, product_id, qty=0, context=None): values = super(sale_order, self)._website_product_id_change(cr, uid, ids, order_id, product_id, qty=qty, context=None) diff --git a/addons/website_sale/models/sale_order.py b/addons/website_sale/models/sale_order.py index 31319d3d0cf..3294f802bd1 100644 --- a/addons/website_sale/models/sale_order.py +++ b/addons/website_sale/models/sale_order.py @@ -41,11 +41,11 @@ class SaleOrder(models.Model): @api.multi def _cart_find_product_line(self, product_id=None, line_id=None, **kwargs): - for so in self: - domain = [('order_id', '=', so.id), ('product_id', '=', product_id)] - if line_id: - domain += [('id', '=', line_id)] - return self.env['sale.order.line'].sudo().search(domain).ids + self.ensure_one() + domain = [('order_id', '=', self.id), ('product_id', '=', product_id)] + if line_id: + domain += [('id', '=', line_id)] + return self.env['sale.order.line'].sudo().search(domain) @api.multi def _website_product_id_change(self, order_id, product_id, qty=0): @@ -75,39 +75,38 @@ class SaleOrder(models.Model): @api.multi def _cart_update(self, product_id=None, line_id=None, add_qty=0, set_qty=0, **kwargs): """ Add or set product quantity, add_qty can be negative """ + self.ensure_one() SaleOrderLineSudo = self.env['sale.order.line'].sudo() quantity = 0 order_line = False - for so in self: - if so.state != 'draft': - request.session['sale_order_id'] = None - raise UserError(_('It is forbidden to modify a sale order which is not in draft status')) - if line_id is not False: - line_ids = so._cart_find_product_line(product_id, line_id, **kwargs) - if line_ids: - order_line = SaleOrderLineSudo.browse(line_ids[0]) + if self.state != 'draft': + request.session['sale_order_id'] = None + raise UserError(_('It is forbidden to modify a sale order which is not in draft status')) + if line_id is not False: + order_lines = self._cart_find_product_line(product_id, line_id, **kwargs) + order_line = order_lines and order_lines[0] - # Create line if no line with product_id can be located - if not order_line: - values = self._website_product_id_change(so.id, product_id, qty=1) - order_line = SaleOrderLineSudo.create(values) - order_line._compute_tax_id() - if add_qty: - add_qty -= 1 + # Create line if no line with product_id can be located + if not order_line: + values = self._website_product_id_change(self.id, product_id, qty=1) + order_line = SaleOrderLineSudo.create(values) + order_line._compute_tax_id() + if add_qty: + add_qty -= 1 - # compute new quantity - if set_qty: - quantity = set_qty - elif add_qty is not None: - quantity = order_line.product_uom_qty + (add_qty or 0) + # compute new quantity + if set_qty: + quantity = set_qty + elif add_qty is not None: + quantity = order_line.product_uom_qty + (add_qty or 0) - # Remove zero of negative lines - if quantity <= 0: - order_line.unlink() - else: - # update line - values = self._website_product_id_change(so.id, product_id, qty=quantity) - order_line.write(values) + # Remove zero of negative lines + if quantity <= 0: + order_line.unlink() + else: + # update line + values = self._website_product_id_change(self.id, product_id, qty=quantity) + order_line.write(values) return {'line_id': order_line.id, 'quantity': quantity} diff --git a/addons/website_sale_options/models/sale_order.py b/addons/website_sale_options/models/sale_order.py index ece80065d47..4473f4a79aa 100644 --- a/addons/website_sale_options/models/sale_order.py +++ b/addons/website_sale_options/models/sale_order.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -from openerp import SUPERUSER_ID +from openerp import SUPERUSER_ID, api from openerp.osv import osv, orm, fields from openerp.tools.translate import _ @@ -15,20 +15,22 @@ class sale_order_line(osv.Model): class sale_order(osv.Model): _inherit = "sale.order" - def _cart_find_product_line(self, cr, uid, ids, product_id=None, line_id=None, context=None, **kwargs): - line_ids = super(sale_order, self)._cart_find_product_line(cr, uid, ids, product_id, line_id, context=context) + @api.multi + def _cart_find_product_line(self, product_id=None, line_id=None, **kwargs): + self.ensure_one() + lines = super(sale_order, self)._cart_find_product_line(product_id, line_id) if line_id: - return line_ids - linked_line_id = kwargs.get('linked_line_id') - optional_product_ids = kwargs.get('optional_product_ids') - for so in self.browse(cr, uid, ids, context=context): - domain = [('id', 'in', line_ids)] - domain += linked_line_id and [('linked_line_id', '=', linked_line_id)] or [('linked_line_id', '=', False)] - if optional_product_ids: - domain += [('option_line_ids.product_id', '=', pid) for pid in optional_product_ids] - else: - domain += [('option_line_ids', '=', False)] - return self.pool.get('sale.order.line').search(cr, SUPERUSER_ID, domain, context=context) + return lines + linked_line_id = kwargs.get('linked_line_id', False) + optional_product_ids = set(kwargs.get('optional_product_ids', [])) + + lines = lines.filtered(lambda line: line.linked_line_id.id == linked_line_id) + if optional_product_ids: + # only match the lines where where chosen subproducs is a subset of optional products chosen on the existing lines + lines = lines.filtered(lambda line: optional_product_ids.issubset(line.mapped('option_line_ids.product_id.id'))) + else: + lines = lines.filtered(lambda line: not line.option_line_ids) + return lines def _cart_update(self, cr, uid, ids, product_id=None, line_id=None, add_qty=0, set_qty=0, context=None, **kwargs): """ Add or set product quantity, add_qty can be negative """