From 4df15778e737ca24b9af261971958506c17898e6 Mon Sep 17 00:00:00 2001 From: Odoo Wired Date: Tue, 31 Mar 2026 09:48:48 +0200 Subject: [PATCH] account_usability: cache values and proper search on group --- account_usability/models/account_account.py | 111 +++++++++++++----- account_usability/models/account_group.py | 19 ++- account_usability/tests/test_account_group.py | 61 ++++++++++ 3 files changed, 158 insertions(+), 33 deletions(-) diff --git a/account_usability/models/account_account.py b/account_usability/models/account_account.py index a4bdd674e..b3e3a37d1 100644 --- a/account_usability/models/account_account.py +++ b/account_usability/models/account_account.py @@ -1,4 +1,7 @@ +from collections import defaultdict + from odoo import api, fields, models +from odoo.tools import SQL class Account(models.Model): @@ -7,47 +10,90 @@ class Account(models.Model): group_id = fields.Many2one(search="_search_group_id") def _search_group_id(self, operator, value): - if operator not in ("in", "="): - raise NotImplementedError - - # Browse groups because value can be an odoo.tools.query.Query - groups = self.env["account.group"].browse(value) - - if not groups: - return [("id", "=", 0)] - - query = """ - SELECT - a.id - FROM - account_account a - JOIN - account_group g - ON g.code_prefix_start <= LEFT( - (a.code_store::json ->> %(company_id)s), - char_length(g.code_prefix_start) - ) - AND g.code_prefix_end >= LEFT( - (a.code_store::json ->> %(company_id)s), - char_length(g.code_prefix_end) - ) - AND g.company_id = %(company_id)s - WHERE g.id IN %(group_ids)s + if operator in ("=", "in"): + # Browse groups because value can be an odoo.tools.query.Query + groups = self.env["account.group"].browse(value) + mapping = self._get_group_to_accounts_mapping() + account_ids = [] + for gid in groups.ids: + account_ids.extend(mapping.get(gid, [])) + return [("id", "in", account_ids)] + if operator in ("!=", "not in"): + groups = self.env["account.group"].browse(value) + mapping = self._get_group_to_accounts_mapping() + account_ids = [] + for gid in groups.ids: + account_ids.extend(mapping.get(gid, [])) + return [("id", "not in", account_ids)] + raise NotImplementedError( + f"Search on group_id with operator '{operator}' is not supported." + ) + + def _get_group_to_accounts_mapping(self): + """Build a mapping of group_id -> [account_ids]. + + Each account is assigned to its most specific matching group + (longest code prefix). The mapping is cached on self.env for + the duration of the current request to avoid repeated queries + when multiple search calls are made (e.g. MIS report rendering). """ - self.env.cr.execute( - query, + cache_attr = "_group_to_accounts_cache" + root_company_id = self.env.company.root_id.id + cached = getattr(self.env, cache_attr, None) + if cached and cached.get("company_id") == root_company_id: + return cached["mapping"] + company_key = str(root_company_id) + results = self.env.execute_query( + SQL( + """ + SELECT DISTINCT ON (aa.id) + aa.id AS account_id, + agroup.id AS group_id + FROM account_account aa + LEFT JOIN account_group agroup + ON agroup.code_prefix_start <= LEFT( + aa.code_store->>%(company_key)s, + char_length(agroup.code_prefix_start) + ) + AND agroup.code_prefix_end >= LEFT( + aa.code_store->>%(company_key)s, + char_length(agroup.code_prefix_end) + ) + AND agroup.company_id = %(root_company_id)s + WHERE aa.code_store->>%(company_key)s IS NOT NULL + ORDER BY aa.id, + char_length(agroup.code_prefix_start) DESC, + agroup.id + """, + company_key=company_key, + root_company_id=root_company_id, + ) + ) + mapping = defaultdict(list) + for account_id, group_id in results: + if group_id: + mapping[group_id].append(account_id) + setattr( + self.env, + cache_attr, { - "group_ids": tuple(groups.ids), - "company_id": str(self.env.company.root_id.id), + "company_id": root_company_id, + "mapping": mapping, }, ) - account_ids = [row[0] for row in self.env.cr.fetchall()] - return [("id", "in", account_ids)] + return mapping + + @api.model + def _invalidate_group_to_accounts_cache(self): + cache_attr = "_group_to_accounts_cache" + if hasattr(self.env, cache_attr): + delattr(self.env, cache_attr) @api.model_create_multi def create(self, vals_list): res = super().create(vals_list) res.mapped("group_id").invalidate_recordset() + self._invalidate_group_to_accounts_cache() return res def write(self, vals): @@ -55,4 +101,5 @@ def write(self, vals): res = super().write(vals) if "code" in vals: (self.mapped("group_id") | groups).invalidate_recordset() + self._invalidate_group_to_accounts_cache() return res diff --git a/account_usability/models/account_group.py b/account_usability/models/account_group.py index 214462450..a65a4f8be 100644 --- a/account_usability/models/account_group.py +++ b/account_usability/models/account_group.py @@ -1,7 +1,7 @@ # Copyright 2018 FOREST AND BIOMASS ROMANIA SA # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). -from odoo import fields, models +from odoo import api, fields, models class AccountGroup(models.Model): @@ -12,3 +12,20 @@ class AccountGroup(models.Model): inverse_name="group_id", string="Accounts", ) + + @api.model_create_multi + def create(self, vals_list): + res = super().create(vals_list) + self.env["account.account"]._invalidate_group_to_accounts_cache() + return res + + def write(self, vals): + res = super().write(vals) + if "code_prefix_start" in vals or "code_prefix_end" in vals: + self.env["account.account"]._invalidate_group_to_accounts_cache() + return res + + def unlink(self): + res = super().unlink() + self.env["account.account"]._invalidate_group_to_accounts_cache() + return res diff --git a/account_usability/tests/test_account_group.py b/account_usability/tests/test_account_group.py index fb4f32ac7..ab2adda65 100644 --- a/account_usability/tests/test_account_group.py +++ b/account_usability/tests/test_account_group.py @@ -69,3 +69,64 @@ def test_search_accounts_on_group(self): self.assertIn(account_3.id, accounts_by_group.ids) self.assertNotIn(account_2.id, accounts_by_group.ids) self.assertIn(account.id, accounts_by_group.ids) + + def test_search_group_id_parent_child_overlap(self): + """Accounts must appear in their most specific group only. + + When a parent group has a broad prefix range and a child group + has a more specific prefix within that range, searching by the + parent group_id must NOT return accounts that belong to the + child group. This prevents duplicate rows in financial reports + like MIS Builder. + """ + parent_group = self.env["account.group"].create( + { + "name": "Parent Group", + "code_prefix_start": "9840", + "code_prefix_end": "9849", + } + ) + child_group = self.env["account.group"].create( + { + "name": "Child Group", + "code_prefix_start": "98410", + "code_prefix_end": "98419", + } + ) + account_child = self.env["account.account"].create( + {"code": "98410", "name": "Account in Child Range"} + ) + account_parent = self.env["account.account"].create( + {"code": "98400", "name": "Account in Parent Range Only"} + ) + account_outside = self.env["account.account"].create( + {"code": "98500", "name": "Account Outside"} + ) + + child_accounts = self.env["account.account"].search( + [("group_id", "=", child_group.id)] + ) + self.assertIn(account_child.id, child_accounts.ids) + self.assertNotIn(account_parent.id, child_accounts.ids) + self.assertNotIn(account_outside.id, child_accounts.ids) + + parent_accounts = self.env["account.account"].search( + [("group_id", "=", parent_group.id)] + ) + self.assertIn(account_parent.id, parent_accounts.ids) + self.assertNotIn( + account_child.id, + parent_accounts.ids, + "Account in child group range must not appear in parent group search", + ) + self.assertNotIn(account_outside.id, parent_accounts.ids) + + parent_via_path = self.env["account.account"].search( + [("group_id.code_prefix_start", "=", "9840")] + ) + self.assertIn(account_parent.id, parent_via_path.ids) + self.assertNotIn( + account_child.id, + parent_via_path.ids, + "Dotted path search must also respect most-specific group", + )