Merge lp:~roberto-v/account-payment/adding_account_vat_on_payment_7 into lp:~domsense/account-payment/adding_account_vat_on_payment_7
- adding_account_vat_on_payment_7
- Merge into adding_account_vat_on_paym...
Status: | Merged |
---|---|
Merged at revision: | 111 |
Proposed branch: | lp:~roberto-v/account-payment/adding_account_vat_on_payment_7 |
Merge into: | lp:~domsense/account-payment/adding_account_vat_on_payment_7 |
Diff against target: |
141 lines (+102/-0) 5 files modified
account_vat_on_payment/__init__.py (+1/-0) account_vat_on_payment/__openerp__.py (+1/-0) account_vat_on_payment/account.py (+16/-0) account_vat_on_payment/partner.py (+36/-0) account_vat_on_payment/partner_view.xml (+48/-0) |
To merge this branch: | bzr merge lp:~roberto-v/account-payment/adding_account_vat_on_payment_7 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Lorenzo Battistini | Approve | ||
Review via email: mp+205723@code.launchpad.net |
Commit message
Description of the change
- 112. By Roberto Gianassi
-
[IMP] Copyright fix.
Roberto Gianassi (roberto-v) wrote : | # |
> Ciao Roberto,
Ciao Lorenzo,
> many thanks for your contribution.
>
> Some remarks:
> - Why is default_
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_
> 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.
- 113. By Roberto Gianassi
-
[IMP] PEP8 fixings.
- 114. By Roberto Gianassi
-
[FIX] Fixed a bug on XML record creation.
If a module (like fetchmail_invoice) containing a partner record to be
preinstalled through a data.xml file was installed after this one, a
bug occurred.
The bug shows because the record is created with the
default_has_vat_ on_payment flag to empty and then validation fails.
To solve this, the create function for partners is changed to check for
the presence of that flag in the vals dictionary. If the flag is found,
we assume the creation happens through the UI and so normal checking goes
on. If the flag is not found, we assume the creation happens for an XML
record and so we add an arbitrary, but valid value for the flag. - 115. By Roberto Gianassi
-
[FIX] Typo.
Lorenzo Battistini (elbati) wrote : | # |
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_
>
> 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://
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_
> 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_
Thanks
Roberto Gianassi (roberto-v) wrote : | # |
> 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_
> >
> > 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://
> payment/
>
> See below about setting a global default
>
Yes, I see your point.
The problem is that the vat_on_payment flag has to be set differently for every supplier.
There are some rules (for example the supplier has to be Italian) on when you can use VAT on Payment or not and an error here may have legal/financial consequences.
Technically, the selection will be equivalent to a required field. I know you can make a Boolean required, but, in practice, it has no effect (the user can simply ignore it).
I don't know if there are other ways to force a user to make a Boolean choice. In case, let me know.
I need the patch this way. If this is not possible I will go with a custom module.
Thanks for your time.
- 116. By Roberto Gianassi
-
[ADD] Moved default_
has_vat_ on_payment on account. fiscal. position. After some research, the VAT on Payment seems dependent on the account
fiscal position of the customer/supplier.
Moved the default_has_vat_ on_payment flag to that model to take that
into account.
The field is now a boolean because is not needed anymore to force the
user to make a choice. - 117. By Roberto Gianassi
-
[FIX] When partner field is empty value is False.
The value passed to the onchange_partner_id for the partner is False
when the field is empty. In this case, it is not possible to use such a
value with the browse function or an error arises. Fixed that.
Lorenzo Battistini (elbati) wrote : | # |
Roberto, thanks for your changes.
I think using fiscal positions is a better choice.
Some remarks:
line 39: 'for partner_id in partner_
line 41: if 'property_
line 89, I think the _defaults is unnecessary
- 118. By Roberto Gianassi
-
[FIX] Fixing remarks.
Lorenzo Battistini (elbati) wrote : | # |
Thanks.
PS: please send a message after the changes, launchpad does not automatically notifies of new commits
Pedro Manuel Baeza (pedro.baeza) wrote : | # |
This project is now hosted on https:/
Preview Diff
1 | === modified file 'account_vat_on_payment/__init__.py' |
2 | --- account_vat_on_payment/__init__.py 2013-05-15 09:35:50 +0000 |
3 | +++ account_vat_on_payment/__init__.py 2014-02-24 09:06:14 +0000 |
4 | @@ -22,3 +22,4 @@ |
5 | ############################################################################## |
6 | import account |
7 | import company |
8 | +import partner |
9 | |
10 | === modified file 'account_vat_on_payment/__openerp__.py' |
11 | --- account_vat_on_payment/__openerp__.py 2013-11-27 14:37:50 +0000 |
12 | +++ account_vat_on_payment/__openerp__.py 2014-02-24 09:06:14 +0000 |
13 | @@ -52,6 +52,7 @@ |
14 | 'update_xml': [ |
15 | 'account_view.xml', |
16 | 'company_view.xml', |
17 | + 'partner_view.xml', |
18 | ], |
19 | 'demo_xml': [], # TODO YAML tests |
20 | 'installable': True, |
21 | |
22 | === modified file 'account_vat_on_payment/account.py' |
23 | --- account_vat_on_payment/account.py 2013-11-28 07:11:33 +0000 |
24 | +++ account_vat_on_payment/account.py 2014-02-24 09:06:14 +0000 |
25 | @@ -336,6 +336,22 @@ |
26 | new_move_lines.append(line_tup) |
27 | return new_move_lines |
28 | |
29 | + def onchange_partner_id( |
30 | + self, cr, uid, ids, type, partner_id, date_invoice=False, |
31 | + payment_term=False, partner_bank_id=False, company_id=False): |
32 | + res = super(account_invoice, self).onchange_partner_id( |
33 | + cr, uid, ids, type, partner_id, date_invoice, payment_term, |
34 | + partner_bank_id, company_id) |
35 | + # default value for VAT on Payment is changed every time the |
36 | + # customer/supplier is changed |
37 | + partner_obj = self.pool.get("res.partner") |
38 | + if partner_id: |
39 | + p = partner_obj.browse(cr, uid, partner_id) |
40 | + res['value']['vat_on_payment'] = p.property_account_position\ |
41 | + and p.property_account_position.default_has_vat_on_payment\ |
42 | + or False |
43 | + return res |
44 | + |
45 | _inherit = "account.invoice" |
46 | _columns = { |
47 | 'vat_on_payment': fields.boolean('Vat on payment'), |
48 | |
49 | === added file 'account_vat_on_payment/partner.py' |
50 | --- account_vat_on_payment/partner.py 1970-01-01 00:00:00 +0000 |
51 | +++ account_vat_on_payment/partner.py 2014-02-24 09:06:14 +0000 |
52 | @@ -0,0 +1,36 @@ |
53 | +# -*- coding: utf-8 -*- |
54 | +############################################################################## |
55 | +# |
56 | +# OpenERP, Open Source Management Solution |
57 | +# Copyright (C) 2011-2012 Domsense s.r.l. (<http://www.domsense.com>). |
58 | +# Copyright (C) 2012-2013 Agile Business Group sagl |
59 | +# (<http://www.agilebg.com>) |
60 | +# Copyright (C) 2014 Develer srl (<http://www.develer.com>) |
61 | +# |
62 | +# This program is free software: you can redistribute it and/or modify |
63 | +# it under the terms of the GNU Affero General Public License as |
64 | +# published by the Free Software Foundation, either version 3 of the |
65 | +# License, or (at your option) any later version. |
66 | +# |
67 | +# This program is distributed in the hope that it will be useful, |
68 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of |
69 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
70 | +# GNU Affero General Public License for more details. |
71 | +# |
72 | +# You should have received a copy of the GNU Affero General Public License |
73 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
74 | +# |
75 | +############################################################################## |
76 | + |
77 | + |
78 | +from openerp.osv import fields, orm |
79 | +from openerp.tools.translate import _ |
80 | + |
81 | + |
82 | +class account_fiscal_position(orm.Model): |
83 | + _inherit = 'account.fiscal.position' |
84 | + |
85 | + _columns = { |
86 | + 'default_has_vat_on_payment': fields.boolean( |
87 | + 'VAT on Payment Default Flag'), |
88 | + } |
89 | |
90 | === added file 'account_vat_on_payment/partner_view.xml' |
91 | --- account_vat_on_payment/partner_view.xml 1970-01-01 00:00:00 +0000 |
92 | +++ account_vat_on_payment/partner_view.xml 2014-02-24 09:06:14 +0000 |
93 | @@ -0,0 +1,48 @@ |
94 | +<?xml version="1.0"?> |
95 | + |
96 | +<!-- |
97 | +# -*- coding: utf-8 -*- |
98 | +############################################################################## |
99 | +# |
100 | +# OpenERP, Open Source Management Solution |
101 | +# Copyright (C) 2011-2012 Domsense s.r.l. (<http://www.domsense.com>). |
102 | +# Copyright (C) 2012-2013 Agile Business Group sagl |
103 | +# (<http://www.agilebg.com>) |
104 | +# Copyright (C) 2014 Develer srl (<http://www.develer.com>) |
105 | +# |
106 | +# This program is free software: you can redistribute it and/or modify |
107 | +# it under the terms of the GNU Affero General Public License as |
108 | +# published by the Free Software Foundation, either version 3 of the |
109 | +# License, or (at your option) any later version. |
110 | +# |
111 | +# This program is distributed in the hope that it will be useful, |
112 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of |
113 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
114 | +# GNU Affero General Public License for more details. |
115 | +# |
116 | +# You should have received a copy of the GNU Affero General Public License |
117 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
118 | +# |
119 | +############################################################################## |
120 | +--> |
121 | + |
122 | + |
123 | +<openerp> |
124 | + <data> |
125 | + |
126 | + <!-- res.partner customizations --> |
127 | + |
128 | + <record id="view_partner_form" model="ir.ui.view"> |
129 | + <field name="name">account.fiscal.position.vat.default.view</field> |
130 | + <field name="model">account.fiscal.position</field> |
131 | + <field name="inherit_id" ref="account.view_account_position_form"/> |
132 | + <field name="arch" type="xml"> |
133 | + <field name="active" position="after" version="7.0"> |
134 | + <field name="default_has_vat_on_payment" class="oe_inline"/> |
135 | + </field> |
136 | + </field> |
137 | + </record> |
138 | + |
139 | + |
140 | + </data> |
141 | +</openerp> |
Ciao Roberto,
many thanks for your contribution.
Some remarks: has_vat_ on_payment a selection instead of a boolean? has_vat_ on_payment when creating new partners
- Why is 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_
- Please run a PEP8 validator on your code