Tryton Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(62)

Issue 255351002: tryton-env: Improve domain validation error from server

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month ago by ced
Modified:
3 days, 10 hours ago
Reviewers:
dave, reviewbot, pokoli
Visibility:
Public.

Description

tryton-env: Improve domain validation error from server We add to DomainValidationError the invalid domain and the field definition so the client can format and append it to the description of the error. issue8288

Patch Set 1 #

Total comments: 2

Patch Set 2 : Better wording #

Total comments: 2

Patch Set 3 : Typo and wording #

Patch Set 4 : Update to tip #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+936 lines, -458 lines) Patch
M sao/src/rpc.js View 1 chunk +9 lines, -0 lines 0 comments Download
M tryton/tryton/common/common.py View 2 chunks +7 lines, -1 line 2 comments Download
M tryton/tryton/common/domain_inversion.py View 1 chunk +0 lines, -410 lines 0 comments Download
M trytond/CHANGELOG View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M trytond/doc/ref/exceptions.rst View 1 2 1 chunk +4 lines, -1 line 0 comments Download
M trytond/trytond/exceptions.py View 1 chunk +3 lines, -2 lines 0 comments Download
M trytond/trytond/model/modelstorage.py View 1 2 3 8 chunks +41 lines, -40 lines 0 comments Download
M trytond/trytond/tests/test_modelstorage.py View 3 chunks +9 lines, -3 lines 2 comments Download
M trytond/trytond/tests/test_tools.py View 3 chunks +450 lines, -1 line 0 comments Download
A trytond/trytond/tools/domain_inversion.py View 1 chunk +410 lines, -0 lines 0 comments Download

Messages

Total messages: 13
ced
1 month ago (2019-04-17 17:17:19 UTC) #1
reviewbot
https://codereview.tryton.org/255351002/diff/277261002/trytond/trytond/tests/test_tools.py#newcode36 trytond/trytond/tests/test_tools.py:36: E501 line too long (90 > 79 characters) https://codereview.tryton.org/255351002/diff/277261002/trytond/trytond/model/modelstorage.py#newcode291 trytond/trytond/model/modelstorage.py:291: E501 line too ...
1 month ago (2019-04-17 17:36:35 UTC) #2
dave
https://codereview.tryton.org/255351002/diff/277261002/trytond/doc/ref/exceptions.rst File trytond/doc/ref/exceptions.rst (right): https://codereview.tryton.org/255351002/diff/277261002/trytond/doc/ref/exceptions.rst#newcode17 trytond/doc/ref/exceptions.rst:17: fields description to format and append to the description. ...
1 month ago (2019-04-19 12:20:34 UTC) #3
ced
https://codereview.tryton.org/255351002/diff/277261002/trytond/doc/ref/exceptions.rst File trytond/doc/ref/exceptions.rst (right): https://codereview.tryton.org/255351002/diff/277261002/trytond/doc/ref/exceptions.rst#newcode17 trytond/doc/ref/exceptions.rst:17: fields description to format and append to the description. ...
1 month ago (2019-04-19 12:38:10 UTC) #4
ced
1 month ago (2019-04-19 12:40:21 UTC) #5
reviewbot
https://codereview.tryton.org/255351002/diff/263381002/trytond/trytond/tests/test_tools.py#newcode36 trytond/trytond/tests/test_tools.py:36: E501 line too long (90 > 79 characters) https://codereview.tryton.org/255351002/diff/263381002/trytond/trytond/model/modelstorage.py#newcode291 trytond/trytond/model/modelstorage.py:291: E501 line too ...
1 month ago (2019-04-19 12:50:13 UTC) #6
dave
https://codereview.tryton.org/255351002/diff/263381002/trytond/doc/ref/exceptions.rst File trytond/doc/ref/exceptions.rst (right): https://codereview.tryton.org/255351002/diff/263381002/trytond/doc/ref/exceptions.rst#newcode17 trytond/doc/ref/exceptions.rst:17: a dictionnary of field definitions to format the domain ...
1 month ago (2019-04-19 13:32:35 UTC) #7
ced
1 month ago (2019-04-20 08:06:41 UTC) #8
reviewbot
https://codereview.tryton.org/255351002/diff/279301002/trytond/trytond/tests/test_tools.py#newcode36 trytond/trytond/tests/test_tools.py:36: E501 line too long (90 > 79 characters) https://codereview.tryton.org/255351002/diff/279301002/trytond/trytond/model/modelstorage.py#newcode291 trytond/trytond/model/modelstorage.py:291: E501 line too ...
1 month ago (2019-04-20 08:14:29 UTC) #9
ced
2 weeks, 2 days ago (2019-05-07 13:45:05 UTC) #10
reviewbot
https://codereview.tryton.org/255351002/diff/269501002/trytond/trytond/tests/test_tools.py#newcode36 trytond/trytond/tests/test_tools.py:36: E501 line too long (90 > 79 characters) https://codereview.tryton.org/255351002/diff/269501002/trytond/trytond/model/modelstorage.py#newcode291 trytond/trytond/model/modelstorage.py:291: E501 line too ...
2 weeks, 2 days ago (2019-05-07 14:12:17 UTC) #11
pokoli
https://codereview.tryton.org/255351002/diff/269501002/tryton/tryton/common/common.py File tryton/tryton/common/common.py (right): https://codereview.tryton.org/255351002/diff/269501002/tryton/tryton/common/common.py#newcode869 tryton/tryton/common/common.py:869: if domain_parser.stringable(domain): Does it make sense to include the ...
6 days ago (2019-05-17 20:21:18 UTC) #12
ced
3 days, 10 hours ago (2019-05-20 10:10:26 UTC) #13
https://codereview.tryton.org/255351002/diff/269501002/tryton/tryton/common/c...
File tryton/tryton/common/common.py (right):

https://codereview.tryton.org/255351002/diff/269501002/tryton/tryton/common/c...
tryton/tryton/common/common.py:869: if domain_parser.stringable(domain):
On 2019/05/17 20:21:18, pokoli wrote:
> Does it make sense to include the domain when it's not stringable? 

I do not think we should show raw domain to end user.
It is better to improve the stringable landscape.

https://codereview.tryton.org/255351002/diff/269501002/trytond/trytond/tests/...
File trytond/trytond/tests/test_modelstorage.py (right):

https://codereview.tryton.org/255351002/diff/269501002/trytond/trytond/tests/...
trytond/trytond/tests/test_modelstorage.py:180:
self.assertTrue(cm.exception.domain[1]['value'])
On 2019/05/17 20:21:18, pokoli wrote:
> why not using assertListEqual to ensure the fields list matches the expected
> value?

Because there is no constraint on being a list, it is just a indexed container.
Also we may add more parameters in the future.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 0147766