← Back to team overview

credativ team mailing list archive

Re: [Merge] lp:~jamesj/account-banking/us-canada-payments into lp:~credativ/account-banking/upgrade-6.0

 

Hi Stefan,

I understand the approach that your suggesting, and that sounds 
reasonable. I debated taking that approach and decided to opt for the 
one that included the smallest code change for now, allowing 
alternatives to be investigated.

If the SWIFT online lookup is to be replaced by another online lookup, I 
think that should be optional (triggered by a parameter). The reason is 
that some deployments of OpenERP will be on servers that do not allow 
external server access. Also, the user experience is poor when the 
online lookup is slow - the UI just locks, without informing the user.

I think it would be better to place call the lookup code when the user 
saves the record (the SWIFT code will only be in the vals dictionary 
when the code is entered or changed).

Thanks

James

On 17/01/12 09:12, Stefan Rijnhart (Therp) wrote:
> Hi James,
>
> please excuse me for commenting on your own branches, but I think you are interested in merging with banking-addons or account-banking eventually. Therefore I have a comment on your change to account_banking/account_banking_view.xml:8
>
> You signaled the issue of the 404 on the SWIFT online lookup in lp:914922. Your solution here is to disable the on_change method. I would prefer to keep the on-change method, but disable the online function until we have found a viable alternative and parametrized the online lookup like you suggested in the bug report. See the following branch:
>
> https://code.launchpad.net/~banking-addons-team/banking-addons/lp914922
>
> If you feel that you must disable the on-change method, you can simply replace the field again in the HSBC module.
>
> Cheers,
> Stefan.
>
>
>
>
>
>
>

-- 
https://code.launchpad.net/~jamesj/account-banking/us-canada-payments/+merge/88822
Your team credativ is requested to review the proposed merge of lp:~jamesj/account-banking/us-canada-payments into lp:~credativ/account-banking/upgrade-6.0.


References