Merge lp:~the-clone-master/ocb-addons/6.1-lp212930-purchase_anglo_saxon_invoice_from_PO_lines into lp:ocb-addons/6.1
- 6.1-lp212930-purchase_anglo_saxon_invoice_from_PO_lines
- Merge into 6.1
Status: | Merged |
---|---|
Merged at revision: | 6800 |
Proposed branch: | lp:~the-clone-master/ocb-addons/6.1-lp212930-purchase_anglo_saxon_invoice_from_PO_lines |
Merge into: | lp:ocb-addons/6.1 |
Diff against target: |
163 lines (+35/-56) 3 files modified
account_anglo_saxon/purchase.py (+7/-7) purchase/purchase.py (+15/-13) purchase/wizard/purchase_line_invoice.py (+13/-36) |
To merge this branch: | bzr merge lp:~the-clone-master/ocb-addons/6.1-lp212930-purchase_anglo_saxon_invoice_from_PO_lines |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Paulius Sladkevičius @ hbee (community) | Needs Fixing | ||
Holger Brunn (Therp) | code review | Approve | |
Virgil Dupras (community) | Approve | ||
Stefan Rijnhart (Opener) | code review | Approve | |
Review via email: mp+180621@code.launchpad.net |
Commit message
Description of the change
Mario Arias (the-clone-master) wrote : | # |
Stefan Rijnhart (Opener) (stefan-opener) wrote : | # |
Hi Mario,
Great work! I verified that the diff is equivalent to http://
I was going to ask you about the status in 7.0 (we don't want to introduce regressions by fixing things in older releases only) but I noticed that for 7.0 it is already fixed here: http://
By convention, I am setting the related issue to 'Affecting OpenERP 6.1' with status Won't Fix and the status in OCB-Addons to Fix Committed, as your proposal has not yet been merged in the main branch.
Mario Arias (the-clone-master) wrote : | # |
Hi Stefan,
Sorry, I forgot to mention that it was already fixed for 7.0.
I am working on Kevin McMenamin's modules for landed cost, to port them
to 6.1 and later on to 7.0.
As part of their work, they have fixed a lot of issues with official
Anglo Saxon module, and I will be reporting them on launchpad to try to
get them fixed on official and OCB branches.
When done, I will ask him for permission to make them public, as they
made an excellent job. Maybe we can have them on community project...
Regards,
-Mario
On Sat 17 Aug 2013 03:40:07 AM CST, Stefan Rijnhart (Therp) wrote:
> Review: Approve code review
>
> Hi Mario,
>
> Great work! I verified that the diff is equivalent to http://
>
> I was going to ask you about the status in 7.0 (we don't want to introduce regressions by fixing things in older releases only) but I noticed that for 7.0 it is already fixed here: http://
>
> By convention, I am setting the related issue to 'Affecting OpenERP 6.1' with status Won't Fix and the status in OCB-Addons to Fix Committed, as your proposal has not yet been merged in the main branch.
Mario Arias (the-clone-master) wrote : | # |
Hi,
Is there any other step required for getting this merged on main OCB branch ?
Just let me know to proceed...
Thanks
Stefan Rijnhart (Opener) (stefan-opener) wrote : | # |
To put it simply, it takes more reviewers to deal with all the proposals.
In some cases I would be comfortable with merging a proposal that carry only my own approval when no one else came to support the proposal after some time, but this is not such a trivial change as that so I would really like someone else to approve.
Virgil Dupras (hsoft) wrote : | # |
I just want to say that I'm not ignoring this MP. I will review it, but being unfamiliar with the module and problem domain (cost of goods sold, anglo-saxon vs continental, etc.), I have a lot of catching on to do to be able to do a meaningful review.
Mario Arias (the-clone-master) wrote : | # |
Thanks Virgil, Stefan. I know it is hard work to review code...
In case this helps reviewers, this change is already applied to 7.0 and
trunk branches.
I just checked that underline modules had not changed between versions and
then applied the same changes...
If there is anything I can do to help, just let me know.
El 20/09/2013 06:31, "Virgil Dupras" <email address hidden>
escribió:
> I just want to say that I'm not ignoring this MP. I will review it, but
> being unfamiliar with the module and problem domain (cost of goods sold,
> anglo-saxon vs continental, etc.), I have a lot of catching on to do to be
> able to do a meaningful review.
> --
>
> https:/
> You are the owner of
> lp:~the-clone-master/ocb-addons/6.1-lp212930-purchase_anglo_saxon_invoice_from_PO_lines.
>
Mario Arias (the-clone-master) wrote : | # |
Any reviewer that can approve this and merge, please...
It comes from v7/trunk, and is already tested in a couple production systems...
Thanks !!
Virgil Dupras (hsoft) wrote : | # |
I lack the expertise and time to properly review this MP, but I did like Stefan and compared the diff from the trunk with this MP and they look identical except for the lines 16-19 which aren't in the trunk's diff.
Now it's up to the maintainer of this project to determine whether code-only reviews are sufficient for merging.
Holger Brunn (Therp) (hbrunn) : | # |
Paulius Sladkevičius @ hbee (komsas) wrote : | # |
Hi Mario,
I found that ccount_
Preview Diff
1 | === modified file 'account_anglo_saxon/purchase.py' |
2 | --- account_anglo_saxon/purchase.py 2011-12-19 16:54:40 +0000 |
3 | +++ account_anglo_saxon/purchase.py 2013-08-16 17:00:41 +0000 |
4 | @@ -26,17 +26,17 @@ |
5 | _inherit = "purchase.order" |
6 | _description = "Purchase Order" |
7 | |
8 | - def _prepare_inv_line(self, cr, uid, account_id, order_line, context=None): |
9 | - line = super(purchase_order, self)._prepare_inv_line(cr, uid, account_id, order_line, context=context) |
10 | + def _choose_account_from_po_line(self, cr, uid, order_line, context=None): |
11 | + account_id = super(purchase_order, self)._choose_account_from_po_line(cr, uid, order_line, context=context) |
12 | if order_line.product_id and not order_line.product_id.type == 'service': |
13 | acc_id = order_line.product_id.property_stock_account_input and order_line.product_id.property_stock_account_input.id |
14 | if not acc_id: |
15 | acc_id = order_line.product_id.categ_id.property_stock_account_input_categ and order_line.product_id.categ_id.property_stock_account_input_categ.id |
16 | - if acc_id: |
17 | + if not acc_id: |
18 | + raise osv.except_osv(_('Error !'), _('There is no stock input account defined for this product or category: "%s" (id:%d)') % (order_line.product_id.name, order_line.product_id.id,)) |
19 | + else: |
20 | fpos = order_line.order_id.fiscal_position or False |
21 | - new_account_id = self.pool.get('account.fiscal.position').map_account(cr, uid, fpos, acc_id) |
22 | - line.update({'account_id': new_account_id}) |
23 | - return line |
24 | -purchase_order() |
25 | + account_id = self.pool.get('account.fiscal.position').map_account(cr, uid, fpos, acc_id) |
26 | + return account_id |
27 | |
28 | # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: |
29 | |
30 | === modified file 'purchase/purchase.py' (properties changed: -x to +x) |
31 | --- purchase/purchase.py 2012-10-26 15:32:12 +0000 |
32 | +++ purchase/purchase.py 2013-08-16 17:00:41 +0000 |
33 | @@ -294,6 +294,20 @@ |
34 | self.write(cr, uid, [id], {'state' : 'confirmed', 'validator' : uid}) |
35 | return True |
36 | |
37 | + def _choose_account_from_po_line(self, cr, uid, po_line, context=None): |
38 | + fiscal_obj = self.pool.get('account.fiscal.position') |
39 | + property_obj = self.pool.get('ir.property') |
40 | + if po_line.product_id: |
41 | + acc_id = po_line.product_id.property_account_expense.id |
42 | + if not acc_id: |
43 | + acc_id = po_line.product_id.categ_id.property_account_expense_categ.id |
44 | + if not acc_id: |
45 | + raise osv.except_osv(_('Error!'), _('Define expense account for this company: "%s" (id:%d).') % (po_line.product_id.name, po_line.product_id.id,)) |
46 | + else: |
47 | + acc_id = property_obj.get(cr, uid, 'property_account_expense_categ', 'product.category').id |
48 | + fpos = po_line.order_id.fiscal_position or False |
49 | + return fiscal_obj.map_account(cr, uid, fpos, acc_id) |
50 | + |
51 | def _prepare_inv_line(self, cr, uid, account_id, order_line, context=None): |
52 | """Collects require data from purchase order line that is used to create invoice line |
53 | for that purchase order line |
54 | @@ -338,8 +352,6 @@ |
55 | journal_obj = self.pool.get('account.journal') |
56 | inv_obj = self.pool.get('account.invoice') |
57 | inv_line_obj = self.pool.get('account.invoice.line') |
58 | - fiscal_obj = self.pool.get('account.fiscal.position') |
59 | - property_obj = self.pool.get('ir.property') |
60 | |
61 | for order in self.browse(cr, uid, ids, context=context): |
62 | pay_acc_id = order.partner_id.property_account_payable.id |
63 | @@ -351,16 +363,7 @@ |
64 | # generate invoice line correspond to PO line and link that to created invoice (inv_id) and PO line |
65 | inv_lines = [] |
66 | for po_line in order.order_line: |
67 | - if po_line.product_id: |
68 | - acc_id = po_line.product_id.product_tmpl_id.property_account_expense.id |
69 | - if not acc_id: |
70 | - acc_id = po_line.product_id.categ_id.property_account_expense_categ.id |
71 | - if not acc_id: |
72 | - raise osv.except_osv(_('Error !'), _('There is no expense account defined for this product: "%s" (id:%d)') % (po_line.product_id.name, po_line.product_id.id,)) |
73 | - else: |
74 | - acc_id = property_obj.get(cr, uid, 'property_account_expense_categ', 'product.category').id |
75 | - fpos = order.fiscal_position or False |
76 | - acc_id = fiscal_obj.map_account(cr, uid, fpos, acc_id) |
77 | + acc_id = self._choose_account_from_po_line(cr, uid, po_line, context=context) |
78 | |
79 | inv_line_data = self._prepare_inv_line(cr, uid, acc_id, po_line, context=context) |
80 | inv_line_id = inv_line_obj.create(cr, uid, inv_line_data, context=context) |
81 | @@ -539,7 +542,6 @@ |
82 | @param context: A standard dictionary |
83 | |
84 | @return: new purchase order id |
85 | - |
86 | """ |
87 | #TOFIX: merged order line should be unlink |
88 | wf_service = netsvc.LocalService("workflow") |
89 | |
90 | === modified file 'purchase/wizard/purchase_line_invoice.py' (properties changed: -x to +x) |
91 | --- purchase/wizard/purchase_line_invoice.py 2011-09-08 09:27:55 +0000 |
92 | +++ purchase/wizard/purchase_line_invoice.py 2013-08-16 17:00:41 +0000 |
93 | @@ -48,12 +48,11 @@ |
94 | if record_ids: |
95 | res = False |
96 | invoices = {} |
97 | - invoice_obj=self.pool.get('account.invoice') |
98 | - purchase_line_obj=self.pool.get('purchase.order.line') |
99 | - property_obj=self.pool.get('ir.property') |
100 | - account_fiscal_obj=self.pool.get('account.fiscal.position') |
101 | - invoice_line_obj=self.pool.get('account.invoice.line') |
102 | - account_jrnl_obj=self.pool.get('account.journal') |
103 | + invoice_obj = self.pool.get('account.invoice') |
104 | + purchase_obj = self.pool.get('purchase.order') |
105 | + purchase_line_obj = self.pool.get('purchase.order.line') |
106 | + invoice_line_obj = self.pool.get('account.invoice.line') |
107 | + account_jrnl_obj = self.pool.get('account.journal') |
108 | |
109 | def multiple_order_invoice_notes(orders): |
110 | notes = "" |
111 | @@ -62,7 +61,6 @@ |
112 | return notes |
113 | |
114 | |
115 | - |
116 | def make_invoice_by_partner(partner, orders, lines_ids): |
117 | """ |
118 | create a new invoice for one supplier |
119 | @@ -99,37 +97,16 @@ |
120 | order.write({'invoice_ids': [(4, inv_id)]}) |
121 | return inv_id |
122 | |
123 | - for line in purchase_line_obj.browse(cr,uid,record_ids): |
124 | - if (not line.invoiced) and (line.state not in ('draft','cancel')): |
125 | + for line in purchase_line_obj.browse(cr, uid, record_ids, context=context): |
126 | + if (not line.invoiced) and (line.state not in ('draft', 'cancel')): |
127 | if not line.partner_id.id in invoices: |
128 | invoices[line.partner_id.id] = [] |
129 | - if line.product_id: |
130 | - a = line.product_id.product_tmpl_id.property_account_expense.id |
131 | - if not a: |
132 | - a = line.product_id.categ_id.property_account_expense_categ.id |
133 | - if not a: |
134 | - raise osv.except_osv(_('Error !'), |
135 | - _('There is no expense account defined ' \ |
136 | - 'for this product: "%s" (id:%d)') % \ |
137 | - (line.product_id.name, line.product_id.id,)) |
138 | - else: |
139 | - a = property_obj.get(cr, uid, |
140 | - 'property_account_expense_categ', 'product.category', |
141 | - context=context).id |
142 | - fpos = line.order_id.fiscal_position or False |
143 | - a = account_fiscal_obj.map_account(cr, uid, fpos, a) |
144 | - inv_id = invoice_line_obj.create(cr, uid, { |
145 | - 'name': line.name, |
146 | - 'origin': line.order_id.name, |
147 | - 'account_id': a, |
148 | - 'price_unit': line.price_unit, |
149 | - 'quantity': line.product_qty, |
150 | - 'uos_id': line.product_uom.id, |
151 | - 'product_id': line.product_id.id or False, |
152 | - 'invoice_line_tax_id': [(6, 0, [x.id for x in line.taxes_id])], |
153 | - 'note': line.notes, |
154 | - 'account_analytic_id': line.account_analytic_id and line.account_analytic_id.id or False, |
155 | - }) |
156 | + |
157 | + acc_id = purchase_obj._choose_account_from_po_line(cr, uid, line, context=context) |
158 | + inv_line_data = purchase_obj._prepare_inv_line(cr, uid, acc_id, line, context=context) |
159 | + inv_line_data.update({'origin': line.order_id.name}) |
160 | + inv_id = invoice_line_obj.create(cr, uid, inv_line_data, context=context) |
161 | + |
162 | purchase_line_obj.write(cr, uid, [line.id], {'invoiced': True, 'invoice_lines': [(4, inv_id)]}) |
163 | invoices[line.partner_id.id].append((line,inv_id)) |
164 |
Oppss... correct bug related to this branch is lp:1212930... I skipped the first "1" when reporting...