[19.0][FIX] hr_expense_payment: derive payment links from reconciliation (rework for OCA#355 review)

hr_expense_payment/README.rst

@@ -32,11 +32,22 @@ HR Expense Payment
 
 |badge1| |badge2| |badge3| |badge4| |badge5|
 
-This module links each employee expense to the payment(s) that settled
-it. When you Register Payment on a posted employee-paid expense, the
-resulting payment back-links to the source expense, and vice versa. A
-post-install hook backfills the link for payments made before the module
-was installed.
+This module links each employee-paid expense to the payment(s) that
+settled it, in both directions: ``hr.expense.payment_ids`` and
+``account.payment.reconciled_expense_ids``.
+
+The link is computed from the reconciliation between the expense's
+journal entry and the payment's journal items — the same way core
+derives ``reconciled_bill_ids`` on payments. It therefore always
+reflects the current accounting state (partial payments included), needs
+no manual bookkeeping, and covers payments however they were registered
+(expense's Register Payment button, the journal entry's own button, or
+manual reconciliation).
+
+Odoo 19.0 core's ``account.payment.expense_ids`` only covers
+company-paid expenses (whose journal entry *is* the payment's move);
+this module covers the employee-reimbursement direction that core does
+not track.
 
 **Table of contents**
 

hr_expense_payment/__init__.py

@@ -1,5 +1,3 @@
 # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
 
 from . import models
-from . import wizard
-from .hooks import post_init_hook

hr_expense_payment/__manifest__.py

@@ -4,13 +4,12 @@
 
 {
     "name": "HR Expense Payment",
-    "version": "18.0.1.0.0",
+    "version": "19.0.1.0.0",
     "category": "Human Resources",
     "author": "Tecnativa, Ecosoft, Odoo Community Association (OCA)",
     "license": "AGPL-3",
     "website": "https://github.com/OCA/hr-expense",
     "depends": ["hr_expense"],
     "data": [],
     "installable": True,
-    "post_init_hook": "post_init_hook",
 }

hr_expense_payment/models/__init__.py

@@ -1,4 +1,4 @@
 # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
 
 from . import account_payment
-from . import hr_expense_sheet
+from . import hr_expense

hr_expense_payment/models/account_payment.py

@@ -2,18 +2,37 @@
 # Copyright 2021 Ecosoft Co., Ltd (http://ecosoft.co.th/)
 # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html).
 
-from odoo import fields, models
+from odoo import api, fields, models
 
 
 class AccountPayment(models.Model):
     _inherit = "account.payment"
 
-    expense_sheet_ids = fields.Many2many(
-        comodel_name="hr.expense.sheet",
-        relation="payment_expense_sheet_rel",
-        column1="payment_id",
-        column2="sheet_id",
-        string="Expense sheet",
-        readonly=True,
-        copy=False,
+    # Core's own expense_ids (related to move_id.expense_ids) only covers
+    # company-paid expenses; this covers the reimbursement direction, derived
+    # from reconciliation like core's reconciled_bill_ids.
+    reconciled_expense_ids = fields.Many2many(
+        comodel_name="hr.expense",
+        string="Reimbursed Expenses",
+        compute="_compute_reconciled_expense_ids",
+        compute_sudo=True,
+        search="_search_reconciled_expense_ids",
+        help="Employee-paid expenses whose journal items have been reconciled "
+        "with this payment.",
     )
+
+    @api.depends(
+        "move_id.line_ids.matched_debit_ids",
+        "move_id.line_ids.matched_credit_ids",
+    )
+    def _compute_reconciled_expense_ids(self):
+        for payment in self:
+            payment.reconciled_expense_ids = (
+                payment.move_id._get_reconciled_amls().move_id.expense_ids
+            )
+
+    def _search_reconciled_expense_ids(self, operator, value):
+        if operator not in ("in", "="):
+            return NotImplemented
+        expenses = self.env["hr.expense"].browse(value)
+        return [("id", "in", expenses.payment_ids.ids)]

hr_expense_payment/models/hr_expense.py

@@ -0,0 +1,35 @@
+# Copyright 2019 Tecnativa - Ernesto Tejeda
+# Copyright 2021 Ecosoft Co., Ltd (http://ecosoft.co.th/)
+# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html).
+
+from odoo import api, fields, models
+
+
+class HrExpense(models.Model):
+    _inherit = "hr.expense"
+
+    payment_ids = fields.Many2many(
+        comodel_name="account.payment",
+        string="Payments",
+        compute="_compute_payment_ids",
+        # Employees read their expenses without accounting access; the
+        # reconciliation walk needs sudo.
+        compute_sudo=True,
+        search="_search_payment_ids",
+        help="Payments whose journal items have been reconciled with this "
+        "expense's journal entry.",
+    )
+
+    @api.depends(
+        "account_move_id.line_ids.matched_debit_ids",
+        "account_move_id.line_ids.matched_credit_ids",
+    )
+    def _compute_payment_ids(self):
+        for expense in self:
+            expense.payment_ids = expense.account_move_id._get_reconciled_payments()
+
+    def _search_payment_ids(self, operator, value):
+        if operator not in ("in", "="):
+            return NotImplemented
+        payments = self.env["account.payment"].browse(value)
+        return [("id", "in", payments.reconciled_expense_ids.ids)]

hr_expense_payment/readme/DESCRIPTION.md

@@ -1,4 +1,14 @@
-This module links each employee expense to the payment(s) that settled it.
-When you Register Payment on a posted employee-paid expense, the resulting
-payment back-links to the source expense, and vice versa. A post-install
-hook backfills the link for payments made before the module was installed.
+This module links each employee-paid expense to the payment(s) that settled
+it, in both directions: `hr.expense.payment_ids` and
+`account.payment.reconciled_expense_ids`.
+
+The link is computed from the reconciliation between the expense's journal
+entry and the payment's journal items — the same way core derives
+`reconciled_bill_ids` on payments. It therefore always reflects the current
+accounting state (partial payments included), needs no manual bookkeeping,
+and covers payments however they were registered (expense's Register
+Payment button, the journal entry's own button, or manual reconciliation).
+
+Odoo 19.0 core's `account.payment.expense_ids` only covers company-paid
+expenses (whose journal entry *is* the payment's move); this module covers
+the employee-reimbursement direction that core does not track.

hr_expense_payment/static/description/index.html

@@ -3,7 +3,7 @@
 <head>
 <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
 <meta name="generator" content="Docutils: https://docutils.sourceforge.io/" />
-<title>HR Expense Payment</title>
+<title>README.rst</title>
 <style type="text/css">
 
 /*
@@ -375,11 +375,20 @@ <h1>HR Expense Payment</h1>
 !! source digest: sha256:5517c634f0be5269334c9330a559431ffb4ee03d0b2a1b7fcf65c70a1976ef95
 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -->
 <p><a class="reference external image-reference" href="https://odoo-community.org/page/development-status"><img alt="Beta" src="https://img.shields.io/badge/maturity-Beta-yellow.png" /></a> <a class="reference external image-reference" href="http://www.gnu.org/licenses/agpl-3.0-standalone.html"><img alt="License: AGPL-3" src="https://img.shields.io/badge/license-AGPL--3-blue.png" /></a> <a class="reference external image-reference" href="https://github.com/OCA/hr-expense/tree/19.0/hr_expense_payment"><img alt="OCA/hr-expense" src="https://img.shields.io/badge/github-OCA%2Fhr--expense-lightgray.png?logo=github" /></a> <a class="reference external image-reference" href="https://translation.odoo-community.org/projects/hr-expense-19-0/hr-expense-19-0-hr_expense_payment"><img alt="Translate me on Weblate" src="https://img.shields.io/badge/weblate-Translate%20me-F47D42.png" /></a> <a class="reference external image-reference" href="https://runboat.odoo-community.org/builds?repo=OCA/hr-expense&amp;target_branch=19.0"><img alt="Try me on Runboat" src="https://img.shields.io/badge/runboat-Try%20me-875A7B.png" /></a></p>
-<p>This module links each employee expense to the payment(s) that settled
-it. When you Register Payment on a posted employee-paid expense, the
-resulting payment back-links to the source expense, and vice versa. A
-post-install hook backfills the link for payments made before the module
-was installed.</p>
+<p>This module links each employee-paid expense to the payment(s) that
+settled it, in both directions: <tt class="docutils literal">hr.expense.payment_ids</tt> and
+<tt class="docutils literal">account.payment.reconciled_expense_ids</tt>.</p>
+<p>The link is computed from the reconciliation between the expense’s
+journal entry and the payment’s journal items — the same way core
+derives <tt class="docutils literal">reconciled_bill_ids</tt> on payments. It therefore always
+reflects the current accounting state (partial payments included), needs
+no manual bookkeeping, and covers payments however they were registered
+(expense’s Register Payment button, the journal entry’s own button, or
+manual reconciliation).</p>
+<p>Odoo 19.0 core’s <tt class="docutils literal">account.payment.expense_ids</tt> only covers
+company-paid expenses (whose journal entry <em>is</em> the payment’s move);
+this module covers the employee-reimbursement direction that core does
+not track.</p>
 <p><strong>Table of contents</strong></p>
 <div class="contents local topic" id="contents">
 <ul class="simple">

hr_expense_payment/tests/test_hr_expense_payment.py

@@ -7,46 +7,120 @@
 
 from odoo.addons.hr_expense.tests.common import TestExpenseCommon
 
-from ..hooks import post_init_hook
-
 
 @tagged("-at_install", "post_install")
 class TestHrExpensePayment(TestExpenseCommon):
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
-        # Create expense + sheet + approve
-        cls.expense = cls.create_expense(cls)
-        res = cls.expense.action_submit_expenses()
-        cls.expense_sheet = cls.env[res["res_model"]].browse(res["res_id"])
-        cls.expense_sheet.action_submit_sheet()
-        cls.expense_sheet.action_approve_expense_sheets()
-
-    def _get_payment_wizard(self):
-        res = self.expense_sheet.action_register_payment()
-        register_form = Form(self.env[res["res_model"]].with_context(**res["context"]))
-        register_form.journal_id = self.company_data["default_journal_bank"]
-        register_form.amount = self.expense_sheet.total_amount
-        return register_form.save()
-
-    def test_post_init_hook(self):
-        self.expense_sheet.action_sheet_move_post()
-        payment_wizard = self._get_payment_wizard()
-        payment_wizard.action_create_payments()
-        payment = self.expense_sheet.payment_ids
+        # Build an employee-paid expense, submit + approve + post (= receipt).
+        cls.expense = cls.create_expenses({"payment_mode": "own_account"})
+        cls.expense.action_submit()
+        cls.expense.action_approve()
+        cls.post_expenses_with_wizard(cls.expense)
+
+    def _get_payment_wizard(self, expenses, amount=None):
+        action = expenses.action_pay()
+        wizard_form = Form(
+            self.env[action["res_model"]].with_context(**action["context"])
+        )
+        wizard_form.journal_id = self.company_data["default_journal_bank"]
+        if amount is not None:
+            wizard_form.amount = amount
+        return wizard_form.save()
+
+    def test_core_expense_ids_field_not_overridden(self):
+        """The module must not redefine core's account.payment.expense_ids
+        (related to move_id.expense_ids)."""
+        field = self.env["account.payment"]._fields["expense_ids"]
+        self.assertEqual(field.type, "one2many")
+        self.assertEqual(field.related, "move_id.expense_ids")
+
+    def test_action_pay_links_payment_back_to_expense(self):
+        """Registering payment from the expense links both directions."""
+        self.assertFalse(self.expense.payment_ids)
+        wizard = self._get_payment_wizard(self.expense)
+        wizard.action_create_payments()
+        self.assertEqual(len(self.expense.payment_ids), 1)
+        payment = self.expense.payment_ids
+        self.assertIn(self.expense, payment.reconciled_expense_ids)
+        # Core's related field stays empty for reimbursement payments.
+        self.assertFalse(payment.expense_ids)
+
+    def test_partial_payment_links(self):
+        """Partially reconciled payments are linked too."""
+        wizard = self._get_payment_wizard(
+            self.expense, amount=self.expense.total_amount / 2
+        )
+        wizard.action_create_payments()
+        self.assertEqual(len(self.expense.payment_ids), 1)
+        self.assertIn(self.expense, self.expense.payment_ids.reconciled_expense_ids)
+        # Pay the remainder: both payments end up linked.
+        wizard = self._get_payment_wizard(self.expense)
+        wizard.action_create_payments()
+        self.assertEqual(len(self.expense.payment_ids), 2)
+
+    def test_grouped_payment_links_all_expenses(self):
+        """One grouped payment for several expenses back-links to all."""
+        expense_2 = self.create_expenses(
+            {"payment_mode": "own_account", "total_amount_currency": 123.0}
+        )
+        expense_2.action_submit()
+        expense_2.action_approve()
+        self.post_expenses_with_wizard(expense_2)
+        expenses = self.expense | expense_2
+        action = expenses.action_pay()
+        wizard = (
+            self.env[action["res_model"]]
+            .with_context(**action["context"])
+            .create({"group_payment": True})
+        )
+        wizard.action_create_payments()
+        payment = self.expense.payment_ids
         self.assertEqual(len(payment), 1)
-        self.assertEqual(len(payment.expense_sheet_ids), 1)
-        payment.expense_sheet_ids = False
-        # Recompute many2one
-        payment = self.expense_sheet.payment_ids
-        self.assertFalse(payment)
-        self.assertFalse(payment.expense_sheet_ids)
-        post_init_hook(self.env)
-        self.assertEqual(len(self.expense_sheet.payment_ids), 1)
-
-    def test_get_payment_vals(self):
-        self.expense_sheet.action_sheet_move_post()
-        payment_wizard = self._get_payment_wizard()
-        self.assertFalse(self.expense_sheet.payment_ids)
-        payment_wizard.action_create_payments()
-        self.assertEqual(len(self.expense_sheet.payment_ids), 1)
+        self.assertEqual(expense_2.payment_ids, payment)
+        self.assertEqual(payment.reconciled_expense_ids, expenses)
+
+    def test_unreconcile_dissolves_link(self):
+        """Removing the reconciliation clears both sides of the link."""
+        wizard = self._get_payment_wizard(self.expense)
+        wizard.action_create_payments()
+        payment = self.expense.payment_ids
+        self.expense.account_move_id.line_ids.remove_move_reconcile()
+        self.assertFalse(self.expense.payment_ids)
+        self.assertFalse(payment.reconciled_expense_ids)
+
+    def test_search_payment_ids(self):
+        """Both computed fields are searchable."""
+        wizard = self._get_payment_wizard(self.expense)
+        wizard.action_create_payments()
+        payment = self.expense.payment_ids
+        found_expenses = self.env["hr.expense"].search(
+            [("payment_ids", "in", payment.ids)]
+        )
+        self.assertEqual(found_expenses, self.expense)
+        found_payments = self.env["account.payment"].search(
+            [("reconciled_expense_ids", "in", self.expense.ids)]
+        )
+        self.assertEqual(found_payments, payment)
+
+    def test_payment_ids_readable_by_employee(self):
+        """Employees without accounting access can read the computed link."""
+        wizard = self._get_payment_wizard(self.expense)
+        wizard.action_create_payments()
+        expense = self.expense.with_user(self.expense_user_employee)
+        self.assertEqual(len(expense.payment_ids), 1)
+
+    def test_company_paid_expense_not_linked(self):
+        """Company-paid expenses stay core-only; module fields stay empty."""
+        expense = self.create_expenses(
+            {"payment_mode": "company_account", "total_amount_currency": 200.0}
+        )
+        expense.action_submit()
+        expense.action_approve()
+        expense.action_post()
+        self.assertFalse(expense.payment_ids)
+        payment = expense.account_move_id.origin_payment_id
+        if payment:
+            self.assertFalse(payment.reconciled_expense_ids)
+            self.assertIn(expense, payment.expense_ids)