Unable to set a new default name for product.product via copy() call

[Niels Huylebroeck]

Due to how the code is written in product.product method copy[1] we cannot call copy method with default['name'] = 'My New product name' because the value in default['name'] is unconditionally overwritten with the product name + '(copy)'.

A workaround is to do copy() and then a write to update the name to the calculated name.

However I would find it a lot more logical if you would just do a simple test like:

if 'name' not in default or default['name'] == product['name']:
    default['name'] = product['name'] + _(' (copy)')

Which would then enable people to properly copy products and replace the name at the same time.
I checked also the trunk version (r6646) and it seems the same issue is still there[2] as well as on the 6.1 r6637 [3]

[1] http://bazaar.launchpad.net/~openerp/openobject-addons/6.0/view/head:/product/product.py#L617
[2] http://bazaar.launchpad.net/~openerp/openobject-addons/trunk/view/head:/product/product.py#L660
[3] http://bazaar.launchpad.net/~openerp/openobject-addons/6.1/view/head:/product/product.py#L673

[Graeme Gellatly (gdgellatly)]

What about using copy_data?

On Wed, Feb 29, 2012 at 4:59 AM, Niels Huylebroeck <
<email address hidden>> wrote:

> Public bug reported:
>
> Due to how the code is written in product.product method copy[1] we
> cannot call copy method with default['name'] = 'My New product name'
> because the value in default['name'] is unconditionally overwritten with
> the product name + '(copy)'.
>
> A workaround is to do copy() and then a write to update the name to the
> calculated name.
>
> However I would find it a lot more logical if you would just do a simple
> test like:
>
> if 'name' not in default or default['name'] == product['name']:
> default['name'] = product['name'] + _(' (copy)')
>
> Which would then enable people to properly copy products and replace the
> name at the same time.
> I checked also the trunk version (r6646) and it seems the same issue is
> still there[2] as well as on the 6.1 r6637 [3]
>
> [1]
> http://bazaar.launchpad.net/~openerp/openobject-addons/6.0/view/head:/product/product.py#L617
> [2]
> http://bazaar.launchpad.net/~openerp/openobject-addons/trunk/view/head:/product/product.py#L660
> [3]
> http://bazaar.launchpad.net/~openerp/openobject-addons/6.1/view/head:/product/product.py#L673
>
> ** Affects: openobject-addons
> Importance: Undecided
> Status: New
>
> --
> You received this bug notification because you are subscribed to OpenERP
> Addons.
> https://bugs.launchpad.net/bugs/942699
>
> Title:
> Unable to set a new default name for product.product via copy() call
>
> Status in OpenERP Addons (modules):
> New
>
> Bug description:
> Due to how the code is written in product.product method copy[1] we
> cannot call copy method with default['name'] = 'My New product name'
> because the value in default['name'] is unconditionally overwritten
> with the product name + '(copy)'.
>
> A workaround is to do copy() and then a write to update the name to
> the calculated name.
>
> However I would find it a lot more logical if you would just do a
> simple test like:
>
> if 'name' not in default or default['name'] == product['name']:
> default['name'] = product['name'] + _(' (copy)')
>
> Which would then enable people to properly copy products and replace the
> name at the same time.
> I checked also the trunk version (r6646) and it seems the same issue is
> still there[2] as well as on the 6.1 r6637 [3]
>
> [1]
> http://bazaar.launchpad.net/~openerp/openobject-addons/6.0/view/head:/product/product.py#L617
> [2]
> http://bazaar.launchpad.net/~openerp/openobject-addons/trunk/view/head:/product/product.py#L660
> [3]
> http://bazaar.launchpad.net/~openerp/openobject-addons/6.1/view/head:/product/product.py#L673
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/openobject-addons/+bug/942699/+subscriptions
>

[Vishal Parmar(Open ERP) (vpa-openerp)]

Hello Niels Huylebroeck,

In trunk copy method of product.product pass the default = None and also check :

 if not default:
            default = {}

It's not logically correct to pass name in default and in your logical view after all you assign the product['name'].

Would you please elaborate more what you exactly want to do ? with related use case.

Thanks and waiting for your reply.

[Niels Huylebroeck (red15)]

I created a wizard which creates a product by searching for a correct template then copying that and altering certain aspects while copying.

This might not be a use that was thought of when it was coded, but like I showed you this fix would never change the original behavior and is a nice improvement for future developers to use copy() with a proper defaults dictionary instead of having to do a write afterwards.

It also makes the copy() method more predictable whereas now *only* for product.product model you cannot pass a default['name'] = "My desired new name" which is what anyone who is learning the copy() method will expect.

Or as PEP20 states it: Special cases aren't special enough to break the rules.

[Vishal Parmar(Open ERP) (vpa-openerp)]

Hello Niels Huylebroeck,,

Thanks for your reply,

That would indeed be a useful improvement, and technically relatively easy to
do but It may be for your customization . I have tried with

default['name'] = "My desired new name" and tried to copy / duplicated the record and it's work.

However the cases where you need to perform creates a product by searching for a correct template then copying that and altering certain aspects while copying by using wizard .

Current behavior of copy() method is as per expected so we can't implement it currently so I set this issue as 'won't fix' Also It would be a nice idea for a improvement that's why I am setting this issue as 'wishlist'.

Thanks.

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

Just got bitten by this. I do not understand Vishal's reasoning. The product's copy() method does not work as expected. As per orm API, the defaults dictionary argument allows the developer to set values on the resulting resource. In the case of the product however, the 'name' argument is ignored for no good reason. Niels' solution is perfectly reasonable and does not break the case in which the 'name' argument is not present in the defaults dictionary. So I am setting this to 'confirmed' with a low priority.

[Amit Parik (amit-parik)]

Hi Stefan,

Thanks for correcting and sorry for the vishal's mistake, this thing should be improved.

Thank you!