test(spp_audit): add decorator tests, fix Markup sanitization in write/unlink

Add 11 tests covering all three audit decorator methods (create, write,
unlink) including multi-record operations, audit log verification, and
recursive audit prevention.

Fix Markup sanitization in audit_write and audit_unlink to iterate all
records instead of only the first — the same bug pattern fixed in
audit_create by the previous commit.

Author: gonzalesedwin1123

spp_audit/README.rst

@@ -150,6 +150,8 @@ Changelog
 
 - fix: use @api.model_create_multi for audit_create to support Odoo 19
   create overrides (#138)
+- fix: Markup sanitization in audit_write and audit_unlink now handles
+  all records, not just the first
 
 19.0.2.0.0
 ~~~~~~~~~~

spp_audit/readme/HISTORY.md

@@ -1,6 +1,7 @@
 ### 19.0.2.0.1
 
 - fix: use @api.model_create_multi for audit_create to support Odoo 19 create overrides (#138)
+- fix: Markup sanitization in audit_write and audit_unlink now handles all records, not just the first
 
 ### 19.0.2.0.0
 

spp_audit/static/description/index.html

@@ -520,6 +520,8 @@ <h1>19.0.2.0.1</h1>
 <ul class="simple">
 <li>fix: use &#64;api.model_create_multi for audit_create to support Odoo 19
 create overrides (#138)</li>
+<li>fix: Markup sanitization in audit_write and audit_unlink now handles
+all records, not just the first</li>
 </ul>
 </div>
 <div class="section" id="section-2">

spp_audit/tests/test_spp_audit_rule.py

@@ -2,6 +2,8 @@
 
 
 class AuditRuleTest(TransactionCase):
+    """Tests for spp.audit.rule core functionality."""
+
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
@@ -88,3 +90,268 @@ def test_get_audit_log_vals(self):
         self.assertEqual(res_id, vals["res_id"])
         self.assertEqual(method, vals["method"])
         self.assertEqual(repr(data[res_id]), vals["data"])
+
+
+class TestAuditCreateMulti(TransactionCase):
+    """Regression tests for audit_create with @api.model_create_multi.
+
+    The audit decorator monkey-patches create() on audited models. It must
+    use @api.model_create_multi so the vals_list flows correctly through
+    the origin chain to downstream create overrides.
+    """
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.partner_model = cls.env["ir.model"].search([("model", "=", "res.partner")], limit=1)
+        # Ensure an audit rule with create logging exists for res.partner
+        cls.audit_rule = cls.env["spp.audit.rule"].search([("model_id", "=", cls.partner_model.id)], limit=1)
+        if not cls.audit_rule:
+            cls.audit_rule = cls.env["spp.audit.rule"].create(
+                {
+                    "name": "Test Partner Audit",
+                    "model_id": cls.partner_model.id,
+                    "is_log_create": True,
+                    "is_log_write": False,
+                    "is_log_unlink": False,
+                }
+            )
+        else:
+            cls.audit_rule.write({"is_log_create": True})
+
+    def test_multi_create_returns_all_records(self):
+        """Creating multiple records in one call must return all of them."""
+        partners = self.env["res.partner"].create(
+            [
+                {"name": "Audit Multi A"},
+                {"name": "Audit Multi B"},
+                {"name": "Audit Multi C"},
+            ]
+        )
+        self.assertEqual(len(partners), 3)
+        self.assertEqual(partners[0].name, "Audit Multi A")
+        self.assertEqual(partners[1].name, "Audit Multi B")
+        self.assertEqual(partners[2].name, "Audit Multi C")
+
+    def test_multi_create_produces_audit_logs(self):
+        """Each record from a multi-create should have an audit log entry."""
+        partners = self.env["res.partner"].create(
+            [
+                {"name": "Audit Log A"},
+                {"name": "Audit Log B"},
+            ]
+        )
+        logs = self.env["spp.audit.log"].search(
+            [
+                ("model_id", "=", self.partner_model.id),
+                ("method", "=", "create"),
+                ("res_id", "in", partners.ids),
+            ]
+        )
+        logged_ids = set(logs.mapped("res_id"))
+        for partner in partners:
+            self.assertIn(
+                partner.id,
+                logged_ids,
+                f"Audit log missing for partner {partner.name} (id={partner.id})",
+            )
+
+    def test_single_create_still_works(self):
+        """Single-dict create must still work with the model_create_multi decorator."""
+        partner = self.env["res.partner"].create({"name": "Audit Single"})
+        self.assertTrue(partner.exists())
+        self.assertEqual(partner.name, "Audit Single")
+
+
+class TestAuditWrite(TransactionCase):
+    """Tests for the audit_write decorator method."""
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.partner_model = cls.env["ir.model"].search([("model", "=", "res.partner")], limit=1)
+        cls.audit_rule = cls.env["spp.audit.rule"].search([("model_id", "=", cls.partner_model.id)], limit=1)
+        if not cls.audit_rule:
+            cls.audit_rule = cls.env["spp.audit.rule"].create(
+                {
+                    "name": "Test Partner Audit Write",
+                    "model_id": cls.partner_model.id,
+                    "is_log_create": False,
+                    "is_log_write": True,
+                    "is_log_unlink": False,
+                }
+            )
+        else:
+            cls.audit_rule.write({"is_log_write": True})
+
+    def test_write_produces_audit_log(self):
+        """Writing to an audited record should produce an audit log entry."""
+        partner = self.env["res.partner"].create({"name": "Write Test"})
+        partner.write({"name": "Write Test Updated"})
+
+        logs = self.env["spp.audit.log"].search(
+            [
+                ("model_id", "=", self.partner_model.id),
+                ("method", "=", "write"),
+                ("res_id", "=", partner.id),
+            ]
+        )
+        self.assertTrue(logs, "Expected an audit log for the write operation")
+
+    def test_write_captures_old_and_new_values(self):
+        """Audit log data should contain both old and new values."""
+        partner = self.env["res.partner"].create({"name": "Old Name"})
+        partner.write({"name": "New Name"})
+
+        log = self.env["spp.audit.log"].search(
+            [
+                ("model_id", "=", self.partner_model.id),
+                ("method", "=", "write"),
+                ("res_id", "=", partner.id),
+            ],
+            limit=1,
+            order="id desc",
+        )
+        self.assertTrue(log)
+        # The data field stores repr() of the diff dict which includes old/new
+        self.assertIn("old", log.data)
+        self.assertIn("new", log.data)
+
+    def test_write_multiple_records(self):
+        """Writing to multiple records at once should log each one."""
+        partners = self.env["res.partner"].create(
+            [
+                {"name": "Batch Write A"},
+                {"name": "Batch Write B"},
+            ]
+        )
+        partners.write({"phone": "+1234567890"})
+
+        logs = self.env["spp.audit.log"].search(
+            [
+                ("model_id", "=", self.partner_model.id),
+                ("method", "=", "write"),
+                ("res_id", "in", partners.ids),
+            ]
+        )
+        logged_ids = set(logs.mapped("res_id"))
+        for partner in partners:
+            self.assertIn(partner.id, logged_ids)
+
+    def test_write_no_recursive_audit(self):
+        """Write with audit_in_progress context should not create duplicate logs."""
+        partner = self.env["res.partner"].create({"name": "Recurse Test"})
+
+        # Count logs before
+        log_count_before = self.env["spp.audit.log"].search_count(
+            [
+                ("model_id", "=", self.partner_model.id),
+                ("method", "=", "write"),
+                ("res_id", "=", partner.id),
+            ]
+        )
+
+        # Write with audit_in_progress flag — should skip audit logging
+        partner.with_context(audit_in_progress=True).write({"name": "Skipped"})
+
+        log_count_after = self.env["spp.audit.log"].search_count(
+            [
+                ("model_id", "=", self.partner_model.id),
+                ("method", "=", "write"),
+                ("res_id", "=", partner.id),
+            ]
+        )
+        self.assertEqual(
+            log_count_before,
+            log_count_after,
+            "audit_in_progress context should prevent audit logging",
+        )
+
+
+class TestAuditUnlink(TransactionCase):
+    """Tests for the audit_unlink decorator method."""
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.partner_model = cls.env["ir.model"].search([("model", "=", "res.partner")], limit=1)
+        cls.audit_rule = cls.env["spp.audit.rule"].search([("model_id", "=", cls.partner_model.id)], limit=1)
+        if not cls.audit_rule:
+            cls.audit_rule = cls.env["spp.audit.rule"].create(
+                {
+                    "name": "Test Partner Audit Unlink",
+                    "model_id": cls.partner_model.id,
+                    "is_log_create": False,
+                    "is_log_write": False,
+                    "is_log_unlink": True,
+                }
+            )
+        else:
+            cls.audit_rule.write({"is_log_unlink": True})
+
+    def test_unlink_produces_audit_log(self):
+        """Deleting an audited record should produce an audit log entry."""
+        partner = self.env["res.partner"].create({"name": "Delete Me"})
+        partner_id = partner.id
+        partner.unlink()
+
+        logs = self.env["spp.audit.log"].search(
+            [
+                ("model_id", "=", self.partner_model.id),
+                ("method", "=", "unlink"),
+                ("res_id", "=", partner_id),
+            ]
+        )
+        self.assertTrue(logs, "Expected an audit log for the unlink operation")
+
+    def test_unlink_logs_old_values(self):
+        """Audit log for unlink should contain the old values of the deleted record."""
+        partner = self.env["res.partner"].create({"name": "Unlink Values Test"})
+        partner_id = partner.id
+        partner.unlink()
+
+        log = self.env["spp.audit.log"].search(
+            [
+                ("model_id", "=", self.partner_model.id),
+                ("method", "=", "unlink"),
+                ("res_id", "=", partner_id),
+            ],
+            limit=1,
+            order="id desc",
+        )
+        self.assertTrue(log)
+        # Unlink logs old values (the state before deletion)
+        self.assertIn("old", log.data)
+
+    def test_unlink_multiple_records(self):
+        """Deleting multiple records at once should log each one."""
+        partners = self.env["res.partner"].create(
+            [
+                {"name": "Batch Delete A"},
+                {"name": "Batch Delete B"},
+            ]
+        )
+        partner_ids = partners.ids
+        partners.unlink()
+
+        logs = self.env["spp.audit.log"].search(
+            [
+                ("model_id", "=", self.partner_model.id),
+                ("method", "=", "unlink"),
+                ("res_id", "in", partner_ids),
+            ]
+        )
+        logged_ids = set(logs.mapped("res_id"))
+        for pid in partner_ids:
+            self.assertIn(pid, logged_ids)
+
+    def test_unlink_record_is_actually_deleted(self):
+        """The record should actually be deleted after audit logging."""
+        partner = self.env["res.partner"].create({"name": "Really Delete"})
+        partner_id = partner.id
+        partner.unlink()
+
+        self.assertFalse(
+            self.env["res.partner"].search([("id", "=", partner_id)]),
+            "Record should be deleted after audit_unlink",
+        )

spp_audit/tools/decorator.py

@@ -73,9 +73,11 @@ def audit_write(self, vals):
         if new_values and old_values_copy:
             keys = new_values[0].keys()
             for key in keys:
-                if str(type(new_values[0][key])) == "<class 'markupsafe.Markup'>":
-                    new_values[0][key] = str(new_values[0][key])
-                    old_values_copy[0][key] = str(old_values_copy[0][key])
+                for nv, ov in zip(new_values, old_values_copy, strict=False):
+                    if str(type(nv[key])) == "<class 'markupsafe.Markup'>":
+                        nv[key] = str(nv[key])
+                    if str(type(ov[key])) == "<class 'markupsafe.Markup'>":
+                        ov[key] = str(ov[key])
 
             rules.log("write", old_values_copy, new_values)
         return result
@@ -92,8 +94,9 @@ def audit_unlink(self):
         if old_values:
             keys = old_values[0].keys()
             for key in keys:
-                if str(type(old_values[0][key])) == "<class 'markupsafe.Markup'>":
-                    old_values[0][key] = str(old_values[0][key])
+                for ov in old_values:
+                    if str(type(ov[key])) == "<class 'markupsafe.Markup'>":
+                        ov[key] = str(ov[key])
 
             rules.log("unlink", old_values)
         return audit_unlink.origin(self)