[18.0][IMP] web_form_banner: client-side fast path for simple rules#1
Open
dnplkndll wants to merge 2 commits into
Open
[18.0][IMP] web_form_banner: client-side fast path for simple rules#1dnplkndll wants to merge 2 commits into
dnplkndll wants to merge 2 commits into
Conversation
Every banner rule today fires a server RPC (`compute_message`) per
trigger-field change. For typical conditions like
`state == 'draft' and amount_total > 1000`, Odoo's view compiler can
evaluate the expression client-side in `invisible=` attributes via
py.js — no round-trip needed.
## What changed
Per-rule `client_side` opt-in. When enabled the rule emits a
self-contained `<div invisible="not (...)" class="alert ...">` into
the form arch via `get_view`. Visibility is evaluated by Odoo
natively; field interpolation uses inline `<field name="..."/>` tags
or the `${field_name}` sugar (rewritten to those tags at view-load).
The existing server-driven path is unchanged.
Defensive piece: if `client_condition` references a field that the
form view doesn't declare, py.js raises `Name 'X' is not defined` at
runtime and crashes any browser test that opens the form. The
`get_view` override parses the condition via `ast.parse`, collects
every leftmost-Name reference, filters out py.js reserved names +
names not on the model + names already declared, and auto-injects the
remaining ones as `<field name="X" invisible="True"/>` siblings
adjacent to the banner. Any admin-defined rule referencing a
non-loaded field now Just Works.
## Notable correctness details
- Banner element is built via lxml's `etree.Element` API rather than
f-string XML — single quotes in conditions like `state == 'draft'`
would otherwise terminate the `invisible='...'` attribute
mid-expression.
- `${var}` sugar matches only bare field names. Dotted paths like
`${partner_id.email}` would rewrite to invalid Odoo form arch.
Documented in USAGE; admins use a stored related field instead.
- 20 tests covering arch emission, ${var} sugar, HTML escaping,
malformed/missing condition rejection, hidden-field auto-injection
with dedup, py.js-reserved-name skipping, dotted-${} rejection,
quote-bearing conditions, position=after, and multi-rule field
sharing.
## Files
- `models/web_form_banner_rule.py` — `client_side` + `client_condition`
fields, `_check_client_condition` constraint, `_to_client_arch` for
`${var}` rewrite.
- `models/ir_model.py` — `get_view` branches on `rule.client_side`;
`_client_rule_missing_fields` returns hidden field shims;
`_build_client_banner` uses lxml element API.
- `static/src/js/web_form_banner.esm.js` — one-line filter in
`bannersIn()` skips `data-client="1"`.
- `views/web_form_banner_rule_views.xml` — new fields exposed,
notebook page toggles between server- and client-side help.
- `tests/test_web_form_banner.py` — +11 test cases.
- `demo/web_form_banner_rule_demo.xml` — extra demo using base-only
fields (`not email and name`) to exercise the fast path on a clean
install.
- `readme/USAGE.md` — Client-side mode section + limitations.
- `readme/ROADMAP.md` — 5 follow-ups.
- `readme/CONTRIBUTORS.md` — add Ledoweb / dkendall.
- `__manifest__.py` — bump to 18.0.1.2.0, add co-author + maintainer.
Backward-compatible: existing rules and the server-side path are
unchanged. Admins opt in by ticking `Client-side` on the rule form.
Verified locally via fresh-DB `--test-enable`: 20 tests, 0 failures.
Verified on fork CI (#1, ledoent self-hosted runners):
all 5 jobs green including the chatgpt tour that hit the original
hidden-field bug.
5087a55 to
cf9cac4
Compare
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.
Why
Every banner rule today fires a server RPC (
compute_message) per trigger-field change. For typical conditions —state == 'draft' and amount_total > 1000— Odoo's view compiler already evaluates exactly that kind of expression purely client-side viapy.jsininvisible=attributes. The round-trip isn't needed for the 80% case.What
Per-rule
client_sideopt-in. When enabled:<field name="..."/>tags (Odoo-native, reactive) plus the existing${field_name}sugar that rewrites to those tags atget_viewtime.Generated arch
For a rule with
client_condition: not email and name, messageContact <strong>${name}</strong> has no email., target xpath//sheet, onres.partnerform:Toggle
emailon/off in the form → banner appears/disappears instantly. Zero Network-tab activity, zerocompute_messageRPC.Defensive auto-injection
If
client_conditionreferences a field that the form view doesn't declare, py.js raisesName 'X' is not definedat render time and crashes any browser test that opens the form. Theget_viewoverride parses the condition viaast.parse, extracts every leftmost-Name reference, filters out py.js reserved names + names not on the model + names already declared, and auto-injects the remaining as<field name="X" invisible="True"/>siblings. Any admin-defined rule referencing a non-loaded field now Just Works.What works in
client_conditionComparisons, boolean ops,
in/not in, attribute access (partner_id.email), built-ins (len,bool,min,max,set), ternaryx if cond else y.What does NOT work — keep server-side mode
Arbitrary method calls (
.filtered(),.mapped()), slicing, lambdas, comprehensions, ORM searches, per-record severity overrides,${X.Y}dotted interpolation.Files
models/web_form_banner_rule.pyclient_side+client_conditionfields,_check_client_conditionconstraint viaast.parse,_to_client_archfor${var}→<field/>models/ir_model.pyget_viewbranches onrule.client_side;_client_rule_missing_fieldsreturns hidden field shims; banner built via lxml element APIstatic/src/js/web_form_banner.esm.jsbannersIn()skipsdata-client="1"divsviews/web_form_banner_rule_views.xmltests/test_web_form_banner.py${var}sugar, HTML escaping, missing/malformed condition rejection, hidden-field auto-injection with dedup, py.js-reserved-name skipping, dotted-${}rejection, quote-bearing conditions, position=after, multi-rule field sharingdemo/web_form_banner_rule_demo.xmlnot email and name)readme/USAGE.md+ROADMAP.md+CONTRIBUTORS.md__manifest__.py18.0.1.2.0, add Ledoweb co-author +maintainers=["dnplkndll"]Notable correctness details
etree.ElementAPI rather than f-string XML — single quotes in conditions likestate == 'draft'would otherwise terminate theinvisible='...'attribute mid-expression. Caught during review; covered bytest_client_side_handles_quoted_string_literals.${var}sugar restricted to bare field names. Dotted paths like${partner_id.email}would rewrite to invalid form arch. Covered bytest_client_side_rejects_dotted_var_sugar. Documented in USAGE.Verification
Local fresh-DB
--test-enable: 20 tests, 0 failures.Fork CI on ledoent self-hosted runners (
hetzner-k3s-ledoent): all 5 jobs green (commiteffeea0on 18.0-imp-web_form_banner-client-side).Practice run scope
PR opened against the fork (not OCA upstream) to exercise the review workflow. Will close after squashing/promoting to OCA/web.
Test plan
>>> FORK-LOCAL BLOCK <<<before promoting upstreamSpeculative migration test (19.0)
ledoent/openupgrade-lab@feat/banner-migration-speculative-test
adds a Playwright spec + admin seeder +
make benchmark-bannertarget that catches future field-spec migrations of
web.form.banner.rule. Auto-skipped whileweb_form_bannerisabsent from OCA/web 19.0; un-skip when PR OCA#3309 lands.
Performance — server vs client, matched 100-vuser comparison
After validating the harness on
agent_test_18(server-side only),ran both modes against the same target — the OpenUpgrade lab's
oca_18_enrichedDB onlocalhost:8018with our18.0.1.2.0buildinstalled. Same Odoo configuration (4 workers, 256 DB connections),
same 280-partner seed, same 100-vuser ramp over 5 minutes.
Per-endpoint breakdown (server-side):
POST compute_messagePOST web_read partnerPOST authenticatePer-endpoint (client-side):
POST web_read partnerPOST authenticateSaturation threshold reached. Server-side p95 (6.9 s) is 328×
the client-side baseline (21 ms). The
compute_messageRPC stream(22 RPS, p95 5.8 s on success) is what the client-side mode
eliminates entirely — those 6,579 calls become zero in client
mode, and the freed worker capacity lets the lab serve 6.4× more
form opens (14,349 vs 2,218) without a single failure.
What this means
requests to pool exhaustion + timeout. Client-side mode handles
the same workload with zero failures and 21 ms p95.
a keystroke that previously required a 560-ms RPC round-trip now
evaluates in py.js in < 1 ms.
doodba 4-worker sizing. Any customer with > ~25 simultaneous
editors hitting a server-side banner rule will see queueing.
Harness
Locust-based, reusable across PRs. Live at
ledoent/odoo-agent-clerks@feat/load-test-banner-clientside/load/.
~10 min setup to add a bench for another PR; pattern documented in
the perf-kit roadmap.