From 043dadaa913c9646cf83d2a7d0293b4766aae07f Mon Sep 17 00:00:00 2001
From: det <det@odoo.com>
Date: Fri, 24 Feb 2023 15:39:36 +0000
Subject: [PATCH] [FIX] hr_expense: currency and payable line discrepancy

Explanation:

A rework of hr_expense has been introduced in master and landed in 16.2.
https://github.com/odoo/odoo/commit/f79ff3fb374650670e89f231aaf17bb8d6596c25

There was a need to backport it to 16.0 then 16.1.
https://github.com/odoo/odoo/commit/90affb562962d0dc233526e3f15f5b60f383331d

During the backport;
- a computed fields 'journal_displayed_id' has been introduced and will need to be removed again in master.
- 2 bugs have been discovered:
-- the analytic.account were not created anymore (function not called)
-- the payment terms for expense paid by company could not be modified as a filter was missing

closes odoo/odoo#115650

X-original-commit: 672f4efd2c990ab6eef8f934c41625860b8ded46
Signed-off-by: Laurent Smet <las@odoo.com>
Signed-off-by: Detry Thomas (det) <det@odoo.com>
---
 addons/account/tests/common.py                | 14 ++---
 addons/account/views/account_payment_view.xml |  6 +-
 addons/hr_expense/i18n/hr_expense.pot         | 21 +++++++
 addons/hr_expense/models/account_move.py      |  4 +-
 addons/hr_expense/models/account_move_line.py |  6 --
 addons/hr_expense/models/account_payment.py   |  3 +
 addons/hr_expense/models/hr_expense.py        | 13 +++-
 addons/hr_expense/tests/common.py             | 13 ++++
 addons/hr_expense/tests/test_expenses.py      | 62 +++++++++----------
 .../hr_expense/views/account_move_views.xml   |  4 +-
 addons/sale_expense/models/hr_expense.py      | 14 -----
 .../sale_expense/models/hr_expense_sheet.py   | 14 +++++
 12 files changed, 108 insertions(+), 66 deletions(-)

diff --git a/addons/account/tests/common.py b/addons/account/tests/common.py
index 50743012fc5..5d7d68de955 100644
--- a/addons/account/tests/common.py
+++ b/addons/account/tests/common.py
@@ -1,6 +1,6 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
-from odoo import fields
+from odoo import fields, Command
 from odoo.tests.common import TransactionCase, HttpCase, tagged, Form
 
 import json
@@ -60,7 +60,7 @@ class AccountTestInvoicingCommon(TransactionCase):
         cls.company_data = cls.setup_company_data('company_1_data', chart_template=chart_template_ref)
 
         user.write({
-            'company_ids': [(6, 0, (cls.company_data['company'] + cls.company_data_2['company']).ids)],
+            'company_ids': [Command.set((cls.company_data['company'] + cls.company_data_2['company']).ids)],
             'company_id': cls.company_data['company'].id,
         })
 
@@ -81,8 +81,8 @@ class AccountTestInvoicingCommon(TransactionCase):
             'standard_price': 800.0,
             'property_account_income_id': cls.company_data['default_account_revenue'].id,
             'property_account_expense_id': cls.company_data['default_account_expense'].id,
-            'taxes_id': [(6, 0, cls.tax_sale_a.ids)],
-            'supplier_taxes_id': [(6, 0, cls.tax_purchase_a.ids)],
+            'taxes_id': [Command.set(cls.tax_sale_a.ids)],
+            'supplier_taxes_id': [Command.set(cls.tax_purchase_a.ids)],
         })
         cls.product_b = cls.env['product.product'].create({
             'name': 'product_b',
@@ -91,8 +91,8 @@ class AccountTestInvoicingCommon(TransactionCase):
             'standard_price': 160.0,
             'property_account_income_id': cls.copy_account(cls.company_data['default_account_revenue']).id,
             'property_account_expense_id': cls.copy_account(cls.company_data['default_account_expense']).id,
-            'taxes_id': [(6, 0, (cls.tax_sale_a + cls.tax_sale_b).ids)],
-            'supplier_taxes_id': [(6, 0, (cls.tax_purchase_a + cls.tax_purchase_b).ids)],
+            'taxes_id': [Command.set((cls.tax_sale_a + cls.tax_sale_b).ids)],
+            'supplier_taxes_id': [Command.set((cls.tax_purchase_a + cls.tax_purchase_b).ids)],
         })
 
         # ==== Fiscal positions ====
@@ -666,7 +666,7 @@ class TestAccountReconciliationCommon(AccountTestInvoicingCommon):
                 'name': 'product that cost %s' % invoice_amount,
                 'quantity': 1,
                 'price_unit': invoice_amount,
-                'tax_ids': [(6, 0, [])],
+                'tax_ids': [Command.set([])],
             })]
         }
 
diff --git a/addons/account/views/account_payment_view.xml b/addons/account/views/account_payment_view.xml
index 2132705d522..c2d1107eb1c 100644
--- a/addons/account/views/account_payment_view.xml
+++ b/addons/account/views/account_payment_view.xml
@@ -227,8 +227,10 @@
                             </button>
 
                             <!-- Journal Entry  button -->
-                            <button name="button_open_journal_entry" type="object"
-                                    class="oe_stat_button" icon="fa-bars"
+                            <button name="button_open_journal_entry"
+                                    type="object"
+                                    class="oe_stat_button"
+                                    icon="fa-bars"
                                     groups="account.group_account_user,account.group_account_readonly">
                                 <div class="o_stat_info">
                                     <span class="o_stat_text">Journal Entry</span>
diff --git a/addons/hr_expense/i18n/hr_expense.pot b/addons/hr_expense/i18n/hr_expense.pot
index b984555d1a1..74b4abbe600 100644
--- a/addons/hr_expense/i18n/hr_expense.pot
+++ b/addons/hr_expense/i18n/hr_expense.pot
@@ -714,6 +714,7 @@ msgstr ""
 #: model_terms:ir.ui.view,arch_db:hr_expense.hr_expense_sheet_view_search
 #: model_terms:ir.ui.view,arch_db:hr_expense.view_hr_expense_sheet_tree
 #: model_terms:ir.ui.view,arch_db:hr_expense.view_move_form_inherit_expense
+#: model_terms:ir.ui.view,arch_db:hr_expense.view_payment_form_inherit_expense
 msgid "Expense Report"
 msgstr ""
 
@@ -1066,6 +1067,7 @@ msgstr ""
 
 #. module: hr_expense
 #: model:ir.model,name:hr_expense.model_account_journal
+#: model:ir.model.fields,field_description:hr_expense.field_hr_expense_sheet__journal_displayed_id
 #: model_terms:ir.ui.view,arch_db:hr_expense.hr_expense_sheet_view_search
 msgid "Journal"
 msgstr ""
@@ -1417,6 +1419,18 @@ msgstr ""
 msgid "Payment Status"
 msgstr ""
 
+#. module: hr_expense
+#. odoo-python
+#: code:addons/hr_expense/models/account_payment.py:0
+#, python-format
+msgid "Payment created for: %s"
+msgstr ""
+
+#. module: hr_expense
+#: model:ir.model,name:hr_expense.model_account_payment
+msgid "Payments"
+msgstr ""
+
 #. module: hr_expense
 #: model_terms:product.template,description:hr_expense.expense_product_communication_product_template
 msgid "Phone bills, postage, etc."
@@ -2163,6 +2177,13 @@ msgstr ""
 msgid "You cannot delete a posted or approved expense."
 msgstr ""
 
+#. module: hr_expense
+#. odoo-python
+#: code:addons/hr_expense/models/account_payment.py:0
+#, python-format
+msgid "You cannot do this modification since the payment is linked to an expense report."
+msgstr ""
+
 #. module: hr_expense
 #. odoo-python
 #: code:addons/hr_expense/models/hr_expense.py:0
diff --git a/addons/hr_expense/models/account_move.py b/addons/hr_expense/models/account_move.py
index b7bb1562d17..9fc829ed4e8 100644
--- a/addons/hr_expense/models/account_move.py
+++ b/addons/hr_expense/models/account_move.py
@@ -45,9 +45,11 @@ class AccountMove(models.Model):
     @api.depends('expense_sheet_id')
     def _compute_needed_terms(self):
         # EXTENDS account
+        # We want to set the account destination based on the 'payment_mode'.
         super()._compute_needed_terms()
         for move in self:
             if move.expense_sheet_id:
+                balance = -sum(move.line_ids.filtered(lambda l: l.display_type != 'payment_term').mapped("amount_currency"))
                 move.needed_terms = {
                     frozendict(
                         {
@@ -56,7 +58,7 @@ class AccountMove(models.Model):
                             or fields.Date.context_today(move.expense_sheet_id),
                         }
                     ): {
-                        "balance": -sum(move.line_ids.mapped("balance")),
+                        "balance": balance,
                         "name": "",
                         "account_id": move.expense_sheet_id._get_expense_account_destination(),
                     }
diff --git a/addons/hr_expense/models/account_move_line.py b/addons/hr_expense/models/account_move_line.py
index 79247880449..00fa11683c6 100644
--- a/addons/hr_expense/models/account_move_line.py
+++ b/addons/hr_expense/models/account_move_line.py
@@ -40,12 +40,6 @@ class AccountMoveLine(models.Model):
         super(AccountMoveLine, expenses.with_context(force_price_include=True))._compute_totals()
         super(AccountMoveLine, self - expenses)._compute_totals()
 
-    def _compute_term_key(self):
-        super()._compute_term_key()
-        for line in self:
-            if line.expense_id:
-                line.term_key = line.term_key and frozendict(**line.term_key, expense_id=line.expense_id.id)
-
     def _convert_to_tax_base_line_dict(self):
         result = super()._convert_to_tax_base_line_dict()
         if self.expense_id:
diff --git a/addons/hr_expense/models/account_payment.py b/addons/hr_expense/models/account_payment.py
index b3fe58b6de5..a16a8af66e5 100644
--- a/addons/hr_expense/models/account_payment.py
+++ b/addons/hr_expense/models/account_payment.py
@@ -20,6 +20,7 @@ class AccountPayment(models.Model):
         }
 
     def _synchronize_from_moves(self, changed_fields):
+        # EXTENDS account
         if self.expense_sheet_id:
             # Constraints bypass when entry is linked to an expense.
             # Context is not enough, as we want to be able to delete
@@ -28,11 +29,13 @@ class AccountPayment(models.Model):
         return super()._synchronize_from_moves(changed_fields)
 
     def _synchronize_to_moves(self, changed_fields):
+        # EXTENDS account
         if self.expense_sheet_id:
             raise UserError(_("You cannot do this modification since the payment is linked to an expense report."))
         return super()._synchronize_to_moves(changed_fields)
 
     def _creation_message(self):
+        # EXTENDS mail
         self.ensure_one()
         if self.move_id.expense_sheet_id:
             return _("Payment created for: %s", self.move_id.expense_sheet_id._get_html_link())
diff --git a/addons/hr_expense/models/hr_expense.py b/addons/hr_expense/models/hr_expense.py
index 7d5f44e208d..e01465ca73a 100644
--- a/addons/hr_expense/models/hr_expense.py
+++ b/addons/hr_expense/models/hr_expense.py
@@ -576,14 +576,16 @@ class HrExpense(models.Model):
             'name': self.employee_id.name + ': ' + self.name.split('\n')[0][:64],
             'account_id': self.account_id.id,
             'quantity': self.quantity or 1,
-            'price_unit': self.unit_amount if self.unit_amount != 0 else self.total_amount,
+            # 'unit_amount' is there when the product selected has a cost defined.
+            # This cost will always be in company currency.
+            'price_unit': self.unit_amount if self.unit_amount != 0 else self.total_amount_company,
             'product_id': self.product_id.id,
             'product_uom_id': self.product_uom_id.id,
             'analytic_distribution': self.analytic_distribution,
             'expense_id': self.id,
             'partner_id': False if self.payment_mode == 'company_account' else self.employee_id.sudo().address_home_id.commercial_partner_id.id,
             'tax_ids': [Command.set(self.tax_ids.ids)],
-            'currency_id': self.currency_id.id,
+            'currency_id': self.company_currency_id.id,
         }
 
     @api.model
@@ -868,6 +870,7 @@ class HrExpenseSheet(models.Model):
     accounting_date = fields.Date("Accounting Date")
     account_move_id = fields.Many2one('account.move', string='Journal Entry', ondelete='set null', copy=False, readonly=True)
     journal_id = fields.Many2one('account.journal', compute='_compute_journal_id', string="Expense Journal", store=True)
+    journal_displayed_id = fields.Many2one('account.journal', compute='_compute_journal_displayed_id') # fix in stable TODO: remove
 
     # === Security fields === #
     can_reset = fields.Boolean('Can Reset', compute='_compute_can_reset')
@@ -879,6 +882,12 @@ class HrExpenseSheet(models.Model):
         ('journal_id_required_posted', "CHECK((state IN ('post', 'done') AND journal_id IS NOT NULL) OR (state NOT IN ('post', 'done')))", 'The journal must be set on posted expense'),
     ]
 
+    # TODO: remove
+    @api.depends('journal_id')
+    def _compute_journal_displayed_id(self):
+        for sheet in self:
+            sheet.journal_displayed_id = sheet.journal_id
+
     @api.depends('expense_line_ids.total_amount_company', 'expense_line_ids.amount_tax_company')
     def _compute_amount(self):
         for sheet in self:
diff --git a/addons/hr_expense/tests/common.py b/addons/hr_expense/tests/common.py
index 1a2e3903c98..6292cd401fa 100644
--- a/addons/hr_expense/tests/common.py
+++ b/addons/hr_expense/tests/common.py
@@ -1,6 +1,8 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
+from odoo import Command
+
 from odoo.addons.account.tests.common import AccountTestInvoicingCommon
 from odoo.addons.mail.tests.common import mail_new_test_user
 
@@ -61,5 +63,16 @@ class TestExpenseCommon(AccountTestInvoicingCommon):
             'plan_id': cls.analytic_plan.id,
         })
 
+        cls.product_c = cls.env['product.product'].create({
+            'name': 'product_c with no cost',
+            'uom_id': cls.env.ref('uom.product_uom_dozen').id,
+            'lst_price': 200.0,
+            'property_account_income_id': cls.copy_account(cls.company_data['default_account_revenue']).id,
+            'property_account_expense_id': cls.copy_account(cls.company_data['default_account_expense']).id,
+            'taxes_id': [Command.set((cls.tax_sale_a + cls.tax_sale_b).ids)],
+            'supplier_taxes_id': [Command.set((cls.tax_purchase_a + cls.tax_purchase_b).ids)],
+            'can_be_expensed': True,
+        })
+
         # Ensure products can be expensed.
         (cls.product_a + cls.product_b).write({'can_be_expensed': True})
diff --git a/addons/hr_expense/tests/test_expenses.py b/addons/hr_expense/tests/test_expenses.py
index ae8e046515f..a6a26ce43e8 100644
--- a/addons/hr_expense/tests/test_expenses.py
+++ b/addons/hr_expense/tests/test_expenses.py
@@ -185,7 +185,7 @@ class TestExpenses(TestExpenseCommon):
             'expense_line_ids': [
                 (0, 0, {
                     # Expense without foreign currency.
-                    'name': 'expense_1',
+                    'name': 'expense_company_currency',
                     'date': '2016-01-01',
                     'product_id': self.product_a.id,
                     'unit_amount': 1000.0,
@@ -195,10 +195,10 @@ class TestExpenses(TestExpenseCommon):
                 }),
                 (0, 0, {
                     # Expense with foreign currency (rate 1:3).
-                    'name': 'expense_1',
+                    'name': 'expense_foreign_currency',
                     'date': '2016-01-01',
-                    'product_id': self.product_b.id,
-                    'unit_amount': 1500.0,
+                    'product_id': self.product_c.id, # product with no cost, else not possible to enter amount in different currency
+                    'total_amount': 1500.0,
                     'tax_ids': [(6, 0, self.company_data['default_tax_purchase'].ids)],
                     'analytic_distribution': {self.analytic_account_2.id: 100},
                     'currency_id': self.currency_data['currency'].id,
@@ -218,8 +218,8 @@ class TestExpenses(TestExpenseCommon):
             # Receivable line (company currency):
             {
                 'debit': 0.0,
-                'credit': 1750.0,
-                'amount_currency': -1750.0,
+                'credit': 1500.0,
+                'amount_currency': -1500.0,
                 'account_id': self.company_data['default_account_payable'].id,
                 'product_id': False,
                 'currency_id': self.company_data['currency'].id,
@@ -228,12 +228,12 @@ class TestExpenses(TestExpenseCommon):
             },
             # Tax line (foreign currency):
             {
-                'debit': 97.83,
+                'debit': 65.22,
                 'credit': 0.0,
-                'amount_currency': 195.652,
+                'amount_currency': 65.22,
                 'account_id': self.company_data['default_account_tax_purchase'].id,
                 'product_id': False,
-                'currency_id': self.currency_data['currency'].id,
+                'currency_id': self.company_data['currency'].id,
                 'tax_line_id': self.company_data['default_tax_purchase'].id,
                 'analytic_distribution': False,
             },
@@ -250,18 +250,18 @@ class TestExpenses(TestExpenseCommon):
             },
             # Product line (foreign currency):
             {
-                'debit': 652.17,
+                'debit': 434.78, # 1500 * 1:3 (rate) / 1.15 (incl. tax)
                 'credit': 0.0,
-                'amount_currency': 1304.348, # untaxed amount
-                'account_id': self.product_b.property_account_expense_id.id,
-                'product_id': self.product_b.id,
-                'currency_id': self.currency_data['currency'].id,
+                'amount_currency': 434.78, # untaxed amount
+                'account_id': self.product_c.property_account_expense_id.id,
+                'product_id': self.product_c.id,
+                'currency_id': self.company_data['currency'].id,
                 'tax_line_id': False,
                 'analytic_distribution': {str(self.analytic_account_2.id): 100},
             },
             # Product line (company currency):
             {
-                'debit': 869.57,
+                'debit': 869.57, # 1000 * 1:1 (rate) / 1.15 (incl. tax)
                 'credit': 0.0,
                 'amount_currency': 869.57,
                 'account_id': self.company_data['default_account_expense'].id,
@@ -281,7 +281,7 @@ class TestExpenses(TestExpenseCommon):
                 'currency_id': self.company_data['currency'].id,
             },
             {
-                'amount': -652.17,
+                'amount': -434.78,
                 'date': fields.Date.from_string('2017-01-01'),
                 'account_id': self.analytic_account_2.id,
                 'currency_id': self.company_data['currency'].id,
@@ -319,7 +319,7 @@ class TestExpenses(TestExpenseCommon):
             'employee_id': self.expense_employee.id,
         })
         tax = self.env['account.tax'].create({
-            'name': 'Expense 10%',
+            'name': 'Tax Expense 10%',
             'amount': 10,
             'amount_type': 'percent',
             'type_tax_use': 'purchase',
@@ -328,12 +328,12 @@ class TestExpenses(TestExpenseCommon):
         self.env['hr.expense'].create({
             'name': 'Choucroute Saucisse',
             'employee_id': self.expense_employee.id,
-            'product_id': self.product_a.id,
-            'unit_amount': 700.00,
+            'product_id': self.product_c.id, # product with no cost, else not possible to enter amount in different currency
+            'total_amount': 700.0,
             'tax_ids': [(6, 0, tax.ids)],
             'sheet_id': expense.id,
             'analytic_distribution': {self.analytic_account_1.id: 100},
-            'currency_id': self.currency_data['currency'].id,
+            'currency_id': self.currency_data['currency'].id, # rate is 1:2
         })
 
         # State should default to draft
@@ -352,16 +352,16 @@ class TestExpenses(TestExpenseCommon):
         self.assertEqual(len(analytic_line), 1)
         self.assertInvoiceValues(expense.account_move_id, [
             {
-                'balance': 318.18,
-                'amount_currency': 636.364,
-                'product_id': self.product_a.id,
-                'price_unit': 700.0,
-                'price_subtotal': 636.364,
-                'price_total': 700.0,
+                'balance': 318.18, # 700 * 1:2 (rate) / 1.1 (incl. tax)
+                'amount_currency': 318.18,
+                'product_id': self.product_c.id,
+                'price_unit': 350.0,
+                'price_subtotal': 318.18,
+                'price_total': 350.0,
                 'analytic_line_ids': analytic_line.ids,
             }, {
                 'balance': 31.82,
-                'amount_currency': 63.636,
+                'amount_currency': 31.82,
                 'product_id': False,
                 'price_unit': 0.0,
                 'price_subtotal': 0.0,
@@ -377,7 +377,7 @@ class TestExpenses(TestExpenseCommon):
                 'analytic_line_ids': [],
             },
         ], {
-            'amount_total': 700.0,
+            'amount_total': 350.0,
         })
 
     def test_expenses_with_tax_and_lockdate(self):
@@ -568,7 +568,7 @@ class TestExpenses(TestExpenseCommon):
         self.assertRecordValues(expense_sheet.account_move_id.line_ids.sorted('balance'), [
             # Receivable lines:
             {
-                'balance': -345.0,
+                'balance': -345.0, # 115 + 230
                 'account_id': self.company_data['default_account_payable'].id,
             },
             # Tax lines:
@@ -582,12 +582,12 @@ class TestExpenses(TestExpenseCommon):
             },
             # Expense line 1:
             {
-                'balance': 100.0,
+                'balance': 100.0, # 115 / 1.15 (tax incl.)
                 'account_id': account_expense_1.id,
             },
             # Expense line 2:
             {
-                'balance': 200.0,
+                'balance': 200.0, # 230 / 1.15 (tax incl.)
                 'account_id': account_expense_2.id,
             },
         ])
diff --git a/addons/hr_expense/views/account_move_views.xml b/addons/hr_expense/views/account_move_views.xml
index 2dcf00fc992..76bfd69e185 100644
--- a/addons/hr_expense/views/account_move_views.xml
+++ b/addons/hr_expense/views/account_move_views.xml
@@ -13,9 +13,7 @@
                             string="Expense Report"
                             icon="fa-file-text-o"
                             type="object"
-                            attrs="{'invisible': [('expense_sheet_id', '=', [])]}"
-                            groups="account.group_account_user,account.group_account_readonly">
-                        </button>
+                            attrs="{'invisible': [('expense_sheet_id', '=', [])]}"/>
                 </xpath>
             </field>
         </record>
diff --git a/addons/sale_expense/models/hr_expense.py b/addons/sale_expense/models/hr_expense.py
index 6299dbb46b4..536cd75cea6 100644
--- a/addons/sale_expense/models/hr_expense.py
+++ b/addons/sale_expense/models/hr_expense.py
@@ -42,17 +42,3 @@ class Expense(models.Model):
         for split_value in vals:
             split_value['sale_order_id'] = self.sale_order_id.id
         return vals
-
-    def action_move_create(self):
-        """ When posting expense, if the AA is given, we will track cost in that
-            If a SO is set, this means we want to reinvoice the expense. But to do so, we
-            need the analytic entries to be generated, so a AA is required to reinvoice. So,
-            we ensure the AA if a SO is given.
-        """
-        for expense in self.filtered(lambda expense: expense.sale_order_id and not expense.analytic_distribution):
-            if not expense.sale_order_id.analytic_account_id:
-                expense.sale_order_id._create_analytic_account()
-            expense.write({
-                'analytic_distribution': {expense.sale_order_id.analytic_account_id.id: 100}
-            })
-        return super(Expense, self).action_move_create()
diff --git a/addons/sale_expense/models/hr_expense_sheet.py b/addons/sale_expense/models/hr_expense_sheet.py
index b65d0ee4716..768373dbf59 100644
--- a/addons/sale_expense/models/hr_expense_sheet.py
+++ b/addons/sale_expense/models/hr_expense_sheet.py
@@ -34,3 +34,17 @@ class HrExpenseSheet(models.Model):
             'name': _('Reinvoiced Sales Orders'),
             'domain': [('id', 'in', self.expense_line_ids.sale_order_id.ids)],
         }
+
+    def _do_create_moves(self):
+        """ When posting expense, if the AA is given, we will track cost in that
+            If a SO is set, this means we want to reinvoice the expense. But to do so, we
+            need the analytic entries to be generated, so a AA is required to reinvoice. So,
+            we ensure the AA if a SO is given.
+        """
+        for expense in self.expense_line_ids.filtered(lambda expense: expense.sale_order_id and not expense.analytic_distribution):
+            if not expense.sale_order_id.analytic_account_id:
+                expense.sale_order_id._create_analytic_account()
+            expense.write({
+                'analytic_distribution': {expense.sale_order_id.analytic_account_id.id: 100}
+            })
+        return super()._do_create_moves()