Skip to content

Commit 93e529d

Browse files
committed
fix(spp_cel_domain): handle relational fields and SQL JOINs in CEL
- Handle one2many/many2many fields as bare CEL predicates - Use query.select() for SQL with JOINs instead of raw field access - Add tests for relational predicate handling
1 parent 3258a93 commit 93e529d

File tree

7 files changed

+397
-61
lines changed

7 files changed

+397
-61
lines changed

spp_cel_domain/models/cel_executor.py

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -85,20 +85,11 @@ def _domain_to_id_sql(self, model: str, domain: list[Any]) -> SQL | None:
8585
expr = osv_expression.expression(model=Model, domain=domain)
8686
query = expr.query
8787

88-
# Get the WHERE clause
89-
where_clause = query.where_clause
90-
if not where_clause:
91-
# No WHERE clause means all records
92-
return SQL("(SELECT id FROM %s)", SQL.identifier(table))
93-
94-
# Build the full SELECT query
95-
# Note: where_clause is already a SQL object in Odoo 19
96-
return SQL(
97-
"(SELECT %s.id FROM %s WHERE %s)",
98-
SQL.identifier(table),
99-
SQL.identifier(table),
100-
where_clause,
101-
)
88+
# Use query.select() to get the full SQL including FROM clause
89+
# with all JOINs (needed for related field domains like gender_id.uri)
90+
select_sql = query.select(SQL.identifier(table, "id"))
91+
92+
return SQL("(%s)", select_sql)
10293
except Exception as e:
10394
self._logger.debug("[CEL SQL] Failed to convert domain to SQL: %s", e)
10495
return None
@@ -503,7 +494,7 @@ def compile_and_preview(
503494
parts.append(
504495
f"metric={mi.get('metric')} period={mi.get('period_key')} "
505496
f"requested={mi.get('requested')} cache_hits={mi.get('cache_hits')} "
506-
f"fresh={mi.get('fresh_fetches')} coverage={round(cov*100,1)}%"
497+
f"fresh={mi.get('fresh_fetches')} coverage={round(cov * 100, 1)}%"
507498
+ (f" warnings={','.join(mi_warnings)}" if mi_warnings else "")
508499
)
509500
metrics_section = " | Metrics: " + "; ".join(parts)

spp_cel_domain/models/cel_sql_builder.py

Lines changed: 9 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -157,57 +157,28 @@ def select_ids_from_domain(self, model: str, domain: list[Any]) -> SQL | None:
157157
"""Generate SELECT id FROM model WHERE domain.
158158
159159
Uses expression.expression() to include record rules.
160+
Uses Query.select() to include all necessary JOINs (e.g. for
161+
related field lookups like gender_id.uri).
160162
Returns None if domain cannot be converted.
161163
162164
This is the PRIMARY method for generating subqueries.
163165
All other methods should use this for model references.
164166
"""
165-
if not domain:
166-
# Empty domain selects all IDs (respecting record rules)
167-
Model = self.env[model]
168-
table = Model._table
169-
# Still need to apply record rules even for empty domain
170-
try:
171-
from odoo.osv import expression as osv_expression
172-
173-
expr = osv_expression.expression(model=Model, domain=[])
174-
query = expr.query
175-
where_clause = query.where_clause
176-
if where_clause:
177-
return SQL(
178-
"(SELECT %s.id FROM %s WHERE %s)",
179-
SQL.identifier(table),
180-
SQL.identifier(table),
181-
where_clause,
182-
)
183-
return SQL("(SELECT id FROM %s)", SQL.identifier(table))
184-
except Exception as e:
185-
_logger.debug("[SQLBuilder] Failed for empty domain: %s", e)
186-
return SQL("(SELECT id FROM %s)", SQL.identifier(table))
187-
188167
try:
189168
from odoo.osv import expression as osv_expression
190169

191170
Model = self.env[model]
192171
table = Model._table
193172

194-
# Build the expression and extract query
195-
expr = osv_expression.expression(model=Model, domain=domain)
173+
# Build the expression (applies record rules even for empty domain)
174+
expr = osv_expression.expression(model=Model, domain=domain or [])
196175
query = expr.query
197176

198-
# Get the WHERE clause
199-
where_clause = query.where_clause
200-
if not where_clause:
201-
# No WHERE clause means all records
202-
return SQL("(SELECT id FROM %s)", SQL.identifier(table))
203-
204-
# Build the full SELECT query
205-
return SQL(
206-
"(SELECT %s.id FROM %s WHERE %s)",
207-
SQL.identifier(table),
208-
SQL.identifier(table),
209-
where_clause,
210-
)
177+
# Use query.select() to get the full SQL including FROM clause
178+
# with all JOINs (needed for related field domains like gender_id.uri)
179+
select_sql = query.select(SQL.identifier(table, "id"))
180+
181+
return SQL("(%s)", select_sql)
211182
except Exception as e:
212183
_logger.debug("[SQLBuilder] Failed to convert domain to SQL: %s", e)
213184
return None

spp_cel_domain/models/cel_translator.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,7 @@ def _flatten_attr(a):
628628
rec = self.env["spp.program"].search([("name", "=", name)], limit=1)
629629
pid = rec.id or None
630630
return LeafDomain(model, [("id", "!=", 0)]), f"PROGRAM({name})={pid}"
631-
# Boolean field used as predicate (e.g., m._link.is_ended)
631+
# Field used as bare predicate (e.g., m._link.is_ended, program_membership_ids)
632632
if isinstance(node, P.Attr | P.Ident):
633633
fld, mdl = self._resolve_field(model, node, cfg, ctx)
634634
target_model = mdl or model
@@ -637,7 +637,10 @@ def _flatten_attr(a):
637637
ft = model_fields.get(fld)
638638
if ft and getattr(ft, "type", None) == "boolean":
639639
return LeafDomain(target_model, [(fld, "=", True)]), f"{fld} is True"
640-
# Field exists but is not boolean — cannot use as bare predicate
640+
# Relational fields as bare predicates: treat as "has records"
641+
if ft and getattr(ft, "type", None) in ("one2many", "many2many"):
642+
return LeafDomain(target_model, [(fld, "!=", False)]), f"{fld} is not empty"
643+
# Field exists but is not boolean or relational — cannot use as bare predicate
641644
if ft:
642645
raise NotImplementedError(
643646
f"Field '{fld}' is of type '{getattr(ft, 'type', '?')}', not boolean. "

spp_cel_domain/models/cel_variable.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -530,16 +530,25 @@ def unlink(self):
530530
return super().unlink()
531531

532532
def _invalidate_resolver_cache(self):
533-
"""Invalidate the variable resolver cache.
533+
"""Invalidate all CEL caches.
534534
535-
Called when variable definitions change to ensure deferred resolution
536-
uses the updated definitions.
535+
Called when variable definitions change to ensure:
536+
- Variable resolver cache uses updated variable definitions
537+
- Translation cache rebuilds with new variable expansions
538+
- Profile cache reflects any configuration changes
539+
540+
This prevents stale cache issues where expressions continue to
541+
use old variable definitions after modifications.
537542
"""
538543
try:
539-
resolver = self.env["spp.cel.variable.resolver"]
540-
resolver.invalidate_variable_cache()
544+
# Invalidate all CEL caches via the service facade
545+
# This ensures profile cache, translation cache, and resolver cache
546+
# are all cleared in a coordinated manner
547+
cel_service = self.env["spp.cel.service"]
548+
cel_service.invalidate_caches()
549+
_logger.debug("CEL caches invalidated after variable change")
541550
except Exception as e:
542-
_logger.debug("Could not invalidate resolver cache: %s", e)
551+
_logger.warning("Could not invalidate CEL caches: %s", e)
543552

544553
# ═══════════════════════════════════════════════════════════════════════
545554
# HELPER METHODS

spp_cel_domain/tests/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,4 @@
2525
from . import test_data_value
2626
from . import test_data_provider
2727
from . import test_multi_company
28+
from . import test_cel_relational_predicate
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
# Part of OpenSPP. See LICENSE file for full copyright and licensing details.
2+
"""Tests for relational fields (one2many, many2many) used as bare predicates.
3+
4+
When a user writes a CEL expression like `program_membership_ids` (without
5+
a comparison operator), the translator should treat it as a truthiness
6+
check: "has records" → True, "no records" → False. This matches Python
7+
and CEL semantics where non-empty collections are truthy.
8+
"""
9+
10+
from odoo.tests import TransactionCase, tagged
11+
12+
13+
@tagged("post_install", "-at_install")
14+
class TestRelationalBarePredicates(TransactionCase):
15+
"""Test that one2many and many2many fields work as bare predicates."""
16+
17+
def setUp(self):
18+
super().setUp()
19+
self.service = self.env["spp.cel.service"]
20+
21+
def test_one2many_field_as_bare_predicate_compiles(self):
22+
"""A one2many field used as a bare predicate should compile.
23+
24+
Expression like `program_membership_ids` should be treated as
25+
checking whether the field has records (not empty).
26+
"""
27+
# program_membership_ids is a One2many on res.partner
28+
# (from spp_programs module)
29+
if "program_membership_ids" not in self.env["res.partner"]._fields:
30+
self.skipTest("spp_programs not installed (no program_membership_ids field)")
31+
32+
result = self.service.compile_expression(
33+
"program_membership_ids",
34+
"registry_groups",
35+
)
36+
self.assertTrue(result["valid"], f"Error: {result.get('error')}")
37+
# Should produce a domain checking the field is not empty
38+
self.assertIsInstance(result["domain"], list)
39+
40+
def test_many2many_field_as_bare_predicate_compiles(self):
41+
"""A many2many field used as a bare predicate should compile."""
42+
# Find any many2many field on res.partner for testing
43+
partner_fields = self.env["res.partner"]._fields
44+
m2m_field = None
45+
for fname, fobj in partner_fields.items():
46+
if getattr(fobj, "type", None) == "many2many":
47+
m2m_field = fname
48+
break
49+
50+
if not m2m_field:
51+
self.skipTest("No many2many field found on res.partner")
52+
53+
result = self.service.compile_expression(
54+
m2m_field,
55+
"registry_groups",
56+
)
57+
self.assertTrue(result["valid"], f"Error: {result.get('error')}")
58+
self.assertIsInstance(result["domain"], list)
59+
60+
def test_one2many_predicate_produces_correct_domain(self):
61+
"""Bare one2many predicate should produce a '!= False' domain.
62+
63+
This is the standard Odoo pattern for checking if a relational
64+
field has records.
65+
"""
66+
if "program_membership_ids" not in self.env["res.partner"]._fields:
67+
self.skipTest("spp_programs not installed (no program_membership_ids field)")
68+
69+
result = self.service.compile_expression(
70+
"program_membership_ids",
71+
"registry_groups",
72+
)
73+
self.assertTrue(result["valid"], f"Error: {result.get('error')}")
74+
# The domain should contain a check for "has records"
75+
domain = result["domain"]
76+
# Look for the field check in the domain
77+
found = False
78+
for leaf in domain:
79+
if isinstance(leaf, tuple) and leaf[0] == "program_membership_ids" and leaf[1] == "!=":
80+
found = True
81+
break
82+
self.assertTrue(found, f"Expected '!= False' domain for one2many, got: {domain}")

0 commit comments

Comments
 (0)