[7.0] account_statement_base_import: default account_id to false (mp:197755) breaks import of line without account_id

Hi,

I re-open the bug cause we still have trouble on this one. Making the import of a file manually still raise the error mention above.

I see here 2 remaining troubles:

 * The _insert_lines and _update_line method in *_base_completion/statement.py should call the symbol_f method to prorperly initiate the default value (e.g. integer default null value should be Null not False).

 * The mechanism that allow to have a null account_id in bank statement line should be part of *_base_completion, not *_base_import.

My args:

  - account_bank_statement_ext should keep the original behavior where account_id is mandatory

  - account_statement_completion provide a way of completing a statement line from only partial info, it is his responsability to provide an account_id if none has been provided. So IMO this is in this module that we should allow not to provide an account_id for a line. But then, required one for the statement confirmation.

  - account_statement_base_import which depends on account_statement_completion will benefit from that

Regards,

Trouble arrives here with revision 143 and was partially solved by revision 146

> * The _insert_lines and _update_line method in *_base_completion/statement.py should call the symbol_f method to prorperly initiate the default value (e.g. integer default null value should be Null not False).

Definitely

> * The mechanism that allow to have a null account_id in bank statement line should be part of *_base_completion, not *_base_import.

+1, but IMO we need to provide a completion rule that mimics the current behavior so the user keeps a way to fill the account_id with the same logic as the one provided by account_bank_statement_ext by using this completion rule. To avoid unexpected behavior after the module's upgrade, we probably need to provide a migration script adding this completion rule on existing profiles

Hi Laurent,

Thanks for the feedback here ! For the second point, I think it's not necessary to have such a "default" rules. My args are:

 * First, a profil will at least have one rule to compute the account (and/or partner) so in any case the completion will provide one, even after this modification

 * If it is not the case anyway, why a rule must set the account_id on completion ? We have a default value for account_id through the _defaults = {'account_id': _get_default_account,} that call the get_values_for_line. The completion will only do the same a second time.

 * Entering the line manually will call the on_change of the bank_statement_ext and provide an account_id

So in the following use cases, I see no trouble,:

 * Entering a statement line manually with *_base_completion installed or not will set a default account_id
 * Importing lines with *_base_import installed and no account provided, the account_id will be set on completion following the rules as all of them at least give the account_id.

Do I miss one ?

Regards,

Joël

Hi Joël,

You're right.

Thanks for the changes,

Regards,

lmi

On Wed, May 7, 2014 at 11:55 AM, Joël Grand-Guillaume @ camptocamp <
<email address hidden>> wrote:

> Hi Laurent,
>
>
> Thanks for the feedback here ! For the second point, I think it's not
> necessary to have such a "default" rules. My args are:
>
> * First, a profil will at least have one rule to compute the account
> (and/or partner) so in any case the completion will provide one, even
> after this modification
>
> * If it is not the case anyway, why a rule must set the account_id on
> completion ? We have a default value for account_id through the
> _defaults = {'account_id': _get_default_account,} that call the
> get_values_for_line. The completion will only do the same a second time.
>
> * Entering the line manually will call the on_change of the
> bank_statement_ext and provide an account_id
>
> So in the following use cases, I see no trouble,:
>
> * Entering a statement line manually with *_base_completion installed or
> not will set a default account_id
> * Importing lines with *_base_import installed and no account provided,
> the account_id will be set on completion following the rules as all of them
> at least give the account_id.
>
> Do I miss one ?
>
>
> Regards,
>
> Joël
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1313689
>
> Title:
> [7.0] account_statement_base_import: default account_id to false
> (mp:197755) breaks import of line without account_id
>
> Status in OpenERP Banking Addons:
> Confirmed
>
> Bug description:
> For performance reasons, the creation of lines in the
> account_statement_base_import is done by bypassing the ORM . (method
> _insert_lines defined in
> account_base_completion.statement.AccountStatementLine). The method
> _insert_lines must be called with a list of values (as dict) used to
> build a plain batch SQL insert 'INSERT INTO
> account_bank_statement_line (%s) VALUES (%s)'
>
> The problem with the mp:197755 (https://code.launchpad.net/~akretion-
> team/banking-addons/bank-statement-reconcile-70-account-not-
> mandatory/+merge/197755) is that if no value is provided for the
> account_id by the parser, the default value is added by the call to
> "statement_line_obj._add_missing_default_values(cr, uid, values,
> context)" at line 130 in account_statement_base_import.statement.py.
> (values = [.... {..., 'account_id':False, ..} ... ] )
>
> As these values are used to build a plain SQL insert (" Insert into
> .... (..., account_id) values ((...,False )) the method fails with the
> exception: except_osv:
> (u'Statement import error', u'The statement cannot be created: Error:
> except_osv\nDescription: (u\'ORM bypass error\', \'ERROR: column
> "account_id" is of type integer but expression is of type boolean\\nLINE 1:
> ...note, period_id, ref, statement_id, type) VALUES (false, \\\'{"...\\n
> ^\\nHINT: You
> will need to rewrite or cast the expression.\\n\')\nTraceback: File
> "/home/lmi/projects/openerp/openerp-deldrive-multi-company/addons-...