Merge lp:~akretion-team/openobject-addons/trunk-extensible-sale-action-invoice-create into lp:openobject-addons

[Alexis de Lattre]

This merge proposal adds a function _prepare_invoice() to the sale_order object, that builds the dict that will be used to create the invoice. It allows developers to easily change the values of the account.invoice object before it is created. It is the same kind of development as these other merge proposals by Akretion that were merged in trunk :
https://code.launchpad.net/~akretion-team/openobject-addons/addons-sale-extensible-action-ship-create/+merge/76609
https://code.launchpad.net/~alexis-via/openobject-addons/extensible-stock-action_invoice_create/+merge/87689

This merge proposal removes the function _inv_get() ; developers should now inherit the function _prepare_invoice().
This function _inv_get() is not used anywhere in the addons. It is used by 3 modules in extra-trunk :
- cci_sales : they made a near-hard copy of the function _make_invoice... they were waiting for the function _prepare_invoice() ! So they can now inherit _prepare_invoice() (or just remove the line that calls _inv_get() in _make_invoice())
- purchase_tax_include -> this functionnality is now part of the addons
- sale_tax_include -> this functionnality is now part of the addons
If you think it is a problem to remove the function _inv_get(), we can keep it in the code with a warning "depracated, inherit _prepare_invoice() instead".

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

Hello,

I approve Alexis's change: yet one of those last occasion to improve our daily accounting throughput once the base modules have been overridden, even by a simple localization.

Also Alexis cleaned up the _prepare_order_line_invoice_line methods I introduced in a previous merge and removed the order ids arg as it is totally useless (we may retrieve current order id by doing line.order_id.id).

So please can this go to 6.1 too?
I feel that's cool we have been somewhat consistent with all picking and standard invoice modularization with that last change.

review: Approve

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

up (as it changes and fixes the API, better have it before 6.1...)

[Olivier Dony (Odoo) (odo-openerp)]

Hello,

It looks good to me, the only few things we will change while merging are the following:

- restore _inv_get() method for compatibility (to be removed after 6.1 for example)
- add missing context guard in _prepare_invoice (our convention is that all methods that access the context should care for the None case, even if they are currently always called with a context)
- some docstrings improvements to follow our current docstring conventions (but I understand that you copied an existing one ;-))

Thanks for the clean refactoring!

review: Approve

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

Ok, perfect! Thanks.

On Wed, Feb 15, 2012 at 3:15 PM, Olivier Dony (OpenERP) <email address hidden>wrote:

> Review: Approve
>
> Hello,
>
> It looks good to me, the only few things we will change while merging are
> the following:
>
> - restore _inv_get() method for compatibility (to be removed after 6.1 for
> example)
> - add missing context guard in _prepare_invoice (our convention is that
> all methods that access the context should care for the None case, even if
> they are currently always called with a context)
> - some docstrings improvements to follow our current docstring conventions
> (but I understand that you copied an existing one ;-))
>
> Thanks for the clean refactoring!
> --
>
> https://code.launchpad.net/~akretion-team/openobject-addons/trunk-extensible-sale-action-invoice-create/+merge/91147
> You are reviewing the proposed merge of
> lp:~akretion-team/openobject-addons/trunk-extensible-sale-action-invoice-create
> into lp:openobject-addons.
>

=== modified file 'sale/sale.py'
--- sale/sale.py	2012-01-31 13:36:57 +0000
+++ sale/sale.py	2012-02-01 18:56:22 +0000
@@ -393,43 +393,24 @@
     def button_dummy(self, cr, uid, ids, context=None):
         return True
 
-    #FIXME: the method should return the list of invoices created (invoice_ids)
-    # and not the id of the last invoice created (res). The problem is that we
-    # cannot change it directly since the method is called by the sales order
-    # workflow and I suppose it expects a single id...
-    def _inv_get(self, cr, uid, order, context=None):
-        return {}
-
-    def _make_invoice(self, cr, uid, order, lines, context=None):
-        journal_obj = self.pool.get('account.journal')
-        inv_obj = self.pool.get('account.invoice')
-        obj_invoice_line = self.pool.get('account.invoice.line')
-        if context is None:
-            context = {}
-
-        journal_ids = journal_obj.search(cr, uid, [('type', '=', 'sale'), ('company_id', '=', order.company_id.id)], limit=1)
+    def _prepare_invoice(self, cr, uid, order, lines, context=None):
+        """ Builds the dict containing the values for the invoice
+            @param order: order object
+            @param line: list of invoice line IDs that must be attached to the invoice
+            @return: dict that will be used to create the invoice object
+        """
+        journal_ids = self.pool.get('account.journal').search(cr, uid,
+            [('type', '=', 'sale'), ('company_id', '=', order.company_id.id)],
+            limit=1)
         if not journal_ids:
             raise osv.except_osv(_('Error !'),
                 _('There is no sales journal defined for this company: "%s" (id:%d)') % (order.company_id.name, order.company_id.id))
-        a = order.partner_id.property_account_receivable.id
-        pay_term = order.payment_term and order.payment_term.id or False
-        invoiced_sale_line_ids = self.pool.get('sale.order.line').search(cr, uid, [('order_id', '=', order.id), ('invoiced', '=', True)], context=context)
-        from_line_invoice_ids = []
-        for invoiced_sale_line_id in self.pool.get('sale.order.line').browse(cr, uid, invoiced_sale_line_ids, context=context):
-            for invoice_line_id in invoiced_sale_line_id.invoice_lines:
-                if invoice_line_id.invoice_id.id not in from_line_invoice_ids:
-                    from_line_invoice_ids.append(invoice_line_id.invoice_id.id)
-        for preinv in order.invoice_ids:
-            if preinv.state not in ('cancel',) and preinv.id not in from_line_invoice_ids:
-                for preline in preinv.invoice_line:
-                    inv_line_id = obj_invoice_line.copy(cr, uid, preline.id, {'invoice_id': False, 'price_unit': -preline.price_unit})
-                    lines.append(inv_line_id)
-        inv = {
+        return {
             'name': order.client_order_ref or '',
             'origin': order.name,
             'type': 'out_invoice',
             'reference': order.client_order_ref or order.name,
-            'account_id': a,
+            'account_id': order.partner_id.property_account_receivable.id,
             'partner_id': order.partner_id.id,
             'journal_id': journal_ids[0],
             'address_invoice_id': order.partner_invoice_id.id,
@@ -437,15 +418,32 @@
             'invoice_line': [(6, 0, lines)],
             'currency_id': order.pricelist_id.currency_id.id,
             'comment': order.note,
-            'payment_term': pay_term,
+            'payment_term': order.payment_term and order.payment_term.id or False,
             'fiscal_position': order.fiscal_position.id or order.partner_id.property_account_position.id,
-            'date_invoice': context.get('date_invoice',False),
+            'date_invoice': context.get('date_invoice', False),
             'company_id': order.company_id.id,
             'user_id': order.user_id and order.user_id.id or False
         }
-        inv.update(self._inv_get(cr, uid, order))
+
+    def _make_invoice(self, cr, uid, order, lines, context=None):
+        inv_obj = self.pool.get('account.invoice')
+        obj_invoice_line = self.pool.get('account.invoice.line')
+        if context is None:
+            context = {}
+        invoiced_sale_line_ids = self.pool.get('sale.order.line').search(cr, uid, [('order_id', '=', order.id), ('invoiced', '=', True)], context=context)
+        from_line_invoice_ids = []
+        for invoiced_sale_line_id in self.pool.get('sale.order.line').browse(cr, uid, invoiced_sale_line_ids, context=context):
+            for invoice_line_id in invoiced_sale_line_id.invoice_lines:
+                if invoice_line_id.invoice_id.id not in from_line_invoice_ids:
+                    from_line_invoice_ids.append(invoice_line_id.invoice_id.id)
+        for preinv in order.invoice_ids:
+            if preinv.state not in ('cancel',) and preinv.id not in from_line_invoice_ids:
+                for preline in preinv.invoice_line:
+                    inv_line_id = obj_invoice_line.copy(cr, uid, preline.id, {'invoice_id': False, 'price_unit': -preline.price_unit})
+                    lines.append(inv_line_id)
+        inv = self._prepare_invoice(cr, uid, order, lines, context=context)
         inv_id = inv_obj.create(cr, uid, inv, context=context)
-        data = inv_obj.onchange_payment_term_date_invoice(cr, uid, [inv_id], pay_term, time.strftime(DEFAULT_SERVER_DATE_FORMAT))
+        data = inv_obj.onchange_payment_term_date_invoice(cr, uid, [inv_id], inv['payment_term'], time.strftime(DEFAULT_SERVER_DATE_FORMAT))
         if data.get('value', False):
             inv_obj.write(cr, uid, [inv_id], data['value'], context=context)
         inv_obj.button_compute(cr, uid, [inv_id])
@@ -981,7 +979,7 @@
         'price_unit': 0.0,
     }
 
-    def _prepare_order_line_invoice_line(self, cr, uid, ids, line, account_id=False, context=None):
+    def _prepare_order_line_invoice_line(self, cr, uid, line, account_id=False, context=None):
         """ Builds the invoice line dict from a sale order line
             @param line: sale order line object
             @param account_id: the id of the account to force eventually (the method is used for picking return including service)
@@ -1055,7 +1053,7 @@
         create_ids = []
         sales = set()
         for line in self.browse(cr, uid, ids, context=context):
-            vals = self._prepare_order_line_invoice_line(cr, uid, ids, line, False, context)
+            vals = self._prepare_order_line_invoice_line(cr, uid, line, False, context)
             if vals:
                 inv_id = self.pool.get('account.invoice.line').create(cr, uid, vals, context=context)
                 cr.execute('insert into sale_order_line_invoice_rel (order_line_id,invoice_id) values (%s,%s)', (line.id, inv_id))

=== modified file 'sale/stock.py'
--- sale/stock.py	2012-01-31 13:36:57 +0000
+++ sale/stock.py	2012-02-01 18:56:22 +0000
@@ -176,7 +176,7 @@
                             account_id = sale_line.product_id.categ_id.\
                                     property_account_expense_categ.id
 
-                    vals = order_line_obj._prepare_order_line_invoice_line(cursor, user, ids, sale_line, account_id, context)
+                    vals = order_line_obj._prepare_order_line_invoice_line(cursor, user, sale_line, account_id, context)
                     if vals: #note: in some cases we may not want to include all service lines as invoice lines
                         vals['name'] = name
                         vals['account_analytic_id'] = self._get_account_analytic_invoice(cursor, user, picking, sale_line)