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

Issue 35291002: trytond: Add ir.message

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 7 months ago by xcodinas
Modified:
4 months, 1 week ago
Reviewers:
pokoli, reviewbot, nicoe, ced
Visibility:
Public.

Description

issue3672 COLLABORATOR=sergi@koolpi.com

Patch Set 1 #

Total comments: 5

Patch Set 2 : Fix comments #

Patch Set 3 : Add form view #

Patch Set 4 : Set errors on register #

Total comments: 10

Patch Set 5 : Add cache, raise UserError, deprecate raise_user_error #

Total comments: 18

Patch Set 6 : Fix pokoli comments #

Patch Set 7 : Add changelog entry #

Patch Set 8 : Change get_error to get_message #

Total comments: 3

Patch Set 9 : Fix pokoli comments #

Patch Set 10 : Clear message cache when translation #

Total comments: 6

Patch Set 11 : Fix records returns and correctly def classmethod #

Patch Set 12 : Remove write and delete return #

Patch Set 13 : Fix comments #

Total comments: 22

Patch Set 14 : Fix comments, update to tip and add unit test #

Patch Set 15 : Add deprecation notice on docs #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+282 lines, -50 lines) Patch
M CHANGELOG View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -0 lines 0 comments Download
M doc/ref/models/models.rst View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 3 chunks +12 lines, -0 lines 0 comments Download
M trytond/error.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 3 chunks +7 lines, -0 lines 0 comments Download
A trytond/i18n.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +14 lines, -0 lines 0 comments Download
M trytond/ir/__init__.py View 1 2 3 4 2 chunks +2 lines, -0 lines 0 comments Download
M trytond/ir/lang.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 7 chunks +11 lines, -18 lines 1 comment Download
A trytond/ir/message.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +44 lines, -0 lines 0 comments Download
A trytond/ir/message.xml View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +38 lines, -0 lines 1 comment Download
M trytond/ir/sequence.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 4 chunks +4 lines, -12 lines 0 comments Download
M trytond/ir/translation.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 5 chunks +14 lines, -8 lines 0 comments Download
M trytond/ir/translation.xml View 1 2 3 4 5 6 7 8 9 10 11 12 13 5 chunks +31 lines, -4 lines 1 comment Download
M trytond/ir/trigger.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 3 chunks +5 lines, -7 lines 0 comments Download
M trytond/ir/trigger.xml View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +0 lines, -1 line 1 comment Download
M trytond/ir/tryton.cfg View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
A trytond/ir/view/message_form.xml View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +7 lines, -0 lines 0 comments Download
A trytond/ir/view/message_list.xml View 1 2 3 4 5 1 chunk +6 lines, -0 lines 0 comments Download
M trytond/res/ir.xml View 1 2 3 4 5 1 chunk +16 lines, -0 lines 0 comments Download
A trytond/tests/message.xml View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +10 lines, -0 lines 1 comment Download
A trytond/tests/test_i18n.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +58 lines, -0 lines 1 comment Download
M trytond/tests/tryton.cfg View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 44
xcodinas
1 year, 7 months ago (2017-03-08 12:39:45 UTC) #1
reviewbot
https://codereview.tryton.org/35291002/diff/1/trytond/ir/__init__.py#newcode4 trytond/ir/__init__.py:4: F403 'from configuration import *' used; unable to detect undefined names https://codereview.tryton.org/35291002/diff/1/trytond/ir/__init__.py#newcode5 trytond/ir/__init__.py:5: ...
1 year, 7 months ago (2017-03-08 12:59:44 UTC) #2
pokoli
https://tryton-rietveld-hrd.appspot.com/35291002/diff/1/trytond/error.py File trytond/error.py (right): https://tryton-rietveld-hrd.appspot.com/35291002/diff/1/trytond/error.py#newcode34 trytond/error.py:34: Translation = Pool().get('ir.translation') i think translation is no more ...
1 year, 7 months ago (2017-03-08 13:01:50 UTC) #3
pokoli
trytond/ir/translation.py should also be updated to remove the no more required methods docs should also ...
1 year, 7 months ago (2017-03-08 13:13:15 UTC) #4
pokoli
_sql_contraints should be also taken in account.
1 year, 7 months ago (2017-03-08 13:18:00 UTC) #5
xcodinas
Fix comments
1 year, 7 months ago (2017-03-09 10:50:36 UTC) #6
xcodinas
Add form view
1 year, 7 months ago (2017-03-09 10:51:54 UTC) #7
reviewbot
https://codereview.tryton.org/35291002/diff/20002/trytond/ir/__init__.py#newcode4 trytond/ir/__init__.py:4: F403 'from configuration import *' used; unable to detect undefined names https://codereview.tryton.org/35291002/diff/20002/trytond/ir/__init__.py#newcode5 trytond/ir/__init__.py:5: ...
1 year, 7 months ago (2017-03-09 11:01:21 UTC) #8
xcodinas
Set errors on register
1 year, 7 months ago (2017-03-14 10:20:37 UTC) #9
pokoli
https://tryton-rietveld.appspot.com/35291002/diff/40001/trytond/error.py File trytond/error.py (left): https://tryton-rietveld.appspot.com/35291002/diff/40001/trytond/error.py#oldcode39 trytond/error.py:39: res = Translation.get_source(cls.__name__, 'error', language, error) get_source has a ...
1 year, 7 months ago (2017-03-14 10:29:02 UTC) #10
reviewbot
https://codereview.tryton.org/35291002/diff/40001/trytond/model/model.py#newcode184 trytond/model/model.py:184: E127 continuation line over-indented for visual indent https://codereview.tryton.org/35291002/diff/40001/trytond/model/model.py#newcode245 trytond/model/model.py:245: E127 continuation line over-indented ...
1 year, 7 months ago (2017-03-14 10:35:08 UTC) #11
ced
https://tryton-rietveld.appspot.com/35291002/diff/40001/trytond/error.py File trytond/error.py (right): https://tryton-rietveld.appspot.com/35291002/diff/40001/trytond/error.py#newcode14 trytond/error.py:14: def raise_user_error(cls, error, error_args=None, I think this should become ...
1 year, 2 months ago (2017-08-03 20:14:13 UTC) #12
xcodinas
Add cache, raise UserError, deprecate raise_user_error
1 year, 1 month ago (2017-08-17 09:41:00 UTC) #13
reviewbot
https://codereview.tryton.org/35291002/diff/60001/trytond/model/model.py#newcode179 trytond/model/model.py:179: E127 continuation line over-indented for visual indent https://codereview.tryton.org/35291002/diff/60001/trytond/model/model.py#newcode240 trytond/model/model.py:240: E127 continuation line over-indented ...
1 year, 1 month ago (2017-08-17 10:02:21 UTC) #14
pokoli
Missing CHANGELOG https://tryton-rietveld.appspot.com/35291002/diff/60001/trytond/error.py File trytond/error.py (right): https://tryton-rietveld.appspot.com/35291002/diff/60001/trytond/error.py#newcode12 trytond/error.py:12: _message_cache = Cache('ir.message', size_limit=10240, context=False) Why having ...
1 year, 1 month ago (2017-08-17 10:14:39 UTC) #15
xcodinas
Fix pokoli comments
1 year, 1 month ago (2017-08-17 11:03:02 UTC) #16
xcodinas
https://tryton-rietveld.appspot.com/35291002/diff/60001/trytond/error.py File trytond/error.py (right): https://tryton-rietveld.appspot.com/35291002/diff/60001/trytond/error.py#newcode12 trytond/error.py:12: _message_cache = Cache('ir.message', size_limit=10240, context=False) On 2017/08/17 10:14:39, pokoli ...
1 year, 1 month ago (2017-08-17 11:03:10 UTC) #17
xcodinas
Add changelog entry
1 year, 1 month ago (2017-08-17 11:06:46 UTC) #18
reviewbot
https://codereview.tryton.org/35291002/diff/100001/trytond/model/model.py#newcode179 trytond/model/model.py:179: E127 continuation line over-indented for visual indent https://codereview.tryton.org/35291002/diff/100001/trytond/model/model.py#newcode240 trytond/model/model.py:240: E127 continuation line over-indented ...
1 year, 1 month ago (2017-08-17 11:09:38 UTC) #19
xcodinas
Change get_error to get_message
1 year, 1 month ago (2017-08-17 14:46:56 UTC) #20
xcodinas
Change get_error to get_message
1 year, 1 month ago (2017-08-17 14:55:00 UTC) #21
reviewbot
https://codereview.tryton.org/35291002/diff/130001/trytond/model/model.py#newcode179 trytond/model/model.py:179: E127 continuation line over-indented for visual indent https://codereview.tryton.org/35291002/diff/130001/trytond/model/model.py#newcode240 trytond/model/model.py:240: E127 continuation line over-indented ...
1 year, 1 month ago (2017-08-17 15:05:19 UTC) #22
pokoli
https://tryton-rietveld.appspot.com/35291002/diff/130001/trytond/error.py File trytond/error.py (right): https://tryton-rietveld.appspot.com/35291002/diff/130001/trytond/error.py#newcode5 trytond/error.py:5: from trytond.pool import Pool why changing pool? https://tryton-rietveld.appspot.com/35291002/diff/130001/trytond/ir/message.py File ...
1 year, 1 month ago (2017-08-17 15:21:58 UTC) #23
xcodinas
Fix pokoli comments
1 year, 1 month ago (2017-08-21 08:53:14 UTC) #24
reviewbot
https://codereview.tryton.org/35291002/diff/150001/trytond/model/model.py#newcode179 trytond/model/model.py:179: E127 continuation line over-indented for visual indent https://codereview.tryton.org/35291002/diff/150001/trytond/model/model.py#newcode240 trytond/model/model.py:240: E127 continuation line over-indented ...
1 year, 1 month ago (2017-08-21 09:01:37 UTC) #25
xcodinas
Clear message cache when translation
1 year, 1 month ago (2017-08-21 15:22:34 UTC) #26
reviewbot
https://codereview.tryton.org/35291002/diff/170001/trytond/model/model.py#newcode179 trytond/model/model.py:179: E127 continuation line over-indented for visual indent https://codereview.tryton.org/35291002/diff/170001/trytond/model/model.py#newcode240 trytond/model/model.py:240: E127 continuation line over-indented ...
1 year, 1 month ago (2017-08-21 15:36:12 UTC) #27
xcodinas
Fix records returns and correctly def classmethod
1 year, 1 month ago (2017-08-22 15:30:09 UTC) #28
pokoli
https://tryton-rietveld.appspot.com/35291002/diff/170001/trytond/ir/message.py File trytond/ir/message.py (right): https://tryton-rietveld.appspot.com/35291002/diff/170001/trytond/ir/message.py#newcode24 trytond/ir/message.py:24: message_id = ModelData.get_id(module, message) There is no need to ...
1 year, 1 month ago (2017-08-22 15:32:35 UTC) #29
xcodinas
Remove write and delete return
1 year, 1 month ago (2017-08-22 15:34:56 UTC) #30
xcodinas
https://tryton-rietveld.appspot.com/35291002/diff/170001/trytond/ir/message.py File trytond/ir/message.py (right): https://tryton-rietveld.appspot.com/35291002/diff/170001/trytond/ir/message.py#newcode24 trytond/ir/message.py:24: message_id = ModelData.get_id(module, message) On 2017/08/22 15:32:35, pokoli wrote: ...
1 year, 1 month ago (2017-08-22 15:41:21 UTC) #31
reviewbot
https://codereview.tryton.org/35291002/diff/210001/trytond/model/model.py#newcode179 trytond/model/model.py:179: E127 continuation line over-indented for visual indent https://codereview.tryton.org/35291002/diff/210001/trytond/model/model.py#newcode240 trytond/model/model.py:240: E127 continuation line over-indented ...
1 year, 1 month ago (2017-08-22 15:41:42 UTC) #32
pokoli
https://tryton-rietveld.appspot.com/35291002/diff/170001/trytond/ir/message.py File trytond/ir/message.py (right): https://tryton-rietveld.appspot.com/35291002/diff/170001/trytond/ir/message.py#newcode24 trytond/ir/message.py:24: message_id = ModelData.get_id(module, message) On 2017/08/22 15:41:20, xcodinas wrote: ...
1 year, 1 month ago (2017-08-22 15:43:24 UTC) #33
xcodinas
Fix comments
1 year, 1 month ago (2017-08-23 11:34:56 UTC) #34
reviewbot
https://codereview.tryton.org/35291002/diff/220001/trytond/model/model.py#newcode179 trytond/model/model.py:179: E127 continuation line over-indented for visual indent https://codereview.tryton.org/35291002/diff/220001/trytond/model/model.py#newcode240 trytond/model/model.py:240: E127 continuation line over-indented ...
1 year, 1 month ago (2017-08-23 12:05:59 UTC) #35
pokoli
https://codereview.tryton.org/35291002/diff/220001/trytond/ir/message.py File trytond/ir/message.py (right): https://codereview.tryton.org/35291002/diff/220001/trytond/ir/message.py#newcode20 trytond/ir/message.py:20: no need for empty line
11 months, 1 week ago (2017-11-09 15:50:09 UTC) #36
ced
Missing documentation. https://codereview.tryton.org/35291002/diff/220001/trytond/error.py File trytond/error.py (right): https://codereview.tryton.org/35291002/diff/220001/trytond/error.py#newcode22 trytond/error.py:22: 'Message.get_error instead', DeprecationWarning, stacklevel=2) Do not put ...
11 months ago (2017-11-13 11:07:09 UTC) #37
nicoe
https://codereview.tryton.org/35291002/diff/220001/trytond/ir/lang.py File trytond/ir/lang.py (right): https://codereview.tryton.org/35291002/diff/220001/trytond/ir/lang.py#newcode145 trytond/ir/lang.py:145: }) On 2017/11/13 11:07:08, ced wrote: > I think ...
10 months, 1 week ago (2017-12-09 12:26:50 UTC) #38
ced
https://codereview.tryton.org/35291002/diff/220001/trytond/ir/message.py File trytond/ir/message.py (right): https://codereview.tryton.org/35291002/diff/220001/trytond/ir/message.py#newcode19 trytond/ir/message.py:19: def get_message(cls, module, message): On 2017/12/09 12:26:50, nicoe wrote: ...
10 months, 1 week ago (2017-12-09 13:49:33 UTC) #39
pokoli
Fix comments, update to tip and add unit test
8 months, 1 week ago (2018-02-06 16:21:47 UTC) #40
pokoli
https://codereview.tryton.org/35291002/diff/220001/trytond/ir/lang.py File trytond/ir/lang.py (right): https://codereview.tryton.org/35291002/diff/220001/trytond/ir/lang.py#newcode145 trytond/ir/lang.py:145: }) On 2017/12/09 12:26:50, nicoe wrote: > On 2017/11/13 ...
8 months, 1 week ago (2018-02-06 16:22:13 UTC) #41
pokoli
Add deprecation notice on docs
8 months, 1 week ago (2018-02-06 16:31:04 UTC) #42
reviewbot
https://codereview.tryton.org/35291002/diff/260001/trytond/tests/test_i18n.py#newcode7 trytond/tests/test_i18n.py:7: F401 'Transaction' imported but unused https://codereview.tryton.org/35291002/diff/260001/trytond/ir/__init__.py#newcode4 trytond/ir/__init__.py:4: F403 'from configuration import *' used; ...
8 months, 1 week ago (2018-02-06 16:56:30 UTC) #43
ced
4 months, 1 week ago (2018-06-06 09:07:47 UTC) #44
Missing documentation about i18n
The module res has some error messages.

https://codereview.tryton.org/35291002/diff/220001/trytond/ir/lang.py
File trytond/ir/lang.py (right):

https://codereview.tryton.org/35291002/diff/220001/trytond/ir/lang.py#newcode145
trytond/ir/lang.py:145: })
On 2018/02/06 16:22:13, pokoli wrote:
> On 2017/12/09 12:26:50, nicoe wrote:
> > On 2017/11/13 11:07:08, ced wrote:
> > > I think we should have a tool that easy this usage and abstract from the
> Model
> > > base.
> > > 
> > > I propose:
> > > 
> > > from trytond.i18n import gettext
> > > 
> > > gettext('ir.invalid_grouping')
> > 
> > I think we should only pass to UserError the ID of the error and as second
> > argument the dictionary. The within the UserError class, do the translation
> > stuffs.
> 
> One of the objectives of this review is to be able to translate not only
errors
> but also the string of a dinamically defined selection. 
> > 
> > That way, the error could have an attribute being the ID of the error and
> people
> > could handle the error according to it if they wish to do so. It also reach
> the
> > goal of being easier to use. And also this ID could be used in our test
> because
> > I have always hated the way we test the errors.
> 
> I agree that this will be a good adition, but I think it should be done on a
> separate issue. 

Indeed for me, when needed we should use custom Exception inheriting UserError.

https://codereview.tryton.org/35291002/diff/260001/trytond/ir/lang.py
File trytond/ir/lang.py (right):

https://codereview.tryton.org/35291002/diff/260001/trytond/ir/lang.py#newcode202
trytond/ir/lang.py:202: cls.raise_user_error(gettext('ir.invalid_grouping') % {
Should raise UserError.

https://codereview.tryton.org/35291002/diff/260001/trytond/ir/message.xml
File trytond/ir/message.xml (right):

https://codereview.tryton.org/35291002/diff/260001/trytond/ir/message.xml#new...
trytond/ir/message.xml:7: <record model="ir.message" id="invalid_grouping">
I think id should be prefixed by 'message_'

https://codereview.tryton.org/35291002/diff/260001/trytond/ir/translation.xml
File trytond/ir/translation.xml (right):

https://codereview.tryton.org/35291002/diff/260001/trytond/ir/translation.xml...
trytond/ir/translation.xml:90: action="act_message_tree" id="menu_message_tree"
sequence="10"/>
New guideline style, all in one line or each attribute on a new line.
I prefer long line in this case.

https://codereview.tryton.org/35291002/diff/260001/trytond/ir/trigger.xml
File trytond/ir/trigger.xml (left):

https://codereview.tryton.org/35291002/diff/260001/trytond/ir/trigger.xml#old...
trytond/ir/trigger.xml:32: 
Not for this patch

https://codereview.tryton.org/35291002/diff/260001/trytond/tests/message.xml
File trytond/tests/message.xml (right):

https://codereview.tryton.org/35291002/diff/260001/trytond/tests/message.xml#...
trytond/tests/message.xml:6: <record model="ir.message" id="test_message">
message_test

https://codereview.tryton.org/35291002/diff/260001/trytond/tests/test_i18n.py
File trytond/tests/test_i18n.py (right):

https://codereview.tryton.org/35291002/diff/260001/trytond/tests/test_i18n.py...
trytond/tests/test_i18n.py:7: from trytond.transaction import Transaction
not used.
Sign in to reply to this message.

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