Merge lp:~camptocamp/sale-wkfl/7.0-add-sale-default-discount-lep into lp:~sale-core-editors/sale-wkfl/7.0

Proposed by Leonardo Pistone
Status: Needs review
Proposed branch: lp:~camptocamp/sale-wkfl/7.0-add-sale-default-discount-lep
Merge into: lp:~sale-core-editors/sale-wkfl/7.0
Diff against target: 335 lines (+287/-0)
9 files modified
sale_default_discount/__init__.py (+22/-0)
sale_default_discount/__openerp__.py (+56/-0)
sale_default_discount/model/__init__.py (+23/-0)
sale_default_discount/model/partner.py (+34/-0)
sale_default_discount/model/sale.py (+47/-0)
sale_default_discount/test/no_default_discount.yml (+21/-0)
sale_default_discount/test/partner_default_discount.yml (+30/-0)
sale_default_discount/view/partner.xml (+20/-0)
sale_default_discount/view/sale.xml (+34/-0)
To merge this branch: bzr merge lp:~camptocamp/sale-wkfl/7.0-add-sale-default-discount-lep
Reviewer Review Type Date Requested Status
Pedro Manuel Baeza Needs Resubmitting
Review via email: mp+227214@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

Hi, Leonardo,

Thanks for the MP.

I'm seeing the code, and I feel more comfortable having the field called as default_sale_discount, or at least sale_discount, not only discount, because partners can be also suppliers.

About the functionality itself, I'm not seeing in the code the part that applies this default value to sale order lines. Where it is?

Regards.

review: Needs Information (code review)
50. By Leonardo Pistone on 2014-07-18

rename field to sale_default_discount

As suggested by Pedro

51. By Leonardo Pistone on 2014-07-18

add tests to sale_default_discount

As discussed in the yaml file itself, I hit a situation where manual testing
works, but a default value does not show in the order line when in a yaml
test.

I prefer not to write extra production code just to make already working code
pass yaml tests. If YAML code are meant to be full-stack integrated tests (I
got the impression they are, since for example they run onchange methods),
then this looks like a bug in the yaml processor.

Revision history for this message
Leonardo Pistone (lepistone) wrote :

Pedro,

I renamed the field as you suggested.

The default in the line works because of the attribute "context" in sale.xml. Thanks to Romain Deheele for showing me that.

I also added two tests, and got a bit frustrated when I realized that the YAML test does not get the same behaviour of manual testing (all is OK when manual testing). More details in the commit message and the yaml file.

A broader discussion we could continue later on the community list is: what are yaml tests supposed to test?

Thanks

Revision history for this message
Yann Papouin (yann-papouin) wrote :

Just for the record, the almost same result can be achieved with a product.pricelist by checking the visible_discount field and setting this pricelist as the sale one in the partner form.

Revision history for this message
Leonardo Pistone (lepistone) wrote :

Yann,

you are right. Still, this module came out from a situation where pricelists were already used for other things, and a simple default discount was useful anyway.

Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

This project is now hosted on https://github.com/OCA/sale-workflow. 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

review: Needs Resubmitting
Revision history for this message
Leonardo Pistone (lepistone) wrote :

Unmerged revisions

51. By Leonardo Pistone on 2014-07-18

add tests to sale_default_discount

As discussed in the yaml file itself, I hit a situation where manual testing
works, but a default value does not show in the order line when in a yaml
test.

I prefer not to write extra production code just to make already working code
pass yaml tests. If YAML code are meant to be full-stack integrated tests (I
got the impression they are, since for example they run onchange methods),
then this looks like a bug in the yaml processor.

50. By Leonardo Pistone on 2014-07-18

rename field to sale_default_discount

As suggested by Pedro

49. By Leonardo Pistone on 2014-07-17

[add] new module sale_default_discount

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added directory 'sale_default_discount'
2=== added file 'sale_default_discount/__init__.py'
3--- sale_default_discount/__init__.py 1970-01-01 00:00:00 +0000
4+++ sale_default_discount/__init__.py 2014-07-18 08:39:44 +0000
5@@ -0,0 +1,22 @@
6+# -*- coding: utf-8 -*-
7+##############################################################################
8+#
9+# Author: Leonardo Pistone
10+# Copyright 2014 Camptocamp SA
11+#
12+# This program is free software: you can redistribute it and/or modify
13+# it under the terms of the GNU Affero General Public License as
14+# published by the Free Software Foundation, either version 3 of the
15+# License, or (at your option) any later version.
16+#
17+# This program is distributed in the hope that it will be useful,
18+# but WITHOUT ANY WARRANTY; without even the implied warranty of
19+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
20+# GNU Affero General Public License for more details.
21+#
22+# You should have received a copy of the GNU Affero General Public License
23+# along with this program. If not, see <http://www.gnu.org/licenses/>.
24+#
25+##############################################################################
26+
27+from . import model # noqa
28
29=== added file 'sale_default_discount/__openerp__.py'
30--- sale_default_discount/__openerp__.py 1970-01-01 00:00:00 +0000
31+++ sale_default_discount/__openerp__.py 2014-07-18 08:39:44 +0000
32@@ -0,0 +1,56 @@
33+# -*- coding: utf-8 -*-
34+##############################################################################
35+#
36+# Author: Leonardo Pistone
37+# Copyright 2014 Camptocamp SA
38+#
39+# This program is free software: you can redistribute it and/or modify
40+# it under the terms of the GNU Affero General Public License as
41+# published by the Free Software Foundation, either version 3 of the
42+# License, or (at your option) any later version.
43+#
44+# This program is distributed in the hope that it will be useful,
45+# but WITHOUT ANY WARRANTY; without even the implied warranty of
46+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
47+# GNU Affero General Public License for more details.
48+#
49+# You should have received a copy of the GNU Affero General Public License
50+# along with this program. If not, see <http://www.gnu.org/licenses/>.
51+#
52+##############################################################################
53+{
54+ 'name': 'Sale Default Discount',
55+ 'version': '1.0',
56+ 'category': 'Generic Modules/Sale',
57+ 'description': """
58+Sale Default Discount
59+=====================
60+
61+This module allows to set a default discount per customer and per sale order.
62+
63+The customer discount is used by default when creating an order, and the
64+order default is used when creating new order lines.
65+
66+The defaults can be left empty, keeping the default behaviour.
67+
68+Contributors
69+------------
70+
71+ * Leonardo Pistone <leonardo.pistone@camptocamp.com>
72+
73+""",
74+ 'author': 'Camptocamp',
75+ 'depends': ['sale'],
76+ 'website': 'http://www.camptocamp.com',
77+ 'data': [
78+ 'view/partner.xml',
79+ 'view/sale.xml',
80+ ],
81+ 'test': [
82+ 'test/no_default_discount.yml',
83+ 'test/partner_default_discount.yml',
84+ ],
85+ 'demo': [],
86+ 'installable': True,
87+ 'active': False,
88+}
89
90=== added directory 'sale_default_discount/model'
91=== added file 'sale_default_discount/model/__init__.py'
92--- sale_default_discount/model/__init__.py 1970-01-01 00:00:00 +0000
93+++ sale_default_discount/model/__init__.py 2014-07-18 08:39:44 +0000
94@@ -0,0 +1,23 @@
95+# -*- coding: utf-8 -*-
96+##############################################################################
97+#
98+# Author: Leonardo Pistone
99+# Copyright 2014 Camptocamp SA
100+#
101+# This program is free software: you can redistribute it and/or modify
102+# it under the terms of the GNU Affero General Public License as
103+# published by the Free Software Foundation, either version 3 of the
104+# License, or (at your option) any later version.
105+#
106+# This program is distributed in the hope that it will be useful,
107+# but WITHOUT ANY WARRANTY; without even the implied warranty of
108+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
109+# GNU Affero General Public License for more details.
110+#
111+# You should have received a copy of the GNU Affero General Public License
112+# along with this program. If not, see <http://www.gnu.org/licenses/>.
113+#
114+##############################################################################
115+
116+from . import partner # noqa
117+from . import sale # noqa
118
119=== added file 'sale_default_discount/model/partner.py'
120--- sale_default_discount/model/partner.py 1970-01-01 00:00:00 +0000
121+++ sale_default_discount/model/partner.py 2014-07-18 08:39:44 +0000
122@@ -0,0 +1,34 @@
123+# -*- coding: utf-8 -*-
124+##############################################################################
125+#
126+# Author: Leonardo Pistone
127+# Copyright 2014 Camptocamp SA
128+#
129+# This program is free software: you can redistribute it and/or modify
130+# it under the terms of the GNU Affero General Public License as
131+# published by the Free Software Foundation, either version 3 of the
132+# License, or (at your option) any later version.
133+#
134+# This program is distributed in the hope that it will be useful,
135+# but WITHOUT ANY WARRANTY; without even the implied warranty of
136+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
137+# GNU Affero General Public License for more details.
138+#
139+# You should have received a copy of the GNU Affero General Public License
140+# along with this program. If not, see <http://www.gnu.org/licenses/>.
141+#
142+##############################################################################
143+
144+from openerp.osv import orm, fields
145+import openerp.addons.decimal_precision as dp
146+
147+
148+class Partner(orm.Model):
149+ _inherit = 'res.partner'
150+
151+ _columns = {
152+ 'sale_default_discount': fields.float(
153+ 'Default sale discount (%)',
154+ digits_compute=dp.get_precision('discount'),
155+ ),
156+ }
157
158=== added file 'sale_default_discount/model/sale.py'
159--- sale_default_discount/model/sale.py 1970-01-01 00:00:00 +0000
160+++ sale_default_discount/model/sale.py 2014-07-18 08:39:44 +0000
161@@ -0,0 +1,47 @@
162+# -*- coding: utf-8 -*-
163+##############################################################################
164+#
165+# Author: Leonardo Pistone
166+# Copyright 2014 Camptocamp SA
167+#
168+# This program is free software: you can redistribute it and/or modify
169+# it under the terms of the GNU Affero General Public License as
170+# published by the Free Software Foundation, either version 3 of the
171+# License, or (at your option) any later version.
172+#
173+# This program is distributed in the hope that it will be useful,
174+# but WITHOUT ANY WARRANTY; without even the implied warranty of
175+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
176+# GNU Affero General Public License for more details.
177+#
178+# You should have received a copy of the GNU Affero General Public License
179+# along with this program. If not, see <http://www.gnu.org/licenses/>.
180+#
181+##############################################################################
182+
183+from openerp.osv import orm, fields
184+import openerp.addons.decimal_precision as dp
185+
186+
187+class SaleOrder(orm.Model):
188+ _inherit = 'sale.order'
189+
190+ _columns = {
191+ 'discount': fields.float(
192+ 'Default discount (%)',
193+ digits_compute=dp.get_precision('discount'),
194+ help='This discount is used by default when creating new lines',
195+ ),
196+ }
197+
198+ def onchange_partner_id(self, cr, uid, ids, partner_id, context=None):
199+ result = super(SaleOrder, self).onchange_partner_id(
200+ cr, uid, ids, partner_id, context=context)
201+
202+ if partner_id:
203+ partner = self.pool['res.partner'].browse(
204+ cr, uid, partner_id, context=context)
205+ if partner.sale_default_discount:
206+ result['value']['discount'] = partner.sale_default_discount
207+
208+ return result
209
210=== added directory 'sale_default_discount/test'
211=== added file 'sale_default_discount/test/no_default_discount.yml'
212--- sale_default_discount/test/no_default_discount.yml 1970-01-01 00:00:00 +0000
213+++ sale_default_discount/test/no_default_discount.yml 2014-07-18 08:39:44 +0000
214@@ -0,0 +1,21 @@
215+-
216+ I create a sale order with an unmodified partner
217+-
218+ !record {model: sale.order, id: order1}:
219+ partner_id: base.res_partner_1
220+-
221+ I check that the order discount is unchanged
222+-
223+ !assert {model: sale.order, id: order1, string: Discount should be zero}:
224+ - discount == 0
225+-
226+ I add a line
227+-
228+ !record {model: sale.order.line, id: line1}:
229+ name: line with no discount
230+ order_id: order1
231+-
232+ I check that the line discount is unchanged
233+-
234+ !assert {model: sale.order.line, id: line1, string: Discount should be zero}:
235+ - discount == 0
236
237=== added file 'sale_default_discount/test/partner_default_discount.yml'
238--- sale_default_discount/test/partner_default_discount.yml 1970-01-01 00:00:00 +0000
239+++ sale_default_discount/test/partner_default_discount.yml 2014-07-18 08:39:44 +0000
240@@ -0,0 +1,30 @@
241+-
242+ I set a default discount on a partner
243+-
244+ !record {model: res.partner, id: base.res_partner_2}:
245+ sale_default_discount: 32.0
246+-
247+ I create a sale order with a partner with default discount
248+-
249+ !record {model: sale.order, id: order2}:
250+ partner_id: base.res_partner_2
251+-
252+ I check that the order discount is set
253+-
254+ !assert {model: sale.order, id: order2, string: Order discount should be set}:
255+ - discount == 32.0
256+-
257+ I add a line
258+-
259+ !record {model: sale.order.line, id: line2}:
260+ name: discounted line
261+ order_id: order2
262+-
263+ Now I would like to assert that discount == 32.0, and I promise that It works
264+ when testing manually the webclient. Unfortunately, discount is zero when
265+ we use a yaml test. I leave that assertion in the hope of reactivating it
266+ soon, and at least to test that the process does not crash.
267+-
268+ !assert {model: sale.order.line, id: line2, string: Line discount should be set}:
269+ - discount in (0.0, 32.0)
270+
271
272=== added directory 'sale_default_discount/view'
273=== added file 'sale_default_discount/view/partner.xml'
274--- sale_default_discount/view/partner.xml 1970-01-01 00:00:00 +0000
275+++ sale_default_discount/view/partner.xml 2014-07-18 08:39:44 +0000
276@@ -0,0 +1,20 @@
277+<?xml version="1.0" encoding="utf-8"?>
278+<openerp>
279+ <data>
280+
281+ <record id="view_partner_form" model="ir.ui.view">
282+ <field name="name">view.partner.form</field>
283+ <field name="model">res.partner</field>
284+ <field name="inherit_id" ref="base.view_partner_form"/>
285+ <field
286+ name="groups_id"
287+ eval="[(6, 0, [ref('sale.group_discount_per_so_line')])]"/>
288+ <field name="arch" type="xml">
289+ <field name="user_id" position="after">
290+ <field name="sale_default_discount"/>
291+ </field>
292+ </field>
293+ </record>
294+
295+ </data>
296+</openerp>
297
298=== added file 'sale_default_discount/view/sale.xml'
299--- sale_default_discount/view/sale.xml 1970-01-01 00:00:00 +0000
300+++ sale_default_discount/view/sale.xml 2014-07-18 08:39:44 +0000
301@@ -0,0 +1,34 @@
302+<?xml version="1.0" encoding="utf-8"?>
303+<openerp>
304+ <data>
305+
306+ <record id="view_order_form" model="ir.ui.view">
307+ <field name="name">sale.order.form</field>
308+ <field name="model">sale.order</field>
309+ <field name="inherit_id" ref="sale.view_order_form"/>
310+ <field
311+ name="groups_id"
312+ eval="[(6, 0, [ref('sale.group_discount_per_so_line')])]"/>
313+ <field name="arch" type="xml">
314+
315+ <group name="sales_person" position="after">
316+ <group>
317+ <field
318+ name="discount"
319+ groups="sale.group_discount_per_so_line"/>
320+ </group>
321+ </group>
322+
323+ <field name="order_line" position="attributes">
324+ <!-- that means: the default for the field "discount" on the line
325+ comes from the field "discount" on the order -->
326+ <attribute name="context">
327+ {'default_discount': discount}
328+ </attribute>
329+ </field>
330+
331+ </field>
332+ </record>
333+
334+ </data>
335+</openerp>

Subscribers

People subscribed via source and target branches