Tryton - Issues

 

Issue9324

Title Check against misconfiguration of price_digits
Priority feature Status resolved
Superseder Nosy List albertca, ced, reviewbot, roundup-bot
Type behavior Components product, trytond
Assigned To ced Keywords review
Reviews 317381002
View: 317381002

Created on 2020-05-12.12:52:04 by ced, last changed by roundup-bot.

Messages
New changeset 933a75960148 by Cédric Krier in branch 'default':
Check price_decimal configuration
https://hg.tryton.org/tryton-env/rev/933a75960148
New changeset 7183035066d4 by Cédric Krier in branch 'default':
Check price_decimal configuration
https://hg.tryton.org/trytond/rev/7183035066d4
New changeset 3d7294bedb52 by Cédric Krier in branch 'default':
Check price_decimal configuration
https://hg.tryton.org/modules/product/rev/3d7294bedb52
msg57978 (view) Author: [hidden] (albertca) (Tryton committer) Date: 2020-05-12.18:51:01
Missatge de Cédric Krier <bugs@tryton.org> del dia dt., 12 de maig 2020 a
les 17:09:

>
> Cédric Krier <cedric.krier@b2ck.com> added the comment:
>
> On 2020-05-12 16:59, Albert Cervera i Areny wrote:
> > The proposal is an improvement because at least prevents corrupting data
> but at the same time what is the purpose of considering both configuration
> file and ir.configuration settings?
>
> By design the field definition can not depend on a setting in the
> database because the field define the database schema.
> And for performance, we cannot read in the database each time we need
> this value. It would be too expensive and slow. And using a Function
> field would add a lot of code on many models for no benefit (as any way
> the UI user can not modify it).
>

Understood.

>
> ______________________________________
> Tryton issue tracker <bugs@tryton.org>
> <https://bugs.tryton.org/issue9324>
> ______________________________________
>
msg57970 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-05-12.17:09:02
On 2020-05-12 16:59, Albert Cervera i Areny wrote:
> The proposal is an improvement because at least prevents corrupting data but at the same time what is the purpose of considering both configuration file and ir.configuration settings?

By design the field definition can not depend on a setting in the
database because the field define the database schema.
And for performance, we cannot read in the database each time we need
this value. It would be too expensive and slow. And using a Function
field would add a lot of code on many models for no benefit (as any way
the UI user can not modify it).
msg57969 (view) Author: [hidden] (albertca) (Tryton committer) Date: 2020-05-12.16:59:16
The proposal is an improvement because at least prevents corrupting data but at the same time what is the purpose of considering both configuration file and ir.configuration settings?

We already know that the right setting is the one in ir.configuration so why should we care about what's in the configuration file? Specially if there's no setting in the configuration file. We could emit a warning in the log if we really care about that.

But for me this and the language, is a setting that the user should supply the very first time it creates the database, and later we just use what's in ir.configuration.
review317381002 updated at https://codereview.tryton.org/317381002/#ps291711002
msg57956 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-05-12.12:52:04
Following https://discuss.tryton.org/t/store-unit-price-digits-config-setting-in-the-database/2218

It may be a damaging issue to change the price_digits so we need to ensure to not start working with the wrong value. For that we store the initial value in ir.configuration and check on pool initialization.
History
Date User Action Args
2020-05-28 18:23:13roundup-botsetmessages: + msg58353
2020-05-28 18:23:09roundup-botsetmessages: + msg58352
2020-05-28 18:23:05roundup-botsetstatus: testing -> resolved
nosy: + roundup-bot
messages: + msg58351
2020-05-18 00:36:39albertcasetstatus: resolved -> testing
2020-05-18 00:36:03albertcasetstatus: testing -> resolved
2020-05-12 18:51:01albertcasetmessages: + msg57978
2020-05-12 17:09:03cedsetmessages: + msg57970
2020-05-12 16:59:17albertcasetnosy: + albertca
messages: + msg57969
2020-05-12 13:09:32reviewbotsetnosy: + reviewbot
messages: + msg57957
2020-05-12 12:54:59cedsetstatus: in-progress -> testing
reviews: 317381002
keyword: + review
2020-05-12 12:52:04cedcreate