Merge lp:~roberto-v/openobject-italia/7.0 into lp:~openobject-italia-core-devs/openobject-italia/italian-addons-7.0

Proposed by Roberto Gianassi
Status: Needs review
Proposed branch: lp:~roberto-v/openobject-italia/7.0
Merge into: lp:~openobject-italia-core-devs/openobject-italia/italian-addons-7.0
Diff against target: 166 lines (+114/-4)
5 files modified
l10n_it_withholding_tax/__init__.py (+1/-0)
l10n_it_withholding_tax/__openerp__.py (+3/-1)
l10n_it_withholding_tax/account.py (+27/-3)
l10n_it_withholding_tax/partner.py (+36/-0)
l10n_it_withholding_tax/partner_view.xml (+47/-0)
To merge this branch: bzr merge lp:~roberto-v/openobject-italia/7.0
Reviewer Review Type Date Requested Status
Lorenzo Battistini code review Approve
Review via email: mp+205739@code.launchpad.net
To post a comment you must log in.
lp:~roberto-v/openobject-italia/7.0 updated
235. By Roberto Gianassi

[IMP] Copyright fix.

236. By Roberto Gianassi

[IMP] PEP8 fixings.

237. 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_withholding 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.

238. By Roberto Gianassi

[FIX] Typo.

239. By Roberto Gianassi

[IMP] Changed default_has_withholding type to Boolean.

240. By Roberto Gianassi

[FIX] Bug fix to save record.

For unknown reasons, if a boolean value has the required field to True,
it is not possible to save the corresponding supplier record.
Removed the field to fix that.

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

Revision history for this message
Lorenzo Battistini (elbati) wrote :

Following this MP
https://code.launchpad.net/~roberto-v/account-payment/adding_account_vat_on_payment_7/+merge/205723
maybe better if we use fiscal positions for withholding taxes too?

Thanks

review: Needs Information
Revision history for this message
Roberto Gianassi (roberto-v) wrote :

> Following this MP
> https://code.launchpad.net/~roberto-v/account-
> payment/adding_account_vat_on_payment_7/+merge/205723
> maybe better if we use fiscal positions for withholding taxes too?

Depends... :)

After discussing this in my shop, the reasoning is as follows:

- VAT on Payment gives a financial advantage, so it is always used where applicable by law; linking it to fiscal position seems the best bet, because it depends on law. After I set it to True, I have the best possible advantage. Only in rare cases, I will manually set it to False.

- Withholding Taxes is something you have to do with some parties, but it doesn't give any financial advantage; normally companies prefer to avoid it because it implies more accounting work; linking it to a supplier seems the best choice because you want to use it as little as possible. I don't see any advantages on linking it to fiscal position: it will quite always be False and you still have to set it manually to True on every supplier you need to.

However, if you have valid reasons to do it differently let me know and we may discuss a solution.

Revision history for this message
Lorenzo Battistini (elbati) wrote :

Ok, let's set it on partner.

line 87, _defaults is unnecessary

please apply PEP8 to the new code

review: Needs Fixing
lp:~roberto-v/openobject-italia/7.0 updated
242. By Roberto Gianassi

[FIX] Fixed pep8 warnings on new code.

243. By Roberto Gianassi

[MERGE]

Revision history for this message
Roberto Gianassi (roberto-v) wrote :

Ok, fixed.

Revision history for this message
Lorenzo Battistini (elbati) wrote :

Please apply PEP8 to onchange_partner_id too

We would like to avoid introducing other not PEP8 code (there's enough ;-) )

lp:~roberto-v/openobject-italia/7.0 updated
244. By Roberto Gianassi

[FIX] Missing pep8 fixes.

Revision history for this message
Roberto Gianassi (roberto-v) wrote :

Ok, fixed that, too.

Thank you!

Revision history for this message
Lorenzo Battistini (elbati) :
review: Approve (code review)
Revision history for this message
Lorenzo Battistini (elbati) wrote :

This project is now hosted on https://github.com/OCA/l10n-italy. Please move your proposal there. This guide may help you https://github.com/OCA/maintainers-tools/wiki/How-to-move-a-Merge-Proposal-to-GitHub

Unmerged revisions

244. By Roberto Gianassi

[FIX] Missing pep8 fixes.

243. By Roberto Gianassi

[MERGE]

242. By Roberto Gianassi

[FIX] Fixed pep8 warnings on new code.

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

240. By Roberto Gianassi

[FIX] Bug fix to save record.

For unknown reasons, if a boolean value has the required field to True,
it is not possible to save the corresponding supplier record.
Removed the field to fix that.

239. By Roberto Gianassi

[IMP] Changed default_has_withholding type to Boolean.

238. By Roberto Gianassi

[FIX] Typo.

237. 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_withholding 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.

236. By Roberto Gianassi

[IMP] PEP8 fixings.

235. By Roberto Gianassi

[IMP] Copyright fix.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'l10n_it_withholding_tax/__init__.py'
--- l10n_it_withholding_tax/__init__.py 2013-04-17 10:18:29 +0000
+++ l10n_it_withholding_tax/__init__.py 2014-02-28 08:04:32 +0000
@@ -22,3 +22,4 @@
22##############################################################################22##############################################################################
2323
24import account24import account
25import partner
2526
=== modified file 'l10n_it_withholding_tax/__openerp__.py'
--- l10n_it_withholding_tax/__openerp__.py 2013-04-17 14:19:03 +0000
+++ l10n_it_withholding_tax/__openerp__.py 2014-02-28 08:04:32 +0000
@@ -48,7 +48,9 @@
48 'license': 'AGPL-3',48 'license': 'AGPL-3',
49 "depends" : ['account_voucher_cash_basis'],49 "depends" : ['account_voucher_cash_basis'],
50 "data" : [50 "data" : [
51 'account_view.xml',],51 'account_view.xml',
52 'partner_view.xml',
53 ],
52 "demo" : [54 "demo" : [
53 'account_demo.xml',55 'account_demo.xml',
54 ],56 ],
5557
=== modified file 'l10n_it_withholding_tax/account.py'
--- l10n_it_withholding_tax/account.py 2013-05-09 14:30:36 +0000
+++ l10n_it_withholding_tax/account.py 2014-02-28 08:04:32 +0000
@@ -91,18 +91,42 @@
91 return res91 return res
9292
93class account_invoice(orm.Model):93class account_invoice(orm.Model):
94 94
95 def _net_pay(self, cr, uid, ids, field_name, arg, context=None):95 def _net_pay(self, cr, uid, ids, field_name, arg, context=None):
96 res = {}96 res = {}
97 for invoice in self.browse(cr, uid, ids, context):97 for invoice in self.browse(cr, uid, ids, context):
98 res[invoice.id] = invoice.amount_total - invoice.withholding_amount98 res[invoice.id] = invoice.amount_total - invoice.withholding_amount
99 return res99 return res
100100
101 def onchange_partner_id(
102 self, cr, uid, ids, type, partner_id,
103 date_invoice=False, payment_term=False, partner_bank_id=False,
104 company_id=False):
105 res = super(account_invoice, self).onchange_partner_id(
106 cr, uid, ids, type, partner_id, date_invoice, payment_term,
107 partner_bank_id, company_id)
108 # default value for VAT on Payment is changed
109 # every time the supplier is changed
110 partner_obj = self.pool.get("res.partner")
111 if partner_id:
112 for partner in partner_obj.browse(cr, uid, [partner_id]):
113 if partner.supplier:
114 res['value']['has_withholding'] = \
115 partner.default_has_withholding
116 return res
117
101 _inherit = "account.invoice"118 _inherit = "account.invoice"
102119
103 _columns = {120 _columns = {
104 'withholding_amount': fields.float('Withholding amount', digits_compute=dp.get_precision('Account'), readonly=True, states={'draft':[('readonly',False)]}),121 'withholding_amount': fields.float(
105 'has_withholding': fields.boolean('With withholding tax', readonly=True, states={'draft':[('readonly',False)]}),122 'Withholding amount',
123 digits_compute=dp.get_precision('Account'),
124 readonly=True,
125 states={'draft': [('readonly', False)]}),
126 'has_withholding': fields.boolean(
127 'With withholding tax',
128 readonly=True,
129 states={'draft': [('readonly', False)]}),
106 'net_pay': fields.function(_net_pay, string="Net Pay"),130 'net_pay': fields.function(_net_pay, string="Net Pay"),
107 }131 }
108132
109133
=== added file 'l10n_it_withholding_tax/partner.py'
--- l10n_it_withholding_tax/partner.py 1970-01-01 00:00:00 +0000
+++ l10n_it_withholding_tax/partner.py 2014-02-28 08:04:32 +0000
@@ -0,0 +1,36 @@
1# -*- coding: utf-8 -*-
2##############################################################################
3#
4# OpenERP, Open Source Management Solution
5# Copyright (C) 2011-2012 Domsense s.r.l. (<http://www.domsense.com>).
6# Copyright (C) 2012-2013 Agile Business Group sagl
7# (<http://www.agilebg.com>)
8# Copyright (C) 2014 Develer srl (<http://www.develer.com>)
9#
10# This program is free software: you can redistribute it and/or modify
11# it under the terms of the GNU Affero General Public License as
12# published by the Free Software Foundation, either version 3 of the
13# License, or (at your option) any later version.
14#
15# This program is distributed in the hope that it will be useful,
16# but WITHOUT ANY WARRANTY; without even the implied warranty of
17# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
18# GNU Affero General Public License for more details.
19#
20# You should have received a copy of the GNU Affero General Public License
21# along with this program. If not, see <http://www.gnu.org/licenses/>.
22#
23##############################################################################
24
25
26from openerp.osv import fields, orm
27from openerp.tools.translate import _
28
29
30class res_partner(orm.Model):
31 _inherit = 'res.partner'
32
33 _columns = {
34 'default_has_withholding': fields.boolean(
35 'WithHolding Tax Default Flag'),
36 }
037
=== added file 'l10n_it_withholding_tax/partner_view.xml'
--- l10n_it_withholding_tax/partner_view.xml 1970-01-01 00:00:00 +0000
+++ l10n_it_withholding_tax/partner_view.xml 2014-02-28 08:04:32 +0000
@@ -0,0 +1,47 @@
1<?xml version="1.0"?>
2
3<!--
4# -*- coding: utf-8 -*-
5##############################################################################
6#
7# OpenERP, Open Source Management Solution
8# Copyright (C) 2011-2012 Domsense s.r.l. (<http://www.domsense.com>).
9# Copyright (C) 2012-2013 Agile Business Group sagl
10# (<http://www.agilebg.com>)
11# Copyright (C) 2014 Develer srl (<http://www.develer.com>)
12#
13# This program is free software: you can redistribute it and/or modify
14# it under the terms of the GNU Affero General Public License as
15# published by the Free Software Foundation, either version 3 of the
16# License, or (at your option) any later version.
17#
18# This program is distributed in the hope that it will be useful,
19# but WITHOUT ANY WARRANTY; without even the implied warranty of
20# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
21# GNU Affero General Public License for more details.
22#
23# You should have received a copy of the GNU Affero General Public License
24# along with this program. If not, see <http://www.gnu.org/licenses/>.
25#
26##############################################################################
27-->
28
29
30<openerp>
31 <data>
32
33 <!-- res.partner customizations -->
34
35 <record id="view_partner_form" model="ir.ui.view">
36 <field name="name">res.partner.vat.default.view</field>
37 <field name="model">res.partner</field>
38 <field name="inherit_id" ref="account.view_partner_property_form"/>
39 <field name="arch" type="xml">
40 <field name="property_account_position" position="after" version="7.0">
41 <field name="default_has_withholding" class="oe_inline" attrs="{'invisible': [('supplier','!=',True)]}"/>
42 </field>
43 </field>
44 </record>
45
46 </data>
47</openerp>

Subscribers

People subscribed via source and target branches