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

Proposed by Alexis de Lattre on 2012-02-01
Status: Merged
Merged at revision: 6597
Proposed branch: lp:~akretion-team/openobject-addons/trunk-extensible-sale-action-invoice-create
Merge into: lp:openobject-addons
Diff against target: 126 lines (+35/-37)
2 files modified
sale/sale.py (+34/-36)
sale/stock.py (+1/-1)
To merge this branch: bzr merge lp:~akretion-team/openobject-addons/trunk-extensible-sale-action-invoice-create
Reviewer Review Type Date Requested Status
Olivier Dony (Odoo) 2012-02-01 Approve on 2012-02-15
Raphaël Valyi - http://www.akretion.com (community) Approve on 2012-02-08
qdp (OpenERP) 2012-02-03 Pending
Review via email: mp+91147@code.launchpad.net

Description of the Change

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".

To post a comment you must log in.

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

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

Olivier Dony (Odoo) (odo-openerp) wrote :

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

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.
>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'sale/sale.py'
2--- sale/sale.py 2012-01-31 13:36:57 +0000
3+++ sale/sale.py 2012-02-01 18:56:22 +0000
4@@ -393,43 +393,24 @@
5 def button_dummy(self, cr, uid, ids, context=None):
6 return True
7
8- #FIXME: the method should return the list of invoices created (invoice_ids)
9- # and not the id of the last invoice created (res). The problem is that we
10- # cannot change it directly since the method is called by the sales order
11- # workflow and I suppose it expects a single id...
12- def _inv_get(self, cr, uid, order, context=None):
13- return {}
14-
15- def _make_invoice(self, cr, uid, order, lines, context=None):
16- journal_obj = self.pool.get('account.journal')
17- inv_obj = self.pool.get('account.invoice')
18- obj_invoice_line = self.pool.get('account.invoice.line')
19- if context is None:
20- context = {}
21-
22- journal_ids = journal_obj.search(cr, uid, [('type', '=', 'sale'), ('company_id', '=', order.company_id.id)], limit=1)
23+ def _prepare_invoice(self, cr, uid, order, lines, context=None):
24+ """ Builds the dict containing the values for the invoice
25+ @param order: order object
26+ @param line: list of invoice line IDs that must be attached to the invoice
27+ @return: dict that will be used to create the invoice object
28+ """
29+ journal_ids = self.pool.get('account.journal').search(cr, uid,
30+ [('type', '=', 'sale'), ('company_id', '=', order.company_id.id)],
31+ limit=1)
32 if not journal_ids:
33 raise osv.except_osv(_('Error !'),
34 _('There is no sales journal defined for this company: "%s" (id:%d)') % (order.company_id.name, order.company_id.id))
35- a = order.partner_id.property_account_receivable.id
36- pay_term = order.payment_term and order.payment_term.id or False
37- invoiced_sale_line_ids = self.pool.get('sale.order.line').search(cr, uid, [('order_id', '=', order.id), ('invoiced', '=', True)], context=context)
38- from_line_invoice_ids = []
39- for invoiced_sale_line_id in self.pool.get('sale.order.line').browse(cr, uid, invoiced_sale_line_ids, context=context):
40- for invoice_line_id in invoiced_sale_line_id.invoice_lines:
41- if invoice_line_id.invoice_id.id not in from_line_invoice_ids:
42- from_line_invoice_ids.append(invoice_line_id.invoice_id.id)
43- for preinv in order.invoice_ids:
44- if preinv.state not in ('cancel',) and preinv.id not in from_line_invoice_ids:
45- for preline in preinv.invoice_line:
46- inv_line_id = obj_invoice_line.copy(cr, uid, preline.id, {'invoice_id': False, 'price_unit': -preline.price_unit})
47- lines.append(inv_line_id)
48- inv = {
49+ return {
50 'name': order.client_order_ref or '',
51 'origin': order.name,
52 'type': 'out_invoice',
53 'reference': order.client_order_ref or order.name,
54- 'account_id': a,
55+ 'account_id': order.partner_id.property_account_receivable.id,
56 'partner_id': order.partner_id.id,
57 'journal_id': journal_ids[0],
58 'address_invoice_id': order.partner_invoice_id.id,
59@@ -437,15 +418,32 @@
60 'invoice_line': [(6, 0, lines)],
61 'currency_id': order.pricelist_id.currency_id.id,
62 'comment': order.note,
63- 'payment_term': pay_term,
64+ 'payment_term': order.payment_term and order.payment_term.id or False,
65 'fiscal_position': order.fiscal_position.id or order.partner_id.property_account_position.id,
66- 'date_invoice': context.get('date_invoice',False),
67+ 'date_invoice': context.get('date_invoice', False),
68 'company_id': order.company_id.id,
69 'user_id': order.user_id and order.user_id.id or False
70 }
71- inv.update(self._inv_get(cr, uid, order))
72+
73+ def _make_invoice(self, cr, uid, order, lines, context=None):
74+ inv_obj = self.pool.get('account.invoice')
75+ obj_invoice_line = self.pool.get('account.invoice.line')
76+ if context is None:
77+ context = {}
78+ invoiced_sale_line_ids = self.pool.get('sale.order.line').search(cr, uid, [('order_id', '=', order.id), ('invoiced', '=', True)], context=context)
79+ from_line_invoice_ids = []
80+ for invoiced_sale_line_id in self.pool.get('sale.order.line').browse(cr, uid, invoiced_sale_line_ids, context=context):
81+ for invoice_line_id in invoiced_sale_line_id.invoice_lines:
82+ if invoice_line_id.invoice_id.id not in from_line_invoice_ids:
83+ from_line_invoice_ids.append(invoice_line_id.invoice_id.id)
84+ for preinv in order.invoice_ids:
85+ if preinv.state not in ('cancel',) and preinv.id not in from_line_invoice_ids:
86+ for preline in preinv.invoice_line:
87+ inv_line_id = obj_invoice_line.copy(cr, uid, preline.id, {'invoice_id': False, 'price_unit': -preline.price_unit})
88+ lines.append(inv_line_id)
89+ inv = self._prepare_invoice(cr, uid, order, lines, context=context)
90 inv_id = inv_obj.create(cr, uid, inv, context=context)
91- data = inv_obj.onchange_payment_term_date_invoice(cr, uid, [inv_id], pay_term, time.strftime(DEFAULT_SERVER_DATE_FORMAT))
92+ data = inv_obj.onchange_payment_term_date_invoice(cr, uid, [inv_id], inv['payment_term'], time.strftime(DEFAULT_SERVER_DATE_FORMAT))
93 if data.get('value', False):
94 inv_obj.write(cr, uid, [inv_id], data['value'], context=context)
95 inv_obj.button_compute(cr, uid, [inv_id])
96@@ -981,7 +979,7 @@
97 'price_unit': 0.0,
98 }
99
100- def _prepare_order_line_invoice_line(self, cr, uid, ids, line, account_id=False, context=None):
101+ def _prepare_order_line_invoice_line(self, cr, uid, line, account_id=False, context=None):
102 """ Builds the invoice line dict from a sale order line
103 @param line: sale order line object
104 @param account_id: the id of the account to force eventually (the method is used for picking return including service)
105@@ -1055,7 +1053,7 @@
106 create_ids = []
107 sales = set()
108 for line in self.browse(cr, uid, ids, context=context):
109- vals = self._prepare_order_line_invoice_line(cr, uid, ids, line, False, context)
110+ vals = self._prepare_order_line_invoice_line(cr, uid, line, False, context)
111 if vals:
112 inv_id = self.pool.get('account.invoice.line').create(cr, uid, vals, context=context)
113 cr.execute('insert into sale_order_line_invoice_rel (order_line_id,invoice_id) values (%s,%s)', (line.id, inv_id))
114
115=== modified file 'sale/stock.py'
116--- sale/stock.py 2012-01-31 13:36:57 +0000
117+++ sale/stock.py 2012-02-01 18:56:22 +0000
118@@ -176,7 +176,7 @@
119 account_id = sale_line.product_id.categ_id.\
120 property_account_expense_categ.id
121
122- vals = order_line_obj._prepare_order_line_invoice_line(cursor, user, ids, sale_line, account_id, context)
123+ vals = order_line_obj._prepare_order_line_invoice_line(cursor, user, sale_line, account_id, context)
124 if vals: #note: in some cases we may not want to include all service lines as invoice lines
125 vals['name'] = name
126 vals['account_analytic_id'] = self._get_account_analytic_invoice(cursor, user, picking, sale_line)

Subscribers

People subscribed via source and target branches

to all changes: