Tryton Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(445)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 14901002: trytond: Allow to browse history just before a given datetime

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years ago by jean.cavallo
Modified:
4 months, 3 weeks ago
Reviewers:
pokoli, ced, reviewbot
Visibility:
Public.

Description

issue4498 COLLABORATOR=nicolas.evrard@b2ck.com

Patch Set 1 #

Total comments: 5

Patch Set 2 : Only load context once #

Patch Set 3 : Update to trunk #

Total comments: 2

Patch Set 4 : Update to trunk, rename _datetime_exclude to _before_datetime #

Total comments: 4

Patch Set 5 : Rename to datetime_included #

Total comments: 2

Patch Set 6 : Update to tip #

Total comments: 4

Patch Set 7 : Use context #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -11 lines) Patch
M trytond/model/modelsql.py View 1 2 3 4 5 6 7 chunks +24 lines, -9 lines 2 comments Download
M trytond/rpc.py View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M trytond/tests/test_history.py View 1 2 3 4 5 6 1 chunk +8 lines, -0 lines 1 comment Download
M trytond/transaction.py View 1 2 3 4 5 1 chunk +3 lines, -1 line 0 comments Download

Messages

Total messages: 19
reviewbot
http://codereview.tryton.org/14901002/diff/1/trytond/model/modelsql.py File trytond/model/modelsql.py (right): http://codereview.tryton.org/14901002/diff/1/trytond/model/modelsql.py#newcode221 trytond/model/modelsql.py:221: or (values[field_name] in create_records)) E127 continuation line over-indented for ...
8 years ago (2015-01-20 09:31:32 UTC) #1
jean.cavallo
8 years ago (2015-01-27 16:44:32 UTC) #2
ced
http://codereview.tryton.org/14901002/diff/1/trytond/model/modelsql.py File trytond/model/modelsql.py (right): http://codereview.tryton.org/14901002/diff/1/trytond/model/modelsql.py#newcode567 trytond/model/modelsql.py:567: history_clause = (column <= Transaction().context['_datetime']) Fetch only once the ...
8 years ago (2015-01-29 10:47:41 UTC) #3
reviewbot
patch is not applicable
8 years ago (2015-02-03 13:20:24 UTC) #4
reviewbot
http://codereview.tryton.org/14901002/diff/40001/trytond/model/modelsql.py File trytond/model/modelsql.py (right): http://codereview.tryton.org/14901002/diff/40001/trytond/model/modelsql.py#newcode221 trytond/model/modelsql.py:221: or (values[field_name] in create_records)) E127 continuation line over-indented for ...
8 years ago (2015-02-03 13:41:15 UTC) #5
reviewbot
https://codereview.tryton.org/14901002/diff/60001/trytond/model/modelsql.py#newcode294 trytond/model/modelsql.py:294: E127 continuation line over-indented for visual indent URL: https://codereview.tryton.org/14901002
7 years ago (2016-01-29 11:31:54 UTC) #6
pokoli
https://tryton-rietveld.appspot.com/14901002/diff/60001/trytond/model/modelsql.py File trytond/model/modelsql.py (right): https://tryton-rietveld.appspot.com/14901002/diff/60001/trytond/model/modelsql.py#newcode646 trytond/model/modelsql.py:646: if context.get('_before_datetime', False): I see this code repeated over ...
7 years ago (2016-01-29 11:42:38 UTC) #7
ced
https://tryton-rietveld.appspot.com/14901002/diff/60001/trytond/rpc.py File trytond/rpc.py (right): https://tryton-rietveld.appspot.com/14901002/diff/60001/trytond/rpc.py#newcode40 trytond/rpc.py:40: '_before_datetime'): not very readable indented like that. And now ...
7 years ago (2016-01-29 11:55:40 UTC) #8
jean.cavallo
I renamed _datetime_exclude to _datetime_included (which "invert" the logic) https://tryton-rietveld.appspot.com/14901002/diff/60001/trytond/model/modelsql.py File trytond/model/modelsql.py (right): https://tryton-rietveld.appspot.com/14901002/diff/60001/trytond/model/modelsql.py#newcode646 trytond/model/modelsql.py:646: ...
7 years ago (2016-01-29 16:39:55 UTC) #9
reviewbot
https://codereview.tryton.org/14901002/diff/80001/trytond/model/modelsql.py#newcode301 trytond/model/modelsql.py:301: E127 continuation line over-indented for visual indent URL: https://codereview.tryton.org/14901002
7 years ago (2016-01-29 16:46:22 UTC) #10
ced
Missing changelog and doc https://tryton-rietveld.appspot.com/14901002/diff/80001/trytond/model/modelsql.py File trytond/model/modelsql.py (right): https://tryton-rietveld.appspot.com/14901002/diff/80001/trytond/model/modelsql.py#newcode265 trytond/model/modelsql.py:265: def _get_history_operator(cls): I think it ...
7 years ago (2016-01-29 17:13:17 UTC) #11
nicoe
6 months, 3 weeks ago (2022-07-13 15:43:35 UTC) #12
nicoe
https://codereview.tryton.org/14901002/diff/80001/trytond/model/modelsql.py File trytond/model/modelsql.py (right): https://codereview.tryton.org/14901002/diff/80001/trytond/model/modelsql.py#newcode265 trytond/model/modelsql.py:265: def _get_history_operator(cls): On 2016/01/29 17:13:17, ced wrote: > I ...
6 months, 3 weeks ago (2022-07-13 15:43:51 UTC) #13
pokoli
https://codereview.tryton.org/14901002/diff/423471003/trytond/model/modelsql.py File trytond/model/modelsql.py (right): https://codereview.tryton.org/14901002/diff/423471003/trytond/model/modelsql.py#newcode779 trytond/model/modelsql.py:779: if Transaction().context.get('_datetime_included', True): please use context instead of Transaction().context ...
6 months, 3 weeks ago (2022-07-13 15:46:23 UTC) #14
reviewbot
https://codereview.tryton.org/14901002/diff/423471003/trytond/rpc.py#newcode61 trytond/rpc.py:61: line break after binary operator URL: https://codereview.tryton.org/14901002
6 months, 3 weeks ago (2022-07-13 15:53:29 UTC) #15
nicoe
https://codereview.tryton.org/14901002/diff/423471003/trytond/model/modelsql.py File trytond/model/modelsql.py (right): https://codereview.tryton.org/14901002/diff/423471003/trytond/model/modelsql.py#newcode779 trytond/model/modelsql.py:779: if Transaction().context.get('_datetime_included', True): On 2022/07/13 15:46:23, pokoli wrote: > ...
4 months, 3 weeks ago (2022-09-09 23:15:51 UTC) #16
nicoe
4 months, 3 weeks ago (2022-09-09 23:16:10 UTC) #17
reviewbot
https://codereview.tryton.org/14901002/diff/411681003/trytond/rpc.py#newcode61 trytond/rpc.py:61: line break after binary operator URL: https://codereview.tryton.org/14901002
4 months, 3 weeks ago (2022-09-09 23:36:19 UTC) #18
ced
4 months, 3 weeks ago (2022-09-10 07:30:03 UTC) #19 
I think we are missing documentation for this new context keyword but also for
_datetime keyword.

https://codereview.tryton.org/14901002/diff/411681003/trytond/model/modelsql.py
File trytond/model/modelsql.py (right):

https://codereview.tryton.org/14901002/diff/411681003/trytond/model/modelsql....
trytond/model/modelsql.py:781: if context.get('_datetime_included', True):
Will it not be more logical to use _datetime_excluded?

https://codereview.tryton.org/14901002/diff/411681003/trytond/model/modelsql....
trytond/model/modelsql.py:785: history_clause = history_operator(column,
context['_datetime'])
I think it will be better to make a function that construct the clause. This
will ensure to use the same way for each usage.

https://codereview.tryton.org/14901002/diff/411681003/trytond/tests/test_hist...
File trytond/tests/test_history.py (right):

https://codereview.tryton.org/14901002/diff/411681003/trytond/tests/test_hist...
trytond/tests/test_history.py:445: print(first_stamp, second_stamp)
must be removed
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld d9ca037-tainted