[14.0][IMP] connector_sage: detect employee code reassignment on import

connector_sage/__manifest__.py

@@ -4,7 +4,7 @@
 
 {
     "name": "Sage-Odoo connector",
-    "version": "14.0.1.0.6",
+    "version": "14.0.1.0.7",
     "author": "NuoBiT Solutions, S.L., Eric Antones",
     "license": "AGPL-3",
     "category": "Connector",

connector_sage/components/importer.py

@@ -1,5 +1,6 @@
 # Copyright NuoBiT Solutions - Eric Antones <eantones@nuobit.com>
 # Copyright NuoBiT Solutions - Kilian Niubo <kniubo@nuobit.com>
+# Copyright NuoBiT Solutions - Deniz Gallo <dgallo@nuobit.com>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl)
 
 import logging
@@ -88,6 +89,12 @@ def _import_dependencies(self):
         """
         return
 
+    def _validate_update(self, binding, values):
+        """Hook called on an existing binding before writing the update.
+
+        Raise an exception to abort the import of the current record.
+        """
+
     def run(self, external_id):
         # get_data
         # this one knows how to speak to sage
@@ -119,9 +126,9 @@ def run(self, external_id):
         # if not force and self._is_uptodate(binding):
         #     return _('Already up-to-date.')
         if binding:
-            binding.with_company(self.backend_record.company_id).write(
-                internal_data.values()
-            )
+            values = internal_data.values()
+            self._validate_update(binding, values)
+            binding.with_company(self.backend_record.company_id).write(values)
             _logger.debug("%d updated from Sage %s", binding, external_id)
         else:
             # or we create it

connector_sage/models/res_partner/importer.py

@@ -1,6 +1,9 @@
 # Copyright NuoBiT Solutions - Eric Antones <eantones@nuobit.com>
+# Copyright NuoBiT Solutions - Deniz Gallo <dgallo@nuobit.com>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl)
 
+from odoo import _
+from odoo.exceptions import ValidationError
 
 from odoo.addons.component.core import Component
 
@@ -20,3 +23,56 @@ class ResPartnerImporter(Component):
     _name = "sage.res.partner.importer"
     _inherit = "sage.importer"
     _apply_on = "sage.res.partner"
+
+    def _validate_update(self, binding, values):
+        """Stop the import when the employee code was reassigned in Sage.
+
+        Sage reuses employee codes: a code can be given to a different
+        person. The partner vat holds the identity the code belonged to
+        when it was first imported (the euvat mapping is @only_create, so
+        no later sync rewrites it): a different incoming SiglaNacion+Dni
+        means the code changed hands, and updating would relabel the
+        employee while the partner keeps the old person, misattributing
+        payroll payments. The error message documents both unlock
+        procedures.
+
+        Partners without vat (e.g. excluded by
+        employees_exclude_nif_pattern) cannot be checked and are skipped.
+        """
+        vat = binding.odoo_id.vat
+        mapper = self.component(usage="import.mapper")
+        incoming_dni = mapper.sage_identity(self.external_data)
+        if not vat or not incoming_dni or vat == incoming_dni:
+            return
+        incoming_name = " ".join(
+            part
+            for part in (
+                self.external_data["NombreEmpleado"],
+                self.external_data["PrimerApellidoEmpleado"],
+                self.external_data["SegundoApellidoEmpleado"],
+            )
+            if part
+        )
+        raise ValidationError(
+            _(
+                "The Sage employee code %(company)s/%(code)s seems to have "
+                "been reassigned to a different person: it belonged to "
+                "'%(partner)s' (VAT %(vat)s) but Sage now returns "
+                "'%(incoming_name)s' (DNI %(incoming_dni)s). The import of "
+                "this record has been stopped to avoid mixing people.\n"
+                "- If this is the same person with a new identity document, "
+                "update the partner VAT and retry the job.\n"
+                "- If the code was really reassigned to a new person, delete "
+                "the Sage bindings (partner and employee) of this code and "
+                "retry the job; the old partner and employee will be kept, "
+                "unbound."
+            )
+            % {
+                "company": binding.sage_codigo_empresa,
+                "code": binding.sage_codigo_empleado,
+                "partner": binding.odoo_id.display_name,
+                "vat": vat,
+                "incoming_name": incoming_name,
+                "incoming_dni": incoming_dni,
+            }
+        )

connector_sage/models/res_partner/mapper.py

@@ -1,4 +1,5 @@
 # Copyright NuoBiT Solutions - Eric Antones <eantones@nuobit.com>
+# Copyright NuoBiT Solutions - Deniz Gallo <dgallo@nuobit.com>
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl)
 import re
 
@@ -92,6 +93,16 @@ def employee_as_supplier_account(self, record):
     def type(self, record):
         return {"type": "contact"}
 
+    def sage_identity(self, record):
+        """Normalize a Sage person identity (SiglaNacion+Dni) to a string.
+
+        Single source of the normalization: euvat seeds the partner vat
+        with it and the reassignment guard compares against that vat, so
+        both sides must build the string exactly the same way.
+        """
+        parts = [part for part in (record["SiglaNacion"], record["Dni"]) if part]
+        return "".join(parts).strip().upper()
+
     @only_create
     @mapping
     def euvat(self, record):
@@ -100,5 +111,4 @@ def euvat(self, record):
             m = re.match(nif_pattern, record["Dni"])
             if m:
                 return {"vat": None}
-        parts = [part for part in (record["SiglaNacion"], record["Dni"]) if part]
-        return {"vat": "".join(parts).strip().upper()}
+        return {"vat": self.sage_identity(record)}

connector_sage/tests/__init__.py

@@ -0,0 +1 @@
+from . import test_employee_code_reassignment

connector_sage/tests/test_employee_code_reassignment.py

@@ -0,0 +1,131 @@
+# Copyright NuoBiT Solutions - Deniz Gallo <dgallo@nuobit.com>
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl)
+
+from unittest import mock
+
+from odoo.exceptions import ValidationError
+from odoo.tests.common import SavepointCase, tagged
+
+ADAPTER_READ = "odoo.addons.connector_sage.components.adapter.GenericAdapter.read"
+
+SAGE_COMPANY = 1
+CODE = 501
+
+PERSON_A = {
+    "CodigoEmpresa": SAGE_COMPANY,
+    "CodigoEmpleado": CODE,
+    "SiglaNacion": "XX",
+    "Dni": "TRIP0001A",
+    "NombreEmpleado": "MARIA",
+    "PrimerApellidoEmpleado": "GARCIA",
+    "SegundoApellidoEmpleado": "PEREZ",
+    "NumeroHijos": 0,
+    "FechaNacimiento": False,
+    "Email1": "maria@test.example.com",
+    "Email2": "maria@test.example.com",
+}
+
+PERSON_B = {
+    **PERSON_A,
+    "Dni": "TRIP0002B",
+    "NombreEmpleado": "JUAN",
+    "PrimerApellidoEmpleado": "LOPEZ",
+    "SegundoApellidoEmpleado": "RUIZ",
+    "Email1": "juan@test.example.com",
+    "Email2": "juan@test.example.com",
+}
+
+
+@tagged("post_install", "-at_install")
+class TestEmployeeCodeReassignment(SavepointCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.env = cls.env(context=dict(cls.env.context, tracking_disable=True))
+        cls.company = cls.env["res.company"].create({"name": "Sage Test Company"})
+        cls.account_payable = cls.env["account.account"].create(
+            {
+                "name": "Test Account Payable",
+                "code": "TESTPAY01",
+                "user_type_id": cls.env.ref("account.data_account_type_payable").id,
+                "reconcile": True,
+                "company_id": cls.company.id,
+            }
+        )
+        cls.backend = cls.env["sage.backend"].create(
+            {
+                "name": "Test Sage Backend",
+                "server": "localhost",
+                "port": 1433,
+                "database": "test",
+                "schema": "dbo",
+                "username": "test",
+                "password": "test",
+                "company_id": cls.company.id,
+                "sage_company_id": SAGE_COMPANY,
+                "import_employees_default_account_payable_id": cls.account_payable.id,
+            }
+        )
+
+    def _import_record(self, binding_model, record):
+        with mock.patch(ADAPTER_READ, return_value=dict(record)):
+            self.env[binding_model].import_record(
+                self.backend, (record["CodigoEmpresa"], record["CodigoEmpleado"])
+            )
+        return self.env[binding_model].search(
+            [
+                ("backend_id", "=", self.backend.id),
+                ("sage_codigo_empresa", "=", record["CodigoEmpresa"]),
+                ("sage_codigo_empleado", "=", record["CodigoEmpleado"]),
+            ]
+        )
+
+    def test_first_import_sets_partner_vat(self):
+        """The first import creates the partner with
+        vat = SiglaNacion+Dni: the frozen identity every later sync
+        validates against."""
+        binding = self._import_record("sage.res.partner", PERSON_A)
+        self.assertEqual(binding.odoo_id.vat, "XXTRIP0001A")
+        self.assertEqual(binding.odoo_id.name, "MARIA GARCIA PEREZ")
+
+    def test_same_person_reimport_passes(self):
+        """A re-sync of the same person (e.g. renamed) must pass the
+        guard and keep the partner untouched: partner data is
+        @only_create."""
+        binding = self._import_record("sage.res.partner", PERSON_A)
+        renamed = dict(PERSON_A, PrimerApellidoEmpleado="GARCIA VIUDA")
+        self._import_record("sage.res.partner", renamed)
+        self.assertEqual(binding.odoo_id.vat, "XXTRIP0001A")
+        self.assertEqual(binding.odoo_id.name, "MARIA GARCIA PEREZ")
+
+    def test_reassigned_code_stops_import(self):
+        """A code coming back with a different identity aborts before
+        writing: neither the partner nor the binding may change."""
+        binding = self._import_record("sage.res.partner", PERSON_A)
+        with self.assertRaisesRegex(ValidationError, "reassigned"):
+            self._import_record("sage.res.partner", PERSON_B)
+        self.assertEqual(binding.odoo_id.name, "MARIA GARCIA PEREZ")
+        self.assertEqual(binding.odoo_id.vat, "XXTRIP0001A")
+
+    def test_partner_without_vat_skips_check(self):
+        """Partners whose vat is empty (e.g. excluded by
+        employees_exclude_nif_pattern) cannot be checked: the import
+        proceeds and vat stays empty (@only_create never rewrites
+        it)."""
+        binding = self._import_record("sage.res.partner", PERSON_A)
+        binding.odoo_id.vat = False
+        self._import_record("sage.res.partner", PERSON_B)
+        self.assertFalse(binding.odoo_id.vat)
+
+    def test_employee_import_stops_before_updating_employee(self):
+        """The employee import runs the partner import as a dependency
+        (always=True), so the partner guard aborts the job before the
+        employee is touched."""
+        emp_binding = self._import_record("sage.hr.employee", PERSON_A)
+        employee = emp_binding.odoo_id
+        self.assertEqual(employee.name, "MARIA GARCIA PEREZ")
+        self.assertEqual(employee.address_home_id.vat, "XXTRIP0001A")
+        with self.assertRaisesRegex(ValidationError, "reassigned"):
+            self._import_record("sage.hr.employee", PERSON_B)
+        self.assertEqual(employee.name, "MARIA GARCIA PEREZ")
+        self.assertEqual(employee.identification_id, "XXTRIP0001A")