Skip to content

Bug: CEL Expressions tab crashes and hidden menus reset on upgrade#166

Open
haklyray wants to merge 1 commit into19.0from
fix/cel-codeeditor-mode
Open

Bug: CEL Expressions tab crashes and hidden menus reset on upgrade#166
haklyray wants to merge 1 commit into19.0from
fix/cel-codeeditor-mode

Conversation

@haklyray
Copy link
Copy Markdown
Contributor

1. CodeEditor crash on Approval Definition CEL Expressions tab

Steps to reproduce:

  1. Go to Approvals > Configuration > Approval Definitions
  2. Open any approval definition
  3. Click the "CEL Expressions" tab

Expected: Tab opens normally
Actual: JS crash: Invalid props for component 'CodeEditor': 'mode' is not valid

Root cause: spp_approval/views/approval_definition_views_cel.xml uses options="{'mode': 'text'}" on widget="ace" fields. Odoo 19's CodeEditor component only accepts modes: javascript, xml, qweb, scss, python. text is not a valid mode.

2. Hidden menus become visible after module upgrade

Steps to reproduce:

  1. Hide a menu via spp_hide_menus_base (e.g. Discuss, Contacts)
  2. Upgrade any module that defines that menu
  3. The menu reappears despite being marked as hidden

Root cause: Module upgrade re-applies menu XML data (noupdate="0"), resetting group_ids. The hide_menus() hook in ir.module.module.next() skips menus with state="hide", so the reset is never corrected.

1. spp_approval: change ace widget mode from 'text' to 'python' in
   CEL expression fields. Odoo 19 CodeEditor only accepts
   javascript/xml/qweb/scss/python — 'text' crashes with
   "Invalid props for component 'CodeEditor': 'mode' is not valid".

2. spp_hide_menus_base: re-apply hiding when module upgrade resets
   menu group_ids via XML (noupdate="0"). Previously, menus with
   state="hide" were skipped in hide_menus(), so an upgrade that
   re-applied the original group_ids left them visible.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request modifies the ace widget configuration for CEL fields to use python mode and adds logic to re-apply menu hiding settings during module upgrades. The reviewer recommends using javascript mode instead of python for CEL expressions to ensure correct syntax highlighting, as CEL's syntax aligns more closely with JavaScript.

name="cel_condition"
widget="ace"
options="{'mode': 'text'}"
options="{'mode': 'python'}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

CEL (Common Expression Language) uses C-style operators such as &&, ||, and !, which are not valid in Python. Using python mode will cause the CodeEditor to highlight these as syntax errors, leading to a poor user experience. Since Odoo 19's CodeEditor supports javascript, it is a much better fit for CEL expressions as the syntax is largely compatible.

Suggested change
options="{'mode': 'python'}"
options="{'mode': 'javascript'}"

name="cel_reviewer_expression"
widget="ace"
options="{'mode': 'text'}"
options="{'mode': 'python'}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

CEL (Common Expression Language) uses C-style operators such as &&, ||, and !, which are not valid in Python. Using python mode will cause the CodeEditor to highlight these as syntax errors, leading to a poor user experience. Since Odoo 19's CodeEditor supports javascript, it is a much better fit for CEL expressions as the syntax is largely compatible.

Suggested change
options="{'mode': 'python'}"
options="{'mode': 'javascript'}"

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.46%. Comparing base (b061135) to head (73c8be5).
⚠️ Report is 11 commits behind head on 19.0.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             19.0     #166      +/-   ##
==========================================
+ Coverage   71.45%   71.46%   +0.01%     
==========================================
  Files         932      932              
  Lines       54792    54846      +54     
==========================================
+ Hits        39152    39198      +46     
- Misses      15640    15648       +8     
Flag Coverage Δ
spp_analytics 93.13% <ø> (ø)
spp_api_v2 80.10% <ø> (ø)
spp_api_v2_change_request 66.85% <ø> (ø)
spp_api_v2_cycles 71.12% <ø> (ø)
spp_api_v2_data 64.41% <ø> (ø)
spp_api_v2_entitlements 70.19% <ø> (ø)
spp_api_v2_gis 71.52% <ø> (ø)
spp_api_v2_products 66.27% <ø> (ø)
spp_api_v2_service_points 70.94% <ø> (ø)
spp_api_v2_simulation 71.12% <ø> (ø)
spp_api_v2_vocabulary 57.26% <ø> (ø)
spp_approval 50.29% <ø> (ø)
spp_area 79.26% <ø> (ø)
spp_area_hdx 81.43% <ø> (ø)
spp_audit 72.60% <ø> (+0.06%) ⬆️
spp_base_common 90.26% <ø> (ø)
spp_case_cel 89.01% <ø> (ø)
spp_case_demo 94.34% <ø> (ø)
spp_programs 64.48% <ø> (+0.15%) ⬆️
spp_security 66.66% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant