Merge lp:~the-clone-master/ocb-addons/6.1-lp212930-purchase_anglo_saxon_invoice_from_PO_lines into lp:ocb-addons/6.1

[Mario Arias]

[Mario Arias (the-clone-master)]

Oppss... correct bug related to this branch is lp:1212930... I skipped the first "1" when reporting...

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

Hi Mario,

Great work! I verified that the diff is equivalent to http://bazaar.launchpad.net/~openerp/openobject-addons/trunk/revision/8787, and that the relevant sections of the files involved have not been changed in trunk in a significant way ever since.

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://bazaar.launchpad.net/~openerp/openobject-addons/7.0/revision/9293

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.

review: Approve (code review)

[Mario Arias (the-clone-master)]

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://bazaar.launchpad.net/~openerp/openobject-addons/trunk/revision/8787, and that the relevant sections of the files involved have not been changed in trunk in a significant way ever since.
>
> 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://bazaar.launchpad.net/~openerp/openobject-addons/7.0/revision/9293
>
> 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)]

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)]

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)]

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)]

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://code.launchpad.net/~the-clone-master/ocb-addons/6.1-lp212930-purchase_anglo_saxon_invoice_from_PO_lines/+merge/180621
> 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)]

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)]

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.

review: Approve

[Holger Brunn (Therp) (hbrunn)]

review: Approve (code review)

[Paulius Sladkevičius @ hbee (komsas)]

Hi Mario,

I found that ccount_anglo_saxon/purchase.py have a bug import is missing: from tools.translate import _

review: Needs Fixing

=== modified file 'account_anglo_saxon/purchase.py'
--- account_anglo_saxon/purchase.py	2011-12-19 16:54:40 +0000
+++ account_anglo_saxon/purchase.py	2013-08-16 17:00:41 +0000
@@ -26,17 +26,17 @@
     _inherit = "purchase.order"
     _description = "Purchase Order"
 
-    def _prepare_inv_line(self, cr, uid, account_id, order_line, context=None):
-        line = super(purchase_order, self)._prepare_inv_line(cr, uid, account_id, order_line, context=context)
+    def _choose_account_from_po_line(self, cr, uid, order_line, context=None):
+        account_id = super(purchase_order, self)._choose_account_from_po_line(cr, uid, order_line, context=context)
         if order_line.product_id and not order_line.product_id.type == 'service':
             acc_id = order_line.product_id.property_stock_account_input and order_line.product_id.property_stock_account_input.id
             if not acc_id:
                 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
-            if acc_id:
+            if not acc_id:
+                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,))    
+            else:    
                 fpos = order_line.order_id.fiscal_position or False
-                new_account_id = self.pool.get('account.fiscal.position').map_account(cr, uid, fpos, acc_id)
-                line.update({'account_id': new_account_id})
-        return line
-purchase_order()
+                account_id = self.pool.get('account.fiscal.position').map_account(cr, uid, fpos, acc_id)
+        return account_id
 
 # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:
 
=== modified file 'purchase/purchase.py' (properties changed: -x to +x)
--- purchase/purchase.py	2012-10-26 15:32:12 +0000
+++ purchase/purchase.py	2013-08-16 17:00:41 +0000
@@ -294,6 +294,20 @@
             self.write(cr, uid, [id], {'state' : 'confirmed', 'validator' : uid})
         return True
 
+    def _choose_account_from_po_line(self, cr, uid, po_line, context=None):
+        fiscal_obj = self.pool.get('account.fiscal.position')
+        property_obj = self.pool.get('ir.property')
+        if po_line.product_id:
+            acc_id = po_line.product_id.property_account_expense.id
+            if not acc_id:
+                acc_id = po_line.product_id.categ_id.property_account_expense_categ.id
+            if not acc_id:
+                raise osv.except_osv(_('Error!'), _('Define expense account for this company: "%s" (id:%d).') % (po_line.product_id.name, po_line.product_id.id,))
+        else:
+            acc_id = property_obj.get(cr, uid, 'property_account_expense_categ', 'product.category').id
+        fpos = po_line.order_id.fiscal_position or False
+        return fiscal_obj.map_account(cr, uid, fpos, acc_id)
+
     def _prepare_inv_line(self, cr, uid, account_id, order_line, context=None):
         """Collects require data from purchase order line that is used to create invoice line 
         for that purchase order line
@@ -338,8 +352,6 @@
         journal_obj = self.pool.get('account.journal')
         inv_obj = self.pool.get('account.invoice')
         inv_line_obj = self.pool.get('account.invoice.line')
-        fiscal_obj = self.pool.get('account.fiscal.position')
-        property_obj = self.pool.get('ir.property')
 
         for order in self.browse(cr, uid, ids, context=context):
             pay_acc_id = order.partner_id.property_account_payable.id
@@ -351,16 +363,7 @@
             # generate invoice line correspond to PO line and link that to created invoice (inv_id) and PO line
             inv_lines = []
             for po_line in order.order_line:
-                if po_line.product_id:
-                    acc_id = po_line.product_id.product_tmpl_id.property_account_expense.id
-                    if not acc_id:
-                        acc_id = po_line.product_id.categ_id.property_account_expense_categ.id
-                    if not acc_id:
-                        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,))
-                else:
-                    acc_id = property_obj.get(cr, uid, 'property_account_expense_categ', 'product.category').id
-                fpos = order.fiscal_position or False
-                acc_id = fiscal_obj.map_account(cr, uid, fpos, acc_id)
+                acc_id = self._choose_account_from_po_line(cr, uid, po_line, context=context)
 
                 inv_line_data = self._prepare_inv_line(cr, uid, acc_id, po_line, context=context)
                 inv_line_id = inv_line_obj.create(cr, uid, inv_line_data, context=context)
@@ -539,7 +542,6 @@
          @param context: A standard dictionary
 
          @return: new purchase order id
-
         """
         #TOFIX: merged order line should be unlink
         wf_service = netsvc.LocalService("workflow")
 
=== modified file 'purchase/wizard/purchase_line_invoice.py' (properties changed: -x to +x)
--- purchase/wizard/purchase_line_invoice.py	2011-09-08 09:27:55 +0000
+++ purchase/wizard/purchase_line_invoice.py	2013-08-16 17:00:41 +0000
@@ -48,12 +48,11 @@
         if record_ids:
             res = False
             invoices = {}
-            invoice_obj=self.pool.get('account.invoice')
-            purchase_line_obj=self.pool.get('purchase.order.line')
-            property_obj=self.pool.get('ir.property')
-            account_fiscal_obj=self.pool.get('account.fiscal.position')
-            invoice_line_obj=self.pool.get('account.invoice.line')
-            account_jrnl_obj=self.pool.get('account.journal')
+            invoice_obj = self.pool.get('account.invoice')
+            purchase_obj = self.pool.get('purchase.order')
+            purchase_line_obj = self.pool.get('purchase.order.line')
+            invoice_line_obj = self.pool.get('account.invoice.line')
+            account_jrnl_obj = self.pool.get('account.journal')
 
             def multiple_order_invoice_notes(orders):
                 notes = ""
@@ -62,7 +61,6 @@
                 return notes
 
 
-
             def make_invoice_by_partner(partner, orders, lines_ids):
                 """
                     create a new invoice for one supplier
@@ -99,37 +97,16 @@
                     order.write({'invoice_ids': [(4, inv_id)]})
                 return inv_id
 
-            for line in purchase_line_obj.browse(cr,uid,record_ids):
-                if (not line.invoiced) and (line.state not in ('draft','cancel')):
+            for line in purchase_line_obj.browse(cr, uid, record_ids, context=context):
+                if (not line.invoiced) and (line.state not in ('draft', 'cancel')):
                     if not line.partner_id.id in invoices:
                         invoices[line.partner_id.id] = []
-                    if line.product_id:
-                        a = line.product_id.product_tmpl_id.property_account_expense.id
-                        if not a:
-                            a = line.product_id.categ_id.property_account_expense_categ.id
-                        if not a:
-                            raise osv.except_osv(_('Error !'),
-                                    _('There is no expense account defined ' \
-                                            'for this product: "%s" (id:%d)') % \
-                                            (line.product_id.name, line.product_id.id,))
-                    else:
-                        a = property_obj.get(cr, uid,
-                                'property_account_expense_categ', 'product.category',
-                                context=context).id
-                    fpos = line.order_id.fiscal_position or False
-                    a = account_fiscal_obj.map_account(cr, uid, fpos, a)
-                    inv_id = invoice_line_obj.create(cr, uid, {
-                        'name': line.name,
-                        'origin': line.order_id.name,
-                        'account_id': a,
-                        'price_unit': line.price_unit,
-                        'quantity': line.product_qty,
-                        'uos_id': line.product_uom.id,
-                        'product_id': line.product_id.id or False,
-                        'invoice_line_tax_id': [(6, 0, [x.id for x in line.taxes_id])],
-                        'note': line.notes,
-                        'account_analytic_id': line.account_analytic_id and line.account_analytic_id.id or False,
-                    })
+                        
+                    acc_id = purchase_obj._choose_account_from_po_line(cr, uid, line, context=context)
+                    inv_line_data = purchase_obj._prepare_inv_line(cr, uid, acc_id, line, context=context)
+                    inv_line_data.update({'origin': line.order_id.name})
+                    inv_id = invoice_line_obj.create(cr, uid, inv_line_data, context=context)
+                    
                     purchase_line_obj.write(cr, uid, [line.id], {'invoiced': True, 'invoice_lines': [(4, inv_id)]})
                     invoices[line.partner_id.id].append((line,inv_id))