From e3902280e313d764a1f3437e4b8171359c10a21c Mon Sep 17 00:00:00 2001
From: "Julien Alardot (jual)" <jual@odoo.com>
Date: Thu, 2 Nov 2023 09:30:48 +0100
Subject: [PATCH] [FIX] hr_expense: Don't always recompute product cost

Fix a bug introduced by 67901a4429c69fbba96c32af5d8f58aff54f0be5

When an expense is submitted and for the steps after, there is no need
to recompute the product_cost as it may be confusing
or generate discrepancies with the account move by changing the totals.

task-3580004

closes odoo/odoo#141400

Signed-off-by: Olivier Colson (oco) <oco@odoo.com>
---
 addons/hr_expense/i18n/hr_expense.pot       |  5 +-
 addons/hr_expense/models/hr_expense.py      | 12 +++--
 addons/hr_expense/models/product_product.py |  4 +-
 addons/hr_expense/tests/test_expenses.py    | 56 +++++++++++++++++++++
 4 files changed, 70 insertions(+), 7 deletions(-)

diff --git a/addons/hr_expense/i18n/hr_expense.pot b/addons/hr_expense/i18n/hr_expense.pot
index 5938fd693b05..d7a4202ed14b 100644
--- a/addons/hr_expense/i18n/hr_expense.pot
+++ b/addons/hr_expense/i18n/hr_expense.pot
@@ -1971,8 +1971,9 @@ msgstr ""
 #: code:addons/hr_expense/models/product_product.py:0
 #, python-format
 msgid ""
-"There are unposted expenses linked to this category. Updating the category "
-"cost will change expense amounts. Make sure it is what you want to do."
+"There are unsubmitted expenses linked to this category. Updating the "
+"category cost will change expense amounts. Make sure it is what you want to "
+"do."
 msgstr ""
 
 #. module: hr_expense
diff --git a/addons/hr_expense/models/hr_expense.py b/addons/hr_expense/models/hr_expense.py
index 56ffb4177385..294b3bd75da9 100644
--- a/addons/hr_expense/models/hr_expense.py
+++ b/addons/hr_expense/models/hr_expense.py
@@ -117,13 +117,14 @@ class HrExpense(models.Model):
 
     @api.depends('product_has_cost')
     def _compute_currency_id(self):
-        for expense in self.filtered("product_has_cost"):
-            expense.currency_id = expense.company_currency_id
+        for expense in self:
+            if expense.product_has_cost and expense.state == 'draft':
+                expense.currency_id = expense.company_currency_id
 
     @api.onchange('product_has_cost')
     def _onchange_product_has_cost(self):
         # Reset quantity to 1, in case of 0-cost product
-        if not self.product_has_cost:
+        if not self.product_has_cost and self.state == 'draft':
             self.quantity = 1
 
     @api.depends('date', 'currency_id', 'company_currency_id', 'company_id')
@@ -149,6 +150,9 @@ class HrExpense(models.Model):
             expense.product_has_cost = expense.product_id and (float_compare(expense.product_id.standard_price, 0.0, precision_digits=2) != 0)
             tax_ids = expense.product_id.supplier_taxes_id.filtered(lambda tax: tax.company_id == expense.company_id)
             expense.product_has_tax = bool(tax_ids)
+            if not expense.product_has_cost and expense.state == 'draft':
+                expense.unit_amount = expense.total_amount_company
+                expense.quantity = 1
 
     @api.depends('sheet_id', 'sheet_id.account_move_id', 'sheet_id.state')
     def _compute_state(self):
@@ -293,6 +297,8 @@ class HrExpense(models.Model):
     @api.depends('product_id', 'attachment_number', 'currency_rate')
     def _compute_unit_amount(self):
         for expense in self:
+            if expense.state != 'draft':
+                continue
             product_id = expense.product_id
             if product_id and expense.product_has_cost and not expense.attachment_number or (expense.attachment_number and not expense.unit_amount):
                 expense.unit_amount = product_id.price_compute(
diff --git a/addons/hr_expense/models/product_product.py b/addons/hr_expense/models/product_product.py
index 51fb93e98bdb..adba8c46a7e5 100644
--- a/addons/hr_expense/models/product_product.py
+++ b/addons/hr_expense/models/product_product.py
@@ -9,7 +9,7 @@ class ProductProduct(models.Model):
     @api.onchange('standard_price')
     def _compute_standard_price_update_warning(self):
         undone_expenses = self.env['hr.expense']._read_group(
-            domain=[('state', 'in', ['draft', 'reported', 'approved']), ('product_id', 'in', self.ids)],
+            domain=[('state', '=', 'draft'), ('product_id', 'in', self.ids)],
             fields=['unit_amount:array_agg'],
             groupby=['product_id'],
             )
@@ -23,6 +23,6 @@ class ProductProduct(models.Model):
                 rounded_price = self.env.company.currency_id.round(product.standard_price)
                 if rounded_price and (len(unit_amounts_no_warning) > 1 or (len(unit_amounts_no_warning) == 1 and rounded_price not in unit_amounts_no_warning)):
                     product.standard_price_update_warning = _(
-                            "There are unposted expenses linked to this category. Updating the category cost will change expense amounts. "
+                            "There are unsubmitted expenses linked to this category. Updating the category cost will change expense amounts. "
                             "Make sure it is what you want to do."
                         )
diff --git a/addons/hr_expense/tests/test_expenses.py b/addons/hr_expense/tests/test_expenses.py
index 1f0218d73540..7878403db84d 100644
--- a/addons/hr_expense/tests/test_expenses.py
+++ b/addons/hr_expense/tests/test_expenses.py
@@ -1335,3 +1335,59 @@ class TestExpenses(TestExpenseCommon):
             sheet_names,
             "The report name should be 'New Expense Report, paid by (employee|company)' as a fallback",
         )
+
+    def test_expense_product_update(self):
+        """ Test that the expense line is correctly updated or not when its product price is updated."""
+        #pylint: disable=bad-whitespace
+        product = self.env['product.product'].create({
+            'name': 'product',
+            'uom_id': self.env.ref('uom.product_uom_unit').id,
+            'lst_price': 100.0,
+            'standard_price': 0.0,
+            'property_account_income_id': self.company_data['default_account_revenue'].id,
+            'property_account_expense_id': self.company_data['default_account_expense'].id,
+            'supplier_taxes_id': False,
+        })
+
+        sheet_no_update, sheet_update = sheets = self.env['hr.expense.sheet'].create([{
+            'company_id': self.env.company.id,
+            'employee_id': self.expense_employee.id,
+            'name': name,
+            'expense_line_ids': [
+                Command.create({
+                    'name': name,
+                    'date': '2016-01-01',
+                    'product_id': product.id,
+                    'total_amount': 100.0,
+                    'employee_id': self.expense_employee.id
+                }),
+            ],
+        } for name in ('test sheet no update', 'test sheet update')])
+
+        sheet_no_update.action_submit_sheet()  # No update when sheet is submitted
+        self.assertRecordValues(sheets.expense_line_ids.sorted('name'), [
+            {'name': 'test sheet no update', 'unit_amount': 100.0, 'quantity': 1, 'total_amount': 100.0},
+            {'name':    'test sheet update', 'unit_amount': 100.0, 'quantity': 1, 'total_amount': 100.0},
+        ])
+        product.standard_price = 50.0
+        self.assertRecordValues(sheets.expense_line_ids.sorted('name'), [
+            {'name': 'test sheet no update', 'unit_amount': 100.0, 'quantity': 1, 'total_amount': 100.0},
+            {'name':    'test sheet update', 'unit_amount':  50.0, 'quantity': 1, 'total_amount':  50.0},  # unit_amount is updated
+        ])
+        sheet_update.expense_line_ids.quantity = 5
+        self.assertRecordValues(sheets.expense_line_ids.sorted('name'), [
+            {'name': 'test sheet no update', 'unit_amount': 100.0, 'quantity': 1, 'total_amount': 100.0},
+            {'name':    'test sheet update', 'unit_amount':  50.0, 'quantity': 5, 'total_amount': 250.0},  # quantity & total are updated
+        ])
+        product.standard_price = 0.0
+        self.assertRecordValues(sheets.expense_line_ids.sorted('name'), [
+            {'name': 'test sheet no update', 'unit_amount': 100.0, 'quantity': 1, 'total_amount': 100.0},
+            {'name':    'test sheet update', 'unit_amount': 250.0, 'quantity': 1, 'total_amount': 250.0},  # quantity & unit_amount only are updated
+        ])
+
+        sheet_update.action_submit_sheet()  # This sheet should not be updated any more
+        product.standard_price = 300.0
+        self.assertRecordValues(sheets.expense_line_ids.sorted('name'), [
+            {'name': 'test sheet no update', 'unit_amount': 100.0, 'quantity': 1, 'total_amount': 100.0},
+            {'name':    'test sheet update', 'unit_amount': 250.0, 'quantity': 1, 'total_amount': 250.0},  # no update
+        ])
-- 
GitLab