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

[FIX] website: keep only specific views on _views_get recursive calls #31286

Closed
wants to merge 1 commit into
base: 12.0
from

Conversation

Projects
None yet
4 participants
@rdeodoo
Copy link
Contributor

rdeodoo commented Feb 20, 2019

Before this commit:
If a view A was doing a t-call on a view B and view B had view C as child.
And view A had view D as child.
And view D also t-call view B (that as mentionned above has view C as child).
And view D was inactive.

Then COWing C to set it as inactive would make get_related_views() on A to
return both generic active C and COW inactive C.

This rare case appeared for customize option that were enabled by default, like
wishlist, compare..

Disabling this customize option would show both the generic customize view and
the COW customize view.
Resulting in the customize toggle being shown twice, one enabled and one
disabled.

Detailed explanation:

  1. website_sale.products would call _views_get() with options=True for its
    t-call node website_sale.products_item, returning the full view tree,
    including COW wishlist.
  2. website_sale.products would then call _views_get() on its child views
    with options=extension.active.
    It is the case for website_sale.products_list_view, which also has a
    t-call node website_sale.products_item.
    It would then perform the same views_get() as in step 1. but with
    options set to the view active state, in this case False.
    At that point, the _view_get_inherited_children method would filter by
    active before filter_duplicate, leaving the generic view over the specific
    one as it should.

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

[FIX] website: keep only specific views on _views_get recursive calls
Before this commit:
If a view A was doing a t-call on a view B and view B had view C as child.
And view A had view D as child.
And view D also t-call view B (that as mentionned above has view C as child).
And view D was inactive.

Then COWing C to set it as inactive would make `get_related_views()` on A to
return both generic active C and COW inactive C.

This rare case appeared for customize option that were enabled by default, like
wishlist, compare..

Disabling this customize option would show both the generic customize view and
the COW customize view.
Resulting in the customize toggle being shown twice, one enabled and one
disabled.

Detailed explanation:
1. `website_sale.products` would call `_views_get()` with `options=True` for its
   t-call node `website_sale.products_item`, returning the full view tree,
   including COW wishlist.
2. `website_sale.products` would then call `_views_get()` on its child views
   with `options=extension.active`.
   It is the case for `website_sale.products_list_view`, which also has a
   t-call node `website_sale.products_item`.
   It would then perform the same `views_get()` as in step 1. but with
   `options` set to the view active state, in this case False.
   At that point, the `_view_get_inherited_children` method would filter by
   active before filter_duplicate, leaving the generic view over the specific
   one as it should.

@rdeodoo rdeodoo requested a review from JKE-be Feb 20, 2019

@robodoo robodoo added the seen 🙂 label Feb 20, 2019

@C3POdoo C3POdoo added the RD label Feb 20, 2019

@rdeodoo

This comment has been minimized.

Copy link
Contributor Author

rdeodoo commented Feb 20, 2019

@JKE-be This one was a bit tricky to understand..
Tried to be as clear as possible on the commit message.
To resume, if a view A t-call another view B and has a child view inactive D that also t-call view B
And view B has a child view C.
Then setting view C as inactive (during COW) would make get_related_views() on A to return both generic active C and COW inactive C (instead of just COW inactive C that should predomine).

It might look like a twisted case (3 levels of _views_get() recursive and a mix of _views_get() in t-call node and child views) but it is the standard case of /shop customize option like wishlist, compare..

@JKE-be

This comment has been minimized.

Copy link
Contributor

JKE-be commented Feb 20, 2019

robodoo r+ rebase-ff

@robodoo robodoo added the r+ 👌 label Feb 20, 2019

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 20, 2019

Merge method set to rebase and fast-forward

@robodoo robodoo added the CI 🤖 label Feb 20, 2019

robodoo pushed a commit that referenced this pull request Feb 20, 2019

[FIX] website: keep only specific views on _views_get recursive calls
Before this commit:
If a view A was doing a t-call on a view B and view B had view C as child.
And view A had view D as child.
And view D also t-call view B (that as mentionned above has view C as child).
And view D was inactive.

Then COWing C to set it as inactive would make `get_related_views()` on A to
return both generic active C and COW inactive C.

This rare case appeared for customize option that were enabled by default, like
wishlist, compare..

Disabling this customize option would show both the generic customize view and
the COW customize view.
Resulting in the customize toggle being shown twice, one enabled and one
disabled.

Detailed explanation:
1. `website_sale.products` would call `_views_get()` with `options=True` for its
   t-call node `website_sale.products_item`, returning the full view tree,
   including COW wishlist.
2. `website_sale.products` would then call `_views_get()` on its child views
   with `options=extension.active`.
   It is the case for `website_sale.products_list_view`, which also has a
   t-call node `website_sale.products_item`.
   It would then perform the same `views_get()` as in step 1. but with
   `options` set to the view active state, in this case False.
   At that point, the `_view_get_inherited_children` method would filter by
   active before filter_duplicate, leaving the generic view over the specific
   one as it should.

closes #31286

@robodoo robodoo closed this in edce2fd Feb 20, 2019

@robodoo robodoo added merged 🎉 and removed merging 👷 labels Feb 20, 2019

@robodoo

This comment has been minimized.

Copy link
Contributor

robodoo commented Feb 20, 2019

Merged, thanks!

@rdeodoo rdeodoo deleted the odoo-dev:12.0-fix-get-related-view-recursive-rde branch Feb 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.