[MIG][website_sale_require_legal] Migrate to v10 #205

yajo · 2017-09-06T10:58:50Z

Normal migration.

@Tecnativa

rafaelbn

Go to runbot (don't make login)
Buy and ipad
Fill the form
You will see field "Country" cannot be modified + as is a mandatory field you cannot continue buying

Please fix, thanks!

rafaelbn · 2017-09-06T12:53:33Z

@yajo it doesnt' work as expected check #205 (review)

yajo · 2017-09-07T07:30:02Z

It should work now.

pedrobaeza

Tested on runbot

chienandalu

Tested on runbot 👍

chienandalu · 2017-09-06T15:45:03Z

website_sale_require_legal/views/website_sale.xml

+    </template>
+
+    <template id="address" inherit_id="website_sale.address">
+        <xpath expr="//div[@class='clearfix'][last()]" position="before">


Maybe if you put it after or add another clearfix before the legal block it wouldn't hide the country selector.

yajo · 2017-09-08T07:29:01Z

@rafaelbn please re-review

rafaelbn · 2017-10-08T23:00:13Z

Hi @yajo , rebuilding runbot for testing! Test are pending isn't it? Thanks!

yajo · 2017-10-27T13:02:01Z

This has tests now

pedrobaeza · 2017-11-22T13:54:38Z

Travis is failing

yajo · 2017-11-30T11:18:37Z

The tour wasn't being loaded, let's see now.

pedrobaeza · 2017-11-30T16:36:47Z

Still red.

pedrobaeza · 2018-02-23T15:51:50Z

@yajo Travis is still red. Maybe a rebase?

yajo · 2018-03-27T11:46:18Z

Rebased.

pedrobaeza · 2018-03-29T12:08:02Z

This is still in red

rafaelbn

Thanks but travis is stilll 🔴

rafaelbn · 2018-04-27T00:34:45Z

If travis 💚 @yajo merge!

grindtildeath

Just wondering if it's been considered, but address page won't appear for connected user with an address already existing. Shouldn't the acceptance checkbox be moved to 'checkout' or 'payment' page ?
Or at least state this in the README ?

grindtildeath · 2018-05-03T08:49:16Z

website_sale_require_legal/views/website_sale.xml

+<odoo>
+
+    <template id="accept_input">
+        <div t-attf-class="form-group col-md-12 #{'has-error' if error.get('accepted_legal_terms') else ''}">


if error.get(...) should be if error and error.get(..) to avoid following rendering error :

QWebException: 'NoneType' object has no attribute 'get' Traceback (most recent call last): File "/opt/odoo/src/odoo/addons/base/ir/ir_qweb/qweb.py", line 315, in _compiled_fn return compiled(self, append, values, options, log) File "<template>", line 1, in template_website_sale_require_legal_accept_input_50 AttributeError: 'NoneType' object has no attribute 'get' Error to render compiling AST AttributeError: 'NoneType' object has no attribute 'get' Template: website_sale_require_legal.accept_input Path: /templates/t/div Node: <div t-attf-class="form-group col-md-12 #{'has-error' if error.get('accepted_legal_terms') else ''}"> <label for="accepted_legal_terms" class="control-label"> <input type="checkbox" name="accepted_legal_terms" id="accepted_legal_terms" required="required" data-oe-id="1617" data-oe-model="ir.ui.view" data-oe-field="arch" data-oe-xpath="/t[1]/div[1]/label[1]/input[1]"/> <t t-call="website_legal_page.acceptance_full"/> </label> </div>

I opened Tecnativa#3 as it's blocking for us 😉

pedrobaeza · 2018-05-03T09:44:19Z

@grindtildeath do you give your approval here then?

grindtildeath · 2018-05-03T12:16:59Z

@pedrobaeza I tried it out on runbot without our customization, and to me the module doesn't work as expected, since you can place an order without having to accept explicitly.
IMO the validation checkbox should appear on the payment page, because we need legal things to be confirmed to place an order, and not to enter any address....

pedrobaeza · 2018-05-03T23:21:22Z

Not really, as with new GDPR, giving your address and clicking on continue, is registering personal information, so we need the legal page here.

yajo · 2018-05-15T10:29:56Z

It should go 💚 now.

It turns out that on v10, tour matches must be case sensitive.

rafaelbn · 2018-05-15T11:42:06Z

@yajo this is still not GDPR compliance as metadata of acceptance is not stored in the customer. Please review. Thanks

rafaelbn · 2018-05-25T13:55:53Z

This matter has been double-checked with Lawyer and this approach is valid and legal.

About your @grindtildeath doubt

you'll skip the checkbox and can register an order without having to explicitly accept the conditions

This happens when the customer is already registered so he/she has accepted the terms before. If you change terms, then you should inform to all customers in other way. But not, is not needed to put another checkbox. Said that, it could be an improvement to allow more options in this module but we cannot afford this now because the main objective it migrating this module with GDPR compliance approach which is in the form when user fill the personal data and accept legal advice and term and conditions of the ecommerce.

Please @chienandalu finish this PR with the metadata info

Thanks

grindtildeath · 2018-05-25T15:00:07Z

@rafaelbn Again, I understand your point of view and the goal is legit, but I persist in thinking you need to reconsider the name of the module as well as its description.
For example, if this module is called website_gdpr_compliance, or website_address_registration_require_legal, or anything which is not what it is actually, I have no issues with it.
But IMO my point still stands, that is : website_sale_require_legal is misleading, because acceptance of legal stuff is required to register information and not to register a sale order.

As I don't want to block your work, I see two easy solutions :

Use a module name which is closer to what the module does
or
At the very least explain this loophole in the issues roadmap section of the readme.

Don't you think that would make it much easier for who wants to use this module ?

pedrobaeza · 2018-05-25T15:31:57Z

OK, we will add it to the know issues / roadmap section, as the technical name is correct due to the dependencies and this module is already in several versions with this name.

rafaelbn · 2018-05-25T18:11:14Z

OK, let's go with At the very least explain this loophole in the issues roadmap section of the readme.

This modules was needed before GDPR (comes from 2016 in v8), this is for improving it making it GDPR compliance.

@grindtildeath I really thank for you point of view (22 days ago #205 (review)) which make me to call and write some emails to be sure with several opinions (outsite Tecnativa, outside Odoo software). This make me be sure, really sure on what we make. Asking Lawyers, Ecommerce Manager and trying some other ecommerce that are not Odoo but Prestashop o Magento. 😄 Really we want the best solution 👍

IMHO:

The module name is correct as it requires accept legal advice.
The module is missing (in you opinion) is a new one called website_sale_require_term_of_use which apply in the Payment doesn't need yo have a checkbox. Just a text bellow (see Amazon example when I'm already registered)

Said that, please send me 2 or 3 ecommerce in EU to test them in both, legal and terms of use cases.

Thanks

rafaelbn

Tested in runbot. After filling the form http://3327035-205-68b4d3.runbot2.odoo-community.org/shop/address and click next you get
500: Internal Server Error

yajo · 2018-05-28T07:05:25Z

website_sale_require_legal/controllers/main.py

+                    )
+                )
+            partner_id = request.env['res.partner'].browse(res)
+            partner_id.message_post(body=metadata)


I imagine this message should be of type log. Don't you need to specify that? We don't want this to reach the buyer, right?

chienandalu · 2018-05-28T17:09:31Z

@rafaelbn I couldn't reproduce your error. Can you detail STR?

rafaelbn · 2018-05-29T19:40:30Z

@chienandalu please check video: https://drive.google.com/file/d/1ppuxpDWWMciCQGWj_C5IFA6dBPptmABm/view?usp=sharing

MeetKD · 2018-05-30T19:17:20Z

website_sale_require_legal/__manifest__.py

+    "version": "10.0.1.0.0",
+    "category": "Website",
+    "website": "https://www.tecnativa.com",
+    "author": "Tecnativa, "


'website': 'https://github.com/OCA/e-commerce',

chienandalu · 2018-05-31T13:26:41Z

@rafaelbn Can you test again?

chienandalu · 2018-05-31T15:07:00Z

I don't know why LINT is failing now 😕

simahawk · 2018-06-01T06:39:48Z

@chienandalu

website_sale_wishlist/README.rst:7: [E7901(rst-syntax-error), ]  Duplicate explicit target name: "your shop".

this should be the cause (we have stricter rules now for RST).

chienandalu · 2018-06-01T09:41:02Z

Right! Fix here: #234

rafaelbn

Tested 👍

rafaelbn · 2018-09-28T22:50:37Z

Hello @grindtildeath ,

Shouldn't the acceptance checkbox be moved to 'checkout' or 'payment' page ?

For your need you have an standard functionality. You don't need this module.

Regards

cc @yajo @carlosdauden @sergio-teruel @chienandalu @pedrobaeza

sergio-teruel · 2018-09-29T19:09:07Z

Yes, I think it too...

yajo self-assigned this Sep 6, 2017

yajo added the needs review label Sep 6, 2017

yajo added this to the 10.0 milestone Sep 6, 2017

rafaelbn self-requested a review September 6, 2017 11:01

rafaelbn requested changes Sep 6, 2017

View reviewed changes

pedrobaeza approved these changes Sep 7, 2017

View reviewed changes

chienandalu approved these changes Sep 7, 2017

View reviewed changes

pedrobaeza mentioned this pull request Oct 26, 2017

Migration to version 10.0 #133

Closed

15 tasks

yajo force-pushed the 10.0-website_sale_require_legal branch from f10a2c6 to a780d3a Compare March 27, 2018 11:45

rafaelbn approved these changes Apr 27, 2018

View reviewed changes

grindtildeath reviewed May 3, 2018

View reviewed changes

yajo force-pushed the 10.0-website_sale_require_legal branch from 0a057fd to c925bda Compare May 15, 2018 10:27

yajo added the work in progress label May 16, 2018

rafaelbn requested changes May 25, 2018

View reviewed changes

yajo commented May 28, 2018

View reviewed changes

chienandalu force-pushed the 10.0-website_sale_require_legal branch from 68b4d38 to 9e6007b Compare May 28, 2018 15:00

rafaelbn mentioned this pull request May 30, 2018

10.0 website_signup_legal_page_required OCA/website#467

Merged

MeetKD reviewed May 30, 2018

View reviewed changes

yajo and others added 6 commits June 1, 2018 12:01

[ADD] website_sale_require_legal

e8a8ff4

[9.0] [MIG] website_sale_require_legal module

4c37640

OCA Transbot updated translations from Transifex

d1b8b9d

[MIG][website_sale_require_legal] Migrate to v10

6de9b52

website_sale_require_legal: Fix rendering error when error list is empty

9fc2aa0

[IMP] record metadata on registration and acceptance of conditions

ef3cc2a

chienandalu force-pushed the 10.0-website_sale_require_legal branch from 5d41aac to ef3cc2a Compare June 1, 2018 10:02

rafaelbn added needs review and removed work in progress labels Jun 1, 2018

rafaelbn approved these changes Jun 1, 2018

View reviewed changes

rafaelbn merged commit d65fe8a into OCA:10.0 Jun 1, 2018

rafaelbn deleted the 10.0-website_sale_require_legal branch June 1, 2018 11:22

rafaelbn mentioned this pull request Jun 5, 2018

11.0 mig website_sale_require_legal #237

Merged

[MIG][website_sale_require_legal] Migrate to v10 #205

[MIG][website_sale_require_legal] Migrate to v10 #205

Conversation

yajo commented Sep 6, 2017

rafaelbn left a comment

Choose a reason for hiding this comment

rafaelbn commented Sep 6, 2017

yajo commented Sep 7, 2017

pedrobaeza left a comment

Choose a reason for hiding this comment

chienandalu left a comment

Choose a reason for hiding this comment

chienandalu Sep 6, 2017

Choose a reason for hiding this comment

yajo commented Sep 8, 2017

rafaelbn commented Oct 8, 2017

yajo commented Oct 27, 2017

pedrobaeza commented Nov 22, 2017

yajo commented Nov 30, 2017

pedrobaeza commented Nov 30, 2017

pedrobaeza commented Feb 23, 2018

yajo commented Mar 27, 2018

pedrobaeza commented Mar 29, 2018

rafaelbn left a comment

Choose a reason for hiding this comment

rafaelbn commented Apr 27, 2018 • edited by pedrobaeza Loading

grindtildeath left a comment • edited Loading

Choose a reason for hiding this comment

grindtildeath May 3, 2018

Choose a reason for hiding this comment

pedrobaeza commented May 3, 2018

grindtildeath commented May 3, 2018 • edited Loading

pedrobaeza commented May 3, 2018

yajo commented May 15, 2018

rafaelbn commented May 15, 2018

rafaelbn commented May 25, 2018

grindtildeath commented May 25, 2018

pedrobaeza commented May 25, 2018

rafaelbn commented May 25, 2018

rafaelbn left a comment

Choose a reason for hiding this comment

yajo May 28, 2018

Choose a reason for hiding this comment

chienandalu commented May 28, 2018

rafaelbn commented May 29, 2018

MeetKD May 30, 2018

Choose a reason for hiding this comment

chienandalu commented May 31, 2018

chienandalu commented May 31, 2018

simahawk commented Jun 1, 2018

chienandalu commented Jun 1, 2018

rafaelbn left a comment

Choose a reason for hiding this comment

rafaelbn commented Sep 28, 2018

sergio-teruel commented Sep 29, 2018 • edited Loading

rafaelbn commented Apr 27, 2018 •

edited by pedrobaeza

Loading

grindtildeath left a comment •

edited

Loading

grindtildeath commented May 3, 2018 •

edited

Loading

sergio-teruel commented Sep 29, 2018 •

edited

Loading