Issue 10244

Title
Crash if list_price is None
Priority
bug
Status
resolved
Superseder
Carrier compute price based on context company (issue 10666)
Project-Id-Version: Roundup 0.7.0 Report-Msgid-Bugs-To: roundup-devel@lists.sourceforge.net POT-Creation-Date: 2020-07-12 23:40-0400 PO-Revision-Date: 2004-11-20 13:47+0200 Language-Team: English Language: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii
Restore instance context for price_list_used (issue 10664)
Nosy list
albertca, ced, pokoli, reviewbot, roundup-bot
Assigned to
ced
Keywords
review

Created on 2021-04-01.17:13:58 by albertca, last changed 7 days ago by roundup-bot.

Messages

New changeset eceb95967428 by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/tryton-env/rev/eceb95967428
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 4d5b99c67d93 by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/tryton-env/rev/4d5b99c67d93
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 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 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 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 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 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 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 e646c3af990e by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/sale/rev/e646c3af990e
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 00702ac99acc by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/purchase/rev/00702ac99acc
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 48e257979e4b by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/product/rev/48e257979e4b
New changeset 78fc7dad87a8 by Cédric Krier in branch 'default':
Support empty list price on product
https://hg.tryton.org/modules/carrier/rev/78fc7dad87a8
Author: [hidden] (albertca)
Date: 2021-08-27.13:53:36
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-08-27.13:34:22

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

Author: [hidden] (albertca)
Date: 2021-08-27.10:10:44

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.

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-08-17.17:38:00

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.

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-08-17.17:35:31

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.

Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-05-21.22:47:16

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

Author: [hidden] (pokoli) Tryton committer Tryton translator
Date: 2021-04-04.08:05:07

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.

Author: [hidden] (albertca)
Date: 2021-04-01.17:13:57

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
History
Date User Action Args
2021-10-12 10:31:29roundup-botsetmessages: + msg70951
2021-10-12 10:31:21roundup-botsetmessages: + msg70950
2021-09-13 23:31:52roundup-botsetmessages: + msg70074
2021-09-13 23:31:49roundup-botsetmessages: + msg70073
2021-09-13 23:31:46roundup-botsetmessages: + msg70072
2021-09-13 23:31:39roundup-botsetmessages: + msg70071
2021-09-13 23:31:36roundup-botsetmessages: + msg70070
2021-09-13 23:31:33roundup-botsetmessages: + msg70069
2021-09-13 23:31:28roundup-botsetmessages: + msg70068
2021-09-13 23:31:22roundup-botsetmessages: + msg70067

Showing 10 items. Show all history (warning: this could be VERY long)