diff --git a/connector_sage/__manifest__.py b/connector_sage/__manifest__.py index 101db3043..3754c7967 100644 --- a/connector_sage/__manifest__.py +++ b/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", diff --git a/connector_sage/components/importer.py b/connector_sage/components/importer.py index bfb386209..165fff9dc 100644 --- a/connector_sage/components/importer.py +++ b/connector_sage/components/importer.py @@ -1,5 +1,6 @@ # Copyright NuoBiT Solutions - Eric Antones # Copyright NuoBiT Solutions - Kilian Niubo +# Copyright NuoBiT Solutions - Deniz Gallo # 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 diff --git a/connector_sage/models/res_partner/importer.py b/connector_sage/models/res_partner/importer.py index d1cc63e15..73e57ffe9 100644 --- a/connector_sage/models/res_partner/importer.py +++ b/connector_sage/models/res_partner/importer.py @@ -1,6 +1,9 @@ # Copyright NuoBiT Solutions - Eric Antones +# Copyright NuoBiT Solutions - Deniz Gallo # 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, + } + ) diff --git a/connector_sage/models/res_partner/mapper.py b/connector_sage/models/res_partner/mapper.py index 6383388f1..2190aa596 100644 --- a/connector_sage/models/res_partner/mapper.py +++ b/connector_sage/models/res_partner/mapper.py @@ -1,4 +1,5 @@ # Copyright NuoBiT Solutions - Eric Antones +# Copyright NuoBiT Solutions - Deniz Gallo # 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)} diff --git a/connector_sage/tests/__init__.py b/connector_sage/tests/__init__.py new file mode 100644 index 000000000..eacabfb44 --- /dev/null +++ b/connector_sage/tests/__init__.py @@ -0,0 +1 @@ +from . import test_employee_code_reassignment diff --git a/connector_sage/tests/test_employee_code_reassignment.py b/connector_sage/tests/test_employee_code_reassignment.py new file mode 100644 index 000000000..cecf42231 --- /dev/null +++ b/connector_sage/tests/test_employee_code_reassignment.py @@ -0,0 +1,131 @@ +# Copyright NuoBiT Solutions - Deniz Gallo +# 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")