Additional payment, bank statement and voucher addons for OpenERP

Code review comment for lp:~roberto-v/account-payment/adding_account_vat_on_payment_7

  1. adding_account_vat_on_payment_7
  2. Merge into adding_account_vat_on_paym...
Revision history for this message
Roberto Gianassi (roberto-v) wrote :

> Ciao Roberto,

Ciao Lorenzo,

> many thanks for your contribution.
>
> Some remarks:
> - Why is default_has_vat_on_payment a selection instead of a boolean?

It is that way to force the user to always fill the right value.
If it were a boolean, it would be rendered on the UI as an initial empty checkbox: it would be very simple to forget to check the flag (when it has to be checked).

> - Now, if the vat_on_payment invoice field is (correctly) set by the
> onchange_partner_id method, probably the _get_vat_on_payment method should be
> used to set the default value of default_has_vat_on_payment when creating new
> partners

Please, could you elaborate a bit more on this?
What is the purpose of the _get_vat_on_payment method?

> - Please run a PEP8 validator on your code

Ok.

Thank you for the review.

« Back to merge proposal