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

[IMP] sale : pay down payment #30481

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@Feyensv
Copy link

Feyensv commented Jan 23, 2019

Description of the issue/feature this PR addresses: Partial payment amount to confirm quotations

Current behavior before PR: Quotation auto-confirmation through e-sign and/or full payment

Desired behavior after PR is merged: Quotation auto-confirmation through e-sign and/or full/down payment

  • sale:
    • Percentage of payment required in Settings if Online confirmation through payment is activated (% of quotation total amount, default = 100%)
    • Application in mail templates and quotation portal
  • payment : Display of the amount of transactions (for manual acquirers)
  • sale_management : Quotation templates support of required payment percentage

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@robodoo robodoo added the seen 🙂 label Jan 23, 2019

@C3POdoo C3POdoo added the RD label Jan 23, 2019

@Feyensv Feyensv changed the title Master sale pay down payment vfe [IMP] sale : pay down payment Jan 24, 2019

@robodoo robodoo added the CI 🤖 label Jan 25, 2019

@Feyensv Feyensv force-pushed the odoo-dev:master-sale-pay-down-payment-vfe branch Jan 25, 2019

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Jan 25, 2019

@Feyensv Feyensv force-pushed the odoo-dev:master-sale-pay-down-payment-vfe branch to 0652403 Feb 20, 2019

@robodoo robodoo added CI 🤖 and removed CI 🤖 labels Feb 20, 2019

@Feyensv Feyensv force-pushed the odoo-dev:master-sale-pay-down-payment-vfe branch from 0652403 to 11f6923 Mar 19, 2019

@robodoo robodoo removed the CI 🤖 label Mar 19, 2019

@Feyensv Feyensv force-pushed the odoo-dev:master-sale-pay-down-payment-vfe branch 5 times, most recently from dc8fefe to c27d6f0 Mar 19, 2019

@robodoo robodoo added the CI 🤖 label Mar 19, 2019

@jem-odoo
Copy link
Contributor

jem-odoo left a comment

Small review on "la forme" (and not "le fond")
guidelines, naming, ...

Show resolved Hide resolved addons/sale/models/res_config_settings.py Outdated
Show resolved Hide resolved addons/sale/models/res_company.py
Show resolved Hide resolved addons/sale/models/sale.py Outdated
Show resolved Hide resolved addons/sale/models/sale.py Outdated
Show resolved Hide resolved addons/sale/models/sale.py Outdated
Show resolved Hide resolved addons/sale/models/sale.py Outdated
Show resolved Hide resolved addons/sale_management/models/sale_order_template.py Outdated
Show resolved Hide resolved addons/website_sale/security/website_sale.xml Outdated

@Feyensv Feyensv force-pushed the odoo-dev:master-sale-pay-down-payment-vfe branch from c27d6f0 to 1698e90 Mar 20, 2019

@robodoo robodoo removed the CI 🤖 label Mar 20, 2019

@Feyensv Feyensv force-pushed the odoo-dev:master-sale-pay-down-payment-vfe branch from 1698e90 to a5478f1 Mar 20, 2019

@Feyensv Feyensv requested a review from jem-odoo Mar 20, 2019

@Feyensv Feyensv force-pushed the odoo-dev:master-sale-pay-down-payment-vfe branch from a5478f1 to a1ff47a Mar 20, 2019

@robodoo robodoo added the CI 🤖 label Mar 20, 2019

Show resolved Hide resolved addons/sale/models/payment.py Outdated
Show resolved Hide resolved addons/sale/models/res_config_settings.py Outdated
Show resolved Hide resolved addons/sale/models/res_company.py
@tde-banana-odoo
Copy link
Contributor

tde-banana-odoo left a comment

First technical review: models, global code architecture

Show resolved Hide resolved addons/sale/models/payment.py Outdated
Show resolved Hide resolved addons/sale/models/res_company.py
Show resolved Hide resolved addons/sale/models/res_config_settings.py Outdated
Show resolved Hide resolved addons/sale/models/res_config_settings.py Outdated
@@ -142,6 +145,10 @@ def _get_payment_type(self):
require_payment = fields.Boolean('Online Payment', default=_get_default_require_payment, readonly=True,
states={'draft': [('readonly', False)], 'sent': [('readonly', False)]},
help='Request an online payment to the customer in order to confirm orders automatically.')
required_payment_percentage = fields.Float('Payment Amount', default=_default_required_payment_percentage, readonly=True,

This comment has been minimized.

@tde-banana-odoo

tde-banana-odoo Mar 25, 2019

Contributor

I would rename it to require_confirmation_percentage to match the other fields naming convention (require_ stuff and _confirmation_percentage). That way grepping confirmation_percentage allows you to find a bit everything related to it. Also check through the whole diff naming convention is correctly used everywhere.

@@ -501,6 +529,11 @@ def _create_invoices(self, grouped=False, final=False):
invoices_name = {}

for order in self:
if order.has_to_be_down_paid():

This comment has been minimized.

@tde-banana-odoo

tde-banana-odoo Mar 25, 2019

Contributor

I am not sure it should be done here. Shouldn't the caller check that it is effectively in a case where invoices can be created before calling _create_invoices instead of simply skipping it without notifying anybody ? Purpose of this method is to create invoice based on SO. I think caller should ensure it is doable. This method should have standard I/O .

This comment has been minimized.

@tde-banana-odoo

tde-banana-odoo Mar 25, 2019

Contributor

cc @jem-odoo the man who knows everything about invoice / so confirmation (or not, maybe)

This comment has been minimized.

@jem-odoo

jem-odoo Mar 25, 2019

Contributor

Yes, I agree with tde ! Otherwise, invoice for given SO will not be created and the user is not warned. I think we can say that all record in self should be invoicable, as prerequisite.

This comment has been minimized.

@Feyensv

Feyensv Mar 25, 2019

Author

Two cases :

  1. Draft/Sent : if the user pays for the SO, it is to confirm it and as the specs says, no invoice should be generated for the partial payment of confirmation (the invoices have to be generated manually afterwards). If the confirmation payment = 100% of SO amount, the invoice is generated.
  2. Confirmed : has_to_be_paid/down_paid == False and invoices are generated manually with the wizard on the SO.
    Invoices are never generated for unconfirmed quotations (I don't see cases where it can happen).

Therefore, I only avoid the case where "automatic invoice" is checked and a partial payment is done for the SO so that it doesn't generate an invoice (a warning is added in the settings on this subject).

This comment has been minimized.

@Feyensv

Feyensv Mar 26, 2019

Author

Moved to payment.py in _invoice_sale_orders to avoid modifying the general _create_invoices in sale.py.

Show resolved Hide resolved addons/sale/models/sale.py Outdated
Show resolved Hide resolved addons/sale/views/sale_portal_templates.xml Outdated
@@ -24,6 +27,7 @@ def _get_default_require_payment(self):
help='Number of days for the validity date computation of the quotation')
require_signature = fields.Boolean('Online Signature', default=_get_default_require_signature, help='Request a online signature to the customer in order to confirm orders automatically.')
require_payment = fields.Boolean('Online Payment', default=_get_default_require_payment, help='Request an online payment to the customer in order to confirm orders automatically.')
required_payment_percentage = fields.Float('Payment Amount', default=_default_required_payment_percentage, help='Percentage of payment requested to the customer in order to confirm orders automatically.')

This comment has been minimized.

@tde-banana-odoo

tde-banana-odoo Mar 25, 2019

Contributor

Note: just update fields accordingly of other fields (require_confirmation_percentage if this name is chosen for example).

Show resolved Hide resolved addons/payment/views/payment_portal_templates.xml Outdated

@Feyensv Feyensv force-pushed the odoo-dev:master-sale-pay-down-payment-vfe branch from a1ff47a to 18be946 Mar 25, 2019

@robodoo robodoo removed the CI 🤖 label Mar 25, 2019

@Feyensv Feyensv force-pushed the odoo-dev:master-sale-pay-down-payment-vfe branch from 18be946 to 40375a0 Mar 25, 2019

@robodoo robodoo added the CI 🤖 label Mar 25, 2019

@Feyensv Feyensv force-pushed the odoo-dev:master-sale-pay-down-payment-vfe branch from 40375a0 to 02e04e8 Mar 26, 2019

@robodoo robodoo removed the CI 🤖 label Mar 26, 2019

@Feyensv Feyensv force-pushed the odoo-dev:master-sale-pay-down-payment-vfe branch from 02e04e8 to bc56bd4 Mar 26, 2019

@robodoo robodoo added the CI 🤖 label Mar 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.