Skip to content
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

[REF] check_xml #36373

Closed
wants to merge 10 commits into from

Conversation

@Xavier-Do
Copy link
Contributor

Xavier-Do commented Sep 3, 2019

Add a way to test filter (global post_install test on ir.filters and migration test for views + additionnal validation in check_xml for views + corresponding fixes

Also removes a deprecated module for graph view in order to simplify ir.ui.view

Install of all modules average time goes from ~8m to ~6m31

@robodoo robodoo added the seen 🙂 label Sep 3, 2019
@C3POdoo C3POdoo added the RD label Sep 3, 2019
@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-test-filters-xdo branch from 58eaeec to c9d1027 Sep 3, 2019
@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-test-filters-xdo branch 2 times, most recently from 0b60e1a to a60c68d Sep 3, 2019
@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Sep 3, 2019
@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-test-filters-xdo branch 8 times, most recently from 6bad271 to be9a7a9 Sep 4, 2019
@robodoo robodoo added the CI 🤖 label Sep 6, 2019
@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-test-filters-xdo branch from 99062ad to b9c9539 Sep 10, 2019
@robodoo robodoo removed the CI 🤖 label Sep 10, 2019
@@ -208,7 +208,7 @@
</group>
<group>
<field name="user_id" domain="[('share', '=', False)]"
context="{'default_groups_ref': ['base.group_user', 'base.group_partner_manager', 'sales_team.group_sale_salesman_all_leads'], 'team_id': team_id}"/>

This comment has been minimized.

Copy link
@tde-banana-odoo

tde-banana-odoo Sep 10, 2019

Contributor

Oooh remaining of our famous "create users with some default groups" ... snif. Good ol' openerp.

@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-test-filters-xdo branch 8 times, most recently from 38eec68 to 1ae8bfa Sep 10, 2019
@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Sep 17, 2019
@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Nov 13, 2019
@Xavier-Do Xavier-Do force-pushed the odoo-dev:master-test-filters-xdo branch 2 times, most recently from 14bf925 to f6fbbe0 Nov 13, 2019
Xavier-Do added 3 commits Sep 5, 2019
Model presence in env is already done by postprocess_and_fields,
and model shouldn't change in postprocess exept for a subview,
in which case post_process_and_field will be called again.
@rco-odoo rco-odoo force-pushed the odoo-dev:master-test-filters-xdo branch from 036f71a to 9d10aae Nov 13, 2019
@rco-odoo rco-odoo marked this pull request as ready for review Nov 13, 2019
@robodoo robodoo added the CI 🤖 label Nov 13, 2019
@rco-odoo

This comment has been minimized.

Copy link
Member

rco-odoo commented Nov 13, 2019

@robodoo rebase-ff r+

@robodoo robodoo added the r+ 👌 label Nov 13, 2019
@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Nov 13, 2019

Merge method set to rebase and fast-forward

@rco-odoo

This comment has been minimized.

Copy link
Member

rco-odoo commented Nov 13, 2019

@robodoo robodoo removed the r+ 👌 label Nov 13, 2019
Xavier-Do added 2 commits Oct 24, 2019
View creation/edition represent a important part of an install and a lot
of possible view errors are not detected, like fields used in domain
filters. Some part of the code a difficult to maintain, and view checks
are splitted in multiple places.

This commit aims at refactoring view validation by regrouping most part
of the logic in ir_ui_view and trying to optimize the overall process.

Since most of the lines were touched, this task was also an opportunity
to modernize the API.

Main changes on method `check_xml`:
 - extract node processign and validation to individual postprocessor
 - add validation for filter node, buttons, ...
 - fix accessibility checks (and improve their performance)
 - move xpath check to specific Python node validator
 - clarify error messages (wip to continue)

Main changes on method `read_combined`:
 - optimize the search for inheriting views in a single query doing the
   whole recursive search

Indeed, after removing xpath validations, `get_inheriting_views_arch`
was the most expensive method in `check_xml`, spending most of the time
in `search` because of recursive calls to retrieve children views.

The view Backend Assets is a good example of the latter point, since a
line is added in the view for almost every module.  70 views (community)
are added at first level, `get_inheriting_views_arch` is efficient and
returns all 70 views.  Then at the second level, the method
`get_inheriting_views_arch` is called 70 times for nothing. 70 calls to
`search` (squared/2 since each view is checked independently) are almost
useless. The same case applies to the settings view.

As a result, the average module installation time is 25% faster, and the
average time spent in `read_combined` is almost divided by 2.
@rco-odoo rco-odoo force-pushed the odoo-dev:master-test-filters-xdo branch from 9d10aae to f653d0c Nov 13, 2019
@rco-odoo

This comment has been minimized.

Copy link
Member

rco-odoo commented Nov 13, 2019

@robodoo robodoo added r+ 👌 CI 🤖 and removed CI 🤖 labels Nov 13, 2019
robodoo pushed a commit that referenced this pull request Nov 13, 2019
closes #36373

Signed-off-by: Raphael Collet (rco) <rco@openerp.com>
@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Nov 13, 2019

Merged at 8bb0530, thanks!

@robodoo robodoo closed this Nov 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.