Purchase lines without Invoice Lines Relationship

Bug #724131 reported by Renato Lima - http://www.akretion.com
44
This bug affects 6 people
Affects Status Importance Assigned to Milestone
Odoo Addons (MOVED TO GITHUB)
Fix Released
Low
OpenERP R&D Addons Team 2

Bug Description

Hello,

Using branch addons-6.0 revision number: 4422

I created a purchase order, but the relationship between purchase order line and invoice line does not exist, the field purchase.order.line.invoice_lines is always empty.

Renato Lima - http://www.akretion.com

Related branches

Amit Parik (amit-parik)
Changed in openobject-addons:
assignee: nobody → OpenERP R&D Addons Team 2 (openerp-dev-addons2)
importance: Undecided → Low
status: New → Confirmed
Changed in openobject-addons:
status: Confirmed → In Progress
Revision history for this message
Kirti Savalia(OpenERP) (ksa-openerp) wrote :

hello,

it has been commit in lp:~openerp-dev/openobject-addons/trunk-bug-724131-ksa
Revision no : 4447
Revision ID: <email address hidden>

Thanks.

Changed in openobject-addons:
status: In Progress → Fix Committed
Revision history for this message
Mustufa Rangwala (Open ERP) (mra-tinyerp) wrote :

Kirti,

I can not create new db with purchase module with your patch/commit while i merged your branch.

Changed in openobject-addons:
status: Fix Committed → Confirmed
milestone: none → 6.1
Changed in openobject-addons:
status: Confirmed → In Progress
Revision history for this message
Kirti Savalia(OpenERP) (ksa-openerp) wrote :

Hello,

[1]Create purchase order and in purchase order (second tab) invoice control "Manual".
[2]Then Convert to purchase order.
[3]Go To purchase/invoice control/Purchase Lines Not Invoiced oepn then click on "Create Invoices" wizard and confirm it.
    Then Generate Relationship between purchase order line and invoice line.

Thanks.

Changed in openobject-addons:
status: In Progress → Invalid
Revision history for this message
Kirti Savalia(OpenERP) (ksa-openerp) wrote :

Hello,

I have reverted the changes from branch trunk-bug-724131-ksa with Revision No 4447.

Thanks.

Changed in openobject-addons:
milestone: 6.1 → none
Revision history for this message
Renato Lima - http://www.akretion.com (renatonlima) wrote :

Hello,

If you create an invoice from Purchase/Purchase Order still purchase.order.line.invoice_lines empty without invoice line relationship.

I will set this bug as New

Best Regards,

Renato Lima - www.akretion.com

Changed in openobject-addons:
status: Invalid → New
Revision history for this message
Bpso Administrator (bpso.admin) wrote :

Hello,

We get also this bug when creating a PO with invoicing control on "from order". In this case the invoice is created automatically when confirming the PO. But purchase.order.line.invoice_lines remains empty.

Thanks

Kevin Thompson - BPSO

Revision history for this message
Quentin THEURET @Amaris (qtheuret) wrote :

I confirm Kevin's comment and the bug.

Changed in openobject-addons:
status: New → Confirmed
Revision history for this message
Raphaël Valyi - http://www.akretion.com (rvalyi) wrote :

Hello, I just tested on 6.1 (trunk of 15 October) and I can confirm that the bug is still here.

Revision history for this message
Kirti Savalia(OpenERP) (ksa-openerp) wrote :

Hello,
If you have create relationship between purchase order line and invoice line you must set the invoice control method "Based on purchase order lines".
If you have create relationship between purchase order and invoice then you set the invoice method "Draft invoices pre-generated".
I have attached video for more reference.
Thanks and waiting for your reply!

Revision history for this message
Kevin Thompson (BPSO) (ketho) wrote :

Hello ,

This is the current behaviour in 6.0.3 :
With the invoice control "on order" we get the relationship beteween purchase order and invoice.
With the invoice control "Manual", we get the relationship between purchase order and invoice and between purchase order line and invoice line.

The question (the bug) is that in the 1st case (on order invoicing) the relationships between purchase order line and invoice line are missing.

Hope this is more clear.

Additionaly i'm wondering if this bug will be fixed in 6.0.3 or in 6.1 version?

Best regards,

Kevin

Revision history for this message
Kirti Savalia(OpenERP) (ksa-openerp) wrote :

Hello,

It has been Fixed in http://bazaar.launchpad.net/~openerp-dev/openobject-addons/trunk-bug-724131-ksa/
revision-id: <email address hidden>
revno: 5489
It will be available in trunk soon.

Thanks.

Changed in openobject-addons:
status: Confirmed → Fix Committed
Revision history for this message
Raphaël Valyi - http://www.akretion.com (rvalyi) wrote :

Just a remark about the fix:

of course a fix is certainly better than a bug, so thanks for the fix.

Now, something about the code style:
you create the invoice line here in the middle of the code with:
inv_line_id = self.pool.get('account.invoice.line').create(cr, uid, {...

the issue is that in many localizations (such as our in Brazil) or customization modules, custom fields will need to be added in that invoice line and propagated somehow from the purchase order line (fiscal codes in Brazil to be able to do the legal electronic invoicing for instance).

When you create the order line brutally here. Localization or customization overrides have no choice than browse the invoice line again (from the id) and write into it again.

So for instance if your purchase order has 300 lines, and you need to override that method twice (classical), you will re-browse and re-write into the the invoice lines 300 * 2 = 600 times without any good reason.

Worst: when writing to the invoice line, it will recompute the invoice amount_all stored field 600 times and this one will re-browse into all every single invoice line (eg much slower when hitting line #300).

In a word, this makes OpenERP extremely slow when used in the real world (eg with large documents and localizations). We had cases where confirming and invoice would take 10 minutes or worst. This kind of things makes a customer run away from OpenERP believe me (we lost one customer once because of this).

So yes, this patch is much better than a plain broken functionally, now please consider those aspects in the future.

In contrasts, objects that are highly likely to be localized or customized are better created in their own ovrridable methods
See for instance:
https://code.launchpad.net/~akretion-team/openobject-addons/addons-purchase-extensible-action-picking-create/+merge/79485
or
https://code.launchpad.net/~akretion-team/openobject-addons/addons-sale-extensible-action-ship-create/+merge/76609

Again, it's better to commit that than commit no patch, but please just try to consider my explanation if your have the opportunity or for next OpenERP release.

Revision history for this message
Ferdinand (office-chricar) wrote :

+1
in trunk
* good examples
** sale/sale.py
def _prepare_order_line_move
and others
missing in purchase and many other modules

** purchase/purchase.py
    STATE_SELECTION = [
        ('draft', 'Request for Quotation'),
        ('wait', 'Waiting'),
        ('confirmed', 'Waiting Approval'),
        ('approved', 'Approved'),
        ('except_picking', 'Shipping Exception'),
        ('except_invoice', 'Invoice Exception'),
        ('done', 'Done'),
        ('cancel', 'Cancelled')
    ]

Revision history for this message
Rucha (Open ERP) (rpa-openerp) wrote :

I agree with Raphael, Ferdinand and Graeme,
This is needed to be improved in a flexible way, the fix is not acceptable for now (and later too!),
I am setting it back to confirmed,
Try another workaround for this,

Thanks

Changed in openobject-addons:
status: Fix Committed → Confirmed
Changed in openobject-addons:
status: Confirmed → In Progress
Revision history for this message
Kirti Savalia(OpenERP) (ksa-openerp) wrote :

Hello,

It has been Fixed in http://bazaar.launchpad.net/~openerp-dev/openobject-addons/trunk-bug-724131-ksa/
revision-id: <email address hidden>
revno: 5491
It will be available in trunk soon.

Thanks.

Changed in openobject-addons:
status: In Progress → Fix Committed
Revision history for this message
Raphaël Valyi - http://www.akretion.com (rvalyi) wrote :

Hello,

Kriti and Rucha,

i seems quite better now. But eventually, let me criticize a bit the signature of the new
_prepare_order_line_invoice(self, cr, uid, orders, line_ids, *args)
method you introduced here http://bazaar.launchpad.net/~openerp-dev/openobject-addons/trunk-bug-724131-ksa/revision/5491

Two things don't feel right for me:
1) why pass the orders param. Eventually pass just the order (although you can re(browse it from the order line) but is there any reason to pass the others orders? I mean may be but I'm just curious.
2) the other argument is line_ids. Too me it seems wrong as it has the _id suffix while it's not a id integer but a browse object and also it's in plural while it's actually a single order line. This seems rather confusing to me when you then have in the code things like if line_ids.product_id...
Wouldn't it be better to just call it order_line instead?

Thanks.

Changed in openobject-addons:
status: Fix Committed → New
Revision history for this message
Rucha (Open ERP) (rpa-openerp) wrote :

Hello Raphaël,
Thanks for your review,
I totally agree with you. If you have seen my comment on that merge proposal, you might have found I feel the same for new params (its yet to be approved!)
one more request, don't change the status of bug to "New" as it disappears from our list of bugs "todo", you can change it back to confirmed.

Thanks again for your feedback,

Changed in openobject-addons:
status: New → Confirmed
Revision history for this message
Kirti Savalia(OpenERP) (ksa-openerp) wrote :

Hello,

It has been Fixed in http://bazaar.launchpad.net/~openerp-dev/openobject-addons/trunk-bug-724131-ksa/
revision-id: <email address hidden>
revno: 5659
It will be available in trunk soon.

Thanks.

Changed in openobject-addons:
status: Confirmed → Fix Committed
Revision history for this message
qdp (OpenERP) (qdp) wrote :

merge proposal needs fixing, i'm setting back the bug to confirmed status to reflect it. By the way, that's really nice to see everyone contributing here :-)

Quentin

Changed in openobject-addons:
status: Fix Committed → Confirmed
Revision history for this message
qdp (OpenERP) (qdp) wrote :

Hi,

the final patch landed on trunk in revision 5691. Please note that this revision changed the API by renaming a function and the value it returns.

Thanks everyone for your contribution,
Quentin

Changed in openobject-addons:
status: Confirmed → Fix Released
Revision history for this message
Raphaël Valyi - http://www.akretion.com (rvalyi) wrote :

Hello Quentin,

sorry, I have one last little objection with the current code:

You here defined the following method:
def prepare_inv_line(self, cr, uid, account_id, order_line, context=None)

But in other parts of the code, we rather defined such methods with an underscore prefix such as:
_prepare_inv_line(self, cr, uid, account_id, order_line, context=None)

the reason we did that is because we expect arguments such as order_line which are non RPC serializable primitive objects but server specific "browse" objects. By adding an underscore we avoid to expose those methods as an RPC webservice.

I would expect you would have followed the same convention here. Or else if it's not a good one, let's rework the other such methods to make it consistent.

What do you think?

Changed in openobject-addons:
status: Fix Released → Confirmed
Revision history for this message
qdp (OpenERP) (qdp) wrote :

oops, well seen. This has been fixed by revision 5694!

Changed in openobject-addons:
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Bug attachments

Remote bug watches

Bug watches keep track of this bug in other bug trackers.