Issue 10303

Title
Validate no longer raises warnings
Priority
bug
Status
resolved
Nosy list
albertca, ced, nicoe, resteve, reviewbot, roundup-bot
Assigned to
ced
Keywords
review

Created on 2021-04-16.17:46:30 by albertca, last changed 2 weeks ago by roundup-bot.

Messages

New changeset 7ec765def3e7 by Cédric Krier in branch 'default':
Do not use root user to validate records
https://hg.tryton.org/tryton-env/rev/7ec765def3e7
New changeset 72a4b68b1771 by Cédric Krier in branch 'default':
Do not use root user to validate records
https://hg.tryton.org/trytond/rev/72a4b68b1771
Author: [hidden] (albertca) Tryton committer
Date: 2021-04-19.09:26:59
To be honest, if I had to choose between "being able to raise warnings" or
"skip rules" I'd vote for the latter.

The reason is that I think that validate() should always have the same
behaviour, indistinctive of the rules the system's administrator put in
place.

Missatge de Cédric Krier <bugs@tryton.org> del dia dg., 18 d’abr. 2021 a
les 21:14:

>
> Cédric Krier <cedric.krier@b2ck.com> added the comment:
>
> I agree with @nicoe that raising warning in validate is weird. This is
> because once ignored such warning will still be raised each time a user
> modify the record. So warnings were designed to be used on transition
> workflow, button action or wizard.
> Anyway the `@without_rule` can be now removed as issue4080 has been
> implemented.
> I do not think we must change the behavior of older version as it is not a
> recommended practice and implementers could still restore the original user
> for their warning.
>
> ----------
> component: +trytond
> nosy: +ced
>
> ______________________________________
> Tryton issue tracker <bugs@tryton.org>
> <https://bugs.tryton.org/issue10303>
> ______________________________________
>

-- 

<https://www.nan-tic.com>

Albert Cervera

Tel. (+34) 935 531 803

<https://www.nan-tic.com/en/news/>  <http://twitter.com/nan_tic>
<https://www.linkedin.com/company/nan-tic/>

Pot consultar la informació sobre el tractament que realitzem de les dades
de caràcter personal de conformitat amb l’article 11 LOPD aquí
<https://www.nan-tic.com/lopd>.
Author: [hidden] (ced) Tryton committer Tryton translator
Date: 2021-04-18.21:14:32

I agree with @nicoe that raising warning in validate is weird. This is because once ignored such warning will still be raised each time a user modify the record. So warnings were designed to be used on transition workflow, button action or wizard.
Anyway the @without_rule can be now removed as issue4080 has been implemented.
I do not think we must change the behavior of older version as it is not a recommended practice and implementers could still restore the original user for their warning.

Author: [hidden] (albertca) Tryton committer
Date: 2021-04-17.12:39:16

The advantage of validate is that it works for both write() and create() so it is quite convenient.

Also it seems that we're not the only ones doing it:

https://discuss.tryton.org/t/issue-with-user-warning-on-validate/3554/4

Author: [hidden] (nicoe) Tryton committer
Date: 2021-04-17.12:18:32

Should validate raises warnings ?
According to me either the record is valid or it isn't. If you want to raise a warning you should probably do it when saving the record

Author: [hidden] (albertca) Tryton committer
Date: 2021-04-16.17:46:30

I marked this as bug because I think this was an unexpected consequence of issue9796.

Since then, _validate() is decorated with without_rule():

https://hg.tryton.org/trytond/file/tip/trytond/model/modelstorage.py#l1060

which uses set_user(0), which in turn disables warnings:

https://hg.tryton.org/trytond/file/tip/trytond/res/user.py#l1019

This means that raising a UserWarning in validate() does not have the expected behaviour which is showing the message to the user.

History
Date User Action Args
2021-04-23 22:59:44roundup-botsetmessages: + msg66891
2021-04-23 22:59:40roundup-botsetmessages: + msg66890
nosy: + roundup-bot
status: testing -> resolved
2021-04-19 09:26:59albertcasetmessages: + msg66718
2021-04-19 00:48:38reviewbotsetmessages: + msg66717
nosy: + reviewbot
2021-04-19 00:34:03cedsetassignedto: ced
keyword: + review
reviews: 357871002
status: chatting -> testing
2021-04-18 21:14:32cedsetcomponent: + trytond
messages: + msg66715
nosy: + ced
2021-04-17 12:39:16albertcasetmessages: + msg66689
2021-04-17 12:18:32nicoesetmessages: + msg66688
nosy: + nicoe
status: unread -> chatting
2021-04-16 17:54:34restevesetnosy: + resteve
2021-04-16 17:46:30albertcacreate

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