[18.0][ADD] web_field_provenance: ORM dirty bit + SAP-style badge#2
Open
dnplkndll wants to merge 6 commits into
Open
[18.0][ADD] web_field_provenance: ORM dirty bit + SAP-style badge#2dnplkndll wants to merge 6 commits into
dnplkndll wants to merge 6 commits into
Conversation
…ic stamping API
Schema change in `_provenance`:
- Absence of an entry → field still at its default (no stamp needed).
- {"s":"u", "b":<login>, "t":<unix>} → user-set.
- {"s":"r", "b":<id>, "t":<unix>, "r":<lbl>} → rule / cascade / EDI.
Defaults are intentionally not stamped to keep the JSON column compact;
`_provenance_for` uses `record.create_date` as the "default since" proxy
without storing extra data.
Adds:
- Public `_stamp_provenance(keys, *, source, by, rule=None, when=None)`
for cascades, integrations, and EDI writers. Rejects unknown sources
and empty `by` to keep tooltips informative.
- `_provenance_for(fname)` returning the tooltip dict the OWL widget
renders. Three states: default, user, rule.
- OWL widget renders three icons (grey dot / green cog / pencil) and
a hover tooltip with writer + timestamp.
Tests: 17 cases covering the user/rule/default paths, the public API
contract (rejected sources, empty `by`, untracked-key filter), the
override flow (rule -> user write flips state), legacy bare-string
entries (backward-compat read), and explicit-timestamp injection.
Test fixture uses SQL to flip `track_provenance` on a base field
because Odoo blocks `ir.model.fields.write()` on `state='base'`. The
production opt-in wizard is a documented ROADMAP item.
ROADMAP extended with the API / EDI integration sub-track and the
opt-in wizard.
…+ dead code
Self-review findings addressed before opening the PR:
* **Security**: the OWL `onBadgeClick` writes `_provenance` directly
with a client-supplied `b` (login) and `t` (timestamp). The server
accepted this verbatim, so a malicious client could spoof attribution.
Adds `_sanitize_client_provenance` invoked from `write()` whenever
`_provenance` appears in the incoming vals:
- any entry the client provides is rewritten to
`{s:"u", b:env.user.login, t:int(time.time())}`
- any client-asserted `s="r"` entry is dropped — only server-side
cascade callers (via `_stamp_provenance`) may stamp rule
provenance
- non-dict payloads sanitize to `{}` rather than persist junk
Server-side `_stamp_provenance_keys` continues to bypass this path
via the `_prov_skip` context.
* **SCSS**: template emits `o_provenance_default | _rule | _user` but
the stylesheet only colored `_user` and the obsolete `_system`. Adds
rules for `_default` (muted) and `_rule` (success/green), drops the
obsolete `_system` selector.
* **Dead code**: `_logger` was imported but never used; `badge.visible`
state + the `t-if=badge.visible` template branch never toggled. Both
removed.
* **Docs**: `DESCRIPTION.md` and `USAGE.md` still described the v0.1
two-state model (gear / pencil) and missed the new
`_stamp_provenance` public API. Rewritten to describe the three
states, the cascade API, and the sanitization guarantee.
No behavior change for the existing 17 tests; the follow-up commit adds
coverage for the security path and the surfaced gaps.
…record, cache
Six new tests filling the gaps surfaced in self-review.
Security path coverage:
* test_client_provenance_write_forces_env_user — a client write
with a spoofed b="attacker" is rewritten to env.user.login by
_sanitize_client_provenance; timestamp replaced with int(time.time()).
* test_client_cannot_claim_rule_provenance — a client payload with
s="r" is dropped entirely. Only _stamp_provenance (server-side)
may stamp rule provenance.
* test_client_provenance_non_dict_payload_rejected — a string,
list, etc. sanitizes to empty {} rather than persisting junk.
Backward-compat:
* test_legacy_system_string_falls_back_to_default — v0.1 stored bare
strings "u" and "s". The bare "s" mapped to "system assigned" in
v0.1; in the 3-state schema it correctly resolves to the "default"
badge state and _user_set() returns False.
Multi-record:
* test_multirecord_stamp_writes_each — (p1+p2)._stamp_provenance(...)
writes the stamp on each record (one UPDATE per record by design;
see _stamp_provenance_keys).
Cache invalidation:
* test_unlink_invalidates_track_cache — toggling track_provenance off
via the documented SQL path invalidates the cached _field_track_set
so subsequent create() does not stamp.
Combined suite: 23/23 pass.
Two integration bugs surfaced when wiring the first consumer (sale_order_type cascade via ledoent/sale-workflow#3): * **Onchange diff leak**: stamping `_provenance` via `write()` during a compute leaked the field into Odoo's onchange diff. Form views that don't declare `_provenance` (most do not — the badge consumes it via `web_read` only) raised `KeyError: '_provenance'`. Switched `_stamp_provenance_keys` to raw SQL + `invalidate_recordset` so the stamp does not surface as an in-flight ORM change. * **NewId records cannot be SQL-updated**: skip records whose `id` is not yet a database integer. This affects two paths: 1. Form / onchange — stamping is the wrong thing anyway, see above. 2. `create()` precompute — there is no DB row to UPDATE. Both paths fall back to the cascade attribution being applied on the next persistent-record write that touches the field, which matches typical user flows (the cascade re-fires when a user changes `type_id` on a saved SO). Documented in the docstring; a deferred-flush mechanism is the right long-term fix and stays on the ROADMAP. Existing 23 web_field_provenance tests still pass; the sibling sale-workflow PR's 60-test suite goes 60/60 once the SO cascade is exercised on persistent records (separate commit there).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
New ledoent module that fixes the OCA-wide bug class where
compute=..., store=True, readonly=Falsefields silently overwrite manual user overrides on every recompute pass. Ships:record._user_set(fname)— combine a persisted provenance map with an_origintransactional fallback. Compute overrides call this to gatesuper()on real user intent.record._stamp_provenance(keys, *, source, by, rule=None, when=None)— public API for cascade methods, EDI connectors, and import loaders to stamp rule/cascade provenance on writes they perform on behalf of users.record._provenance_for(fname)— returns the badge-tooltip dict (also useful as golden-output target in tests).provenance_m2owidget — small badge next to opted-in fields with three states:Hover reveals the writer and timestamp. Clicking promotes the value to user-anchored; the server sanitizes the payload so writer identity cannot be spoofed.
_provenanceJSON column on every model — compact{s, b, t, r?}entries, only populated for fields opted in viair.model.fields.track_provenance. Default state is implicit (no entry) so the column stays compact.Why
OCA
sale_order_typeis one example: every time a partner or type changes, the seven mirror computes callsuper()and clobber manual user-set values for pricelist, payment term, warehouse, etc. PR OCA/sale-workflow#4273 patches one symptom; this module is the missing infrastructure.First consumer: ledoent/sale-workflow#3 —
_sot_resolvealready has a soft-dependency hook (hasattr(self, "_user_set")) that integrates the moment this module is installed.What's inside
models/base_model.py_provenancefield,_user_set,_stamp_provenance,_provenance_for,web_readinjection, sanitization of client writesmodels/ir_model_fields.pytrack_provenanceopt-in flag + cache invalidation on togglestatic/src/provenance_badge.{esm.js,xml,scss}tests/test_provenance.pyreadme/{DESCRIPTION,USAGE,ROADMAP}.mdviews/ir_model_fields_views.xmlTests (23/23 pass)
Test plan
odoo -i web_field_provenance --test-enable --test-tags '/web_field_provenance'→ 23/23 pass.sale.order.payment_term_id, verify the badge appears with the three icons.{_provenance: {payment_term_id: {s:"u", b:"admin", t:0}}}— server sanitizes tob=env.user.login.Notes for reviewers
OCA/web._inherit = "base". The cost is gated behindir.model.fields.track_provenance— for any field that hasn't opted in, the write-path overhead is one frozenset lookup and the read-path is untouched.ir.model.fields.write()blocks the UI from flippingtrack_provenanceon fields declared in Python. ROADMAP lists a small admin wizard to do this via the documented SQL bypass; tests use it directly.