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

[David BEAL (ak)]

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

[Sébastien BEAU - http://www.akretion.com (sebastien.beau)]

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

[lp:~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl]

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

[FIX] boolean field evaluation

[David BEAL (ak) (davidbeal)]

@sebastien,

fix done, thank you for review

[Sébastien BEAU - http://www.akretion.com (sebastien.beau)]

LGTM

review: Approve (code review, no test)

[Pedro Manuel Baeza (pedro.baeza)]

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)

[lp:~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl]

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

[David BEAL (ak) (davidbeal)]

Migrate script works for me.

I think it could merge

Only miss Yannick review

[Florian da Costa (florian-dacosta)]

It seems to work fine with chronopost module
Thanks

review: Approve

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

Just one inline comment

Otherwise LGTM

review: Approve (code review, no tests)

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

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

[lp:~akretion-team/carriers-deliveries/7-split-default-option-state-from-deliv-method-dbl]

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

[FIX] remove readonly in view for code field

[David BEAL (ak) (davidbeal)]

l102 fixed, ready to merge

thanks to all

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

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

=== modified file 'base_delivery_carrier_label/__openerp__.py'
--- base_delivery_carrier_label/__openerp__.py	2014-03-28 13:19:50 +0000
+++ base_delivery_carrier_label/__openerp__.py	2014-06-25 10:17:19 +0000
@@ -19,7 +19,7 @@
 #
 ##############################################################################
 {'name': 'Base module for carrier labels',
- 'version': '1.1',
+ 'version': '1.2',
  'author': 'Camptocamp,Akretion',
  'maintainer': 'Camptocamp',
  'category': 'version',
 
=== modified file 'base_delivery_carrier_label/delivery.py'
--- base_delivery_carrier_label/delivery.py	2014-03-31 08:09:39 +0000
+++ base_delivery_carrier_label/delivery.py	2014-06-25 10:17:19 +0000
@@ -32,12 +32,15 @@
             'Partner Carrier'),
         'name': fields.char(
             'Name',
+            readonly=True,
             size=64),
         'code': fields.char(
             'Code',
+            readonly=True,
             size=64),
         'description': fields.char(
             'Description',
+            readonly=True,
             help="Allow to define a more complete description "
                  "than in the name field."),
     }
@@ -55,21 +58,26 @@
     _inherits = {'delivery.carrier.template.option': 'tmpl_option_id'}
 
     _columns = {
-        'state': fields.selection(
-            (('mandatory', 'Mandatory'),
-             ('default_option', 'Optional by Default'),
-             ('option', 'Optional'),
-             ),
-            string='Option Configuration',
-            help="Ensure you add and define correctly all your options or those won't "
-                 "be available for the packager\n"
-                 "- Mandatory: This option will be copied on carrier and cannot be removed\n"
-                 "- Optional by Default: This option will be copied but can be removed\n"
-                 "- Optional: This option can be added later by the user on the Delivery Order."),
+        'mandatory': fields.boolean(
+            'Mandatory',
+            help="If checked, this option is necessarily applied to the picking"),
+        'by_default': fields.boolean(
+            'Applied by Default',
+            help="By check, user can choose to apply this option "
+            "to each pickings\n using this delivery method"),
         'tmpl_option_id': fields.many2one(
             'delivery.carrier.template.option',
             string='Option', required=True, ondelete="cascade"),
         'carrier_id': fields.many2one('delivery.carrier', 'Carrier'),
+        'readonly_flag': fields.boolean(
+            'Readonly Flag',
+            help="When True, help to prevent the user to modify some fields "
+                 "option (if attribute is defined in the view)"),
+    }
+
+    _defaults = {
+        'readonly_flag': False,
+        'by_default': False,
     }
 
 
 
=== modified file 'base_delivery_carrier_label/delivery_view.xml'
--- base_delivery_carrier_label/delivery_view.xml	2014-03-12 17:15:06 +0000
+++ base_delivery_carrier_label/delivery_view.xml	2014-06-25 10:17:19 +0000
@@ -35,10 +35,15 @@
       <field name="model">delivery.carrier.option</field>
       <field name="arch" type="xml">
         <form string="delivery_carrier_option">
-          <field name="state"/>
-          <field name="tmpl_option_id"/>
-          <newline/>
-          <field name="description" colspan="4" readonly="True"/>
+          <group col="4">
+            <field name="mandatory" attrs="{'readonly': [('readonly_flag','=',True)]}"/>
+            <field name="by_default" attrs="{'invisible': [('mandatory', '=', True)]}"/>
+            <field name="readonly_flag" attrs="{'invisible': True}"/>
+            <field name="tmpl_option_id" colspan="4"
+                   attrs="{'readonly': [('readonly_flag','=',True)]}"/>
+            <newline/>
+            <field name="description" colspan="4" readonly="True"/>
+          </group>
         </form>
       </field>
     </record>
@@ -49,9 +54,9 @@
       <field name="type">tree</field>
       <field name="arch" type="xml">
         <tree string="delivery_carrier_option">
-          <field name="state" />
+          <field name="mandatory" />
           <field name="tmpl_option_id" />
-          <field name="code" readonly="1"/>
+          <field name="code"/>
         </tree>
       </field>
     </record>
 
=== added directory 'base_delivery_carrier_label/migrations'
=== added directory 'base_delivery_carrier_label/migrations/7.0.1.2'
=== added file 'base_delivery_carrier_label/migrations/7.0.1.2/pre-migration.py'
--- base_delivery_carrier_label/migrations/7.0.1.2/pre-migration.py	1970-01-01 00:00:00 +0000
+++ base_delivery_carrier_label/migrations/7.0.1.2/pre-migration.py	2014-06-25 10:17:19 +0000
@@ -0,0 +1,42 @@
+# -*- coding: utf-8 -*-
+##############################################################################
+#
+#    Author: David BEAL
+#    Copyright 2014 Akretion
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU Affero General Public License as
+#    published by the Free Software Foundation, either version 3 of the
+#    License, or (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU Affero General Public License for more details.
+#
+#    You should have received a copy of the GNU Affero General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+##############################################################################
+
+
+def migrate(cr, version):
+    if not version:
+        return
+
+    queries = [
+        # build new fields
+        """ALTER TABLE delivery_carrier_option
+        ADD COLUMN mandatory BOOLEAN,
+        ADD COLUMN by_default BOOLEAN;""",
+        # move datas to new fields
+        """UPDATE delivery_carrier_option
+        SET mandatory = 't' WHERE state='mandatory';""",
+        """UPDATE delivery_carrier_option
+        SET by_default = 't' WHERE state='default_option';""",
+        # Delete old column
+        "ALTER TABLE delivery_carrier_option DROP COLUMN state;"
+    ]
+
+    for query in queries:
+        cr.execute(query)
 
=== modified file 'base_delivery_carrier_label/stock.py'
--- base_delivery_carrier_label/stock.py	2014-06-17 11:16:42 +0000
+++ base_delivery_carrier_label/stock.py	2014-06-25 10:17:19 +0000
@@ -163,7 +163,8 @@
             available_option_ids = []
             for available_option in carrier.available_option_ids:
                 available_option_ids.append(available_option.id)
-                if available_option.state in ['default_option', 'mandatory']:
+                if available_option.mandatory \
+                        or available_option.by_default:
                     default_option_ids.append(available_option.id)
             res = {
                 'value': {'carrier_type': carrier.type,
@@ -182,7 +183,7 @@
             return res
         carrier = carrier_obj.browse(cr, uid, carrier_id, context=context)
         for available_option in carrier.available_option_ids:
-            if (available_option.state == 'mandatory'
+            if (available_option.mandatory
                     and not available_option.id in option_ids[0][2]):
                 res['warning'] = {
                     'title': _('User Error !'),