From 943075fd94eec2bc61b751e8828e3bd69054b2f8 Mon Sep 17 00:00:00 2001 From: oco-odoo Date: Tue, 22 May 2018 10:20:49 +0200 Subject: [PATCH] [FIX] purchase: anglosaxon stock valuation: use correct currency rates to determine whether or not to create a price difference line for invoices. (#24815) Before that, the following scenario (tested in purchase/tests/test_anglosaxon_valuation_reconciliation.py) failed: - create a PO - change of currency rate - create and validate vendor bill in foreign currency - receive goods - change of currency rate - return the goods - change of currency rate - refund the bill At the end of it, both invoices (bill and refund) were not in 'paid' state, but they should have been! The anglosaxon stock valuation code had some issues in the way it managed currency rates. The main one was that we converted the computed valuation_price_unit variable in company currency using the date of the invoice as a reference, which was wrong in case the invoice was a refund, as it should then use the date of the valuation for this. This problem caused the code to detect an exchange rate difference between refund and valuation as a price difference, hence generating a line in the price difference account, which caused the debtor account line of the associated account.move to be wrong, and hence impacted the residual field of the invoice ("amount due", in GUI). Finally, as this field contained an inconsistent value, the refund invoice was not marked as paid after reconciling it with the original one. Solving this needed to refactor the code quite a bit, so that we don't convert everything in invoice currency in the end, but rather do it directly everytime we compute a part of valuation_price_unit. The test had also to be partly re-written as it was too naive in the way it used currency rates, we needed something closer to a real utilisation of the program. --- addons/purchase/models/account_invoice.py | 20 ++++----- ...st_anglo_saxon_valuation_reconciliation.py | 43 ++++++++++++++++--- ...o_saxon_valuation_reconciliation_common.py | 11 ++++- 3 files changed, 56 insertions(+), 18 deletions(-) diff --git a/addons/purchase/models/account_invoice.py b/addons/purchase/models/account_invoice.py index 23d40d15452..fd01afa29a0 100644 --- a/addons/purchase/models/account_invoice.py +++ b/addons/purchase/models/account_invoice.py @@ -163,12 +163,14 @@ class AccountInvoice(models.Model): # calculate and write down the possible price difference between invoice price and product price for line in res: if line.get('invl_id', 0) == i_line.id and reference_account_id == line['account_id']: - valuation_price_unit = i_line.product_id.uom_id._compute_price(i_line.product_id.standard_price, i_line.uom_id) + # valuation_price unit is always expressed in invoice currency, so that it can always be computed with the good rate + valuation_price_unit = company_currency.compute(i_line.product_id.uom_id._compute_price(i_line.product_id.standard_price, i_line.uom_id), inv.currency_id, round=False) line_quantity = line['quantity'] if i_line.product_id.cost_method != 'standard' and i_line.purchase_line_id: + po_currency = i_line.purchase_id.currency_id #for average/fifo/lifo costing method, fetch real cost price from incomming moves - valuation_price_unit = i_line.purchase_line_id.product_uom._compute_price(i_line.purchase_line_id.price_unit, i_line.uom_id) + valuation_price_unit = po_currency.with_context(date=inv.date_invoice).compute(i_line.purchase_line_id.product_uom._compute_price(i_line.purchase_line_id.price_unit, i_line.uom_id), inv.currency_id, round=False) stock_move_obj = self.env['stock.move'] valuation_stock_move = stock_move_obj.search([ ('purchase_line_id', '=', i_line.purchase_line_id.id), @@ -183,22 +185,20 @@ class AccountInvoice(models.Model): valuation_price_unit_total = 0 valuation_total_qty = 0 for val_stock_move in valuation_stock_move: - valuation_price_unit_total += abs(val_stock_move.price_unit) * val_stock_move.product_qty + # In case val_stock_move is a return move, its valuation entries have been made with the + # currency rate corresponding to the original stock move + valuation_date = val_stock_move.origin_returned_move_id.date or val_stock_move.date_expected + valuation_price_unit_total += company_currency.with_context(date=valuation_date).compute(abs(val_stock_move.price_unit) * val_stock_move.product_qty, inv.currency_id, round=False) valuation_total_qty += val_stock_move.product_qty valuation_price_unit = valuation_price_unit_total / valuation_total_qty valuation_price_unit = i_line.product_id.uom_id._compute_price(valuation_price_unit, i_line.uom_id) line_quantity = valuation_total_qty - elif i_line.product_id.cost_method == 'real': + elif i_line.product_id.cost_method == 'fifo': # In this condition, we have a real price-valuated product which has not yet been received - valuation_price_unit = i_line.purchase_line_id.price_unit + valuation_price_unit = po_currency.with_context(date=inv.date_invoice).compute(i_line.purchase_line_id.price_unit, inv.currency_id, round=False) interim_account_price = valuation_price_unit * line_quantity - if inv.currency_id.id != company_currency.id: - # We express everyhting in the invoice currency - valuation_price_unit = company_currency.with_context(date=inv.date_invoice).compute(valuation_price_unit, inv.currency_id, round=False) - interim_account_price = company_currency.with_context(date=inv.date_invoice).compute(interim_account_price, inv.currency_id, round=False) - invoice_cur_prec = inv.currency_id.decimal_places if float_compare(valuation_price_unit, i_line.price_unit, precision_digits=invoice_cur_prec) != 0 and float_compare(line['price_unit'], i_line.price_unit, precision_digits=invoice_cur_prec) == 0: diff --git a/addons/purchase/tests/test_anglo_saxon_valuation_reconciliation.py b/addons/purchase/tests/test_anglo_saxon_valuation_reconciliation.py index fdb0311662b..ab20b43b524 100644 --- a/addons/purchase/tests/test_anglo_saxon_valuation_reconciliation.py +++ b/addons/purchase/tests/test_anglo_saxon_valuation_reconciliation.py @@ -36,8 +36,9 @@ class TestValuationReconciliation(ValuationReconciliationTestCase): 'product_qty': quantity, 'product_uom': product.uom_po_id.id, 'price_unit': self.product_price_unit, - 'date_planned': datetime.today(), + 'date_planned': '2017-10-07', })], + 'date_order': '2017-10-01', }) rslt.button_confirm() return rslt @@ -51,7 +52,7 @@ class TestValuationReconciliation(ValuationReconciliationTestCase): 'currency_id': self.currency_two.id, 'name': 'vendor bill', 'type': 'in_invoice', - 'date_invoice': time.strftime('%Y') + '-12-22', + 'date_invoice': '2017-12-31', 'account_id': account_receivable.id, }) rslt.purchase_order_change() @@ -76,6 +77,7 @@ class TestValuationReconciliation(ValuationReconciliationTestCase): def test_invoice_shipment(self): """ Tests the case into which we make the invoice first, and then receive the goods. """ + # Create a PO and an invoice for it test_product = self.test_product_order purchase_order = self._create_purchase(test_product) @@ -83,15 +85,29 @@ class TestValuationReconciliation(ValuationReconciliationTestCase): invoice_line = self.env['account.invoice.line'].search([('invoice_id', '=', invoice.id)]) invoice_line.quantity = 1 - self.currency_rate.rate = 13.834739702 + # The currency rate changes + self.currency_rate = self.env['res.currency.rate'].create({ + 'currency_id': self.currency_one.id, + 'company_id': self.company.id, + 'rate': 13.834739702, + 'name': '2017-12-22', + }) + # Validate the invoice and refund the goods invoice.action_invoice_open() - self._process_pickings(purchase_order.picking_ids) + self._process_pickings(purchase_order.picking_ids, date='2017-12-24') picking = self.env['stock.picking'].search([('purchase_id', '=', purchase_order.id)]) self.check_reconciliation(invoice, picking) - #return the goods and refund the invoice - self.currency_rate.rate = 10.54739702 + # The currency rate changes again + self.currency_rate = self.env['res.currency.rate'].create({ + 'currency_id': self.currency_one.id, + 'company_id': self.company.id, + 'rate': 10.54739702, + 'name': '2018-01-01', + }) + + # Return the goods and refund the invoice stock_return_picking = self.env['stock.return.picking']\ .with_context(active_ids=[picking.id], active_id=picking.id).create({}) stock_return_picking.product_return_moves.quantity = 1.0 @@ -100,12 +116,25 @@ class TestValuationReconciliation(ValuationReconciliationTestCase): return_pick.action_assign() return_pick.move_lines.quantity_done = 1 return_pick.action_done() - self.currency_rate.rate = 9.56564564 + self._change_pickings_date(return_pick, '2018-01-13') + + # The currency rate changes again + self.currency_rate = self.env['res.currency.rate'].create({ + 'currency_id': self.currency_one.id, + 'company_id': self.company.id, + 'rate': 9.56564564, + 'name': '2018-03-12', + }) + + # Refund the invoice refund_invoice_wiz = self.env['account.invoice.refund'].with_context(active_ids=[invoice.id]).create({ 'description': 'test_invoice_shipment_refund', 'filter_refund': 'cancel', + 'date': '2018-03-15', }) refund_invoice_wiz.invoice_refund() + + # Check the result refund_invoice = self.env['account.invoice'].search([('name', '=', 'test_invoice_shipment_refund')])[0] self.assertTrue(invoice.state == refund_invoice.state == 'paid'), "Invoice and refund should both be in 'Paid' state" self.check_reconciliation(refund_invoice, return_pick) diff --git a/addons/stock_account/tests/test_anglo_saxon_valuation_reconciliation_common.py b/addons/stock_account/tests/test_anglo_saxon_valuation_reconciliation_common.py index 3732cddbd7b..b2e3bd0d095 100644 --- a/addons/stock_account/tests/test_anglo_saxon_valuation_reconciliation_common.py +++ b/addons/stock_account/tests/test_anglo_saxon_valuation_reconciliation_common.py @@ -3,6 +3,7 @@ from odoo.addons.account.tests.account_test_classes import AccountingTestCase +from odoo import fields class ValuationReconciliationTestCase(AccountingTestCase): """ Base class for tests checking interim accounts reconciliation works @@ -24,13 +25,20 @@ class ValuationReconciliationTestCase(AccountingTestCase): else: self.assertFalse(valuation_line.full_reconcile_id, "The reconciliation should not be total at that point.") - def _process_pickings(self, pickings, quantity=False): + def _process_pickings(self, pickings, date=False, quantity=False): + if not date: + date = fields.Date.today() pickings.action_confirm() pickings.action_assign() for picking in pickings: for ml in picking.move_line_ids: ml.qty_done = quantity or ml.product_qty pickings.action_done() + self._change_pickings_date(pickings, date) + + def _change_pickings_date(self, pickings, date): + pickings.mapped('move_lines').write({'date': date}) + pickings.mapped('move_lines.account_move_ids').write({'date': date}) def _create_product_category(self): return self.env['product.category'].create({ @@ -54,6 +62,7 @@ class ValuationReconciliationTestCase(AccountingTestCase): 'currency_id': self.currency_one.id, 'company_id': self.company.id, 'rate': 1.234343354, # Totally arbitratry value + 'name': '2017-01-01', }) self.input_account = self.env['account.account'].create({