Crash if list_price is None

Although list_price field is defined as required in product.template:

https://hg.tryton.org/modules/product/file/tip/product.py#l74

This cannot really be enforced because it is MultiValue. So a product created from company A will have list_price set to NULL on company B.

This will cause crash on get_sale_price() if there needs to be a currency conversion:

https://hg.tryton.org/modules/sale/file/tip/product.py#l158

I think we should:

• Make list_price not required (we already removed the required attribute from cost_price)
• Either do not supply None to currency's compute() or make compute() return None if the value to convert is None

added sales type::crash + 1 deleted label

To properly solve the issue all modules should be reviewed to ensure that they do not crash when the list price is not set. Then we can make the list price not required.

added 1 deleted label and removed 1 deleted label

Idem for cost_price. I think the proper solution is to reuse the same design as account_used because for me it must raise an error if there is no price defined instead of using a default value (which will probably be zero).

Indeed we can not use the same design as account_used because we call sometime list_price_used but not really use it. Indeed most of the time it is to fill a field which is required. So for me it is better to avoid any crash by using a None list price as value in computation and let the required error message be raised.

assigned to @ced

added 1 deleted label and removed 1 deleted label

Also the cost_price is required and must stay required because it has a default value of 0 when it is not filled (the internal storage should make it required). It is not a real problem as user can update the cost price later and have it recomputed the full history.

added product + 1 deleted label and removed 1 deleted label

New review350341002 at https://codereview.tryton.org/350341002/#ps350351002

review350341002 updated at https://codereview.tryton.org/350341002/#ps363781003

I understand that the addition of the new Null class is to prevent crashes but I think it's making things more complex.

The addition of a new Null concept not used anywhere else adds a new way of handling stuff which makes learning the framework more complex. I'd rather see a bunch bugs popuping up than having this special case in the code.

That said, I know I'm not the one that doing the hard work nor being the one fixing the bugs that may arise so I understand the other POV. Just wanted to add my 2 cents here.

The Null object should not leave the product_price_list module so I do not see it as something to learn from the framework.

AFAICS, it is published here:

https://codereview.tryton.org/350341002/diff/363781003/modules/product_price_list/__init__.py

and used by other modules which extend product_price_list:

https://codereview.tryton.org/350341002/diff/363781003/modules/sale_promotion/sale.py?context=10&column_width=80

review350341002 updated at https://codereview.tryton.org/350341002/#ps363861002

New changeset 78fc7dad87a8 by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/carrier/rev/78fc7dad87a8

added 1 deleted label and removed 1 deleted label

closed

New changeset 48e257979e4b by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/product/rev/48e257979e4b

New changeset ca49a47fefe0 by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/product_price_list/rev/ca49a47fefe0

New changeset 00702ac99acc by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/purchase/rev/00702ac99acc

New changeset 94ba77090407 by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/purchase_shipment_cost/rev/94ba77090407

New changeset e646c3af990e by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/sale/rev/e646c3af990e

New changeset bfe9f61c856d by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/sale_price_list/rev/bfe9f61c856d

New changeset f0d0ff90fff6 by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/sale_promotion/rev/f0d0ff90fff6

New changeset 3796a0f48971 by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/sale_shipment_cost/rev/3796a0f48971

New changeset 11309a377837 by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/sale_subscription/rev/11309a377837

New changeset 55b0b2e54266 by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/stock_consignment/rev/55b0b2e54266

New changeset 32c42e2dc2fe by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/web_shop/rev/32c42e2dc2fe

New changeset e87b2dcb5655 by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/web_shop_vue_storefront/rev/e87b2dcb5655

New changeset 4d5b99c67d93 by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/tryton-env/rev/4d5b99c67d93

New changeset 277353eb1142 by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/customs/rev/277353eb1142

New changeset eceb95967428 by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/tryton-env/rev/eceb95967428

mentioned in issue #10791 (closed)

mentioned in issue #11084 (closed)

mentioned in issue #11751 (closed)

marked #4426 (closed) as a duplicate of this issue

marked this issue as related to #4426 (closed)