[IMP] delivery,website_sale_delivery: refactoring and cleaning

addons/delivery/data/delivery_data.xml

@@ -5,7 +5,7 @@
         <record id="free_delivery_carrier" model="delivery.carrier">
             <field name="name">Free delivery charges</field>
             <field name="fixed_price">0</field>
-            <field name="free_if_more_than" eval="True"/>
+            <field name="free_over" eval="True"/>
             <field name="amount">1000</field>
             <field name="sequence">1</field>
             <field name="delivery_type">fixed</field>

addons/delivery/models/__init__.py

@@ -2,7 +2,7 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from . import delivery_carrier
-from . import delivery_price_rule
+from . import delivery_grid
 from . import product_template
 from . import product_packaging
 from . import sale_order

addons/delivery/models/delivery_carrier.py

@@ -4,8 +4,6 @@
 import logging
 
 from odoo import api, fields, models, _
-from odoo.exceptions import UserError, ValidationError
-from odoo.tools.safe_eval import safe_eval
 
 _logger = logging.getLogger(__name__)
 
@@ -24,9 +22,9 @@ class DeliveryCarrier(models.Model):
     2. Extend the selection of the field "delivery_type" with a pair
        ('<my_provider>', 'My Provider')
     3. Add your methods:
-       <my_provider>_get_shipping_price_from_so
+       <my_provider>_rate_shipment
        <my_provider>_send_shipping
-       <my_provider>_open_tracking_page
+       <my_provider>_get_tracking_link
        <my_provider>_cancel_shipment
        (they are documented hereunder)
     '''
@@ -37,29 +35,30 @@ class DeliveryCarrier(models.Model):
 
     sequence = fields.Integer(help="Determine the display order", default=10)
     # This field will be overwritten by internal shipping providers by adding their own type (ex: 'fedex')
-    delivery_type = fields.Selection([('fixed', 'Fixed Price'), ('base_on_rule', 'Based on Rules')], string='Provider', default='fixed', required=True)
+    delivery_type = fields.Selection([('fixed', 'Fixed Price')], string='Provider', default='fixed', required=True)
+    integration_level = fields.Selection([('rate', 'Get Rate'), ('rate_and_ship', 'Get Rate and Create Shipment')], string="Integration Level", default='rate_and_ship', help="Action while validating Delivery Orders")
+    prod_environment = fields.Boolean("Environment", help="Set to True if your credentials are certified for production.")
+
     product_type = fields.Selection(related='product_id.type', default='service')
     product_sale_ok = fields.Boolean(related='product_id.sale_ok', default=False)
     product_id = fields.Many2one('product.product', string='Delivery Product', required=True, ondelete="cascade")
-    free_if_more_than = fields.Boolean('Free if Order total is more than', help="If the order is more expensive than a certain amount, the customer can benefit from a free shipping", default=False)
-    amount = fields.Float(string='Amount', help="Amount of the order to benefit from a free shipping, expressed in the company currency")
+
     country_ids = fields.Many2many('res.country', 'delivery_carrier_country_rel', 'carrier_id', 'country_id', 'Countries')
     state_ids = fields.Many2many('res.country.state', 'delivery_carrier_state_rel', 'carrier_id', 'state_id', 'States')
     zip_from = fields.Char('Zip From')
     zip_to = fields.Char('Zip To')
-    price_rule_ids = fields.One2many('delivery.price.rule', 'carrier_id', 'Pricing Rules', copy=True)
-    fixed_price = fields.Float(compute='_compute_fixed_price', inverse='_set_product_fixed_price', store=True, string='Fixed Price',help="Keep empty if the pricing depends on the advanced pricing per destination")
-    integration_level = fields.Selection([('rate', 'Get Rate'), ('rate_and_ship', 'Get Rate and Create Shipment')], string="Integration Level", default='rate_and_ship', help="Action while validating Delivery Orders")
-    prod_environment = fields.Boolean("Environment", help="Set to True if your credentials are certified for production.")
+
     margin = fields.Integer(help='This percentage will be added to the shipping price.')
+    free_over = fields.Boolean('Free if Order total is more than', help="If the order is more expensive than a certain amount, the customer can benefit from a free shipping", default=False, oldname='free_if_more_than')
+    amount = fields.Float(string='Amount', help="Amount of the order to benefit from a free shipping, expressed in the company currency")
 
     _sql_constraints = [
         ('margin_not_under_100_percent', 'CHECK (margin >= -100)', 'Margin cannot be lower than -100%'),
     ]
 
-    @api.one
     def toggle_prod_environment(self):
-        self.prod_environment = not self.prod_environment
+        for c in self:
+            c.prod_environment = not c.prod_environment
 
     @api.multi
     def install_more_provider(self):
@@ -74,65 +73,53 @@ class DeliveryCarrier(models.Model):
                 </p>'''),
         }
 
-    @api.depends('product_id.list_price', 'product_id.product_tmpl_id.list_price')
-    def _compute_fixed_price(self):
-        for carrier in self:
-            carrier.fixed_price = carrier.product_id.list_price
+    def available_carriers(self, partner):
+        return self.filtered(lambda c: c._match_address(partner))
 
-    def _set_product_fixed_price(self):
-        for carrier in self:
-            carrier.product_id.list_price = carrier.fixed_price
+    def _match_address(self, partner):
+        self.ensure_one()
+        if self.country_ids and partner.country_id not in self.country_ids:
+            return False
+        if self.state_ids and partner.state_id not in self.state_ids:
+            return False
+        if self.zip_from and (partner.zip or '') < self.zip_from:
+            return False
+        if self.zip_to and (partner.zip or '') > self.zip_to:
+            return False
+        return True
 
-    @api.one
-    def get_price(self):
-        SaleOrder = self.env['sale.order']
+    @api.onchange('state_ids')
+    def onchange_states(self):
+        self.country_ids = [(6, 0, self.country_ids.ids + self.state_ids.mapped('country_id.id'))]
 
-        self.available = False
-        self.price = False
-
-        order_id = self.env.context.get('order_id')
-        if order_id:
-            # FIXME: temporary hack until we refactor the delivery API in master
-
-            order = SaleOrder.browse(order_id)
-            if self.delivery_type not in ['fixed', 'base_on_rule']:
-                try:
-                    computed_price = self.get_shipping_price_from_so(order)
-                    self.available = True
-                except ValidationError as e:
-                    # No suitable delivery method found, probably configuration error
-                    _logger.info("Carrier %s: %s, not found", self.name, e.name)
-                    computed_price = 0.0
-            else:
-                carrier = self.verify_carrier(order.partner_shipping_id)
-                if carrier:
-                    try:
-                        computed_price = carrier.get_price_available(order)
-                        self.available = True
-                    except UserError as e:
-                        # No suitable delivery method found, probably configuration error
-                        _logger.info("Carrier %s: %s", carrier.name, e.name)
-                        computed_price = 0.0
-                else:
-                    computed_price = 0.0
-
-            self.price = computed_price * (1.0 + (float(self.margin) / 100.0))
+    @api.onchange('country_ids')
+    def onchange_countries(self):
+        self.state_ids = [(6, 0, self.state_ids.filtered(lambda state: state.id in self.country_ids.mapped('state_ids').ids).ids)]
 
     # -------------------------- #
     # API for external providers #
     # -------------------------- #
 
-    # TODO define and handle exceptions that could be thrown by providers
-
-    def get_shipping_price_from_so(self, order):
+    def rate_shipment(self, order):
         ''' Compute the price of the order shipment
 
-        :param order: A recordset of a sales order
-        :return list: A float, the estimated price for the shipping of the sales order
+        :param order: record of sale.order
+        :return dict: {'success': boolean,
+                       'price': a float,
+                       'error_message': a string containing an error message,
+                       'warning_message': a string containing a warning message}
+                       # TODO maybe the currency code?
         '''
         self.ensure_one()
-        if hasattr(self, '%s_get_shipping_price_from_so' % self.delivery_type):
-            return getattr(self, '%s_get_shipping_price_from_so' % self.delivery_type)(order)
+        if hasattr(self, '%s_rate_shipment' % self.delivery_type):
+            res = getattr(self, '%s_rate_shipment' % self.delivery_type)(order)
+            # apply margin on computed price
+            res['price'] = res['price'] * (1.0 + (float(self.margin) / 100.0))
+            # free when order is large enough
+            if res['success'] and self.free_over and order._compute_amount_total_without_delivery >= self.amount:
+                res['warning_message'] = _('Warning:\nTotal amount of this order is over %.2f, free shipping!\n(actual cost: %.2f)') % (self.amount, res['price'])
+                res['price'] = 0.0
+            return res
 
     def send_shipping(self, pickings):
         ''' Send the package to the service provider
@@ -141,6 +128,9 @@ class DeliveryCarrier(models.Model):
         :return list: A list of dictionaries (one per picking) containing of the form::
                          { 'exact_price': price,
                            'tracking_number': number }
+                           # TODO missing labels per package
+                           # TODO missing currency
+                           # TODO missing success, error, warnings
         '''
         self.ensure_one()
         if hasattr(self, '%s_send_shipping' % self.delivery_type):
@@ -165,113 +155,36 @@ class DeliveryCarrier(models.Model):
         if hasattr(self, '%s_cancel_shipment' % self.delivery_type):
             return getattr(self, '%s_cancel_shipment' % self.delivery_type)(pickings)
 
-    @api.onchange('state_ids')
-    def onchange_states(self):
-        self.country_ids = [(6, 0, self.country_ids.ids + self.state_ids.mapped('country_id.id'))]
+    # ------------------------------------------------ #
+    # Fixed price shipping, aka a very simple provider #
+    # ------------------------------------------------ #
 
-    @api.onchange('country_ids')
-    def onchange_countries(self):
-        self.state_ids = [(6, 0, self.state_ids.filtered(lambda state: state.id in self.country_ids.mapped('state_ids').ids).ids)]
+    fixed_price = fields.Float(compute='_compute_fixed_price', inverse='_set_product_fixed_price', store=True, string='Fixed Price')
 
-    @api.multi
-    def verify_carrier(self, contact):
-        self.ensure_one()
-        if self.country_ids and contact.country_id not in self.country_ids:
-            return False
-        if self.state_ids and contact.state_id not in self.state_ids:
-            return False
-        if self.zip_from and (contact.zip or '') < self.zip_from:
-            return False
-        if self.zip_to and (contact.zip or '') > self.zip_to:
-            return False
-        return self
+    @api.depends('product_id.list_price', 'product_id.product_tmpl_id.list_price')
+    def _compute_fixed_price(self):
+        for carrier in self:
+            carrier.fixed_price = carrier.product_id.list_price
 
-    @api.multi
-    def create_price_rules(self):
-        PriceRule = self.env['delivery.price.rule']
-        for record in self:
-            # If using advanced pricing per destination: do not change
-            if record.delivery_type == 'base_on_rule':
-                continue
+    def _set_product_fixed_price(self):
+        for carrier in self:
+            carrier.product_id.list_price = carrier.fixed_price
 
-            # Not using advanced pricing per destination: override lines
-            if record.delivery_type == 'base_on_rule' and not (record.fixed_price is not False or record.free_if_more_than):
-                record.price_rule_ids.unlink()
+    def fixed_rate_shipment(self, order):
+        return {'success': True,
+                'price': self.fixed_price,
+                'error_message': False,
+                'warning_message': False}
 
-            # Check that float, else 0.0 is False
-            if not (record.fixed_price is not False or record.free_if_more_than):
-                continue
-
-            if record.delivery_type == 'fixed':
-                PriceRule.search([('carrier_id', '=', record.id)]).unlink()
-
-                line_data = {
-                    'carrier_id': record.id,
-                    'variable': 'price',
-                    'operator': '>=',
-                }
-                # Create the delivery price rules
-                if record.free_if_more_than:
-                    line_data.update({
-                        'max_value': record.amount,
-                        'standard_price': 0.0,
-                        'list_base_price': 0.0,
-                    })
-                    PriceRule.create(line_data)
-                if record.fixed_price is not False:
-                    line_data.update({
-                        'max_value': 0.0,
-                        'standard_price': record.fixed_price,
-                        'list_base_price': record.fixed_price,
-                    })
-                    PriceRule.create(line_data)
-        return True
-
-    @api.model
-    def create(self, vals):
-        res = super(DeliveryCarrier, self).create(vals)
-        res.create_price_rules()
+    def fixed_send_shipping(self, pickings):
+        res = []
+        for p in pickings:
+            res = res + [{'exact_price': p.carrier_id.fixed_price,
+                          'tracking_number': False}]
         return res
 
-    @api.multi
-    def write(self, vals):
-        res = super(DeliveryCarrier, self).write(vals)
-        self.create_price_rules()
-        return res
+    def fixed_get_tracking_link(self, pickings):
+        raise NotImplementedError()
 
-    @api.multi
-    def get_price_available(self, order):
-        self.ensure_one()
-        total = weight = volume = quantity = 0
-        total_delivery = 0.0
-        for line in order.order_line:
-            if line.state == 'cancel':
-                continue
-            if line.is_delivery:
-                total_delivery += line.price_total
-            if not line.product_id or line.is_delivery:
-                continue
-            qty = line.product_uom._compute_quantity(line.product_uom_qty, line.product_id.uom_id)
-            weight += (line.product_id.weight or 0.0) * qty
-            volume += (line.product_id.volume or 0.0) * qty
-            quantity += qty
-        total = (order.amount_total or 0.0) - total_delivery
-
-        total = order.currency_id.with_context(date=order.date_order).compute(total, order.company_id.currency_id)
-
-        return self.get_price_from_picking(total, weight, volume, quantity)
-
-    def get_price_from_picking(self, total, weight, volume, quantity):
-        price = 0.0
-        criteria_found = False
-        price_dict = {'price': total, 'volume': volume, 'weight': weight, 'wv': volume * weight, 'quantity': quantity}
-        for line in self.price_rule_ids:
-            test = safe_eval(line.variable + line.operator + str(line.max_value), price_dict)
-            if test:
-                price = line.list_base_price + line.list_price * price_dict[line.variable_factor]
-                criteria_found = True
-                break
-        if not criteria_found:
-            raise UserError(_("Selected product in the delivery method doesn't fulfill any of the delivery carrier(s) criteria."))
-
-        return price
+    def fixed_cancel_shipment(self, pickings):
+        raise NotImplementedError()

addons/delivery/models/delivery_grid.py

@@ -0,0 +1,115 @@
+# -*- coding: utf-8 -*-
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
+
+from odoo import models, fields, api, _
+from odoo.addons import decimal_precision as dp
+from odoo.tools.safe_eval import safe_eval
+from odoo.exceptions import UserError, ValidationError
+
+
+class PriceRule(models.Model):
+    _name = "delivery.price.rule"
+    _description = "Delivery Price Rules"
+    _order = 'sequence, list_price, id'
+
+    @api.depends('variable', 'operator', 'max_value', 'list_base_price', 'list_price', 'variable_factor')
+    def _compute_name(self):
+        for rule in self:
+            name = 'if %s %s %s then' % (rule.variable, rule.operator, rule.max_value)
+            if rule.list_base_price and not rule.list_price:
+                name = '%s fixed price %s' % (name, rule.list_base_price)
+            elif rule.list_price and not rule.list_base_price:
+                name = '%s %s times %s' % (name, rule.list_price, rule.variable_factor)
+            else:
+                name = '%s fixed price %s and %s times %s Extra' % (name, rule.list_base_price, rule.list_price, rule.variable_factor)
+            rule.name = name
+
+    name = fields.Char(compute='_compute_name')
+    sequence = fields.Integer(required=True, default=10)
+    carrier_id = fields.Many2one('delivery.carrier', 'Carrier', required=True, ondelete='cascade')
+
+    variable = fields.Selection([('weight', 'Weight'), ('volume', 'Volume'), ('wv', 'Weight * Volume'), ('price', 'Price'), ('quantity', 'Quantity')], required=True, default='weight')
+    operator = fields.Selection([('==', '='), ('<=', '<='), ('<', '<'), ('>=', '>='), ('>', '>')], required=True, default='<=')
+    max_value = fields.Float('Maximum Value', required=True)
+    list_base_price = fields.Float(string='Sale Base Price', digits=dp.get_precision('Product Price'), required=True, default=0.0)
+    list_price = fields.Float('Sale Price', digits=dp.get_precision('Product Price'), required=True, default=0.0)
+    variable_factor = fields.Selection([('weight', 'Weight'), ('volume', 'Volume'), ('wv', 'Weight * Volume'), ('price', 'Price'), ('quantity', 'Quantity')], 'Variable Factor', required=True, default='weight')
+
+    # TODO remove me, should be replaced by fixed price
+    standard_price = fields.Float('Cost Price', digits=dp.get_precision('Product Price'), required=True, default=0.0)
+
+
+class ProviderGrid(models.Model):
+    _inherit = 'delivery.carrier'
+
+    delivery_type = fields.Selection(selection_add=[('base_on_rule', 'Based on Rules')])
+    price_rule_ids = fields.One2many('delivery.price.rule', 'carrier_id', 'Pricing Rules', copy=True)
+
+    def base_on_rule_rate_shipment(self, order):
+        carrier = self._match_address(order.partner_shipping_id)
+        if not carrier:
+            return {'success': False,
+                    'price': 0.0,
+                    'error_message': _('Error: no matching grid.'),
+                    'warning_message': False}
+
+        price_unit = self._get_price_available(order)
+        if order.company_id.currency_id.id != order.pricelist_id.currency_id.id:
+            price_unit = order.company_id.currency_id.with_context(date=order.date_order).compute(price_unit, order.pricelist_id.currency_id)
+
+        return {'success': True,
+                'price': self.fixed_price,
+                'error_message': False,
+                'warning_message': False}
+
+    def _get_price_available(self, order):
+        self.ensure_one()
+        total = weight = volume = quantity = 0
+        total_delivery = 0.0
+        for line in order.order_line:
+            if line.state == 'cancel':
+                continue
+            if line.is_delivery:
+                total_delivery += line.price_total
+            if not line.product_id or line.is_delivery:
+                continue
+            qty = line.product_uom._compute_quantity(line.product_uom_qty, line.product_id.uom_id)
+            weight += (line.product_id.weight or 0.0) * qty
+            volume += (line.product_id.volume or 0.0) * qty
+            quantity += qty
+        total = (order.amount_total or 0.0) - total_delivery
+
+        total = order.currency_id.with_context(date=order.date_order).compute(total, order.company_id.currency_id)
+
+        return self._get_price_from_picking(total, weight, volume, quantity)
+
+    def _get_price_from_picking(self, total, weight, volume, quantity):
+        price = 0.0
+        criteria_found = False
+        price_dict = {'price': total, 'volume': volume, 'weight': weight, 'wv': volume * weight, 'quantity': quantity}
+        for line in self.price_rule_ids:
+            test = safe_eval(line.variable + line.operator + str(line.max_value), price_dict)
+            if test:
+                price = line.list_base_price + line.list_price * price_dict[line.variable_factor]
+                criteria_found = True
+                break
+        if not criteria_found:
+            raise UserError(_("Selected product in the delivery method doesn't fulfill any of the delivery carrier(s) criteria."))
+
+        return price
+
+    def base_on_rule_send_shipping(self, pickings):
+        res = []
+        for p in pickings:
+            carrier = self._match_address(p.partner_id)
+            if not carrier:
+                raise ValidationError(_('Error: no matching grid.'))
+            res = res + [{'exact_price': p.carrier_id._get_price_available(p.sale_id),  # TODO cleanme
+                          'tracking_number': False}]
+        return res
+
+    def base_on_rule__get_tracking_link(self, pickings):
+        raise NotImplementedError()
+
+    def base_on_rule__cancel_shipment(self, pickings):
+        raise NotImplementedError()

addons/delivery/models/delivery_price_rule.py

@@ -1,35 +0,0 @@
-# -*- coding: utf-8 -*-
-# Part of Odoo. See LICENSE file for full copyright and licensing details.
-
-from odoo import models, fields, api
-from odoo.addons import decimal_precision as dp
-
-
-class PriceRule(models.Model):
-    _name = "delivery.price.rule"
-    _description = "Delivery Price Rules"
-    _order = 'sequence, list_price, id'
-
-    @api.depends('variable', 'operator', 'max_value', 'list_base_price', 'list_price', 'variable_factor')
-    def _compute_name(self):
-        for rule in self:
-            name = 'if %s %s %s then' % (rule.variable, rule.operator, rule.max_value)
-            if rule.list_base_price and not rule.list_price:
-                name = '%s fixed price %s' % (name, rule.list_base_price)
-            elif rule.list_price and not rule.list_base_price:
-                name = '%s %s times %s' % (name, rule.list_price, rule.variable_factor)
-            else:
-                name = '%s fixed price %s and %s times %s Extra' % (name, rule.list_base_price, rule.list_price, rule.variable_factor)
-            rule.name = name
-
-    name = fields.Char(compute='_compute_name')
-    sequence = fields.Integer(required=True, default=10)
-    carrier_id = fields.Many2one('delivery.carrier', 'Carrier', required=True, ondelete='cascade')
-
-    variable = fields.Selection([('weight', 'Weight'), ('volume', 'Volume'), ('wv', 'Weight * Volume'), ('price', 'Price'), ('quantity', 'Quantity')], required=True, default='weight')
-    operator = fields.Selection([('==', '='), ('<=', '<='), ('<', '<'), ('>=', '>='), ('>', '>')], required=True, default='<=')
-    max_value = fields.Float('Maximum Value', required=True)
-    list_base_price = fields.Float(string='Sale Base Price', digits=dp.get_precision('Product Price'), required=True, default=0.0)
-    list_price = fields.Float('Sale Price', digits=dp.get_precision('Product Price'), required=True, default=0.0)
-    variable_factor = fields.Selection([('weight', 'Weight'), ('volume', 'Volume'), ('wv', 'Weight * Volume'), ('price', 'Price'), ('quantity', 'Quantity')], 'Variable Factor', required=True, default='weight')
-    standard_price = fields.Float('Cost Price', digits=dp.get_precision('Product Price'), required=True, default=0.0)

addons/delivery/models/product_template.py

@@ -1,21 +1,10 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-from odoo import api, models, fields
+from odoo import models, fields
 
 
 class ProductTemplate(models.Model):
     _inherit = 'product.template'
 
-    hs_code = fields.Char(string="HS Code", help="Standardized code for international shipping and goods declaration", oldname="x_hs_code")
-
-    @api.multi
-    def write(self, vals):
-        res = super(ProductTemplate, self).write(vals)
-        if vals.get('list_price'):
-            DeliveryCarrier = self.env['delivery.carrier']
-            for template in self:
-                carrier = DeliveryCarrier.search([('product_id', 'in', template.product_variant_ids.ids)])
-                if carrier:
-                    carrier.create_price_rules()
-        return res
+    hs_code = fields.Char(string="HS Code", help="Standardized code for international shipping and goods declaration")

addons/delivery/models/sale_order.py

@@ -9,27 +9,45 @@ from odoo.addons import decimal_precision as dp
 class SaleOrder(models.Model):
     _inherit = 'sale.order'
 
-    delivery_price = fields.Float(string='Estimated Delivery Price', compute='_compute_delivery_price', store=True)
-    carrier_id = fields.Many2one("delivery.carrier", string="Delivery Method", help="Fill this field if you plan to invoice the shipping based on picking.")
-    invoice_shipping_on_delivery = fields.Boolean(string="Invoice Shipping on Delivery")
+    carrier_id = fields.Many2one('delivery.carrier', string="Delivery Method", help="Fill this field if you plan to invoice the shipping based on picking.")
+    delivery_price = fields.Float(string='Estimated Delivery Price', readonly=True, copy=False)
+    delivery_message = fields.Char(readonly=True, copy=False)
+    delivery_rating_success = fields.Boolean(copy=False)
+    invoice_shipping_on_delivery = fields.Boolean(string="Invoice Shipping on Delivery", copy=False)
 
-    @api.depends('carrier_id', 'order_line')
-    def _compute_delivery_price(self):
-        for order in self:
-            if order.state != 'draft':
-                # We do not want to recompute the shipping price of an already validated/done SO
-                continue
-            elif order.carrier_id.delivery_type != 'grid' and not order.order_line:
-                # Prevent SOAP call to external shipping provider when SO has no lines yet
-                continue
+    def _compute_amount_total_without_delivery(self):
+        self.ensure_one()
+        delivery_cost = sum([l.price_total for l in self.order_line if l.is_delivery])
+        return self.amount_total - delivery_cost
+
+    def get_delivery_price(self):
+        for order in self.filtered(lambda o: o.state in ('draft', 'sent') and len(o.order_line) > 0):
+            # We do not want to recompute the shipping price of an already validated/done SO
+            # or on an SO that has no lines yet
+            order.delivery_rating_success = False
+            res = order.carrier_id.rate_shipment(order)
+            if res['success']:
+                order.delivery_rating_success = True
+                order.delivery_price = res['price']
+                order.delivery_message = res['warning_message']
             else:
-                order.delivery_price = order.carrier_id.with_context(order_id=order.id).price
+                order.delivery_rating_success = False
+                order.delivery_price = 0.0
+                order.delivery_message = res['error_message']
+
+    @api.onchange('carrier_id')
+    def onchange_carrier_id(self):
+        if self.state in ('draft', 'sent'):
+            self.delivery_price = 0.0
+            self.delivery_rating_success = False
 
     @api.onchange('partner_id')
-    def onchange_partner_id_dtype(self):
+    def onchange_partner_id_carrier_id(self):
         if self.partner_id:
             self.carrier_id = self.partner_id.property_delivery_carrier_id
 
+    # TODO onchange sol, clean delivery price
+
     @api.multi
     def action_confirm(self):
         res = super(SaleOrder, self).action_confirm()
@@ -38,39 +56,26 @@ class SaleOrder(models.Model):
         return res
 
     @api.multi
-    def _delivery_unset(self):
+    def _remove_delivery_line(self):
         self.env['sale.order.line'].search([('order_id', 'in', self.ids), ('is_delivery', '=', True)]).unlink()
 
     @api.multi
-    def delivery_set(self):
+    def set_delivery_line(self):
 
         # Remove delivery products from the sales order
-        self._delivery_unset()
+        self._remove_delivery_line()
 
         for order in self:
-            carrier = order.carrier_id
-            if carrier:
-                if order.state not in ('draft', 'sent'):
-                    raise UserError(_('The order state have to be draft to add delivery lines.'))
-
-                if carrier.delivery_type not in ['fixed', 'base_on_rule']:
-                    # Shipping providers are used when delivery_type is other than 'fixed' or 'base_on_rule'
-                    price_unit = order.carrier_id.get_shipping_price_from_so(order)
-                else:
-                    # Classic grid-based carriers
-                    carrier = order.carrier_id.verify_carrier(order.partner_shipping_id)
-                    if not carrier:
-                        raise UserError(_('No carrier matching.'))
-                    price_unit = carrier.get_price_available(order)
-                    if order.company_id.currency_id.id != order.pricelist_id.currency_id.id:
-                        price_unit = order.company_id.currency_id.with_context(date=order.date_order).compute(price_unit, order.pricelist_id.currency_id)
-
-                final_price = price_unit * (1.0 + (float(self.carrier_id.margin) / 100.0))
-                order._create_delivery_line(carrier, final_price)
-
-            else:
+            if order.state not in ('draft', 'sent'):
+                raise UserError(_('You can add delivery price only on unconfirmed quotations.'))
+            elif not order.carrier_id:
                 raise UserError(_('No carrier set for this order.'))
-
+            elif not order.delivery_rating_success:
+                raise UserError(_('Please use "Check price" in order to compute a shipping price for this quotation.'))
+            else:
+                price_unit = order.carrier_id.rate_shipment(order)['price']
+                # TODO check whether it is safe to use delivery_price here
+                order._create_delivery_line(order.carrier_id, price_unit)
         return True
 
     def _create_delivery_line(self, carrier, price_unit):

addons/delivery/models/stock_picking.py

@@ -104,7 +104,7 @@ class StockPicking(models.Model):
         self.ensure_one()
         res = super(StockPicking, self).do_transfer()
 
-        if self.carrier_id and self.carrier_id.delivery_type not in ['fixed', 'base_on_rule'] and self.carrier_id.integration_level == 'rate_and_ship' and self.package_ids:
+        if self.carrier_id and self.carrier_id.integration_level == 'rate_and_ship':
             self.send_to_shipper()
 
         if self.carrier_id:

addons/delivery/tests/test_delivery_cost.py

@@ -74,7 +74,8 @@ class TestDeliveryCost(common.TransactionCase):
         })
 
         # I add delivery cost in Sales order
-        self.sale_normal_delivery_charges.delivery_set()
+        self.sale_normal_delivery_charges.get_delivery_price()
+        self.sale_normal_delivery_charges.set_delivery_line()
 
         # I check sales order after added delivery cost
 
@@ -113,7 +114,8 @@ class TestDeliveryCost(common.TransactionCase):
         })
 
         # I add free delivery cost in Sales order
-        self.delivery_sale_order_cost.delivery_set()
+        self.delivery_sale_order_cost.get_delivery_price()
+        self.delivery_sale_order_cost.set_delivery_line()
 
         # I check sales order after adding delivery cost
         line = self.SaleOrderLine.search([('order_id', '=', self.delivery_sale_order_cost.id),

addons/delivery/tests/test_delivery_stock_move.py

@@ -41,7 +41,8 @@ class StockMoveInvoice(AccountingTestCase):
         })
 
         # I add delivery cost in Sales order
-        self.sale_prepaid.delivery_set()
+        self.sale_prepaid.get_delivery_price()
+        self.sale_prepaid.set_delivery_line()
 
         # I confirm the SO.
         self.sale_prepaid.action_confirm()

addons/delivery/views/delivery_view.xml

@@ -81,6 +81,13 @@
                                     <field name="margin" class="oe_inline"/>%
                                 </div>
                             </group>
+                            <group>
+                                <div class="o_row">
+                                    <label for="free_over"/>
+                                    <field name="free_over"/>
+                                    <field nolabel="1" name="amount" attrs="{'required':[('free_over','!=', False)], 'invisible':[('free_over','=', False)]}"/>
+                                </div>
+                            </group>
                         </group>
                         <notebook>
                             <page string="Pricing" attrs="{'invisible': [('delivery_type', 'not in', ['fixed', 'base_on_rule'])]}">
@@ -88,13 +95,6 @@
                                     <group>
                                         <field name="fixed_price"/>
                                     </group>
-                                    <group>
-                                        <div class="o_row">
-                                            <label for="free_if_more_than"/>
-                                            <field name="free_if_more_than"/>
-                                            <field nolabel="1" name="amount" attrs="{'required':[('free_if_more_than','!=', False)], 'invisible':[('free_if_more_than','=', False)]}"/>
-                                        </div>
-                                    </group>
                                 </group>
                                 <group name="general" attrs="{'invisible':[('delivery_type', '!=', 'base_on_rule')]}">
                                     <field name="price_rule_ids" nolabel="1"/>
@@ -285,11 +285,16 @@
                       <label for="carrier_id"/>
                       <div name='carrier_selection'>
                           <div>
-                            <field name="carrier_id" context="{'order_id': id}" class="oe_inline" options="{'no_create': True, 'no_open': True}" attrs="{'readonly':[('state','not in',('draft','sent'))]}"/>
+                            <field name="carrier_id" context="{'order_id': id}" class="oe_inline" options="{'no_create': True, 'no_open': True}" attrs="{'readonly':[('state','not in',('draft','sent'))]}"/> <i class="fa fa-check text-success" aria-hidden="true" attrs="{'invisible':['|','|',('carrier_id','=',False),('state','not in',('draft','sent')),('delivery_rating_success','=',False)]}"></i>
                           </div>
                           <div>
                             <field name='delivery_price' widget='monetary' class="oe_inline" options="{'currency_field': 'currency_id'}" nolabel="1" attrs="{'invisible': [('carrier_id','=', False)]}"/>
-                            <button name="delivery_set" string="Set price" type="object" class="oe_inline fa fa-arrow-right oe_link" attrs="{'invisible':['|',('carrier_id','=',False),('state','not in',('draft','sent'))]}"/>
+                            <button name="get_delivery_price" string="Check price" type="object" class="oe_inline fa fa-arrow-right oe_link" attrs="{'invisible':['|',('carrier_id','=',False),('state','not in',('draft','sent'))]}"/>
+                            <button name="set_delivery_line" string="Set price" type="object" class="oe_inline fa fa-arrow-right oe_link" attrs="{'invisible':['|','|',('carrier_id','=',False),('state','not in',('draft','sent')),('delivery_rating_success','=',False)]}"/>
+                            <field name='delivery_rating_success' invisible="1"/>
+                          </div>
+                          <div class="alert alert-info" role="alert" attrs="{'invisible': ['|',('carrier_id','=', False),('delivery_message','=',False)]}">
+                            <field name='delivery_message'/>
                           </div>
                       </div>
                   </xpath>

addons/website_sale_delivery/models/sale_order.py

@@ -35,14 +35,12 @@ class SaleOrder(models.Model):
             order.has_delivery = any(order.order_line.filtered('is_delivery'))
 
     def _check_carrier_quotation(self, force_carrier_id=None):
-        # check to add or remove carrier_id
-        if not self:
-            return False
         self.ensure_one()
         DeliveryCarrier = self.env['delivery.carrier']
+
         if self.only_services:
             self.write({'carrier_id': None})
-            self._delivery_unset()
+            self._remove_delivery_line()
             return True
         else:
             carrier = force_carrier_id and DeliveryCarrier.browse(force_carrier_id) or self.carrier_id
@@ -56,45 +54,28 @@ class SaleOrder(models.Model):
                     available_carriers = carrier + available_carriers
             if force_carrier_id or not carrier or carrier not in available_carriers:
                 for delivery in available_carriers:
-                    verified_carrier = delivery.verify_carrier(self.partner_shipping_id)
+                    verified_carrier = delivery._match_address(self.partner_shipping_id)
                     if verified_carrier:
                         carrier = delivery
                         break
                 self.write({'carrier_id': carrier.id})
             if carrier:
-                self.delivery_set()
+                self.get_delivery_price()
+                self.set_delivery_line()
             else:
-                self._delivery_unset()
+                self._remove_delivery_line()
 
         return bool(carrier)
 
     def _get_delivery_methods(self):
-        """Return the available and published delivery carriers"""
-        self.ensure_one()
-        available_carriers = DeliveryCarrier = self.env['delivery.carrier']
-        # Following loop is done to avoid displaying delivery methods who are not available for this order
-        # This can surely be done in a more efficient way, but at the moment, it mimics the way it's
-        # done in delivery_set method of sale.py, from delivery module
-        carrier_ids = DeliveryCarrier.sudo().search(
-            [('website_published', '=', True)]).ids
-        for carrier_id in carrier_ids:
-            carrier = DeliveryCarrier.browse(carrier_id)
-            try:
-                _logger.debug("Checking availability of carrier #%s" % carrier_id)
-                available = carrier.with_context(order_id=self.id).read(fields=['available'])[0]['available']
-                if available:
-                    available_carriers += carrier
-            except ValidationError as e:
-                # RIM TODO: hack to remove, make available field not depend on a SOAP call to external shipping provider
-                # The validation error is used in backend to display errors in fedex config, but should fail silently in frontend
-                _logger.debug("Carrier #%s removed from e-commerce carrier list. %s" % (carrier_id, e))
-        return available_carriers
+        address = self.partner_shipping_id
+        return self.env['delivery.carrier'].sudo().search([('website_published', '=', True)]).available_carriers(address)
 
     @api.multi
     def _cart_update(self, product_id=None, line_id=None, add_qty=0, set_qty=0, **kwargs):
         """ Override to update carrier quotation if quantity changed """
 
-        self._delivery_unset()
+        self._remove_delivery_line()
 
         # When you update a cart, it is not enouf to remove the "delivery cost" line
         # The carrier might also be invalid, eg: if you bought things that are too heavy

addons/website_sale_delivery/views/website_sale_delivery_templates.xml

@@ -28,11 +28,10 @@
                 <h4>Choose a delivery method</h4>
                 <ul class="list-unstyled">
                     <li t-foreach="deliveries" t-as="delivery">
-                        <t t-if="delivery.available">
                             <div id="delivery_method">
                                 <label>
                                     <input t-att-value="delivery.id" type="radio" name="delivery_type"
-                                        t-att-checked="order.carrier_id and order.carrier_id.id == delivery.id and 'checked' or False"/>
+                                        t-att-checked="order.carrier_id and order.carrier_id.id == delivery.id and 'checked' or False" disabled="1"/>
                                     <abbr t-att-title="delivery.website_description"><span t-field="delivery.name"/></abbr>
                                     <span class="badge" t-field="delivery.price"
                                         t-options='{
@@ -43,7 +42,6 @@
                                     <div t-field="delivery.website_description" class="text-muted"/>
                                 </label>
                             </div>
-                        </t>
                     </li>
                 </ul>
                 <a class="btn btn-default btn-block"