From 623b22c09157886c902a970fb06d746f9d36f2e8 Mon Sep 17 00:00:00 2001 From: Jordi Ballester Alomar Date: Mon, 21 Nov 2022 15:13:06 +0100 Subject: [PATCH 1/2] [FIX] include anglo-saxon price unit calculation in refunds. Otherwise the anglo saxon entries won't be correct. For example, the Interim (Delivered) account should balance after receiving and triggering a refund on a customer rma. --- rma/models/rma_order_line.py | 12 ++ rma_account/models/__init__.py | 1 + rma_account/models/account_move.py | 45 +++++++ rma_account/models/rma_order_line.py | 26 ++++ rma_account/models/stock_valuation_layer.py | 16 +++ .../test_account_move_line_rma_order_line.py | 4 +- rma_account/tests/test_rma_stock_account.py | 127 ++++++++++++++++-- .../tests/test_rma_account_unreconciled.py | 101 ++++++++++++++ .../tests/test_rma_stock_account_purchase.py | 50 +++---- rma_sale/tests/test_rma_stock_account_sale.py | 2 +- 10 files changed, 346 insertions(+), 38 deletions(-) create mode 100644 rma_account/models/stock_valuation_layer.py create mode 100644 rma_account_unreconciled/tests/test_rma_account_unreconciled.py diff --git a/rma/models/rma_order_line.py b/rma/models/rma_order_line.py index 4ac27392..c0754908 100644 --- a/rma/models/rma_order_line.py +++ b/rma/models/rma_order_line.py @@ -58,6 +58,18 @@ class RmaOrderLine(models.Model): pickings |= move.picking_id return pickings + @api.model + def _get_in_moves(self): + moves = self.env["stock.move"] + for move in self.move_ids: + first_usage = move._get_first_usage() + last_usage = move._get_last_usage() + if last_usage == "internal" and first_usage != "internal": + moves |= move + elif last_usage == "supplier" and first_usage == "customer": + moves |= moves + return moves + @api.model def _get_out_pickings(self): pickings = self.env["stock.picking"] diff --git a/rma_account/models/__init__.py b/rma_account/models/__init__.py index b1fb9d25..0d8b8255 100644 --- a/rma_account/models/__init__.py +++ b/rma_account/models/__init__.py @@ -5,3 +5,4 @@ from . import account_move from . import account_move_line from . import procurement from . import stock_move +from . import stock_valuation_layer diff --git a/rma_account/models/account_move.py b/rma_account/models/account_move.py index 602bba60..e28f89a3 100644 --- a/rma_account/models/account_move.py +++ b/rma_account/models/account_move.py @@ -121,6 +121,19 @@ class AccountMove(models.Model): line.update({"rma_line_id": find_with_label_rma.id}) return res + def _stock_account_get_last_step_stock_moves(self): + rslt = super(AccountMove, self)._stock_account_get_last_step_stock_moves() + for invoice in self.filtered(lambda x: x.move_type == "out_invoice"): + rslt += invoice.mapped("line_ids.rma_line_id.move_ids").filtered( + lambda x: x.state == "done" and x.location_dest_id.usage == "customer" + ) + for invoice in self.filtered(lambda x: x.move_type == "out_refund"): + # Add refunds generated from the RMA + rslt += invoice.mapped("line_ids.rma_line_id.move_ids").filtered( + lambda x: x.state == "done" and x.location_id.usage == "customer" + ) + return rslt + class AccountMoveLine(models.Model): _inherit = "account.move.line" @@ -203,3 +216,35 @@ class AccountMoveLine(models.Model): ondelete="set null", help="This will contain the rma line that originated this line", ) + + def _stock_account_get_anglo_saxon_price_unit(self): + self.ensure_one() + price_unit = super( + AccountMoveLine, self + )._stock_account_get_anglo_saxon_price_unit() + rma_line = self.rma_line_id or self.env["rma.order.line"] + if rma_line: + is_line_reversing = bool(self.move_id.reversed_entry_id) + qty_to_refund = self.product_uom_id._compute_quantity( + self.quantity, self.product_id.uom_id + ) + posted_invoice_lines = rma_line.move_line_ids.filtered( + lambda l: l.move_id.move_type == "out_refund" + and l.move_id.state == "posted" + and bool(l.move_id.reversed_entry_id) == is_line_reversing + ) + qty_refunded = sum( + x.product_uom_id._compute_quantity(x.quantity, x.product_id.uom_id) + for x in posted_invoice_lines + ) + product = self.product_id.with_company(self.company_id).with_context( + is_returned=is_line_reversing + ) + average_price_unit = product._compute_average_price( + qty_refunded, qty_to_refund, rma_line._get_in_moves() + ) + if average_price_unit: + price_unit = self.product_id.uom_id.with_company( + self.company_id + )._compute_price(average_price_unit, self.product_uom_id) + return price_unit diff --git a/rma_account/models/rma_order_line.py b/rma_account/models/rma_order_line.py index 09a5e326..ece41b2e 100644 --- a/rma_account/models/rma_order_line.py +++ b/rma_account/models/rma_order_line.py @@ -307,3 +307,29 @@ class RmaOrderLine(models.Model): return res else: return super(RmaOrderLine, self).name_get() + + def _stock_account_anglo_saxon_reconcile_valuation(self): + for rma in self: + prod = rma.product_id + if rma.product_id.valuation != "real_time": + continue + if not rma.company_id.anglo_saxon_accounting: + continue + product_accounts = prod.product_tmpl_id._get_product_accounts() + if rma.type == "customer": + product_interim_account = product_accounts["stock_output"] + else: + product_interim_account = product_accounts["stock_input"] + if product_interim_account.reconcile: + # Get the in and out moves + amls = rma.move_ids.mapped( + "stock_valuation_layer_ids.account_move_id.line_ids" + ) + # Search for anglo-saxon lines linked to the product in the journal entry. + amls = amls.filtered( + lambda line: line.product_id == prod + and line.account_id == product_interim_account + and not line.reconciled + ) + # Reconcile. + amls.reconcile() diff --git a/rma_account/models/stock_valuation_layer.py b/rma_account/models/stock_valuation_layer.py new file mode 100644 index 00000000..4495744c --- /dev/null +++ b/rma_account/models/stock_valuation_layer.py @@ -0,0 +1,16 @@ +# Copyright 2017-22 ForgeFlow S.L. (www.forgeflow.com) +# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html). + +from odoo import models + + +class StockValuationLayer(models.Model): + _inherit = "stock.valuation.layer" + + def _validate_accounting_entries(self): + res = super(StockValuationLayer, self)._validate_accounting_entries() + for svl in self: + # Eventually reconcile together the stock interim accounts + if svl.company_id.anglo_saxon_accounting: + svl.stock_move_id.rma_line_id._stock_account_anglo_saxon_reconcile_valuation() + return res diff --git a/rma_account/tests/test_account_move_line_rma_order_line.py b/rma_account/tests/test_account_move_line_rma_order_line.py index 0303727e..4f671931 100644 --- a/rma_account/tests/test_account_move_line_rma_order_line.py +++ b/rma_account/tests/test_account_move_line_rma_order_line.py @@ -42,8 +42,8 @@ class TestAccountMoveLineRmaOrderLine(common.TransactionCase): # Create account for Cost of Goods Sold acc_type = cls._create_account_type("expense", "other", "expense") - name = "Cost of Goods Sold" - code = "cogs" + name = "Goods Delivered Not Invoiced" + code = "gdni" cls.account_cogs = cls._create_account(acc_type, name, code, cls.company) # Create account for Inventory acc_type = cls._create_account_type("asset", "other", "asset") diff --git a/rma_account/tests/test_rma_stock_account.py b/rma_account/tests/test_rma_stock_account.py index ee663988..a0ce458b 100644 --- a/rma_account/tests/test_rma_stock_account.py +++ b/rma_account/tests/test_rma_stock_account.py @@ -14,11 +14,16 @@ class TestRmaStockAccount(TestRma): cls.acc_type_model = cls.env["account.account.type"] cls.account_model = cls.env["account.account"] cls.g_account_user = cls.env.ref("account.group_account_user") + cls.rma_refund_wiz = cls.env["rma.refund"] # we create new products to ensure previous layers do not affect when # running FIFO cls.product_fifo_1 = cls._create_product("product_fifo1") cls.product_fifo_2 = cls._create_product("product_fifo2") cls.product_fifo_3 = cls._create_product("product_fifo3") + # Refs + cls.rma_operation_customer_refund_id = cls.env.ref( + "rma_account.rma_operation_customer_refund" + ) cls.rma_basic_user.write({"groups_id": [(4, cls.g_account_user.id)]}) # The product category created in the base module is not automated valuation # we have to create a new category here @@ -27,11 +32,11 @@ class TestRmaStockAccount(TestRma): name = "Goods Received Not Invoiced" code = "grni" cls.account_grni = cls._create_account(acc_type, name, code, cls.company, True) - # Create account for Cost of Goods Sold - acc_type = cls._create_account_type("expense", "other") - name = "Cost of Goods Sold" - code = "cogs" - cls.account_cogs = cls._create_account(acc_type, name, code, cls.company, False) + # Create account for Goods Delievered + acc_type = cls._create_account_type("asset", "other") + name = "Goods Delivered Not Invoiced" + code = "gdni" + cls.account_gdni = cls._create_account(acc_type, name, code, cls.company, True) # Create account for Inventory acc_type = cls._create_account_type("asset", "other") name = "Inventory" @@ -45,9 +50,9 @@ class TestRmaStockAccount(TestRma): "property_stock_valuation_account_id": cls.account_inventory.id, "property_valuation": "real_time", "property_stock_account_input_categ_id": cls.account_grni.id, - "property_stock_account_output_categ_id": cls.account_cogs.id, + "property_stock_account_output_categ_id": cls.account_gdni.id, "rma_approval_policy": "one_step", - "rma_customer_operation_id": cls.rma_cust_replace_op_id.id, + "rma_customer_operation_id": cls.rma_operation_customer_refund_id.id, "rma_supplier_operation_id": cls.rma_sup_replace_op_id.id, "property_cost_method": "fifo", } @@ -103,7 +108,7 @@ class TestRmaStockAccount(TestRma): self.assertEqual(picking.move_lines.stock_valuation_layer_ids.value, 15.0) account_move = picking.move_lines.stock_valuation_layer_ids.account_move_id self.check_accounts_used( - account_move, debit_account="inventory", credit_account="cogs" + account_move, debit_account="inventory", credit_account="gdni" ) def test_02_cost_from_move(self): @@ -130,8 +135,9 @@ class TestRmaStockAccount(TestRma): dropship=False, ) # Set an incorrect price in the RMA (this should not affect cost) - rma_customer_id.rma_line_ids.price_unit = 999 - rma_customer_id.rma_line_ids.action_rma_to_approve() + rma_lines = rma_customer_id.rma_line_ids + rma_lines.price_unit = 999 + rma_lines.action_rma_to_approve() picking = self._receive_rma(rma_customer_id.rma_line_ids) # Test the value in the layers of the incoming stock move is used for rma_line in rma_customer_id.rma_line_ids: @@ -141,3 +147,104 @@ class TestRmaStockAccount(TestRma): ) value_used = move_product.stock_valuation_layer_ids.value self.assertEqual(value_used, -value_origin) + # Create a refund for the first line + rma = rma_lines[0] + make_refund = self.rma_refund_wiz.with_context( + **{ + "customer": True, + "active_ids": rma.ids, + "active_model": "rma.order.line", + } + ).create({"description": "Test refund"}) + make_refund.item_ids.qty_to_refund = 1 + make_refund.invoice_refund() + rma.refund_line_ids.move_id.action_post() + rma._compute_refund_count() + gdni_amls = rma.refund_line_ids.move_id.line_ids.filtered( + lambda l: l.account_id == self.account_gdni + ) + gdni_amls |= ( + rma.move_ids.stock_valuation_layer_ids.account_move_id.line_ids.filtered( + lambda l: l.account_id == self.account_gdni + ) + ) + gdni_balance = sum(gdni_amls.mapped("balance")) + # When we received we Credited to GDNI 30 + # When we refund we Debit to GDNI 10 + self.assertEqual(gdni_balance, -20.0) + make_refund = self.rma_refund_wizwith_context( + **{ + "customer": True, + "active_ids": rma.ids, + "active_model": "rma.order.line", + } + ).create({"description": "Test refund"}) + make_refund.item_ids.qty_to_refund = 2 + make_refund.invoice_refund() + rma.refund_line_ids.move_id.filtered( + lambda m: m.state != "posted" + ).action_post() + rma._compute_refund_count() + gdni_amls = rma.refund_line_ids.move_id.line_ids.filtered( + lambda l: l.account_id == self.account_gdni + ) + gdni_amls |= ( + rma.move_ids.stock_valuation_layer_ids.account_move_id.line_ids.filtered( + lambda l: l.account_id == self.account_gdni + ) + ) + gdni_balance = sum(gdni_amls.mapped("balance")) + # When we received we Credited to GDNI 30 + # When we refund we Debit to GDNI 30 + self.assertEqual(gdni_balance, 0.0) + # Ensure that the GDNI move lines are all reconciled + self.assertEqual(all(gdni_amls.mapped("reconciled")), True) + + def test_03_cost_from_move(self): + """ + Receive a product and then return it. The Goods Delivered Not Invoiced + should result in 0 + """ + # Set a standard price on the products + self.product_fifo_1.standard_price = 10 + self._create_inventory( + self.product_fifo_1, 20.0, self.env.ref("stock.stock_location_customers") + ) + products2move = [ + (self.product_fifo_1, 3), + ] + self.product_fifo_1.categ_id.rma_customer_operation_id = ( + self.rma_cust_replace_op_id + ) + rma_customer_id = self._create_rma_from_move( + products2move, + "customer", + self.env.ref("base.res_partner_2"), + dropship=False, + ) + # Set an incorrect price in the RMA (this should not affect cost) + rma = rma_customer_id.rma_line_ids + rma.price_unit = 999 + rma.action_rma_to_approve() + self._receive_rma(rma_customer_id.rma_line_ids) + gdni_amls = ( + rma.move_ids.stock_valuation_layer_ids.account_move_id.line_ids.filtered( + lambda l: l.account_id == self.account_gdni + ) + ) + gdni_balance = sum(gdni_amls.mapped("balance")) + self.assertEqual(len(gdni_amls), 1) + # Balance should be -30, as we have only received + self.assertEqual(gdni_balance, -30.0) + self._deliver_rma(rma_customer_id.rma_line_ids) + gdni_amls = ( + rma.move_ids.stock_valuation_layer_ids.account_move_id.line_ids.filtered( + lambda l: l.account_id == self.account_gdni + ) + ) + gdni_balance = sum(gdni_amls.mapped("balance")) + self.assertEqual(len(gdni_amls), 2) + # Balance should be 0, as we have received and shipped + self.assertEqual(gdni_balance, 0.0) + # The GDNI entries should be now reconciled + self.assertEqual(all(gdni_amls.mapped("reconciled")), True) diff --git a/rma_account_unreconciled/tests/test_rma_account_unreconciled.py b/rma_account_unreconciled/tests/test_rma_account_unreconciled.py new file mode 100644 index 00000000..42ce79dd --- /dev/null +++ b/rma_account_unreconciled/tests/test_rma_account_unreconciled.py @@ -0,0 +1,101 @@ +# Copyright 2022 ForgeFlow S.L. +# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html) + + +from odoo.addons.rma.tests.test_rma import TestRma + + +class TestRmaAccountUnreconciled(TestRma): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.rma_refund_wiz = cls.env["rma.refund"] + cls.g_account_manager = cls.env.ref("account.group_account_manager") + cls.rma_manager_user_account = cls._create_user( + "rma manager account", + [cls.g_stock_manager, cls.g_rma_manager, cls.g_account_manager], + cls.company, + ) + for categ in cls.rma_customer_id.with_user(cls.rma_manager_user_account).mapped( + "rma_line_ids.product_id.categ_id" + ): + categ.write( + { + "property_valuation": "real_time", + "property_cost_method": "fifo", + } + ) + categ.property_stock_account_input_categ_id.write( + { + "reconcile": True, + } + ) + categ.property_stock_account_output_categ_id.write( + { + "reconcile": True, + } + ) + for product in cls.rma_customer_id.with_user( + cls.rma_manager_user_account + ).mapped("rma_line_ids.product_id"): + product.write( + { + "standard_price": 10.0, + } + ) + + def test_unreconciled_moves(self): + for rma_line in self.rma_customer_id.rma_line_ids: + rma_line.write( + { + "refund_policy": "received", + } + ) + rma_line.action_rma_approve() + self.assertFalse(rma_line.unreconciled) + self.rma_customer_id.rma_line_ids.action_rma_to_approve() + wizard = self.rma_make_picking.with_context( + { + "active_ids": self.rma_customer_id.rma_line_ids.ids, + "active_model": "rma.order.line", + "picking_type": "incoming", + "active_id": 1, + } + ).create({}) + wizard._create_picking() + res = self.rma_customer_id.rma_line_ids.action_view_in_shipments() + picking = self.env["stock.picking"].browse(res["res_id"]) + picking.action_assign() + for mv in picking.move_lines: + mv.quantity_done = mv.product_uom_qty + picking.button_validate() + for rma_line in self.rma_customer_id.rma_line_ids: + rma_line._compute_unreconciled() + self.assertTrue(rma_line.unreconciled) + make_refund = self.rma_refund_wiz.with_context( + { + "customer": True, + "active_ids": self.rma_customer_id.rma_line_ids.ids, + "active_model": "rma.order.line", + } + ).create( + { + "description": "Test refund", + } + ) + for item in make_refund.item_ids: + item.write( + { + "qty_to_refund": item.product_qty, + } + ) + make_refund.invoice_refund() + self.rma_customer_id.with_user( + self.rma_manager_user_account + ).rma_line_ids.refund_line_ids.move_id.filtered( + lambda x: x.state != "posted" + ).action_post() + for rma_line in self.rma_customer_id.rma_line_ids: + # The debits and credits are reconciled automatically + rma_line._compute_unreconciled() + self.assertFalse(rma_line.unreconciled) diff --git a/rma_purchase/tests/test_rma_stock_account_purchase.py b/rma_purchase/tests/test_rma_stock_account_purchase.py index 2bb25bf1..fe7d21d3 100644 --- a/rma_purchase/tests/test_rma_stock_account_purchase.py +++ b/rma_purchase/tests/test_rma_stock_account_purchase.py @@ -14,40 +14,40 @@ class TestRmaStockAccountPurchase(TestRmaStockAccount): super(TestRmaStockAccountPurchase, cls).setUpClass() cls.pol_model = cls.env["purchase.order.line"] cls.po_model = cls.env["purchase.order"] - # Create PO: - cls.product_fifo_1.standard_price = 1234 - cls.po = cls.po_model.create( - { - "partner_id": cls.partner_id.id, - } - ) - cls.pol_1 = cls.pol_model.create( - { - "name": cls.product_fifo_1.name, - "order_id": cls.po.id, - "product_id": cls.product_fifo_1.id, - "product_qty": 20.0, - "product_uom": cls.product_fifo_1.uom_id.id, - "price_unit": 100.0, - "date_planned": Datetime.now(), - } - ) - cls.po.button_confirm() - cls._do_picking(cls.po.picking_ids) def test_01_cost_from_po_move(self): """ Test the price unit is taken from the cost of the stock move associated to the PO """ + # Create PO: + self.product_fifo_1.standard_price = 1234 + po = self.po_model.create( + { + "partner_id": self.partner_id.id, + } + ) + pol_1 = self.pol_model.create( + { + "name": self.product_fifo_1.name, + "order_id": po.id, + "product_id": self.product_fifo_1.id, + "product_qty": 20.0, + "product_uom": self.product_fifo_1.uom_id.id, + "price_unit": 100.0, + "date_planned": Datetime.now(), + } + ) + po.button_confirm() + self._do_picking(po.picking_ids) self.product_fifo_1.standard_price = 1234 # this should not be taken supplier_view = self.env.ref("rma_purchase.view_rma_line_form") rma_line = Form( self.rma_line.with_context(supplier=1).with_user(self.rma_basic_user), view=supplier_view.id, ) - rma_line.partner_id = self.po.partner_id - rma_line.purchase_order_line_id = self.pol_1 + rma_line.partner_id = po.partner_id + rma_line.purchase_order_line_id = pol_1 rma_line.price_unit = 4356 rma_line = rma_line.save() rma_line.action_rma_to_approve() @@ -55,9 +55,9 @@ class TestRmaStockAccountPurchase(TestRmaStockAccount): # The price is not the standard price, is the value of the incoming layer # of the PO rma_move_value = picking.move_lines.stock_valuation_layer_ids.value - po_move_value = self.po.picking_ids.mapped( - "move_lines.stock_valuation_layer_ids" - )[-1].value + po_move_value = po.picking_ids.mapped("move_lines.stock_valuation_layer_ids")[ + -1 + ].value self.assertEqual(-rma_move_value, po_move_value) # Test the accounts used account_move = picking.move_lines.stock_valuation_layer_ids.account_move_id diff --git a/rma_sale/tests/test_rma_stock_account_sale.py b/rma_sale/tests/test_rma_stock_account_sale.py index 07eca91f..4b30aed3 100644 --- a/rma_sale/tests/test_rma_stock_account_sale.py +++ b/rma_sale/tests/test_rma_stock_account_sale.py @@ -70,5 +70,5 @@ class TestRmaStockAccountSale(TestRmaStockAccount): # Test the accounts used account_move = picking.move_lines.stock_valuation_layer_ids.account_move_id self.check_accounts_used( - account_move, debit_account="inventory", credit_account="cogs" + account_move, debit_account="inventory", credit_account="gdni" ) From 59a94da9bcd65b2d53ce1d18b3dfb4614efbd913 Mon Sep 17 00:00:00 2001 From: Jordi Ballester Alomar Date: Thu, 24 Nov 2022 18:59:36 +0100 Subject: [PATCH 2/2] fix pylint --- rma_account/tests/test_rma_stock_account.py | 2 +- .../tests/test_rma_account_unreconciled.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rma_account/tests/test_rma_stock_account.py b/rma_account/tests/test_rma_stock_account.py index a0ce458b..1e160fee 100644 --- a/rma_account/tests/test_rma_stock_account.py +++ b/rma_account/tests/test_rma_stock_account.py @@ -172,7 +172,7 @@ class TestRmaStockAccount(TestRma): # When we received we Credited to GDNI 30 # When we refund we Debit to GDNI 10 self.assertEqual(gdni_balance, -20.0) - make_refund = self.rma_refund_wizwith_context( + make_refund = self.rma_refund_wiz.with_context( **{ "customer": True, "active_ids": rma.ids, diff --git a/rma_account_unreconciled/tests/test_rma_account_unreconciled.py b/rma_account_unreconciled/tests/test_rma_account_unreconciled.py index 42ce79dd..9d1a43b4 100644 --- a/rma_account_unreconciled/tests/test_rma_account_unreconciled.py +++ b/rma_account_unreconciled/tests/test_rma_account_unreconciled.py @@ -54,8 +54,8 @@ class TestRmaAccountUnreconciled(TestRma): rma_line.action_rma_approve() self.assertFalse(rma_line.unreconciled) self.rma_customer_id.rma_line_ids.action_rma_to_approve() - wizard = self.rma_make_picking.with_context( - { + wizard = self.rma_make_pickingwith_context( + **{ "active_ids": self.rma_customer_id.rma_line_ids.ids, "active_model": "rma.order.line", "picking_type": "incoming", @@ -73,7 +73,7 @@ class TestRmaAccountUnreconciled(TestRma): rma_line._compute_unreconciled() self.assertTrue(rma_line.unreconciled) make_refund = self.rma_refund_wiz.with_context( - { + **{ "customer": True, "active_ids": self.rma_customer_id.rma_line_ids.ids, "active_model": "rma.order.line",