From da8664b7a2430af874d878b4c618036ed483f355 Mon Sep 17 00:00:00 2001 From: Alexis de Lattre Date: Thu, 2 Feb 2017 20:09:35 +0100 Subject: [PATCH 1/3] Add a test to show that the stock.quant.package.move wizard doesn't work Use reserved_quant_ids in stock.move instead of quant_ids Add prepare method for stock move creation and add origin field by default Use 'fields_list' instead of 'fields' argument in default_get to avoid confusion with "fields" from "from openerp import fields" Add visible button on form view of stock.quant and stock.quant.package Block selection of destination location = source location in wizard Rename 'Source Location' to 'Current Location' in wizard (and other string improvements) Use related field for source location instead of onchange Give access to button "move quant" on stock.quant even if not in group stock.group_tracking_lot (but keep it for the button on stock.quant.package) Display moved quants after execution of the stock.quants.move wizard --- .../__openerp__.py | 1 + .../models/stock.py | 36 +++++++++++-------- ...test_stock_quant_packages_moving_wizard.py | 30 ++++++++-------- .../views/stock_view.xml | 28 +++++++++++++++ .../wizard/quant_move_wizard.py | 20 ++++------- .../wizard/quant_move_wizard_view.xml | 17 ++++----- .../wizard/quant_packages_move_wizard.py | 22 +++++------- .../quant_packages_move_wizard_view.xml | 14 ++++---- .../wizard/quants_move_wizard.py | 36 ++++++++++--------- .../wizard/quants_move_wizard_view.xml | 15 ++++---- 10 files changed, 119 insertions(+), 100 deletions(-) create mode 100644 stock_quant_packages_moving_wizard/views/stock_view.xml diff --git a/stock_quant_packages_moving_wizard/__openerp__.py b/stock_quant_packages_moving_wizard/__openerp__.py index c9072857..3bf3000e 100644 --- a/stock_quant_packages_moving_wizard/__openerp__.py +++ b/stock_quant_packages_moving_wizard/__openerp__.py @@ -22,6 +22,7 @@ "wizard/quant_move_wizard_view.xml", "wizard/quants_move_wizard_view.xml", "wizard/quant_packages_move_wizard_view.xml", + "views/stock_view.xml", ], "installable": True, } diff --git a/stock_quant_packages_moving_wizard/models/stock.py b/stock_quant_packages_moving_wizard/models/stock.py index 10742a25..4dfd3049 100644 --- a/stock_quant_packages_moving_wizard/models/stock.py +++ b/stock_quant_packages_moving_wizard/models/stock.py @@ -1,19 +1,20 @@ # -*- coding: utf-8 -*- # License AGPL-3 - See http://www.gnu.org/licenses/agpl-3.0.html -from openerp import api, fields, models +from openerp import api, fields, models, _ from openerp.tools.float_utils import float_compare class StockQuant(models.Model): _inherit = 'stock.quant' - @api.one - def move_to(self, dest_location): - move_obj = self.with_context(quant_moving=True).env['stock.move'] - new_move = move_obj.create({ - 'name': 'Move %s to %s' % (self.product_id.name, - dest_location.name), + @api.multi + def _prepare_move_to(self, dest_location): + self.ensure_one() + vals = { + 'name': '%s: Move to %s' % ( + self.product_id.name_get()[0][1], + dest_location.complete_name), 'product_id': self.product_id.id, 'location_id': self.location_id.id, 'location_dest_id': dest_location.id, @@ -21,18 +22,23 @@ def move_to(self, dest_location): 'product_uom': self.product_id.uom_id.id, 'date_expected': fields.Datetime.now(), 'date': fields.Datetime.now(), - 'quant_ids': [(4, self.id)], - 'restrict_lot_id': self.lot_id.id - }) - new_move.action_done() + 'reserved_quant_ids': [(4, self.id)], + 'restrict_lot_id': self.lot_id.id, + 'origin': _('Quant Move'), + } + return vals + + @api.one + def move_to(self, dest_location): + vals = self._prepare_move_to(dest_location) + new_move = self.env['stock.move'].create(vals) + new_move.with_context(quant_moving=True).action_done() @api.model def quants_get_prefered_domain( self, location, product, qty, domain=None, - prefered_domain_list=None, restrict_lot_id=False, + prefered_domain_list=[], restrict_lot_id=False, restrict_partner_id=False): - if prefered_domain_list is None: - prefered_domain_list = [] quants = super(StockQuant, self).quants_get_prefered_domain( location, product, qty, domain=domain, prefered_domain_list=prefered_domain_list, @@ -40,7 +46,7 @@ def quants_get_prefered_domain( restrict_partner_id=restrict_partner_id) if location.usage not in ['inventory', 'production', 'supplier']: return quants - if self.env.context.get('quant_moving', False): + if self.env.context.get('quant_moving'): if domain is None: domain = [] res_qty = qty diff --git a/stock_quant_packages_moving_wizard/tests/test_stock_quant_packages_moving_wizard.py b/stock_quant_packages_moving_wizard/tests/test_stock_quant_packages_moving_wizard.py index 9bdeed80..f5f44288 100644 --- a/stock_quant_packages_moving_wizard/tests/test_stock_quant_packages_moving_wizard.py +++ b/stock_quant_packages_moving_wizard/tests/test_stock_quant_packages_moving_wizard.py @@ -42,6 +42,7 @@ def setUp(self): 'parent_id': self.package1.id, 'quant_ids': [(6, 0, self.quant2.ids)], }) + self.assertEquals(self.quant2.package_id, self.package2) self.assertEquals(self.quant1.location_id.id, self.location_from_id) self.assertEquals(self.quant2.location_id.id, self.location_from_id) self.assertEquals(self.package1.location_id.id, self.location_from_id) @@ -51,11 +52,9 @@ def test_move_quant(self): move_wiz = self.quant_move_model.create({ 'pack_move_items': [(0, 0, {'quant': self.quant1.id, - 'source_loc': self.quant1.location_id.id, 'dest_loc': self.location_to_id})], }) move_wiz.do_transfer() - self.assertNotEquals(self.quant1.location_id.id, self.location_from_id) self.assertEquals(self.quant1.location_id.id, self.location_to_id) def test_move_quant_default(self): @@ -64,23 +63,22 @@ def test_move_quant_default(self): self.assertEquals( move_wiz_vals['pack_move_items'][0]['quant'], self.quant1.id) - def test_move_quant_item_onchange_quant(self): - item = self.quant_move_item_model.new({'quant': self.quant1.id}) - item.onchange_quant() - self.assertEquals(item.source_loc, self.quant1.location_id) - def test_move_package_default(self): move_wiz_vals = self.package_move_model.with_context( active_ids=self.package1.ids).default_get([]) self.assertEquals( move_wiz_vals['pack_move_items'][0]['package'], self.package1.id) - def test_move_package_item_onchange_quant(self): - item = self.package_move_item_model.new({'package': self.package1.id}) - item.onchange_quant() - self.assertEquals(item.source_loc, self.package1.location_id) - - def test_move_quants_item_onchange_quant(self): - item = self.quants_move_item_model.new({'quant': self.quant1.id}) - item.onchange_quant() - self.assertEquals(item.source_loc, self.quant1.location_id) + def test_move_quant_package(self): + move_wiz = self.package_move_model.create({ + 'pack_move_items': [(0, 0, + {'package': self.package1.id, + 'dest_loc': self.location_to_id})], + }) + move_wiz.do_detailed_transfer() + self.assertEquals(self.quant2.location_id.id, self.location_to_id) + # It fails here because of a bug in the module + # quant2 is not inside the package2 + self.assertEquals(self.quant2.package_id, self.package2) + self.assertEquals(self.package1.location_id.id, self.location_to_id) + self.assertEquals(self.package2.location_id.id, self.location_to_id) diff --git a/stock_quant_packages_moving_wizard/views/stock_view.xml b/stock_quant_packages_moving_wizard/views/stock_view.xml new file mode 100644 index 00000000..6814672f --- /dev/null +++ b/stock_quant_packages_moving_wizard/views/stock_view.xml @@ -0,0 +1,28 @@ + + + + + + move_wizard.stock.quant.form + stock.quant + + + + + + + + move_wizard.stock.quant.package.form + stock.quant.package + + + + + + + + diff --git a/stock_quant_packages_moving_wizard/wizard/quant_move_wizard.py b/stock_quant_packages_moving_wizard/wizard/quant_move_wizard.py index 253eb51f..9cd04d7e 100644 --- a/stock_quant_packages_moving_wizard/wizard/quant_move_wizard.py +++ b/stock_quant_packages_moving_wizard/wizard/quant_move_wizard.py @@ -7,13 +7,14 @@ class StockQuantMove(models.TransientModel): _name = 'stock.quant.move' + # TODO port v9: rename this field to remove 'pack_', which is confusing pack_move_items = fields.One2many( comodel_name='stock.quant.move_items', inverse_name='move_id', string='Packs') @api.model - def default_get(self, fields): - res = super(StockQuantMove, self).default_get(fields) + def default_get(self, fields_list): + res = super(StockQuantMove, self).default_get(fields_list) quants_ids = self.env.context.get('active_ids', []) if not quants_ids: return res @@ -22,11 +23,7 @@ def default_get(self, fields): items = [] for quant in quants: if not quant.package_id: - item = { - 'quant': quant.id, - 'source_loc': quant.location_id.id, - } - items.append(item) + items.append({'quant': quant.id}) res.update(pack_move_items=items) return res @@ -44,15 +41,10 @@ class StockQuantMoveItems(models.TransientModel): move_id = fields.Many2one( comodel_name='stock.quant.move', string='Quant move') quant = fields.Many2one( - comodel_name='stock.quant', string='Quant', + comodel_name='stock.quant', string='Quant', required=True, domain=[('package_id', '=', False)]) source_loc = fields.Many2one( - comodel_name='stock.location', string='Source Location', required=True) + string='Current Location', related='quant.location_id', readonly=True) dest_loc = fields.Many2one( comodel_name='stock.location', string='Destination Location', required=True) - - @api.one - @api.onchange('quant') - def onchange_quant(self): - self.source_loc = self.quant.location_id diff --git a/stock_quant_packages_moving_wizard/wizard/quant_move_wizard_view.xml b/stock_quant_packages_moving_wizard/wizard/quant_move_wizard_view.xml index 9d80ac66..680d39ef 100644 --- a/stock_quant_packages_moving_wizard/wizard/quant_move_wizard_view.xml +++ b/stock_quant_packages_moving_wizard/wizard/quant_move_wizard_view.xml @@ -2,17 +2,17 @@ - Enter transfer details + stock.quant.move.wizard.form stock.quant.move -
- - + + + - + @@ -27,8 +27,9 @@ - + - \ No newline at end of file + diff --git a/stock_quant_packages_moving_wizard/wizard/quant_packages_move_wizard.py b/stock_quant_packages_moving_wizard/wizard/quant_packages_move_wizard.py index 1d09369f..d7644ae6 100644 --- a/stock_quant_packages_moving_wizard/wizard/quant_packages_move_wizard.py +++ b/stock_quant_packages_moving_wizard/wizard/quant_packages_move_wizard.py @@ -12,8 +12,8 @@ class StockQuantPackageMove(models.TransientModel): string='Packs') @api.model - def default_get(self, fields): - res = super(StockQuantPackageMove, self).default_get(fields) + def default_get(self, fields_list): + res = super(StockQuantPackageMove, self).default_get(fields_list) packages_ids = self.env.context.get('active_ids', []) if not packages_ids: return res @@ -22,16 +22,13 @@ def default_get(self, fields): items = [] for package in packages: if not package.parent_id and package.location_id: - item = { - 'package': package.id, - 'source_loc': package.location_id.id, - } - items.append(item) + items.append({'package': package.id}) res.update(pack_move_items=items) return res - @api.one + @api.multi def do_detailed_transfer(self): + self.ensure_one() for item in self.pack_move_items: if item.dest_loc is not item.source_loc: for quant in item.package.quant_ids: @@ -50,14 +47,11 @@ class StockQuantPackageMoveItems(models.TransientModel): comodel_name='stock.quant.package.move', string='Package move') package = fields.Many2one( comodel_name='stock.quant.package', string='Quant package', + required=True, domain=[('parent_id', '=', False), ('location_id', '!=', False)]) source_loc = fields.Many2one( - comodel_name='stock.location', string='Source Location', required=True) + string='Current Location', related='package.location_id', + readonly=True) dest_loc = fields.Many2one( comodel_name='stock.location', string='Destination Location', required=True) - - @api.one - @api.onchange('package') - def onchange_quant(self): - self.source_loc = self.package.location_id diff --git a/stock_quant_packages_moving_wizard/wizard/quant_packages_move_wizard_view.xml b/stock_quant_packages_moving_wizard/wizard/quant_packages_move_wizard_view.xml index f96d6dad..012edd3c 100644 --- a/stock_quant_packages_moving_wizard/wizard/quant_packages_move_wizard_view.xml +++ b/stock_quant_packages_moving_wizard/wizard/quant_packages_move_wizard_view.xml @@ -2,17 +2,17 @@ - Enter transfer details + stock.quant.package.move.wizard.form stock.quant.package.move - - - + + + - + @@ -27,7 +27,7 @@ - diff --git a/stock_quant_packages_moving_wizard/wizard/quants_move_wizard.py b/stock_quant_packages_moving_wizard/wizard/quants_move_wizard.py index 5996ec4e..b7f2aa98 100644 --- a/stock_quant_packages_moving_wizard/wizard/quants_move_wizard.py +++ b/stock_quant_packages_moving_wizard/wizard/quants_move_wizard.py @@ -7,6 +7,7 @@ class StockQuantsMoveWizard(models.TransientModel): _name = 'stock.quants.move' + # TODO port v9: rename this field to remove 'pack_', which is confusing pack_move_items = fields.One2many( comodel_name='stock.quants.move_items', inverse_name='move_id', string='Quants') @@ -15,8 +16,8 @@ class StockQuantsMoveWizard(models.TransientModel): required=True) @api.model - def default_get(self, fields): - res = super(StockQuantsMoveWizard, self).default_get(fields) + def default_get(self, fields_list): + res = super(StockQuantsMoveWizard, self).default_get(fields_list) quants_ids = self.env.context.get('active_ids', []) if not quants_ids: return res @@ -24,19 +25,25 @@ def default_get(self, fields): quants = quant_obj.browse(quants_ids) items = [] for quant in quants.filtered(lambda q: not q.package_id): - item = { - 'quant': quant.id, - 'source_loc': quant.location_id.id, - } - items.append(item) + items.append({'quant': quant.id}) res.update(pack_move_items=items) return res - @api.one + @api.multi def do_transfer(self): + self.ensure_one() + quant_ids = [] for item in self.pack_move_items: - item.quant.move_to(self.dest_loc) - return True + quant_ids.append(item.quant.id) + if item.quant.location_id != self.dest_loc: + item.quant.move_to(self.dest_loc) + action = self.env['ir.actions.act_window'].for_xml_id( + 'stock', 'quantsact') + action.update({ + 'domain': [('id', 'in', quant_ids)], + 'context': {}, + }) + return action class StockQuantsMoveItems(models.TransientModel): @@ -46,12 +53,7 @@ class StockQuantsMoveItems(models.TransientModel): move_id = fields.Many2one( comodel_name='stock.quants.move', string='Quant move') quant = fields.Many2one( - comodel_name='stock.quant', string='Quant', + comodel_name='stock.quant', string='Quant', required=True, domain=[('package_id', '=', False)]) source_loc = fields.Many2one( - comodel_name='stock.location', string='Source Location', required=True) - - @api.one - @api.onchange('quant') - def onchange_quant(self): - self.source_loc = self.quant.location_id + string='Current Location', related='quant.location_id', readonly=True) diff --git a/stock_quant_packages_moving_wizard/wizard/quants_move_wizard_view.xml b/stock_quant_packages_moving_wizard/wizard/quants_move_wizard_view.xml index 518ecca5..ea5ddaa1 100644 --- a/stock_quant_packages_moving_wizard/wizard/quants_move_wizard_view.xml +++ b/stock_quant_packages_moving_wizard/wizard/quants_move_wizard_view.xml @@ -2,14 +2,13 @@ - Enter transfer details + stock.quants.move.wizard.form stock.quants.move - - + + - + @@ -30,14 +29,12 @@ Move Stock Quants To stock.quants.move - form - tree,form - + form new From 3a9a8794f45c1b598cc8583be806deff7a4dc07d Mon Sep 17 00:00:00 2001 From: Alexis de Lattre Date: Fri, 3 Feb 2017 10:14:40 +0100 Subject: [PATCH 2/3] Fix access right issue on stock.quant Fix missing source_loc field when the line of the wizard is generated by default_get() --- stock_quant_packages_moving_wizard/models/stock.py | 3 ++- .../wizard/quant_move_wizard.py | 6 +++++- .../wizard/quant_packages_move_wizard.py | 6 +++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/stock_quant_packages_moving_wizard/models/stock.py b/stock_quant_packages_moving_wizard/models/stock.py index 4dfd3049..8b0a56f1 100644 --- a/stock_quant_packages_moving_wizard/models/stock.py +++ b/stock_quant_packages_moving_wizard/models/stock.py @@ -22,7 +22,6 @@ def _prepare_move_to(self, dest_location): 'product_uom': self.product_id.uom_id.id, 'date_expected': fields.Datetime.now(), 'date': fields.Datetime.now(), - 'reserved_quant_ids': [(4, self.id)], 'restrict_lot_id': self.lot_id.id, 'origin': _('Quant Move'), } @@ -32,6 +31,8 @@ def _prepare_move_to(self, dest_location): def move_to(self, dest_location): vals = self._prepare_move_to(dest_location) new_move = self.env['stock.move'].create(vals) + # No group has write access on stock.quant -> we need sudo() + self.sudo().reservation_id = new_move new_move.with_context(quant_moving=True).action_done() @api.model diff --git a/stock_quant_packages_moving_wizard/wizard/quant_move_wizard.py b/stock_quant_packages_moving_wizard/wizard/quant_move_wizard.py index 9cd04d7e..9f17074c 100644 --- a/stock_quant_packages_moving_wizard/wizard/quant_move_wizard.py +++ b/stock_quant_packages_moving_wizard/wizard/quant_move_wizard.py @@ -23,7 +23,11 @@ def default_get(self, fields_list): items = [] for quant in quants: if not quant.package_id: - items.append({'quant': quant.id}) + items.append({ + 'quant': quant.id, + # source_loc is needed even if it's a related field... + 'source_loc': quant.location_id.id, + }) res.update(pack_move_items=items) return res diff --git a/stock_quant_packages_moving_wizard/wizard/quant_packages_move_wizard.py b/stock_quant_packages_moving_wizard/wizard/quant_packages_move_wizard.py index d7644ae6..e6ce2049 100644 --- a/stock_quant_packages_moving_wizard/wizard/quant_packages_move_wizard.py +++ b/stock_quant_packages_moving_wizard/wizard/quant_packages_move_wizard.py @@ -22,7 +22,11 @@ def default_get(self, fields_list): items = [] for package in packages: if not package.parent_id and package.location_id: - items.append({'package': package.id}) + items.append({ + 'package': package.id, + # source_loc is needed even if it's a related field... + 'source_loc': package.location_id.id, + }) res.update(pack_move_items=items) return res From 6d1fb8ae6548a90a2d211a48e38e715109b2419a Mon Sep 17 00:00:00 2001 From: Alexis de Lattre Date: Fri, 3 Feb 2017 17:23:21 +0100 Subject: [PATCH 3/3] Unreserve before moving the quant, to avoid problems on the picking that booked the quant --- stock_quant_packages_moving_wizard/models/stock.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/stock_quant_packages_moving_wizard/models/stock.py b/stock_quant_packages_moving_wizard/models/stock.py index 8b0a56f1..a64d8ad5 100644 --- a/stock_quant_packages_moving_wizard/models/stock.py +++ b/stock_quant_packages_moving_wizard/models/stock.py @@ -29,6 +29,12 @@ def _prepare_move_to(self, dest_location): @api.one def move_to(self, dest_location): + # if the quant is reserved for another move, + # we should cleanly un-reserve it first, so that + # the picking that booked this quant comes back from + # available to waiting availability + if self.reservation_id: + self.reservation_id.do_unreserve() vals = self._prepare_move_to(dest_location) new_move = self.env['stock.move'].create(vals) # No group has write access on stock.quant -> we need sudo()