fix: audit_create uses @api.model breaking model_create_multi overrides#138
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical issue where the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates the audit_create method in spp_audit/tools/decorator.py to handle multiple record creations by switching to @api.model_create_multi and adjusting the processing of new_values accordingly. A review comment suggests improving the robustness and efficiency of the markupsafe.Markup conversion loop by iterating through record items directly and using isinstance for type checking.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 19.0 #138 +/- ##
==========================================
- Coverage 72.25% 72.17% -0.08%
==========================================
Files 898 947 +49
Lines 52637 56253 +3616
==========================================
+ Hits 38032 40602 +2570
- Misses 14605 15651 +1046
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
emjay0921
left a comment
There was a problem hiding this comment.
Approving — the @api.model → @api.model_create_multi fix is correct and needed for Odoo 19. The Markup loop fix for batch records is also good.
One suggestion (non-blocking): consider adding a test that verifies a @api.model_create_multi override on an audited model actually runs its logic. This would prevent regressions if the audit decorator is changed again in the future.
audit_create was decorated with @api.model (single-dict signature) and called origin(self, vals) with a single dict. This bypassed the @api.model_create_multi wrapper on downstream create overrides, causing their logic to be silently skipped when audit rules are active. Changed audit_create to use @api.model_create_multi so it passes vals_list through the origin chain correctly. Also updated the Markup sanitization loop to handle multiple records. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
29a965b to
0530092
Compare
…e/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.
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.
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.
e3ed195 to
46a7365
Compare
Problem
audit_createinspp_audit/tools/decorator.pyis decorated with@api.model(single-dict signature). When it callsaudit_create.origin(self, vals), it passes a single dict to the next function in the origin chain. If that function is decorated with@api.model_create_multi(the standard Odoo 19 pattern forcreate), the wrapper is bypassed becauseorigincalls the raw function directly.This causes any module that overrides
createwith@api.model_create_multion an audited model to have its create logic silently skipped.Impact
createoverrides doesn't runcreatedoesn't runcreatedoesn't runcreateonres.partner(or other audited models) is affectedRoot cause
The monkey-patch in
_register_hookdoes:Because
audit_createwas@api.model, it received a single dict and passed it throughorigin()as a single dict — bypassing the@api.model_create_multiwrapper on downstream overrides.Fix
audit_create: Changed from@api.modelto@api.model_create_multisovals_listflows correctly through the origin chain. Removed deadlong = intalias andisinstance(result, int | long)branch.Markup sanitization in
audit_writeandaudit_unlink: Fixed the same class of bug — the Markup-to-string conversion only processed the first record ([0]), skipping all others in multi-record operations. Now iterates all records, matching the pattern already fixed inaudit_create.Tests added (11 new tests)
TestAuditCreateMulti(3 tests): Multi-create returns all records, each gets an audit log, single-dict create still worksTestAuditWrite(4 tests): Write produces audit log, captures old/new values, batch write logs each record,audit_in_progresscontext prevents duplicate logsTestAuditUnlink(4 tests): Delete produces audit log, logs old values, batch delete logs each record, record is actually deletedAll 30 tests pass. Pre-commit clean.