|
|
Patch Set 1 #
Total comments: 4
Patch Set 2 : Think that adding required to states is only necessary if account.mandatory #
Total comments: 5
Patch Set 3 : no need for getting actual 'states' because the method is updating the res of the field_get #
Total comments: 4
Total messages: 12
|
hoRn
analytic_account/account.py
|
14 years, 11 months ago (2011年02月08日 11:29:49 UTC) #1 |
analytic_account/account.py
http://codereview.appspot.com/4149041/diff/1/account.py File account.py (right): http://codereview.appspot.com/4149041/diff/1/account.py#newcode176 account.py:176: res[account_id] = -res[account_id] Don't change this it is not PEP8 http://codereview.appspot.com/4149041/diff/1/account.py#newcode285 account.py:285: res[name]['states'] = PYSONEncoder().encode({'required':Equal(Eval('type'), 'line')}) 80 chars http://codereview.appspot.com/4149041/diff/1/account.py#newcode285 account.py:285: res[name]['states'] = PYSONEncoder().encode({'required':Equal(Eval('type'), 'line')}) You erase the initial states of the field. You must only change the required state. And by the way, why not only use states? http://codereview.appspot.com/4149041/diff/1/account.py#newcode286 account.py:286: else: res[name]['required'] = False Split into 2 lines
changed http://codereview.appspot.com/4149041/diff/4001/account.py File account.py (right): http://codereview.appspot.com/4149041/diff/4001/account.py#newcode176 account.py:176: res[account_id] = -res[account_id] don't know where to set it in eclipse - any hint welcome
http://codereview.appspot.com/4149041/diff/4001/account.py File account.py (right): http://codereview.appspot.com/4149041/diff/4001/account.py#newcode176 account.py:176: res[account_id] = -res[account_id] On 2011年02月08日 12:55:30, hoRn wrote: > don't know where to set it in eclipse - any hint welcome Don't use eclipse :-) http://codereview.appspot.com/4149041/diff/4001/account.py#newcode286 account.py:286: encode({'required':Equal(Eval('type'), 'line')}) one space after :
http://codereview.appspot.com/4149041/diff/4001/account.py File account.py (right): http://codereview.appspot.com/4149041/diff/4001/account.py#newcode286 account.py:286: encode({'required':Equal(Eval('type'), 'line')}) Is this correct with double required? I would propose: res[name]['states']['required'] = PYSONEncoder().encode(Equal(Eval('type'), 'line'))
http://codereview.appspot.com/4149041/diff/4001/account.py File account.py (right): http://codereview.appspot.com/4149041/diff/4001/account.py#newcode286 account.py:286: encode({'required':Equal(Eval('type'), 'line')}) sorry - need to upload a new file - this will not work at all, because res[name]['states'] is a string ;(
http://codereview.appspot.com/4149041/diff/4003/account.py File account.py (right): http://codereview.appspot.com/4149041/diff/4003/account.py#newcode176 account.py:176: res[account_id] = -res[account_id] Still there. http://codereview.appspot.com/4149041/diff/4003/account.py#newcode285 account.py:285: encode({'required': Equal(Eval('type'), 'line')}) As we don't have yet a PYSONDecoder without evaluation, the simple way to fix this is to redefine states. So you must also set the value of invisible like it is in the origin.
http://codereview.appspot.com/4149041/diff/4003/account.py File account.py (right): http://codereview.appspot.com/4149041/diff/4003/account.py#newcode285 account.py:285: encode({'required': Equal(Eval('type'), 'line')}) don't know why, but in the resulting fields_get of analytic_** there is 'invisible' in the states. Think it's because analytic_accounts_fields_get is updating the fields_get. I tried to get the states, but the value of states is a string. so updating this string needs something like eval() - don't know ....
http://codereview.appspot.com/4149041/diff/4003/account.py File account.py (right): http://codereview.appspot.com/4149041/diff/4003/account.py#newcode285 account.py:285: encode({'required': Equal(Eval('type'), 'line')}) On 2011年02月09日 07:56:54, hoRn wrote: > don't know why, but in the resulting fields_get of analytic_** there is > 'invisible' in the states. With or without your patch? On a mandatory account chart? > Think it's because analytic_accounts_fields_get is > updating the fields_get. I tried to get the states, but the value of states is a > string. so updating this string needs something like eval() - don't know .... In the current state, it is not possible to update the states because of missing a tool in PYSON. But you can just set an invisible key in the states dictionary that you encoding.