-
Notifications
You must be signed in to change notification settings - Fork 0
fix(odoo-spo): read selection_add + merge across classes (codex #530 follow-up) #532
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
|
@@ -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) | ||
| return out | ||
|
|
||
|
|
||
|
|
@@ -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), []) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This merge is applied to every 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 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This now treats
selection_addas enough to return a non-empty value list even when the baseselectionwas dynamic or a bare constant, which this helper intentionally cannot resolve. In a common inherited-field case like a basestate = fields.Selection(STATES)plusstate = fields.Selection(selection_add=[('reviewed', ...)]), the scan records onlyreviewed;enrichthen emits a closedselection_valueset with just the extension key, so downstream ASSERT generation rejects the original legal base states. Please keepselection_addas a pending extension and only emit a closed domain once a static base list is known.Useful? React with 👍 / 👎.