10.0 mig website sale require login #138

iledarn · 2016-11-02T04:34:44Z

This depends on #137 and includes its commits. You have to review only commit e0cd9e3

yajo

I guess you rebased this on top of #137, so I'm marking this one as WIP until it gets merged.

Some of my comments go for that one, but it's not my fault, you should have said this, please take the time to write a description for your PRs from now on, and do 1 PR per addon when possible 😉

yajo · 2016-11-03T10:05:38Z

website_sale_require_login/README.rst

+
+.. image:: https://odoo-community.org/website/image/ir.attachment/5784_f2813bd/datas
+   :alt: Try me on Runbot
+   :target: https://runbot.odoo-community.org/runbot/113/9.0


yajo · 2016-11-03T10:10:19Z

website_sale_require_login/__manifest__.py

+        "web_tour",
+    ],
+    "data": [
+        "views/assets.xml",


Please put this as demo data: OCA/maintainer-tools#213 (comment)

yajo · 2016-11-03T10:18:17Z

website_sale_suggest_create_account/views/website_sale.xml

+                      not optional_products and website_sale_order and
+                      website_sale_order.website_order_line)">
+                <a class="btn btn-primary pull-right mb32"
+                   href="/web/signup?redirect=/shop/checkout">


Clicking on this gives a 404 error if I create the config parameter auth_signup.allow_uninvited and set it to True:

I'm thinking that, given the controller is now for users, probably all these patches in the view are simply not required. Just go to /shop/checkout and you will get redirected to /web/login?redirect=/shop/checkout automatically, and enable the sign up option and there it will be. It is a good moment to drop many (if not all) of this XML if possible.

yajo · 2016-11-03T10:18:37Z

website_sale_suggest_create_account/README.rst

+
+.. image:: https://odoo-community.org/website/image/ir.attachment/5784_f2813bd/datas
+   :alt: Try me on Runbot
+   :target: https://runbot.odoo-community.org/runbot/113/9.0


do 1 PR per addon when possible

@yajo, I agree about the description. But in this case when website_sale_require_login depends on website_sale_suggest_create_account I cannot separate commits.
@pedrobaeza says here OCA/hr#259:
"Second PR must include all the commits from the first one (the migration of hr_public_holidays), plus the commit/s for migrating the module hr_holidays_compute_days. This way, you can review the first PR, as well as reviewing code from specific commits in the second PR, or reviewing functionally the second one. If the first one gets merged, you rebase the second one and you have the second PR clean with only the commits for the second module."

So I did as he said

Weird, sometimes he asks for 1 and sometimes for some... 😏

Well, I reviewed commits from #137 here, so that's the consequence. At least do add a description in your pulls for us to know that before losing time on it, please 😊

@yajo, it's not weird. If you read carefully my instructions, the rule of one module per PR is still being respected. But if someone wants to review it, he/she needs all the files. The only problem is that @iledarn hadn't included proper instructions. I have added this text in the main comment:

This depends on #137 and includes its commits. You have to review only commit e0cd9e355025e75465b84fe62e59b9731c6fad45

ovnicraft · 2017-04-11T15:08:53Z

@iledarn any updates for this module ? if you resolve @yajo reviews IMO is ready to merge.

pedrobaeza · 2017-09-22T11:00:58Z

Please solve conflicts (removing the commits from website_sale_suggest_create_account)

iledarn · 2017-09-22T11:33:07Z

@pedrobaeza listo

yajo · 2017-09-25T08:16:08Z

website_sale_require_login/views/assets.xml

+
+<odoo>
+
+    <template id="assets_frontend" inherit_id="website_sale.assets_frontend">


Call the template demo_assets_frontend and move the file into demo directory.

Still missing.

iledarn · 2017-09-25T09:42:12Z

@yajo done

yajo · 2017-09-26T07:30:52Z

website_sale_require_login/__manifest__.py

        "views/website_sale.xml",
    ],
+    'demo': [
+        "demo/demo_assets_frontend.xml",


Just demo/demo_assets.xml

yajo · 2017-09-26T07:31:24Z

website_sale_require_login/views/assets.xml

+
+<odoo>
+
+    <template id="assets_frontend" inherit_id="website_sale.assets_frontend">


Still missing.

yajo · 2017-09-26T07:32:37Z

website_sale_require_login/demo/demo_assets_frontend.xml

+
+<odoo>
+
+    <template id="assets_frontend" inherit_id="website_sale.assets_frontend">


Still missing rename of this view.

iledarn · 2017-09-27T03:40:05Z

@yajo done

yelizariev · 2017-09-28T22:40:47Z

I have added unittests here: iledarn#1
@iledarn please merge

yelizariev · 2017-09-29T00:07:18Z

It's completely green now

yelizariev · 2017-10-01T16:11:46Z

Can "work in progress" tag be removed?

pedrobaeza · 2017-10-02T15:52:37Z

#172 is now merged, so you need to rebase + removing useless commits (you can do it making an interactive rebase) for having this ready to be merged.

iledarn · 2017-10-02T21:06:51Z

@pedrobaeza I've already done the ineractive rebase dropping #172 commits . There are 20 website_sale_require_login commits and 2 website_sale_wishlist commits in this PR

…ire_login to v9

pedrobaeza · 2017-10-26T16:26:49Z

Merging after organizing a bit the commit story. Thanks to all for working on having this + fixing tests.

pedrobaeza mentioned this pull request Nov 2, 2016

Migration to version 10.0 #133

Closed

15 tasks

yajo requested changes Nov 3, 2016

View reviewed changes

yajo added the work in progress label Nov 3, 2016

yajo mentioned this pull request Nov 3, 2016

10.0 mig website sale suggest create account #137

Closed

zimbora69 mentioned this pull request Nov 18, 2016

[v10] website_sale_require_login it-projects-llc/odoo-saas-tools#398

Closed

pedrobaeza mentioned this pull request Apr 11, 2017

[10.0][IMP] migrated sale_require_login and dependency #170

Closed

iledarn force-pushed the 10.0-mig-website_sale_require_login branch from 653b0dd to 2325ff4 Compare September 22, 2017 11:30

iledarn force-pushed the 10.0-mig-website_sale_require_login branch from 2325ff4 to 67265e8 Compare September 22, 2017 11:37

yajo requested changes Sep 25, 2017

View reviewed changes

yajo requested changes Sep 26, 2017

View reviewed changes

yajo approved these changes Sep 27, 2017

View reviewed changes

atchuthan approved these changes Sep 27, 2017

View reviewed changes

yajo added needs review and removed work in progress labels Oct 2, 2017

yelizariev approved these changes Oct 26, 2017

View reviewed changes

yajo and others added 4 commits October 26, 2017 18:25

Add module website_sale_require_login.

8ca8794

OCA Transbot updated translations from Transifex

e91babf

version in website_sale_require_login

1005552

website_sale_require_login: Upgrade to v9 * Migrate website_sale_requ…

5677e67

…ire_login to v9

Ildar Nasyrov and others added 4 commits October 26, 2017 18:25

[MIG] website_sale_require_login: Migrated to 10.0

668ef08

[FIX] website_sale_require_login: run unittests w/o workflow + PEP8

6315f55

[FIX] website_sale_wishlist doesn't work in test environment at least

5246336

[FIX] website_sale_require_login: name test numbering

cc01fe2

pedrobaeza force-pushed the 10.0-mig-website_sale_require_login branch from 182ede4 to cc01fe2 Compare October 26, 2017 16:26

pedrobaeza merged commit 295b68b into OCA:10.0 Oct 26, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

10.0 mig website sale require login #138

10.0 mig website sale require login #138

iledarn commented Nov 2, 2016 •

edited by pedrobaeza

Loading

yajo left a comment

yajo Nov 3, 2016

yajo Nov 3, 2016

yajo Nov 3, 2016

yajo Nov 3, 2016

iledarn Nov 3, 2016 •

edited

Loading

yajo Nov 3, 2016

pedrobaeza Nov 4, 2016 •

edited

Loading

ovnicraft commented Apr 11, 2017

pedrobaeza commented Sep 22, 2017

iledarn commented Sep 22, 2017

yajo Sep 25, 2017

yajo Sep 26, 2017

iledarn commented Sep 25, 2017

yajo Sep 26, 2017

yajo Sep 26, 2017

yajo Sep 26, 2017

iledarn commented Sep 27, 2017

yelizariev commented Sep 28, 2017

yelizariev commented Sep 29, 2017

yelizariev commented Oct 1, 2017

pedrobaeza commented Oct 2, 2017

iledarn commented Oct 2, 2017 •

edited

Loading

pedrobaeza commented Oct 26, 2017


		<odoo>

		<template id="assets_frontend" inherit_id="website_sale.assets_frontend">

10.0 mig website sale require login #138

10.0 mig website sale require login #138

Conversation

iledarn commented Nov 2, 2016 • edited by pedrobaeza Loading

yajo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iledarn Nov 3, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedrobaeza Nov 4, 2016 • edited Loading

Choose a reason for hiding this comment

ovnicraft commented Apr 11, 2017

pedrobaeza commented Sep 22, 2017

iledarn commented Sep 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iledarn commented Sep 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iledarn commented Sep 27, 2017

yelizariev commented Sep 28, 2017

yelizariev commented Sep 29, 2017

yelizariev commented Oct 1, 2017

pedrobaeza commented Oct 2, 2017

iledarn commented Oct 2, 2017 • edited Loading

pedrobaeza commented Oct 26, 2017

iledarn commented Nov 2, 2016 •

edited by pedrobaeza

Loading

iledarn Nov 3, 2016 •

edited

Loading

pedrobaeza Nov 4, 2016 •

edited

Loading

iledarn commented Oct 2, 2017 •

edited

Loading