[WIP] Migrate website_logo to v9

[lasley]

Tested & implemented suggestion from #190 + code upgraded to new OCA guidelines.

[pedrobaeza]

Follow lates README template: https://raw.githubusercontent.com/OCA/maintainer-tools/master/template/module/README.rst

[lasley]

Ouch @coveralls - I'll add tests 😉

[lasley]

I have no idea how to get these tests to pass. I have tried everything from a standard .write on the company, to straight up SQLing the logo in the right place.

It looks like OpenERP decided to not test this section of their code either; maybe the same problems?

Regardless, I have rebased away from my tests if we want to merge as is. I'm open to pointers as well, if a better Odoo-er than me wants to take a look - https://github.com/laslabs/website/blob/feature/9.0/website_logo_test/website_logo/tests/test_logo.py

[pedrobaeza]

Controllers are usually tested with PhantomJS tests, but you can import and call directly the method as an alternative method, but you will have to deal with the answer as an object.

[lasley]

Interesting, I thought I was using Phantom helpers in the HttpCase with the url_open. TIL, thanks @pedrobaeza I will try your suggestions 😄

[lasley]

Alright woot got it!

I wrote them in Phantom at first, but wasn't getting the coverage I was looking for; so I reverted to the good ol' mock pattern. It seems the Phantom tests are designed pretty heavily around the tours, which somewhat conflicted with the scope of this module.

This is ready IMO

[pedrobaeza]

Please put only 2016, because TODAY is relative 😉

[coveralls]

Coverage increased (+10.4%) to 70.37% when pulling 9af84d5 on laslabs:feature/9.0/website_logo into 6088e65 on OCA:9.0.

[pedrobaeza]

👍

[eLBati]

Please replace project_repo

[eLBati]

@lasley looking at the diff from 8.0 version
http://pastie.org/10809125
it seems some changes are missing, like 1616b12

Please add them, thanks

[lasley]

Wow I really need to do better at following up on my PRs.

@eLBati & @pedrobaeza - Sorry if this should be more clear, but is there a way for me to extract pieces of a commit in order to be included in this PR? The only way I can think of to bring commits such as c3bd993 in is by literally copying the diff and creating a new commit. I feel like there should be a more elegant way here, like some sort of partial cherry pick, but I am totally at a loss.

[coveralls]

Coverage increased (+10.4%) to 70.37% when pulling d4b96da on laslabs:feature/9.0/website_logo into 6088e65 on OCA:9.0.

[pedrobaeza]

This is what I do on 9.0 branch (you can use the migration guide I made for 9.0 as inspiration: https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-9.0):

git fetch origin
git checkout origin/8.0
git checkout -b 8.0-extract
git filter-branch --subdirectory-filter website_logo
git filter-branch -f --tree-filter 'mkdir -v website_logo ; git mv -k * website_logo' HEAD

Now I have the module isolated in one branch. I see the log (git log) and copy the commit numbers of the OCA Transbot. I go now again to the 9.0 branch, and make git cherry-pick <commit> for each of the commits I want to bring, but now I'm sure this commit only makes modifications over my module.

[lasley]

This is amazing, thank you @pedrobaeza - this is really going to help me move all my vertical medical PRs up too (we're about 200 commits ahead of the PRs at this point, and I was honestly dreading that rebase nightmare).

I will play around, and update this PR with the results. Thanks again, as always 😄

[lasley]

Migrations removed

[rafaelbn]

Rebuilding for testing

[rafaelbn]

This instructions are not valid in v9. Please change where you can change logo.

[rafaelbn]

Hi @lasley , I don't see any please to change logo:

[lasley]

Hi @rafaelbn - Thanks for the review. ReadMe instructions updated to correspond, although I am thinking these might have been invalid for v8 too.

For easy reference:

• Navigate to Company Settings, such as by clicking logo on left nav bar
• Click the Configuration tab
• Load a logo image in the field under the Website logo header

[lasley]

Hmmm so after comparing deeper between current v8 and this branch, I think maybe the rebase swapped some things. I am going to tag this with a WIP, and will be able to take a look sometime this week.

[coveralls]

Coverage increased (+7.5%) to 67.521% when pulling f148c99 on laslabs:feature/9.0/website_logo into 5c85477 on OCA:9.0.

[pedrobaeza]

Is this completed?

[lasley]

TBH I can't remember. It seems to work in my local, but I haven't been able to get Runbot to build. Do we have some sort of active issue with Runbot at the moment? Hmmm ok maybe the status was just incredibly slow to update. I'll double check this and let everyone know.

[pedrobaeza]

It seems that it's working properly now, so no clue.

[eLBati]

Hello @lasley
please also check travis:

Error details:
View inheritance may not use attribute 'string' as a selector.

[lasley]

Superseded by #245