account_statement: Add statment origin and wizard to import them

issue5882

COLLABORATOR=cedric.krier@b2ck.com

[reviewbot, 2016-09-14 15:02:51]

patch is not applicable
URL: https://codereview.tryton.org/30501002

[pokoli, 2016-10-11 11:34:59]

Add statment origin and wizard to import them

[reviewbot, 2016-10-11 11:52:19]

https://codereview.tryton.org/30501002/diff/20001/__init__.py#newcode5
__init__.py:5:
F403 'from .journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/20001/__init__.py#newcode6
__init__.py:6:
F403 'from .statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/20001/__init__.py#newcode7
__init__.py:7:
F403 'from .account import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/20001/__init__.py#newcode13
__init__.py:13:
F999 Journal may be undefined, or defined from star imports: .account, .journal,
.statement

https://codereview.tryton.org/30501002/diff/20001/__init__.py#newcode14
__init__.py:14:
F999 Statement may be undefined, or defined from star imports: .account,
.journal, .statement

https://codereview.tryton.org/30501002/diff/20001/__init__.py#newcode15
__init__.py:15:
F999 Line may be undefined, or defined from star imports: .account, .journal,
.statement

https://codereview.tryton.org/30501002/diff/20001/__init__.py#newcode16
__init__.py:16:
F999 LineGroup may be undefined, or defined from star imports: .account,
.journal, .statement

https://codereview.tryton.org/30501002/diff/20001/__init__.py#newcode17
__init__.py:17:
F999 Move may be undefined, or defined from star imports: .account, .journal,
.statement

https://codereview.tryton.org/30501002/diff/20001/__init__.py#newcode18
__init__.py:18:
F999 Origin may be undefined, or defined from star imports: .account, .journal,
.statement

https://codereview.tryton.org/30501002/diff/20001/__init__.py#newcode19
__init__.py:19:
F999 ImportStatementStart may be undefined, or defined from star imports:
.account, .journal, .statement

https://codereview.tryton.org/30501002/diff/20001/__init__.py#newcode23
__init__.py:23:
F999 ImportStatement may be undefined, or defined from star imports: .account,
.journal, .statement

https://codereview.tryton.org/30501002/diff/20001/__init__.py#newcode26
__init__.py:26:
F999 StatementReport may be undefined, or defined from star imports: .account,
.journal, .statement

URL: https://codereview.tryton.org/30501002

[albert, 2016-10-31 12:33:13]

https://tryton-rietveld.appspot.com/30501002/diff/20001/CHANGELOG
File CHANGELOG (right):

https://tryton-rietveld.appspot.com/30501002/diff/20001/CHANGELOG#newcode2
CHANGELOG:2: * Add statment origins
*statement*

[pokoli, 2016-12-01 16:09:11]

Update to tip and fix comments

[reviewbot, 2016-12-01 16:35:38]

https://codereview.tryton.org/30501002/diff/40001/__init__.py#newcode5
__init__.py:5:
F403 'from .journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/40001/__init__.py#newcode6
__init__.py:6:
F403 'from .statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/40001/__init__.py#newcode7
__init__.py:7:
F403 'from .account import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/40001/__init__.py#newcode13
__init__.py:13:
F999 Journal may be undefined, or defined from star imports: .account, .journal,
.statement

https://codereview.tryton.org/30501002/diff/40001/__init__.py#newcode14
__init__.py:14:
F999 Statement may be undefined, or defined from star imports: .account,
.journal, .statement

https://codereview.tryton.org/30501002/diff/40001/__init__.py#newcode15
__init__.py:15:
F999 Line may be undefined, or defined from star imports: .account, .journal,
.statement

https://codereview.tryton.org/30501002/diff/40001/__init__.py#newcode16
__init__.py:16:
F999 LineGroup may be undefined, or defined from star imports: .account,
.journal, .statement

https://codereview.tryton.org/30501002/diff/40001/__init__.py#newcode17
__init__.py:17:
F999 Move may be undefined, or defined from star imports: .account, .journal,
.statement

https://codereview.tryton.org/30501002/diff/40001/__init__.py#newcode18
__init__.py:18:
F999 Origin may be undefined, or defined from star imports: .account, .journal,
.statement

https://codereview.tryton.org/30501002/diff/40001/__init__.py#newcode19
__init__.py:19:
F999 ImportStatementStart may be undefined, or defined from star imports:
.account, .journal, .statement

https://codereview.tryton.org/30501002/diff/40001/__init__.py#newcode23
__init__.py:23:
F999 ImportStatement may be undefined, or defined from star imports: .account,
.journal, .statement

https://codereview.tryton.org/30501002/diff/40001/__init__.py#newcode26
__init__.py:26:
F999 StatementReport may be undefined, or defined from star imports: .account,
.journal, .statement

URL: https://codereview.tryton.org/30501002

[ced, 2016-12-18 18:18:04]

https://tryton-rietveld.appspot.com/30501002/diff/40001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/40001/statement.py#newcode159
statement.py:159: Bool(Eval('origins', []))),
I would use a default value [-1] like that if origins is not present we do not
allow to import.

https://tryton-rietveld.appspot.com/30501002/diff/40001/statement.py#newcode367
statement.py:367: def validation_lines(self):
I do not think it is needed. We can always validate the lines because the
origins must always be converted as line.

https://tryton-rietveld.appspot.com/30501002/diff/40001/statement.py#newcode536
statement.py:536: cls.raise_user_error('post_with_pending_amount', {
Should also display statement.rec_name

https://tryton-rietveld.appspot.com/30501002/diff/40001/statement.py#newcode555
statement.py:555: def import_origin(cls, statements):
I would not suffix with origin. This should be the only import way.

https://tryton-rietveld.appspot.com/30501002/diff/40001/statement.py#newcode573
statement.py:573: date = fields.Date('Date', required=True, states=_states,
depends=_depends)
I think it should be readonly if there is an origin and if it is filled.

https://tryton-rietveld.appspot.com/30501002/diff/40001/statement.py#newcode580
statement.py:580: 'on_change_with_company')
Should have a searcher to change the ir.rule

https://tryton-rietveld.appspot.com/30501002/diff/40001/statement.py#newcode659
statement.py:659: '_parent_statement.journal', 'statement')
no in this patch.

https://tryton-rietveld.appspot.com/30501002/diff/40001/statement.py#newcode892
statement.py:892: fields.Selection(STATES, 'Statement State'),
It should use a method to retrieve the states values from Statement object
otherwise it is not modular.

https://tryton-rietveld.appspot.com/30501002/diff/40001/statement.py#newcode921
statement.py:921: 'done_with_pending_amount': ('Origin "%(origin)s" can not be '
Do not see where it is used.

https://tryton-rietveld.appspot.com/30501002/diff/40001/statement.py#newcode946
statement.py:946: validate = fields.Boolean('Validate Statement',
I do not think it is useful if the wizard opens the statement as it will not
reduce the number of action from the user.

https://tryton-rietveld.appspot.com/30501002/diff/40001/statement.py#newcode977
statement.py:977: data = getattr(self, 'read_%s' % self.start.file_format)()
I do not find this design very useful.
For me, there is no need for read_* method but instead get_statement should be:


def get_statement(self):
   statement = Statement()
   return statement

def get_origins(self, statement):
   for line in getattr(self, 'get_lines_%s' % self.start.file_format)():
       yield origin


def do_import(self, action):
    statement = self.get_statement()
    statement.origins = list(self.get_origins(statement))
    statement.save()

https://tryton-rietveld.appspot.com/30501002/diff/40001/statement.xml
File statement.xml (right):

https://tryton-rietveld.appspot.com/30501002/diff/40001/statement.xml#newcode349
statement.xml:349: </record>
I think we need to have a menu entry to list all the origin with domain to get
those that are pending.

https://tryton-rietveld.appspot.com/30501002/diff/40001/statement.xml#newcode352
statement.xml:352: <field name="model" search="[('model', '=',
'account.statement.origin')]"/>
80 cols

https://tryton-rietveld.appspot.com/30501002/diff/40001/statement.xml#newcode401
statement.xml:401: <field name="wiz_name">account.statement.import</field>
missing model

https://tryton-rietveld.appspot.com/30501002/diff/40001/tests/scenario_statem...
File tests/scenario_statement_origin.rst (right):

https://tryton-rietveld.appspot.com/30501002/diff/40001/tests/scenario_statem...
tests/scenario_statement_origin.rst:118: Testing amount validation::
I do not think it is needed to test validation as it should be the same without
origin.

https://tryton-rietveld.appspot.com/30501002/diff/40001/tests/scenario_statem...
tests/scenario_statement_origin.rst:229: Statment can not be posted until all
origins are finished::
Statement

https://tryton-rietveld.appspot.com/30501002/diff/40001/view/import_statement...
File view/import_statement_start_view_form.xml (right):

https://tryton-rietveld.appspot.com/30501002/diff/40001/view/import_statement...
view/import_statement_start_view_form.xml:8: <field name="file"/>
I think it is more logical to first fill the file and after select the format.
We could even imagine to have a kind of auto-detection in the future.

https://tryton-rietveld.appspot.com/30501002/diff/40001/view/statement_line_f...
File view/statement_line_form.xml (right):

https://tryton-rietveld.appspot.com/30501002/diff/40001/view/statement_line_f...
view/statement_line_form.xml:6: <field name="statement" colspan="3"/>
I would put the origin here.

[pokoli, 2016-12-19 16:36:41]

Fix ced comments

[reviewbot, 2016-12-19 16:53:03]

https://codereview.tryton.org/30501002/diff/60001/__init__.py#newcode5
__init__.py:5:
F403 'from .journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/60001/__init__.py#newcode6
__init__.py:6:
F403 'from .statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/60001/__init__.py#newcode7
__init__.py:7:
F403 'from .account import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/60001/__init__.py#newcode13
__init__.py:13:
F999 Journal may be undefined, or defined from star imports: .account, .journal,
.statement

https://codereview.tryton.org/30501002/diff/60001/__init__.py#newcode14
__init__.py:14:
F999 Statement may be undefined, or defined from star imports: .account,
.journal, .statement

https://codereview.tryton.org/30501002/diff/60001/__init__.py#newcode15
__init__.py:15:
F999 Line may be undefined, or defined from star imports: .account, .journal,
.statement

https://codereview.tryton.org/30501002/diff/60001/__init__.py#newcode16
__init__.py:16:
F999 LineGroup may be undefined, or defined from star imports: .account,
.journal, .statement

https://codereview.tryton.org/30501002/diff/60001/__init__.py#newcode17
__init__.py:17:
F999 Move may be undefined, or defined from star imports: .account, .journal,
.statement

https://codereview.tryton.org/30501002/diff/60001/__init__.py#newcode18
__init__.py:18:
F999 Origin may be undefined, or defined from star imports: .account, .journal,
.statement

https://codereview.tryton.org/30501002/diff/60001/__init__.py#newcode19
__init__.py:19:
F999 ImportStatementStart may be undefined, or defined from star imports:
.account, .journal, .statement

https://codereview.tryton.org/30501002/diff/60001/__init__.py#newcode23
__init__.py:23:
F999 ImportStatement may be undefined, or defined from star imports: .account,
.journal, .statement

https://codereview.tryton.org/30501002/diff/60001/__init__.py#newcode26
__init__.py:26:
F999 StatementReport may be undefined, or defined from star imports: .account,
.journal, .statement

URL: https://codereview.tryton.org/30501002

[ced, 2016-12-26 17:07:33]

The documentation should also be updated.

https://tryton-rietveld.appspot.com/30501002/diff/60001/CHANGELOG
File CHANGELOG (right):

https://tryton-rietveld.appspot.com/30501002/diff/60001/CHANGELOG#newcode1
CHANGELOG:1: * Add wizard to import origins from files
"import origins" does not seem to meaningful.
I think it could be just:
Add generic wizard to import statement from files

https://tryton-rietveld.appspot.com/30501002/diff/60001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/60001/statement.py#newcode114
statement.py:114: 'readonly': (Eval('state') != 'draft') | ~Eval('journal'),
Why journal?

https://tryton-rietveld.appspot.com/30501002/diff/60001/statement.py#newcode135
statement.py:135: 'post_with_pending_amount': ('There are origins "%(origin)s"'
I think "origins" is not very meaning file. I would use "original lines" or
"imported lines".
Also it should not be plural if only one is shown.

https://tryton-rietveld.appspot.com/30501002/diff/60001/statement.py#newcode558
statement.py:558: class Line(sequence_ordered(), ModelSQL, ModelView):
I think we could use a Mixin for shared field between Line and Origin.

https://tryton-rietveld.appspot.com/30501002/diff/60001/statement.py#newcode574
statement.py:574: }, depends=_depends + ['origin'])
If you do not use _states, you should not use _depends

https://tryton-rietveld.appspot.com/30501002/diff/60001/statement.py#newcode619
statement.py:619: cls.__rpc__['get_statement_states'] = RPC()
Normally it is no more needed.

https://tryton-rietveld.appspot.com/30501002/diff/60001/statement.py#newcode714
statement.py:714: def on_change_origin(self):
Should test if origin is set.

https://tryton-rietveld.appspot.com/30501002/diff/60001/statement.py#newcode960
statement.py:960: depends=['file_format'])
Should not we ask for the journal?

https://tryton-rietveld.appspot.com/30501002/diff/60001/statement.py#newcode990
statement.py:990: statement.origins = list(self.get_origins(statement))
I'm wondering if statement and origin should not be instantiated from the same
method.

[pokoli, 2017-01-18 08:54:08]

Add doc and fix comments

[pokoli, 2017-01-18 08:54:26]

https://tryton-rietveld.appspot.com/30501002/diff/60001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/60001/statement.py#newcode558
statement.py:558: class Line(sequence_ordered(), ModelSQL, ModelView):
On 2016/12/26 17:07:33, ced wrote:
> I think we could use a Mixin for shared field between Line and Origin.

Maybe, but they do not have the same states.

https://tryton-rietveld.appspot.com/30501002/diff/60001/statement.py#newcode574
statement.py:574: }, depends=_depends + ['origin'])
On 2016/12/26 17:07:33, ced wrote:
> If you do not use _states, you should not use _depends

I use _states, so then I use _depends

[reviewbot, 2017-01-18 09:12:25]

https://codereview.tryton.org/30501002/diff/80001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/80001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/80001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

URL: https://codereview.tryton.org/30501002

[ced, 2017-01-18 10:13:46]

https://tryton-rietveld.appspot.com/30501002/diff/60001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/60001/statement.py#newcode558
statement.py:558: class Line(sequence_ordered(), ModelSQL, ModelView):
On 2017/01/18 08:54:25, pokoli wrote:
> On 2016/12/26 17:07:33, ced wrote:
> > I think we could use a Mixin for shared field between Line and Origin.
> 
> Maybe, but they do not have the same states.

This could be configurable. But the advantage it to ensure synchronisation.

https://tryton-rietveld.appspot.com/30501002/diff/80001/doc/index.rst
File doc/index.rst (right):

https://tryton-rietveld.appspot.com/30501002/diff/80001/doc/index.rst#newcode55
doc/index.rst:55: The statement origin represents an statement line on an origin
system that
I do not understand "on an origin system".
I would expect having referencing to importation.

https://tryton-rietveld.appspot.com/30501002/diff/80001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/80001/statement.py#newcode135
statement.py:135: 'post_with_pending_amount': ('Imported line "%(origin)s"'
missing ending space

https://tryton-rietveld.appspot.com/30501002/diff/80001/statement.py#newcode995
statement.py:995: statement = Statement(Transaction().context.get('active_id'))
I do not understand this.
How could it work if there are already lines on this statement?

https://tryton-rietveld.appspot.com/30501002/diff/80001/statement.py#newcode997
statement.py:997: statement.origins = list(self.get_origins(statement))
For me, the statement and the origins must be created from the same parsing
method to avoid to parse twice.

[pokoli, 2017-01-27 11:54:47]

Use Mixin and fix comments

[reviewbot, 2017-01-27 12:13:56]

https://codereview.tryton.org/30501002/diff/100001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/100001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/100001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/100001/statement.py#newcode16
statement.py:16:
F401 'RPC' imported but unused

URL: https://codereview.tryton.org/30501002

[pokoli, 2017-02-02 12:00:23]

Update to tip

[reviewbot, 2017-02-02 12:15:25]

https://codereview.tryton.org/30501002/diff/120001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/120001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/120001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/120001/statement.py#newcode16
statement.py:16:
F401 'RPC' imported but unused

URL: https://codereview.tryton.org/30501002

[albert, 2017-02-02 22:35:18]

https://tryton-rietveld.appspot.com/30501002/diff/120001/doc/index.rst
File doc/index.rst (right):

https://tryton-rietveld.appspot.com/30501002/diff/120001/doc/index.rst#newcode55
doc/index.rst:55: The statement origin represents an statement line on an
external system that
an -> a

https://tryton-rietveld.appspot.com/30501002/diff/120001/doc/index.rst#newcode56
doc/index.rst:56: is imported to be splited to multiple lines on our system.
splited -> split

https://tryton-rietveld.appspot.com/30501002/diff/120001/doc/index.rst#newcode90
doc/index.rst:90: A wizard to import statement from a external system. It
creates an statement
statement -> statements
a -> an
an -> a

https://tryton-rietveld.appspot.com/30501002/diff/120001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/120001/statement.py#newcode942
statement.py:942: lines_amount = sum(getattr(l, 'amount') or Decimal(0)
Why using not using l.amount?

[ced, 2017-02-03 00:06:37]

https://tryton-rietveld.appspot.com/30501002/diff/120001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/120001/statement.py#newcode942
statement.py:942: lines_amount = sum(getattr(l, 'amount') or Decimal(0)
On 2017/02/02 22:35:18, albert wrote:
> Why using not using l.amount?

it is an on_change so it is the right way.

[pokoli, 2017-02-03 08:14:53]

Fix albert's comments

[reviewbot, 2017-02-03 08:43:27]

https://codereview.tryton.org/30501002/diff/140001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/140001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/140001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/140001/statement.py#newcode16
statement.py:16:
F401 'RPC' imported but unused

URL: https://codereview.tryton.org/30501002

[pokoli, 2017-02-03 09:16:24]

Remove unused RPC import

[reviewbot, 2017-02-03 09:45:06]

https://codereview.tryton.org/30501002/diff/160001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/160001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/160001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

URL: https://codereview.tryton.org/30501002

[ced, 2017-02-14 22:03:12]

https://tryton-rietveld.appspot.com/30501002/diff/160001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/160001/statement.py#newcode998
statement.py:998: return self.origins
I do not understand what is self.origins?

[pokoli, 2017-02-15 10:35:07]

Return and empty list of origins

[reviewbot, 2017-02-15 10:36:17]

https://codereview.tryton.org/30501002/diff/180001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/180001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/180001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

URL: https://codereview.tryton.org/30501002

[pokoli, 2017-02-15 11:31:31]

Remove unused create_payment_term form scenario

[reviewbot, 2017-02-15 11:33:40]

https://codereview.tryton.org/30501002/diff/200001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/200001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/200001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

URL: https://codereview.tryton.org/30501002

[pokoli, 2017-02-15 12:19:27]

Fix icon name and remove journal from import start

[pokoli, 2017-02-15 12:29:08]

Add missing functions for company field

[reviewbot, 2017-02-15 12:35:58]

https://codereview.tryton.org/30501002/diff/230001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/230001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/230001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

URL: https://codereview.tryton.org/30501002

[pokoli, 2017-02-15 13:07:54]

Test if statement company is set

[reviewbot, 2017-02-15 13:09:07]

https://codereview.tryton.org/30501002/diff/250001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/250001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/250001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

URL: https://codereview.tryton.org/30501002

[EdbO, 2017-02-23 18:42:55]

https://tryton-rietveld.appspot.com/30501002/diff/250001/__init__.py
File __init__.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/250001/__init__.py#newcode18
__init__.py:18: Origin,
Please move Origin before Line, otherwise you will get errors when you upgrade
the software

[ced, 2017-02-23 19:28:51]

https://tryton-rietveld.appspot.com/30501002/diff/250001/tests/scenario_state...
File tests/scenario_statement_origin.rst (right):

https://tryton-rietveld.appspot.com/30501002/diff/250001/tests/scenario_state...
tests/scenario_statement_origin.rst:105: >>> line.statement = statement
This is not done by the client which makes impossible to create line.
Indeed the field is cleared when clicking on OK.

[ced, 2017-02-23 19:31:44]

https://tryton-rietveld.appspot.com/30501002/diff/250001/__init__.py
File __init__.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/250001/__init__.py#newcode18
__init__.py:18: Origin,
On 2017/02/23 18:42:55, EdbO.design wrote:
> Please move Origin before Line, otherwise you will get errors when you upgrade
> the software

This should no more happen on trunk.

[pokoli, 2017-02-24 12:12:37]

Manage line statement when nested with origins

[pokoli, 2017-02-24 12:13:40]

Remove uneeded view changes

[reviewbot, 2017-02-24 12:35:01]

https://codereview.tryton.org/30501002/diff/310001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/310001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/310001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

URL: https://codereview.tryton.org/30501002

[ced, 2017-02-24 12:37:35]

On 2017/02/24 12:12:37, pokoli wrote:
> Manage line statement when nested with origins

I do not understand what you have to do that.
It really needs a clear explanation in the code.

[ced, 2017-02-24 12:37:54]

https://tryton-rietveld.appspot.com/30501002/diff/310001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/310001/statement.py#newcode749
statement.py:749: return super(Line, self).on_change_with_company()
always call super

[pokoli, 2017-02-24 12:52:59]

Add comment, always call supper an add company field on statment view

[reviewbot, 2017-02-24 12:59:21]

https://codereview.tryton.org/30501002/diff/330001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/330001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/330001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

URL: https://codereview.tryton.org/30501002

[ced, 2017-02-24 14:15:08]

https://tryton-rietveld.appspot.com/30501002/diff/330001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/330001/statement.py#newcode779
statement.py:779: # created lines related to origin related to an unsaved
statement
Why? I think it is better to have it set correctly.

[pokoli, 2017-02-24 14:34:20]

https://tryton-rietveld.appspot.com/30501002/diff/330001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/330001/statement.py#newcode779
statement.py:779: # created lines related to origin related to an unsaved
statement
On 2017/02/24 14:15:08, ced wrote:
> Why? I think it is better to have it set correctly.

And how can I set correctly via on_change if the parent record is not still
saved?

[ced, 2017-02-24 14:45:51]

https://tryton-rietveld.appspot.com/30501002/diff/330001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/330001/statement.py#newcode779
statement.py:779: # created lines related to origin related to an unsaved
statement
On 2017/02/24 14:34:20, pokoli wrote:
> On 2017/02/24 14:15:08, ced wrote:
> > Why? I think it is better to have it set correctly.
> 
> And how can I set correctly via on_change if the parent record is not still
> saved?

Could not we ensure that it must be saved before creating?

[pokoli, 2017-02-24 15:16:11]

Make lines readonly for non saved statement and don't clear statement value on
line on_change_amount

[reviewbot, 2017-02-24 15:31:49]

https://codereview.tryton.org/30501002/diff/340001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/340001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/340001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

URL: https://codereview.tryton.org/30501002

[ced, 2017-07-31 14:11:38]

Add party and account to origin

[reviewbot, 2017-07-31 14:13:22]

https://codereview.tryton.org/30501002/diff/360001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/360001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/360001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

URL: https://codereview.tryton.org/30501002

[ced, 2017-07-31 14:54:27]

Add relate from statement to pending origins

[reviewbot, 2017-07-31 15:04:56]

https://codereview.tryton.org/30501002/diff/360002/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/360002/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/360002/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

URL: https://codereview.tryton.org/30501002

[ced, 2017-07-31 15:52:46]

Make import wizard create statement

[reviewbot, 2017-07-31 16:13:37]

https://codereview.tryton.org/30501002/diff/390001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/390001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/390001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

URL: https://codereview.tryton.org/30501002

[ced, 2017-07-31 17:16:09]

Add bank account on journal for search on import

[reviewbot, 2017-07-31 17:33:51]

https://codereview.tryton.org/30501002/diff/410001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/410001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/410001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

URL: https://codereview.tryton.org/30501002

[pokoli, 2017-08-01 08:19:43]

https://tryton-rietveld.appspot.com/30501002/diff/410001/statement.xml
File statement.xml (right):

https://tryton-rietveld.appspot.com/30501002/diff/410001/statement.xml#newcod...
statement.xml:353: <record model="ir.action.act_window"
id="act_statement_origin_form_statement">
80cols

https://tryton-rietveld.appspot.com/30501002/diff/410001/statement.xml#newcod...
statement.xml:363: <record model="ir.action.act_window.view"
id="act_statement_origin_form_statement_view1">
80cols

https://tryton-rietveld.appspot.com/30501002/diff/410001/statement.xml#newcod...
statement.xml:368: <record model="ir.action.act_window.view"
id="act_statement_origin_form_statement_view2">
80cols

https://tryton-rietveld.appspot.com/30501002/diff/410001/statement.xml#newcod...
statement.xml:373: <record model="ir.action.keyword"
id="act_statement_origin_form_statement_keyword1">
80cols

https://tryton-rietveld.appspot.com/30501002/diff/410001/statement.xml#newcod...
statement.xml:380: <field name="model" search="[('model', '=',
'account.statement.origin')]"/>
80cols

https://tryton-rietveld.appspot.com/30501002/diff/410001/statement.xml#newcod...
statement.xml:396: <record model="ir.model.access"
id="access_statement_origin_account_admin">
80cols

https://tryton-rietveld.appspot.com/30501002/diff/410001/statement.xml#newcod...
statement.xml:397: <field name="model" search="[('model', '=',
'account.statement.origin')]"/>
80cols

https://tryton-rietveld.appspot.com/30501002/diff/410001/statement.xml#newcod...
statement.xml:405: <field name="model" search="[('model', '=',
'account.statement.origin')]"/>
80cols

https://tryton-rietveld.appspot.com/30501002/diff/410001/statement.xml#newcod...
statement.xml:413: <field name="model" search="[('model', '=',
'account.statement.origin')]"/>
80cols

https://tryton-rietveld.appspot.com/30501002/diff/410001/statement.xml#newcod...
statement.xml:434: <record model="ir.action-res.group"
id="statement_import-group_account_admin">
80cols

https://tryton-rietveld.appspot.com/30501002/diff/410001/statement.xml#newcod...
statement.xml:442: <record model="ir.action-res.group"
id="statement_import-group_statement">
80cols

[ced, 2017-08-01 08:58:06]

But I'm wondering if we should not drop this limit for XML file because in all
cases XML is not nice to read.

https://tryton-rietveld.appspot.com/30501002/diff/410001/statement.xml
File statement.xml (right):

https://tryton-rietveld.appspot.com/30501002/diff/410001/statement.xml#newcod...
statement.xml:353: <record model="ir.action.act_window"
id="act_statement_origin_form_statement">
On 2017/08/01 08:19:42, pokoli wrote:
> 80cols

Done.

[pokoli, 2017-08-01 09:01:10]

On 2017/08/01 08:58:06, ced wrote:
> But I'm wondering if we should not drop this limit for XML file because in all
> cases XML is not nice to read.

I find it usefull when it's possible to have it, as it makes xml easier to read.

[ced, 2017-08-01 10:39:03]

Fix wizard, journal_by_account etc.

[reviewbot, 2017-08-01 11:00:23]

https://codereview.tryton.org/30501002/diff/430001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/430001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/430001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

URL: https://codereview.tryton.org/30501002

[ced, 2017-08-01 13:34:34]

Store origin file

[reviewbot, 2017-08-01 14:05:41]

https://codereview.tryton.org/30501002/diff/450001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/450001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/450001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

URL: https://codereview.tryton.org/30501002

[pokoli, 2017-08-03 08:57:57]

https://tryton-rietveld.appspot.com/30501002/diff/450001/CHANGELOG
File CHANGELOG (right):

https://tryton-rietveld.appspot.com/30501002/diff/450001/CHANGELOG#newcode1
CHANGELOG:1: * Add generic wizard to import statement from files
why "generic"?. I prefer "Add wizard to import statement from files"

https://tryton-rietveld.appspot.com/30501002/diff/450001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/450001/statement.py#newcode571
statement.py:571:
@ModelView.button_action('account_statement.wizard_statement_import')
this is no more required, isn't it?

https://tryton-rietveld.appspot.com/30501002/diff/450001/statement.py#newcode572
statement.py:572: def import_(cls, statements):
idem

https://tryton-rietveld.appspot.com/30501002/diff/450001/statement.py#newcode884
statement.py:884: del _states, _depends
why not deleting after field definition?

https://tryton-rietveld.appspot.com/30501002/diff/450001/statement.py#newcode...
statement.py:1033: file_format = fields.Selection([(None, '')], 'File Format',
required=True)
I'm not sure if it's really usefull to have an empty option here.

[ced, 2017-08-03 09:32:25]

https://tryton-rietveld.appspot.com/30501002/diff/450001/CHANGELOG
File CHANGELOG (right):

https://tryton-rietveld.appspot.com/30501002/diff/450001/CHANGELOG#newcode1
CHANGELOG:1: * Add generic wizard to import statement from files
On 2017/08/03 08:57:57, pokoli wrote:
> why "generic"?. I prefer "Add wizard to import statement from files"

Because it is one wizard to rule all formats.

https://tryton-rietveld.appspot.com/30501002/diff/450001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/450001/statement.py#newcode571
statement.py:571:
@ModelView.button_action('account_statement.wizard_statement_import')
On 2017/08/03 08:57:57, pokoli wrote:
> this is no more required, isn't it?

Acknowledged.

https://tryton-rietveld.appspot.com/30501002/diff/450001/statement.py#newcode572
statement.py:572: def import_(cls, statements):
On 2017/08/03 08:57:57, pokoli wrote:
> idem

Acknowledged.

https://tryton-rietveld.appspot.com/30501002/diff/450001/statement.py#newcode884
statement.py:884: del _states, _depends
On 2017/08/03 08:57:57, pokoli wrote:
> why not deleting after field definition?

Because it is not defined in the class.

https://tryton-rietveld.appspot.com/30501002/diff/450001/statement.py#newcode...
statement.py:1033: file_format = fields.Selection([(None, '')], 'File Format',
required=True)
On 2017/08/03 08:57:57, pokoli wrote:
> I'm not sure if it's really usefull to have an empty option here. 

It is the default value.

[ced, 2017-08-03 09:32:41]

Remove unused button

[reviewbot, 2017-08-03 09:42:38]

https://codereview.tryton.org/30501002/diff/470001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/470001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/470001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

URL: https://codereview.tryton.org/30501002

[pokoli, 2017-08-09 08:17:26]

new scenario must be added on setup.py convert_2to3_doctests

https://tryton-rietveld.appspot.com/30501002/diff/470001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/470001/statement.py#newcode149
statement.py:149: 'post_with_pending_amount': ('Imported line "%(origin)s" '
Instead of "Imported line", I think we should use "Origin"

[ced, 2017-08-09 09:04:45]

https://tryton-rietveld.appspot.com/30501002/diff/470001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/470001/statement.py#newcode149
statement.py:149: 'post_with_pending_amount': ('Imported line "%(origin)s" '
On 2017/08/09 08:17:26, pokoli wrote:
> Instead of "Imported line", I think we should use "Origin"

Done.

[ced, 2017-08-09 09:04:45]

Fix comments

[reviewbot, 2017-08-09 09:31:01]

https://codereview.tryton.org/30501002/diff/490001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/490001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/490001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/490001/setup.py#newcode88
setup.py:88:
E501 line too long (85 > 79 characters)

URL: https://codereview.tryton.org/30501002

[pokoli, 2017-08-09 13:09:52]

It seems there is something wrong with latest patchset, as I see some missing
features: 

1. Changelog is not updated
2. Doc about filestore is removed
3. ORigins Information is removed
4. Import button re-added

[ced, 2017-08-09 13:28:08]

Update right version

[reviewbot, 2017-08-09 13:34:13]

https://codereview.tryton.org/30501002/diff/510001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/510001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/510001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/510001/setup.py#newcode88
setup.py:88:
E501 line too long (85 > 79 characters)

URL: https://codereview.tryton.org/30501002

[pokoli, 2017-08-09 14:00:12]

https://tryton-rietveld.appspot.com/30501002/diff/510001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/510001/statement.py#newcode123
statement.py:123: 'Origins', states={
double quotes for Origins

https://tryton-rietveld.appspot.com/30501002/diff/510001/statement.py#newcode124
statement.py:124: 'readonly': (Eval('state') != 'draft'),
parenthesis can be removed.

[ced, 2017-08-09 14:05:02]

Fix rem

[reviewbot, 2017-08-09 14:07:15]

https://codereview.tryton.org/30501002/diff/530001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/530001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/530001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/530001/setup.py#newcode88
setup.py:88:
E501 line too long (85 > 79 characters)

URL: https://codereview.tryton.org/30501002

[pokoli, 2017-08-09 14:36:31]

LGTM

[albert, 2017-08-09 22:03:54]

https://tryton-rietveld.appspot.com/30501002/diff/530001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/530001/statement.py#newcode555
statement.py:555: 'statment': statement.rec_name,
'statment' -> 'statement'

https://tryton-rietveld.appspot.com/30501002/diff/530001/statement.py#newcode616
statement.py:616: if self.statement and self.statement.company:
Don't think we need to check "if self.statement" given that the 'statement'
field is required.

https://tryton-rietveld.appspot.com/30501002/diff/530001/statement.xml
File statement.xml (right):

https://tryton-rietveld.appspot.com/30501002/diff/530001/statement.xml#newcod...
statement.xml:406: <field name="group" ref="account.group_account_admin"/>
I don't understand why we need this ir.model.access. A user in the
group_account_admin will already be in group_account which already will provide
full access to the record.

https://tryton-rietveld.appspot.com/30501002/diff/530001/statement.xml#newcod...
statement.xml:433: search="[('model', '=',
'account.statement.origin.information')]"/>
I don't think we should grant read access to all users of the system to
information obtained from a bank statement.

I think access to that information should be granted to users in
account.group_account and group_statement.

https://tryton-rietveld.appspot.com/30501002/diff/530001/statement.xml#newcod...
statement.xml:456: <field name="group" ref="account.group_account_admin"/>
I don't think we need to grant access to users in account.group_account_admin

[ced, 2017-08-11 21:10:10]

fix typo

[ced, 2017-08-11 21:10:50]

https://tryton-rietveld.appspot.com/30501002/diff/530001/statement.py
File statement.py (right):

https://tryton-rietveld.appspot.com/30501002/diff/530001/statement.py#newcode555
statement.py:555: 'statment': statement.rec_name,
On 2017/08/09 22:03:54, albert wrote:
> 'statment' -> 'statement'

Done.

https://tryton-rietveld.appspot.com/30501002/diff/530001/statement.py#newcode616
statement.py:616: if self.statement and self.statement.company:
On 2017/08/09 22:03:54, albert wrote:
> Don't think we need to check "if self.statement" given that the 'statement'
> field is required.

This is a on_change, everything can happen.

https://tryton-rietveld.appspot.com/30501002/diff/530001/statement.xml
File statement.xml (right):

https://tryton-rietveld.appspot.com/30501002/diff/530001/statement.xml#newcod...
statement.xml:406: <field name="group" ref="account.group_account_admin"/>
On 2017/08/09 22:03:54, albert wrote:
> I don't understand why we need this ir.model.access. A user in the
> group_account_admin will already be in group_account which already will
provide
> full access to the record.

This is how account modules are designed.

https://tryton-rietveld.appspot.com/30501002/diff/530001/statement.xml#newcod...
statement.xml:433: search="[('model', '=',
'account.statement.origin.information')]"/>
On 2017/08/09 22:03:54, albert wrote:
> I don't think we should grant read access to all users of the system to
> information obtained from a bank statement.
> 
> I think access to that information should be granted to users in
> account.group_account and group_statement.

There is no valuable information there. It is easier that everyone has access.

https://tryton-rietveld.appspot.com/30501002/diff/530001/statement.xml#newcod...
statement.xml:456: <field name="group" ref="account.group_account_admin"/>
On 2017/08/09 22:03:54, albert wrote:
> I don't think we need to grant access to users in account.group_account_admin

Same answer.

[reviewbot, 2017-08-11 21:43:30]

https://codereview.tryton.org/30501002/diff/550001/__init__.py#newcode5
__init__.py:5:
F403 'from journal import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/550001/__init__.py#newcode6
__init__.py:6:
F403 'from statement import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/550001/__init__.py#newcode7
__init__.py:7:
F403 'from account import *' used; unable to detect undefined names

https://codereview.tryton.org/30501002/diff/550001/setup.py#newcode88
setup.py:88:
E501 line too long (85 > 79 characters)

URL: https://codereview.tryton.org/30501002

[rietveld-bot_tryton.org, 2017-09-27 07:01:33]

New changeset 18464ae5629c by Cédric Krier in branch 'default':
Add statment origin and wizard to import them
http://hg.tryton.org/modules/account_statement/rev/18464ae5629c