Merge lp:~alexis-via/openobject-addons/extensible-stock-action_invoice_create into lp:openobject-addons

[Alexis de Lattre]

This merge proposal is a refactoring to make it easier and cleaner to override invoice creation from deliveries in order to customize the invoice creation process.
It is similar to other merge proposals developped by Akretion this last months and merged in trunk (like this one : https://code.launchpad.net/~akretion-team/openobject-addons/addons-sale-extensible-action-ship-create/+merge/76609)

It also fixes the following (example) scenario :
Customer A is ordering some goods for a construction project and would like to have the goods delivered directly on the construction site. The address of the construction site is linked to partner B. On the sale order, the Invoice policy is "Invoice based on deliveries".
So far, when the invoice was created from the picking, the invoice would have all the information of partner A, except the fiscal position and the partner account which would be the one of partner B ! This merge proposal fixes this, by a clean handling of the "partner" variable (which fixes the account_id as a consequence) and a proper handling of the fiscal position.

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

Hello I didn't take time to look at the detail of the proposal yet, bu I should say I strongly recommend to do something about the lack of modularity here. Hopefully I take some time Friday to look at it.

Also I recommend looking at the first attempt by Renato Lima a few months ago:
https://code.launchpad.net/~akretion-team/openobject-addons/addons-stock-extensible-action-invoice-create

And also I would like to point that additionally we could try to refactor also the invoice line creation. In our Brazilian localization for instance, service lines should go to a different invoice than product invoices and delivery cost is a global tax instead of an invoice line. So making line creation modular would improve a lot the performance here and compatibility between modules hacking this currently.

[Alexis de Lattre (alexis-via)]

The first attempt by Renato Lima (unfortunately, I was not aware of it) was rejected by Fabien "as it crashed the buildbot when installing stock_invoice_directly". I tested the installation of stock_invoice_directly with my merge proposal : it installs fine. I tested a sale order + delivery with this module : it crashes... but it also crashes with a standard addons/trunk, so it's not caused by the changes introduced in my merge proposal.

[Nhomar - Vauxoo (nhomar)]

I tried Basic enviroments and it works as expected.

Just some comments:

Method should be documented as:

http://doc.openerp.com/v6.0/contribute/15_guidelines/documentation_guidelines.html

def action_send(self, cr, uid, ids, context=None):
    """ This sends an email to ALL the addresses of the selected partners.
    @param self: The object pointer
    @param cr: the current row, from the database cursor,
    @param uid: the current user ID for security checks
    @param ids: List of Phonecall to Opportunity's IDs
    @param context: A standard dictionary for contextual values
    """

With this little change everybody can use and inherit your methods with less code reading.

IMHO. Great Job...

I will mark as needs fixing just for logistic reason, in the moment you put this improvement in comments, i will approve.

review: Needs Fixing (pre-aproved)

[Alexis de Lattre (alexis-via)]

In my commit 6274, I introduced the standard syntax to document the new method I introduced.

[Nhomar - Vauxoo (nhomar)]

review: Approve (aproved)

[Raphael Collet (OpenERP) (rco-openerp)]

I tried it, and it seems to work fine.

review: Approve

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

Hello Alexis,

first kudos for all this work! For me it makes total sense in all aspects (and we badly need that for our Brazilian localization).
However I wanted to complete your work a bit, but couldn't do it as you set you as the owner of the branch. So I created this branch for akretion-team where I made further commits:
https://code.launchpad.net/~akretion-team/openobject-addons/extensible-stock-action_invoice_create2

What you could do is is merge my branch in yours and OpenERP SA could merge the whole if they agree.

Basically my 1st commit was to allow grouping invoice lines.
Then I did the same job as you (extracted only one method) for invoicing upon sale order.
Then I reused that newly extracted method in the invoicing of picking including services, as you can see aside from reducing the code size, I also fixed a bug with fiscal positions.
Then I wanted to see if we could deprecate the invoice_hook and _invoice_line_hooks as they are those nasty methods rebrowsing save records and recomputing the world. But it turns out we should keep them as they are used to link source and target records together (so can only work after target record creation).
Still, as you can see in my last commit, I found a bug in the purchase module invoicing and fixed it.

So Alexis, could you please merge my branch if you agree? Then we ask OpenERP SA if they still agree to merge that.

Overall that's cool, in 6.1 we modularized the picking creation previously, now with this merge we will modularize the invoice creation in the most common cases so overall this is an excellent news for localizations and other daily customizations that use to touch those objects a lot and need information to be propagated from orders to pickings to invoices without killing the database or the modularity.

review: Needs Fixing

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

BTW, I technically said need fixing till I get a position about my further merges. Aside from that I'm for the merge.

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

Hello, so now I feel like it's all quite consistent and a good leap forward. Thank you for having merged my further refactored related to invoice on picking. I approve for a merge into 6.1 addons now!

review: Approve

[Raphael Collet (OpenERP) (rco-openerp)]

On its way to be merged.

Thanks,
Raphael

review: Approve

=== modified file 'purchase/stock.py'
--- purchase/stock.py	2011-12-19 16:54:40 +0000
+++ purchase/stock.py	2012-01-27 15:50:31 +0000
@@ -58,18 +58,25 @@
         'purchase_id': False,
     }
 
-    def _get_address_invoice(self, cr, uid, picking):
-        """ Gets invoice address of a partner
-        @return {'contact': address, 'invoice': address} for invoice
-        """
-        res = super(stock_picking, self)._get_address_invoice(cr, uid, picking)
-        if picking.purchase_id:
-            partner_obj = self.pool.get('res.partner')
-            partner = picking.purchase_id.partner_id or picking.address_id.partner_id
-            data = partner_obj.address_get(cr, uid, [partner.id],
-                ['contact', 'invoice'])
-            res.update(data)
-        return res
+    def _get_partner_to_invoice(self, cr, uid, picking, context=None):
+        """Inherit the original function of the 'stock' module
+        We select the partner of the sale order as the partner of the customer invoice
+        """
+        if picking.purchase_id:
+            return picking.purchase_id.partner_id
+        return super(stock_picking, self)._get_partner_to_invoice(cr, uid, picking, context=context)
+
+    def _prepare_invoice(self, cr, uid, picking, partner, inv_type, journal_id, context=None):
+        """Inherit the original function of the 'stock' module in order to override some
+        values if the picking has been generated by a purchase order
+        """
+        invoice_vals = super(stock_picking, self)._prepare_invoice(cr, uid, picking, partner, inv_type, journal_id, context=context)
+        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['fiscal_position'] = picking.purchase_id.fiscal_position.id
+        return invoice_vals
 
     def get_currency_id(self, cursor, user, picking):
         if picking.purchase_id:
@@ -111,6 +118,12 @@
     def _invoice_line_hook(self, cursor, user, move_line, invoice_line_id):
         if move_line.purchase_line_id:
             invoice_line_obj = self.pool.get('account.invoice.line')
+            purchase_line_obj = self.pool.get('purchase.order.line') 
+            purchase_line_obj.write(cursor, user, [move_line.purchase_line_id.id],
+                                    {
+                                        'invoiced': True,
+                                        'invoice_lines': [(4, invoice_line_id)],
+                                    })
             invoice_line_obj.write(cursor, user, [invoice_line_id], {'note':  move_line.purchase_line_id.notes,})
         return super(stock_picking, self)._invoice_line_hook(cursor, user, move_line, invoice_line_id)
 
 
=== modified file 'sale/sale.py'
--- sale/sale.py	2012-01-05 08:57:53 +0000
+++ sale/sale.py	2012-01-27 15:50:31 +0000
@@ -981,10 +981,12 @@
         'price_unit': 0.0,
     }
 
-    def invoice_line_create(self, cr, uid, ids, context=None):
-        if context is None:
-            context = {}
-
+    def _prepare_order_line_invoice_line(self, cr, uid, ids, 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)
+        @return: dict that will be used to create the invoice line"""
+        
         def _get_line_qty(line):
             if (line.order_id.invoice_quantity=='order') or not line.procurement_id:
                 if line.product_uos:
@@ -1003,48 +1005,59 @@
                 return self.pool.get('procurement.order').uom_get(cr, uid,
                         line.procurement_id.id, context=context)
 
-        create_ids = []
-        sales = {}
-        for line in self.browse(cr, uid, ids, context=context):
-            if not line.invoiced:
+        if not line.invoiced:
+            if not account_id:
                 if line.product_id:
-                    a = line.product_id.product_tmpl_id.property_account_income.id
-                    if not a:
-                        a = line.product_id.categ_id.property_account_income_categ.id
-                    if not a:
+                    account_id = line.product_id.product_tmpl_id.property_account_income.id
+                    if not account_id:
+                        account_id = line.product_id.categ_id.property_account_income_categ.id
+                    if not account_id:
                         raise osv.except_osv(_('Error !'),
                                 _('There is no income account defined ' \
-                                        'for this product: "%s" (id:%d)') % \
-                                        (line.product_id.name, line.product_id.id,))
+                                    'for this product: "%s" (id:%d)') % \
+                                    (line.product_id.name, line.product_id.id,))
                 else:
                     prop = self.pool.get('ir.property').get(cr, uid,
                             'property_account_income_categ', 'product.category',
                             context=context)
-                    a = prop and prop.id or False
-                uosqty = _get_line_qty(line)
-                uos_id = _get_line_uom(line)
-                pu = 0.0
-                if uosqty:
-                    pu = round(line.price_unit * line.product_uom_qty / uosqty,
-                            self.pool.get('decimal.precision').precision_get(cr, uid, 'Sale Price'))
-                fpos = line.order_id.fiscal_position or False
-                a = self.pool.get('account.fiscal.position').map_account(cr, uid, fpos, a)
-                if not a:
-                    raise osv.except_osv(_('Error !'),
-                                _('There is no income category account defined in default Properties for Product Category or Fiscal Position is not defined !'))
-                inv_id = self.pool.get('account.invoice.line').create(cr, uid, {
-                    'name': line.name,
-                    'origin': line.order_id.name,
-                    'account_id': a,
-                    'price_unit': pu,
-                    'quantity': uosqty,
-                    'discount': line.discount,
-                    'uos_id': uos_id,
-                    'product_id': line.product_id.id or False,
-                    'invoice_line_tax_id': [(6, 0, [x.id for x in line.tax_id])],
-                    'note': line.notes,
-                    'account_analytic_id': line.order_id.project_id and line.order_id.project_id.id or False,
-                })
+                    account_id = prop and prop.id or False
+            uosqty = _get_line_qty(line)
+            uos_id = _get_line_uom(line)
+            pu = 0.0
+            if uosqty:
+                pu = round(line.price_unit * line.product_uom_qty / uosqty,
+                        self.pool.get('decimal.precision').precision_get(cr, uid, 'Sale Price'))
+            fpos = line.order_id.fiscal_position or False
+            account_id = self.pool.get('account.fiscal.position').map_account(cr, uid, fpos, account_id)
+            if not account_id:
+                raise osv.except_osv(_('Error !'),
+                            _('There is no income category account defined in default Properties for Product Category or Fiscal Position is not defined !'))
+            return {
+                'name': line.name,
+                'origin': line.order_id.name,
+                'account_id': account_id,
+                'price_unit': pu,
+                'quantity': uosqty,
+                'discount': line.discount,
+                'uos_id': uos_id,
+                'product_id': line.product_id.id or False,
+                'invoice_line_tax_id': [(6, 0, [x.id for x in line.tax_id])],
+                'note': line.notes,
+                'account_analytic_id': line.order_id.project_id and line.order_id.project_id.id or False,
+            }
+        else:
+            return False
+
+    def invoice_line_create(self, cr, uid, ids, context=None):
+        if context is None:
+            context = {}
+
+        create_ids = []
+        sales = {}
+        for line in self.browse(cr, uid, ids, context=context):
+            vals = self._prepare_order_line_invoice_line(cr, uid, ids, 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))
                 self.write(cr, uid, [line.id], {'invoiced': True})
                 sales[line.order_id.id] = True
 
=== modified file 'sale/stock.py'
--- sale/stock.py	2011-12-28 13:25:20 +0000
+++ sale/stock.py	2012-01-27 15:50:31 +0000
@@ -48,24 +48,32 @@
         else:
             return super(stock_picking, self).get_currency_id(cursor, user, picking)
 
-    def _get_payment_term(self, cursor, user, picking):
-        if picking.sale_id and picking.sale_id.payment_term:
-            return picking.sale_id.payment_term.id
-        return super(stock_picking, self)._get_payment_term(cursor, user, picking)
-
-    def _get_address_invoice(self, cursor, user, picking):
-        res = {}
+    def _get_partner_to_invoice(self, cr, uid, picking, context=None):
+        """Inherit the original function of the 'stock' module
+        We select the partner of the sale order as the partner of the customer invoice
+        """
         if picking.sale_id:
-            res['contact'] = picking.sale_id.partner_order_id.id
-            res['invoice'] = picking.sale_id.partner_invoice_id.id
-            return res
-        return super(stock_picking, self)._get_address_invoice(cursor, user, picking)
+            return picking.sale_id.partner_id
+        return super(stock_picking, self)._get_partner_to_invoice(cr, uid, picking, context=context)
 
     def _get_comment_invoice(self, cursor, user, picking):
         if picking.note or (picking.sale_id and picking.sale_id.note):
             return picking.note or picking.sale_id.note
         return super(stock_picking, self)._get_comment_invoice(cursor, user, picking)
 
+    def _prepare_invoice(self, cr, uid, picking, partner, inv_type, journal_id, context=None):
+        """Inherit the original function of the 'stock' module in order to override some
+        values if the picking has been generated by a sale order
+        """
+        invoice_vals = super(stock_picking, self)._prepare_invoice(cr, uid, picking, partner, inv_type, journal_id, context=context)
+        if picking.sale_id:
+            invoice_vals['address_contact_id'] = picking.sale_id.partner_order_id.id
+            invoice_vals['address_invoice_id'] = picking.sale_id.partner_invoice_id.id
+            invoice_vals['fiscal_position'] = picking.sale_id.fiscal_position.id
+            invoice_vals['payment_term'] = picking.sale_id.payment_term.id
+            invoice_vals['user_id'] = picking.sale_id.user_id.id
+        return invoice_vals
+
     def _get_price_unit_invoice(self, cursor, user, move_line, type):
         if move_line.sale_line_id and move_line.sale_line_id.product_id.id == move_line.product_id.id:
             uom_id = move_line.product_id.uom_id.id
@@ -167,32 +175,15 @@
                         if not account_id:
                             account_id = sale_line.product_id.categ_id.\
                                     property_account_expense_categ.id
-                    price_unit = sale_line.price_unit
-                    discount = sale_line.discount
-                    tax_ids = sale_line.tax_id
-                    tax_ids = map(lambda x: x.id, tax_ids)
-
-                    account_analytic_id = self._get_account_analytic_invoice(cursor,
-                            user, picking, sale_line)
-
-                    account_id = fiscal_position_obj.map_account(cursor, user, picking.sale_id.partner_id.property_account_position, account_id)
-                    invoice = invoices[result[picking.id]]
-                    invoice_line_id = invoice_line_obj.create(cursor, user, {
-                        'name': name,
-                        'invoice_id': invoice.id,
-                        'uos_id': sale_line.product_uos.id or sale_line.product_uom.id,
-                        'product_id': sale_line.product_id.id,
-                        'account_id': account_id,
-                        'price_unit': price_unit,
-                        'discount': discount,
-                        'quantity': sale_line.product_uos_qty,
-                        'invoice_line_tax_id': [(6, 0, tax_ids)],
-                        'account_analytic_id': account_analytic_id,
-                        'notes':sale_line.notes
-                    }, context=context)
-                    order_line_obj.write(cursor, user, [sale_line.id], {'invoiced': True,
-                        'invoice_lines': [(6, 0, [invoice_line_id])],
-                    })
+
+                    vals = order_line_obj._prepare_order_line_invoice_line(cursor, user, ids, 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)
+                        vals['invoice_id'] = invoices[result[picking.id]].id
+                        invoice_line_id = invoice_line_obj.create(cursor, user, vals, context=context)
+                        order_line_obj.write(cursor, user, [sale_line.id], {'invoiced': True,
+                        'invoice_lines': [(6, 0, [invoice_line_id])]})
         return result
 
-# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:
 \ No newline at end of file
+# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:
 
=== modified file 'stock/stock.py'
--- stock/stock.py	2012-01-27 13:55:37 +0000
+++ stock/stock.py	2012-01-27 15:50:31 +0000
@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 ##############################################################################
 #
 #    OpenERP, Open Source Management Solution
@@ -864,21 +865,13 @@
     def get_currency_id(self, cr, uid, picking):
         return False
 
-    def _get_payment_term(self, cr, uid, picking):
-        """ Gets payment term from partner.
-        @return: Payment term
-        """
-        partner = picking.address_id.partner_id
-        return partner.property_payment_term and partner.property_payment_term.id or False
-
-    def _get_address_invoice(self, cr, uid, picking):
-        """ Gets invoice address of a partner
-        @return {'contact': address, 'invoice': address} for invoice
-        """
-        partner_obj = self.pool.get('res.partner')
-        partner = picking.address_id.partner_id
-        return partner_obj.address_get(cr, uid, [partner.id],
-                ['contact', 'invoice'])
+    def _get_partner_to_invoice(self, cr, uid, picking, context=None):
+        """ Gets the partner that will be invoiced
+        Note that this function is inherited in the sale module
+        @param picking: object of the picking for which we are selecting the partner to invoice
+        @return: object of the partner to invoice
+        """
+        return picking.address_id and picking.address_id.partner_id
 
     def _get_comment_invoice(self, cr, uid, picking):
         """
@@ -958,6 +951,116 @@
                 inv_type = 'out_invoice'
         return inv_type
 
+    def _prepare_invoice_group(self, cr, uid, picking, partner, invoice, context=None):
+        """Builds the dict for grouped invoices
+        @param picking: picking object
+        @param partner: object of the partner to invoice (not used here, but may be usefull if this function is inherited)
+        @param invoice: object of the invoice that we are updating
+        @return: dict that will be used to update the invoice
+        """
+        comment = self._get_comment_invoice(cr, uid, picking)
+
+        return {
+            'name': (invoice.name or '') + ', ' + (picking.name or ''),
+            'origin': (invoice.origin or '') + ', ' + (picking.name or '') + (picking.origin and (':' + picking.origin) or ''),
+            'comment': (comment and (invoice.comment and invoice.comment + "\n" + comment or comment)) or (invoice.comment and invoice.comment or ''),
+            'date_invoice': context.get('date_inv', False),
+            'user_id': uid
+        }
+
+    def _prepare_invoice(self, cr, uid, picking, partner, inv_type, journal_id, context=None):
+        """Builds the dict containing the values for the invoice
+        @param picking: picking object
+        @param partner: object of the partner to invoice
+        @param inv_type: type of the invoice ('out_invoice', 'in_invoice', ...)
+        @param journal_id: ID of the accounting journal
+        @return: dict that will be used to create the invoice object
+        """
+        if inv_type in ('out_invoice', 'out_refund'):
+            account_id = partner.property_account_receivable.id
+        else:
+            account_id = partner.property_account_payable.id
+        address_contact_id, address_invoice_id = \
+                self.pool.get('res.partner').address_get(cr, uid, [partner.id],
+                        ['contact', 'invoice']).values()
+        comment = self._get_comment_invoice(cr, uid, picking)
+        invoice_vals = {
+            'name': picking.name,
+            'origin': (picking.name or '') + (picking.origin and (':' + picking.origin) or ''),
+            'type': inv_type,
+            'account_id': account_id,
+            'partner_id': partner.id,
+            'address_invoice_id': address_invoice_id,
+            'address_contact_id': address_contact_id,
+            'comment': comment,
+            'payment_term': partner.property_payment_term and partner.property_payment_term.id
+                    or False,
+            'fiscal_position': partner.property_account_position.id,
+            'date_invoice': context.get('date_inv', False),
+            'company_id': picking.company_id.id,
+            'user_id': uid
+        }
+        cur_id = self.get_currency_id(cr, uid, picking)
+        if cur_id:
+            invoice_vals['currency_id'] = cur_id
+        if journal_id:
+            invoice_vals['journal_id'] = journal_id
+        return invoice_vals
+
+    def _prepare_invoice_line(self, cr, uid, group, picking, move_line, invoice_id,
+        invoice_vals, context=None):
+        """Builds the dict containing the values for the invoice line
+        @param group: True or False
+        @param picking: picking object
+        @param: move_line: move_line object
+        @param: invoice_id: ID of the related invoice
+        @param: invoice_vals: dict used to created the invoice
+        @return: dict that will be used to create the invoice line
+        """
+        if group:
+            name = (picking.name or '') + '-' + move_line.name
+        else:
+            name = move_line.name
+        origin = move_line.picking_id.name or ''
+        if move_line.picking_id.origin:
+            origin += ':' + move_line.picking_id.origin
+
+        if invoice_vals['type'] in ('out_invoice', 'out_refund'):
+            account_id = move_line.product_id.product_tmpl_id.\
+                    property_account_income.id
+            if not account_id:
+                account_id = move_line.product_id.categ_id.\
+                        property_account_income_categ.id
+        else:
+            account_id = move_line.product_id.product_tmpl_id.\
+                    property_account_expense.id
+            if not account_id:
+                account_id = move_line.product_id.categ_id.\
+                        property_account_expense_categ.id
+        if invoice_vals['fiscal_position']:
+            fp_obj = self.pool.get('account.fiscal.position')
+            fiscal_position = fp_obj.browse(cr, uid, invoice_vals['fiscal_position'], context=context)
+            account_id = fp_obj.map_account(cr, uid, fiscal_position, account_id)
+        # set UoS if it's a sale and the picking doesn't have one
+        uos_id = move_line.product_uos and move_line.product_uos.id or False
+        if not uos_id and invoice_vals['type'] in ('out_invoice', 'out_refund'):
+            uos_id = move_line.product_uom.id
+
+        return {
+            'name': name,
+            'origin': origin,
+            'invoice_id': invoice_id,
+            'uos_id': uos_id,
+            'product_id': move_line.product_id.id,
+            'account_id': account_id,
+            'price_unit': self._get_price_unit_invoice(cr, uid, move_line, invoice_vals['type']),
+            'discount': self._get_discount_invoice(cr, uid, move_line),
+            'quantity': move_line.product_uos_qty or move_line.product_qty,
+            'invoice_line_tax_id': [(6, 0,
+                    self._get_taxes_invoice(cr, uid, move_line, invoice_vals['type']))],
+            'account_analytic_id': self._get_account_analytic_invoice(cr, uid, picking, move_line),
+            }
+
     def action_invoice_create(self, cr, uid, ids, journal_id=False,
             group=False, type='out_invoice', context=None):
         """ Creates invoice based on the invoice state selected for picking.
@@ -971,14 +1074,13 @@
 
         invoice_obj = self.pool.get('account.invoice')
         invoice_line_obj = self.pool.get('account.invoice.line')
-        address_obj = self.pool.get('res.partner.address')
         invoices_group = {}
         res = {}
         inv_type = type
         for picking in self.browse(cr, uid, ids, context=context):
             if picking.invoice_state != '2binvoiced':
                 continue
-            partner =  picking.address_id and picking.address_id.partner_id
+            partner = self._get_partner_to_invoice(cr, uid, picking, context=context)
             if not partner:
                 raise osv.except_osv(_('Error, no partner !'),
                     _('Please put a partner on the picking list if you want to generate invoice.'))
@@ -986,101 +1088,24 @@
             if not inv_type:
                 inv_type = self._get_invoice_type(picking)
 
-            if inv_type in ('out_invoice', 'out_refund'):
-                account_id = partner.property_account_receivable.id
-            else:
-                account_id = partner.property_account_payable.id
-            address_contact_id, address_invoice_id = \
-                    self._get_address_invoice(cr, uid, picking).values()
-            address = address_obj.browse(cr, uid, address_contact_id, context=context)
-
-            comment = self._get_comment_invoice(cr, uid, picking)
             if group and partner.id in invoices_group:
                 invoice_id = invoices_group[partner.id]
                 invoice = invoice_obj.browse(cr, uid, invoice_id)
-                invoice_vals = {
-                    'name': (invoice.name or '') + ', ' + (picking.name or ''),
-                    'origin': (invoice.origin or '') + ', ' + (picking.name or '') + (picking.origin and (':' + picking.origin) or ''),
-                    'comment': (comment and (invoice.comment and invoice.comment+"\n"+comment or comment)) or (invoice.comment and invoice.comment or ''),
-                    'date_invoice':context.get('date_inv',False),
-                    'user_id':uid
-                }
-                invoice_obj.write(cr, uid, [invoice_id], invoice_vals, context=context)
+                invoice_vals_group = self._prepare_invoice_group(cr, uid, picking, partner, invoice, context=context)
+                invoice_obj.write(cr, uid, [invoice_id], invoice_vals_group, context=context)
             else:
-                invoice_vals = {
-                    'name': picking.name,
-                    'origin': (picking.name or '') + (picking.origin and (':' + picking.origin) or ''),
-                    'type': inv_type,
-                    'account_id': account_id,
-                    'partner_id': address.partner_id.id,
-                    'address_invoice_id': address_invoice_id,
-                    'address_contact_id': address_contact_id,
-                    'comment': comment,
-                    'payment_term': self._get_payment_term(cr, uid, picking),
-                    'fiscal_position': partner.property_account_position.id,
-                    'date_invoice': context.get('date_inv',False),
-                    'company_id': picking.company_id.id,
-                    'user_id':uid
-                }
-                cur_id = self.get_currency_id(cr, uid, picking)
-                if cur_id:
-                    invoice_vals['currency_id'] = cur_id
-                if journal_id:
-                    invoice_vals['journal_id'] = journal_id
-                invoice_id = invoice_obj.create(cr, uid, invoice_vals,
-                        context=context)
+                invoice_vals = self._prepare_invoice(cr, uid, picking, partner, inv_type, journal_id, context=context)
+                invoice_id = invoice_obj.create(cr, uid, invoice_vals, context=context)
                 invoices_group[partner.id] = invoice_id
             res[picking.id] = invoice_id
             for move_line in picking.move_lines:
                 if move_line.state == 'cancel':
                     continue
-                origin = move_line.picking_id.name or ''
-                if move_line.picking_id.origin:
-                    origin += ':' + move_line.picking_id.origin
-                if group:
-                    name = (picking.name or '') + '-' + move_line.name
-                else:
-                    name = move_line.name
-
-                if inv_type in ('out_invoice', 'out_refund'):
-                    account_id = move_line.product_id.product_tmpl_id.\
-                            property_account_income.id
-                    if not account_id:
-                        account_id = move_line.product_id.categ_id.\
-                                property_account_income_categ.id
-                else:
-                    account_id = move_line.product_id.product_tmpl_id.\
-                            property_account_expense.id
-                    if not account_id:
-                        account_id = move_line.product_id.categ_id.\
-                                property_account_expense_categ.id
-
-                price_unit = self._get_price_unit_invoice(cr, uid,
-                        move_line, inv_type)
-                discount = self._get_discount_invoice(cr, uid, move_line)
-                tax_ids = self._get_taxes_invoice(cr, uid, move_line, inv_type)
-                account_analytic_id = self._get_account_analytic_invoice(cr, uid, picking, move_line)
-
-                #set UoS if it's a sale and the picking doesn't have one
-                uos_id = move_line.product_uos and move_line.product_uos.id or False
-                if not uos_id and inv_type in ('out_invoice', 'out_refund'):
-                    uos_id = move_line.product_uom.id
-
-                account_id = self.pool.get('account.fiscal.position').map_account(cr, uid, partner.property_account_position, account_id)
-                invoice_line_id = invoice_line_obj.create(cr, uid, {
-                    'name': name,
-                    'origin': origin,
-                    'invoice_id': invoice_id,
-                    'uos_id': uos_id,
-                    'product_id': move_line.product_id.id,
-                    'account_id': account_id,
-                    'price_unit': price_unit,
-                    'discount': discount,
-                    'quantity': move_line.product_uos_qty or move_line.product_qty,
-                    'invoice_line_tax_id': [(6, 0, tax_ids)],
-                    'account_analytic_id': account_analytic_id,
-                }, context=context)
-                self._invoice_line_hook(cr, uid, move_line, invoice_line_id)
+                vals = self._prepare_invoice_line(cr, uid, group, picking, move_line,
+                                invoice_id, invoice_vals, context=context)
+                if vals:
+                    invoice_line_id = invoice_line_obj.create(cr, uid, vals, context=context)
+                    self._invoice_line_hook(cr, uid, move_line, invoice_line_id)
 
             invoice_obj.button_compute(cr, uid, [invoice_id], context=context,
                     set_total=(inv_type in ('in_invoice', 'in_refund')))