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

2014-02-11 18:03 GMT+01:00 Roberto Gianassi <email address hidden>:

> > 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).
>

I mean, conceptually it is a boolean field, it can accept two values.
if I understand what you mean, you want to force the user to do a choice.
But this can be applied to every boolean field of OpenERP. So, in this
case, the boolean field interface behaviour should be changed system wide.

Moreover, using a boolean field allows you to avoid workaround like
this<http://bazaar.launchpad.net/~roberto-v/account-payment/adding_account_vat_on_payment_7/revision/114>

See below about setting a global default

>
> > - 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?
>

At the moment, it sets the default value for 'vat_on_payment' field of
account.invoice, retrieving it from the global setting 'vat_on_payment'.
It think, after your changes, the global vat_on_payment setting should be
used for the default_has_vat_on_payment field.

Thanks