Skip to content

Commit 696029f

Browse files
Merge pull request #138 from OpenSPP/fix/audit-create-model-create-multi
fix: audit_create uses @api.model breaking model_create_multi overrides
2 parents b8e640f + 46a7365 commit 696029f

6 files changed

Lines changed: 320 additions & 23 deletions

File tree

spp_audit/README.rst

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ OpenSPP Audit
77
!! This file is generated by oca-gen-addon-readme !!
88
!! changes will be overwritten. !!
99
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
10-
!! source digest: sha256:0000000000000000000000000000000000000000000000000000000000000000
10+
!! source digest: sha256:force_regen
1111
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
1212
1313
.. |badge1| image:: https://img.shields.io/badge/maturity-Production%2FStable-green.png
@@ -145,6 +145,16 @@ External Python: ``requests`` (HTTP backend)
145145
Changelog
146146
=========
147147

148+
19.0.2.0.1
149+
~~~~~~~~~~
150+
151+
- fix: use @api.model_create_multi for audit_create to support Odoo 19
152+
create overrides (#138)
153+
- fix: Markup sanitization in audit_write and audit_unlink now handles
154+
all records, not just the first
155+
- refactor: use ``isinstance(value, Markup)`` instead of fragile
156+
``str(type(...))`` comparison in all audit decorator methods
157+
148158
19.0.2.0.0
149159
~~~~~~~~~~
150160

spp_audit/__manifest__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"name": "OpenSPP Audit",
44
"summary": "Comprehensively tracks all data modifications and user actions across the OpenSPP platform, recording old and new values for configured data. It enhances accountability and data integrity by maintaining an immutable history of changes, crucial for internal audits, compliance, and detecting unauthorized alterations. Supports multiple backends (database, file, syslog, HTTP) with tamper-resistant configuration.",
55
"category": "OpenSPP/Monitoring",
6-
"version": "19.0.2.0.0",
6+
"version": "19.0.2.0.1",
77
"sequence": 1,
88
"author": "OpenSPP.org",
99
"website": "https://github.com/OpenSPP/OpenSPP2",

spp_audit/readme/HISTORY.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
### 19.0.2.0.1
2+
3+
- fix: use @api.model_create_multi for audit_create to support Odoo 19 create overrides (#138)
4+
- fix: Markup sanitization in audit_write and audit_unlink now handles all records, not just the first
5+
- refactor: use `isinstance(value, Markup)` instead of fragile `str(type(...))` comparison in all audit decorator methods
6+
17
### 19.0.2.0.0
28

39
- Initial migration to OpenSPP2

spp_audit/static/description/index.html

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ <h1 class="title">OpenSPP Audit</h1>
367367
!! This file is generated by oca-gen-addon-readme !!
368368
!! changes will be overwritten. !!
369369
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
370-
!! source digest: sha256:0000000000000000000000000000000000000000000000000000000000000000
370+
!! source digest: sha256:force_regen
371371
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -->
372372
<p><a class="reference external image-reference" href="https://odoo-community.org/page/development-status"><img alt="Production/Stable" src="https://img.shields.io/badge/maturity-Production%2FStable-green.png" /></a> <a class="reference external image-reference" href="http://www.gnu.org/licenses/lgpl-3.0-standalone.html"><img alt="License: LGPL-3" src="https://img.shields.io/badge/license-LGPL--3-blue.png" /></a> <a class="reference external image-reference" href="https://github.com/OpenSPP/OpenSPP2/tree/19.0/spp_audit"><img alt="OpenSPP/OpenSPP2" src="https://img.shields.io/badge/github-OpenSPP%2FOpenSPP2-lightgray.png?logo=github" /></a></p>
373373
<p>Tracks all data modifications and user actions across OpenSPP models by
@@ -516,6 +516,17 @@ <h2><a class="toc-backref" href="#toc-entry-1">Changelog</a></h2>
516516
</div>
517517
</div>
518518
<div class="section" id="section-1">
519+
<h1>19.0.2.0.1</h1>
520+
<ul class="simple">
521+
<li>fix: use &#64;api.model_create_multi for audit_create to support Odoo 19
522+
create overrides (#138)</li>
523+
<li>fix: Markup sanitization in audit_write and audit_unlink now handles
524+
all records, not just the first</li>
525+
<li>refactor: use <tt class="docutils literal">isinstance(value, Markup)</tt> instead of fragile
526+
<tt class="docutils literal"><span class="pre">str(type(...))</span></tt> comparison in all audit decorator methods</li>
527+
</ul>
528+
</div>
529+
<div class="section" id="section-2">
519530
<h1>19.0.2.0.0</h1>
520531
<ul class="simple">
521532
<li>Initial migration to OpenSPP2</li>

spp_audit/tests/test_spp_audit_rule.py

Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33

44
class AuditRuleTest(TransactionCase):
5+
"""Tests for spp.audit.rule core functionality."""
6+
57
@classmethod
68
def setUpClass(cls):
79
super().setUpClass()
@@ -88,3 +90,268 @@ def test_get_audit_log_vals(self):
8890
self.assertEqual(res_id, vals["res_id"])
8991
self.assertEqual(method, vals["method"])
9092
self.assertEqual(repr(data[res_id]), vals["data"])
93+
94+
95+
class TestAuditCreateMulti(TransactionCase):
96+
"""Regression tests for audit_create with @api.model_create_multi.
97+
98+
The audit decorator monkey-patches create() on audited models. It must
99+
use @api.model_create_multi so the vals_list flows correctly through
100+
the origin chain to downstream create overrides.
101+
"""
102+
103+
@classmethod
104+
def setUpClass(cls):
105+
super().setUpClass()
106+
cls.partner_model = cls.env["ir.model"].search([("model", "=", "res.partner")], limit=1)
107+
# Ensure an audit rule with create logging exists for res.partner
108+
cls.audit_rule = cls.env["spp.audit.rule"].search([("model_id", "=", cls.partner_model.id)], limit=1)
109+
if not cls.audit_rule:
110+
cls.audit_rule = cls.env["spp.audit.rule"].create(
111+
{
112+
"name": "Test Partner Audit",
113+
"model_id": cls.partner_model.id,
114+
"is_log_create": True,
115+
"is_log_write": False,
116+
"is_log_unlink": False,
117+
}
118+
)
119+
else:
120+
cls.audit_rule.write({"is_log_create": True})
121+
122+
def test_multi_create_returns_all_records(self):
123+
"""Creating multiple records in one call must return all of them."""
124+
partners = self.env["res.partner"].create(
125+
[
126+
{"name": "Audit Multi A"},
127+
{"name": "Audit Multi B"},
128+
{"name": "Audit Multi C"},
129+
]
130+
)
131+
self.assertEqual(len(partners), 3)
132+
self.assertEqual(partners[0].name, "Audit Multi A")
133+
self.assertEqual(partners[1].name, "Audit Multi B")
134+
self.assertEqual(partners[2].name, "Audit Multi C")
135+
136+
def test_multi_create_produces_audit_logs(self):
137+
"""Each record from a multi-create should have an audit log entry."""
138+
partners = self.env["res.partner"].create(
139+
[
140+
{"name": "Audit Log A"},
141+
{"name": "Audit Log B"},
142+
]
143+
)
144+
logs = self.env["spp.audit.log"].search(
145+
[
146+
("model_id", "=", self.partner_model.id),
147+
("method", "=", "create"),
148+
("res_id", "in", partners.ids),
149+
]
150+
)
151+
logged_ids = set(logs.mapped("res_id"))
152+
for partner in partners:
153+
self.assertIn(
154+
partner.id,
155+
logged_ids,
156+
f"Audit log missing for partner {partner.name} (id={partner.id})",
157+
)
158+
159+
def test_single_create_still_works(self):
160+
"""Single-dict create must still work with the model_create_multi decorator."""
161+
partner = self.env["res.partner"].create({"name": "Audit Single"})
162+
self.assertTrue(partner.exists())
163+
self.assertEqual(partner.name, "Audit Single")
164+
165+
166+
class TestAuditWrite(TransactionCase):
167+
"""Tests for the audit_write decorator method."""
168+
169+
@classmethod
170+
def setUpClass(cls):
171+
super().setUpClass()
172+
cls.partner_model = cls.env["ir.model"].search([("model", "=", "res.partner")], limit=1)
173+
cls.audit_rule = cls.env["spp.audit.rule"].search([("model_id", "=", cls.partner_model.id)], limit=1)
174+
if not cls.audit_rule:
175+
cls.audit_rule = cls.env["spp.audit.rule"].create(
176+
{
177+
"name": "Test Partner Audit Write",
178+
"model_id": cls.partner_model.id,
179+
"is_log_create": False,
180+
"is_log_write": True,
181+
"is_log_unlink": False,
182+
}
183+
)
184+
else:
185+
cls.audit_rule.write({"is_log_write": True})
186+
187+
def test_write_produces_audit_log(self):
188+
"""Writing to an audited record should produce an audit log entry."""
189+
partner = self.env["res.partner"].create({"name": "Write Test"})
190+
partner.write({"name": "Write Test Updated"})
191+
192+
logs = self.env["spp.audit.log"].search(
193+
[
194+
("model_id", "=", self.partner_model.id),
195+
("method", "=", "write"),
196+
("res_id", "=", partner.id),
197+
]
198+
)
199+
self.assertTrue(logs, "Expected an audit log for the write operation")
200+
201+
def test_write_captures_old_and_new_values(self):
202+
"""Audit log data should contain both old and new values."""
203+
partner = self.env["res.partner"].create({"name": "Old Name"})
204+
partner.write({"name": "New Name"})
205+
206+
log = self.env["spp.audit.log"].search(
207+
[
208+
("model_id", "=", self.partner_model.id),
209+
("method", "=", "write"),
210+
("res_id", "=", partner.id),
211+
],
212+
limit=1,
213+
order="id desc",
214+
)
215+
self.assertTrue(log)
216+
# The data field stores repr() of the diff dict which includes old/new
217+
self.assertIn("old", log.data)
218+
self.assertIn("new", log.data)
219+
220+
def test_write_multiple_records(self):
221+
"""Writing to multiple records at once should log each one."""
222+
partners = self.env["res.partner"].create(
223+
[
224+
{"name": "Batch Write A"},
225+
{"name": "Batch Write B"},
226+
]
227+
)
228+
partners.write({"phone": "+1234567890"})
229+
230+
logs = self.env["spp.audit.log"].search(
231+
[
232+
("model_id", "=", self.partner_model.id),
233+
("method", "=", "write"),
234+
("res_id", "in", partners.ids),
235+
]
236+
)
237+
logged_ids = set(logs.mapped("res_id"))
238+
for partner in partners:
239+
self.assertIn(partner.id, logged_ids)
240+
241+
def test_write_no_recursive_audit(self):
242+
"""Write with audit_in_progress context should not create duplicate logs."""
243+
partner = self.env["res.partner"].create({"name": "Recurse Test"})
244+
245+
# Count logs before
246+
log_count_before = self.env["spp.audit.log"].search_count(
247+
[
248+
("model_id", "=", self.partner_model.id),
249+
("method", "=", "write"),
250+
("res_id", "=", partner.id),
251+
]
252+
)
253+
254+
# Write with audit_in_progress flag — should skip audit logging
255+
partner.with_context(audit_in_progress=True).write({"name": "Skipped"})
256+
257+
log_count_after = self.env["spp.audit.log"].search_count(
258+
[
259+
("model_id", "=", self.partner_model.id),
260+
("method", "=", "write"),
261+
("res_id", "=", partner.id),
262+
]
263+
)
264+
self.assertEqual(
265+
log_count_before,
266+
log_count_after,
267+
"audit_in_progress context should prevent audit logging",
268+
)
269+
270+
271+
class TestAuditUnlink(TransactionCase):
272+
"""Tests for the audit_unlink decorator method."""
273+
274+
@classmethod
275+
def setUpClass(cls):
276+
super().setUpClass()
277+
cls.partner_model = cls.env["ir.model"].search([("model", "=", "res.partner")], limit=1)
278+
cls.audit_rule = cls.env["spp.audit.rule"].search([("model_id", "=", cls.partner_model.id)], limit=1)
279+
if not cls.audit_rule:
280+
cls.audit_rule = cls.env["spp.audit.rule"].create(
281+
{
282+
"name": "Test Partner Audit Unlink",
283+
"model_id": cls.partner_model.id,
284+
"is_log_create": False,
285+
"is_log_write": False,
286+
"is_log_unlink": True,
287+
}
288+
)
289+
else:
290+
cls.audit_rule.write({"is_log_unlink": True})
291+
292+
def test_unlink_produces_audit_log(self):
293+
"""Deleting an audited record should produce an audit log entry."""
294+
partner = self.env["res.partner"].create({"name": "Delete Me"})
295+
partner_id = partner.id
296+
partner.unlink()
297+
298+
logs = self.env["spp.audit.log"].search(
299+
[
300+
("model_id", "=", self.partner_model.id),
301+
("method", "=", "unlink"),
302+
("res_id", "=", partner_id),
303+
]
304+
)
305+
self.assertTrue(logs, "Expected an audit log for the unlink operation")
306+
307+
def test_unlink_logs_old_values(self):
308+
"""Audit log for unlink should contain the old values of the deleted record."""
309+
partner = self.env["res.partner"].create({"name": "Unlink Values Test"})
310+
partner_id = partner.id
311+
partner.unlink()
312+
313+
log = self.env["spp.audit.log"].search(
314+
[
315+
("model_id", "=", self.partner_model.id),
316+
("method", "=", "unlink"),
317+
("res_id", "=", partner_id),
318+
],
319+
limit=1,
320+
order="id desc",
321+
)
322+
self.assertTrue(log)
323+
# Unlink logs old values (the state before deletion)
324+
self.assertIn("old", log.data)
325+
326+
def test_unlink_multiple_records(self):
327+
"""Deleting multiple records at once should log each one."""
328+
partners = self.env["res.partner"].create(
329+
[
330+
{"name": "Batch Delete A"},
331+
{"name": "Batch Delete B"},
332+
]
333+
)
334+
partner_ids = partners.ids
335+
partners.unlink()
336+
337+
logs = self.env["spp.audit.log"].search(
338+
[
339+
("model_id", "=", self.partner_model.id),
340+
("method", "=", "unlink"),
341+
("res_id", "in", partner_ids),
342+
]
343+
)
344+
logged_ids = set(logs.mapped("res_id"))
345+
for pid in partner_ids:
346+
self.assertIn(pid, logged_ids)
347+
348+
def test_unlink_record_is_actually_deleted(self):
349+
"""The record should actually be deleted after audit logging."""
350+
partner = self.env["res.partner"].create({"name": "Really Delete"})
351+
partner_id = partner.id
352+
partner.unlink()
353+
354+
self.assertFalse(
355+
self.env["res.partner"].search([("id", "=", partner_id)]),
356+
"Record should be deleted after audit_unlink",
357+
)

0 commit comments

Comments
 (0)