Merge lp:~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl into lp:~stock-logistic-core-editors/carriers-deliveries/7.0

Proposed by David BEAL (ak)
Status: Needs review
Proposed branch: lp:~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl
Merge into: lp:~stock-logistic-core-editors/carriers-deliveries/7.0
Diff against target: 178 lines (+76/-20)
5 files modified
base_delivery_carrier_label/__openerp__.py (+1/-1)
base_delivery_carrier_label/delivery.py (+19/-11)
base_delivery_carrier_label/delivery_view.xml (+11/-6)
base_delivery_carrier_label/migrations/7.0.1.2/pre-migration.py (+42/-0)
base_delivery_carrier_label/stock.py (+3/-2)
To merge this branch: bzr merge lp:~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl
Reviewer Review Type Date Requested Status
Yannick Vaucher @ Camptocamp Approve
Florian da Costa (community) Approve
Pedro Manuel Baeza code review Approve
Sébastien BEAU - http://www.akretion.com code review, no test Approve
Review via email: mp+224027@code.launchpad.net

Description of the change

Improve : split state field in two boolean fields with the same feature

To post a comment you must log in.
Revision history for this message
Sébastien BEAU - http://www.akretion.com (sebastien.beau) wrote :

Hi
You don't have to test "if available_option.mandatory is True" but you can only use "if available_option.mandatory" as it's a boolean the "is True" is useless.

review: Needs Fixing
53. By David BEAL (ak) on 2014-06-23

[FIX] boolean field evaluation

Revision history for this message
David BEAL (ak) (davidbeal) wrote :

@sebastien,

fix done, thank you for review

Revision history for this message
Sébastien BEAU - http://www.akretion.com (sebastien.beau) wrote :

LGTM

review: Approve (code review, no test)
Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

Hi, David, thanks for the improvement.

You have to change module version number and provide a proper migration script for those that have previously the module installed.

Regards.

review: Approve (code review)
54. By David BEAL (ak) on 2014-06-23

[FIX] add migration script

55. By David BEAL (ak) on 2014-06-23

[FIX] module version

Revision history for this message
David BEAL (ak) (davidbeal) wrote :

Migrate script works for me.

I think it could merge

Only miss Yannick review

Revision history for this message
Florian da Costa (florian-dacosta) wrote :

It seems to work fine with chronopost module
Thanks

review: Approve
Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

Just one inline comment

Otherwise LGTM

review: Approve (code review, no tests)
Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

Ok failed my inline comment

l.102 <field name="code" readonly="1"/>

You can remove readonly here as you setted it on model

review: Approve
56. By David BEAL (ak) on 2014-06-25

[FIX] remove readonly in view for code field

Revision history for this message
David BEAL (ak) (davidbeal) wrote :

l102 fixed, ready to merge

thanks to all

Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

This project is now hosted on https://github.com/OCA/carriers-deliveries. Please move your proposal there if you still want to merge it once fixed. This guide may help you https://github.com/OCA/maintainers-tools/wiki/How-to-move-a-Merge-Proposal-to-GitHub

(sorry David should have merged it first at least this will make you some training to create PR on github ;) )

Unmerged revisions

56. By David BEAL (ak) on 2014-06-25

[FIX] remove readonly in view for code field

55. By David BEAL (ak) on 2014-06-23

[FIX] module version

54. By David BEAL (ak) on 2014-06-23

[FIX] add migration script

53. By David BEAL (ak) on 2014-06-23

[FIX] boolean field evaluation

52. By David BEAL (ak) on 2014-06-21

[FIX] split state field in two boolean field with the same feature

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'base_delivery_carrier_label/__openerp__.py'
2--- base_delivery_carrier_label/__openerp__.py 2014-03-28 13:19:50 +0000
3+++ base_delivery_carrier_label/__openerp__.py 2014-06-25 10:17:19 +0000
4@@ -19,7 +19,7 @@
5 #
6 ##############################################################################
7 {'name': 'Base module for carrier labels',
8- 'version': '1.1',
9+ 'version': '1.2',
10 'author': 'Camptocamp,Akretion',
11 'maintainer': 'Camptocamp',
12 'category': 'version',
13
14=== modified file 'base_delivery_carrier_label/delivery.py'
15--- base_delivery_carrier_label/delivery.py 2014-03-31 08:09:39 +0000
16+++ base_delivery_carrier_label/delivery.py 2014-06-25 10:17:19 +0000
17@@ -32,12 +32,15 @@
18 'Partner Carrier'),
19 'name': fields.char(
20 'Name',
21+ readonly=True,
22 size=64),
23 'code': fields.char(
24 'Code',
25+ readonly=True,
26 size=64),
27 'description': fields.char(
28 'Description',
29+ readonly=True,
30 help="Allow to define a more complete description "
31 "than in the name field."),
32 }
33@@ -55,21 +58,26 @@
34 _inherits = {'delivery.carrier.template.option': 'tmpl_option_id'}
35
36 _columns = {
37- 'state': fields.selection(
38- (('mandatory', 'Mandatory'),
39- ('default_option', 'Optional by Default'),
40- ('option', 'Optional'),
41- ),
42- string='Option Configuration',
43- help="Ensure you add and define correctly all your options or those won't "
44- "be available for the packager\n"
45- "- Mandatory: This option will be copied on carrier and cannot be removed\n"
46- "- Optional by Default: This option will be copied but can be removed\n"
47- "- Optional: This option can be added later by the user on the Delivery Order."),
48+ 'mandatory': fields.boolean(
49+ 'Mandatory',
50+ help="If checked, this option is necessarily applied to the picking"),
51+ 'by_default': fields.boolean(
52+ 'Applied by Default',
53+ help="By check, user can choose to apply this option "
54+ "to each pickings\n using this delivery method"),
55 'tmpl_option_id': fields.many2one(
56 'delivery.carrier.template.option',
57 string='Option', required=True, ondelete="cascade"),
58 'carrier_id': fields.many2one('delivery.carrier', 'Carrier'),
59+ 'readonly_flag': fields.boolean(
60+ 'Readonly Flag',
61+ help="When True, help to prevent the user to modify some fields "
62+ "option (if attribute is defined in the view)"),
63+ }
64+
65+ _defaults = {
66+ 'readonly_flag': False,
67+ 'by_default': False,
68 }
69
70
71
72=== modified file 'base_delivery_carrier_label/delivery_view.xml'
73--- base_delivery_carrier_label/delivery_view.xml 2014-03-12 17:15:06 +0000
74+++ base_delivery_carrier_label/delivery_view.xml 2014-06-25 10:17:19 +0000
75@@ -35,10 +35,15 @@
76 <field name="model">delivery.carrier.option</field>
77 <field name="arch" type="xml">
78 <form string="delivery_carrier_option">
79- <field name="state"/>
80- <field name="tmpl_option_id"/>
81- <newline/>
82- <field name="description" colspan="4" readonly="True"/>
83+ <group col="4">
84+ <field name="mandatory" attrs="{'readonly': [('readonly_flag','=',True)]}"/>
85+ <field name="by_default" attrs="{'invisible': [('mandatory', '=', True)]}"/>
86+ <field name="readonly_flag" attrs="{'invisible': True}"/>
87+ <field name="tmpl_option_id" colspan="4"
88+ attrs="{'readonly': [('readonly_flag','=',True)]}"/>
89+ <newline/>
90+ <field name="description" colspan="4" readonly="True"/>
91+ </group>
92 </form>
93 </field>
94 </record>
95@@ -49,9 +54,9 @@
96 <field name="type">tree</field>
97 <field name="arch" type="xml">
98 <tree string="delivery_carrier_option">
99- <field name="state" />
100+ <field name="mandatory" />
101 <field name="tmpl_option_id" />
102- <field name="code" readonly="1"/>
103+ <field name="code"/>
104 </tree>
105 </field>
106 </record>
107
108=== added directory 'base_delivery_carrier_label/migrations'
109=== added directory 'base_delivery_carrier_label/migrations/7.0.1.2'
110=== added file 'base_delivery_carrier_label/migrations/7.0.1.2/pre-migration.py'
111--- base_delivery_carrier_label/migrations/7.0.1.2/pre-migration.py 1970-01-01 00:00:00 +0000
112+++ base_delivery_carrier_label/migrations/7.0.1.2/pre-migration.py 2014-06-25 10:17:19 +0000
113@@ -0,0 +1,42 @@
114+# -*- coding: utf-8 -*-
115+##############################################################################
116+#
117+# Author: David BEAL
118+# Copyright 2014 Akretion
119+#
120+# This program is free software: you can redistribute it and/or modify
121+# it under the terms of the GNU Affero General Public License as
122+# published by the Free Software Foundation, either version 3 of the
123+# License, or (at your option) any later version.
124+#
125+# This program is distributed in the hope that it will be useful,
126+# but WITHOUT ANY WARRANTY; without even the implied warranty of
127+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
128+# GNU Affero General Public License for more details.
129+#
130+# You should have received a copy of the GNU Affero General Public License
131+# along with this program. If not, see <http://www.gnu.org/licenses/>.
132+#
133+##############################################################################
134+
135+
136+def migrate(cr, version):
137+ if not version:
138+ return
139+
140+ queries = [
141+ # build new fields
142+ """ALTER TABLE delivery_carrier_option
143+ ADD COLUMN mandatory BOOLEAN,
144+ ADD COLUMN by_default BOOLEAN;""",
145+ # move datas to new fields
146+ """UPDATE delivery_carrier_option
147+ SET mandatory = 't' WHERE state='mandatory';""",
148+ """UPDATE delivery_carrier_option
149+ SET by_default = 't' WHERE state='default_option';""",
150+ # Delete old column
151+ "ALTER TABLE delivery_carrier_option DROP COLUMN state;"
152+ ]
153+
154+ for query in queries:
155+ cr.execute(query)
156
157=== modified file 'base_delivery_carrier_label/stock.py'
158--- base_delivery_carrier_label/stock.py 2014-06-17 11:16:42 +0000
159+++ base_delivery_carrier_label/stock.py 2014-06-25 10:17:19 +0000
160@@ -163,7 +163,8 @@
161 available_option_ids = []
162 for available_option in carrier.available_option_ids:
163 available_option_ids.append(available_option.id)
164- if available_option.state in ['default_option', 'mandatory']:
165+ if available_option.mandatory \
166+ or available_option.by_default:
167 default_option_ids.append(available_option.id)
168 res = {
169 'value': {'carrier_type': carrier.type,
170@@ -182,7 +183,7 @@
171 return res
172 carrier = carrier_obj.browse(cr, uid, carrier_id, context=context)
173 for available_option in carrier.available_option_ids:
174- if (available_option.state == 'mandatory'
175+ if (available_option.mandatory
176 and not available_option.id in option_ids[0][2]):
177 res['warning'] = {
178 'title': _('User Error !'),

Subscribers

People subscribed via source and target branches