Merge lp:~camptocamp/stock-logistic-flows/7.0-add_stock_picking_priority-afe into lp:stock-logistic-flows

[Alexandre Fayolle - camptocamp]

[ADD] stock_pricking_priority

Add a priority attribute to pickings.

This priority can be changed after the picking is confirmed and a
wizard can be run to recompute the availability of pickings
depending on the new priorities.

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

__openerp__: pep8 issues
 * spaces before the colon on l.56,57,58,59,66,67,68,71

picking_priority.py:
 * l.123 you're not checking if ids is an int or a list, this could lead to a problem with search and an error on l.137

review: Needs Fixing (code review)

[lp:~camptocamp/stock-logistic-flows/7.0-add_stock_picking_priority-afe]

42. By Alexandre Fayolle - camptocamp on 2013-12-11

[FIX] code formatting issue

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

> __openerp__: pep8 issues
> * spaces before the colon on l.56,57,58,59,66,67,68,71

Fixed, thanks.

> picking_priority.py:
> * l.123 you're not checking if ids is an int or a list, this could lead to a
> problem with search and an error on l.137

I respectfully disagree. The name of the parameter implies that expected type is a list. Adding such tests only clutters the code imo. Calling the function with a single integer would be a bug in the calling function.

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

> > picking_priority.py:
> > * l.123 you're not checking if ids is an int or a list, this could lead to
> a
> > problem with search and an error on l.137
>
> I respectfully disagree. The name of the parameter implies that expected type
> is a list. Adding such tests only clutters the code imo. Calling the function
> with a single integer would be a bug in the calling function.

I agree with Alexandre. Only some methods of the ORM (write, unlink, browse, name_get, perm_read) do accept both, which is a regretted choice today AFAIK, but can no longer be changed to keep the backward compatibility. But all the other methods do not have to support boths.

LGTM, thanks

review: Approve (code review)

[lp:~camptocamp/stock-logistic-flows/7.0-add_stock_picking_priority-afe]

43. By Nicolas Bessi - Camptocamp on 2013-12-11

[PEP8]

44. By Nicolas Bessi - Camptocamp on 2013-12-11

[RM] unused variable

[Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount)]

Hello,

I agree with Alexandre and Guewen.

I generally do such tests in opposite situation,
when a public function is waiting for one id list,in and where nested form can do the call to a button, on change, webclient is not always consistent...

I fixed an indentation and remove unused _columns

LGTM

review: Approve (no test, code review)

[Brendan Clune (Logic Supply) (brendan-clune)]

Code looks good and we have done basic testing on our development environment.

review: Approve

[Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives)]

Technically,
The selection values ("[('0', 'Normal'),('1', 'Urgent'),('2', 'Very Urgent')]") could be usefully factored into a constant.
That would allow others to create modules to add other priorities.
Also, would a priority of "-1" be correctly placed after all positive priorities?

Functionally, it would be useful push this priority up to the sale orders too (but maybe that would belong to another module?)

Other than that it seems to work fine and it rocks.

review: Approve (quick code review and testing)

[Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives)]

review: Needs Fixing (quick code review and testing)

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

On 01/24/2014 04:09 PM, Lionel Sausin - Numérigraphe wrote:
> Review: Approve quick code review and testing
>
> Technically,
> The selection values ("[('0', 'Normal'),('1', 'Urgent'),('2', 'Very Urgent')]") could be usefully factored into a constant.
> That would allow others to create modules to add other priorities.
> Also, would a priority of "-1" be correctly placed after all positive priorities?
>
> Functionally, it would be useful push this priority up to the sale orders too (but maybe that would belong to another module?)
>
> Other than that it seems to work fine and it rocks.
>

This MP has been merged already. The changes you propose could be done
in a new proposal. I agree for the sales, in a new module to not add a
dependency on 'sale' on this module.

--
Guewen Baconnier
Business Solutions Software Developer

Camptocamp SA
PSE A, CH-1015 Lausanne
Phone: [phone]
Office: [phone]
http://www.camptocamp.com/

[Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives)]

Oh, I really thought I found it in the list of non-merged branches, but it's the end of the week and probably I should get a rest.
Sorry about the noise!

=== added directory 'stock_picking_priority'
=== added file 'stock_picking_priority/__init__.py'
--- stock_picking_priority/__init__.py	1970-01-01 00:00:00 +0000
+++ stock_picking_priority/__init__.py	2013-12-11 07:58:44 +0000
@@ -0,0 +1,22 @@
+# -*- coding: utf-8 -*-
+##############################################################################
+#
+#    Author: Alexandre Fayolle
+#    Copyright 2013 Camptocamp SA
+#
+#    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/>.
+#
+##############################################################################
+
+from . import picking_priority

=== added file 'stock_picking_priority/__openerp__.py'
--- stock_picking_priority/__openerp__.py	1970-01-01 00:00:00 +0000
+++ stock_picking_priority/__openerp__.py	2013-12-11 07:58:44 +0000
@@ -0,0 +1,44 @@
+# -*- coding: utf-8 -*-
+##############################################################################
+#
+#    Author: Alexandre Fayolle
+#    Copyright 2013 Camptocamp SA
+#
+#    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/>.
+#
+##############################################################################
+
+
+{
+    "name": "Picking priority",
+    "version": "0.1",
+    "depends": ['stock'],
+    "author": "Camptocamp",
+    'license': 'AGPL-3',
+    "description": """Add a priority attribute to pickings.
+
+    This priority can be changed after the picking is confirmed and a
+    wizard can be run to recompute the availability of pickings
+    depending on the new priorities.""",
+    "website": "http://www.camptocamp.com",
+    "category": "Warehouse management",
+    "demo": [
+        'picking_priority_demo.yml',
+        ],
+    "data": ['picking_priority_view.xml',
+             ],
+    "test": ['tests/test_picking_priority.yml',
+             ],
+    "installable": True,
+}

=== added file 'stock_picking_priority/picking_priority.py'
--- stock_picking_priority/picking_priority.py	1970-01-01 00:00:00 +0000
+++ stock_picking_priority/picking_priority.py	2013-12-11 07:58:44 +0000
@@ -0,0 +1,121 @@
+# -*- coding: utf-8 -*-
+##############################################################################
+#
+#    Author: Alexandre Fayolle
+#    Copyright 2013 Camptocamp SA
+#
+#    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/>.
+#
+#############################################################################
+import logging
+
+from openerp.osv import orm, osv, fields
+from openerp.tools.translate import _
+_logger = logging.getLogger(__name__)
+
+
+class StockPicking(orm.Model):
+    _inherit = "stock.picking"
+    _columns = {
+        'priority': fields.selection([('0', 'Normal'),
+                                      ('1', 'Urgent'),
+                                      ('2', 'Very Urgent')],
+                                     'Priority',
+                                     required=True,
+                                     help='The priority of the picking'),
+        }
+    _defaults = {
+        'priority': '0',
+        }
+
+    def retry_assign_all(self, cr, uid, ids, context=None):
+        domain = [('type', '!=', 'in'),
+                  ('move_lines', '!=', []),
+                  ('state', 'in', ('confirmed', 'assigned'))]
+        if ids:
+            domain += [('ids', 'in', ids)]
+        picking_ids = self.search(cr, uid, domain,
+                                  order='priority desc, min_date',
+                                  context=context)
+        _logger.info('cancelling pickings')
+        cancelled_ids = self.cancel_assign(cr, uid, picking_ids, context)
+        assigned_ids = []
+        errors = []
+        _logger.info('reassigning pickings')
+        for picking_id in picking_ids:
+            try:
+                assigned_id = self.action_assign(cr, uid, [picking_id],
+                                                 context)
+                assigned_ids.append(assigned_id)
+            except osv.except_osv, exc:
+                name = self.read(cr, uid, picking_id, ['name'],
+                                 context=context)['name']
+                errors.append(u'%s: %s' % (name, exc.args[-1]))
+                _logger.info('error in action_assign for picking %s: %s'
+                             % (name, exc))
+        if errors:
+            message = '\n'.join(errors)
+            raise orm.except_orm(_(u'Warning'),
+                                 _(u'No operations validated due '
+                                   'to the following errors:\n%s') % message)
+        return cancelled_ids, assigned_ids
+
+
+class StockPickingOut(orm.Model):
+    _inherit = 'stock.picking.out'
+    _columns = {
+        'priority': fields.selection([('0', 'Normal'),
+                                      ('1', 'Urgent'),
+                                      ('2', 'Very Urgent')],
+                                     'Priority',
+                                     required=True,
+                                     help='The priority of the picking'),
+        }
+    _defaults = {
+        'priority': '0',
+        }
+
+    def retry_assign_all(self, cr, uid, ids, context=None):
+        return self.pool.get('stock.picking').retry_assign_all(cr, uid, ids,
+                                                               context=context)
+
+
+class StockPickingIn(orm.Model):
+    _inherit = 'stock.picking.in'
+    _columns = {
+        'priority': fields.selection([('0', 'Normal'),
+                                      ('1', 'Urgent'),
+                                      ('2', 'Very Urgent')],
+                                     'Priority',
+                                     required=True,
+                                     help='The priority of the picking'),
+        }
+    _defaults = {
+        'priority': '0',
+        }
+
+    def retry_assign_all(self, cr, uid, ids, context=None):
+        return self.pool.get('stock.picking').retry_assign_all(cr, uid, ids,
+                                                               context=context)
+
+
+class StockPickingRetryAvailability(orm.TransientModel):
+    _name = "stock.picking.retry.availability"
+
+    def action_retry_assign(self, cr, uid, ids, context=None):
+        pick_obj = self.pool['stock.picking']
+
+        pick_obj.retry_assign_all(cr, uid, [], context=context)
+
+        return {'type': 'ir.actions.act_window_close'}

=== added file 'stock_picking_priority/picking_priority_demo.yml'
--- stock_picking_priority/picking_priority_demo.yml	1970-01-01 00:00:00 +0000
+++ stock_picking_priority/picking_priority_demo.yml	2013-12-11 07:58:44 +0000
@@ -0,0 +1,81 @@
+-
+ Create 3 pickings and procurements for 3 M-Opt on shelf 1 due Dec 10, 1st and 20
+ M-Opt (product.product_product_10)
+ initial stock is 8 units in shelf 1 (stock.stock_location_components)
+-
+ !record {model: stock.picking.out, id: ship_mopt_1}:
+   name: mopt1_1
+   move_lines:
+     - product_id: product.product_product_10
+       product_qty: 3
+       product_uom: product.product_uom_unit
+       location_id: stock.stock_location_components
+       location_dest_id: stock.stock_location_output
+       date: 2013-12-10
+       date_expected: 2013-12-10
+       procurements:
+         - name: mopt1_1
+           origin: test
+           date_planned: 2013-12-10
+           product_id: product.product_product_10
+           product_qty: 3
+           product_uom: product.product_uom_unit
+           location_id: stock.stock_location_components
+           procure_method: make_to_stock
+-
+ !record {model: stock.picking.out, id: ship_mopt_2}:
+   name: mopt1_2
+   move_lines:
+     - product_id: product.product_product_10
+       product_qty: 3
+       product_uom: product.product_uom_unit
+       location_id: stock.stock_location_components
+       location_dest_id: stock.stock_location_output
+       date: 2013-12-01
+       date_expected: 2013-12-01
+       procurements:
+         - name: mopt1_2
+           origin: test
+           date_planned: 2013-12-01
+           product_id: product.product_product_10
+           product_qty: 3
+           product_uom: product.product_uom_unit
+           location_id: stock.stock_location_components
+           procure_method: make_to_stock
+-
+ !record {model: stock.picking.out, id: ship_mopt_3}:
+   name: mopt1_3
+   move_lines:
+     - product_id: product.product_product_10
+       product_qty: 3
+       product_uom: product.product_uom_unit
+       location_id: stock.stock_location_components
+       location_dest_id: stock.stock_location_output
+       date: 2013-12-20
+       date_expected: 2013-12-20
+       procurements:
+         - name: mopt1_3
+           origin: test
+           date_planned: 2013-12-20
+           product_id: product.product_product_10
+           product_qty: 3
+           product_uom: product.product_uom_unit
+           location_id: stock.stock_location_components
+           procure_method: make_to_stock
+-
+ Confirm the 3 pickings
+-
+  !workflow {model: stock.picking, action: button_confirm, ref: ship_mopt_1}
+-
+  !workflow {model: stock.picking, action: button_confirm, ref: ship_mopt_2}
+-
+  !workflow {model: stock.picking, action: button_confirm, ref: ship_mopt_3}
+-
+ Confirm the procurements
+-
+ !python {model: procurement.order}: |
+   procurement_ids = self.search(cr, uid, [('name', 'in', ('mopt1_1', 'mopt1_2', 'mopt1_3'))], context=context)
+   from openerp import netsvc
+   wf_service = netsvc.LocalService("workflow")
+   for id in procurement_ids:
+     wf_service.trg_validate(uid, 'procurement.order', id, 'button_confirm', cr)

=== added file 'stock_picking_priority/picking_priority_view.xml'
--- stock_picking_priority/picking_priority_view.xml	1970-01-01 00:00:00 +0000
+++ stock_picking_priority/picking_priority_view.xml	2013-12-11 07:58:44 +0000
@@ -0,0 +1,64 @@
+<?xml version="1.0" encoding="utf-8"?>
+<openerp>
+  <data>
+    <record model="ir.ui.view" id="stock_picking_form">
+      <field name="name">stock.picking.form priority</field>
+      <field name="model">stock.picking</field>
+      <field name="inherit_id" ref="stock.view_picking_form"/>
+      <field name="arch" type="xml">
+        <field name="date_done" position="after">
+          <field name="priority"/>
+        </field>
+      </field>
+    </record>
+    <record model="ir.ui.view" id="stock_picking_out_form">
+      <field name="name">stock.picking.out.form priority</field>
+      <field name="model">stock.picking.out</field>
+      <field name="inherit_id" ref="stock.view_picking_form"/>
+      <field name="arch" type="xml">
+        <field name="date_done" position="after">
+          <field name="priority"/>
+        </field>
+      </field>
+    </record>
+
+    <act_window id="action_retry_assign"
+                name="Retry picking assignment"
+                res_model="stock.picking.retry.availability"
+                view_type="form"
+                view_mode="form"
+                target="new"
+                />
+
+    <menuitem id="menu_retry_availability"
+              name="Recompute picking availability"
+              parent="procurement.menu_stock_sched"
+              sequence="40"
+              action="action_retry_assign"
+              groups="stock.group_stock_user,stock.group_stock_manager"
+              />
+
+    <record model="ir.ui.view" id="picking_retry_availability_form">
+      <field name="name">stock.picking.retry.availability form</field>
+      <field name="model">stock.picking.retry.availability</field>
+      <field name="arch" type="xml">
+        <form string="Recompute picking availability" version="7.0">
+          <sheet>
+            <p class="oe_grey">
+              This action will recompute the picking availability based on their
+              priority. Before running it, make sure that you have changed the
+              priority of at least one picking.
+            </p>
+          </sheet>
+          <footer>
+            <button name="action_retry_assign" string="Recompute availability" type="object"
+                    class="oe_highlight"/>
+            or
+            <button string="Cancel" class="oe_link" special="cancel" />
+          </footer>
+        </form>
+      </field>
+    </record>
+
+  </data>
+</openerp>

=== added directory 'stock_picking_priority/tests'
=== added file 'stock_picking_priority/tests/test_picking_priority.yml'
--- stock_picking_priority/tests/test_picking_priority.yml	1970-01-01 00:00:00 +0000
+++ stock_picking_priority/tests/test_picking_priority.yml	2013-12-11 07:58:44 +0000
@@ -0,0 +1,136 @@
+-
+ Create 3 pickings and procurements for 10 M-Las on shelf 1 due Dec 10, 1st and 20
+ M-Las (product.product_product_11)
+ initial stock is 26 units in shelf 1 (stock.stock_location_components)
+-
+ !record {model: stock.picking.out, id: ship_mlas_1}:
+   name: mlas1_1
+   move_lines:
+     - product_id: product.product_product_11
+       product_qty: 10
+       product_uom: product.product_uom_unit
+       location_id: stock.stock_location_components
+       location_dest_id: stock.stock_location_output
+       date: 2013-12-20
+       date_expected: 2013-12-20
+       procurements:
+         - name: mlas1_1
+           origin: test
+           date_planned: 2013-12-20
+           product_id: product.product_product_11
+           product_qty: 10
+           product_uom: product.product_uom_unit
+           location_id: stock.stock_location_components
+           procure_method: make_to_stock
+-
+ !record {model: stock.picking.out, id: ship_mlas_2}:
+   name: mlas1_2
+   move_lines:
+     - product_id: product.product_product_11
+       product_qty: 10
+       product_uom: product.product_uom_unit
+       location_id: stock.stock_location_components
+       location_dest_id: stock.stock_location_output
+       date: 2013-12-01
+       date_expected: 2013-12-01
+       procurements:
+         - name: mlas1_2
+           origin: test
+           date_planned: 2013-12-01
+           product_id: product.product_product_11
+           product_qty: 10
+           product_uom: product.product_uom_unit
+           location_id: stock.stock_location_components
+           procure_method: make_to_stock
+-
+ !record {model: stock.picking.out, id: ship_mlas_3}:
+   name: mlas1_3
+   move_lines:
+     - product_id: product.product_product_11
+       product_qty: 10
+       product_uom: product.product_uom_unit
+       location_id: stock.stock_location_components
+       location_dest_id: stock.stock_location_output
+       date: 2013-12-10
+       date_expected: 2013-12-10
+       procurements:
+         - name: mlas1_3
+           origin: test
+           date_planned: 2013-12-10
+           product_id: product.product_product_11
+           product_qty: 10
+           product_uom: product.product_uom_unit
+           location_id: stock.stock_location_components
+           procure_method: make_to_stock
+-
+ Confirm the 3 pickings
+-
+  !workflow {model: stock.picking, action: button_confirm, ref: ship_mlas_1}
+-
+  !workflow {model: stock.picking, action: button_confirm, ref: ship_mlas_2}
+-
+  !workflow {model: stock.picking, action: button_confirm, ref: ship_mlas_3}
+-
+ Confirm the procurements
+-
+ !python {model: procurement.order}: |
+   procurement_ids = self.search(cr, uid, [('name', 'in', ('mlas1_1', 'mlas1_2', 'mlas1_3'))], context=context)
+   from openerp import netsvc
+   wf_service = netsvc.LocalService("workflow")
+   for id in procurement_ids:
+     wf_service.trg_validate(uid, 'procurement.order', id, 'button_confirm', cr)
+-
+  I run the scheduler.
+-
+  !python {model: procurement.order}: |
+    self.run_scheduler(cr, uid)
+-
+ Check the procurement states
+-
+ !python {model: procurement.order}: |
+   procurement_ids = self.search(cr, uid, [('name', 'in', ('mlas1_1', 'mlas1_2', 'mlas1_3'))], context=context)
+   states = {}
+   for proc in self.browse(cr, uid, procurement_ids, context=context):
+       states[proc.name] = proc.state
+   expected = {'mlas1_1': 'exception', 'mlas1_2': 'ready', 'mlas1_3': 'ready'}
+   assert states == expected, 'wrong procurement states: %s (expected %s)' % (states, expected)
+-
+ Check the picking states
+-
+ !python {model: stock.picking}: |
+   picking_ids = [ref("ship_mlas_1"), ref("ship_mlas_2"), ref("ship_mlas_3")]
+   states = {}
+   for pick in self.browse(cr, uid, picking_ids, context=context):
+       states[pick.id] = pick.state
+   expected = {ref("ship_mlas_1"): 'confirmed', ref("ship_mlas_2"): 'assigned', ref("ship_mlas_3"): 'assigned'}
+   assert states == expected, 'wrong picking states %s (expected: %s)' % (states, expected)
+-
+ I set the priority of picking ship_mlas_1 to Urgent
+-
+ !python {model: stock.picking.out}: |
+   self.write(cr, uid, ref("ship_mlas_1"), {'priority': '1'}, context=context)
+-
+ I retry the assignments
+-
+ !python {model: stock.picking.out}: |
+   self.retry_assign_all(cr, uid, [], context=context)
+-
+ Check the procurement states
+-
+ !python {model: procurement.order}: |
+   procurement_ids = self.search(cr, uid, [('name', 'in', ('mlas1_1', 'mlas1_2', 'mlas1_3'))], context=context)
+   states = {}
+   for proc in self.browse(cr, uid, procurement_ids, context=context):
+       states[proc.name] = proc.state
+   expected = {'mlas1_1': 'exception', 'mlas1_2': 'ready', 'mlas1_3': 'ready'}
+   assert states == expected, 'wrong procurement states: %s (expected %s)' % (states, expected)
+-
+ Check the picking states
+-
+ !python {model: stock.picking}: |
+   picking_ids = [ref("ship_mlas_1"), ref("ship_mlas_2"), ref("ship_mlas_3")]
+   states = {}
+   for pick in self.browse(cr, uid, picking_ids, context=context):
+       states[pick.id] = pick.state
+   expected = {ref("ship_mlas_1"): 'assigned', ref("ship_mlas_2"): 'assigned', ref("ship_mlas_3"): 'confirmed'}
+   assert states == expected, 'wrong picking states %s (expected: %s)' % (states, expected)