Merge lp:~akretion-team/openobject-addons/trunk-payment-term-on-purchase-v2 into lp:openobject-addons

[Alexis de Lattre]

This is a new version of my merge proposal to add payment term on purchase orders.

My previous merge proposal https://code.launchpad.net/~akretion-team/openobject-addons/trunk-payment-term-on-purchase/+merge/90872 was refused by Quentin for a number of reasons. I have addresses Quentin's objections in this new merge proposal :

1. We now have 2 payment terms on res.partner : one when the partner is a customer ("Customer payment term"), another one when the partner is a supplier ("Supplier payment term")

2. I have a better handling of the cases where no payment_term are given on the PO, as suggested by Quentin : when there is no payment term on the PO, the "Supplier payment term" is used

I hope that this merge proposal will be accepted. If you would like more modifications in it, please tell me and I will work on it.

[Fabien (Open ERP) (fp-tinyerp)]

Hello,

I would tend to refuse this merge proposal. I's a good contribution but I propose to put it in a separate modules.

Most of the time, when you do a Purchase Order, you just want to set a due date provided by the supplier (like on the supplier invoice) and not create a complex payment term object that reflect the due date. As every supplier may have different payment terms or due dates, it's complex to create a payment term on each PO.

So, in my opinion, the official way of working on OpenERP should be to just set a due date on the PO and not a payment term. This is the most simple solution and most SMEs will prefer that way of working because it's easier to record a PO.

But I agree some companies may need a complex payment term management (as an example in the case where you negociate a payment term with your supplier for future purchaes). In that case, those companies will install the module of Alexis.

review: Disapprove

[Alexis de Lattre (alexis-via)]

Dear Fabien,

You say "Most of the time, when you do a Purchase Order, you just want to set a due date provided by the supplier", but it doesn't make sense to me : when your write a PO, you won't print a "due date" on your PO, you will print the "Payment term" that will apply on the supplier invoice that you will receive after the delivery of the goods/services. How could you print a "due date" on the PO when you don't know the delivery date yet (I suppose that delivery date = invoice date) ?

Let's take an example :
On 01/01/2012, Company A sends a PO to company B to buy a laptop. On the PO, it displays "Payment term = 30 days net". The laptop is delivered on 15/01/2012. On the invoice, company B wrote :
- date = 15/01/2012
- payment term = 30 days net
- due date = 14/02/2012.
When company A generates the supplier invoice in it's ERP, they may just want to set the "due date" that is displayed on the invoice... but they still needed to have the "payment term" displayed on the PO !

I could agree if you had said "When you do a Supplier invoice, you just want to set the due date that is displayed on the supplier invoice".

So I here is what I propose :
- we create a field 'supplier_payment_term' on res.partner
- we create a field 'payment_term' on purchase.order and we display it on the PO's report
- on purchase.order, the on_change of partner_id copies the "supplier_payment_term" of res.partner to the "payment_term" of purchase.order (that way, if there is a specific payment term for a particular PO, the user can change it on the PO without impacting the default payment term of future POs)
- when invoice control = "Based on receptions", the wizard asks for the invoice date AND the due date (it currently only asks for the invoice date). The payment term of the PO is NOT copied on the supplier invoice.
- when invoice control = "Based on generated draft invoice", we don't change the current behaviour. The payment term of the PO is NOT copied on the supplier invoice.

If you agree with what I propose, I will prepare a new merge proposal.

In fact, it was not a good thing to copy the "payment_term" from the PO to the supplier invoice (as I did in this merge proposal) because, if, on the supplier invoice, you manually set the "invoice date" and the "payment term" and the "due date" and you click on "Create", the "due date" is re-computed from the payment term and the invoice date just before the real validation. So you may end-up having a due date that is different from the due date that you entered manually by looking at the "due date" displayed on the supplier invoice.

[Ana Juaristi Olalde (ajuaristio)]

Payment terms on PO is needed functionality. It should be included on core as said by Raphaël and Alexis.
It's needed verifying that the due date coming in invoice is correct regarding the agreement you made on po.
We have got it included on l10n_es since 5.0 If it's also needed in other countries, I think it should be on core. Just my opinion.

[Stefan Rijnhart (Opener) (stefan-opener)]

+1

Stefan

review: Approve (functional)

[Alexis de Lattre (alexis-via)]

Just a short comment to inform you that Akretion is working on a third version of the merge proposal to add payment term on purchase.order. This third merge proposal will include :
- the remarks made by Quentin on the first (refused) merge proposal
https://code.launchpad.net/~akretion-team/openobject-addons/trunk-payment-term-on-purchase/+merge/90872
- the remarks made by Fabien on this merge proposal (= the second one, also refused). In fact, this third merge proposal will implement what I proposed 3 comments above this comment.

The code is being prepared in this branch : https://code.launchpad.net/~akretion-team/openobject-addons/trunk-payment-term-on-purchase-v3

The code is mostly written, but we can't test it because of this bug :
https://bugs.launchpad.net/openobject-server/+bug/1001926
(there is another bug concerning the generation of invoice from picking : https://bugs.launchpad.net/openobject-addons/+bug/1001806 but I wrote a patch for this one, so it's not blocking).

[qdp (OpenERP) (qdp)]

as per the last comment of Alexis, this merge proposal is deprecated.

Let's reject this one for the sake of "having-the-most-rejected-merge-proposal-that-finally-land-to-trunk" and let's focuss on what happens in v3 of this MP.

See you,
Quentin

Unmerged revisions

6459. By Alexis de Lattre on 2012-02-14

Remove useless 'a' variable.

6458. By Alexis de Lattre on 2012-02-14

Took into account the remarks made by Quentin de Paoli on my previous MP :
1. add a property "Supplier payment term" on res.partner
. rename the existing property to "Customer payment term"
. in the res.partner form view, add the "Supplier payment term" next to the "Customer payment term"
. modify the code of the 'account' module to use the supplier or customer payment term
  depending on the journal
2. better handle the case where no payment_term is given on the PO : if there is no payment_ter
  m on the PO, we use the payment term on the partner.

6457. By Alexis de Lattre on 2012-01-31

Remove my code change on fiscal_position (it had no impact... but I guess it is better not to change anything else)

6456. By Alexis de Lattre on 2012-01-31

Add payment_term on purchase.order object.
The behavior of payment_term is the same as for the sale order :
- the payment_term is copied from the partner to the purchase order
- then it is copied from the purchase order to the supplier invoice
Add payment_term to the purchase.order form view, next to fiscal position (same as for sale.order form view)

=== modified file 'account/account.py'
--- account/account.py	2012-02-14 13:30:53 +0000
+++ account/account.py	2012-02-14 18:43:08 +0000
@@ -2291,8 +2291,12 @@
                     if not line.partner_id:
                         raise osv.except_osv(_('Error !'), _("Maturity date of entry line generated by model line '%s' of model '%s' is based on partner payment term!" \
                                                                 "\nPlease define partner on it!")%(line.name, model.name))
-                    if line.partner_id.property_payment_term:
+                    payment_term_id = False
+                    if model.journal_id.type in ('purchase', 'purchase_refund') and line.partner_id.property_supplier_payment_term:
+                        payment_term_id = line.partner_id.property_supplier_payment_term.id
+                    elif line.partner_id.property_payment_term:
                         payment_term_id = line.partner_id.property_payment_term.id
+                    if payment_term_id:
                         pterm_list = pt_obj.compute(cr, uid, payment_term_id, value=1, date_ref=date_maturity)
                         if pterm_list:
                             pterm_list = [l[0] for l in pterm_list]

=== modified file 'account/account_invoice.py'
--- account/account_invoice.py	2012-02-14 12:25:20 +0000
+++ account/account_invoice.py	2012-02-14 18:43:08 +0000
@@ -434,10 +434,11 @@
 
             if type in ('out_invoice', 'out_refund'):
                 acc_id = p.property_account_receivable.id
+                partner_payment_term = p.property_payment_term and p.property_payment_term.id or False
             else:
                 acc_id = p.property_account_payable.id
+                partner_payment_term = p.property_supplier_payment_term and p.property_supplier_payment_term.id or False
             fiscal_position = p.property_account_position and p.property_account_position.id or False
-            partner_payment_term = p.property_payment_term and p.property_payment_term.id or False
             if p.bank_ids:
                 bank_id = p.bank_ids[0].id
 

=== modified file 'account/account_move_line.py'
--- account/account_move_line.py	2012-02-13 18:07:41 +0000
+++ account/account_move_line.py	2012-02-14 18:43:08 +0000
@@ -652,17 +652,24 @@
             return {'value':val}
         if not date:
             date = datetime.now().strftime('%Y-%m-%d')
+        jt = False
+        if journal:
+            jt = journal_obj.browse(cr, uid, journal).type
         part = partner_obj.browse(cr, uid, partner_id)
 
-        if part.property_payment_term:
-            res = payment_term_obj.compute(cr, uid, part.property_payment_term.id, 100, date)
+        payment_term_id = False
+        if jt and jt in ('purchase', 'purchase_refund') and part.property_supplier_payment_term:
+            payment_term_id = part.property_supplier_payment_term.id
+        elif jt and part.property_payment_term:
+            payment_term_id = part.property_payment_term.id
+        if payment_term_id:
+            res = payment_term_obj.compute(cr, uid, payment_term_id, 100, date)
             if res:
                 val['date_maturity'] = res[0][0]
         if not account_id:
             id1 = part.property_account_payable.id
             id2 =  part.property_account_receivable.id
-            if journal:
-                jt = journal_obj.browse(cr, uid, journal).type
+            if jt:
                 if jt in ('sale', 'purchase_refund'):
                     val['account_id'] = fiscal_pos_obj.map_account(cr, uid, part and part.property_account_position or False, id2)
                 elif jt in ('purchase', 'sale_refund'):

=== modified file 'account/partner.py'
--- account/partner.py	2011-08-24 21:19:43 +0000
+++ account/partner.py	2012-02-14 18:43:08 +0000
@@ -180,9 +180,16 @@
             'account.payment.term',
             type='many2one',
             relation='account.payment.term',
-            string ='Payment Term',
-            view_load=True,
-            help="This payment term will be used instead of the default one for the current partner"),
+            string ='Customer Payment Term',
+            view_load=True,
+            help="This payment term will be used instead of the default one for sale orders and customer invoices"),
+        'property_supplier_payment_term': fields.property(
+            'account.payment.term',
+            type='many2one',
+            relation='account.payment.term',
+            string ='Supplier Payment Term',
+            view_load=True,
+            help="This payment term will be used instead of the default one for purchase orders and supplier invoices"),
         'ref_companies': fields.one2many('res.company', 'partner_id',
             'Companies that refers to partner'),
         'last_reconciliation_date': fields.datetime('Latest Reconciliation Date', help='Date on which the partner accounting entries were reconciled last time')

=== modified file 'account/partner_view.xml'
--- account/partner_view.xml	2011-09-21 07:13:12 +0000
+++ account/partner_view.xml	2012-02-14 18:43:08 +0000
@@ -80,12 +80,13 @@
                     <group col="2" colspan="2">
                         <separator string="Customer Accounting Properties" colspan="2"/>
                         <field name="property_account_receivable" groups="account.group_account_invoice" />
+                        <field name="property_payment_term" widget="selection"/>
                         <field name="property_account_position" widget="selection"/>
-                        <field name="property_payment_term" widget="selection"/>
                     </group>
                     <group col="2" colspan="2">
                         <separator string="Supplier Accounting Properties" colspan="2"/>
                         <field name="property_account_payable" groups="account.group_account_invoice"/>
+                        <field name="property_supplier_payment_term" widget="selection"/>
                     </group>
                     <group col="2" colspan="2">
                         <separator string="Customer Credit" colspan="2"/>

=== modified file 'base_vat/base_vat_view.xml'
--- base_vat/base_vat_view.xml	2012-02-08 15:26:47 +0000
+++ base_vat/base_vat_view.xml	2012-02-14 18:43:08 +0000
@@ -7,7 +7,7 @@
             <field name="model">res.partner</field>
             <field name="inherit_id" ref="account.view_partner_property_form"/>
             <field name="arch" type="xml">
-                <field name="property_account_payable" position="after">
+                <field name="property_supplier_payment_term" position="after">
                     <group colspan="2" col="6">
                         <field name="vat" on_change="vat_change(vat)"/>
                         <button name="button_check_vat" string="Check VAT" type="object" icon="gtk-execute"/>

=== modified file 'purchase/purchase.py'
--- purchase/purchase.py	2012-02-14 12:25:20 +0000
+++ purchase/purchase.py	2012-02-14 18:43:08 +0000
@@ -204,6 +204,7 @@
                 'purchase.order.line': (_get_order, None, 10),
             }, multi="sums",help="The total amount"),
         'fiscal_position': fields.many2one('account.fiscal.position', 'Fiscal Position'),
+        'payment_term': fields.many2one('account.payment.term', 'Payment Term'),
         'product_id': fields.related('order_line','product_id', type='many2one', relation='product.product', string='Product'),
         'create_uid':  fields.many2one('res.users', 'Responsible'),
         'company_id': fields.many2one('res.company','Company',required=True,select=1),
@@ -266,12 +267,19 @@
     def onchange_partner_id(self, cr, uid, ids, partner_id):
         partner = self.pool.get('res.partner')
         if not partner_id:
-            return {'value':{'partner_address_id': False, 'fiscal_position': False}}
+            return {'value':{
+                'partner_address_id': False,
+                'fiscal_position': False,
+                'payment_term': False
+                }}
         supplier_address = partner.address_get(cr, uid, [partner_id], ['default'])
         supplier = partner.browse(cr, uid, partner_id)
-        pricelist = supplier.property_product_pricelist_purchase.id
-        fiscal_position = supplier.property_account_position and supplier.property_account_position.id or False
-        return {'value':{'partner_address_id': supplier_address['default'], 'pricelist_id': pricelist, 'fiscal_position': fiscal_position}}
+        return {'value': {
+            'partner_address_id': supplier_address['default'],
+            'pricelist_id': supplier.property_product_pricelist_purchase.id,
+            'fiscal_position': supplier.property_account_position and supplier.property_account_position.id or False,
+            'payment_term': supplier.property_supplier_payment_term and supplier.property_supplier_payment_term.id or False,
+            }}
 
     def wkf_approve_order(self, cr, uid, ids, context=None):
         self.write(cr, uid, ids, {'state': 'approved', 'date_approve': fields.date.context_today(self,cr,uid,context=context)})
@@ -382,7 +390,7 @@
                 'invoice_line': [(6, 0, inv_lines)], 
                 'origin': order.name,
                 'fiscal_position': order.fiscal_position.id or order.partner_id.property_account_position.id,
-                'payment_term': order.partner_id.property_payment_term and order.partner_id.property_payment_term.id or False,
+                'payment_term': order.payment_term.id or order.partner_id.property_supplier_payment_term.id,
                 'company_id': order.company_id.id,
             }
             inv_id = inv_obj.create(cr, uid, inv_data, context=context)
@@ -915,7 +923,8 @@
                 'pricelist_id': pricelist_id,
                 'date_order': purchase_date.strftime(DEFAULT_SERVER_DATETIME_FORMAT),
                 'company_id': procurement.company_id.id,
-                'fiscal_position': partner.property_account_position and partner.property_account_position.id or False
+                'fiscal_position': partner.property_account_position and partner.property_account_position.id or False,
+                'payment_term': partner.property_payment_term.id or False
             }
             res[procurement.id] = self.create_procurement_purchase_order(cr, uid, procurement, po_vals, line_vals, context=context)
             self.write(cr, uid, [procurement.id], {'state': 'running', 'purchase_id': res[procurement.id]})

=== modified file 'purchase/purchase_view.xml'
--- purchase/purchase_view.xml	2012-01-31 13:36:57 +0000
+++ purchase/purchase_view.xml	2012-02-14 18:43:08 +0000
@@ -217,6 +217,7 @@
                             <group colspan="2" col="2">
                                 <separator string="Invoice Control" colspan="2"/>
                                 <field name="invoice_method"/>
+                                <field name="payment_term" widget="selection"/>
                                 <field name="fiscal_position" widget="selection"/>
                             </group>
                             <newline/>

=== modified file 'purchase/stock.py'
--- purchase/stock.py	2012-02-01 11:37:24 +0000
+++ purchase/stock.py	2012-02-14 18:43:08 +0000
@@ -60,6 +60,8 @@
         if picking.purchase_id:
             invoice_vals['address_contact_id'], invoice_vals['address_invoice_id'] = \
                 self.pool.get('res.partner').address_get(cr, uid, [partner.id], ['contact', 'invoice']).values()
+            invoice_vals['payment_term'] = picking.purchase_id.payment_term.id or \
+                partner.property_supplier_payment_term.id
             invoice_vals['fiscal_position'] = picking.purchase_id.fiscal_position.id
         return invoice_vals
 

=== modified file 'purchase/wizard/purchase_line_invoice.py'
--- purchase/wizard/purchase_line_invoice.py	2011-09-08 09:27:55 +0000
+++ purchase/wizard/purchase_line_invoice.py	2012-02-14 18:43:08 +0000
@@ -73,25 +73,21 @@
                 name = orders and orders[0].name or ''
                 journal_id = account_jrnl_obj.search(cr, uid, [('type', '=', 'purchase')], context=None)
                 journal_id = journal_id and journal_id[0] or False
-                a = partner.property_account_payable.id
-                if partner and partner.property_payment_term.id:
-                    pay_term = partner.property_payment_term.id
-                else:
-                    pay_term = False
                 inv = {
                     'name': name,
                     'origin': name,
                     'type': 'in_invoice',
                     'journal_id':journal_id,
                     'reference' : partner.ref,
-                    'account_id': a,
+                    'account_id': partner.property_account_payable.id,
                     'partner_id': partner.id,
                     'address_invoice_id': orders[0].partner_address_id.id,
                     'address_contact_id': orders[0].partner_address_id.id,
                     'invoice_line': [(6,0,lines_ids)],
                     'currency_id' : orders[0].pricelist_id.currency_id.id,
                     'comment': multiple_order_invoice_notes(orders),
-                    'payment_term': pay_term,
+                    'payment_term': orders[0].payment_term.id or \
+                        partner.property_payment_term.id,
                     'fiscal_position': partner.property_account_position.id
                 }
                 inv_id = invoice_obj.create(cr, uid, inv)