[REF] website_sale_aplicoop: price through website_sale instead of privately
Eskaera carried its own pricing path: a `website_sale_aplicoop.pricelist_id` setting resolved inside the Eskaera controllers, plus a local reimplementation of the tax and discount maths. The pricelist part meant /shop and Eskaera could quote different prices on the same website, and the maths part had drifted from the original it was copied from. Pricing is now the standard one. The setting is gone (a migration drops the parameter, which would otherwise linger looking like live configuration) and `_resolve_pricelist` is `request.website.pricelist_id`. Scoping a pricelist to a website is already core's job through `product.pricelist.website_id`, so running the plain shop on one website and the co-op on another needs no code here. Taxes are applied by `product.template._apply_taxes_to_price`, which fixes a real defect: it calls `_get_tax_included_unit_price_from_price` first, and this module did not. With a fiscal position remapping a tax-included tax, a product at 121 (100 + 21%) was displayed at 121 instead of 110, because the price was handed to the mapped tax as if it were already that tax's gross amount. The helper is a no-op without a remapping, so ordinary pricing is untouched. It also means the website's `show_line_subtotals_tax_selection` is respected rather than overridden with a hardcoded tax-included display. Listing a page now costs one `_compute_price_rule` call instead of one per product, which matters on the lazy-loading path. Two smaller things found on the way. `_get_product_price_rule` was being passed `target_currency=`, which is not one of its arguments: it fell into **kwargs and was ignored, so the conversion never happened. And `_compute_price_info` resolved `product.product_variant_ids[0]`, but it receives variants, so a multi-variant template was priced from its first variant rather than the one asked for. The delivery display price loses its hardcoded 5.74 fallback and reuses the same helpers as everything else. Kept local, because none of it is pricing: the /Kg and /L suffixes, the 0.1 quantity step for bulk goods, the base unit price and the supplier name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
3e4bd5e5db
commit
817ff31d39
10 changed files with 496 additions and 202 deletions
|
|
@ -23,3 +23,4 @@ from . import test_product_discovery # noqa: F401
|
|||
from . import test_date_edge_cases # noqa: F401
|
||||
from . import test_portal_routes # noqa: F401
|
||||
from . import test_price_with_taxes_included # noqa: F401
|
||||
from . import test_pricing_delegation # noqa: F401
|
||||
|
|
|
|||
|
|
@ -58,8 +58,14 @@ def _build_request_mock(env, payload=None, website=None):
|
|||
call pricing helpers).
|
||||
"""
|
||||
if website is None:
|
||||
# `pricelist_id` is what the pricing helpers read now that Eskaera
|
||||
# prices with the standard website pricelist. Ordered by id on
|
||||
# purpose: the multilang cases build an env whose language is not
|
||||
# installed, and the default order is by the translated `name`.
|
||||
website = SimpleNamespace(
|
||||
_get_current_pricelist=lambda: False,
|
||||
pricelist_id=env["product.pricelist"]
|
||||
.sudo()
|
||||
.search([], limit=1, order="id"),
|
||||
show_line_subtotals_tax_selection="tax_excluded",
|
||||
fiscal_position_id=False,
|
||||
company_id=False,
|
||||
|
|
|
|||
280
website_sale_aplicoop/tests/test_pricing_delegation.py
Normal file
280
website_sale_aplicoop/tests/test_pricing_delegation.py
Normal file
|
|
@ -0,0 +1,280 @@
|
|||
# Copyright 2025 Criptomart
|
||||
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl)
|
||||
|
||||
"""Pricing that is delegated to website_sale instead of reimplemented.
|
||||
|
||||
Three things this pins down:
|
||||
|
||||
- Eskaera prices with the standard website pricelist. The addon no longer
|
||||
carries a pricelist setting of its own, nor overrides how the website
|
||||
resolves one, so `/shop` and Eskaera cannot quote different prices.
|
||||
- taxes are applied by `product.template._apply_taxes_to_price`, so a fiscal
|
||||
position that remaps a tax-included tax rebases the price -- the step this
|
||||
addon used to skip.
|
||||
- a page of products costs one pricelist resolution, not one per product.
|
||||
"""
|
||||
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import patch
|
||||
|
||||
from odoo.tests import tagged
|
||||
from odoo.tests.common import TransactionCase
|
||||
|
||||
from ..controllers.website_sale import AplicoopWebsiteSale
|
||||
|
||||
REQUEST_PATCH_TARGET = (
|
||||
"odoo.addons.website_sale_aplicoop.controllers.website_sale.request"
|
||||
)
|
||||
|
||||
|
||||
class PricingDelegationCommon:
|
||||
"""A company, a website and a tax-included product to price."""
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self.controller = AplicoopWebsiteSale()
|
||||
self.company = self.env["res.company"].create({"name": "Pricing Deleg Co"})
|
||||
self.website = self.env.ref("website.default_website").sudo()
|
||||
self.website.write(
|
||||
{
|
||||
"company_id": self.company.id,
|
||||
"show_line_subtotals_tax_selection": "tax_included",
|
||||
}
|
||||
)
|
||||
self.tax_group = self.env["account.tax.group"].create(
|
||||
{"name": "IVA Deleg", "company_id": self.company.id}
|
||||
)
|
||||
self.country_es = self.env.ref("base.es")
|
||||
|
||||
def _create_pricelist(self, name, percent=None):
|
||||
vals = {"name": name, "company_id": self.company.id}
|
||||
if percent is not None:
|
||||
vals["item_ids"] = [
|
||||
(
|
||||
0,
|
||||
0,
|
||||
{
|
||||
"compute_price": "percentage",
|
||||
"percent_price": percent,
|
||||
"applied_on": "3_global",
|
||||
},
|
||||
)
|
||||
]
|
||||
return self.env["product.pricelist"].create(vals)
|
||||
|
||||
def _create_tax(self, name, amount, included=False):
|
||||
return self.env["account.tax"].create(
|
||||
{
|
||||
"name": name,
|
||||
"amount": amount,
|
||||
"amount_type": "percent",
|
||||
"type_tax_use": "sale",
|
||||
"price_include_override": (
|
||||
"tax_included" if included else "tax_excluded"
|
||||
),
|
||||
"company_id": self.company.id,
|
||||
"country_id": self.country_es.id,
|
||||
"tax_group_id": self.tax_group.id,
|
||||
}
|
||||
)
|
||||
|
||||
def _create_product(self, name, list_price, tax=None):
|
||||
return self.env["product.product"].create(
|
||||
{
|
||||
"name": name,
|
||||
"type": "consu",
|
||||
"list_price": list_price,
|
||||
"is_published": True,
|
||||
"sale_ok": True,
|
||||
"company_id": self.company.id,
|
||||
"taxes_id": [(6, 0, [tax.id])] if tax else False,
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
@tagged("post_install", "-at_install")
|
||||
class TestStandardPricelistResolution(PricingDelegationCommon, TransactionCase):
|
||||
"""Eskaera prices with the website pricelist, like the rest of the shop."""
|
||||
|
||||
def test_no_aplicoop_pricelist_setting_is_left(self):
|
||||
"""The old per-addon pricelist setting is gone for good.
|
||||
|
||||
It used to let Eskaera quote a different pricelist from `/shop`, which
|
||||
is exactly the drift this addon should not introduce.
|
||||
"""
|
||||
self.assertNotIn(
|
||||
"aplicoop_pricelist_id", self.env["res.config.settings"]._fields
|
||||
)
|
||||
|
||||
def test_website_resolution_is_left_untouched(self):
|
||||
"""`_get_current_pricelist` is core's; the addon does not override it."""
|
||||
website_cls = type(self.env["website"])
|
||||
|
||||
self.assertNotIn("_get_current_pricelist", website_cls.__dict__)
|
||||
self.assertEqual(
|
||||
self.website.pricelist_id, self.website._get_current_pricelist()
|
||||
)
|
||||
|
||||
def test_the_controller_quotes_the_website_pricelist(self):
|
||||
"""The Eskaera helper hands back exactly what the website resolved."""
|
||||
expected = self._create_pricelist("Website PL", percent=15.0)
|
||||
request_mock = SimpleNamespace(
|
||||
env=self.env,
|
||||
website=SimpleNamespace(pricelist_id=expected),
|
||||
)
|
||||
|
||||
with patch(REQUEST_PATCH_TARGET, request_mock):
|
||||
resolved = self.controller._resolve_pricelist()
|
||||
|
||||
self.assertEqual(resolved, expected)
|
||||
|
||||
|
||||
@tagged("post_install", "-at_install")
|
||||
class TestTaxIncludedFiscalPosition(PricingDelegationCommon, TransactionCase):
|
||||
"""A remapped tax-included tax must rebase the displayed price."""
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self.pricelist = self._create_pricelist("Plain PL")
|
||||
self.tax_21_incl = self._create_tax("IVA 21% incl", 21.0, included=True)
|
||||
self.tax_10_incl = self._create_tax("IVA 10% incl", 10.0, included=True)
|
||||
# 100 net + 21% already inside the price.
|
||||
self.product = self._create_product("Tax Incl Product", 121.0, self.tax_21_incl)
|
||||
|
||||
def _fiscal_position(self):
|
||||
position = self.env["account.fiscal.position"].create(
|
||||
{"name": "Remap 21->10", "company_id": self.company.id}
|
||||
)
|
||||
self.env["account.fiscal.position.tax"].create(
|
||||
{
|
||||
"position_id": position.id,
|
||||
"tax_src_id": self.tax_21_incl.id,
|
||||
"tax_dest_id": self.tax_10_incl.id,
|
||||
}
|
||||
)
|
||||
return position
|
||||
|
||||
def test_price_is_rebased_onto_the_mapped_tax(self):
|
||||
"""121 gross at 21% becomes 110 gross at 10%, not 121.
|
||||
|
||||
Without `_get_tax_included_unit_price_from_price` the 121 was handed
|
||||
straight to the mapped tax, which read it as already being the 10%
|
||||
gross price and displayed 121.
|
||||
"""
|
||||
self.website.fiscal_position_id = self._fiscal_position()
|
||||
|
||||
pricing = self.controller._get_pricing_info(
|
||||
self.product, self.pricelist, quantity=1.0
|
||||
)
|
||||
|
||||
self.assertAlmostEqual(pricing["price"], 110.0, places=2)
|
||||
|
||||
def test_price_is_untouched_without_a_fiscal_position(self):
|
||||
"""The rebasing is a no-op in the ordinary case."""
|
||||
self.website.fiscal_position_id = False
|
||||
|
||||
pricing = self.controller._get_pricing_info(
|
||||
self.product, self.pricelist, quantity=1.0
|
||||
)
|
||||
|
||||
self.assertAlmostEqual(pricing["price"], 121.0, places=2)
|
||||
|
||||
|
||||
@tagged("post_install", "-at_install")
|
||||
class TestPricingBatching(PricingDelegationCommon, TransactionCase):
|
||||
"""A product listing resolves the pricelist once for the whole page."""
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self.pricelist = self._create_pricelist("Batch PL", percent=10.0)
|
||||
self.tax = self._create_tax("IVA 21%", 21.0)
|
||||
self.products = self.env["product.product"]
|
||||
for index in range(5):
|
||||
self.products |= self._create_product(
|
||||
f"Batch Product {index}", 100.0 + index, self.tax
|
||||
)
|
||||
|
||||
def test_pricelist_is_resolved_once_for_the_page(self):
|
||||
"""One `_compute_price_rule` call, not one per product."""
|
||||
calls = []
|
||||
original = type(self.pricelist)._compute_price_rule
|
||||
|
||||
def counting_compute_price_rule(pricelist_self, products, *args, **kwargs):
|
||||
calls.append(len(products))
|
||||
return original(pricelist_self, products, *args, **kwargs)
|
||||
|
||||
self.patch(
|
||||
type(self.pricelist), "_compute_price_rule", counting_compute_price_rule
|
||||
)
|
||||
|
||||
self.controller._compute_price_info(self.products, self.pricelist)
|
||||
|
||||
self.assertEqual(
|
||||
calls,
|
||||
[len(self.products)],
|
||||
msg=f"Expected a single batched call, got {calls}",
|
||||
)
|
||||
|
||||
def test_batched_prices_match_the_single_product_helper(self):
|
||||
"""Batching must not change any number it produces."""
|
||||
batched = self.controller._compute_price_info(self.products, self.pricelist)
|
||||
|
||||
for product in self.products:
|
||||
single = self.controller._get_pricing_info(
|
||||
product, self.pricelist, quantity=1.0
|
||||
)
|
||||
self.assertAlmostEqual(
|
||||
batched[product.id]["price"], single["price"], places=2
|
||||
)
|
||||
self.assertAlmostEqual(
|
||||
batched[product.id]["price_unit"], single["price_unit"], places=2
|
||||
)
|
||||
|
||||
def test_prices_the_given_variant(self):
|
||||
"""A variant is priced as itself, not as its template's first one."""
|
||||
attribute = self.env["product.attribute"].create(
|
||||
{
|
||||
"name": "Size Deleg",
|
||||
"value_ids": [
|
||||
(0, 0, {"name": "Small"}),
|
||||
(0, 0, {"name": "Large"}),
|
||||
],
|
||||
}
|
||||
)
|
||||
template = self.env["product.template"].create(
|
||||
{
|
||||
"name": "Multi Variant Product",
|
||||
"type": "consu",
|
||||
"list_price": 100.0,
|
||||
"is_published": True,
|
||||
"sale_ok": True,
|
||||
"company_id": self.company.id,
|
||||
"attribute_line_ids": [
|
||||
(
|
||||
0,
|
||||
0,
|
||||
{
|
||||
"attribute_id": attribute.id,
|
||||
"value_ids": [(6, 0, attribute.value_ids.ids)],
|
||||
},
|
||||
)
|
||||
],
|
||||
}
|
||||
)
|
||||
small, large = template.product_variant_ids[0], template.product_variant_ids[1]
|
||||
# Make the two variants cost visibly different amounts.
|
||||
large.write({"list_price": 100.0})
|
||||
self.env["product.pricelist.item"].create(
|
||||
{
|
||||
"pricelist_id": self.pricelist.id,
|
||||
"applied_on": "0_product_variant",
|
||||
"product_id": large.id,
|
||||
"compute_price": "fixed",
|
||||
"fixed_price": 500.0,
|
||||
}
|
||||
)
|
||||
|
||||
priced = self.controller._compute_price_info(large, self.pricelist)
|
||||
|
||||
self.assertAlmostEqual(priced[large.id]["price_unit"], 500.0, places=2)
|
||||
self.assertNotEqual(large.id, small.id)
|
||||
|
|
@ -500,8 +500,8 @@ class TestPricingWithPricelist(TransactionCase):
|
|||
# If it raises, that's also acceptable behavior
|
||||
self.assertTrue(True, "Negative quantity properly rejected")
|
||||
|
||||
def test_pricing_helper_uses_config_pricelist_and_taxes(self):
|
||||
"""Pricing helper must apply configured pricelist and include taxes for display."""
|
||||
def test_pricing_helper_applies_pricelist_and_taxes(self):
|
||||
"""Pricing helper must apply the given pricelist and include taxes."""
|
||||
|
||||
website = self.env.ref("website.default_website").sudo()
|
||||
website.write(
|
||||
|
|
@ -529,17 +529,12 @@ class TestPricingWithPricelist(TransactionCase):
|
|||
}
|
||||
)
|
||||
|
||||
self.env["ir.config_parameter"].sudo().set_param(
|
||||
"website_sale_aplicoop.pricelist_id", pricelist_discount.id
|
||||
)
|
||||
|
||||
product = self.product_with_tax # 100€ + 21%
|
||||
|
||||
pricing = self.controller._get_pricing_info(
|
||||
product,
|
||||
pricelist_discount,
|
||||
quantity=1.0,
|
||||
partner=self.partner,
|
||||
)
|
||||
|
||||
# price_unit uses pricelist (10% discount)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue