From 886dd355fb8846f48be2ddf0ef3c0d3e16981f22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Honor=C3=A9?= Date: Fri, 28 Jun 2019 14:20:59 +0200 Subject: [PATCH 1/3] [ADD][10.0] add delivery price simulation for cart --- .../models/__init__.py | 1 + .../models/delivery_carrier.py | 66 ++++++ shopinvader_delivery_carrier/services/cart.py | 48 +++- shopinvader_delivery_carrier/tests/common.py | 1 + .../tests/test_carrier.py | 205 ++++++++++++++++++ 5 files changed, 315 insertions(+), 6 deletions(-) create mode 100644 shopinvader_delivery_carrier/models/delivery_carrier.py diff --git a/shopinvader_delivery_carrier/models/__init__.py b/shopinvader_delivery_carrier/models/__init__.py index 00eb3a0564..98b91e7657 100644 --- a/shopinvader_delivery_carrier/models/__init__.py +++ b/shopinvader_delivery_carrier/models/__init__.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- from . import backend +from . import delivery_carrier from . import sale from . import shopinvader_notification from . import stock_picking diff --git a/shopinvader_delivery_carrier/models/delivery_carrier.py b/shopinvader_delivery_carrier/models/delivery_carrier.py new file mode 100644 index 0000000000..7f9b0b35a1 --- /dev/null +++ b/shopinvader_delivery_carrier/models/delivery_carrier.py @@ -0,0 +1,66 @@ +# -*- coding: utf-8 -*- +# Copyright 2019 ACSONE SA/NV () +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). +from contextlib import contextmanager + +from odoo import api, models + + +class DeliveryCarrier(models.Model): + _inherit = "delivery.carrier" + + @contextmanager + def _simulate_delivery_cost(self, partner): + """ + Change the env mode (draft) to avoid real update on the partner. + Then, restore the partner with previous values. + :param partner: res.partner recordset + :return: + """ + partner.read() + partner_values = partner._convert_to_write(partner._cache) + with partner.env.do_in_draft(): + yield + # Restore values + partner.update(partner_values) + + @api.model + def _load_country(self): + """ + Load the country from context + :return: res.country recordset + """ + country_id = self.env.context.get("delivery_force_country_id", 0) + return self.env["res.country"].browse(country_id) + + @api.model + def _load_zip_code(self): + """ + Load the zip code from context + :return: str + """ + return self.env.context.get("delivery_force_zip_code", "") + + @api.multi + def verify_carrier(self, contact): + """ + Inherit the function to force some values on the given contact + (only in cache). + :param contact: res.partner recordset + :return: False or self + """ + country = self._load_country() + zip_code = self._load_zip_code() + if country or zip_code: + with self._simulate_delivery_cost(contact): + # Edit country and zip + # Even if some info are not provided, we have to fill them + # Ex: if the zip code is not provided, we have to do the + # simulation with an empty zip code on the partner. Because his + # current zip could be related to another country and simulate + # a wrong price. + contact.update({"country_id": country.id, "zip": zip_code}) + result = super(DeliveryCarrier, self).verify_carrier(contact) + else: + result = super(DeliveryCarrier, self).verify_carrier(contact) + return result diff --git a/shopinvader_delivery_carrier/services/cart.py b/shopinvader_delivery_carrier/services/cart.py index acde64205d..eac782280d 100644 --- a/shopinvader_delivery_carrier/services/cart.py +++ b/shopinvader_delivery_carrier/services/cart.py @@ -3,6 +3,7 @@ # @author Sébastien BEAU # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). +from odoo.addons.base_rest.components.service import to_int from odoo.addons.component.core import Component from odoo.exceptions import UserError from odoo.tools.translate import _ @@ -11,14 +12,25 @@ class CartService(Component): _inherit = "shopinvader.cart.service" - def get_delivery_methods(self): + def get_delivery_methods(self, **params): """ - This service will return all possible delivery methods for the - current cart - :return: + This service will return all possible delivery methods for the + current cart (depending on country/zip) + The cart is not updated with the given country/zip. The change is done + only in memory. + :param params: dict + :return: dict """ cart = self._get() - return self._get_available_carrier(cart) + country = self._load_country(params) + zip_code = self._load_zip_code(params) + if country or zip_code: + cart = cart.with_context( + delivery_force_country_id=country.id, + delivery_force_zip_code=zip_code, + ) + result = self._get_available_carrier(cart) + return result def apply_delivery_method(self, **params): """ @@ -51,9 +63,33 @@ def _validator_apply_delivery_method(self): } def _validator_get_delivery_methods(self): - return {} + return { + "country_id": { + "coerce": to_int, + "required": False, + "type": "integer", + }, + "zip_code": {"required": False, "type": "string"}, + } # internal methods + def _load_country(self, params): + """ + Load the country from given params + :param params: dict + :return: res.country recordset + """ + country_id = params.pop("country_id", 0) + return self.env["res.country"].browse(country_id) + + def _load_zip_code(self, params): + """ + Load the country from given params + :param params: dict + :return: str + """ + return params.pop("zip_code", "") + def _add_item(self, cart, params): res = super(CartService, self)._add_item(cart, params) self._unset_carrier(cart) diff --git a/shopinvader_delivery_carrier/tests/common.py b/shopinvader_delivery_carrier/tests/common.py index e110b4d53e..0a73f00afc 100644 --- a/shopinvader_delivery_carrier/tests/common.py +++ b/shopinvader_delivery_carrier/tests/common.py @@ -12,6 +12,7 @@ def setUp(self): self.free_carrier = self.env.ref("delivery.free_delivery_carrier") self.poste_carrier = self.env.ref("delivery.delivery_carrier") self.product_1 = self.env.ref("product.product_product_4b") + self.precision = 2 def extract_cart(self, response): self.shopinvader_session["cart_id"] = response["set_session"][ diff --git a/shopinvader_delivery_carrier/tests/test_carrier.py b/shopinvader_delivery_carrier/tests/test_carrier.py index dfab861bf7..d084689dc6 100644 --- a/shopinvader_delivery_carrier/tests/test_carrier.py +++ b/shopinvader_delivery_carrier/tests/test_carrier.py @@ -71,3 +71,208 @@ def test_reset_carrier_on_delte_item(self): cart = self.delete_item(items[0]["id"]) self.assertEqual(cart["shipping"]["amount"]["total"], 0) self.assertFalse(cart["shipping"]["selected_carrier"]) + + def test_get_cart_price_by_country1(self): + """ + Check the service get_cart_price_by_country. + For this case, the cart doesn't have an existing delivery line. + :return: + """ + french_country = self.env.ref("base.fr") + belgium = self.env.ref("base.be") + self.backend.carrier_ids.write( + {"country_ids": [(6, False, [belgium.id, french_country.id])]} + ) + partner = self.service.partner + # Use the partner of the service + self.cart.write({"partner_id": partner.id}) + partner.write({"country_id": False}) + self.cart.write({"carrier_id": False}) + # Force load every fields + self.cart.read() + cart_values_before = self.cart._convert_to_write(self.cart._cache) + lines = {} + for line in self.cart.order_line: + line.read() + lines.update({line.id: line._convert_to_write(line._cache)}) + nb_lines_before = self.env["sale.order.line"].search_count([]) + self.service.shopinvader_session.update({"cart_id": self.cart.id}) + params = {"country_id": belgium.id} + result = self.service.dispatch("get_delivery_methods", params=params) + self.cart.read() + cart_values_after = self.cart._convert_to_write(self.cart._cache) + nb_lines_after = self.env["sale.order.line"].search_count([]) + self.assertDictEqual(cart_values_before, cart_values_after) + self.assertEquals(nb_lines_after, nb_lines_before) + + partner.write({"country_id": french_country.id}) + self.cart.write({"carrier_id": self.poste_carrier.id}) + # Force load every fields + self.cart.read() + cart_values_before = self.cart._convert_to_write(self.cart._cache) + lines = {} + for line in self.cart.order_line: + line.read() + lines.update({line.id: line._convert_to_write(line._cache)}) + nb_lines_before = self.env["sale.order.line"].search_count([]) + self.service.shopinvader_session.update({"cart_id": self.cart.id}) + params = {"country_id": belgium.id} + result = self.service.dispatch("get_delivery_methods", params=params) + self.assertEquals(self.cart.name, cart_values_before.get("name", "")) + self.cart.read() + cart_values_after = self.cart._convert_to_write(self.cart._cache) + self.assertDictEqual(cart_values_before, cart_values_after) + nb_lines_after = self.env["sale.order.line"].search_count([]) + self.assertEquals(nb_lines_after, nb_lines_before) + # Ensure lines still ok + self.assertEquals(len(lines), len(self.cart.order_line)) + for line_id, line_values in lines.iteritems(): + order_line = self.cart.order_line.filtered( + lambda l, lid=line_id: l.id == lid + ) + order_line.read() + self.assertDictEqual( + order_line._convert_to_write(order_line._cache), line_values + ) + self.assertEquals(self.cart.partner_id, partner) + self.assertEquals(french_country, partner.country_id) + self._check_carriers(result) + + def test_get_cart_price_by_country_anonymous(self): + """ + Check the service get_cart_price_by_country. + For this case, the cart doesn't have an existing delivery line. + :return: + """ + with self.work_on_services( + partner=self.backend.anonymous_partner_id, shopinvader_session={} + ) as work: + self.service = work.component(usage="cart") + self.test_get_cart_price_by_country1() + + def _check_carriers(self, result): + """ + Check carrier for current cart based on given result list of dict. + :param result: list of dict + :return: bool + """ + available_carriers = self.backend.carrier_ids.with_context( + order_id=self.cart.id + ).filtered(lambda c: c.available) + self.assertEquals(len(available_carriers), len(result)) + for carrier_result in result: + carrier = available_carriers.filtered( + lambda c: c.id == carrier_result.get("id") + ) + self.assertEquals(len(carrier), 1) + self.assertAlmostEquals( + carrier.price, + carrier_result.get("price"), + places=self.precision, + ) + self.assertEquals(carrier.name, carrier_result.get("name")) + return True + + def test_get_cart_price_by_country2(self): + """ + Check the service get_cart_price_by_country. + For this case, the cart have 1 delivery line set + :return: + """ + french_country = self.env.ref("base.fr") + belgium = self.env.ref("base.be") + partner = self.cart.partner_id + partner.write({"country_id": french_country.id}) + self.cart.write({"carrier_id": self.poste_carrier.id}) + self.cart.delivery_set() + # Force load every fields + self.cart.read() + cart_values_before = self.cart._convert_to_write(self.cart._cache) + cart_values_before.pop("order_line", None) + lines = {} + for line in self.cart.order_line: + line.read() + lines.update({line.id: line._convert_to_write(line._cache)}) + nb_lines_before = self.env["sale.order.line"].search_count([]) + self.service.shopinvader_session.update({"cart_id": self.cart.id}) + params = {"country_id": belgium.id} + result = self.service.dispatch("get_delivery_methods", params=params) + self.assertEquals(self.cart.name, cart_values_before.get("name", "")) + self.cart.read() + cart_values_after = self.cart._convert_to_write(self.cart._cache) + cart_values_after.pop("order_line", None) + self.assertDictEqual(cart_values_before, cart_values_after) + nb_lines_after = self.env["sale.order.line"].search_count([]) + self.assertEquals(nb_lines_after, nb_lines_before) + # Ensure lines still ok + self.assertEquals(len(lines), len(self.cart.order_line)) + for line_id, line_values in lines.iteritems(): + order_line = self.cart.order_line.filtered( + lambda l, lid=line_id: l.id == lid + ) + # Because delivery line has changed and the ID doesn't match + # anymore. + # But should still similar! + if not order_line: + order_line = self.cart.order_line.filtered( + lambda l: l.is_delivery + ) + order_line.read() + self.assertDictEqual( + order_line._convert_to_write(order_line._cache), line_values + ) + self.assertEquals(self.cart.partner_id, partner) + self.assertEquals(french_country, partner.country_id) + self._check_carriers(result) + + def test_get_cart_price_by_country3(self): + """ + Check the service get_cart_price_by_country. + For this case, the cart have 1 delivery line set + The cart doesn't have a carrier set. + :return: + """ + french_country = self.env.ref("base.fr") + belgium = self.env.ref("base.be") + partner = self.cart.partner_id + partner.write({"country_id": french_country.id}) + self.assertFalse(self.cart.carrier_id) + # Force load every fields + self.cart.read() + cart_values_before = self.cart._convert_to_write(self.cart._cache) + cart_values_before.pop("order_line", None) + lines = {} + for line in self.cart.order_line: + line.read() + lines.update({line.id: line._convert_to_write(line._cache)}) + nb_lines_before = self.env["sale.order.line"].search_count([]) + self.service.shopinvader_session.update({"cart_id": self.cart.id}) + params = {"country_id": belgium.id} + result = self.service.dispatch("get_delivery_methods", params=params) + self.assertEquals(self.cart.name, cart_values_before.get("name", "")) + self.cart.read() + cart_values_after = self.cart._convert_to_write(self.cart._cache) + cart_values_after.pop("order_line", None) + self.assertDictEqual(cart_values_before, cart_values_after) + nb_lines_after = self.env["sale.order.line"].search_count([]) + self.assertEquals(nb_lines_after, nb_lines_before) + # Ensure lines still ok + self.assertEquals(len(lines), len(self.cart.order_line)) + for line_id, line_values in lines.iteritems(): + order_line = self.cart.order_line.filtered( + lambda l, lid=line_id: l.id == lid + ) + # Because delivery line has changed and the ID doesn't match + # anymore. + # But should still similar! + if not order_line: + order_line = self.cart.order_line.filtered( + lambda l: l.is_delivery + ) + order_line.read() + self.assertDictEqual( + order_line._convert_to_write(order_line._cache), line_values + ) + self.assertEquals(self.cart.partner_id, partner) + self.assertEquals(french_country, partner.country_id) + self._check_carriers(result) From 17a673710dc03d1a382861e5b95bad467b34cb42 Mon Sep 17 00:00:00 2001 From: "Laurent Mignon (ACSONE)" Date: Fri, 2 Aug 2019 18:46:27 +0200 Subject: [PATCH 2/3] [IMP] shopinvader_delivery_carrier: Refactor code to provide delivery_carrier service The goal is to have a dedicated service to retrieve information of available delivery carriers --- .../controllers/main.py | 1 + .../services/__init__.py | 1 + shopinvader_delivery_carrier/services/cart.py | 62 +++++++++-- .../services/delivery_carrier.py | 104 ++++++++++++++++++ .../tests/__init__.py | 1 + shopinvader_delivery_carrier/tests/common.py | 19 +++- .../tests/test_carrier.py | 4 + .../tests/test_delivery_carrier.py | 57 ++++++++++ 8 files changed, 234 insertions(+), 15 deletions(-) create mode 100644 shopinvader_delivery_carrier/services/delivery_carrier.py create mode 100644 shopinvader_delivery_carrier/tests/test_delivery_carrier.py diff --git a/shopinvader_delivery_carrier/controllers/main.py b/shopinvader_delivery_carrier/controllers/main.py index a270b22fcb..3b837e4490 100644 --- a/shopinvader_delivery_carrier/controllers/main.py +++ b/shopinvader_delivery_carrier/controllers/main.py @@ -14,4 +14,5 @@ class InvaderController(main.InvaderController): @route(["/shopinvader/cart/get_delivery_methods"], methods=["GET"]) def get_delivery_methods(self, **params): + # deprecated!!! return self._process_method("cart", "get_delivery_methods", params) diff --git a/shopinvader_delivery_carrier/services/__init__.py b/shopinvader_delivery_carrier/services/__init__.py index 59e6a305ef..ad1c437439 100644 --- a/shopinvader_delivery_carrier/services/__init__.py +++ b/shopinvader_delivery_carrier/services/__init__.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- from . import abstract_sale from . import cart +from . import delivery_carrier from . import delivery diff --git a/shopinvader_delivery_carrier/services/cart.py b/shopinvader_delivery_carrier/services/cart.py index eac782280d..783c38538b 100644 --- a/shopinvader_delivery_carrier/services/cart.py +++ b/shopinvader_delivery_carrier/services/cart.py @@ -12,7 +12,37 @@ class CartService(Component): _inherit = "shopinvader.cart.service" + def set_carrier(self, **params): + """ + This service will set the given delivery method to the current + cart + :param params: The carrier_id to set + :return: + """ + cart = self._get() + if not cart: + raise UserError(_("There is not cart")) + else: + self._set_carrier(cart, params["carrier_id"]) + return self._to_json(cart) + def get_delivery_methods(self, **params): + def set_carrier(self, **params): + """ + This service will set the given delivery method to the current + cart + :param params: The carrier_id to set + :return: + """ + cart = self._get() + if not cart: + raise UserError(_("There is not cart")) + else: + self._set_carrier(cart, params["carrier_id"]) + return self._to_json(cart) + + # DEPRECATED METHODS # + def get_delivery_methods(self): """ This service will return all possible delivery methods for the current cart (depending on country/zip) @@ -20,6 +50,12 @@ def get_delivery_methods(self, **params): only in memory. :param params: dict :return: dict + !!!!DEPRECATED!!!!! Uses delivery_carrier.search + + This service will return all possible delivery methods for the + current cart + + :return: """ cart = self._get() country = self._load_country(params) @@ -31,20 +67,20 @@ def get_delivery_methods(self, **params): ) result = self._get_available_carrier(cart) return result + return self.component("delivery_carrier").search( + target="current_cart" + )["rows"] def apply_delivery_method(self, **params): """ + !!!!DEPRECATED!!!!! Uses set_carrier + This service will apply the given delivery method to the current cart :param params: Dict containing delivery method to apply :return: """ - cart = self._get() - if not cart: - raise UserError(_("There is not cart")) - else: - self._set_carrier(cart, params["carrier"]["id"]) - return self._to_json(cart) + return self.set_carrier(carrier_id=params["carrier"]["id"]) # Validator def _validator_apply_delivery_method(self): @@ -72,6 +108,16 @@ def _validator_get_delivery_methods(self): "zip_code": {"required": False, "type": "string"}, } + def _validator_set_carrier(self): + return { + "carrier_id": { + "coerce": int, + "nullable": True, + "required": True, + "type": "integer", + } + } + # internal methods def _load_country(self, params): """ @@ -106,9 +152,7 @@ def _delete_item(self, cart, params): return res def _set_carrier(self, cart, carrier_id): - if carrier_id not in [ - x["id"] for x in self._get_available_carrier(cart) - ]: + if carrier_id not in [x["id"] for x in cart._get_available_carrier()]: raise UserError( _("This delivery method is not available for you order") ) diff --git a/shopinvader_delivery_carrier/services/delivery_carrier.py b/shopinvader_delivery_carrier/services/delivery_carrier.py new file mode 100644 index 0000000000..cf23279ff2 --- /dev/null +++ b/shopinvader_delivery_carrier/services/delivery_carrier.py @@ -0,0 +1,104 @@ +# -*- coding: utf-8 -*- +# Copyright 2017 Akretion (http://www.akretion.com). +# @author Sébastien BEAU +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). +# pylint: disable=consider-merging-classes-inherited,method-required-super + +from openerp.addons.component.core import Component + + +class DeliveryCarrierService(Component): + _inherit = "base.shopinvader.service" + _name = "shopinvader.delivery.carrier.service" + _usage = "delivery_carrier" + _description = """ + This service allows you to retrieve the informations of available + delivery carriers. + """ + + # Public services: + + def search(self, **params): + """ + Returns the list of available carriers + + If the target params == current_cart, the list will be limited to the + carriers applying to the current cart and a price will be filled + into the response otherwise the price is not relevant (always 0). + + The field type is a technical field only use inform if the carrier + provides some specialized functionalities + """ + delivery_carriers = self._search(**params) + return { + "count": len(delivery_carriers), + "rows": [self._prepare_carrier(dc) for dc in delivery_carriers], + } + + # Validators + + def _validator_search(self): + return { + "target": { + "type": "string", + "required": False, + "allowed": ["current_cart"], + } + } + + def _validator_return_search(self): + return { + "count": {"type": "integer", "required": True}, + "rows": { + "type": "list", + "required": True, + "schema": { + "type": "dict", + "schema": { + "id": {"type": "integer", "required": True}, + "name": { + "type": "string", + "required": False, + "nullable": True, + }, + "description": { + "type": "string", + "required": False, + "nullable": True, + }, + "price": {"type": "float", "required": False}, + "type": { + "type": "string", + "required": False, + "allowed": self.allowed_carrier_types, + "nullable": True, + }, + }, + }, + }, + } + + # Services implementation + + def _search(self, **params): + """ + Search for delively carriers + :param params: see _validator_search + :return: a list of delivery.carriers + """ + if params.get("target") == "current_cart": + return self.component(usage="cart")._get()._get_available_carrier() + return self.shopinvader_backend.carrier_ids + + def _prepare_carrier(self, carrier): + res = carrier.jsonify(self._json_parser_carrier)[0] + res["type"] = None + return res + + @property + def allowed_carrier_types(self): + return [] + + @property + def _json_parser_carrier(self): + return ["id", "name", "description", "price"] diff --git a/shopinvader_delivery_carrier/tests/__init__.py b/shopinvader_delivery_carrier/tests/__init__.py index a0c6873cdf..ac8c6c5094 100644 --- a/shopinvader_delivery_carrier/tests/__init__.py +++ b/shopinvader_delivery_carrier/tests/__init__.py @@ -2,4 +2,5 @@ from . import test_carrier from . import test_notification +from . import test_delivery_carrier from . import test_delivery_service diff --git a/shopinvader_delivery_carrier/tests/common.py b/shopinvader_delivery_carrier/tests/common.py index 0a73f00afc..2e9d021bc8 100644 --- a/shopinvader_delivery_carrier/tests/common.py +++ b/shopinvader_delivery_carrier/tests/common.py @@ -7,12 +7,12 @@ class CommonCarrierCase(CommonConnectedCartCase): - def setUp(self): - super(CommonCarrierCase, self).setUp() - self.free_carrier = self.env.ref("delivery.free_delivery_carrier") - self.poste_carrier = self.env.ref("delivery.delivery_carrier") - self.product_1 = self.env.ref("product.product_product_4b") - self.precision = 2 + @classmethod + def setUpClass(cls): + super(CommonCarrierCase, cls).setUpClass() + cls.free_carrier = cls.env.ref("delivery.free_delivery_carrier") + cls.poste_carrier = cls.env.ref("delivery.delivery_carrier") + cls.product_1 = cls.env.ref("product.product_product_4b") def extract_cart(self, response): self.shopinvader_session["cart_id"] = response["set_session"][ @@ -41,6 +41,13 @@ def delete_item(self, item_id): ) def _set_carrier(self, carrier): + response = self.service.dispatch( + "set_carrier", params={"carrier_id": carrier.id} + ) + self.assertEqual(self.cart.carrier_id.id, carrier.id) + return response["data"] + + def _apply_delivery_method(self, carrier): response = self.service.dispatch( "apply_delivery_method", params={"carrier": {"id": carrier.id}} ) diff --git a/shopinvader_delivery_carrier/tests/test_carrier.py b/shopinvader_delivery_carrier/tests/test_carrier.py index d084689dc6..719678cc75 100644 --- a/shopinvader_delivery_carrier/tests/test_carrier.py +++ b/shopinvader_delivery_carrier/tests/test_carrier.py @@ -11,6 +11,10 @@ def test_available_carriers(self): response = self.service.dispatch("get_delivery_methods") self.assertEqual(len(response), 2) + def test_deprecated_apply_delivery_method(self): + cart = self._apply_delivery_method(self.free_carrier) + self.assertEqual(cart["shipping"]["amount"]["total"], 0) + def test_setting_free_carrier(self): cart = self._set_carrier(self.free_carrier) self.assertEqual(cart["shipping"]["amount"]["total"], 0) diff --git a/shopinvader_delivery_carrier/tests/test_delivery_carrier.py b/shopinvader_delivery_carrier/tests/test_delivery_carrier.py new file mode 100644 index 0000000000..0e1fab3686 --- /dev/null +++ b/shopinvader_delivery_carrier/tests/test_delivery_carrier.py @@ -0,0 +1,57 @@ +# -*- coding: utf-8 -*- +# Copyright 2017 Akretion (http://www.akretion.com). +# @author Sébastien BEAU +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). +from .common import CommonCarrierCase + + +class TestDeliveryCarrier(CommonCarrierCase): + def setUp(self): + super(CommonCarrierCase, self).setUp() + self.carrier_service = self.service.component("delivery_carrier") + + def test_search_all(self): + res = self.carrier_service.search() + expected = { + "count": 2, + "rows": [ + { + "price": 0.0, + "description": None, + "id": self.free_carrier.id, + "name": self.free_carrier.name, + "type": None, + }, + { + "price": 0.0, + "description": None, + "id": self.poste_carrier.id, + "name": self.poste_carrier.name, + "type": None, + }, + ], + } + self.assertDictEqual(res, expected) + + def test_search_current_cart(self): + res = self.carrier_service.search(target="current_cart") + expected = { + "count": 2, + "rows": [ + { + "price": 0.0, + "description": None, + "id": self.free_carrier.id, + "name": self.free_carrier.name, + "type": None, + }, + { + "price": 20.0, + "description": None, + "id": self.poste_carrier.id, + "name": self.poste_carrier.name, + "type": None, + }, + ], + } + self.assertDictEqual(res, expected) From 16950a09d0acc987c970a264a0dccafcefa58d10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Honor=C3=A9?= Date: Wed, 18 Sep 2019 15:46:41 +0200 Subject: [PATCH 3/3] Update after cherry-pick --- .../controllers/main.py | 4 -- .../models/delivery_carrier.py | 10 ++-- shopinvader_delivery_carrier/services/cart.py | 55 +++---------------- .../services/delivery_carrier.py | 42 ++++++++++++-- shopinvader_delivery_carrier/tests/common.py | 1 + .../tests/test_carrier.py | 35 ++++++------ .../tests/test_delivery_carrier.py | 8 +-- 7 files changed, 72 insertions(+), 83 deletions(-) diff --git a/shopinvader_delivery_carrier/controllers/main.py b/shopinvader_delivery_carrier/controllers/main.py index 3b837e4490..5d011da415 100644 --- a/shopinvader_delivery_carrier/controllers/main.py +++ b/shopinvader_delivery_carrier/controllers/main.py @@ -3,13 +3,9 @@ # Cédric Pigeon # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). -import logging - from odoo.addons.shopinvader.controllers import main from odoo.http import route -_logger = logging.getLogger(__name__) - class InvaderController(main.InvaderController): @route(["/shopinvader/cart/get_delivery_methods"], methods=["GET"]) diff --git a/shopinvader_delivery_carrier/models/delivery_carrier.py b/shopinvader_delivery_carrier/models/delivery_carrier.py index 7f9b0b35a1..0cd8453246 100644 --- a/shopinvader_delivery_carrier/models/delivery_carrier.py +++ b/shopinvader_delivery_carrier/models/delivery_carrier.py @@ -17,7 +17,7 @@ def _simulate_delivery_cost(self, partner): :param partner: res.partner recordset :return: """ - partner.read() + partner.read(["country_id", "zip"]) partner_values = partner._convert_to_write(partner._cache) with partner.env.do_in_draft(): yield @@ -25,7 +25,7 @@ def _simulate_delivery_cost(self, partner): partner.update(partner_values) @api.model - def _load_country(self): + def _get_country_from_context(self): """ Load the country from context :return: res.country recordset @@ -34,7 +34,7 @@ def _load_country(self): return self.env["res.country"].browse(country_id) @api.model - def _load_zip_code(self): + def _get_zip_from_context(self): """ Load the zip code from context :return: str @@ -49,8 +49,8 @@ def verify_carrier(self, contact): :param contact: res.partner recordset :return: False or self """ - country = self._load_country() - zip_code = self._load_zip_code() + country = self._get_country_from_context() + zip_code = self._get_zip_from_context() if country or zip_code: with self._simulate_delivery_cost(contact): # Edit country and zip diff --git a/shopinvader_delivery_carrier/services/cart.py b/shopinvader_delivery_carrier/services/cart.py index 783c38538b..17ab61d61a 100644 --- a/shopinvader_delivery_carrier/services/cart.py +++ b/shopinvader_delivery_carrier/services/cart.py @@ -2,31 +2,19 @@ # Copyright 2017 Akretion (http://www.akretion.com). # @author Sébastien BEAU # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). +import logging from odoo.addons.base_rest.components.service import to_int from odoo.addons.component.core import Component from odoo.exceptions import UserError from odoo.tools.translate import _ +_logger = logging.getLogger(__name__) + class CartService(Component): _inherit = "shopinvader.cart.service" - def set_carrier(self, **params): - """ - This service will set the given delivery method to the current - cart - :param params: The carrier_id to set - :return: - """ - cart = self._get() - if not cart: - raise UserError(_("There is not cart")) - else: - self._set_carrier(cart, params["carrier_id"]) - return self._to_json(cart) - - def get_delivery_methods(self, **params): def set_carrier(self, **params): """ This service will set the given delivery method to the current @@ -44,12 +32,6 @@ def set_carrier(self, **params): # DEPRECATED METHODS # def get_delivery_methods(self): """ - This service will return all possible delivery methods for the - current cart (depending on country/zip) - The cart is not updated with the given country/zip. The change is done - only in memory. - :param params: dict - :return: dict !!!!DEPRECATED!!!!! Uses delivery_carrier.search This service will return all possible delivery methods for the @@ -57,16 +39,11 @@ def get_delivery_methods(self): :return: """ - cart = self._get() - country = self._load_country(params) - zip_code = self._load_zip_code(params) - if country or zip_code: - cart = cart.with_context( - delivery_force_country_id=country.id, - delivery_force_zip_code=zip_code, - ) - result = self._get_available_carrier(cart) - return result + _logger.warning( + "DEPRECATED: You should use %s in service %s", + "search", + "delivery_carrier", + ) return self.component("delivery_carrier").search( target="current_cart" )["rows"] @@ -119,22 +96,6 @@ def _validator_set_carrier(self): } # internal methods - def _load_country(self, params): - """ - Load the country from given params - :param params: dict - :return: res.country recordset - """ - country_id = params.pop("country_id", 0) - return self.env["res.country"].browse(country_id) - - def _load_zip_code(self, params): - """ - Load the country from given params - :param params: dict - :return: str - """ - return params.pop("zip_code", "") def _add_item(self, cart, params): res = super(CartService, self)._add_item(cart, params) diff --git a/shopinvader_delivery_carrier/services/delivery_carrier.py b/shopinvader_delivery_carrier/services/delivery_carrier.py index cf23279ff2..98dae3bce4 100644 --- a/shopinvader_delivery_carrier/services/delivery_carrier.py +++ b/shopinvader_delivery_carrier/services/delivery_carrier.py @@ -4,7 +4,8 @@ # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). # pylint: disable=consider-merging-classes-inherited,method-required-super -from openerp.addons.component.core import Component +from odoo.addons.base_rest.components.service import to_int +from odoo.addons.component.core import Component class DeliveryCarrierService(Component): @@ -43,7 +44,13 @@ def _validator_search(self): "type": "string", "required": False, "allowed": ["current_cart"], - } + }, + "country_id": { + "coerce": to_int, + "required": False, + "type": "integer", + }, + "zip_code": {"required": False, "type": "string"}, } def _validator_return_search(self): @@ -82,12 +89,20 @@ def _validator_return_search(self): def _search(self, **params): """ - Search for delively carriers + Search for delivery carriers :param params: see _validator_search - :return: a list of delivery.carriers + :return: delivery.carriers recordset """ if params.get("target") == "current_cart": - return self.component(usage="cart")._get()._get_available_carrier() + cart = self.component(usage="cart")._get() + country = self._load_country(params) + zip_code = self._load_zip_code(params) + if country or zip_code: + cart = cart.with_context( + delivery_force_country_id=country.id, + delivery_force_zip_code=zip_code, + ) + return cart._get_available_carrier() return self.shopinvader_backend.carrier_ids def _prepare_carrier(self, carrier): @@ -95,6 +110,23 @@ def _prepare_carrier(self, carrier): res["type"] = None return res + def _load_country(self, params): + """ + Load the country from given params + :param params: dict + :return: res.country recordset + """ + country_id = params.pop("country_id", 0) + return self.env["res.country"].browse(country_id) + + def _load_zip_code(self, params): + """ + Load the country from given params + :param params: dict + :return: str + """ + return params.pop("zip_code", "") + @property def allowed_carrier_types(self): return [] diff --git a/shopinvader_delivery_carrier/tests/common.py b/shopinvader_delivery_carrier/tests/common.py index 2e9d021bc8..434bb6b952 100644 --- a/shopinvader_delivery_carrier/tests/common.py +++ b/shopinvader_delivery_carrier/tests/common.py @@ -13,6 +13,7 @@ def setUpClass(cls): cls.free_carrier = cls.env.ref("delivery.free_delivery_carrier") cls.poste_carrier = cls.env.ref("delivery.delivery_carrier") cls.product_1 = cls.env.ref("product.product_product_4b") + cls.precision = 2 def extract_cart(self, response): self.shopinvader_session["cart_id"] = response["set_session"][ diff --git a/shopinvader_delivery_carrier/tests/test_carrier.py b/shopinvader_delivery_carrier/tests/test_carrier.py index 719678cc75..64c70e34ec 100644 --- a/shopinvader_delivery_carrier/tests/test_carrier.py +++ b/shopinvader_delivery_carrier/tests/test_carrier.py @@ -7,6 +7,10 @@ class CarrierCase(CommonCarrierCase): + def setUp(self): + super(CarrierCase, self).setUp() + self.carrier_service = self.service.component("delivery_carrier") + def test_available_carriers(self): response = self.service.dispatch("get_delivery_methods") self.assertEqual(len(response), 2) @@ -101,8 +105,8 @@ def test_get_cart_price_by_country1(self): lines.update({line.id: line._convert_to_write(line._cache)}) nb_lines_before = self.env["sale.order.line"].search_count([]) self.service.shopinvader_session.update({"cart_id": self.cart.id}) - params = {"country_id": belgium.id} - result = self.service.dispatch("get_delivery_methods", params=params) + params = {"country_id": belgium.id, "target": "current_cart"} + result = self.carrier_service.dispatch("search", params=params) self.cart.read() cart_values_after = self.cart._convert_to_write(self.cart._cache) nb_lines_after = self.env["sale.order.line"].search_count([]) @@ -120,8 +124,8 @@ def test_get_cart_price_by_country1(self): lines.update({line.id: line._convert_to_write(line._cache)}) nb_lines_before = self.env["sale.order.line"].search_count([]) self.service.shopinvader_session.update({"cart_id": self.cart.id}) - params = {"country_id": belgium.id} - result = self.service.dispatch("get_delivery_methods", params=params) + params = {"country_id": belgium.id, "target": "current_cart"} + result = self.carrier_service.dispatch("search", params=params) self.assertEquals(self.cart.name, cart_values_before.get("name", "")) self.cart.read() cart_values_after = self.cart._convert_to_write(self.cart._cache) @@ -152,6 +156,7 @@ def test_get_cart_price_by_country_anonymous(self): partner=self.backend.anonymous_partner_id, shopinvader_session={} ) as work: self.service = work.component(usage="cart") + # Update with anonymous user self.test_get_cart_price_by_country1() def _check_carriers(self, result): @@ -163,18 +168,19 @@ def _check_carriers(self, result): available_carriers = self.backend.carrier_ids.with_context( order_id=self.cart.id ).filtered(lambda c: c.available) - self.assertEquals(len(available_carriers), len(result)) - for carrier_result in result: + carrier_rows = result.get("rows") + self.assertEquals(len(available_carriers), len(carrier_rows)) + for carrier_result in carrier_rows: carrier = available_carriers.filtered( lambda c: c.id == carrier_result.get("id") ) self.assertEquals(len(carrier), 1) + self.assertEquals(carrier.name, carrier_result.get("name")) self.assertAlmostEquals( carrier.price, carrier_result.get("price"), places=self.precision, ) - self.assertEquals(carrier.name, carrier_result.get("name")) return True def test_get_cart_price_by_country2(self): @@ -199,8 +205,8 @@ def test_get_cart_price_by_country2(self): lines.update({line.id: line._convert_to_write(line._cache)}) nb_lines_before = self.env["sale.order.line"].search_count([]) self.service.shopinvader_session.update({"cart_id": self.cart.id}) - params = {"country_id": belgium.id} - result = self.service.dispatch("get_delivery_methods", params=params) + params = {"country_id": belgium.id, "target": "current_cart"} + result = self.carrier_service.dispatch("search", params=params) self.assertEquals(self.cart.name, cart_values_before.get("name", "")) self.cart.read() cart_values_after = self.cart._convert_to_write(self.cart._cache) @@ -251,8 +257,8 @@ def test_get_cart_price_by_country3(self): lines.update({line.id: line._convert_to_write(line._cache)}) nb_lines_before = self.env["sale.order.line"].search_count([]) self.service.shopinvader_session.update({"cart_id": self.cart.id}) - params = {"country_id": belgium.id} - result = self.service.dispatch("get_delivery_methods", params=params) + params = {"country_id": belgium.id, "target": "current_cart"} + result = self.carrier_service.dispatch("search", params=params) self.assertEquals(self.cart.name, cart_values_before.get("name", "")) self.cart.read() cart_values_after = self.cart._convert_to_write(self.cart._cache) @@ -266,13 +272,6 @@ def test_get_cart_price_by_country3(self): order_line = self.cart.order_line.filtered( lambda l, lid=line_id: l.id == lid ) - # Because delivery line has changed and the ID doesn't match - # anymore. - # But should still similar! - if not order_line: - order_line = self.cart.order_line.filtered( - lambda l: l.is_delivery - ) order_line.read() self.assertDictEqual( order_line._convert_to_write(order_line._cache), line_values diff --git a/shopinvader_delivery_carrier/tests/test_delivery_carrier.py b/shopinvader_delivery_carrier/tests/test_delivery_carrier.py index 0e1fab3686..8f380e7ea0 100644 --- a/shopinvader_delivery_carrier/tests/test_delivery_carrier.py +++ b/shopinvader_delivery_carrier/tests/test_delivery_carrier.py @@ -17,14 +17,14 @@ def test_search_all(self): "rows": [ { "price": 0.0, - "description": None, + "description": self.free_carrier.description or None, "id": self.free_carrier.id, "name": self.free_carrier.name, "type": None, }, { "price": 0.0, - "description": None, + "description": self.poste_carrier.description or None, "id": self.poste_carrier.id, "name": self.poste_carrier.name, "type": None, @@ -40,14 +40,14 @@ def test_search_current_cart(self): "rows": [ { "price": 0.0, - "description": None, + "description": self.free_carrier.description or None, "id": self.free_carrier.id, "name": self.free_carrier.name, "type": None, }, { "price": 20.0, - "description": None, + "description": self.poste_carrier.description or None, "id": self.poste_carrier.id, "name": self.poste_carrier.name, "type": None,