From 1925b23fbd9002375cdbd447519c8288ce7086e3 Mon Sep 17 00:00:00 2001 From: pgu-odoo Date: Tue, 20 Dec 2016 14:39:59 +0100 Subject: [PATCH 01/18] [ADD] sale_coupon_delivery: Allow to offer a free shipping from a coupon --- __init__.py | 4 ++ __manifest__.py | 15 ++++++++ models/__init__.py | 6 +++ models/sale_coupon.py | 14 +++++++ models/sale_coupon_program.py | 26 +++++++++++++ models/sale_coupon_reward.py | 19 ++++++++++ models/sale_order.py | 31 ++++++++++++++++ tests/__init__.py | 4 ++ tests/test_free_shipping_reward.py | 59 ++++++++++++++++++++++++++++++ 9 files changed, 178 insertions(+) create mode 100644 __init__.py create mode 100644 __manifest__.py create mode 100644 models/__init__.py create mode 100644 models/sale_coupon.py create mode 100644 models/sale_coupon_program.py create mode 100644 models/sale_coupon_reward.py create mode 100644 models/sale_order.py create mode 100644 tests/__init__.py create mode 100644 tests/test_free_shipping_reward.py diff --git a/__init__.py b/__init__.py new file mode 100644 index 00000000000..81fc81d29c9 --- /dev/null +++ b/__init__.py @@ -0,0 +1,4 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +import models diff --git a/__manifest__.py b/__manifest__.py new file mode 100644 index 00000000000..f5a84c3da02 --- /dev/null +++ b/__manifest__.py @@ -0,0 +1,15 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. +{ + 'name': "Sale Coupon Delivery", + 'summary': """Allows to offer free shippings in coupon reward""", + 'description': """Integrate coupon mechanism with shipping costs.""", + 'category': 'Sales', + 'version': '1.0', + 'depends': ['sale_coupon', 'delivery'], + 'data': [ + ], + 'demo': [ + ], + 'auto_install': True, +} diff --git a/models/__init__.py b/models/__init__.py new file mode 100644 index 00000000000..0d67cd44d37 --- /dev/null +++ b/models/__init__.py @@ -0,0 +1,6 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +import sale_order +import sale_coupon_program +import sale_coupon_reward \ No newline at end of file diff --git a/models/sale_coupon.py b/models/sale_coupon.py new file mode 100644 index 00000000000..ccde14fcb89 --- /dev/null +++ b/models/sale_coupon.py @@ -0,0 +1,14 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + + +from odoo import models, _ + + +class SaleCoupon(models.Model): + _inherit = "sale.coupon" + + def _check_coupon_code(self, order): + if self.program_id.reward_type == 'free_shipping' and not order.order_line.filtered(lambda line: line.is_delivery): + return {'error': _('The shipping costs are not in the order lines.')} + return super(SaleCoupon, self)._check_coupon_code(order) diff --git a/models/sale_coupon_program.py b/models/sale_coupon_program.py new file mode 100644 index 00000000000..657cbaa179c --- /dev/null +++ b/models/sale_coupon_program.py @@ -0,0 +1,26 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + + +from odoo import models, _ + + +class SaleCouponProgram(models.Model): + _inherit = "sale.coupon.program" + + def _filter_not_ordered_reward_programs(self, order): + """ + Returns the programs when the reward is actually in the order lines + """ + programs = super(SaleCouponProgram, self)._filter_not_ordered_reward_programs(order) + for program in self: + if program.reward_type == 'free_shipping' and \ + not order.order_line.filtered(lambda line: line.is_delivery): + programs -= program + return programs + + def _check_promo_code(self, order, coupon_code): + if self.reward_type == 'free_shipping' and not order.order_line.filtered(lambda line: line.is_delivery): + return {'error': _('The shipping costs are not in the order lines.')} + return super(SaleCouponProgram, self)._check_promo_code(order, coupon_code) + diff --git a/models/sale_coupon_reward.py b/models/sale_coupon_reward.py new file mode 100644 index 00000000000..83be4f11976 --- /dev/null +++ b/models/sale_coupon_reward.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo import fields, models, _ + + +class SaleCouponReward(models.Model): + _inherit = 'sale.coupon.reward' + _description = "Sales Coupon Reward" + + reward_type = fields.Selection(selection_add=[('free_shipping', 'Free Shipping')]) + + def name_get(self): + result = [] + reward_names = super(SaleCouponReward, self).name_get() + free_shipping_reward_ids = self.filtered(lambda reward: reward.reward_type == 'free_shipping').ids + for res in reward_names: + result.append((res[0], res[0] in free_shipping_reward_ids and _("Free Shipping") or res[1])) + return result diff --git a/models/sale_order.py b/models/sale_order.py new file mode 100644 index 00000000000..3924117d218 --- /dev/null +++ b/models/sale_order.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo import models + +class SaleOrder(models.Model): + _inherit = "sale.order" + + def _get_order_lines_untaxed_amount(self): + """ Returns the untaxed sale order total amount without the rewards and shipping amount""" + return sum([x.price_subtotal for x in self.order_line.filtered(lambda x: not (x.is_reward_line or x.is_delivery))]) + + def _get_reward_line_values(self, program): + if program.reward_type == 'free_shipping': + return self._get_reward_values_free_shipping(program) + else: + return super(SaleOrder, self)._get_reward_line_values(program) + + def _get_reward_values_free_shipping(self, program): + return { + 'name': "Discount: %s" % (program.name), + 'product_id': program.discount_line_product_id.id, + 'price_unit': - self.order_line.filtered(lambda x: x.is_delivery).price_unit, + 'product_uom_qty': 1.0, + 'product_uom': program.discount_line_product_id.uom_id.id, + 'order_id': self.id, + 'is_reward_line': True + } + + def _get_lines_unit_prices(self): + return [x.price_unit for x in self.order_line.filtered(lambda x: not x.is_delivery and not x.program_id)] diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 00000000000..3a17aa142c0 --- /dev/null +++ b/tests/__init__.py @@ -0,0 +1,4 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +import test_free_shipping_reward \ No newline at end of file diff --git a/tests/test_free_shipping_reward.py b/tests/test_free_shipping_reward.py new file mode 100644 index 00000000000..fc8b6734919 --- /dev/null +++ b/tests/test_free_shipping_reward.py @@ -0,0 +1,59 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from openerp.addons.sale_coupon.tests.common import TestSaleCouponCommon + + +class TestSaleCouponProgramRules(TestSaleCouponCommon): + # Test a free shipping reward + some expected behavior + # (automatic line addition or removal) + + def test_free_shipping_reward(self): + # Test case 1: The minimum amount is not reached, the reward should + # not be created + self.immediate_promotion_program.active = False + self.env['sale.coupon.program'].create({ + 'name': 'Free Shipping if at least 100 euros', + 'promo_code_usage': 'no_code_needed', + 'reward_type': 'free_shipping', + 'rule_minimum_amount': 100.0, + 'active': True, + }) + + order = self.env['sale.order'].create({ + 'partner_id': self.steve.id, + }) + + order.write({'order_line': [ + (0, False, { + 'product_id': self.product_B.id, + 'name': 'Product B', + 'product_uom': self.uom_unit.id, + 'product_uom_qty': 1.0, + }) + ]}) + self.assertEqual(len(order.order_line.ids), 1) + + order.carrier_id = self.env['delivery.carrier'].search([])[1] + order.delivery_set() + self.assertEqual(len(order.order_line.ids), 2) + + # Test case 2: the amount is sufficient, the shipping should + # be reimbursed + order.write({'order_line': [ + (0, False, { + 'product_id': self.product_A.id, + 'name': 'Product 1', + 'product_uom': self.uom_unit.id, + 'product_uom_qty': 1.0, + 'price_unit': 1000 + }) + ]}) + + self.assertEqual(len(order.order_line.ids), 4) + + # Test case 3: the amount is not sufficient now, the reward should be removed + order.write({'order_line': [ + (2, order.order_line.filtered(lambda line: line.product_id.id == self.product_A.id).id, False) + ]}) + self.assertEqual(len(order.order_line.ids), 2) From 3b71aeb77ce5973717201c1b5d860c7a2c1cc6b3 Mon Sep 17 00:00:00 2001 From: Yannick Tivisse Date: Thu, 23 Feb 2017 10:25:45 +0100 Subject: [PATCH 02/18] [FIX] sale_coupon_delivery: Set taxes on reward line for free shipping --- models/sale_order.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/models/sale_order.py b/models/sale_order.py index 3924117d218..24e0097cdfc 100644 --- a/models/sale_order.py +++ b/models/sale_order.py @@ -17,14 +17,19 @@ class SaleOrder(models.Model): return super(SaleOrder, self)._get_reward_line_values(program) def _get_reward_values_free_shipping(self, program): + delivery_line = self.order_line.filtered(lambda x: x.is_delivery) + taxes = delivery_line.product_id.taxes_id + if self.fiscal_position_id: + taxes = self.fiscal_position_id.map_tax(taxes) return { 'name': "Discount: %s" % (program.name), 'product_id': program.discount_line_product_id.id, - 'price_unit': - self.order_line.filtered(lambda x: x.is_delivery).price_unit, + 'price_unit': - delivery_line.price_unit, 'product_uom_qty': 1.0, 'product_uom': program.discount_line_product_id.uom_id.id, 'order_id': self.id, - 'is_reward_line': True + 'is_reward_line': True, + 'tax_id': [(4, tax.id, False) for tax in taxes], } def _get_lines_unit_prices(self): From 12a06dcfa678d5317d0a1e62c85e5a9508974aa2 Mon Sep 17 00:00:00 2001 From: Yannick Tivisse Date: Fri, 24 Feb 2017 10:47:13 +0100 Subject: [PATCH 03/18] [FIX] sale_coupon_delivery: Do not unlink reward line if 'free_shipping' The methods delivery_set and delivery_unset are called everywhere (mostly on the ecommerce). The reasons for that is that we do not want several shipping lines on the same order. A better way to achieve this would have been to create the line if it doesn't exist or write on it to modify the 'price_unit' instead. The side effect of that behavior is: - A delivery line is set on the order. - We apply a coupon to get a free shipping - We go on the payment page. The method _cart_update is called, which calls delivery_set, so delivery_unset. The shipping line is unlinked to create again a new shipping line with the price updated. As the shipping line is removed, a recomputation of the reward lines is made. The program isn't applicable anymore as there's no shipping line on the order. The reward is thus removed. That way, it's impossible to have a free shipping reward on the ecommerce and achieve the full checkout process. What's done in this commit: If a free shipping is applicable by code or if a free shipping can be automatically applied. It will be done with a price of 0.0 if there's no shipping line or the price_unit on the shipping line if set. If the shipping line is removed (every 2 seconds on a checkout process), the reward line is not unlinked and the price is set to 0.0. That way, it's now possible to make a full flow. NB: If a line is unlinked on an order, we unlink the reward line if it's the last one set on the order. This is to avoid free shipping line coming back on a new order in the ecommerce even if not applicable. --- models/sale_coupon_program.py | 8 +++----- models/sale_order.py | 22 +++++++++++++++++++++- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/models/sale_coupon_program.py b/models/sale_coupon_program.py index 657cbaa179c..6f7e7aa4669 100644 --- a/models/sale_coupon_program.py +++ b/models/sale_coupon_program.py @@ -13,14 +13,12 @@ class SaleCouponProgram(models.Model): Returns the programs when the reward is actually in the order lines """ programs = super(SaleCouponProgram, self)._filter_not_ordered_reward_programs(order) - for program in self: - if program.reward_type == 'free_shipping' and \ - not order.order_line.filtered(lambda line: line.is_delivery): - programs -= program + # Do not filter on free delivery programs. As delivery_unset is called everywhere (which is + # rather stupid), the delivery line is unliked to be created again instead of writing on it to + # modify the price_unit. That way, the reward is unlink and is not set back again. return programs def _check_promo_code(self, order, coupon_code): if self.reward_type == 'free_shipping' and not order.order_line.filtered(lambda line: line.is_delivery): return {'error': _('The shipping costs are not in the order lines.')} return super(SaleCouponProgram, self)._check_promo_code(order, coupon_code) - diff --git a/models/sale_order.py b/models/sale_order.py index 24e0097cdfc..a6968eb5a07 100644 --- a/models/sale_order.py +++ b/models/sale_order.py @@ -24,7 +24,7 @@ class SaleOrder(models.Model): return { 'name': "Discount: %s" % (program.name), 'product_id': program.discount_line_product_id.id, - 'price_unit': - delivery_line.price_unit, + 'price_unit': delivery_line and - delivery_line.price_unit or 0.0, 'product_uom_qty': 1.0, 'product_uom': program.discount_line_product_id.uom_id.id, 'order_id': self.id, @@ -34,3 +34,23 @@ class SaleOrder(models.Model): def _get_lines_unit_prices(self): return [x.price_unit for x in self.order_line.filtered(lambda x: not x.is_delivery and not x.program_id)] + +class SalesOrderLine(models.Model): + _inherit = "sale.order.line" + + def unlink(self): + # Due to delivery_set and delivery_unset methods that are called everywhere, don't unlink + # reward lines if it's a free shipping + orders = self.mapped('order_id') + applied_programs = orders.mapped('no_code_promo_program_ids') + \ + orders.mapped('code_promo_program_id') + \ + orders.mapped('applied_coupon_ids').mapped('program_id') + free_shipping_products = applied_programs.filtered( + lambda program: program.reward_type == 'free_shipping' + ).mapped('discount_line_product_id') + lines_to_unlink = self.filtered(lambda line: line.product_id not in free_shipping_products) + # Unless these lines are the last ones + res = super(SalesOrderLine, lines_to_unlink).unlink() + only_free_shipping_line_orders = orders.filtered(lambda order: len(order.order_line.ids) == 1 and order.order_line.is_reward_line) + super(SalesOrderLine, only_free_shipping_line_orders.mapped('order_line')).unlink() + return res From 7d7ed7940ffc5411505fb89ddbf3e33db1cba597 Mon Sep 17 00:00:00 2001 From: xmo-odoo Date: Fri, 28 Apr 2017 09:07:28 +0200 Subject: [PATCH 04/18] [FIX] implicitly relative imports --- __init__.py | 2 +- models/__init__.py | 6 +++--- tests/__init__.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/__init__.py b/__init__.py index 81fc81d29c9..dc5e6b693d1 100644 --- a/__init__.py +++ b/__init__.py @@ -1,4 +1,4 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -import models +from . import models diff --git a/models/__init__.py b/models/__init__.py index 0d67cd44d37..9c652c3aac3 100644 --- a/models/__init__.py +++ b/models/__init__.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -import sale_order -import sale_coupon_program -import sale_coupon_reward \ No newline at end of file +from . import sale_order +from . import sale_coupon_program +from . import sale_coupon_reward \ No newline at end of file diff --git a/tests/__init__.py b/tests/__init__.py index 3a17aa142c0..0f8b647a735 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,4 +1,4 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -import test_free_shipping_reward \ No newline at end of file +from . import test_free_shipping_reward \ No newline at end of file From cc8debe1e86dd1b50cc9f536481796ee6ffafcf6 Mon Sep 17 00:00:00 2001 From: Yannick Tivisse Date: Wed, 21 Jun 2017 10:24:02 +0200 Subject: [PATCH 05/18] [FIX] sale_coupon: Don't update automatic rewards on the fly Purpose ======= Currently we are updating the coupon rewards on the fly when creating/modifying/deleting a sales order line. To avoid multiple searches on res_partners and product to know whether the customer/product is valid on the sale coupon program, we store the applicable customers and products on the program according to the domain defined on it. This brings 2 issues: - The customers/products stored on the program are not updated (Was the original spec) - The checks are numerous and maybe overkill for the given results Specification ============= - Don't recompute the reward lines on SO modifications. Add a button to compute the line by hand and make an automatic compute on the SO validation or on the page shop/payment. - When recomputing the reward lines, make a search on the res_partners and products instead. Now that the action is not triggered so often we can afford to make a search. - Don't use the fields rule_partners_domain and rule_products_domain. This should be removed in master. - The field is_public_included is useless now. And we decided to make the coupon available for all the partners if they fit the domain on the program. --- tests/test_free_shipping_reward.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_free_shipping_reward.py b/tests/test_free_shipping_reward.py index fc8b6734919..b7780cd6f29 100644 --- a/tests/test_free_shipping_reward.py +++ b/tests/test_free_shipping_reward.py @@ -32,10 +32,12 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): 'product_uom_qty': 1.0, }) ]}) + order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 1) order.carrier_id = self.env['delivery.carrier'].search([])[1] order.delivery_set() + order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 2) # Test case 2: the amount is sufficient, the shipping should @@ -50,10 +52,12 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): }) ]}) + order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 4) # Test case 3: the amount is not sufficient now, the reward should be removed order.write({'order_line': [ (2, order.order_line.filtered(lambda line: line.product_id.id == self.product_A.id).id, False) ]}) + order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 2) From 1893094fe4ddfb84de59e1c1a77e9dd393eeeee2 Mon Sep 17 00:00:00 2001 From: Richard Mathot Date: Fri, 7 Jul 2017 15:07:26 +0200 Subject: [PATCH 06/18] [IMP] delivery_*: refactoring and cleaning Reflect changes in delivery_carrier providers as a big refactoring was implemented in https://github.com/odoo/odoo/commit/924ffc6bc30d9f4f36681a51ebd89849018d2dc8 --- tests/test_free_shipping_reward.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_free_shipping_reward.py b/tests/test_free_shipping_reward.py index b7780cd6f29..488d09c86ba 100644 --- a/tests/test_free_shipping_reward.py +++ b/tests/test_free_shipping_reward.py @@ -36,7 +36,9 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): self.assertEqual(len(order.order_line.ids), 1) order.carrier_id = self.env['delivery.carrier'].search([])[1] - order.delivery_set() + order.get_delivery_price() + order.set_delivery_line() + order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 2) From 40c817b308ad3bb4076deb586ecb4125bc1f79d2 Mon Sep 17 00:00:00 2001 From: "Lucas Perais (lpe)" Date: Tue, 22 Aug 2017 17:18:30 +0200 Subject: [PATCH 07/18] [FIX] sale_coupon_delivery: never include the delivery into the order's base for coupon before this commit, the delivery was included into the base of computation for coupon programs meaning that: have a program free shipping for orders above 50$ have a product with price included 48$ let's shipping costs be 9$ total price is 57$. The program used to apply in that case, giving a total discounted price of 48$, when it should be 57$ with no discounts after this commit, deliveries are excluded from the computation OPW 766661 closes #1311 --- models/sale_coupon_program.py | 19 ++++++++++++++++++- tests/test_free_shipping_reward.py | 23 +++++++++++++++++++++-- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/models/sale_coupon_program.py b/models/sale_coupon_program.py index 6f7e7aa4669..d0d1e9ad786 100644 --- a/models/sale_coupon_program.py +++ b/models/sale_coupon_program.py @@ -2,7 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo import models, _ +from odoo import models, _, api class SaleCouponProgram(models.Model): @@ -22,3 +22,20 @@ class SaleCouponProgram(models.Model): if self.reward_type == 'free_shipping' and not order.order_line.filtered(lambda line: line.is_delivery): return {'error': _('The shipping costs are not in the order lines.')} return super(SaleCouponProgram, self)._check_promo_code(order, coupon_code) + + @api.model + def _filter_on_mimimum_amount(self, order): + # To get actual total amount of SO without any reward or discount + # We exclude any line of the order that is a delivery + # get the round function of the currency: we want to imitate what the sale.order.amount is (computed field) + round_curr = order.pricelist_id.currency_id.round + order_lines = order.order_line.filtered(lambda l: not l.is_delivery) + order_amount = round_curr(sum(order_lines.mapped('price_subtotal'))) + order_tax = round_curr(sum(order_lines.mapped('price_tax'))) + discounted_amount = sum(order_lines.filtered(lambda line: line.is_reward_line).mapped('price_total')) + untaxed_amount = order_amount - discounted_amount + return self.filtered(lambda program: + program.rule_minimum_amount_tax_inclusion == 'tax_included' and + program._compute_program_amount('rule_minimum_amount', order.currency_id) <= untaxed_amount + order_tax or + program.rule_minimum_amount_tax_inclusion == 'tax_excluded' and + program._compute_program_amount('rule_minimum_amount', order.currency_id) <= untaxed_amount) diff --git a/tests/test_free_shipping_reward.py b/tests/test_free_shipping_reward.py index b7780cd6f29..b5666e20c33 100644 --- a/tests/test_free_shipping_reward.py +++ b/tests/test_free_shipping_reward.py @@ -24,6 +24,7 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): 'partner_id': self.steve.id, }) + # Price of order will be 5*1.15 = 5.75 (tax included) order.write({'order_line': [ (0, False, { 'product_id': self.product_B.id, @@ -40,6 +41,24 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 2) + # Test Case 1b: amount is not reached but is on a threshold + # The amount of deliverable product + the one of the delivery exceeds the minimum amount + # yet the program shouldn't be applied + + # Order price will be 5.75 + 81.74*1.15 = 99.75 + order.write({'order_line': [ + (0, False, { + 'product_id': self.product_B.id, + 'name': 'Product 1B', + 'product_uom': self.uom_unit.id, + 'product_uom_qty': 1.0, + 'price_unit': 81.74, + }) + ]}) + + order.recompute_coupon_lines() + self.assertEqual(len(order.order_line.ids), 3) + # Test case 2: the amount is sufficient, the shipping should # be reimbursed order.write({'order_line': [ @@ -53,11 +72,11 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): ]}) order.recompute_coupon_lines() - self.assertEqual(len(order.order_line.ids), 4) + self.assertEqual(len(order.order_line.ids), 5) # Test case 3: the amount is not sufficient now, the reward should be removed order.write({'order_line': [ (2, order.order_line.filtered(lambda line: line.product_id.id == self.product_A.id).id, False) ]}) order.recompute_coupon_lines() - self.assertEqual(len(order.order_line.ids), 2) + self.assertEqual(len(order.order_line.ids), 3) From d0d8f245b335544128fe81506713b3e9a44a2cc2 Mon Sep 17 00:00:00 2001 From: Christophe Simonis Date: Tue, 9 Jan 2018 18:12:47 +0100 Subject: [PATCH 08/18] [MERGE] forward port branch 10.0 up to 0097bff4331 --- __init__.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/__init__.py b/__init__.py index 81fc81d29c9..d5017840d66 100644 --- a/__init__.py +++ b/__init__.py @@ -1,4 +1,12 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +from odoo import api, SUPERUSER_ID import models + + +def uninstall_hook(cr, registry): + env = api.Environment(cr, SUPERUSER_ID, {}) + env['delivery.carrier'].search([ + ('delivery_type', '=', 'temando') + ]).write({'delivery_type': 'fixed', 'fixed_price': 0}) From d8ea1d04d5b9cabb6817fffc78ca4b74079a7a11 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Mon, 18 Dec 2017 18:06:50 +0100 Subject: [PATCH 09/18] [FIX] sale_coupon: fix some use cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some use cases were wrong, this commit attempt to fix those. Here is the most important ones: 1. Discount was offered on free product 'value' aswell Cart: 4 iPad Mini + one free offered + 10% discount | qty | unit value | line total | ---------------------------------------------------- iPad Mini | 4 | 320 | 1280 | - 10% discount| 1 | 128 | 128 | = 1152 - free product| 1 | 320 | 320 | = 832 Should be | qty | unit value | line total | ---------------------------------------------------- iPad Mini | 4 | 320 | 1280 | - free product| 1 | 320 | 320 | = 960 - 10% discount| 1 | 96 | 96 | = 864 The error result is $32 which is the 10% discount applied on the free iPad. 2. Decreasing product quantity below the minimum threshold to be elligible for the promotion doesn't remove the product - (With a promotion offering a free iPad when you buy 3 (fourth is free)) - Create a quotation with 3 iPads Mini, save and update promotions, you don't have a free one, which is 'correct' - Edit, change the quantity to 4 iPads, save, update promotions, you have a free iPad (reduced from total), still correct - Edit, change the quantity back to 3 ipad, update ptomotions, you still have a free iPad even if you only bought 3 of them (the way it is designed, the free iPad should only appears when you order your fourth). The error result in a 'loss' of $320 since at the end we are offering a free iPad and the user only pays 2 instead of 3. 3. Free product will be considered as regular product when updating quantity of regular product afterward This error is kind of exponential: the more you play with the flow explained bellow, the more incorrect it becomes. Buy 100 iPads, get 25 free iPads. Then set quantity (on regular iPad) to 50, it will recalculate the amount of free iPad thinking the 25 previous free iPads are regular iPad. (50 + 25) = 75 regular iPads = 18 free iPads (25 previous free iPads were wrongly used for the calculation but they are simply gone at this point). Cart is now 50 regular iPads and 18 free one, total: $10240 (50×320 - 18×320). It should be 12 free iPads, not 18, so $12160 (50×320 − 12×320) The error result in a loss of $1920: ~ 20% of the total amount of the order. This error will also appear the other way around if you add quantity instead of removing some. Playing with this error multiple times, you can arrive to a negative total (more free than paid products), 0 total or very low total ($320 for 100 iPads). 4. Free product are wrongly calculated, user get too many of them - Create a quotation with 10 iPads, you get 3 free iPads instead of 2 - Indeed, your first 4 iPads should give you a free one - Your next 4 iPads should give you a second one (you got 8) - The last 2 (to reach 10) are not enough to give you a third free iPad The error result in a loss of $320 out of a total of 2560$ which is ~ 15% 5. Paid/regular products related to a promotion won't be taken into account when checking if you have the minimum amount threshold to apply the promotion Promotion: 3 Computer Case(25$) = 1 Free Little Server (40.000$), auto applied -Have a cart with 3 'Computer Case' and 2 'Little Server' -One little server of the 2 is free (because you have 3 computer cases) -You so pay 3x25 + 1x40000 which is 40075$ -Create a promo code that will give you 10% discount if you bought over $2000 -You won't get the reduction even if you bought over $40.000 because the little server you pay is considered as the product of the reward product even if you dont have it free. It doesn't check how many regular product should be substracted according to the number of free product you have 6. Wrong product is checked for the required quantity to apply the promotion Promotion: 3 iPads Mini ($320) = 1 free Bose Mini Bluetooth Speaker ($247) - Add 8 iPads Mini and a Bose Mini Bluetooth Speaker to your cart - The system should give you a free Bose Mini Bluetooth Speaker since you only have one regular Bose - But the system is giving you 2 free Bose Mini Bluetooth Speaker Cart is: 8 iPad ($2560), 1 Bose ($247), 2 free Bose (-$496) The system think that it should give you 2 free Bose because you are eligible to, since you have 8 iPad, but it doesn't check that you have enough regular Bose. The error result in a loss of $247 out of a total of $2311 7. Auto applying promotion are not applied if there is already a promotion code Promotion: - Free iPad Mini if you buy 3 (so the fourth become free), auto applied - 10% discount with code '10pc' - Add 4 iPads Mini, you got a free one - Set quantity back to 1 iPad, free one is gone - Add code 10pc, you got a 10% reduction line - Set quantity to 4 iPads again, you dont have the free iPad since there is already a promo code This could make sense but it is not coherent since the 2 promotions would be applied if they were both on auto apply mode. Note: - If both promotions are on auto apply, you will get both - If 10% discount becomes auto applied and iPad promotion now has a code (reverse situation) it will work 8. Discount is only applied on untaxed amount, not on tax Discount will ONLY be calculated on the untaxed amount and then reduced from the sale order/cart total. The taxes would be unchanged and charged like the product was full price. Let's illustrate with a $100 product, a 15% tax and a 20% discount: 1. Tax included: $100 : Product with 15% included taxes $86.96 : Untaxed product $13.04 : Taxes After aplying 20% discount, result should be: $80 : Product with 15% included taxes $69.57 : Untaxed product $10.43 : Taxes Instead we have: $82.61 : Product with 15% included taxes $69.57 : Untaxed product $13.04 : Taxes There is a difference of $2.61 which is the result of the taxes not being discounted: 13.04-10.43 = 2.61 2. Tax excluded: $115 : Product with 15% excluded taxes $100 : Untaxed product $15 : Taxes After aplying 20% discount, result should be: $92 : Product with 15% included taxes $80 : Untaxed product $12 : Taxes Instead we have: $95 : Product with 15% included taxes $80 : Untaxed product $15 : Taxes There is a difference of $3 which is the result of the taxes not being discounted: 15-12 = 3 9. Taxes of the free shipping reward would be considered as paid value when checking if required threshold is met. ------------------------------------------ Not really a bug, more an improvement: 1. The way it was designed, the rule_minimum_amount would check if your order amount was over the required threshold to be eligible for the reward. 'Problem' is that it your order has already a discount (eg: 10%), it would check if the price without discount is above the required amount. I don't think it is coherent, let's illustrate: - First pomotion (auto applied): 1 iPad Mini for free if you buy over $2000 - Second promotion (auto applied): 50% on order If an user is buying for $2000, he is actually paying $1000 thanks to the 50% discount. Should we give him the free iPad? The current implementation is giving it, which is not convenient IMHO. Now, we use the amount the user really pay to check if he is elligible. 2. Since new promotions have the same sequence by default, they will be sorted and applied by their creation order. This will result in inconsistent behavior: - Promotion 1 : 10% discount - Promotion 2 : free shipping - Both promotion are auto applied and need a minimum of $872.73 tax excluded Let's illustrate with a $873 order: If the promotions were created in the order above, you won't get both rewards since the 10% discount will be applied first and will lower the SO total. The order will then fall below the $872.73 required amount and won't be eligible for the free shipping. But if you created the free shipping promotion before the discount one, you will be eligible for both since the free shipping will be applied first and won't change the SO total. To keep the same behavior independently of the promotion creation order, we now sort by reward_type ('discount' > 'free_shipping' > 'product' to apply discounts first. This is better IMHO, since you want to give promotions with a minimum threshold ONLY if the user really pays that amount. It wouldn't be the case if the total is checked before the discount is applied. --- models/sale_coupon_program.py | 17 ----------------- models/sale_order.py | 18 +++++++++++++----- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/models/sale_coupon_program.py b/models/sale_coupon_program.py index d0d1e9ad786..53995810def 100644 --- a/models/sale_coupon_program.py +++ b/models/sale_coupon_program.py @@ -22,20 +22,3 @@ class SaleCouponProgram(models.Model): if self.reward_type == 'free_shipping' and not order.order_line.filtered(lambda line: line.is_delivery): return {'error': _('The shipping costs are not in the order lines.')} return super(SaleCouponProgram, self)._check_promo_code(order, coupon_code) - - @api.model - def _filter_on_mimimum_amount(self, order): - # To get actual total amount of SO without any reward or discount - # We exclude any line of the order that is a delivery - # get the round function of the currency: we want to imitate what the sale.order.amount is (computed field) - round_curr = order.pricelist_id.currency_id.round - order_lines = order.order_line.filtered(lambda l: not l.is_delivery) - order_amount = round_curr(sum(order_lines.mapped('price_subtotal'))) - order_tax = round_curr(sum(order_lines.mapped('price_tax'))) - discounted_amount = sum(order_lines.filtered(lambda line: line.is_reward_line).mapped('price_total')) - untaxed_amount = order_amount - discounted_amount - return self.filtered(lambda program: - program.rule_minimum_amount_tax_inclusion == 'tax_included' and - program._compute_program_amount('rule_minimum_amount', order.currency_id) <= untaxed_amount + order_tax or - program.rule_minimum_amount_tax_inclusion == 'tax_excluded' and - program._compute_program_amount('rule_minimum_amount', order.currency_id) <= untaxed_amount) diff --git a/models/sale_order.py b/models/sale_order.py index a6968eb5a07..7be352afd52 100644 --- a/models/sale_order.py +++ b/models/sale_order.py @@ -1,14 +1,22 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo import models +from odoo import models, _ class SaleOrder(models.Model): _inherit = "sale.order" - def _get_order_lines_untaxed_amount(self): - """ Returns the untaxed sale order total amount without the rewards and shipping amount""" - return sum([x.price_subtotal for x in self.order_line.filtered(lambda x: not (x.is_reward_line or x.is_delivery))]) + def _get_no_effect_on_threshold_lines(self): + self.ensure_one() + # Do not count shipping and free shipping + free_delivery_product = self.env['sale.coupon.program'].search([('reward_type', '=', 'free_shipping')]).mapped('discount_line_product_id') + lines = self.order_line.filtered(lambda line: line.is_delivery or line.product_id in free_delivery_product) + return lines + super(SaleOrder, self)._get_no_effect_on_threshold_lines() + + def _get_order_lines_tax_included_amount(self): + """ Returns the taxes included sale order total amount without the rewards amount""" + free_reward_product = self.env['sale.coupon.program'].search([('reward_type', '=', 'product')]).mapped('discount_line_product_id') + return sum([x.price_total for x in self.order_line.filtered(lambda x: not (x.is_reward_line or x.is_delivery) or x.product_id in free_reward_product)]) def _get_reward_line_values(self, program): if program.reward_type == 'free_shipping': @@ -22,7 +30,7 @@ class SaleOrder(models.Model): if self.fiscal_position_id: taxes = self.fiscal_position_id.map_tax(taxes) return { - 'name': "Discount: %s" % (program.name), + 'name': _("Discount: ") + program.name, 'product_id': program.discount_line_product_id.id, 'price_unit': delivery_line and - delivery_line.price_unit or 0.0, 'product_uom_qty': 1.0, From bc75ec7c1426866550ef26b8fd53294962f5ffd7 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Fri, 15 Dec 2017 16:05:02 +0100 Subject: [PATCH 10/18] [IMP] sale_coupon, website_sale_coupon: add tests Commit a42c27f99e0061b7c4f recently fixed some bugs on sale_coupon. This commit add some tests to be sure theses bugs are fixed and won't appear again. This closes #1744 --- tests/test_free_shipping_reward.py | 107 ++++++++++++++++++++++++++++- 1 file changed, 104 insertions(+), 3 deletions(-) diff --git a/tests/test_free_shipping_reward.py b/tests/test_free_shipping_reward.py index b5666e20c33..2f69ec8e802 100644 --- a/tests/test_free_shipping_reward.py +++ b/tests/test_free_shipping_reward.py @@ -5,6 +5,11 @@ from openerp.addons.sale_coupon.tests.common import TestSaleCouponCommon class TestSaleCouponProgramRules(TestSaleCouponCommon): + + def setUp(self): + super(TestSaleCouponProgramRules, self).setUp() + self.iPadMini = self.env.ref('product.product_product_6').copy() + # Test a free shipping reward + some expected behavior # (automatic line addition or removal) @@ -17,6 +22,7 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): 'promo_code_usage': 'no_code_needed', 'reward_type': 'free_shipping', 'rule_minimum_amount': 100.0, + 'rule_minimum_amount_tax_inclusion': 'tax_included', 'active': True, }) @@ -44,7 +50,6 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): # Test Case 1b: amount is not reached but is on a threshold # The amount of deliverable product + the one of the delivery exceeds the minimum amount # yet the program shouldn't be applied - # Order price will be 5.75 + 81.74*1.15 = 99.75 order.write({'order_line': [ (0, False, { @@ -55,7 +60,6 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): 'price_unit': 81.74, }) ]}) - order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 3) @@ -67,7 +71,7 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): 'name': 'Product 1', 'product_uom': self.uom_unit.id, 'product_uom_qty': 1.0, - 'price_unit': 1000 + 'price_unit': 0.30, }) ]}) @@ -80,3 +84,100 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): ]}) order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 3) + + def test_shipping_cost(self): + # Free delivery should not be taken into account when checking for minimum required threshold + p_minimum_threshold_free_delivery = self.env['sale.coupon.program'].create({ + 'name': 'free shipping if > 872 tax exl', + 'promo_code_usage': 'no_code_needed', + 'reward_type': 'free_shipping', + 'program_type': 'promotion_program', + 'rule_minimum_amount': 872, + }) + p_minimum_threshold_discount = self.env['sale.coupon.program'].create({ + 'name': '10% reduction if > 872 tax exl', + 'promo_code_usage': 'no_code_needed', + 'reward_type': 'discount', + 'program_type': 'promotion_program', + 'discount_type': 'percentage', + 'discount_percentage': 10.0, + 'rule_minimum_amount': 872, + }) + order = self.empty_order + self.iPadMini.taxes_id = self.tax_10pc_incl + sol1 = self.env['sale.order.line'].create({ + 'product_id': self.iPadMini.id, + 'name': 'iPad Mini', + 'product_uom_qty': 3.0, + 'order_id': order.id, + }) + order.recompute_coupon_lines() + self.assertEqual(len(order.order_line.ids), 2, "We should get the 10% discount line since we bought 872.73$") + order.carrier_id = self.env['delivery.carrier'].search([])[1] + order.delivery_set() + + order.recompute_coupon_lines() + self.assertEqual(len(order.order_line.ids), 3, "We should get the delivery line but not the free delivery since we are below 872.73$ with the 10% discount") + + p_minimum_threshold_free_delivery.sequence = 10 + order.recompute_coupon_lines() + self.assertEqual(len(order.order_line.ids), 4, "We should get both promotion line since the free delivery will be applied first and won't change the SO total") + + def test_shipping_cost_numbers(self): + # Free delivery should not be taken into account when checking for minimum required threshold + tax_15pc_excl = self.env['account.tax'].create({ + 'name': "15% Tax excl", + 'amount_type': 'percent', + 'amount': 15, + }) + p_minimum_threshold_free_delivery = self.env['sale.coupon.program'].create({ + 'name': 'free shipping if > 872 tax exl', + 'promo_code_usage': 'code_needed', + 'promo_code': 'free_shipping', + 'reward_type': 'free_shipping', + 'program_type': 'promotion_program', + 'rule_minimum_amount': 872, + }) + self.p2 = self.env['sale.coupon.program'].create({ + 'name': 'Buy 4 ipads, get one for free', + 'promo_code_usage': 'no_code_needed', + 'reward_type': 'product', + 'program_type': 'promotion_program', + 'reward_product_id': self.iPadMini.id, + 'rule_min_quantity': 3, + 'rule_products_domain': '[["name","ilike","ipad mini"]]', + }) + order = self.empty_order + self.iPadMini.taxes_id = self.tax_10pc_incl + sol1 = self.env['sale.order.line'].create({ + 'product_id': self.iPadMini.id, + 'name': 'iPad Mini', + 'product_uom_qty': 3.0, + 'order_id': order.id, + }) + # The delivery product has 15% tax on dball but not on dbbase. A module is probably adding the tax at some point. + # We want that tax to be set so we can be sure it is not added to the order total paid amount used in coupon as + # it was wrongly done before. + self.env.ref('delivery.delivery_carrier').taxes_id = tax_15pc_excl + order.carrier_id = self.env['delivery.carrier'].search([])[1] + order.delivery_set() + order.recompute_coupon_lines() + self.assertEqual(len(order.order_line.ids), 2, "") + self.assertEqual(order.reward_amount, 0) + # Shipping is 20 + 15%tax + self.assertEqual(sum([line.price_total for line in order._get_no_effect_on_threshold_lines()]), 23) + self.assertEqual(order.amount_untaxed, 872.73 + 20) + + self.env['sale.coupon.apply.code'].sudo().apply_coupon(order, 'free_shipping') + order.recompute_coupon_lines() + self.assertEqual(len(order.order_line.ids), 3, "We should get the delivery line and the free delivery since we are below 872.73$ with the 10% discount") + self.assertEqual(order.reward_amount, -20) + self.assertEqual(sum([line.price_total for line in order._get_no_effect_on_threshold_lines()]), 0) + self.assertEqual(order.amount_untaxed, 872.73) + + sol1.product_uom_qty = 4 + order.recompute_coupon_lines() + self.assertEqual(len(order.order_line.ids), 4, "We should get a free iPad Mini") + self.assertEqual(order.reward_amount, -20 -290.91) + self.assertEqual(sum([line.price_total for line in order._get_no_effect_on_threshold_lines()]), 0) + self.assertEqual(order.amount_untaxed, 872.73) \ No newline at end of file From 4d2c5d1003829cc004517d0e33158d6c155af680 Mon Sep 17 00:00:00 2001 From: Christophe Simonis Date: Thu, 1 Mar 2018 20:14:37 +0100 Subject: [PATCH 11/18] [FIX] sale_coupon_delivery: adapt test to version 11 --- tests/test_free_shipping_reward.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/test_free_shipping_reward.py b/tests/test_free_shipping_reward.py index d2ca33038d9..11b75d08d78 100644 --- a/tests/test_free_shipping_reward.py +++ b/tests/test_free_shipping_reward.py @@ -116,7 +116,8 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 2, "We should get the 10% discount line since we bought 872.73$") order.carrier_id = self.env['delivery.carrier'].search([])[1] - order.delivery_set() + order.get_delivery_price() + order.set_delivery_line() order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 3, "We should get the delivery line but not the free delivery since we are below 872.73$ with the 10% discount") @@ -160,11 +161,13 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): # The delivery product has 15% tax on dball but not on dbbase. A module is probably adding the tax at some point. # We want that tax to be set so we can be sure it is not added to the order total paid amount used in coupon as # it was wrongly done before. - self.env.ref('delivery.delivery_carrier').taxes_id = tax_15pc_excl - order.carrier_id = self.env['delivery.carrier'].search([])[1] - order.delivery_set() + carrier = self.env.ref('delivery.delivery_carrier') + carrier.product_id.taxes_id = tax_15pc_excl + order.carrier_id = carrier + order.get_delivery_price() + order.set_delivery_line() order.recompute_coupon_lines() - self.assertEqual(len(order.order_line.ids), 2, "") + self.assertEqual(len(order.order_line.ids), 2) self.assertEqual(order.reward_amount, 0) # Shipping is 20 + 15%tax self.assertEqual(sum([line.price_total for line in order._get_no_effect_on_threshold_lines()]), 23) @@ -182,4 +185,4 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): self.assertEqual(len(order.order_line.ids), 4, "We should get a free iPad Mini") self.assertEqual(order.reward_amount, -20 -290.91) self.assertEqual(sum([line.price_total for line in order._get_no_effect_on_threshold_lines()]), 0) - self.assertEqual(order.amount_untaxed, 872.73) \ No newline at end of file + self.assertEqual(order.amount_untaxed, 872.73) From 4e81ca7814a9ac6a8eef7e8a1a491ef3b4668557 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Wed, 28 Mar 2018 18:00:48 +0200 Subject: [PATCH 12/18] [FIX] sale_coupon(_delivery): correct discount when specific product Before this commit: 1. With a coupon set on 'specific_product', the discount amount was multiplied by the product quantity twice. (Introduced by a42c27f which apply discount on the full price by using price_total (already multiplied by quantity) over price_unit. 2. With a coupon set on 'cheapest_product', the discount was not discounting taxes. 3. With a coupon set on 'cheapest_product' and `sale_coupon_delivery` installed, it would simply crashes and throws an error since `sale.order.line` has no `program_id` attribute. Now: 1. We no longer multiply by the quantity twice; 2. We apply discount on the full price (with taxes); 3. We use correct attribute to avoid crash. opw-1830986 --- models/sale_order.py | 3 ++- tests/test_free_shipping_reward.py | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/models/sale_order.py b/models/sale_order.py index 7be352afd52..e0f91648823 100644 --- a/models/sale_order.py +++ b/models/sale_order.py @@ -41,7 +41,8 @@ class SaleOrder(models.Model): } def _get_lines_unit_prices(self): - return [x.price_unit for x in self.order_line.filtered(lambda x: not x.is_delivery and not x.program_id)] + # Unit prices tax included + return [x.price_total / x.product_uom_qty for x in self.order_line.filtered(lambda x: not x.is_delivery and not x.is_reward_line)] class SalesOrderLine(models.Model): _inherit = "sale.order.line" diff --git a/tests/test_free_shipping_reward.py b/tests/test_free_shipping_reward.py index 2f69ec8e802..5c8cce3d1a4 100644 --- a/tests/test_free_shipping_reward.py +++ b/tests/test_free_shipping_reward.py @@ -180,4 +180,18 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): self.assertEqual(len(order.order_line.ids), 4, "We should get a free iPad Mini") self.assertEqual(order.reward_amount, -20 -290.91) self.assertEqual(sum([line.price_total for line in order._get_no_effect_on_threshold_lines()]), 0) - self.assertEqual(order.amount_untaxed, 872.73) \ No newline at end of file + self.assertEqual(order.amount_untaxed, 872.73) + + p_specific_product = self.env['sale.coupon.program'].create({ + 'name': '20% reduction on ipad in cart', + 'promo_code_usage': 'no_code_needed', + 'reward_type': 'discount', + 'program_type': 'promotion_program', + 'discount_type': 'percentage', + 'discount_percentage': 20.0, + 'discount_apply_on': 'cheapest_product', + }) + p_specific_product.discount_apply_on = 'cheapest_product' + order.recompute_coupon_lines() + # 872.73 - (20% of 1 iPad) = 872.73 - 64 = 808.73 + self.assertEqual(order.amount_untaxed, 808.73, "One iPad should be discounted by 20%") From eba6034ee876be0d5d51288fb6aeaa5a8b7dd4c6 Mon Sep 17 00:00:00 2001 From: Fabien Pinckaers Date: Wed, 20 Jun 2018 14:00:06 +0200 Subject: [PATCH 13/18] [IMP] product: Unify/Clean product demo data PURPOSE ======= 1. Unified product demo data 2. Less demo: one per use case 3. Demo data for all models Specification ============= 1. Refactor all the brol in the demo data 2. Adapt the tests to make them green, as some products are renamed, removed, created in python instead of as demo data ... --- tests/test_free_shipping_reward.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/test_free_shipping_reward.py b/tests/test_free_shipping_reward.py index 7b40d88c594..6c617bd1063 100644 --- a/tests/test_free_shipping_reward.py +++ b/tests/test_free_shipping_reward.py @@ -109,7 +109,7 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): self.iPadMini.taxes_id = self.tax_10pc_incl sol1 = self.env['sale.order.line'].create({ 'product_id': self.iPadMini.id, - 'name': 'iPad Mini', + 'name': 'Large Cabinet', 'product_uom_qty': 3.0, 'order_id': order.id, }) @@ -142,19 +142,19 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): 'rule_minimum_amount': 872, }) self.p2 = self.env['sale.coupon.program'].create({ - 'name': 'Buy 4 ipads, get one for free', + 'name': 'Buy 4 large cabinet, get one for free', 'promo_code_usage': 'no_code_needed', 'reward_type': 'product', 'program_type': 'promotion_program', 'reward_product_id': self.iPadMini.id, 'rule_min_quantity': 3, - 'rule_products_domain': '[["name","ilike","ipad mini"]]', + 'rule_products_domain': '[["name","ilike","large cabinet"]]', }) order = self.empty_order self.iPadMini.taxes_id = self.tax_10pc_incl sol1 = self.env['sale.order.line'].create({ 'product_id': self.iPadMini.id, - 'name': 'iPad Mini', + 'name': 'Large Cabinet', 'product_uom_qty': 3.0, 'order_id': order.id, }) @@ -182,13 +182,13 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): sol1.product_uom_qty = 4 order.recompute_coupon_lines() - self.assertEqual(len(order.order_line.ids), 4, "We should get a free iPad Mini") + self.assertEqual(len(order.order_line.ids), 4, "We should get a free Large Cabinet") self.assertEqual(order.reward_amount, -20 -290.91) self.assertEqual(sum([line.price_total for line in order._get_no_effect_on_threshold_lines()]), 0) self.assertEqual(order.amount_untaxed, 872.73) p_specific_product = self.env['sale.coupon.program'].create({ - 'name': '20% reduction on ipad in cart', + 'name': '20% reduction on large cabinet in cart', 'promo_code_usage': 'no_code_needed', 'reward_type': 'discount', 'program_type': 'promotion_program', @@ -199,4 +199,4 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): p_specific_product.discount_apply_on = 'cheapest_product' order.recompute_coupon_lines() # 872.73 - (20% of 1 iPad) = 872.73 - 64 = 808.73 - self.assertEqual(order.amount_untaxed, 808.73, "One iPad should be discounted by 20%") + self.assertEqual(order.amount_untaxed, 808.73, "One large cabinet should be discounted by 20%") From 194b83eeea6a1d0846fb7809ead3bfee617381fd Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Tue, 24 Apr 2018 16:09:49 +0200 Subject: [PATCH 14/18] [FIX] sale_coupon(_delivery): fix some flows (mostly on next order) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit is the continuity of a42c27f9 that fixed some bugs. One of the bug was that a discount was only applied on price without taxes. E.G: A $100 product with 20% excluded tax would cost $120 but a 50% discount would give the user only $50 discount and not $60 as expected. User would lose $10 discount. With these fixes, the discount line would correctly be set to $60 but since the discount line would not have tax, the merchant would lose the discount amount on the tax, in this case 50% of $20. This commit will improve this case by adding a tax on a discount line. Thus, since a global discount can be applied to multiple products having different taxes or even multiple taxes on the same product, a discount line should be split for every product taxes the discount applies on. Obviously, this is a big behavior change, even if the discount is now finally correct: the discount is applied on the total price and the merchant does not lose any taxes. To make it more clear, the discount lines description will mention on which taxes it is applied. There won't be any changes in the e-commerce since we will group the discount lines so it will appear as before even if there are multiple lines. This is only possible because we don't show taxes on line on the e-commerce. This commit also fixes more complex flows/use cases. Note: Fixes & flows will be illustrated with a SO but the behavior is the same with a cart on front-end. == Fixes == =========================================================================== The biggest flow that needed to be fixed is when a promotion program give a reward on the next order. Let's illustrate with a promotion program giving a free `product_B` on next order (so it will generate a coupon) if the SO meets the requirements. The requirements are: buy a `product_A` and buy for at least $600. (This program needs a code: 'free_b_on_next') 1. If the SO has a `product_A` and over $600, applying the promotion code `free_b_on_next` will error saying that `product_B` is not in the cart. This error should not appear, since the `product_B` is the reward for the next sale order that will use the generated coupon. Anyway, if you add `product_B` and apply the promotion code `free_b_on_next` to generate the coupon to check the rest of the flow. It will generate the coupon and send email to the customer. 2. Every time you will apply the promotion code `free_b_on_next`, it will regenerate a new coupon and resend a new email to the client. Then, create a second SO, add 2 `product_B` and try to apply the generated coupon from the first SO. 3. It will error saying that the $600 requirement is not met. It should not since the requirement was only required to generate the coupon on the first SO. User has now a coupon to enjoy a free `product_B` and should not buy $600. Anyway, let's add some products to meet the 600$ requirement to check the rest of the flow. 4. It will now error saying that `product_A` is not in the cart. As bug 3., it should not since this requirement was also required to generate the coupon on the first SO. Anyway, let's add `product_A` to meet the requirement to check the rest of the flow. 5. Updating the promotions will remove the reward and you won't be able to reapply the coupon since it was `used`. This bug was even more critical in front-end since any action on the cart will recompute the coupons and automatically delete the reward after adding it. The customer never saw the reward on his cart. This bug is related to 3. and 4. Indeed, when recomputing coupons, it will remove programs that are not applicable anymore. The code wrongly think it is the case with this coupon since we don't have $600 or a `product_A`. `_remove_invalid_reward_lines` should treat coupon coming from a promotion program set on `next_order` differently and not filter on requirements. =========================================================================== 6. (extension of 2.) Every time an user would meet the requirements of a promotion program generating a coupon for a next order, it would generate a coupon and send an email. E.g: If buy 2 iPads get 1 for free on next order. If user add and remove iPad for some reason, every time he has 2 iPads in his cart it will receive an email with a new coupon. 6'. When applying a promotion code that will reward next order, it will not be set in the SO `code_promo_program_id` (I did not change that behavior since it could make sense to be able to apply another coupon that will give -20% on current order). This will allow applying multiple promotion rewarding a next order and/or a promotion rewarding current order. (Which would not be possible if was stored on `code_promo_program_id` since you can only apply one code on an order) But this cause a bug: since the promotion program is not stored on the order, every time you will apply the coupon it will generate a new coupon and send an email. So if you work on a draft sale order and do some tests, it will send many emails to customer.. 7. Having multiple global discounts (on the whole SO) was forbidden: you can't apply a coupon promotion or a program promotion if there is already a global coupon discount or a global promotion discount with code. But still you could apply a global discount if there were a global promotion discount automatically applied (without a code). 8. On the generate coupon wizard, if you select `Number of Selected Customers`, by default it is set to 'Match all records (X records)'. But when you click on generate, it won't generate anything even if visually it selected every partner by default. This is because the domain default value is an empty string. Thus, the domain widget is selecting everything but once it is sent back to the server, since it's an empty string it won't get through the `if` condition. It was even weirder when you add something to the domain and remove it, you then visually come back to where you were at the beginning but the domain got changed from '' to '[]' so in this case it will generate the coupon for every partner. == More like improvements == 1. `action_cancel` and `action_confirm` would handle correctly the states of the applied and generated coupons (valid, consumed, expired..). But `action_draft` would not handle the states at all. 2. The generated coupon on a SO are now sent by email only when the SO is validated. This is part of the fix n°2 on coupon being generated multiple times and sent by email multiple times. The coupon sent by email was not usable anyway since it would error saying the SO that generated the coupon was not validated. 3. Discount line limited to a max amount (set in the program) will mention on the line description that the discount is limited. This will avoid cases where you don't understand why 50% discount of $2000 SO shows $40. 4. If you archive a program and there is still some SO containing this program's reward line, these line will be 'floating free' and will never be updated or removed. Indeed, the `_remove_invalid_reward_lines` is basically checking which programs could be applied on the order and which one are applied on the order. If a program is applied on the order but is not supposed to, his reward line will be removed. Of course, since the program got archived, it won't get retrieve and the line will never get removed. Even if the case of a free iPad: if you buy an iPad and you removed the paid iPad from the SO. The free iPad will remain forever. Note: We can't remove these discount line directly when archiving a program because there might be quotations that we don't want to modify since it is waiting for customer confirmation. Improved behavior: - Archiving a program archives its related reward product but unarchiving the program does not unarchived the product. task-1866977 opw-1832967 opw-1857843 Closes #2224 Comes with https://github.com/odoo/odoo/pull/24971 --- models/sale_order.py | 10 +++++----- tests/test_free_shipping_reward.py | 7 +++++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/models/sale_order.py b/models/sale_order.py index e0f91648823..0d11cd8c159 100644 --- a/models/sale_order.py +++ b/models/sale_order.py @@ -13,14 +13,14 @@ class SaleOrder(models.Model): lines = self.order_line.filtered(lambda line: line.is_delivery or line.product_id in free_delivery_product) return lines + super(SaleOrder, self)._get_no_effect_on_threshold_lines() - def _get_order_lines_tax_included_amount(self): + def _get_paid_order_lines(self): """ Returns the taxes included sale order total amount without the rewards amount""" free_reward_product = self.env['sale.coupon.program'].search([('reward_type', '=', 'product')]).mapped('discount_line_product_id') - return sum([x.price_total for x in self.order_line.filtered(lambda x: not (x.is_reward_line or x.is_delivery) or x.product_id in free_reward_product)]) + return self.order_line.filtered(lambda x: not (x.is_reward_line or x.is_delivery) or x.product_id in free_reward_product) def _get_reward_line_values(self, program): if program.reward_type == 'free_shipping': - return self._get_reward_values_free_shipping(program) + return [self._get_reward_values_free_shipping(program)] else: return super(SaleOrder, self)._get_reward_line_values(program) @@ -40,9 +40,9 @@ class SaleOrder(models.Model): 'tax_id': [(4, tax.id, False) for tax in taxes], } - def _get_lines_unit_prices(self): + def _get_cheapest_line(self): # Unit prices tax included - return [x.price_total / x.product_uom_qty for x in self.order_line.filtered(lambda x: not x.is_delivery and not x.is_reward_line)] + return min(self.order_line.filtered(lambda x: not x.is_reward_line and not x.is_delivery and x.price_unit > 0), key=lambda x: x['price_unit']) class SalesOrderLine(models.Model): _inherit = "sale.order.line" diff --git a/tests/test_free_shipping_reward.py b/tests/test_free_shipping_reward.py index 5c8cce3d1a4..175a06d0c76 100644 --- a/tests/test_free_shipping_reward.py +++ b/tests/test_free_shipping_reward.py @@ -120,6 +120,9 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): self.assertEqual(len(order.order_line.ids), 3, "We should get the delivery line but not the free delivery since we are below 872.73$ with the 10% discount") p_minimum_threshold_free_delivery.sequence = 10 + (order.order_line - sol1).unlink() + order.carrier_id = self.env['delivery.carrier'].search([])[1] + order.delivery_set() order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 4, "We should get both promotion line since the free delivery will be applied first and won't change the SO total") @@ -193,5 +196,5 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): }) p_specific_product.discount_apply_on = 'cheapest_product' order.recompute_coupon_lines() - # 872.73 - (20% of 1 iPad) = 872.73 - 64 = 808.73 - self.assertEqual(order.amount_untaxed, 808.73, "One iPad should be discounted by 20%") + # 872.73 - (20% of 1 iPad) = 872.73 - 58.18 = 814.55 + self.assertEqual(order.amount_untaxed, 814.55, "One iPad should be discounted by 20%") From 9ec4ad31a9711cfcf1333830efd6b050e79a7ee3 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Wed, 16 May 2018 15:19:14 +0200 Subject: [PATCH 15/18] [FIX] sale_coupon: delete 'sibling' lines when deleting a reward line A discount on products with different taxes will result in as many discount lines as there is different taxes. Before this commit, removing one of these lines would not do anything else. Then, recomputing the coupon would regenerate/add back this line. On e-commerce, it means you would never be able to remove the program by deleting its reward lines if there are multiple taxes (and so multiple lines) since it would `recompute_coupon_lines` right away, adding it back directly. Now, this commit will ensure that removing one of these lines will also remove the other ones coming from this program on other taxes. It means removing a regular product will remove it's reward line that will also now remove the other program's discount line on other taxes and ultimately will remove the program from the SO since all the program reward lines got removed. (It won't affect auto applied program since it will be regenerated afterward) task-1866977 Related to #2224 --- models/sale_order.py | 1 + 1 file changed, 1 insertion(+) diff --git a/models/sale_order.py b/models/sale_order.py index 0d11cd8c159..e4002fa9abf 100644 --- a/models/sale_order.py +++ b/models/sale_order.py @@ -50,6 +50,7 @@ class SalesOrderLine(models.Model): def unlink(self): # Due to delivery_set and delivery_unset methods that are called everywhere, don't unlink # reward lines if it's a free shipping + self = self.exists() orders = self.mapped('order_id') applied_programs = orders.mapped('no_code_promo_program_ids') + \ orders.mapped('code_promo_program_id') + \ From 1c5b28fbdd09baa80d6fee87d995753750f646f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Theys?= Date: Tue, 18 Dec 2018 16:15:34 +0000 Subject: [PATCH 16/18] [FIX] sale_coupon(_delivery): fix coupon test with product configurator The copy() is unnecessary, and it is introducing an issue because product.template.attribute.value are not created correctly for copied templates. The test can be done on the original data, the transaction is going to be reverted either way. closes odoo/enterprise#3301 --- tests/test_free_shipping_reward.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_free_shipping_reward.py b/tests/test_free_shipping_reward.py index d2639a047ba..a21027ca00f 100644 --- a/tests/test_free_shipping_reward.py +++ b/tests/test_free_shipping_reward.py @@ -8,7 +8,7 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): def setUp(self): super(TestSaleCouponProgramRules, self).setUp() - self.iPadMini = self.env.ref('product.product_product_6').copy() + self.iPadMini = self.env.ref('product.product_product_6') # Test a free shipping reward + some expected behavior # (automatic line addition or removal) From 3989661c1fd4714f5be589d605f56ce99d09bf53 Mon Sep 17 00:00:00 2001 From: William Henrotin Date: Fri, 8 Feb 2019 15:23:51 +0000 Subject: [PATCH 17/18] [REF] delivery: new delivery carrier wizard Adapt external carriers tests by using the new delivery wizard in sale order. Task : 1908654 --- tests/test_free_shipping_reward.py | 42 ++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/tests/test_free_shipping_reward.py b/tests/test_free_shipping_reward.py index a21027ca00f..1f9ba57afa6 100644 --- a/tests/test_free_shipping_reward.py +++ b/tests/test_free_shipping_reward.py @@ -2,6 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from openerp.addons.sale_coupon.tests.common import TestSaleCouponCommon +from odoo.tests import Form class TestSaleCouponProgramRules(TestSaleCouponCommon): @@ -42,9 +43,13 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 1) - order.carrier_id = self.env['delivery.carrier'].search([])[1] - order.get_delivery_price() - order.set_delivery_line() + # I add delivery cost in Sales order + delivery_wizard = Form(self.env['choose.delivery.carrier'].with_context({ + 'default_order_id': order.id, + 'default_carrier_id': self.env['delivery.carrier'].search([])[1] + })) + choose_delivery_carrier = delivery_wizard.save() + choose_delivery_carrier.button_confirm() order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 2) @@ -116,17 +121,27 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 2, "We should get the 10% discount line since we bought 872.73$") order.carrier_id = self.env['delivery.carrier'].search([])[1] - order.get_delivery_price() - order.set_delivery_line() + + # I add delivery cost in Sales order + delivery_wizard = Form(self.env['choose.delivery.carrier'].with_context({ + 'default_order_id': order.id, + 'default_carrier_id': self.env['delivery.carrier'].search([])[1] + })) + choose_delivery_carrier = delivery_wizard.save() + choose_delivery_carrier.button_confirm() order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 3, "We should get the delivery line but not the free delivery since we are below 872.73$ with the 10% discount") p_minimum_threshold_free_delivery.sequence = 10 (order.order_line - sol1).unlink() - order.carrier_id = self.env['delivery.carrier'].search([])[1] - order.get_delivery_price() - order.set_delivery_line() + # I add delivery cost in Sales order + delivery_wizard = Form(self.env['choose.delivery.carrier'].with_context({ + 'default_order_id': order.id, + 'default_carrier_id': self.env['delivery.carrier'].search([])[1] + })) + choose_delivery_carrier = delivery_wizard.save() + choose_delivery_carrier.button_confirm() order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 4, "We should get both promotion line since the free delivery will be applied first and won't change the SO total") @@ -167,9 +182,14 @@ class TestSaleCouponProgramRules(TestSaleCouponCommon): # it was wrongly done before. carrier = self.env.ref('delivery.delivery_carrier') carrier.product_id.taxes_id = tax_15pc_excl - order.carrier_id = carrier - order.get_delivery_price() - order.set_delivery_line() + + # I add delivery cost in Sales order + delivery_wizard = Form(self.env['choose.delivery.carrier'].with_context({ + 'default_order_id': order.id, + 'default_carrier_id': carrier.id + })) + choose_delivery_carrier = delivery_wizard.save() + choose_delivery_carrier.button_confirm() order.recompute_coupon_lines() self.assertEqual(len(order.order_line.ids), 2) self.assertEqual(order.reward_amount, 0) From 8b7a7179207132175f0cb4e2c0a283b7fb06dfc7 Mon Sep 17 00:00:00 2001 From: Jeremy Kersten Date: Mon, 1 Apr 2019 15:03:02 +0200 Subject: [PATCH 18/18] [MRG] sale_coupon_delivery: prepare move to community repo --- __init__.py => addons/sale_coupon_delivery/__init__.py | 0 __manifest__.py => addons/sale_coupon_delivery/__manifest__.py | 0 {models => addons/sale_coupon_delivery/models}/__init__.py | 0 {models => addons/sale_coupon_delivery/models}/sale_coupon.py | 0 .../sale_coupon_delivery/models}/sale_coupon_program.py | 0 .../sale_coupon_delivery/models}/sale_coupon_reward.py | 0 {models => addons/sale_coupon_delivery/models}/sale_order.py | 0 {tests => addons/sale_coupon_delivery/tests}/__init__.py | 0 .../sale_coupon_delivery/tests}/test_free_shipping_reward.py | 3 ++- 9 files changed, 2 insertions(+), 1 deletion(-) rename __init__.py => addons/sale_coupon_delivery/__init__.py (100%) rename __manifest__.py => addons/sale_coupon_delivery/__manifest__.py (100%) rename {models => addons/sale_coupon_delivery/models}/__init__.py (100%) rename {models => addons/sale_coupon_delivery/models}/sale_coupon.py (100%) rename {models => addons/sale_coupon_delivery/models}/sale_coupon_program.py (100%) rename {models => addons/sale_coupon_delivery/models}/sale_coupon_reward.py (100%) rename {models => addons/sale_coupon_delivery/models}/sale_order.py (100%) rename {tests => addons/sale_coupon_delivery/tests}/__init__.py (100%) rename {tests => addons/sale_coupon_delivery/tests}/test_free_shipping_reward.py (99%) diff --git a/__init__.py b/addons/sale_coupon_delivery/__init__.py similarity index 100% rename from __init__.py rename to addons/sale_coupon_delivery/__init__.py diff --git a/__manifest__.py b/addons/sale_coupon_delivery/__manifest__.py similarity index 100% rename from __manifest__.py rename to addons/sale_coupon_delivery/__manifest__.py diff --git a/models/__init__.py b/addons/sale_coupon_delivery/models/__init__.py similarity index 100% rename from models/__init__.py rename to addons/sale_coupon_delivery/models/__init__.py diff --git a/models/sale_coupon.py b/addons/sale_coupon_delivery/models/sale_coupon.py similarity index 100% rename from models/sale_coupon.py rename to addons/sale_coupon_delivery/models/sale_coupon.py diff --git a/models/sale_coupon_program.py b/addons/sale_coupon_delivery/models/sale_coupon_program.py similarity index 100% rename from models/sale_coupon_program.py rename to addons/sale_coupon_delivery/models/sale_coupon_program.py diff --git a/models/sale_coupon_reward.py b/addons/sale_coupon_delivery/models/sale_coupon_reward.py similarity index 100% rename from models/sale_coupon_reward.py rename to addons/sale_coupon_delivery/models/sale_coupon_reward.py diff --git a/models/sale_order.py b/addons/sale_coupon_delivery/models/sale_order.py similarity index 100% rename from models/sale_order.py rename to addons/sale_coupon_delivery/models/sale_order.py diff --git a/tests/__init__.py b/addons/sale_coupon_delivery/tests/__init__.py similarity index 100% rename from tests/__init__.py rename to addons/sale_coupon_delivery/tests/__init__.py diff --git a/tests/test_free_shipping_reward.py b/addons/sale_coupon_delivery/tests/test_free_shipping_reward.py similarity index 99% rename from tests/test_free_shipping_reward.py rename to addons/sale_coupon_delivery/tests/test_free_shipping_reward.py index 1f9ba57afa6..a3bc762db9a 100644 --- a/tests/test_free_shipping_reward.py +++ b/addons/sale_coupon_delivery/tests/test_free_shipping_reward.py @@ -2,9 +2,10 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from openerp.addons.sale_coupon.tests.common import TestSaleCouponCommon -from odoo.tests import Form +from odoo.tests import Form, tagged +@tagged('post_install', '-at_install') class TestSaleCouponProgramRules(TestSaleCouponCommon): def setUp(self):