Skip to content
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

[9.0][ADD] website_sale_checkout_skip_payment: New module to skip pay ment step in checkout process #193

Merged

Conversation

sergio-teruel
Copy link

Copy link
Member

@yajo yajo left a comment

Choose a reason for hiding this comment

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

Almost perfect, only some details. Please rebase to include 6f15c68 and fix travis and runbot.

==================================

This module allows to logged users buy without payment step.
The sale order is confirmed on ckeckout payment.
Copy link
Member

Choose a reason for hiding this comment

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

"on checkout without payment" maybe?

@@ -0,0 +1,23 @@
# -*- coding: utf-8 -*-
# © 2017 Sergio Teruel <sergio.teruel@tecnativa.com>
Copy link
Member

Choose a reason for hiding this comment

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

Use Copyright instead of ©️ : OCA/maintainer-tools#197

'name': 'Website Sale Checkout Skip Payment',
'summary': 'Skip payment for logged users in checkout process',
'version': '9.0.1.0.0',
'category': 'E-commerce',
Copy link
Member

Choose a reason for hiding this comment

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

"Website" (that's website_sale's category)

partner = self.env['res.users'].browse(
request.session.uid).partner_id
if partner.skip_website_checkout_payment:
rec.checkout_skip_payment = True
Copy link
Member

Choose a reason for hiding this comment

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

You can do rec.checkout_skip_payment = partner.skip_website_checkout_payment

for rec in self:
if request.session.uid:
partner = self.env['res.users'].browse(
request.session.uid).partner_id
Copy link
Member

Choose a reason for hiding this comment

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

I think you can do request.env.user.partner_id instead.

@@ -0,0 +1,19 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- © 2017 Sergio Teruel <sergio.teruel@tecnativa.com>
Copy link
Member

Choose a reason for hiding this comment

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

Copyright

@@ -0,0 +1,19 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- © 2017 Sergio Teruel <sergio.teruel@tecnativa.com>
License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl-3). -->
Copy link
Member

Choose a reason for hiding this comment

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

Wasn't this LGPL?

</div>
</div>
</div>
</t>
Copy link
Member

Choose a reason for hiding this comment

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

Please use consistent indentation.

@rafaelbn rafaelbn added this to the 9.0 milestone Jul 31, 2017
Copy link
Member

@yajo yajo left a comment

Choose a reason for hiding this comment

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

Please add WEBSITE_REPO="1" to travis global variables to make us able to test runbot.

<div id="wrap">
<div class="container oe_website_sale">
<div class="alert alert-danger" role="alert">
<strong>Error!</strong> An error occurred when try to
Copy link
Member

Choose a reason for hiding this comment

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

trying

@chienandalu
Copy link
Member

@yajo Shall I do it in a different PR or in this one

@yajo
Copy link
Member

yajo commented Jul 31, 2017

This one is OK

@chienandalu chienandalu force-pushed the 9.0-PR-website_sale_checkout_skip_payment branch from e7b065b to 88b9e74 Compare July 31, 2017 12:47
Copy link
Member

@yajo yajo left a comment

Choose a reason for hiding this comment

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

Couldn't test on runbot due to OCA/maintainer-quality-tools#471; cc @moylop260 @lasley any ideas?

Code OK

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

I miss configuration part in the README saying that you mark the partners you want to allow to skip checkout.

@yajo
Copy link
Member

yajo commented Aug 2, 2017

It's under usage, isn't it?

@pedrobaeza
Copy link
Member

Well, it should be configuration then, and it's the contrary: you should activate it, not deactivate it.

@chienandalu
Copy link
Member

Yes, although I can extend the explanation a bit more.

@yajo
Copy link
Member

yajo commented Aug 2, 2017

Hmm not sure, maybe somebody wants to enable this by default... Can you do it with partner custom default values? Or should we add a config parameter? Or am I saying something absurd? 😆

@rafaelbn
Copy link
Member

rafaelbn commented Aug 2, 2017

I miss configuration part in the README saying that you mark the partners you want to allow to skip checkout.

If you need this module is because by default you want to skip payment for all registered customers (remember if not registered normal process apply). Registered customers doesn't mean that a res.partner exists, means that a res.partner has access to portal -> has a res.user associated. So this module is for skip payment for all customers which has access to the portal.

So I don't think adding the Checkbox "Skip Website Checkout Payment" to the partner and activating by default it the correct way.

Better as said, all res.partner with res.user skip the payment. If you think is needed a check-box for exceptions, then add it, but unable by default (false) and enable it for the exception.

Copy link
Member

@rafaelbn rafaelbn left a comment

Choose a reason for hiding this comment

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

I will test in runbot

This module allows to logged users to checkout with no payment step. At the
end of the checkout proccess the quotation is sent to the user email address
and set to *Qoutation Sent* state.

Copy link
Member

Choose a reason for hiding this comment

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

Add: "The quotation created in the website_sale will be the same as if it would be created in backend, so all fields from the partner will be in the sale order as "pricelist" "payment term" "sales person", etc..."

==================================
Website Sale Checkout Skip Payment
==================================

Copy link
Member

Choose a reason for hiding this comment

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

This module wants to cover the situation that you current customers could make their own orders maintaining their payment term and payment mode and not paying with a bank transfer, PayPal or similar.


You can deactivate this option for a particular partner unchecking
"Skip Website Checkout Payment" field.

Copy link
Member

Choose a reason for hiding this comment

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

Remember that if you customer is not registered it will go though the standard process

@pedrobaeza
Copy link
Member

I don't agree. The check should be False by default.

@rafaelbn
Copy link
Member

rafaelbn commented Aug 2, 2017

Could we assume that travis is nor related?

2017-08-01 17:04:57,059 15103 INFO openerp_test openerp.tests.common: phantomjs: </phantomLog>

2017-08-01 17:04:57,060 15103 INFO openerp_test openerp.tests.common: phantom_run execution finished

2017-08-01 17:04:57,062 15103 ERROR openerp_test openerp.addons.website_sale_qty.tests.test_js: FAIL

2017-08-01 17:04:57,083 15103 INFO openerp_test openerp.addons.website_sale_qty.tests.test_js: ======================================================================

2017-08-01 17:04:57,084 15103 ERROR openerp_test openerp.addons.website_sale_qty.tests.test_js: FAIL: test_js_tour (openerp.addons.website_sale_qty.tests.test_js.TestJS)

2017-08-01 17:04:57,085 15103 ERROR openerp_test openerp.addons.website_sale_qty.tests.test_js: Traceback (most recent call last):

2017-08-01 17:04:57,085 15103 ERROR openerp_test openerp.addons.website_sale_qty.tests.test_js: `   File "/home/travis/build/OCA/e-commerce/website_sale_qty/tests/test_js.py", line 17, in test_js_tour

2017-08-01 17:04:57,086 15103 ERROR openerp_test openerp.addons.website_sale_qty.tests.test_js: `     login='admin',

2017-08-01 17:04:57,087 15103 ERROR openerp_test openerp.addons.website_sale_qty.tests.test_js: `   File "/home/travis/odoo-9.0/openerp/tests/common.py", line 441, in phantom_js

2017-08-01 17:04:57,088 15103 ERROR openerp_test openerp.addons.website_sale_qty.tests.test_js: `     self.phantom_run(cmd, timeout)

2017-08-01 17:04:57,088 15103 ERROR openerp_test openerp.addons.website_sale_qty.tests.test_js: `   File "/home/travis/odoo-9.0/openerp/tests/common.py", line 393, in phantom_run

2017-08-01 17:04:57,088 15103 ERROR openerp_test openerp.addons.website_sale_qty.tests.test_js: `     "PhantomJS test completed without reporting success; "

2017-08-01 17:04:57,088 15103 ERROR openerp_test openerp.addons.website_sale_qty.tests.test_js: ` AssertionError: None is not true : PhantomJS test completed without reporting success; the log may contain errors or hints.

2017-08-01 17:04:57,089 15103 INFO openerp_test openerp.addons.website_sale_qty.tests.test_js: Ran 1 test in 56.730s

2017-08-01 17:04:57,089 15103 ERROR openerp_test openerp.addons.website_sale_qty.tests.test_js: FAILED

2017-08-01 17:04:57,089 15103 INFO openerp_test openerp.addons.website_sale_qty.tests.test_js:  (failures=1)

2017-08-01 17:04:57,090 15103 INFO openerp_test openerp.modules.module: openerp.addons.website_sale_qty.tests.test_js tested in 56.74s, 2053 queries

2017-08-01 17:04:57,090 15103 ERROR openerp_test openerp.modules.module: Module website_sale_qty: 1 failures, 0 errors

2017-08-01 17:04:57,092 15103 INFO openerp_test openerp.modules.module: openerp.addons.website_sale_qty.tests.test_product_template running tests.

2017-08-01 17:04:57,093 15103 INFO openerp_test openerp.addons.website_sale_qty.tests.test_product_template: test_price_quantity_tiers_first_variant_price_rule (openerp.addons.website_sale_qty.tests.test_product_template.TestProductTemplate)

@rafaelbn
Copy link
Member

rafaelbn commented Aug 2, 2017

About #193 (comment)

I don't agree. The check should be False by default.

I agree. The check should be False by default. 👍 (that's what I said)

But enabling it to True will indicate that the partner doesn't skip payment.

@yajo
Copy link
Member

yajo commented Aug 2, 2017

IMHO adding true values for false results is misleading, so it's OK as it is: true-> skip, false-> don't skip.

About the default value, I guess you can add an ir.values if you want a different default from addon's, so it does not seem much important honestly.

@yajo
Copy link
Member

yajo commented Aug 2, 2017

About travis, rebuilding now that OCA/maintainer-quality-tools#471 is fixed.

@rafaelbn rafaelbn changed the title [9.0][WIP] website_sale_checkout_skip_payment: New module to skip pay ment step in checkout process [9.0][ADD] website_sale_checkout_skip_payment: New module to skip pay ment step in checkout process Aug 2, 2017
@chienandalu chienandalu force-pushed the 9.0-PR-website_sale_checkout_skip_payment branch from fdd3039 to d95e8e6 Compare August 2, 2017 14:04
@chienandalu
Copy link
Member

There is some issue with this repo's runbot 😖

Unable to find image 'oca-e-commerce:60a6fd5_3' locally
docker: Error response from daemon: repository oca-e-commerce not found: does not exist or no pull access.
See 'docker run --help'.

@lasley
Copy link

lasley commented Aug 2, 2017

There is some issue with this repo's runbot 😖

That happens randomly on mine too. Rebuilding doesn't seem to help, but an amend and force push does

@chienandalu chienandalu force-pushed the 9.0-PR-website_sale_checkout_skip_payment branch 2 times, most recently from c23eb0a to 6e762f1 Compare August 3, 2017 08:35
Copy link
Member

@yajo yajo left a comment

Choose a reason for hiding this comment

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

Code OK, couldn't test in runbot. Can you rebase and see if last MQT patches work please?

@chienandalu chienandalu force-pushed the 9.0-PR-website_sale_checkout_skip_payment branch from 6e762f1 to 5d80d71 Compare August 8, 2017 07:59
@chienandalu
Copy link
Member

@yajo Still the same compass issue

@yajo
Copy link
Member

yajo commented Aug 8, 2017

Yes, I guess we won't be able to test until #195 gets fixed 😢

@moylop260
Copy link

I just have created a commit for this branch with the fix: 563b540
But if we have a similar error in the future we won't have a error because: OCA/maintainer-quality-tools#478
Regards!

@moylop260
Copy link

Now the CIs are green

And using ssh connection to the build#3300622 of runbot: ssh -p 8425 odoo@runbot1.odoo-community.org

Running with --test-enable the result was successfully:

~/odoo-9.0/openerp-server --db-filter=openerp_test -d openerp_test --xmlrpc-port=18069 --test-enable --stop-after-init -u website_sale_qty,website_sale_default_country,website_sale_product_brand,website_sale_checkout_skip_payment,product_multi_link,website_sale_require_login,website_sale_wishlist,website_sale_vat_required,website_sale_suggest_create_account,website_sale_checkout_country_vat,website_sale_require_legal --log-level=error

  • screen shot 2017-08-11 at 15 36 44

@lasley
Copy link

lasley commented Aug 11, 2017

@moylop260 to the rescue! 🎉

@moylop260
Copy link

@lasley
@moylop260 to the rescue! 🎉

debugging

@chienandalu
Copy link
Member

Thanks, guys! 😄 👍

@pedrobaeza pedrobaeza force-pushed the 9.0-PR-website_sale_checkout_skip_payment branch from 563b540 to d6a7b3d Compare August 14, 2017 10:08
@pedrobaeza pedrobaeza merged commit ca69663 into OCA:9.0 Aug 14, 2017
@pedrobaeza pedrobaeza deleted the 9.0-PR-website_sale_checkout_skip_payment branch August 14, 2017 10:08
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.

7 participants