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

Issue 35291002: trytond: Add ir.message

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months, 3 weeks ago by xcodinas
Modified:
1 week, 4 days ago
Reviewers:
pokoli, ced, reviewbot
Visibility:
Public.

Description

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: 15
Unified diffs Side-by-side diffs Delta from patch set Stats (+213 lines, -58 lines) Patch
M CHANGELOG View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M trytond/error.py View 1 2 3 4 5 6 7 8 4 chunks +8 lines, -0 lines 1 comment 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 chunks +23 lines, -20 lines 1 comment Download
M trytond/ir/lang.xml View 1 2 3 4 5 1 chunk +12 lines, -0 lines 1 comment Download
A trytond/ir/message.py View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +46 lines, -0 lines 8 comments Download
A trytond/ir/message.xml View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +25 lines, -0 lines 2 comments Download
M trytond/ir/sequence.py View 1 2 3 4 5 6 7 8 6 chunks +16 lines, -16 lines 0 comments Download
M trytond/ir/sequence.xml View 1 2 3 4 5 6 7 8 1 chunk +12 lines, -0 lines 0 comments Download
M trytond/ir/translation.py View 1 2 3 4 5 6 7 8 9 10 11 12 7 chunks +20 lines, -9 lines 1 comment Download
M trytond/ir/translation.xml View 1 2 3 4 5 6 7 8 9 10 11 12 5 chunks +8 lines, -5 lines 1 comment Download
M trytond/ir/trigger.py View 1 2 3 4 5 6 7 8 3 chunks +7 lines, -7 lines 0 comments Download
M trytond/ir/trigger.xml View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments 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 4 5 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/model/model.py View 1 2 3 4 1 chunk +0 lines, -1 line 0 comments Download
M trytond/res/ir.xml View 1 2 3 4 5 1 chunk +16 lines, -0 lines 0 comments Download

Messages

Total messages: 37
xcodinas
8 months, 3 weeks 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: ...
8 months, 3 weeks 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 ...
8 months, 3 weeks 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 ...
8 months, 3 weeks ago (2017-03-08 13:13:15 UTC) #4
pokoli
_sql_contraints should be also taken in account.
8 months, 3 weeks ago (2017-03-08 13:18:00 UTC) #5
xcodinas
Fix comments
8 months, 2 weeks ago (2017-03-09 10:50:36 UTC) #6
xcodinas
Add form view
8 months, 2 weeks 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: ...
8 months, 2 weeks ago (2017-03-09 11:01:21 UTC) #8
xcodinas
Set errors on register
8 months, 2 weeks 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 ...
8 months, 2 weeks 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 ...
8 months, 2 weeks 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 ...
3 months, 3 weeks ago (2017-08-03 20:14:13 UTC) #12
xcodinas
Add cache, raise UserError, deprecate raise_user_error
3 months, 1 week 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 ...
3 months, 1 week 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 ...
3 months, 1 week ago (2017-08-17 10:14:39 UTC) #15
xcodinas
Fix pokoli comments
3 months, 1 week 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 ...
3 months, 1 week ago (2017-08-17 11:03:10 UTC) #17
xcodinas
Add changelog entry
3 months, 1 week 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 ...
3 months, 1 week ago (2017-08-17 11:09:38 UTC) #19
xcodinas
Change get_error to get_message
3 months, 1 week ago (2017-08-17 14:46:56 UTC) #20
xcodinas
Change get_error to get_message
3 months, 1 week 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 ...
3 months, 1 week 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 ...
3 months, 1 week ago (2017-08-17 15:21:58 UTC) #23
xcodinas
Fix pokoli comments
3 months 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 ...
3 months ago (2017-08-21 09:01:37 UTC) #25
xcodinas
Clear message cache when translation
3 months 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 ...
3 months ago (2017-08-21 15:36:12 UTC) #27
xcodinas
Fix records returns and correctly def classmethod
3 months 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 ...
3 months ago (2017-08-22 15:32:35 UTC) #29
xcodinas
Remove write and delete return
3 months 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: ...
3 months 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 ...
3 months 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: ...
3 months ago (2017-08-22 15:43:24 UTC) #33
xcodinas
Fix comments
3 months 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 ...
3 months 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
2 weeks, 1 day ago (2017-11-09 15:50:09 UTC) #36
ced
1 week, 4 days ago (2017-11-13 11:07:09 UTC) #37
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 code before docstring.

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: })
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')

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

https://codereview.tryton.org/35291002/diff/220001/trytond/ir/lang.xml#newcod...
trytond/ir/lang.xml:176: <record model="ir.message" id="invalid_grouping">
I think we should have message.xml for each module.
There is no reason any more to have them on model basis.

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#newc...
trytond/ir/message.py:11: class Message(ModelSQL, ModelView):
I'm wondering if Message is the right name.
Probably Text is better (like gettext).

https://codereview.tryton.org/35291002/diff/220001/trytond/ir/message.py#newc...
trytond/ir/message.py:16: message = fields.Char('Message', translate=True)
We may need multi-line so Text is probably better.

https://codereview.tryton.org/35291002/diff/220001/trytond/ir/message.py#newc...
trytond/ir/message.py:16: message = fields.Char('Message', translate=True)
It should be required to ensure to always return a string.

https://codereview.tryton.org/35291002/diff/220001/trytond/ir/message.py#newc...
trytond/ir/message.py:19: def get_message(cls, module, message):
I think we should use the syntax: "module.id" it will be easier to search for
usage when a message will be replaced and it is already the usage for wizard
views.

https://codereview.tryton.org/35291002/diff/220001/trytond/ir/message.py#newc...
trytond/ir/message.py:19: def get_message(cls, module, message):
I'm wondering if we should not allow to pass a language code as optional
parameter.

https://codereview.tryton.org/35291002/diff/220001/trytond/ir/message.py#newc...
trytond/ir/message.py:22: key = (module, message)
The language should also be part of the key.

https://codereview.tryton.org/35291002/diff/220001/trytond/ir/message.py#newc...
trytond/ir/message.py:35: cls._message_cache.clear()
I do not think it is needed as new message can not be already cached.

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

https://codereview.tryton.org/35291002/diff/220001/trytond/ir/message.xml#new...
trytond/ir/message.xml:19: </record>
missing views

https://codereview.tryton.org/35291002/diff/220001/trytond/ir/message.xml#new...
trytond/ir/message.xml:23: 
too much empty lines

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

https://codereview.tryton.org/35291002/diff/220001/trytond/ir/translation.py#...
trytond/ir/translation.py:976: if any(m.model == 'ir.message' for m in
translations):
I do not think it worth the cost. Let just clear the message cache.

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

https://codereview.tryton.org/35291002/diff/220001/trytond/ir/translation.xml...
trytond/ir/translation.xml:48: action="act_translation_form"
id="menu_translation_form" sequence="10"/>
Why?
Sign in to reply to this message.

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