Skip to content

[10.0] [MIG] website_legal_page - #353

Merged
pedrobaeza merged 5 commits into
OCA:10.0from
LasLabs:hurrinico-10.0-mig-website_legal_page
Jun 30, 2017
Merged

pedrobaeza merged 5 commits into
OCA:10.0from
LasLabs:hurrinico-10.0-mig-website_legal_page

Conversation

@lasley

@lasley lasley commented Jun 13, 2017

Copy link
Copy Markdown
Contributor

I didn't want to keep submitting PRs to an edge branch, so I'm superseding #310 with this.

@yajo - I completed your #310 (review) in 8f3c7b2, which I will squash into 311c6ca before merge.

@lasley lasley added this to the 10.0 milestone Jun 13, 2017
@lasley
lasley force-pushed the hurrinico-10.0-mig-website_legal_page branch 3 times, most recently from aaeaf4b to acd5fab Compare June 13, 2017 18:34
@lasley
lasley requested a review from yajo June 13, 2017 20:25

@yajo yajo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check bots please.

Comment thread website_legal_page/controllers/main.py Outdated
website=True,
)
def show_legal_page(self, **kwargs):
return self.page('website_legal_page.%s' % page, kwargs)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fail! page is undefined.

</template>

<template id="website.legal" name="Legal advice" page="True">
<template id="legal" name="Legal Advice" page="True">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

id should be advice instead, as in line 9

</template>

<template id="website.privacy" name="Privacy policy" page="True">
<template id="privacy" name="Privacy Policy" page="True">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ID here must be privacy-policy, like in line 9

</template>

<template id="website.terms" name="Terms of use" page="True">
<template id="terms" name="Terms of use" page="True">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here, terms-of-use

response = self.url_open('/legal/no-page')
self.assertEqual(
response.getcode(),
404,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to add a test like this for just /legal


def test_privacy(self):
""" It should return a 200 for a defined page. """
response = self.url_open('/legal/privacy')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to add a test like this for each of the 3 pages.
BTW, you said it would be privacy-policy instead. Check below.

@lasley

lasley commented Jun 14, 2017

Copy link
Copy Markdown
Contributor Author

Hah bots are part of the reason that I just took the PR over - I obviously tested nothing! Fixed.

@lasley
lasley force-pushed the hurrinico-10.0-mig-website_legal_page branch 2 times, most recently from 5e3d9dc to 3e6ff23 Compare June 14, 2017 18:53

@yajo yajo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, it seems good enough for me 😊

However, I still want to add a couple of warnings we might want to fix now:

  1. Current design of pages will hit odoo/odoo#15437.
  2. Changing XMLIDS of legal pages will make old ones disappear and new ones get new. I'm pretty sure many people will not want to reach their lawyers again to re-fix their lost legal terms, so a migration script from <10.0 versions would be very desirable.

@lasley

lasley commented Jun 15, 2017

Copy link
Copy Markdown
Contributor Author

Hmmm I'm not sure I totally understand the number 1 point. What do I need to do here in order to avoid that?

Regarding the migration script - isn't that better suited for OpenUpgrade?

@pedrobaeza

Copy link
Copy Markdown
Member

In this case I don't think we should provide separate views for "content containers", as someone may want to modify the entire legal page without keeping the default layout.

@yajo

yajo commented Jun 16, 2017

Copy link
Copy Markdown
Member

What do I need to do here in order to avoid that?

Yes, @pedrobaeza, but just in case @lasley wants to avoid that: just replace those <div class="oe_content"/> by <t t-call="website_legal_page.${page}_content"/> and adding that view as a separate one that just contains <div class="oe_content"/>.

lasley and others added 2 commits June 30, 2017 11:06
* Move legal pages to explicit /legal routes

Fixes by PR review

Fix my obvious lack of testing
- Update copyright headers.
- Update copyright holders data.
- Remove .pot file.
- Add migration script.
@yajo
yajo force-pushed the hurrinico-10.0-mig-website_legal_page branch from 3e6ff23 to 37bf49a Compare June 30, 2017 09:15
@yajo

yajo commented Jun 30, 2017

Copy link
Copy Markdown
Member

@lasley I took the freedom to squash your last commits to get it ready to merge. I also added the required migration script, so this should be ready to merge now.

@yajo yajo self-assigned this Jun 30, 2017
yajo added a commit to hurrinico/website that referenced this pull request Jun 30, 2017
- Privacy policy URL changed.
- Contributors data updated.
- Move templates to templates folder, as per guidelines.
@pedrobaeza pedrobaeza mentioned this pull request Jun 30, 2017
35 tasks
@pedrobaeza

Copy link
Copy Markdown
Member

Merging then

@pedrobaeza
pedrobaeza merged commit 5411bca into OCA:10.0 Jun 30, 2017
yajo added a commit to hurrinico/website that referenced this pull request Jun 30, 2017
- Privacy policy URL changed.
- Contributors data updated.
- Move templates to templates folder, as per guidelines.
@lasley

lasley commented Jun 30, 2017

Copy link
Copy Markdown
Contributor Author

You're awesome, thanks @yajo! Been backed up with the deploys and totally wasn't able to get back to this. Excellent community effort!

@lasley
lasley deleted the hurrinico-10.0-mig-website_legal_page branch June 30, 2017 18:03
@yajo

yajo commented Jul 4, 2017

Copy link
Copy Markdown
Member

Anyone interested on this please take a look at #358

lasley pushed a commit to LasLabs/website that referenced this pull request Aug 11, 2017
- Privacy policy URL changed.
- Contributors data updated.
- Move templates to templates folder, as per guidelines.
njeudy pushed a commit to njeudy/website that referenced this pull request Jan 11, 2018
- Privacy policy URL changed.
- Contributors data updated.
- Move templates to templates folder, as per guidelines.
njeudy pushed a commit to njeudy/website that referenced this pull request Jan 23, 2018
- Privacy policy URL changed.
- Contributors data updated.
- Move templates to templates folder, as per guidelines.
njeudy pushed a commit to njeudy/website that referenced this pull request Jan 26, 2018
- Privacy policy URL changed.
- Contributors data updated.
- Move templates to templates folder, as per guidelines.
eLBati pushed a commit to eLBati/website that referenced this pull request Oct 1, 2018
- Privacy policy URL changed.
- Contributors data updated.
- Move templates to templates folder, as per guidelines.
ernestotejeda pushed a commit to Tecnativa/website that referenced this pull request Nov 20, 2018
- Privacy policy URL changed.
- Contributors data updated.
- Move templates to templates folder, as per guidelines.
pedrobaeza pushed a commit to Tecnativa/website that referenced this pull request Dec 13, 2018
- Privacy policy URL changed.
- Contributors data updated.
- Move templates to templates folder, as per guidelines.
bjornbillen pushed a commit to bjornbillen/website that referenced this pull request Jan 15, 2020
- Privacy policy URL changed.
- Contributors data updated.
- Move templates to templates folder, as per guidelines.
bjornbillen pushed a commit to bjornbillen/website that referenced this pull request Feb 7, 2020
- Privacy policy URL changed.
- Contributors data updated.
- Move templates to templates folder, as per guidelines.
Tardo pushed a commit to Tecnativa/website that referenced this pull request Apr 14, 2020
- Privacy policy URL changed.
- Contributors data updated.
- Move templates to templates folder, as per guidelines.
Tardo pushed a commit to Tecnativa/website that referenced this pull request Apr 14, 2020
- Privacy policy URL changed.
- Contributors data updated.
- Move templates to templates folder, as per guidelines.
ivantodorovich pushed a commit to odoo-it/website that referenced this pull request Dec 28, 2020
- Privacy policy URL changed.
- Contributors data updated.
- Move templates to templates folder, as per guidelines.
ivantodorovich pushed a commit to odoo-it/website that referenced this pull request Apr 20, 2021
- Privacy policy URL changed.
- Contributors data updated.
- Move templates to templates folder, as per guidelines.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants