From b0f0fd2ee3dcc3bbb24a123d4a1b38d17d0ce8c9 Mon Sep 17 00:00:00 2001 From: Yosua Nicolaus Date: Thu, 25 Jan 2024 05:43:18 +0100 Subject: [PATCH] [FIX] account,pos,purchase,sale: AccessError for branch user There are a lot of places in the codebase where insuficcient access to a company blocks a normal flow and raises AccessError. While this should normally never happen with the protections from company domain check, the new multi company branch/parent relationships suffers the most of this behavior; because a children company sometimes requires an information from their parent's field (e.g. currency_id) The correct behavior to do here is to let the children get the required data just for processing the flow in the compute (behind the scenes). Even though the children should normally have no access to read/write it on screen, the backend process should be able to run smoothly without blocking any flow. Granting a few temporary sudo() is necessary to ensure the flow runs as expected on some places. closes odoo/odoo#151562 Task-id: 3551305 Related: odoo/enterprise#55345 Signed-off-by: Brice Bartoletti (bib) Signed-off-by: Yosua Nicolaus (yoni) --- .../models/account_bank_statement_line.py | 2 +- addons/account/models/company.py | 2 +- addons/point_of_sale/models/pos_config.py | 4 +- addons/purchase/models/purchase_order.py | 7 +-- addons/sale/models/sale_order.py | 7 +-- addons/sale/models/sale_order_line.py | 2 +- addons/sale/tests/test_sale_order.py | 51 ++++++++++++++++++- 7 files changed, 63 insertions(+), 12 deletions(-) diff --git a/addons/account/models/account_bank_statement_line.py b/addons/account/models/account_bank_statement_line.py index e93a59eacdb..6fb3c307951 100644 --- a/addons/account/models/account_bank_statement_line.py +++ b/addons/account/models/account_bank_statement_line.py @@ -591,7 +591,7 @@ class AccountBankStatementLine(models.Model): self.journal_id.display_name, )) - company_currency = self.journal_id.company_id.currency_id + company_currency = self.journal_id.company_id.sudo().currency_id journal_currency = self.journal_id.currency_id or company_currency foreign_currency = self.foreign_currency_id or journal_currency or company_currency diff --git a/addons/account/models/company.py b/addons/account/models/company.py index 97e9c9a01c5..7d8fbc92745 100644 --- a/addons/account/models/company.py +++ b/addons/account/models/company.py @@ -261,7 +261,7 @@ class ResCompany(models.Model): @api.depends('parent_id.max_tax_lock_date') def _compute_max_tax_lock_date(self): for company in self: - company.max_tax_lock_date = max(company.tax_lock_date or date.min, company.parent_id.max_tax_lock_date or date.min) + company.max_tax_lock_date = max(company.tax_lock_date or date.min, company.parent_id.sudo().max_tax_lock_date or date.min) @api.model_create_multi def create(self, vals_list): diff --git a/addons/point_of_sale/models/pos_config.py b/addons/point_of_sale/models/pos_config.py index 4d096e88f88..34f16a615ca 100644 --- a/addons/point_of_sale/models/pos_config.py +++ b/addons/point_of_sale/models/pos_config.py @@ -203,9 +203,9 @@ class PosConfig(models.Model): def _compute_currency(self): for pos_config in self: if pos_config.journal_id: - pos_config.currency_id = pos_config.journal_id.currency_id.id or pos_config.journal_id.company_id.currency_id.id + pos_config.currency_id = pos_config.journal_id.currency_id.id or pos_config.journal_id.company_id.sudo().currency_id.id else: - pos_config.currency_id = pos_config.company_id.currency_id.id + pos_config.currency_id = pos_config.company_id.sudo().currency_id.id @api.depends('session_ids', 'session_ids.state') def _compute_current_session(self): diff --git a/addons/purchase/models/purchase_order.py b/addons/purchase/models/purchase_order.py index 218157e23da..3ab5279db49 100644 --- a/addons/purchase/models/purchase_order.py +++ b/addons/purchase/models/purchase_order.py @@ -152,12 +152,13 @@ class PurchaseOrder(models.Model): @api.constrains('company_id', 'order_line') def _check_order_line_company_id(self): for order in self: - companies = order.order_line.product_id.company_id - if companies and companies != order.company_id: + product_company = order.order_line.product_id.company_id + companies = product_company and product_company._accessible_branches() + if companies and order.company_id not in companies: bad_products = order.order_line.product_id.filtered(lambda p: p.company_id and p.company_id != order.company_id) raise ValidationError(_( "Your quotation contains products from company %(product_company)s whereas your quotation belongs to company %(quote_company)s. \n Please change the company of your quotation or remove the products from other companies (%(bad_products)s).", - product_company=', '.join(companies.mapped('display_name')), + product_company=', '.join(companies.sudo().mapped('display_name')), quote_company=order.company_id.display_name, bad_products=', '.join(bad_products.mapped('display_name')), )) diff --git a/addons/sale/models/sale_order.py b/addons/sale/models/sale_order.py index 2b287589424..b53d39684cb 100644 --- a/addons/sale/models/sale_order.py +++ b/addons/sale/models/sale_order.py @@ -673,12 +673,13 @@ class SaleOrder(models.Model): @api.constrains('company_id', 'order_line') def _check_order_line_company_id(self): for order in self: - companies = order.order_line.product_id.company_id - if companies and companies != order.company_id: + product_company = order.order_line.product_id.company_id + companies = product_company and product_company._accessible_branches() + if companies and order.company_id not in companies: bad_products = order.order_line.product_id.filtered(lambda p: p.company_id and p.company_id != order.company_id) raise ValidationError(_( "Your quotation contains products from company %(product_company)s whereas your quotation belongs to company %(quote_company)s. \n Please change the company of your quotation or remove the products from other companies (%(bad_products)s).", - product_company=', '.join(companies.mapped('display_name')), + product_company=', '.join(companies.sudo().mapped('display_name')), quote_company=order.company_id.display_name, bad_products=', '.join(bad_products.mapped('display_name')), )) diff --git a/addons/sale/models/sale_order_line.py b/addons/sale/models/sale_order_line.py index 3a26cdfbbb6..e5612a51570 100644 --- a/addons/sale/models/sale_order_line.py +++ b/addons/sale/models/sale_order_line.py @@ -81,7 +81,7 @@ class SaleOrderLine(models.Model): product_id = fields.Many2one( comodel_name='product.product', string="Product", - change_default=True, ondelete='restrict', check_company=True, index='btree_not_null', + change_default=True, ondelete='restrict', index='btree_not_null', domain="[('sale_ok', '=', True)]") product_template_id = fields.Many2one( string="Product Template", diff --git a/addons/sale/tests/test_sale_order.py b/addons/sale/tests/test_sale_order.py index 48da187fcce..15c1ed1fec4 100644 --- a/addons/sale/tests/test_sale_order.py +++ b/addons/sale/tests/test_sale_order.py @@ -595,7 +595,7 @@ class TestSalesTeam(SaleCommon): company_b = self.env['res.company'].create({'name': 'B'}) tax_group_a = self.env['account.tax.group'].create({'name': 'A', 'company_id': company_a.id}) tax_group_b = self.env['account.tax.group'].create({'name': 'B', 'company_id': company_b.id}) - country = self.env['res.country'].search([])[0] + country = self.env['res.country'].search([], limit=1) tax_a = self.env['account.tax'].create({ 'name': 'A', @@ -629,6 +629,55 @@ class TestSalesTeam(SaleCommon): with self.assertRaises(UserError): sol.tax_id = tax_b + def test_assign_tax_multi_company(self): + root_company = self.env['res.company'].create({'name': 'B0 company'}) + root_company.write({'child_ids': [ + Command.create({'name': 'B1 company'}), + Command.create({'name': 'B2 company'}), + ]}) + + country = self.env['res.country'].search([], limit=1) + basic_tax_group = self.env['account.tax.group'].create({'name': 'basic group', 'country_id': country.id}) + tax_b0 = self.env['account.tax'].create({ + 'name': 'B0 tax', + 'company_id': root_company.id, + 'amount': 10, + 'tax_group_id': basic_tax_group.id, + 'country_id': country.id, + }) + tax_b1 = self.env['account.tax'].create({ + 'name': 'B1 tax', + 'company_id': root_company.child_ids[0].id, + 'amount': 11, + 'tax_group_id': basic_tax_group.id, + 'country_id': country.id, + }) + tax_b2 = self.env['account.tax'].create({ + 'name': 'B2 tax', + 'company_id': root_company.child_ids[1].id, + 'amount': 20, + 'tax_group_id': basic_tax_group.id, + 'country_id': country.id, + }) + + sale_order = self.env['sale.order'].create({'partner_id': self.partner.id, 'company_id': root_company.child_ids[0].id}) + product = self.env['product.product'].create({'name': 'Product'}) + + # In sudo to simulate an user that have access to both companies. + sol_b1 = self.env['sale.order.line'].sudo().create({ + 'name': product.name, + 'product_id': product.id, + 'order_id': sale_order.id, + 'tax_id': tax_b1, + }) + + # should not raise anything + sol_b1.tax_id = tax_b0 + sol_b1.tax_id = tax_b1 + # should raise (b2 is not on the same branch lineage as b1) + with self.assertRaises(UserError): + sol_b1.tax_id = tax_b2 + def test_downpayment_amount_constraints(self): """Down payment amounts should be in the interval ]0, 1]."""