[FIX] point inherit to copied views - #430
Conversation
|
@yajo 😃 |
|
Travis has ❌ |
a138aa7 to
08c5743
Compare
|
it's fixed and squashed Also, I've added separate commit to fix jslints |
|
@pedrobaeza, could you take a look? |
| copied_view.inherit_id = custom_layout | ||
| data.attrib["inherit_id"] = custom_layout.key | ||
| copied_view.arch = etree.tostring(data) | ||
| elif (copied_view.inherit_id and |
There was a problem hiding this comment.
This is very hard to read. Can you somehow split this to be more readable (an else, then prepare variables and make the comparison)?
efa0bf9 to
4c9b1da
Compare
| layout_pattern = self.env.ref("website_multi_theme.layout_pattern") | ||
|
|
||
| def view2module(view): | ||
| return view and view.model_data_id.module |
There was a problem hiding this comment.
I'm not sure if you know that Odoo lets you browse empty recordsets, returning a falsey value for all fields.
Thus, you can use a variable instead of a method if you want, just below line 141 you can write:
copied_view_module = copied_view.model_data_id.moduleIt will be False if len(copied_view)==0.
There was a problem hiding this comment.
Yes, I know. For some reason, I thought that it could be False in the middle of the chain, so I decided just to make the extra check. But it's not needed here. You are right
|
Please rebase after merging the other patch |
32154aa to
aeede0a
Compare
Otherwise theme_customize doesn't work when we have chain of views like in theme_clean: theme_clean.option_bg_shade_light2 inherits theme_clean.less theme_clean.less inherits website.assets_frontend without this update, website_multi_theme copies theme_clean.option_bg_shade_light2 but keeps inherit_id the same (i.e. to theme_clean.less, which is not used, because it has a copy)
aeede0a to
94248f6
Compare
|
Rebased. |
|
@pedrobaeza @yajo could you review the updates please. Thanks |
yajo
left a comment
There was a problem hiding this comment.
Code looks good. It would be nice to have some testing on it.
|
Can you add those tests? |
|
Not soon... |
|
I prefer to have the fix than not having tests for it. It can be a good future improvement, but I'm not blocking just for that. Thanks @yelizariev! |
* [FIX] point inherit to copied views Otherwise theme_customize doesn't work when we have chain of views like in theme_clean: theme_clean.option_bg_shade_light2 inherits theme_clean.less theme_clean.less inherits website.assets_frontend without this update, website_multi_theme copies theme_clean.option_bg_shade_light2 but keeps inherit_id the same (i.e. to theme_clean.less, which is not used, because it has a copy) * [LINT] remove unused arguments, add semicolon
* [FIX] point inherit to copied views Otherwise theme_customize doesn't work when we have chain of views like in theme_clean: theme_clean.option_bg_shade_light2 inherits theme_clean.less theme_clean.less inherits website.assets_frontend without this update, website_multi_theme copies theme_clean.option_bg_shade_light2 but keeps inherit_id the same (i.e. to theme_clean.less, which is not used, because it has a copy) * [LINT] remove unused arguments, add semicolon
* [FIX] point inherit to copied views Otherwise theme_customize doesn't work when we have chain of views like in theme_clean: theme_clean.option_bg_shade_light2 inherits theme_clean.less theme_clean.less inherits website.assets_frontend without this update, website_multi_theme copies theme_clean.option_bg_shade_light2 but keeps inherit_id the same (i.e. to theme_clean.less, which is not used, because it has a copy) * [LINT] remove unused arguments, add semicolon
Otherwise, theme_customize doesn't work when we have a chain of views like in
theme_clean:theme_clean.option_bg_shade_light2inheritstheme_clean.lesstheme_clean.lessinheritswebsite.assets_frontendwithout this update,
website_multi_themecopiestheme_clean.option_bg_shade_light2but keepsinherit_idthe same (i.e. totheme_clean.less, which is not used, because it has a copy)