[6.0.x][purchase] action_picking_create: broken extensibility

Bug #788789 reported by Raphaël Valyi - http://www.akretion.com
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Odoo Addons (MOVED TO GITHUB)
Invalid
Low
OpenERP R&D Addons Team 2

Bug Description

Hello,

there is a design issue in purchase#action_picking_create
the method actually applies to a collection of purchase orders but actually returns only the id of the last picking that is generated.
To play nice efficiently with overriders, you need to change that and return a dictionary of all the generated pickings, just as you do in sale#action_ship_create

Related branches

Revision history for this message
Amit Parik (amit-parik) wrote :

Hello Raphael Valyi,

I have checked your issue with latest trunk and stable both.

But I have seen that in sale module "action_ship_create" return a "True" not dictionary of all the generated pickings.

So would you please elaborate with more information on this and tell where you faced the problem if purchase#action_picking_create method returns a only the id of the last picking.

Thanks.

Changed in openobject-addons:
status: New → Incomplete
Revision history for this message
Raphaël Valyi - http://www.akretion.com (rvalyi) wrote : Re: [Bug 788789] Re: [6.0.x][purchase] action_picking_create: broken extensibility

Hello Amit,

You are right, in the sale module too action_ship_create create shippings
and doesn't properly returns the ids of what it created as one would expect
from a method creating database records for proper extensibility. You can
still find out those ids bit hitting the database again in your overrider
but that's certainly not a clean and efficient solution.

Now, in purchase it's worth because you return some arbitrary id: the last
one, this is confusing and useless cause it will never be robust.
So I would even prefer you return True/False instead.
but ideally methods that create database records return the ids of what they
created. End even more ideally you even get a chance to get the
values/objects that will be created/written in the database BEFORE writing
them to the database, so you can minimize database accesses accross
overriders and change objects before they are persisted instead of having to
persist and them hit the the database again to read them also may be change
them.

Any chance we improve here on the main methods such as
purchase#action_ship_create for 6.1 or will that be postponed with all the
friction that will imply when the user base will be larger?

On Tue, Jun 7, 2011 at 8:38 AM, Amit Parik (OpenERP) <email address hidden>wrote:

> Hello Raphael Valyi,
>
> I have checked your issue with latest trunk and stable both.
>
> But I have seen that in sale module "action_ship_create" return a "True"
> not dictionary of all the generated pickings.
>
> So would you please elaborate with more information on this and tell
> where you faced the problem if purchase#action_picking_create method
> returns a only the id of the last picking.
>
> Thanks.
>
> ** Changed in: openobject-addons
> Status: New => Incomplete
>
> --
> You received this bug notification because you are a member of OpenERP
> Drivers, which is subscribed to OpenERP Addons.
> https://bugs.launchpad.net/bugs/788789
>
> Title:
> [6.0.x][purchase] action_picking_create: broken extensibility
>
> Status in OpenERP Modules (addons):
> Incomplete
>
> Bug description:
> Hello,
>
> there is a design issue in purchase#action_picking_create
> the method actually applies to a collection of purchase orders but
> actually returns only the id of the last picking that is generated.
> To play nice efficiently with overriders, you need to change that and
> return a dictionary of all the generated pickings, just as you do in
> sale#action_ship_create
>

Changed in openobject-addons:
status: Incomplete → Triaged
Revision history for this message
Amit Parik (amit-parik) wrote :

Hello,

I have checked the whole bug report.
Here I have faced the two issue.

1) The action_picking_create method in purchase is return a last generated picking_id but it should be same as a sale#action_ship_create method.

2) As per Raphael said it should be return the dictionary of all the generated pickings not a "True".

@ Raphael: I am agree with you. It's good suggestion that the method will reruns dictionary of all the generated pickings so when we called a method via script then we will get the proper result.
But currently we consider this suggestion as a feature and we will consider it as a feature roadmaps.

@Team : I am confirming this bug for above two issue but the issue #1 as a "Low" importance and issue#2 as a "Wishlist" .

So currently I am setting this as a "Low" importance after completed issue#1 the team will set this as a "Wishlist".

Thanks for understanding!

Changed in openobject-addons:
assignee: nobody → OpenERP R&D Addons Team 2 (openerp-dev-addons2)
importance: Undecided → Low
status: Triaged → Confirmed
Revision history for this message
Rucha (Open ERP) (rpa-openerp) wrote :

hello Amit,
I prefer to have a separate bug report for the point#2 for better understanding and clarity of what's going on which bug,
we consider issue#1 is going to be fixed for this bug report.
thanks for nice explanation :-)

Revision history for this message
Amit Parik (amit-parik) wrote :

Hello Rucha,

Thanks for your reply.

I have posted a new bug report for the issue#2 on lp:834402.
So would you please see also this lp:834402.

Thanks again.

Changed in openobject-addons:
status: Confirmed → In Progress
Revision history for this message
Mayur Maheshwari(OpenERP) (mma-openerp) wrote :

Hello,

Thanks for Reporting.
It has been fixed in lp:~openerp-dev/openobject-addons/trunk-bug-788789-mma
Revision ID: <email address hidden>
Revision num: 4972.

It will be available in trunk soon.

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

finally, I can not apply a change, as it breaks others method.
We keep as it fo rnow on, for compatibility issues.

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

Hello Fabien,

Actually, what would be good I believe would be to extract sub methods out
of this method.
I just proposed such a patch for the sale module here:
https://code.launchpad.net/~akretion-team/openobject-addons/addons-sale-extensible-action-ship-create/+merge/76609
Please give a position on this merge. If you merge it, I have a few other
places such as here where such compaible refactoring would help us with
extension modules.

On Sun, Sep 25, 2011 at 7:53 PM, Fabien (Open ERP) <email address hidden> wrote:

> finally, I can not apply a change, as it breaks others method.
> We keep as it fo rnow on, for compatibility issues.
>
> ** Changed in: openobject-addons
> Status: Fix Committed => Fix Released
>
> ** Changed in: openobject-addons
> Status: Fix Released => Invalid
>
> --
> You received this bug notification because you are a member of OpenERP
> Drivers, which is subscribed to OpenERP Addons.
> https://bugs.launchpad.net/bugs/788789
>
> Title:
> [6.0.x][purchase] action_picking_create: broken extensibility
>
> Status in OpenERP Addons (modules):
> Invalid
>
> Bug description:
> Hello,
>
> there is a design issue in purchase#action_picking_create
> the method actually applies to a collection of purchase orders but
> actually returns only the id of the last picking that is generated.
> To play nice efficiently with overriders, you need to change that and
> return a dictionary of all the generated pickings, just as you do in
> sale#action_ship_create
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/openobject-addons/+bug/788789/+subscriptions
>

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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