Skip to content

Commit 46a7365

Browse files
refactor(spp_audit): use isinstance for Markup checks, simplify loops
Replace fragile str(type(...)) comparisons with isinstance(value, Markup) and restructure sanitization loops to iterate each record's items directly instead of extracting keys from the first record. Addresses gemini-code-assist review feedback on PR #138.
1 parent dc66d1d commit 46a7365

4 files changed

Lines changed: 23 additions & 17 deletions

File tree

spp_audit/README.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,8 @@ Changelog
152152
create overrides (#138)
153153
- fix: Markup sanitization in audit_write and audit_unlink now handles
154154
all records, not just the first
155+
- refactor: use ``isinstance(value, Markup)`` instead of fragile
156+
``str(type(...))`` comparison in all audit decorator methods
155157

156158
19.0.2.0.0
157159
~~~~~~~~~~

spp_audit/readme/HISTORY.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
- fix: use @api.model_create_multi for audit_create to support Odoo 19 create overrides (#138)
44
- 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
56

67
### 19.0.2.0.0
78

spp_audit/static/description/index.html

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,8 @@ <h1>19.0.2.0.1</h1>
522522
create overrides (#138)</li>
523523
<li>fix: Markup sanitization in audit_write and audit_unlink now handles
524524
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>
525527
</ul>
526528
</div>
527529
<div class="section" id="section-2">

spp_audit/tools/decorator.py

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
import copy
55
import logging
66

7+
from markupsafe import Markup
8+
79
from odoo import api
810

911
_logger = logging.getLogger(__name__)
@@ -35,11 +37,10 @@ def audit_create(self, vals_list):
3537
)
3638
)
3739
if new_values:
38-
keys = new_values[0].keys()
39-
for key in keys:
40-
for nv in new_values:
41-
if str(type(nv[key])) == "<class 'markupsafe.Markup'>":
42-
nv[key] = str(nv[key])
40+
for nv in new_values:
41+
for key, value in nv.items():
42+
if isinstance(value, Markup):
43+
nv[key] = str(value)
4344

4445
rules.log("create", new_values=new_values)
4546
return result
@@ -71,13 +72,14 @@ def audit_write(self, vals):
7172
)
7273

7374
if new_values and old_values_copy:
74-
keys = new_values[0].keys()
75-
for key in keys:
76-
for nv, ov in zip(new_values, old_values_copy, strict=False):
77-
if str(type(nv[key])) == "<class 'markupsafe.Markup'>":
78-
nv[key] = str(nv[key])
79-
if str(type(ov[key])) == "<class 'markupsafe.Markup'>":
80-
ov[key] = str(ov[key])
75+
for nv in new_values:
76+
for key, value in nv.items():
77+
if isinstance(value, Markup):
78+
nv[key] = str(value)
79+
for ov in old_values_copy:
80+
for key, value in ov.items():
81+
if isinstance(value, Markup):
82+
ov[key] = str(value)
8183

8284
rules.log("write", old_values_copy, new_values)
8385
return result
@@ -92,11 +94,10 @@ def audit_unlink(self):
9294
)
9395

9496
if old_values:
95-
keys = old_values[0].keys()
96-
for key in keys:
97-
for ov in old_values:
98-
if str(type(ov[key])) == "<class 'markupsafe.Markup'>":
99-
ov[key] = str(ov[key])
97+
for ov in old_values:
98+
for key, value in ov.items():
99+
if isinstance(value, Markup):
100+
ov[key] = str(value)
100101

101102
rules.log("unlink", old_values)
102103
return audit_unlink.origin(self)

0 commit comments

Comments
 (0)