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

11.0 website multi theme port from 10 #450

Merged
merged 18 commits into from May 23, 2018

Conversation

Projects
None yet
7 participants
@yelizariev
Copy link
Member

yelizariev commented Apr 23, 2018

WIP
We will check it internally first.

yelizariev and others added some commits Mar 14, 2018

[FIX] point inherit to copied views (#430)
* [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] website_multi_theme: Manage only Qweb views
Current implementation disabled all backend views from a theme addon when converting it to multiwebsite mode.

This is now fixed, and includes a migration script to make things work again if you hit this situation in your database.
[IMP] make customize_show views part of multi-theme
New field "auto" in "website.theme.asset" to distinguish assets
created from theme and ones created in xml. It's needed, for example,
for non-module themes (like Demo theme) be handled in _convert_assets
(before this update such themes were skipped)

[REF] field multi_theme_generated is replaced with origin_view_id

[IMP] apply default theme after installation

[IMP] reload themes on installing new modules with customize_show

[CI] Website always have Multi-Theme now. Also, increase default
timeout (10 sec) to 60 sec, because it may take more time now
[DOC] remove incorrect note
on installing module with customize_show Reload is done automatically
@emagdalenaC2i

This comment has been minimized.

Copy link

emagdalenaC2i commented Apr 24, 2018

Is not migrated already? Check #398

@pedrobaeza

This comment has been minimized.

Copy link
Contributor

pedrobaeza commented Apr 24, 2018

This is a recent improvement.

@Railia

Railia approved these changes Apr 24, 2018

Copy link

Railia left a comment

👍

@yelizariev

This comment has been minimized.

Copy link
Member Author

yelizariev commented Apr 24, 2018

@emagdalenaC2i #398 was made by me 😉

@yelizariev

This comment has been minimized.

Copy link
Member Author

yelizariev commented Apr 30, 2018

I'll add more updates from 10.0 after #453 and #452

yelizariev added some commits Apr 28, 2018

[FIX] error on applying multi-theme with snippets on few websites
closes #410

[CI] test coverage for duplicated snippets
[FIX] proper place for context
which avoids recursive reloading
[PORT] port updates from 10.0
* new name for res.config
* move snippets lower, because website's tour use 4th element

@rafaelbn rafaelbn added this to the 11.0 milestone May 2, 2018

@yelizariev

This comment has been minimized.

Copy link
Member Author

yelizariev commented May 3, 2018

New updates to be merged here: #457 #456

yelizariev added some commits May 1, 2018

@yelizariev

This comment has been minimized.

Copy link
Member Author

yelizariev commented May 7, 2018

This error is randomly shown on travis checks

ssertionError: ("TypeError: undefined is not an object (evaluating 'manipulatorOffset.top')\n"
' at '
'http://127.0.0.1:8069/web/content/512-6922043/web_editor.assets_editor.js:132 '
'(in cover)\n'
' at '
'http://127.0.0.1:8069/web/content/512-6922043/web_editor.assets_editor.js:133 '
'(in toggleFocus)\n'
' at '
'http://127.0.0.1:8069/web/content/512-6922043/web_editor.assets_editor.js:162\n'
' at http://127.0.0.1:8069/web/content/509-3597dce/web.assets_common.js:802\n'
' at http://127.0.0.1:8069/web/content/509-3597dce/web.assets_common.js:796 '
'(in fire)\n'
' at http://127.0.0.1:8069/web/content/509-3597dce/web.assets_common.js:797 '
'(in add)\n'
' at http://127.0.0.1:8069/web/content/509-3597dce/web.assets_common.js:802\n'
' at http://127.0.0.1:8069/web/content/509-3597dce/web.assets_common.js:625 '
'(in each)\n'
' at http://127.0.0.1:8069/web/content/509-3597dce/web.assets_common.js:802\n'
' at http://127.0.0.1:8069/web/content/509-3597dce/web.assets_common.js:803 '
'(in Deferred)\n'
' at http://127.0.0.1:8069/web/content/509-3597dce/web.assets_common.js:802 '
'(in then)\n'
' at '
'http://127.0.0.1:8069/web/content/512-6922043/web_editor.assets_editor.js:162 '
'(in _activateSnippet)\n'
' at '
'http://127.0.0.1:8069/web/content/512-6922043/web_editor.assets_editor.js:150\n'
' at http://127.0.0.1:8069/web/content/509-3597dce/web.assets_common.js:892 '
'(in dispatch)\n'
' at http://127.0.0.1:8069/web/content/509-3597dce/web.assets_common.js:865 '
'(in handle)\n'
' at :0 (in dispatchEvent)\n'
' at http://127.0.0.1:8069/web/content/509-3597dce/web.assets_common.js:4187 '
'(in trigger_mouse_event)\n'
' at http://127.0.0.1:8069/web/content/509-3597dce/web.assets_common.js:4187 '
'(in _click)\n'
' at http://127.0.0.1:8069/web/content/509-3597dce/web.assets_common.js:4187 '
'(in _text)\n'
' at http://127.0.0.1:8069/web/content/509-3597dce/web.assets_common.js:4186 '
'(in text)\n'
' at http://127.0.0.1:8069/web/content/509-3597dce/web.assets_common.js:4204\n'
' at :0\n'
' at http://127.0.0.1:8069/web/content/509-3597dce/web.assets_common.js:44\n'
'(leaf frame on top)')

@yelizariev

This comment has been minimized.

Copy link
Member Author

yelizariev commented May 7, 2018

I'd like to set status "Needs review" to this issue and postpone with #456 which is not merged yet

@Yajo Yajo added needs review and removed work in progress labels May 14, 2018

@Railia

Railia approved these changes May 14, 2018

@yelizariev

This comment has been minimized.

Copy link
Member Author

yelizariev commented May 15, 2018

now #456 is in this PR too, i.e. all updates from 10.0 are here

@yelizariev yelizariev referenced this pull request May 15, 2018

Closed

11.0 merge fcb85d2 #307

[DOC] note about addint Default Theme to *Sub-themes*
Also, change 127.0.0.1 to 0.0.0.0 because latter is used in odoo for second demo
website
@yelizariev

This comment has been minimized.

Copy link
Member Author

yelizariev commented May 17, 2018

Checked internally and found misunderstanding about Multi-footer. I've added a note to documentation in latest commit

@ilmir-k
Copy link

ilmir-k left a comment

tested 👍
looks good to me

@Yajo

Yajo approved these changes May 22, 2018

Copy link
Member

Yajo left a comment

@yelizariev Is this ready to merge then?

@yelizariev

This comment has been minimized.

Copy link
Member Author

yelizariev commented May 22, 2018

@Yajo yes, it's ready. We did all we can. We will continue to test it with website_multi_company module once this PR is merged

@pedrobaeza

This comment has been minimized.

Copy link
Contributor

pedrobaeza commented May 22, 2018

Please squash a bit some unneeded commits and I'll merge. Thanks for the work checking these rough edges.

@yelizariev

This comment has been minimized.

Copy link
Member Author

yelizariev commented May 22, 2018

I found and done another thing that was in discussion #409 (comment)

@yelizariev

This comment has been minimized.

Copy link
Member Author

yelizariev commented May 22, 2018

@pedrobaeza most commits were cherry-picked. I think it's better to keep them to easy comparing with 10.0 branch.

@Yajo

Yajo approved these changes May 23, 2018

Copy link
Member

Yajo left a comment

Code & functional review

<odoo>

<record model="ir.module.category" id="base.module_category_theme">
<field name="exclusive" eval="False"/>

This comment has been minimized.

Copy link
@Yajo

Yajo May 23, 2018

Member

I can't believe it was so simple! Could you backport this to v10? 🤔 Or was it hardcoded in v10?

This comment has been minimized.

Copy link
@yelizariev

yelizariev May 23, 2018

Author Member

Yes, it's hardcoded and the only way is rewrite (not inherit) one method

@Yajo Yajo merged commit fe94b15 into OCA:11.0 May 23, 2018

4 checks passed

ci/runbot runbot build 3326717-450-8f3c0b (runtime 818s)
Details
codecov/patch 93.87% of diff hit (target 80.99%)
Details
codecov/project 87.89% (+6.9%) compared to 544f287
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
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.