diff --git a/addons/website_sale/models/website.py b/addons/website_sale/models/website.py index 14365a77dd8..77bcedbfee5 100644 --- a/addons/website_sale/models/website.py +++ b/addons/website_sale/models/website.py @@ -60,35 +60,50 @@ class Website(models.Model): :param int order_pl: the current cart pricelist :returns: list of pricelist ids """ + def _check_show_visible(pl): + """ If `show_visible` is True, we will only show the pricelist if + one of this condition is met: + - The pricelist is `selectable`. + - The pricelist is either the currently used pricelist or the + current cart pricelist, we should consider it as available even if + it might not be website compliant (eg: it is not selectable anymore, + it is a backend pricelist, it is not active anymore..). + """ + return (not show_visible or pl.selectable or pl.id in (current_pl, order_pl)) + + # Note: 1. pricelists from all_pl are already website compliant (went through + # `_get_website_pricelists_domain`) + # 2. do not read `property_product_pricelist` here as `_get_pl_partner_order` + # is cached and the result of this method will be impacted by that field value. + # Pass it through `partner_pl` parameter instead to invalidate the cache. + + # If there is a GeoIP country, find a pricelist for it self.ensure_one() pricelists = self.env['product.pricelist'] if country_code: for cgroup in self.env['res.country.group'].search([('country_ids.code', '=', country_code)]): - for group_pricelists in cgroup.pricelist_ids: - if not show_visible or group_pricelists.selectable or group_pricelists.id in (current_pl, order_pl): - pricelists |= group_pricelists + pricelists |= cgroup.pricelist_ids.filtered( + lambda pl: pl._is_available_on_website(self.id) and _check_show_visible(pl) + ) - partner = self.env.user.partner_id.sudo(user=self.env.user) + # no GeoIP or no pricelist for this country + if not country_code or not pricelists: + pricelists |= all_pl.filtered(lambda pl: _check_show_visible(pl)) + + # if logged in, add partner pl (which is `property_product_pricelist`, might not be website compliant) is_public = self.user_id.id == self.env.user.id - if not is_public and (not pricelists or (partner_pl or partner.property_product_pricelist.id) != website_pl): - # `property_product_pricelist` is already multi-website compliant - pricelists |= partner.property_product_pricelist - - if not pricelists: # no pricelist for this country, or no GeoIP - pricelists |= all_pl.filtered(lambda pl: not show_visible or pl.selectable or pl.id in (current_pl, order_pl)) - if not show_visible and not country_code: - pricelists |= all_pl.filtered(lambda pl: pl.sudo().code) + if not is_public: + pricelists |= pricelists.browse(partner_pl).filtered(lambda pl: pl._is_available_on_website(self.id) and _check_show_visible(pl)) # This method is cached, must not return records! See also #8795 - return pricelists.filtered(lambda pl: pl._is_available_on_website(self.id)).ids + return pricelists.ids # DEPRECATED (Not used anymore) -> Remove me in master (saas12.3) def _get_pl(self, country_code, show_visible, website_pl, current_pl, all_pl): pl_ids = self._get_pl_partner_order(country_code, show_visible, website_pl, current_pl, all_pl) return self.env['product.pricelist'].browse(pl_ids) - def get_pricelist_available(self, show_visible=False): - + def _get_pricelist_available(self, req, show_visible=False): """ Return the list of pricelists that can be used on website for the current user. Country restrictions will be detected with GeoIP (if installed). :param bool show_visible: if True, we don't display pricelist where selectable is False (Eg: Code promo) @@ -101,18 +116,21 @@ class Website(models.Model): else: # In the weird case we are coming from the backend (https://github.com/odoo/odoo/issues/20245) website = len(self) == 1 and self or self.search([], limit=1) - isocountry = request and request.session.geoip and request.session.geoip.get('country_code') or False + isocountry = req and req.session.geoip and req.session.geoip.get('country_code') or False partner = self.env.user.partner_id last_order_pl = partner.last_website_so_id.pricelist_id partner_pl = partner.sudo(user=self.env.user).property_product_pricelist pricelists = website._get_pl_partner_order(isocountry, show_visible, website.user_id.sudo().partner_id.property_product_pricelist.id, - request and request.session.get('website_sale_current_pl') or None, + req and req.session.get('website_sale_current_pl') or None, website.pricelist_ids, partner_pl=partner_pl and partner_pl.id or None, order_pl=last_order_pl and last_order_pl.id or None) return self.env['product.pricelist'].browse(pricelists) + def get_pricelist_available(self, show_visible=False): + return self._get_pricelist_available(request, show_visible) + def is_pricelist_available(self, pl_id): """ Return a boolean to specify if a specific pricelist can be manually set on the website. Warning: It check only if pricelist is in the 'selectable' pricelists or the current pricelist. diff --git a/addons/website_sale/tests/test_website_sale_pricelist.py b/addons/website_sale/tests/test_website_sale_pricelist.py index 31d66a094c1..eb3b9d5d1f6 100644 --- a/addons/website_sale/tests/test_website_sale_pricelist.py +++ b/addons/website_sale/tests/test_website_sale_pricelist.py @@ -6,6 +6,24 @@ except ImportError: from mock import patch from odoo.tests.common import HttpCase, TransactionCase +''' /!\/!\ +Calling `get_pricelist_available` after setting `property_product_pricelist` on +a partner will not work as expected. That field will change the output of +`get_pricelist_available` but modifying it will not invalidate the cache. +Thus, tests should not do: + + self.env.user.partner_id.property_product_pricelist = my_pricelist + pls = self.get_pricelist_available() + self.assertEqual(...) + self.env.user.partner_id.property_product_pricelist = another_pricelist + pls = self.get_pricelist_available() + self.assertEqual(...) + +as `_get_pl_partner_order` cache won't be invalidate between the calls, output +won't be the one expected and tests will actually not test anything. +Try to keep one call to `get_pricelist_available` by test method. +''' + class TestWebsitePriceList(TransactionCase): @@ -136,6 +154,176 @@ def simulate_frontend_context(self, website_id=1): self.addCleanup(patcher.stop) +class DotDict(dict): + """dot.notation access to dictionary attributes""" + def __getattr__(*args): + val = dict.get(*args) + return DotDict(val) if type(val) is dict else val + + +class TestWebsitePriceListAvailable(TransactionCase): + # This is enough to avoid a mock (request.session/website do not exist during test) + def get_pricelist_available(self, show_visible=False, website_id=1, country_code=None, website_sale_current_pl=None): + request = DotDict({ + 'website': self.env['website'].browse(website_id), + 'session': { + 'geoip': { + 'country_code': country_code, + }, + 'website_sale_current_pl': website_sale_current_pl, + }, + }) + return self.env['website']._get_pricelist_available(request, show_visible) + + def setUp(self): + super(TestWebsitePriceListAvailable, self).setUp() + Pricelist = self.env['product.pricelist'] + Website = self.env['website'] + + # Set up 2 websites + self.website = Website.browse(1) + self.website2 = Website.create({'name': 'Website 2'}) + + # Remove existing pricelists and create new ones + Pricelist.search([]).write({'active': False}) + self.backend_pl = Pricelist.create({ + 'name': 'Backend Pricelist', + 'website_id': False, + }) + self.generic_pl_select = Pricelist.create({ + 'name': 'Generic Selectable Pricelist', + 'selectable': True, + 'website_id': False, + }) + self.generic_pl_code = Pricelist.create({ + 'name': 'Generic Code Pricelist', + 'code': 'GENERICCODE', + 'website_id': False, + }) + self.generic_pl_code_select = Pricelist.create({ + 'name': 'Generic Code Selectable Pricelist', + 'code': 'GENERICCODESELECT', + 'selectable': True, + 'website_id': False, + }) + self.w1_pl = Pricelist.create({ + 'name': 'Website 1 Pricelist', + 'website_id': self.website.id, + }) + self.w1_pl_select = Pricelist.create({ + 'name': 'Website 1 Pricelist Selectable', + 'website_id': self.website.id, + 'selectable': True, + }) + self.w1_pl_code = Pricelist.create({ + 'name': 'Website 1 Pricelist Code', + 'website_id': self.website.id, + 'code': 'W1CODE', + }) + self.w1_pl_code_select = Pricelist.create({ + 'name': 'Website 1 Pricelist Code Selectable', + 'website_id': self.website.id, + 'code': 'W1CODESELECT', + 'selectable': True, + }) + self.w2_pl = Pricelist.create({ + 'name': 'Website 2 Pricelist', + 'website_id': self.website2.id, + }) + + simulate_frontend_context(self) + + def test_get_pricelist_available(self): + # all_pl = self.backend_pl + self.generic_pl_select + self.generic_pl_code + self.generic_pl_code_select + self.w1_pl + self.w1_pl_select + self.w1_pl_code + self.w1_pl_code_select + self.w2_pl + + # Test get all available pricelists + pls_to_return = self.generic_pl_select + self.generic_pl_code + self.generic_pl_code_select + self.w1_pl + self.w1_pl_select + self.w1_pl_code + self.w1_pl_code_select + pls = self.get_pricelist_available() + self.assertEqual(pls, pls_to_return, "Every pricelist having the correct website_id set or (no website_id but a code or selectable) should be returned") + + # Test get all available and visible pricelists + pls_to_return = self.generic_pl_select + self.generic_pl_code_select + self.w1_pl_select + self.w1_pl_code_select + pls = self.get_pricelist_available(show_visible=True) + self.assertEqual(pls, pls_to_return, "Only selectable pricelists website compliant (website_id False or current website) should be returned") + + def test_property_product_pricelist_for_inactive_partner(self): + # `_get_partner_pricelist_multi` should consider inactive users when searching for pricelists. + # Real case if for public user. His `property_product_pricelist` need to be set as it is passed + # through `_get_pl_partner_order` as the `website_pl` when searching for available pricelists + # for active users. + public_partner = self.env.ref('base.public_partner') + self.assertFalse(public_partner.active, "Ensure public partner is inactive (purpose of this test)") + pl = public_partner.property_product_pricelist + self.assertEqual(len(pl), 1, "Inactive partner should still get a `property_product_pricelist`") + + +class TestWebsitePriceListAvailableGeoIP(TestWebsitePriceListAvailable): + def setUp(self): + super(TestWebsitePriceListAvailableGeoIP, self).setUp() + # clean `property_product_pricelist` for partner for this test (clean setup) + self.env['ir.property'].search([('res_id', '=', 'res.partner,%s' % self.env.user.partner_id.id)]).unlink() + + # set different country groups on pricelists + c_EUR = self.env.ref('base.europe') + c_BENELUX = self.env.ref('website_sale.benelux') + self.BE = self.env.ref('base.be') + NL = self.env.ref('base.nl') + c_BE = self.env['res.country.group'].create({'name': 'Belgium', 'country_ids': [(6, 0, [self.BE.id])]}) + c_NL = self.env['res.country.group'].create({'name': 'Netherlands', 'country_ids': [(6, 0, [NL.id])]}) + + (self.backend_pl + self.generic_pl_select + self.generic_pl_code + self.w1_pl_select).write({'country_group_ids': [(6, 0, [c_BE.id])]}) + (self.generic_pl_code_select + self.w1_pl + self.w2_pl).write({'country_group_ids': [(6, 0, [c_BENELUX.id])]}) + (self.w1_pl_code).write({'country_group_ids': [(6, 0, [c_EUR.id])]}) + (self.w1_pl_code_select).write({'country_group_ids': [(6, 0, [c_NL.id])]}) + + # pricelist | selectable | website | code | country group | + # ----------------------------------------------------------------------| + # backend_pl | | | | BE | + # generic_pl_select | V | | | BE | + # generic_pl_code | | | V | BE | + # generic_pl_code_select | V | | V | BENELUX | + # w1_pl | | 1 | | BENELUX | + # w1_pl_select | V | 1 | | BE | + # w1_pl_code | | 1 | V | EUR | + # w1_pl_code_select | V | 1 | V | NL | + # w2_pl | | 2 | | BENELUX | + + # available pl for website 1 for GeoIP BE (anything except website 2, backend and NL) + self.website1_be_pl = self.generic_pl_select + self.generic_pl_code + self.w1_pl_select + self.generic_pl_code_select + self.w1_pl + self.w1_pl_code + + def test_get_pricelist_available_geoip(self): + # Test get all available pricelists with geoip and no partner pricelist (ir.property) + + # property_product_pricelist will also be returned in the available pricelists + self.website1_be_pl += self.env.user.partner_id.property_product_pricelist + + pls = self.get_pricelist_available(country_code=self.BE.code) + self.assertEqual(pls, self.website1_be_pl, "Only pricelists for BE and accessible on website should be returned, and the partner pl") + + def test_get_pricelist_available_geoip2(self): + # Test get all available pricelists with geoip and a partner pricelist (ir.property) not website compliant + self.env.user.partner_id.property_product_pricelist = self.backend_pl + pls = self.get_pricelist_available(country_code=self.BE.code) + self.assertEqual(pls, self.website1_be_pl, "Only pricelists for BE and accessible on website should be returned as partner pl is not website compliant") + + def test_get_pricelist_available_geoip3(self): + # Test get all available pricelists with geoip and a partner pricelist (ir.property) website compliant (but not geoip compliant) + self.env.user.partner_id.property_product_pricelist = self.w1_pl_code_select + self.website1_be_pl += self.env.user.partner_id.property_product_pricelist + pls = self.get_pricelist_available(country_code=self.BE.code) + self.assertEqual(pls, self.website1_be_pl, "Only pricelists for BE and accessible on website should be returned, plus the partner pricelist as it is website compliant") + + def test_get_pricelist_available_geoip4(self): + # Test get all available with geoip and visible pricelists + promo pl + pls_to_return = self.generic_pl_select + self.w1_pl_select + self.generic_pl_code_select + # property_product_pricelist will also be returned in the available pricelists + pls_to_return += self.env.user.partner_id.property_product_pricelist + + current_pl = self.w1_pl_code + pls = self.get_pricelist_available(country_code=self.BE.code, show_visible=True, website_sale_current_pl=current_pl.id) + self.assertEqual(pls, pls_to_return + current_pl, "Only pricelists for BE, accessible en website and selectable should be returned. It should also return the applied promo pl") + + class TestWebsitePriceListHttp(HttpCase): def test_get_pricelist_available_multi_company(self): ''' Test that the `property_product_pricelist` of `res.partner` is not