From cb5fd87e7fdab3e3aa559adfd6fbe5392e3ee301 Mon Sep 17 00:00:00 2001 From: Wolfgang Taferner Date: Fri, 21 Oct 2016 08:19:31 +0200 Subject: [PATCH] [FIX] point_of_sale: handle returns always correctly We support 'returns' in the POS frontend by allowing the user to specify a negative quantity. Before this patch however, the backend would always generate a single picking per pos.order. This is problematic when a pos.order contains both lines with a positive and negative quantity. The generated move lines would not all have the same source and destination location and so could not be added to the same picking. This commit creates an extra 'return' picking when required and assigns the generated moves to the correct picking. opw-690812 Closes #13929 Forward-port of 19cca50e7c8d7e6b86f3851398725ce64694b063 --- addons/point_of_sale/models/pos_order.py | 74 +++++-- .../tests/test_point_of_sale_flow.py | 201 +++++++++++++++++- 2 files changed, 251 insertions(+), 24 deletions(-) diff --git a/addons/point_of_sale/models/pos_order.py b/addons/point_of_sale/models/pos_order.py index 20e2e40cc71..28973248676 100644 --- a/addons/point_of_sale/models/pos_order.py +++ b/addons/point_of_sale/models/pos_order.py @@ -532,7 +532,10 @@ class PosOrder(models.Model): for order in self: address = order.partner_id.address_get(['delivery']) or {} picking_type = order.picking_type_id - picking_id = False + return_pick_type = order.picking_type_id.return_picking_type_id or order.picking_type_id + order_picking = Picking + return_picking = Picking + moves = Move location_id = order.location_id.id if order.partner_id: destination_id = order.partner_id.property_stock_customer.id @@ -542,9 +545,9 @@ class PosOrder(models.Model): destination_id = customerloc.id else: destination_id = picking_type.default_location_dest_id.id + if picking_type: - pos_qty = all([x.qty >= 0 for x in order.lines]) - picking_id = Picking.create({ + picking_vals = { 'origin': order.name, 'partner_id': address.get('delivery', False), 'date_done': order.date_order, @@ -552,36 +555,61 @@ class PosOrder(models.Model): 'company_id': order.company_id.id, 'move_type': 'direct', 'note': order.note or "", - 'location_id': location_id if pos_qty else destination_id, - 'location_dest_id': destination_id if pos_qty else location_id, - }) - order.write({'picking_id': picking_id.id}) + 'location_id': location_id, + 'location_dest_id': destination_id, + } + pos_qty = any([x.qty >= 0 for x in order.lines]) + if pos_qty: + order_picking = Picking.create(picking_vals.copy()) + neg_qty = any([x.qty < 0 for x in order.lines]) + if neg_qty: + return_vals = picking_vals.copy() + return_vals.update({ + 'location_id': destination_id, + 'location_dest_id': return_pick_type != picking_type and return_pick_type.default_location_dest_id.id or location_id, + 'picking_type_id': return_pick_type.id + }) + return_picking = Picking.create(return_vals) for line in order.lines.filtered(lambda l: l.product_id.type in ['product', 'consu']): - Move += Move.create({ + moves |= Move.create({ 'name': line.name, 'product_uom': line.product_id.uom_id.id, - 'picking_id': picking_id and picking_id.id or False, - 'picking_type_id': picking_type.id, + 'picking_id': order_picking.id if line.qty >= 0 else return_picking.id, + 'picking_type_id': picking_type.id if line.qty >= 0 else return_pick_type.id, 'product_id': line.product_id.id, 'product_uom_qty': abs(line.qty), 'state': 'draft', 'location_id': location_id if line.qty >= 0 else destination_id, - 'location_dest_id': destination_id if line.qty >= 0 else location_id, + 'location_dest_id': destination_id if line.qty >= 0 else return_pick_type != picking_type and return_pick_type.default_location_dest_id.id or location_id, }) - if picking_id: - picking_id.action_confirm() - picking_id.force_assign() - # Mark pack operations as done - for pack in picking_id.pack_operation_ids: - pack.write({'qty_done': pack.product_qty}) - picking_id.action_done() - elif Move: - Move.action_confirm() - Move.force_assign() - Move.action_done() + + # prefer associating the regular order picking, not the return + order.write({'picking_id': order_picking.id or return_picking.id}) + + if return_picking: + order._force_picking_done(return_picking) + if order_picking: + order._force_picking_done(order_picking) + + # when the pos.config has no picking_type_id set only the moves will be created + if moves and not return_picking and not order_picking: + moves.action_confirm() + moves.force_assign() + moves.action_done() + return True + def _force_picking_done(self, picking): + """Force picking in order to be set as done.""" + for order in self: + picking.action_confirm() + picking.force_assign() + # Mark pack operations as done + for pack in picking.pack_operation_ids: + pack.write({'qty_done': pack.product_qty}) + picking.action_done() + def add_payment(self, data): """Create a new payment for the order""" args = { @@ -835,4 +863,4 @@ class ReportSaleDetails(models.AbstractModel): date_stop = self.env.context.get('date_stop', False) data = dict(data or {}, date_start=date_start, date_stop=date_stop) data.update(self.get_sale_details(date_start, date_stop, company)) - return self.env['report'].render('point_of_sale.report_saledetails', data) \ No newline at end of file + return self.env['report'].render('point_of_sale.report_saledetails', data) diff --git a/addons/point_of_sale/tests/test_point_of_sale_flow.py b/addons/point_of_sale/tests/test_point_of_sale_flow.py index 023eff033a0..4f029df9439 100644 --- a/addons/point_of_sale/tests/test_point_of_sale_flow.py +++ b/addons/point_of_sale/tests/test_point_of_sale_flow.py @@ -119,9 +119,208 @@ class TestPointOfSaleFlow(TestPointOfSaleCommon): # I test that the generated journal entry is attached to the PoS order self.assertTrue(self.pos_order_pos0.account_move, "Journal entry has not been attached to Pos order.") + def test_order_to_picking(self): + """ + In order to test the Point of Sale in module, I will do three orders from the sale to the payment, + invoicing + picking, but will only check the picking consistency in the end. + + TODO: Check the negative picking after changing the picking relation to One2many (also for a mixed use case), + check the quantity, the locations and return picking logic + """ + # I click on create a new session button + self.pos_config.open_session_cb() + + # I create a PoS order with 2 units of PCSC234 at 450 EUR + # and 3 units of PCSC349 at 300 EUR. + self.pos_order_pos1 = self.PosOrder.create({ + 'company_id': self.company_id, + 'pricelist_id': self.partner1.property_product_pricelist.id, + 'partner_id': self.partner1.id, + 'lines': [(0, 0, { + 'name': "OL/0001", + 'product_id': self.product3.id, + 'price_unit': 450, + 'discount': 0.0, + 'qty': 2.0, + }), (0, 0, { + 'name': "OL/0002", + 'product_id': self.product4.id, + 'price_unit': 300, + 'discount': 0.0, + 'qty': 3.0, + })] + }) + + # I click on the "Make Payment" wizard to pay the PoS order with the total amount (2*450 + 3*300 = 1800) + context_make_payment = { + "active_ids": [self.pos_order_pos1.id], + "active_id": self.pos_order_pos1.id + } + self.pos_make_payment_2 = self.PosMakePayment.with_context(context_make_payment).create({ + 'amount': 1800 + }) + + # I click on the validate button to register the payment. + context_payment = {'active_id': self.pos_order_pos1.id} + self.pos_make_payment_2.with_context(context_payment).check() + + # I check that the order is marked as paid + self.assertEqual( + self.pos_order_pos1.state, + 'paid', + 'Order should be in paid state.' + ) + + # I generate the journal entries + self.pos_order_pos1._create_account_move_line() + + # I test that the generated journal entry is attached to the PoS order + self.assertTrue( + self.pos_order_pos1.account_move, + "Journal entry has not been attached to Pos order." + ) + + # I test that the pickings are created as expected + # One picking attached and having all the positive move lines in the correct state + self.pos_order_pos1.create_picking() + self.assertEqual( + self.pos_order_pos1.picking_id.state, + 'done', + 'Picking should be in done state.' + ) + self.assertEqual( + self.pos_order_pos1.picking_id.move_lines.mapped('state'), + ['done', 'done'], + 'Move Lines should be in done state.' + ) + + self.pos_order_pos2 = self.PosOrder.create({ + 'company_id': self.company_id, + 'pricelist_id': self.partner1.property_product_pricelist.id, + 'partner_id': self.partner1.id, + 'lines': [(0, 0, { + 'name': "OL/0003", + 'product_id': self.product3.id, + 'price_unit': 450, + 'discount': 0.0, + 'qty': (-2.0), + }), (0, 0, { + 'name': "OL/0004", + 'product_id': self.product4.id, + 'price_unit': 300, + 'discount': 0.0, + 'qty': (-3.0), + })] + }) + + # I click on the "Make Payment" wizard to pay the PoS order with the total amount (-2*450 + -3*300 = -1800) + context_make_payment = { + "active_ids": [self.pos_order_pos2.id], + "active_id": self.pos_order_pos2.id + } + self.pos_make_payment_3 = self.PosMakePayment.with_context(context_make_payment).create({ + 'amount': (-1800) + }) + + # I click on the validate button to register the payment. + context_payment = {'active_id': self.pos_order_pos2.id} + self.pos_make_payment_3.with_context(context_payment).check() + + # I check that the order is marked as paid + self.assertEqual( + self.pos_order_pos2.state, + 'paid', + 'Order should be in paid state.' + ) + + # I generate the journal entries + self.pos_order_pos2._create_account_move_line() + + # I test that the generated journal entry is attached to the PoS order + self.assertTrue( + self.pos_order_pos2.account_move, + "Journal entry has not been attached to PoS order." + ) + + # I test that the pickings are created as expected + # One picking attached and having all the positive move lines in the correct state + self.pos_order_pos2.create_picking() + self.assertEqual( + self.pos_order_pos2.picking_id.state, + 'done', + 'Picking should be in done state.' + ) + self.assertEqual( + self.pos_order_pos2.picking_id.move_lines.mapped('state'), + ['done', 'done'], + 'Move Lines should be in done state.' + ) + + self.pos_order_pos3 = self.PosOrder.create({ + 'company_id': self.company_id, + 'pricelist_id': self.partner1.property_product_pricelist.id, + 'partner_id': self.partner1.id, + 'lines': [(0, 0, { + 'name': "OL/0005", + 'product_id': self.product3.id, + 'price_unit': 450, + 'discount': 0.0, + 'qty': (-2.0), + }), (0, 0, { + 'name': "OL/0006", + 'product_id': self.product4.id, + 'price_unit': 300, + 'discount': 0.0, + 'qty': 3.0, + })] + }) + + # I click on the "Make Payment" wizard to pay the PoS order with the total amount (-2*450 + 3*300 = 0) + context_make_payment = { + "active_ids": [self.pos_order_pos3.id], + "active_id": self.pos_order_pos3.id + } + self.pos_make_payment_4 = self.PosMakePayment.with_context(context_make_payment).create({ + 'amount': 0 + }) + + # I click on the validate button to register the payment. + context_payment = {'active_id': self.pos_order_pos3.id} + self.pos_make_payment_4.with_context(context_payment).check() + + # I check that the order is marked as paid + self.assertEqual( + self.pos_order_pos3.state, + 'paid', + 'Order should be in paid state.' + ) + + # I generate the journal entries + self.pos_order_pos3._create_account_move_line() + + # I test that the generated journal entry is attached to the PoS order + self.assertTrue( + self.pos_order_pos3.account_move, + "Journal entry has not been attached to PoS order." + ) + + # I test that the pickings are created as expected + # One picking attached and having all the positive move lines in the correct state + self.pos_order_pos3.create_picking() + self.assertEqual( + self.pos_order_pos3.picking_id.state, + 'done', + 'Picking should be in done state.' + ) + self.assertEqual( + self.pos_order_pos3.picking_id.move_lines.mapped('state'), + ['done'], + 'Move Lines should be in done state.' + ) + def test_order_to_invoice(self): - #I create a new PoS order with 2 units of PC1 at 450 EUR (Tax Incl) and 3 units of PCSC349 at 300 EUR. (Tax Excl) + # I create a new PoS order with 2 units of PC1 at 450 EUR (Tax Incl) and 3 units of PCSC349 at 300 EUR. (Tax Excl) self.pos_order_pos1 = self.PosOrder.create({ 'company_id': self.company_id, 'partner_id': self.partner1.id,