Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,13 @@

Enables the consumer to lower a Selection field to
`DEFINE FIELD state … ASSERT $value IN ['draft','posted','cancel']`.
Dynamic selections (`selection='_compute_states'`, a bare Name constant,
or `related=`) are skipped — their values aren't statically knowable.
Source order is preserved. Selection fields bind to the same
Both the base list (positional arg 0 or `selection=`) AND a
`_inherit` addon's `selection_add=[…]` extension are read and unioned —
an extension's keys MERGE onto the field's accumulated set across
classes, so the `ASSERT` set includes legal extension-added states (codex
#530). Dynamic selections (`selection='_compute_states'`, a bare Name
constant, or `related=`) are skipped — their values aren't statically
knowable. Source order is preserved. Selection fields bind to the same
`model_names` as relational fields (`_name`, else `_inherit[0]`).

This is *in addition* to the existing shallow relation read
Expand Down Expand Up @@ -162,37 +166,49 @@
_VALIDATION_KINDS = ("presence", "uniqueness", "range", "format", "lookup")


def _selection_tuple_keys(node: Optional[ast.expr], out: List[str], seen: Set[str]) -> None:
"""Append the first-element string key of each 2-tuple in `node` (a list
or tuple of `('key','Label')` pairs) to `out`, de-duplicating against
`seen`, preserving order. Non-list/tuple `node` (a str method-ref, a bare
Name constant, `related=`) contributes nothing — those aren't static."""
if not isinstance(node, (ast.List, ast.Tuple)):
return
for elt in node.elts:
if not isinstance(elt, (ast.Tuple, ast.List)) or len(elt.elts) < 1:
continue
key = _const_str(elt.elts[0])
if key is not None and key not in seen:
seen.add(key)
out.append(key)


def _extract_selection_values(call: ast.Call) -> List[str]:
"""Pull the value KEYS from a `fields.Selection([('k','Label'), …])`
declaration. The selection list is the first positional arg OR the
`selection=` kwarg; each entry is a 2-tuple whose first element is the
stored key.

Returns the keys in source order, de-duplicated (first occurrence wins).
Returns `[]` for dynamic selections — a bare Name (constant reference),
a string (compute-method name), `related=`, or any entry whose first
element isn't a string constant. Those values aren't statically known.
"""Pull the value KEYS from a `fields.Selection(...)` declaration.

Reads BOTH the base list — first positional arg OR `selection=` kwarg —
AND the `selection_add=` kwarg (the Odoo idiom for an `_inherit` addon
that *extends* an existing Selection field's domain without redeclaring
it). Keys from both are unioned, base first, in source order, de-duped.

Returns `[]` when neither source is a static list of 2-tuples — a dynamic
`selection='_compute_x'` (str), a bare Name constant, or `related=`.
A pure `selection_add=` extension (no base list) yields just the added
keys, which the caller MERGES onto the field's accumulated set.
"""
sel: Optional[ast.expr] = None
base: Optional[ast.expr] = None
add: Optional[ast.expr] = None
for kw in call.keywords:
if kw.arg == "selection":
sel = kw.value
break
if sel is None and call.args:
sel = call.args[0]
if not isinstance(sel, (ast.List, ast.Tuple)):
# `selection='_compute_x'` (str), a Name constant, or related= → skip.
return []
base = kw.value
elif kw.arg == "selection_add":
add = kw.value
if base is None and call.args:
base = call.args[0]

out: List[str] = []
seen: Set[str] = set()
for elt in sel.elts:
if not isinstance(elt, (ast.Tuple, ast.List)) or len(elt.elts) < 1:
continue
key = _const_str(elt.elts[0])
if key is not None and key not in seen:
seen.add(key)
out.append(key)
_selection_tuple_keys(base, out, seen)
_selection_tuple_keys(add, out, seen)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid closing domains from selection_add alone

This now treats selection_add as enough to return a non-empty value list even when the base selection was dynamic or a bare constant, which this helper intentionally cannot resolve. In a common inherited-field case like a base state = fields.Selection(STATES) plus state = fields.Selection(selection_add=[('reviewed', ...)]), the scan records only reviewed; enrich then emits a closed selection_value set with just the extension key, so downstream ASSERT generation rejects the original legal base states. Please keep selection_add as a pending extension and only emit a closed domain once a static base list is known.

Useful? React with 👍 / 👎.

return out


Expand Down Expand Up @@ -449,9 +465,15 @@ def _scan_file(
relmap[(mu, field_name)] = (comodel, inverse)
if selections is not None:
for field_name, vals in local_selections.items():
# Last write wins (a later reopening that redeclares the
# Selection with a fuller value list supersedes).
selections[(mu, field_name)] = vals
# MERGE, not overwrite: the base `selection=` declaration
# and any `_inherit` addon's `selection_add=` extensions
# live in DIFFERENT classes, so a given (model, field)
# accumulates across scans (codex #530). Order-preserving,
# de-duplicated union.
acc = selections.setdefault((mu, field_name), [])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Do not union full selection redeclarations

This merge is applied to every fields.Selection declaration, not just selection_add extensions. When an _inherit class redeclares the whole field with selection=[...] to replace or reorder the domain, stale keys from the earlier definition remain in the accumulated set, so the generated selection_value facts allow values the final Odoo field no longer accepts. The scan needs to distinguish additive selection_add values from a full base selection redeclaration and overwrite/reset for the latter.

Useful? React with 👍 / 👎.

for v in vals:
if v not in acc:
acc.append(v)

# ── P1b: inherits_from edges ─────────────────────────────────────
# Only when the class declares a NEW model (_name is set). An
Expand Down
39 changes: 39 additions & 0 deletions tools/odoo-blueprint-extractor/tests/test_spo_enrich.py
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,27 @@ def test_non_tuple_entries_skipped(self):
vals = _classify_sel("fields.Selection([('a','A'), 'bogus', ('b','B')])")
self.assertEqual(vals, ["a", "b"])

# codex #530 — selection_add (the _inherit extension idiom)
def test_selection_add_kwarg_alone(self):
# A pure extension: `selection_add=` with no base list → the added keys.
vals = _classify_sel(
"fields.Selection(selection_add=[('reviewed','Reviewed'),('void','Void')])"
)
self.assertEqual(vals, ["reviewed", "void"])

def test_selection_base_plus_add_union_base_first(self):
vals = _classify_sel(
"fields.Selection(selection=[('draft','Draft'),('posted','Posted')], "
"selection_add=[('reviewed','Reviewed')])"
)
self.assertEqual(vals, ["draft", "posted", "reviewed"])

def test_selection_add_dedups_against_base(self):
vals = _classify_sel(
"fields.Selection(selection=[('a','A')], selection_add=[('a','A2'),('b','B')])"
)
self.assertEqual(vals, ["a", "b"])


class TestSelectionScan(unittest.TestCase):
"""`_scan_file` binds Selection values to model_names (per #525 rule)."""
Expand Down Expand Up @@ -683,6 +704,24 @@ def test_dynamic_selection_field_records_nothing(self):
)
self.assertEqual(sel, {})

def test_selection_add_merges_across_classes(self):
# codex #530: base declaration in the _name class + a selection_add
# extension in a separate _inherit class must ACCUMULATE, not
# overwrite — else the ASSERT $value IN [...] rejects legal added
# states.
sel = _scan_sel(
"class Base:\n"
" _name = 'account.move'\n"
" state = fields.Selection([('draft','Draft'),('posted','Posted')])\n"
"\n"
"class Ext:\n"
" _inherit = 'account.move'\n"
" state = fields.Selection(selection_add=[('reviewed','Reviewed')])\n"
)
self.assertEqual(
sel, {("account_move", "state"): ["draft", "posted", "reviewed"]}
)


class TestP3SelectionValueEmission(unittest.TestCase):
def _triples(self, field_iri="odoo:account_move.state"):
Expand Down