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

[MIG] 11.0 website multi theme #398

Merged
merged 3 commits into from Nov 14, 2017

Conversation

Projects
None yet
4 participants
@yelizariev
Copy link
Member

yelizariev commented Nov 2, 2017

I've fixed installation problems and didn't make further testing

cc @Yajo

Yajo and others added some commits Aug 28, 2017

[ADD][website_multi_theme] Allow different themes by website (#354)
* [ADD][website_multi_theme] Allow different themes by website

This a somewhat hacky addon that adds support for setting a different theme by website.

Always updates views arch in development and demo instances.

This allows faster development, while keeping stability and no surprises in production.

* fixup! [ADD][website_multi_theme] Allow different themes by website

* fixup! fixup! [ADD][website_multi_theme] Allow different themes by website

* fixup! fixup! fixup! [ADD][website_multi_theme] Allow different themes by website

* fixup! fixup! fixup! fixup! [ADD][website_multi_theme] Allow different themes by website

* fixup! fixup! fixup! fixup! fixup! [ADD][website_multi_theme] Allow different themes by website

* fixup! fixup! fixup! fixup! fixup! fixup! [ADD][website_multi_theme] Allow different themes by website

* fixup! fixup! fixup! fixup! fixup! fixup! fixup! [ADD][website_multi_theme] Allow different themes by website

* [FIX][website_legal_page] Correctly raise a 404 error instead of a 500 if a view is not found

* fixup! fixup! fixup! fixup! fixup! fixup! fixup! [ADD][website_multi_theme] Allow different themes by website

* fixup! [FIX][website_legal_page] Correctly raise a 404 error instead of a 500 if a view is not found

* squash! fixup! [FIX][website_legal_page] Correctly raise a 404 error instead of a 500 if a view is not found

Fix tests that were getting warnings logged and random timeouts.
[FIX] website_multi_theme: Incorrect Monglia name
It's "monglia", not "mongolia".

Closes #378

@pedrobaeza pedrobaeza added this to the 11.0 milestone Nov 2, 2017

@pedrobaeza pedrobaeza referenced this pull request Nov 2, 2017

Open

Migration to version 11.0 #388

19 of 38 tasks complete
@Yajo

This comment has been minimized.

Copy link
Member

Yajo commented Nov 3, 2017

Could you please fix travis before continuing further reviews? Thanks!

@yelizariev

This comment has been minimized.

Copy link
Member

yelizariev commented Nov 3, 2017

Travis is green now

@lasley lasley removed the needs fixing label Nov 3, 2017

@Yajo

Yajo approved these changes Nov 7, 2017

Copy link
Member

Yajo left a comment

Code looks OK, although I didn't do manual tests.


- https://github.com/odoo/odoo/commit/15bf41270d3abb607e7b623b59355594cad170cf
- https://github.com/odoo/odoo/commit/7c6714d7fee4125f037ef194f9cff5235a6c5320
- https://github.com/odoo/odoo/commit/48fe0a595308722a26afd5361432f24c610b4ba0

This comment has been minimized.

@Yajo

Yajo Nov 7, 2017

Member

You can remove this now that v11 includes all of that.

@yelizariev

This comment has been minimized.

Copy link
Member

yelizariev commented Nov 7, 2017

I will do manual tests by the end of this week

@Yajo

Yajo approved these changes Nov 10, 2017

Copy link
Member

Yajo left a comment

Yikes, can't get the runbot to work... 😞 Did you do your manual tests? Code looks OK, although I'm a little worried about not manual testing because the relationship between a web page and a ir.ui.view has changed in v11

Not blocking just because of doubts

@lasley

This comment has been minimized.

Copy link
Member

lasley commented Nov 10, 2017

Runbot should be fixed now FYI. I'll be reviewing this one soon as well, thank you for the contribution @yelizariev!

@yelizariev

This comment has been minimized.

Copy link
Member

yelizariev commented Nov 10, 2017

Not sure was @Yajo's comment about runbot itself or about the module, but with latest module updates it should be much easier to try it on runbot -- I've updated demo data and added an instruction to the README file -- just one step is required to finish demo configuration.

@yelizariev

This comment has been minimized.

Copy link
Member

yelizariev commented Nov 11, 2017

Works with Bootswatch Theme

2017-11-11 12 51 57
2017-11-11 12 51 48

@Yajo
Copy link
Member

Yajo left a comment

Pretty cool 😊

<field name="page_id" ref="demo_page" />
</record>

<record id="menu_demo_page2" model="website.menu">

This comment has been minimized.

@Yajo

Yajo Nov 13, 2017

Member

Aren't you able to create these 2 menu records here instead of creating them at the top and re-editing them here at the bottom?

This comment has been minimized.

@yelizariev

yelizariev Nov 13, 2017

Member

I can't. Odoo source has the same approach. The reason in few computed fields that depends on each other. I will send details soon

This comment has been minimized.

@yelizariev

yelizariev Nov 13, 2017

Member

Well, I don't have the answer at the moment. When I did I got some problem and switched to a "splitting solution" and it began work. But now I tried to do it without splitting and it works too.

The splitting in odoo source may somehow related to splitting website.page and ir.ui.view creation for pages:

https://github.com/odoo/odoo/blob/4ecbacaf59576a22ff45615a5aa5c67244e4fb93/addons/website/data/website_data.xml#L34-L42

odoo/odoo@8baabfb#diff-d19576011e13311d9a4dd46879ebd724

response = self.url_open("http://127.0.0.1:%d" % PORT, timeout=30)
def test_0_0_0_0(self):
"""Check 0.0.0.0 downloads its default assets."""
response = self.url_open("http://0.0.0.0:%d" % PORT, timeout=30)

This comment has been minimized.

@Yajo

Yajo Nov 13, 2017

Member

Wow, TIL that you can use 0.0.0.0 to call to localhost!

@Yajo

Yajo approved these changes Nov 13, 2017

@lasley

lasley approved these changes Nov 13, 2017

Copy link
Member

lasley left a comment

Code + test, thanks @yelizariev

@yelizariev

This comment has been minimized.

Copy link
Member

yelizariev commented Nov 14, 2017

Shall I squash some of my commits? Is there any guidelines for it?

@yelizariev yelizariev changed the title [WIP] 11.0 website multi theme [MIG] 11.0 website multi theme Nov 14, 2017

@Yajo

This comment has been minimized.

Copy link
Member

Yajo commented Nov 14, 2017

Take a look at https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md#git

Just squash your migration-related commits in one, while keeping others' history.

[11.0][MIG] website_multi_theme
* update tests to new lib api (stuff related to response)

* update res.config views new framework

* doc: remove commits requirements, because odoo 11.0 has all needed updates

* update demo pages according to new framework

* ci: use 0.0.0.0 as it's used in built-in demo data

* ci: create demo page completely in xml

* doc: How to test on runbot

@yelizariev yelizariev force-pushed the yelizariev:11.0-website_multi_theme branch from 088d74c to 4bb8c4a Nov 14, 2017

@yelizariev

This comment has been minimized.

Copy link
Member

yelizariev commented Nov 14, 2017

@pedrobaeza
Copy link
Contributor

pedrobaeza left a comment

Thank you very much!

@pedrobaeza pedrobaeza merged commit 4184a88 into OCA:11.0 Nov 14, 2017

2 of 4 checks passed

codecov/patch No report found to compare against
Details
codecov/project No report found to compare against
Details
ci/runbot runbot build 3310157-398-4bb8c4 (runtime 749s)
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